Merge "Replace revoke tree with linear search"
This commit is contained in:
commit
6589ac6b69
|
@ -16,34 +16,33 @@ from dogpile.cache import proxy
|
|||
from oslo_context import context as oslo_context
|
||||
from oslo_serialization import msgpackutils
|
||||
|
||||
|
||||
from keystone.models import revoke_model
|
||||
|
||||
|
||||
class _RevokeModelHandler(object):
|
||||
class _RevokeEventHandler(object):
|
||||
# NOTE(morganfainberg): There needs to be reserved "registry" entries set
|
||||
# in oslo_serialization for application-specific handlers. We picked 127
|
||||
# here since it's waaaaaay far out before oslo_serialization will use it.
|
||||
identity = 127
|
||||
handles = (revoke_model.RevokeTree,)
|
||||
handles = (revoke_model.RevokeEvent,)
|
||||
|
||||
def __init__(self, registry):
|
||||
self._registry = registry
|
||||
|
||||
def serialize(self, obj):
|
||||
return msgpackutils.dumps(obj.revoke_map,
|
||||
registry=self._registry)
|
||||
return msgpackutils.dumps(obj.__dict__, registry=self._registry)
|
||||
|
||||
def deserialize(self, data):
|
||||
revoke_map = msgpackutils.loads(data, registry=self._registry)
|
||||
revoke_tree = revoke_model.RevokeTree()
|
||||
revoke_tree.revoke_map = revoke_map
|
||||
return revoke_tree
|
||||
revoke_event_data = msgpackutils.loads(data, registry=self._registry)
|
||||
revoke_event = revoke_model.RevokeEvent(**revoke_event_data)
|
||||
return revoke_event
|
||||
|
||||
|
||||
# Register our new handler.
|
||||
_registry = msgpackutils.default_registry
|
||||
_registry.frozen = False
|
||||
_registry.register(_RevokeModelHandler(registry=_registry))
|
||||
_registry.register(_RevokeEventHandler(registry=_registry))
|
||||
_registry.frozen = True
|
||||
|
||||
|
||||
|
|
|
@ -126,133 +126,85 @@ def attr_keys(event):
|
|||
return list(map(event.key_for_name, _EVENT_NAMES))
|
||||
|
||||
|
||||
class RevokeTree(object):
|
||||
"""Fast Revocation Checking Tree Structure.
|
||||
def is_revoked(events, token_data):
|
||||
"""Check if a token matches a revocation event.
|
||||
|
||||
The Tree is an index to quickly match tokens against events.
|
||||
Each node is a hashtable of key=value combinations from revocation events.
|
||||
The
|
||||
Compare a token against every revocation event. If the token matches an
|
||||
event in the `events` list, the token is revoked. If the token is compared
|
||||
against every item in the list without a match, it is not considered
|
||||
revoked from the `revoke_api`.
|
||||
|
||||
:param events: a list of RevokeEvent instances
|
||||
:param token_data: map based on a flattened view of the token. The required
|
||||
fields are `expires_at`,`user_id`, `project_id`,
|
||||
`identity_domain_id`, `assignment_domain_id`,
|
||||
`trust_id`, `trustor_id`, `trustee_id` `consumer_id` and
|
||||
`access_token_id`
|
||||
:returns: True if the token matches an existing revocation event, meaning
|
||||
the token is revoked. False is returned if the token does not
|
||||
match any revocation events, meaning the token is considered
|
||||
valid by the revocation API.
|
||||
"""
|
||||
return any([matches(e, token_data) for e in events])
|
||||
|
||||
def __init__(self, revoke_events=None):
|
||||
self.revoke_map = dict()
|
||||
self.add_events(revoke_events)
|
||||
|
||||
def add_event(self, event):
|
||||
"""Update the tree based on a revocation event.
|
||||
def matches(event, token_values):
|
||||
"""See if the token matches the revocation event.
|
||||
|
||||
Creates any necessary internal nodes in the tree corresponding to the
|
||||
fields of the revocation event. The leaf node will always be set to
|
||||
the latest 'issued_before' for events that are otherwise identical.
|
||||
A brute force approach to checking.
|
||||
Compare each attribute from the event with the corresponding
|
||||
value from the token. If the event does not have a value for
|
||||
the attribute, a match is still possible. If the event has a
|
||||
value for the attribute, and it does not match the token, no match
|
||||
is possible, so skip the remaining checks.
|
||||
|
||||
:param: Event to add to the tree
|
||||
:param event: a RevokeEvent instance
|
||||
:param token_values: dictionary with set of values taken from the
|
||||
token
|
||||
:returns: True if the token matches the revocation event, indicating the
|
||||
token has been revoked
|
||||
"""
|
||||
# If any one check does not match, the whole token does
|
||||
# not match the event. The numerous return False indicate
|
||||
# that the token is still valid and short-circuits the
|
||||
# rest of the logic.
|
||||
|
||||
:returns: the event that was passed in.
|
||||
# The token has three attributes that can match the user_id
|
||||
if event.user_id is not None:
|
||||
if all(event.user_id != token_values[attribute_name]
|
||||
for attribute_name in ['user_id', 'trustor_id', 'trustee_id']):
|
||||
return False
|
||||
|
||||
"""
|
||||
revoke_map = self.revoke_map
|
||||
for key in attr_keys(event):
|
||||
revoke_map = revoke_map.setdefault(key, {})
|
||||
revoke_map['issued_before'] = max(
|
||||
event.issued_before, revoke_map.get(
|
||||
'issued_before', event.issued_before))
|
||||
return event
|
||||
# The token has two attributes that can match the domain_id
|
||||
if event.domain_id is not None:
|
||||
if all(event.domain_id != token_values[attribute_name]
|
||||
for attribute_name in ['identity_domain_id',
|
||||
'assignment_domain_id']):
|
||||
return False
|
||||
|
||||
def remove_event(self, event):
|
||||
"""Update the tree based on the removal of a Revocation Event.
|
||||
if event.domain_scope_id is not None:
|
||||
if event.domain_scope_id != token_values['assignment_domain_id']:
|
||||
return False
|
||||
|
||||
Removes empty nodes from the tree from the leaf back to the root.
|
||||
# If an event specifies an attribute name, but it does not match,
|
||||
# the token is not revoked.
|
||||
attribute_names = ['project_id',
|
||||
'expires_at', 'trust_id', 'consumer_id',
|
||||
'access_token_id', 'audit_id', 'audit_chain_id']
|
||||
for attribute_name in attribute_names:
|
||||
if getattr(event, attribute_name) is not None:
|
||||
if (getattr(event, attribute_name) !=
|
||||
token_values[attribute_name]):
|
||||
return False
|
||||
|
||||
If multiple events trace the same path, but have different
|
||||
'issued_before' values, only the last is ever stored in the tree.
|
||||
So only an exact match on 'issued_before' ever triggers a removal
|
||||
if event.role_id is not None:
|
||||
roles = token_values['roles']
|
||||
if all(event.role_id != role for role in roles):
|
||||
return False
|
||||
|
||||
:param: Event to remove from the tree
|
||||
|
||||
"""
|
||||
stack = []
|
||||
revoke_map = self.revoke_map
|
||||
for name in _EVENT_NAMES:
|
||||
key = event.key_for_name(name)
|
||||
nxt = revoke_map.get(key)
|
||||
if nxt is None:
|
||||
break
|
||||
stack.append((revoke_map, key, nxt))
|
||||
revoke_map = nxt
|
||||
else:
|
||||
if event.issued_before == revoke_map['issued_before']:
|
||||
revoke_map.pop('issued_before')
|
||||
for parent, key, child in reversed(stack):
|
||||
if not any(child):
|
||||
del parent[key]
|
||||
|
||||
def add_events(self, revoke_events):
|
||||
return list(map(self.add_event, revoke_events or []))
|
||||
|
||||
@staticmethod
|
||||
def _next_level_keys(name, token_data):
|
||||
"""Generate keys based on current field name and token data.
|
||||
|
||||
Generate all keys to look for in the next iteration of revocation
|
||||
event tree traversal.
|
||||
"""
|
||||
yield '*'
|
||||
if name == 'role_id':
|
||||
# Roles are very special since a token has a list of them.
|
||||
# If the revocation event matches any one of them,
|
||||
# revoke the token.
|
||||
for role_id in token_data.get('roles', []):
|
||||
yield role_id
|
||||
else:
|
||||
# For other fields we try to get any branch that concur
|
||||
# with any alternative field in the token.
|
||||
for alt_name in ALTERNATIVES.get(name, [name]):
|
||||
yield token_data[alt_name]
|
||||
|
||||
def _search(self, revoke_map, names, token_data):
|
||||
"""Search for revocation event by token_data.
|
||||
|
||||
Traverse the revocation events tree looking for event matching token
|
||||
data issued after the token.
|
||||
"""
|
||||
if not names:
|
||||
# The last (leaf) level is checked in a special way because we
|
||||
# verify issued_at field differently.
|
||||
try:
|
||||
return revoke_map['issued_before'] >= token_data['issued_at']
|
||||
except KeyError:
|
||||
return False
|
||||
|
||||
name, remaining_names = names[0], names[1:]
|
||||
|
||||
for key in self._next_level_keys(name, token_data):
|
||||
subtree = revoke_map.get('%s=%s' % (name, key))
|
||||
if subtree and self._search(subtree, remaining_names, token_data):
|
||||
return True
|
||||
|
||||
# If we made it out of the loop then no element in revocation tree
|
||||
# corresponds to our token and it is good.
|
||||
if token_values['issued_at'] > event.issued_before:
|
||||
return False
|
||||
|
||||
def is_revoked(self, token_data):
|
||||
"""Check if a token matches the revocation event.
|
||||
|
||||
Compare the values for each level of the tree with the values from
|
||||
the token, accounting for attributes that have alternative
|
||||
keys, and for wildcard matches.
|
||||
if there is a match, continue down the tree.
|
||||
if there is no match, exit early.
|
||||
|
||||
token_data is a map based on a flattened view of token.
|
||||
The required fields are:
|
||||
|
||||
'expires_at','user_id', 'project_id', 'identity_domain_id',
|
||||
'assignment_domain_id', 'trust_id', 'trustor_id', 'trustee_id'
|
||||
'consumer_id', 'access_token_id'
|
||||
|
||||
"""
|
||||
return self._search(self.revoke_map, _EVENT_NAMES, token_data)
|
||||
return True
|
||||
|
||||
|
||||
def build_token_values_v2(access, default_domain_id):
|
||||
|
|
|
@ -12,6 +12,7 @@
|
|||
|
||||
"""Main entry point into the Revoke service."""
|
||||
|
||||
import oslo_cache
|
||||
from oslo_config import cfg
|
||||
from oslo_log import versionutils
|
||||
|
||||
|
@ -47,7 +48,13 @@ EXTENSION_DATA = {
|
|||
extension.register_admin_extension(EXTENSION_DATA['alias'], EXTENSION_DATA)
|
||||
extension.register_public_extension(EXTENSION_DATA['alias'], EXTENSION_DATA)
|
||||
|
||||
MEMOIZE = cache.get_memoization_decorator(group='revoke')
|
||||
# This builds a discrete cache region dedicated to revoke events. The API can
|
||||
# return a filtered list based upon last fetchtime. This is deprecated but
|
||||
# must be maintained.
|
||||
REVOKE_REGION = oslo_cache.create_region()
|
||||
MEMOIZE = cache.get_memoization_decorator(
|
||||
group='revoke',
|
||||
region=REVOKE_REGION)
|
||||
|
||||
|
||||
@dependency.provider('revoke_api')
|
||||
|
@ -68,6 +75,13 @@ class Manager(manager.Manager):
|
|||
self._register_listeners()
|
||||
self.model = revoke_model
|
||||
|
||||
@MEMOIZE
|
||||
def _list_events(self, last_fetch):
|
||||
return self.driver.list_events(last_fetch)
|
||||
|
||||
def list_events(self, last_fetch=None):
|
||||
return self._list_events(last_fetch)
|
||||
|
||||
def _user_callback(self, service, resource_type, operation,
|
||||
payload):
|
||||
self.revoke_by_user(payload['resource_info'])
|
||||
|
@ -180,13 +194,6 @@ class Manager(manager.Manager):
|
|||
self.revoke(revoke_model.RevokeEvent(domain_id=domain_id,
|
||||
role_id=role_id))
|
||||
|
||||
@MEMOIZE
|
||||
def _get_revoke_tree(self):
|
||||
events = self.driver.list_events()
|
||||
revoke_tree = revoke_model.RevokeTree(revoke_events=events)
|
||||
|
||||
return revoke_tree
|
||||
|
||||
def check_token(self, token_values):
|
||||
"""Check the values from a token against the revocation list.
|
||||
|
||||
|
@ -197,12 +204,12 @@ class Manager(manager.Manager):
|
|||
:raises keystone.exception.TokenNotFound: If the token is invalid.
|
||||
|
||||
"""
|
||||
if self._get_revoke_tree().is_revoked(token_values):
|
||||
if revoke_model.is_revoked(self.list_events(), token_values):
|
||||
raise exception.TokenNotFound(_('Failed to validate token'))
|
||||
|
||||
def revoke(self, event):
|
||||
self.driver.revoke(event)
|
||||
self._get_revoke_tree.invalidate(self)
|
||||
REVOKE_REGION.invalidate()
|
||||
|
||||
|
||||
@versionutils.deprecated(
|
||||
|
|
|
@ -38,6 +38,9 @@ def load_backends():
|
|||
cache.apply_invalidation_patch(
|
||||
region=assignment.COMPUTED_ASSIGNMENTS_REGION,
|
||||
region_name=assignment.COMPUTED_ASSIGNMENTS_REGION.name)
|
||||
cache.configure_cache(region=revoke.REVOKE_REGION)
|
||||
cache.apply_invalidation_patch(region=revoke.REVOKE_REGION,
|
||||
region_name=revoke.REVOKE_REGION.name)
|
||||
|
||||
# Ensure that the identity driver is created before the assignment manager
|
||||
# and that the assignment driver is created before the resource manager.
|
||||
|
|
|
@ -15,9 +15,11 @@ import fixtures
|
|||
|
||||
from keystone import catalog
|
||||
from keystone.common import cache
|
||||
from keystone import revoke
|
||||
|
||||
|
||||
CACHE_REGIONS = (cache.CACHE_REGION, catalog.COMPUTED_CATALOG_REGION)
|
||||
CACHE_REGIONS = (cache.CACHE_REGION, catalog.COMPUTED_CATALOG_REGION,
|
||||
revoke.REVOKE_REGION)
|
||||
|
||||
|
||||
class Cache(fixtures.Fixture):
|
||||
|
|
|
@ -171,11 +171,22 @@ class SqlRevokeTests(test_backend_sql.SqlTests, RevokeTests):
|
|||
revoke_by_id=False)
|
||||
|
||||
|
||||
class RevokeTreeTests(unit.TestCase):
|
||||
def add_event(events, event):
|
||||
events.append(event)
|
||||
return event
|
||||
|
||||
|
||||
def remove_event(events, event):
|
||||
for target in events:
|
||||
if target == event:
|
||||
events.remove(target)
|
||||
|
||||
|
||||
class RevokeListTests(unit.TestCase):
|
||||
def setUp(self):
|
||||
super(RevokeTreeTests, self).setUp()
|
||||
super(RevokeListTests, self).setUp()
|
||||
self.events = []
|
||||
self.tree = revoke_model.RevokeTree()
|
||||
self.revoke_events = list()
|
||||
self._sample_data()
|
||||
|
||||
def _sample_data(self):
|
||||
|
@ -219,27 +230,32 @@ class RevokeTreeTests(unit.TestCase):
|
|||
|
||||
def _assertTokenRevoked(self, token_data):
|
||||
self.assertTrue(any([_matches(e, token_data) for e in self.events]))
|
||||
return self.assertTrue(self.tree.is_revoked(token_data),
|
||||
'Token should be revoked')
|
||||
return self.assertTrue(
|
||||
revoke_model.is_revoked(self.revoke_events, token_data),
|
||||
'Token should be revoked')
|
||||
|
||||
def _assertTokenNotRevoked(self, token_data):
|
||||
self.assertFalse(any([_matches(e, token_data) for e in self.events]))
|
||||
return self.assertFalse(self.tree.is_revoked(token_data),
|
||||
'Token should not be revoked')
|
||||
return self.assertFalse(
|
||||
revoke_model.is_revoked(self.revoke_events, token_data),
|
||||
'Token should not be revoked')
|
||||
|
||||
def _revoke_by_user(self, user_id):
|
||||
return self.tree.add_event(
|
||||
return add_event(
|
||||
self.revoke_events,
|
||||
revoke_model.RevokeEvent(user_id=user_id))
|
||||
|
||||
def _revoke_by_audit_id(self, audit_id):
|
||||
event = self.tree.add_event(
|
||||
event = add_event(
|
||||
self.revoke_events,
|
||||
revoke_model.RevokeEvent(audit_id=audit_id))
|
||||
self.events.append(event)
|
||||
return event
|
||||
|
||||
def _revoke_by_audit_chain_id(self, audit_chain_id, project_id=None,
|
||||
domain_id=None):
|
||||
event = self.tree.add_event(
|
||||
event = add_event(
|
||||
self.revoke_events,
|
||||
revoke_model.RevokeEvent(audit_chain_id=audit_chain_id,
|
||||
project_id=project_id,
|
||||
domain_id=domain_id)
|
||||
|
@ -249,7 +265,8 @@ class RevokeTreeTests(unit.TestCase):
|
|||
|
||||
def _revoke_by_expiration(self, user_id, expires_at, project_id=None,
|
||||
domain_id=None):
|
||||
event = self.tree.add_event(
|
||||
event = add_event(
|
||||
self.revoke_events,
|
||||
revoke_model.RevokeEvent(user_id=user_id,
|
||||
expires_at=expires_at,
|
||||
project_id=project_id,
|
||||
|
@ -259,7 +276,8 @@ class RevokeTreeTests(unit.TestCase):
|
|||
|
||||
def _revoke_by_grant(self, role_id, user_id=None,
|
||||
domain_id=None, project_id=None):
|
||||
event = self.tree.add_event(
|
||||
event = add_event(
|
||||
self.revoke_events,
|
||||
revoke_model.RevokeEvent(user_id=user_id,
|
||||
role_id=role_id,
|
||||
domain_id=domain_id,
|
||||
|
@ -268,29 +286,29 @@ class RevokeTreeTests(unit.TestCase):
|
|||
return event
|
||||
|
||||
def _revoke_by_user_and_project(self, user_id, project_id):
|
||||
event = self.tree.add_event(
|
||||
revoke_model.RevokeEvent(project_id=project_id,
|
||||
user_id=user_id))
|
||||
event = add_event(self.revoke_events,
|
||||
revoke_model.RevokeEvent(project_id=project_id,
|
||||
user_id=user_id))
|
||||
self.events.append(event)
|
||||
return event
|
||||
|
||||
def _revoke_by_project_role_assignment(self, project_id, role_id):
|
||||
event = self.tree.add_event(
|
||||
revoke_model.RevokeEvent(project_id=project_id,
|
||||
role_id=role_id))
|
||||
event = add_event(self.revoke_events,
|
||||
revoke_model.RevokeEvent(project_id=project_id,
|
||||
role_id=role_id))
|
||||
self.events.append(event)
|
||||
return event
|
||||
|
||||
def _revoke_by_domain_role_assignment(self, domain_id, role_id):
|
||||
event = self.tree.add_event(
|
||||
revoke_model.RevokeEvent(domain_id=domain_id,
|
||||
role_id=role_id))
|
||||
event = add_event(self.revoke_events,
|
||||
revoke_model.RevokeEvent(domain_id=domain_id,
|
||||
role_id=role_id))
|
||||
self.events.append(event)
|
||||
return event
|
||||
|
||||
def _revoke_by_domain(self, domain_id):
|
||||
event = self.tree.add_event(
|
||||
revoke_model.RevokeEvent(domain_id=domain_id))
|
||||
event = add_event(self.revoke_events,
|
||||
revoke_model.RevokeEvent(domain_id=domain_id))
|
||||
self.events.append(event)
|
||||
|
||||
def _user_field_test(self, field_name):
|
||||
|
@ -303,7 +321,7 @@ class RevokeTreeTests(unit.TestCase):
|
|||
token_data_u2 = _sample_blank_token()
|
||||
token_data_u2[field_name] = _new_id()
|
||||
self._assertTokenNotRevoked(token_data_u2)
|
||||
self.tree.remove_event(event)
|
||||
remove_event(self.revoke_events, event)
|
||||
self.events.remove(event)
|
||||
self._assertTokenNotRevoked(token_data_u1)
|
||||
|
||||
|
@ -357,7 +375,7 @@ class RevokeTreeTests(unit.TestCase):
|
|||
|
||||
def remove_event(self, event):
|
||||
self.events.remove(event)
|
||||
self.tree.remove_event(event)
|
||||
remove_event(self.revoke_events, event)
|
||||
|
||||
def test_by_project_grant(self):
|
||||
token_to_revoke = self.token_to_revoke
|
||||
|
@ -476,64 +494,10 @@ class RevokeTreeTests(unit.TestCase):
|
|||
def _assertEmpty(self, collection):
|
||||
return self.assertEqual(0, len(collection), "collection not empty")
|
||||
|
||||
def _assertEventsMatchIteration(self, turn):
|
||||
self.assertEqual(1, len(self.tree.revoke_map))
|
||||
self.assertEqual(turn + 1, len(self.tree.revoke_map
|
||||
['trust_id=*']
|
||||
['consumer_id=*']
|
||||
['access_token_id=*']
|
||||
['audit_id=*']
|
||||
['audit_chain_id=*']))
|
||||
# two different functions add domain_ids, +1 for None
|
||||
self.assertEqual(2 * turn + 1, len(self.tree.revoke_map
|
||||
['trust_id=*']
|
||||
['consumer_id=*']
|
||||
['access_token_id=*']
|
||||
['audit_id=*']
|
||||
['audit_chain_id=*']
|
||||
['expires_at=*']))
|
||||
# two different functions add project_ids, +1 for None
|
||||
self.assertEqual(2 * turn + 1, len(self.tree.revoke_map
|
||||
['trust_id=*']
|
||||
['consumer_id=*']
|
||||
['access_token_id=*']
|
||||
['audit_id=*']
|
||||
['audit_chain_id=*']
|
||||
['expires_at=*']
|
||||
['domain_id=*']))
|
||||
# 10 users added
|
||||
self.assertEqual(turn, len(self.tree.revoke_map
|
||||
['trust_id=*']
|
||||
['consumer_id=*']
|
||||
['access_token_id=*']
|
||||
['audit_id=*']
|
||||
['audit_chain_id=*']
|
||||
['expires_at=*']
|
||||
['domain_id=*']
|
||||
['project_id=*']))
|
||||
|
||||
def test_cleanup(self):
|
||||
events = self.events
|
||||
self._assertEmpty(self.tree.revoke_map)
|
||||
expiry_base_time = _future_time()
|
||||
self._assertEmpty(self.revoke_events)
|
||||
for i in range(0, 10):
|
||||
events.append(
|
||||
self._revoke_by_user(_new_id()))
|
||||
|
||||
args = (_new_id(),
|
||||
expiry_base_time + datetime.timedelta(seconds=i))
|
||||
events.append(
|
||||
self._revoke_by_expiration(*args))
|
||||
|
||||
self.assertEqual(i + 2, len(self.tree.revoke_map
|
||||
['trust_id=*']
|
||||
['consumer_id=*']
|
||||
['access_token_id=*']
|
||||
['audit_id=*']
|
||||
['audit_chain_id=*']),
|
||||
'adding %s to %s' % (args,
|
||||
self.tree.revoke_map))
|
||||
|
||||
events.append(
|
||||
self._revoke_by_project_role_assignment(_new_id(), _new_id()))
|
||||
events.append(
|
||||
|
@ -542,8 +506,7 @@ class RevokeTreeTests(unit.TestCase):
|
|||
self._revoke_by_domain_role_assignment(_new_id(), _new_id()))
|
||||
events.append(
|
||||
self._revoke_by_user_and_project(_new_id(), _new_id()))
|
||||
self._assertEventsMatchIteration(i + 1)
|
||||
|
||||
for event in self.events:
|
||||
self.tree.remove_event(event)
|
||||
self._assertEmpty(self.tree.revoke_map)
|
||||
remove_event(self.revoke_events, event)
|
||||
self._assertEmpty(self.revoke_events)
|
||||
|
|
Loading…
Reference in New Issue