diff --git a/keystone/common/cache/_context_cache.py b/keystone/common/cache/_context_cache.py index 3895ca1f68..397a89ae83 100644 --- a/keystone/common/cache/_context_cache.py +++ b/keystone/common/cache/_context_cache.py @@ -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 diff --git a/keystone/models/revoke_model.py b/keystone/models/revoke_model.py index 27f7113fa0..87a10b2608 100644 --- a/keystone/models/revoke_model.py +++ b/keystone/models/revoke_model.py @@ -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): diff --git a/keystone/revoke/core.py b/keystone/revoke/core.py index f57aade095..7e92bd161d 100644 --- a/keystone/revoke/core.py +++ b/keystone/revoke/core.py @@ -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']) @@ -194,13 +208,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. @@ -211,12 +218,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( diff --git a/keystone/server/backends.py b/keystone/server/backends.py index a518e7771c..34d8a593b7 100644 --- a/keystone/server/backends.py +++ b/keystone/server/backends.py @@ -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. diff --git a/keystone/tests/unit/ksfixtures/cache.py b/keystone/tests/unit/ksfixtures/cache.py index e0833ae2ee..86293a5f31 100644 --- a/keystone/tests/unit/ksfixtures/cache.py +++ b/keystone/tests/unit/ksfixtures/cache.py @@ -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): diff --git a/keystone/tests/unit/test_revoke.py b/keystone/tests/unit/test_revoke.py index 82c0125a00..b94e851927 100644 --- a/keystone/tests/unit/test_revoke.py +++ b/keystone/tests/unit/test_revoke.py @@ -190,11 +190,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): @@ -238,27 +249,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) @@ -268,7 +284,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, @@ -278,7 +295,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, @@ -287,29 +305,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): @@ -322,7 +340,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) @@ -430,7 +448,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 @@ -549,64 +567,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( @@ -615,8 +579,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)