From c7658abfd644477e1adfbf1c1ab342f0dcd869e6 Mon Sep 17 00:00:00 2001 From: Lance Bragstad Date: Wed, 14 Feb 2018 19:40:17 +0000 Subject: [PATCH] Simplify token persistence callbacks The INVALIDATE_USER_TOKEN_PERSISTENCE and INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE callbacks were meant to clean up invalid tokens from the token storage layer. Now that the sql token driver has been removed, we don't need them any more. This commit removes those notifications and refactors the places where notifications are still needed, making them more specific and not eluding to token persistence. This commit also removes a significant amount of logic from the assignment API that used to notify the token API when assignments were deleted. This made sense when tokens were written to disk because there was an opportunity to invalidate them when users were removed from projects. This is no longer needed since we do validation online and we don't persist tokens anymore. Change-Id: I100b7416e8ba61eb4ea2c2eb4962e952a53ea388 --- keystone/application_credential/core.py | 4 +- keystone/assignment/core.py | 164 +++++++++--------------- keystone/identity/core.py | 41 +++--- keystone/notifications.py | 31 ++++- keystone/resource/core.py | 20 +-- keystone/revoke/core.py | 2 +- keystone/token/provider.py | 6 +- 7 files changed, 120 insertions(+), 148 deletions(-) diff --git a/keystone/application_credential/core.py b/keystone/application_credential/core.py index a3e62959a6..8b03e18c76 100644 --- a/keystone/application_credential/core.py +++ b/keystone/application_credential/core.py @@ -57,9 +57,7 @@ class Manager(manager.Manager): self._delete_app_creds_on_user_delete_callback) notifications.register_event_callback( notifications.ACTIONS.internal, - # This notification is emitted when a role assignment is removed, - # we can take advantage of it even though we're not a token. - notifications.INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE, + notifications.REMOVE_APP_CREDS_FOR_USER, self._delete_app_creds_on_assignment_removal) def _delete_app_creds_on_user_delete_callback( diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index e377b21c28..1f5fb7de38 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -93,6 +93,23 @@ class Manager(manager.Manager): # Use set() to process the list to remove any duplicates return list(set([x['user_id'] for x in assignment_list])) + def _send_app_cred_notification_for_role_removal(self, role_id): + """Delete all application credential for a specific role. + + :param role_id: role identifier + :type role_id: string + """ + assignments = self.list_role_assignments(role_id=role_id) + for assignment in assignments: + if 'user_id' in assignment and 'project_id' in assignment: + payload = { + 'user_id': assignment['user_id'], + 'project_id': assignment['project_id'] + } + notifications.Audit.internal( + notifications.REMOVE_APP_CREDS_FOR_USER, payload + ) + @MEMOIZE_COMPUTED_ASSIGNMENTS def get_roles_for_user_and_project(self, user_id, tenant_id): """Get the roles associated with a user within given project. @@ -241,32 +258,45 @@ class Manager(manager.Manager): self.driver.remove_role_from_user_and_project(user_id, project_id, role_id) - if project_id: - self._emit_invalidate_grant_token_persistence(user_id, project_id) - else: - PROVIDERS.identity_api.emit_invalidate_user_token_persistence( - user_id) + payload = {'user_id': user_id, 'project_id': project_id} + notifications.Audit.internal( + notifications.REMOVE_APP_CREDS_FOR_USER, + payload + ) + self._invalidate_token_cache( + role_id, group_id, user_id, project_id, domain_id + ) def remove_role_from_user_and_project(self, user_id, tenant_id, role_id): self._remove_role_from_user_and_project_adapter( role_id, user_id=user_id, project_id=tenant_id) COMPUTED_ASSIGNMENTS_REGION.invalidate() - def _emit_invalidate_user_token_persistence(self, user_id): - PROVIDERS.identity_api.emit_invalidate_user_token_persistence(user_id) + def _invalidate_token_cache(self, role_id, group_id, user_id, project_id, + domain_id): + if group_id: + actor_type = 'group' + actor_id = group_id + elif user_id: + actor_type = 'user' + actor_id = user_id - # NOTE(lbragstad): The previous notification decorator behavior didn't - # send the notification unless the operation was successful. We - # maintain that behavior here by calling to the notification module - # after the call to emit invalid user tokens. - notifications.Audit.internal( - notifications.INVALIDATE_USER_TOKEN_PERSISTENCE, user_id - ) + if domain_id: + target_type = 'domain' + target_id = domain_id + elif project_id: + target_type = 'project' + target_id = project_id - def _emit_invalidate_grant_token_persistence(self, user_id, project_id): - PROVIDERS.identity_api.emit_invalidate_grant_token_persistence( - {'user_id': user_id, 'project_id': project_id} + reason = ( + 'Invalidating the token cache because role %(role_id)s was ' + 'removed from %(actor_type)s %(actor_id)s on %(target_type)s ' + '%(target_id)s.' % + {'role_id': role_id, 'actor_type': actor_type, + 'actor_id': actor_id, 'target_type': target_type, + 'target_id': target_id} ) + notifications.invalidate_token_cache_notification(reason) @notifications.role_assignment('created') def create_grant(self, role_id, user_id=None, group_id=None, @@ -318,10 +348,6 @@ class Manager(manager.Manager): ) return PROVIDERS.role_api.list_roles_from_ids(grant_ids) - def _emit_revoke_user_grant(self, role_id, user_id, domain_id, project_id, - inherited_to_projects, context): - self._emit_invalidate_grant_token_persistence(user_id, project_id) - @notifications.role_assignment('deleted') def delete_grant(self, role_id, user_id=None, group_id=None, domain_id=None, project_id=None, @@ -337,9 +363,9 @@ class Manager(manager.Manager): project_id=project_id, inherited_to_projects=inherited_to_projects ) - self._emit_revoke_user_grant( - role_id, user_id, domain_id, project_id, - inherited_to_projects, context) + self._invalidate_token_cache( + role_id, group_id, user_id, project_id, domain_id + ) else: try: # check if role exists on the group before revoke @@ -349,13 +375,9 @@ class Manager(manager.Manager): inherited_to_projects=inherited_to_projects ) if CONF.token.revoke_by_id: - # NOTE(morganfainberg): The user ids are the important part - # for invalidating tokens below, so extract them here. - for user in PROVIDERS.identity_api.list_users_in_group( - group_id): - self._emit_revoke_user_grant( - role_id, user['id'], domain_id, project_id, - inherited_to_projects, context) + self._invalidate_token_cache( + role_id, group_id, user_id, project_id, domain_id + ) except exception.GroupNotFound: LOG.debug('Group %s not found, no tokens to invalidate.', group_id) @@ -1053,75 +1075,6 @@ class Manager(manager.Manager): for assignment in system_assignments: self.delete_system_grant_for_group(group_id, assignment['id']) - def delete_tokens_for_role_assignments(self, role_id): - assignments = self.list_role_assignments(role_id=role_id) - - # Iterate over the assignments for this role and build the list of - # user or user+project IDs for the tokens we need to delete - user_ids = set() - user_and_project_ids = list() - for assignment in assignments: - # If we have a project assignment, then record both the user and - # project IDs so we can target the right token to delete. If it is - # a domain assignment, we might as well kill all the tokens for - # the user, since in the vast majority of cases all the tokens - # for a user will be within one domain anyway, so not worth - # trying to delete tokens for each project in the domain. If the - # assignment is a system assignment, invalidate all tokens from the - # cache. A future patch may optimize this to only remove specific - # system-scoped tokens from the cache. - if 'user_id' in assignment: - if 'project_id' in assignment: - user_and_project_ids.append( - (assignment['user_id'], assignment['project_id'])) - elif 'domain_id' or 'system' in assignment: - self._emit_invalidate_user_token_persistence( - assignment['user_id']) - elif 'group_id' in assignment: - # Add in any users for this group, being tolerant of any - # cross-driver database integrity errors. - try: - users = PROVIDERS.identity_api.list_users_in_group( - assignment['group_id']) - except exception.GroupNotFound: - # Ignore it, but log a debug message - if 'project_id' in assignment: - target = _('Project (%s)') % assignment['project_id'] - elif 'domain_id' in assignment: - target = _('Domain (%s)') % assignment['domain_id'] - else: - target = _('Unknown Target') - msg = ('Group (%(group)s), referenced in assignment ' - 'for %(target)s, not found - ignoring.') - LOG.debug(msg, {'group': assignment['group_id'], - 'target': target}) - continue - - if 'project_id' in assignment: - for user in users: - user_and_project_ids.append( - (user['id'], assignment['project_id'])) - elif 'domain_id' or 'system' in assignment: - for user in users: - self._emit_invalidate_user_token_persistence( - user['id']) - - # Now process the built up lists. Before issuing calls to delete any - # tokens, let's try and minimize the number of calls by pruning out - # any user+project deletions where a general token deletion for that - # same user is also planned. - user_and_project_ids_to_action = [] - for user_and_project_id in user_and_project_ids: - if user_and_project_id[0] not in user_ids: - user_and_project_ids_to_action.append(user_and_project_id) - - for user_id, project_id in user_and_project_ids_to_action: - payload = {'user_id': user_id, 'project_id': project_id} - notifications.Audit.internal( - notifications.INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE, - payload - ) - def delete_user_assignments(self, user_id): # FIXME(lbragstad): This should be refactored in the Rocky release so # that we can pass the user_id to the system assignment backend like we @@ -1355,11 +1308,20 @@ class RoleManager(manager.Manager): return ret def delete_role(self, role_id, initiator=None): - PROVIDERS.assignment_api.delete_tokens_for_role_assignments(role_id) PROVIDERS.assignment_api.delete_role_assignments(role_id) + PROVIDERS.assignment_api._send_app_cred_notification_for_role_removal( + role_id + ) self.driver.delete_role(role_id) notifications.Audit.deleted(self._ROLE, role_id, initiator) self.get_role.invalidate(self, role_id) + reason = ( + 'Invalidating the token cache because role %(role_id)s has been ' + 'removed. Role assignments for users will be recalculated and ' + 'enforced accordingly the next time they authenticate or validate ' + 'a token' % {'role_id': role_id} + ) + notifications.invalidate_token_cache_notification(reason) COMPUTED_ASSIGNMENTS_REGION.invalidate() # TODO(ayoung): Add notification diff --git a/keystone/identity/core.py b/keystone/identity/core.py index ccc2f9ee14..d7d0bf47e6 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -1128,7 +1128,14 @@ class Manager(manager.Manager): enabled_change = ((user.get('enabled') is False) and user['enabled'] != old_user_ref.get('enabled')) if enabled_change or user.get('password') is not None: - self.emit_invalidate_user_token_persistence(user_id) + self._persist_revocation_event_for_user(user_id) + reason = ( + 'Invalidating the token cache because user %(user_id)s was ' + 'enabled or disabled. Authorization will be calculated and ' + 'enforced accordingly the next time they authenticate or ' + 'validate a token.' % {'user_id': user_id} + ) + notifications.invalidate_token_cache_notification(reason) return self._set_domain_id_and_mapping( ref, domain_id, driver, mapping.EntityType.USER) @@ -1231,8 +1238,8 @@ class Manager(manager.Manager): # assignment for the group then we do not need to revoke all the users # tokens and can just delete the group. if roles: - for uid in user_ids: - self.emit_invalidate_user_token_persistence(uid) + for user_id in user_ids: + self._persist_revocation_event_for_user(user_id) # Invalidate user role assignments cache region, as it may be caching # role assignments expanded from the specified group to its users @@ -1281,7 +1288,7 @@ class Manager(manager.Manager): user_entity_id, user_driver, group_entity_id, group_driver) group_driver.remove_user_from_group(user_entity_id, group_entity_id) - self.emit_invalidate_user_token_persistence(user_id) + self._persist_revocation_event_for_user(user_id) # Invalidate user role assignments cache region, as it may be caching # role assignments expanded from this group to this user @@ -1289,31 +1296,17 @@ class Manager(manager.Manager): notifications.Audit.removed_from(self._GROUP, group_id, self._USER, user_id, initiator) - def emit_invalidate_user_token_persistence(self, user_id): - """Emit a notification to the callback system to revoke user tokens. + def _persist_revocation_event_for_user(self, user_id): + """Emit a notification to invoke a revocation event callback. - This method and associated callback listener removes the need for - making a direct call to another manager to delete and revoke tokens. + Fire off an internal notification that will be consumed by the + revocation API to store a revocation record for a specific user. :param user_id: user identifier :type user_id: string """ notifications.Audit.internal( - notifications.INVALIDATE_USER_TOKEN_PERSISTENCE, user_id - ) - - def emit_invalidate_grant_token_persistence(self, user_project): - """Emit a notification to the callback system to revoke grant tokens. - - This method and associated callback listener removes the need for - making a direct call to another manager to delete and revoke tokens. - - :param user_project: {'user_id': user_id, 'project_id': project_id} - :type user_project: dict - """ - notifications.Audit.internal( - notifications.INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE, - user_project + notifications.PERSIST_REVOCATION_EVENT_FOR_USER, user_id ) @domains_configured @@ -1408,7 +1401,7 @@ class Manager(manager.Manager): raise notifications.Audit.updated(self._USER, user_id, initiator) - self.emit_invalidate_user_token_persistence(user_id) + self._persist_revocation_event_for_user(user_id) @MEMOIZE def _shadow_nonlocal_user(self, user): diff --git a/keystone/notifications.py b/keystone/notifications.py index cea86fc131..fe74478d9a 100644 --- a/keystone/notifications.py +++ b/keystone/notifications.py @@ -76,8 +76,9 @@ CONF = keystone.conf.CONF # NOTE(morganfainberg): Special case notifications that are only used # internally for handling token persistence token deletions -INVALIDATE_USER_TOKEN_PERSISTENCE = 'invalidate_user_tokens' -INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE = 'invalidate_user_project_tokens' +INVALIDATE_TOKEN_CACHE = 'invalidate_token_cache' +PERSIST_REVOCATION_EVENT_FOR_USER = 'persist_revocation_event_for_user' +REMOVE_APP_CREDS_FOR_USER = 'remove_application_credentials_for_user' INVALIDATE_USER_OAUTH_CONSUMER_TOKENS = 'invalidate_user_consumer_tokens' INVALIDATE_TOKEN_CACHE_DELETED_IDP = 'invalidate_token_cache_from_deleted_idp' DOMAIN_DELETED = 'domain_deleted' @@ -180,6 +181,32 @@ class Audit(object): public, reason) +def invalidate_token_cache_notification(reason): + """A specific notification for invalidating the token cache. + + :param reason: The specific reason why the token cache is being + invalidated. + :type reason: string + + """ + # Since keystone does a lot of work in the authentication and validation + # process to make sure the authorization context for the user is + # update-to-date, invalidating the token cache is a somewhat common + # operation. It's done across various subsystems when role assignments + # change, users are disabled, identity providers deleted or disabled, etc.. + # This notification is meant to make the process of invalidating the token + # cache DRY, instead of have each subsystem implement their own token cache + # invalidation strategy or callbacks. + LOG.debug(reason) + resource_id = None + initiator = None + public = False + Audit._emit( + ACTIONS.internal, INVALIDATE_TOKEN_CACHE, resource_id, initiator, + public, reason=reason + ) + + def _get_callback_info(callback): """Return list containing callback's module and name. diff --git a/keystone/resource/core.py b/keystone/resource/core.py index e4ada76510..c7006d2480 100644 --- a/keystone/resource/core.py +++ b/keystone/resource/core.py @@ -427,16 +427,6 @@ class Manager(manager.Manager): return ret - def _pre_delete_cleanup_project(self, project_id): - project_user_ids = ( - PROVIDERS.assignment_api.list_user_ids_for_project(project_id)) - for user_id in project_user_ids: - payload = {'user_id': user_id, 'project_id': project_id} - notifications.Audit.internal( - notifications.INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE, - payload - ) - def _post_delete_cleanup_project(self, project_id, project, initiator=None): try: @@ -501,16 +491,20 @@ class Manager(manager.Manager): project_list = subtree_list + [project] projects_ids = [x['id'] for x in project_list] - for prj in project_list: - self._pre_delete_cleanup_project(prj['id']) ret = self.driver.delete_projects_from_ids(projects_ids) for prj in project_list: self._post_delete_cleanup_project(prj['id'], prj, initiator) else: - self._pre_delete_cleanup_project(project_id) ret = self.driver.delete_project(project_id) self._post_delete_cleanup_project(project_id, project, initiator) + reason = ( + 'The token cache is being invalidate because project ' + '%(project_id)s was deleted. Authorization will be recalculated ' + 'and enforced accordingly the next time users authenticate or ' + 'validate a token.' % {'project_id': project_id} + ) + notifications.invalidate_token_cache_notification(reason) return ret def _filter_projects_list(self, projects_list, user_id): diff --git a/keystone/revoke/core.py b/keystone/revoke/core.py index 67765338a9..c9f16a40de 100644 --- a/keystone/revoke/core.py +++ b/keystone/revoke/core.py @@ -88,7 +88,7 @@ class Manager(manager.Manager): ['user', self._user_callback] ], notifications.ACTIONS.internal: [ - [notifications.INVALIDATE_USER_TOKEN_PERSISTENCE, + [notifications.PERSIST_REVOCATION_EVENT_FOR_USER, self._user_callback], ] } diff --git a/keystone/token/provider.py b/keystone/token/provider.py index 3800c5425b..28721deb21 100644 --- a/keystone/token/provider.py +++ b/keystone/token/provider.py @@ -80,14 +80,12 @@ class Manager(manager.Manager): ['project', self._drop_token_cache], ], notifications.ACTIONS.internal: [ - [notifications.INVALIDATE_USER_TOKEN_PERSISTENCE, - self._drop_token_cache], - [notifications.INVALIDATE_USER_PROJECT_TOKEN_PERSISTENCE, - self._drop_token_cache], [notifications.INVALIDATE_USER_OAUTH_CONSUMER_TOKENS, self._drop_token_cache], [notifications.INVALIDATE_TOKEN_CACHE_DELETED_IDP, self._drop_token_cache], + [notifications.INVALIDATE_TOKEN_CACHE, + self._drop_token_cache], ] }