diff options
author | Morgan Fainberg <m@metacloud.com> | 2014-01-03 11:44:41 -0800 |
---|---|---|
committer | Morgan Fainberg <m@metacloud.com> | 2014-01-03 13:46:32 -0800 |
commit | 348d66260d883ff7017c9284b7541542d805bb23 (patch) | |
tree | 4c9f8f0d17b4a862d365fdbae202399dac77812b /keystone | |
parent | 331b4ca3310c6df0795c2e5d1b55809d17bc0768 (diff) | |
download | keystone-348d66260d883ff7017c9284b7541542d805bb23.tar.gz |
Move deletion business logic out of controllers
Based upon the feedback on the credential_api deletion changes,
the deletion code should be moved down to the managers from the
controllers. This means the business logic to delete / modify
associated resources based upon a given action no longer resides
in the controller. The changes include (but are not limited to)
mass token deletion (delete_tokens_for_(user|trust|project|domain),
domain content deletion, credential deletion, etc.
This change also moves around the use of @dependency decorator
to ensure the *_api managers are added to the correct objects.
The token_api has been expanded to handle the token revocations
for users, projects, and domains directly (public methods). This
allows the managers to handle deletions rather than the
logic residing in the controllers; it was previously not clean
to issue a `delete_tokens_for_user` outside of the controller.
A few minor test changes and additions were needed to handle the
moving of business logic. Notably, the keystoneclient tests
(test_keystoneclient) CompatTestCase needs to setup SQL for the
credential_api to be able to perform delete logic.
bp: assignment-controller-first-class
Change-Id: I721e49258c10830b3c44f301e8c9e5f954262388
Diffstat (limited to 'keystone')
-rw-r--r-- | keystone/assignment/controllers.py | 163 | ||||
-rw-r--r-- | keystone/assignment/core.py | 203 | ||||
-rw-r--r-- | keystone/auth/controllers.py | 3 | ||||
-rw-r--r-- | keystone/common/controller.py | 92 | ||||
-rw-r--r-- | keystone/common/wsgi.py | 2 | ||||
-rw-r--r-- | keystone/identity/controllers.py | 28 | ||||
-rw-r--r-- | keystone/identity/core.py | 16 | ||||
-rw-r--r-- | keystone/tests/test_auth.py | 2 | ||||
-rw-r--r-- | keystone/tests/test_backend.py | 25 | ||||
-rw-r--r-- | keystone/tests/test_keystoneclient.py | 22 | ||||
-rw-r--r-- | keystone/tests/test_keystoneclient_sql.py | 10 | ||||
-rw-r--r-- | keystone/token/core.py | 50 |
12 files changed, 317 insertions, 299 deletions
diff --git a/keystone/assignment/controllers.py b/keystone/assignment/controllers.py index a6906d21f..9264eb7e4 100644 --- a/keystone/assignment/controllers.py +++ b/keystone/assignment/controllers.py @@ -119,11 +119,6 @@ class Tenant(controller.V2Controller): clean_tenant = tenant.copy() clean_tenant.pop('domain_id', None) - # If the project has been disabled (or enabled=False) we are - # deleting the tokens for that project. - if not tenant.get('enabled', True): - self._delete_tokens_for_project(tenant_id) - tenant_ref = self.assignment_api.update_project( tenant_id, clean_tenant) return {'tenant': tenant_ref} @@ -131,8 +126,6 @@ class Tenant(controller.V2Controller): @controller.v2_deprecated def delete_project(self, context, tenant_id): self.assert_admin(context) - # Delete all tokens belonging to the users for that project - self._delete_tokens_for_project(tenant_id) self.assignment_api.delete_project(tenant_id) @controller.v2_deprecated @@ -225,10 +218,6 @@ class Role(controller.V2Controller): @controller.v2_deprecated def delete_role(self, context, role_id): self.assert_admin(context) - # The driver will delete any assignments for this role. - # We must first, however, revoke any tokens for users that have an - # assignment with this role. - self._delete_tokens_for_role(role_id) self.assignment_api.delete_role(role_id) @controller.v2_deprecated @@ -272,7 +261,6 @@ class Role(controller.V2Controller): # a user also adds them to a tenant, so we must follow up on that self.assignment_api.remove_role_from_user_and_project( user_id, tenant_id, role_id) - self._delete_tokens_for_user(user_id) # COMPAT(diablo): CRUD extension @controller.v2_deprecated @@ -320,7 +308,6 @@ class Role(controller.V2Controller): role_id = role.get('roleId') self.assignment_api.add_role_to_user_and_project( user_id, tenant_id, role_id) - self._delete_tokens_for_user(user_id) role_ref = self.assignment_api.get_role(role_id) return {'role': role_ref} @@ -345,10 +332,9 @@ class Role(controller.V2Controller): role_id = role_ref_ref.get('roleId')[0] self.assignment_api.remove_role_from_user_and_project( user_id, tenant_id, role_id) - self._delete_tokens_for_user(user_id) -@dependency.requires('assignment_api', 'credential_api', 'identity_api') +@dependency.requires('assignment_api') class DomainV3(controller.V3Controller): collection_name = 'domains' member_name = 'domain' @@ -378,143 +364,15 @@ class DomainV3(controller.V3Controller): @controller.protected() def update_domain(self, context, domain_id, domain): self._require_matching_id(domain_id, domain) - ref = self.assignment_api.update_domain(domain_id, domain) - - # disable owned users & projects when the API user specifically set - # enabled=False - # FIXME(dolph): need a driver call to directly revoke all tokens by - # project or domain, regardless of user - if not domain.get('enabled', True): - projects = [x for x in self.assignment_api.list_projects() - if x.get('domain_id') == domain_id] - for user in self.identity_api.list_users(): - # TODO(dolph): disable domain-scoped tokens - """ - self.token_api.revoke_tokens( - user_id=user['id'], - domain_id=domain_id) - """ - # revoke all tokens for users owned by this domain - if user.get('domain_id') == domain_id: - self._delete_tokens_for_user(user['id']) - else: - # only revoke tokens on projects owned by this domain - for project in projects: - self._delete_tokens_for_user( - user['id'], project_id=project['id']) return DomainV3.wrap_member(context, ref) - def _delete_domain_contents(self, context, domain_id): - """Delete the contents of a domain. - - Before we delete a domain, we need to remove all the entities - that are owned by it, i.e. Users, Groups & Projects. To do this we - call the respective delete functions for these entities, which are - themselves responsible for deleting any credentials and role grants - associated with them as well as revoking any relevant tokens. - - The order we delete entities is also important since some types - of backend may need to maintain referential integrity - throughout, and many of the entities have relationship with each - other. The following deletion order is therefore used: - - Projects: Reference user and groups for grants - Groups: Reference users for membership and domains for grants - Users: Reference domains for grants - - """ - # Start by disabling all the users in this domain, to minimize the - # the risk that things are changing under our feet. - # TODO(henry-nash): In theory this step should not be necessary, since - # users of a disabled domain are prevented from authenticating. - # However there are some existing bugs in this area (e.g. 1130236). - # Consider removing this code once these have been fixed. - user_refs = self.identity_api.list_users() - user_refs = [r for r in user_refs if r['domain_id'] == domain_id] - for user in user_refs: - if user['enabled']: - user['enabled'] = False - self.identity_api.update_user(user['id'], user) - self._delete_tokens_for_user(user['id']) - - # Now, for safety, reload list of users, as well as projects, that are - # owned by this domain. - user_refs = self.identity_api.list_users() - user_ids = [r['id'] for r in user_refs if r['domain_id'] == domain_id] - - proj_refs = self.assignment_api.list_projects() - proj_ids = [r['id'] for r in proj_refs if r['domain_id'] == domain_id] - - # Get the list of groups owned by this domain and delete them - group_refs = self.identity_api.list_groups() - group_ids = ([r['id'] for r in group_refs - if r['domain_id'] == domain_id]) - - # First delete the projects themselves - for project_id in proj_ids: - # NOTE(morganfainberg): Ensure we cleanup the credentials for the - # project and any outstanding tokens. This must occur after the - # project deletion has occurred to ensure we don't have a race - # where new cred or token is issued. - self.credential_api.delete_credentials_for_project(project_id) - try: - self._delete_tokens_for_project(project_id) - self.assignment_api.delete_project(project_id) - except exception.ProjectNotFound: - # NOTE(morganfainberg): We should still perform the cleanups - # if the project can't be found for sanity-sake. - pass - - for group_id in group_ids: - # NOTE(morganfainberg): Cleanup any existing groups. - try: - self.identity_api.delete_group(group_id, - domain_scope=r['domain_id']) - except exception.GroupNotFound: - # NOTE(morganfainberg): In the case that a race has occurred - # and the group no longer exists, continue on and delete the - # rest of the groups. - pass - - # And finally, delete the users themselves - for user_id in user_ids: - # Delete any credentials that reference this user - self.credential_api.delete_credentials_for_user(user_id) - # Make sure any tokens are marked as deleted - try: - self._delete_tokens_for_user(user_id) - self.identity_api.delete_user(user_id, - domain_scope=r['domain_id']) - except exception.UserNotFound: - # NOTE(morganfainberg): In the case that a race has occurred - # and the user no longer exists, continue on and delete the - # rest of the users. - pass - @controller.protected() def delete_domain(self, context, domain_id): - # explicitly forbid deleting the default domain (this should be a - # carefully orchestrated manual process involving configuration - # changes, etc) - if domain_id == DEFAULT_DOMAIN_ID: - raise exception.ForbiddenAction(action='delete the default domain') - - # To help avoid inadvertent deletes, we insist that the domain - # has been previously disabled. This also prevents a user deleting - # their own domain since, once it is disabled, they won't be able - # to get a valid token to issue this delete. - ref = self.assignment_api.get_domain(domain_id) - if ref['enabled']: - raise exception.ForbiddenAction( - action='delete a domain that is not disabled') - - # OK, we are go for delete! - self._delete_domain_contents(context, domain_id) return self.assignment_api.delete_domain(domain_id) -@dependency.requires('assignment_api', 'credential_api') +@dependency.requires('assignment_api') class ProjectV3(controller.V3Controller): collection_name = 'projects' member_name = 'project' @@ -551,17 +409,11 @@ class ProjectV3(controller.V3Controller): def update_project(self, context, project_id, project): self._require_matching_id(project_id, project) - # The project was disabled so we delete the tokens - if not project.get('enabled', True): - self._delete_tokens_for_project(project_id) - ref = self.assignment_api.update_project(project_id, project) return ProjectV3.wrap_member(context, ref) @controller.protected() def delete_project(self, context, project_id): - self.credential_api.delete_credentials_for_project(project_id) - self._delete_tokens_for_project(project_id) return self.assignment_api.delete_project(project_id) @@ -601,10 +453,6 @@ class RoleV3(controller.V3Controller): @controller.protected() def delete_role(self, context, role_id): - # The driver will delete any assignments for this role. - # We must first, however, revoke any tokens for users that have an - # assignment with this role. - self._delete_tokens_for_role(role_id) self.assignment_api.delete_role(role_id) def _require_domain_xor_project(self, domain_id, project_id): @@ -702,13 +550,6 @@ class RoleV3(controller.V3Controller): role_id, user_id, group_id, domain_id, project_id, self._check_if_inherited(context)) - # Now delete any tokens for this user or, in the case of a group, - # tokens from all the uses who are members of this group. - if user_id: - self._delete_tokens_for_user(user_id) - else: - self._delete_tokens_for_group(group_id) - @dependency.requires('assignment_api', 'identity_api') class RoleAssignmentV3(controller.V3Controller): diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index e1dd2cbc2..c7b951c0d 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -43,14 +43,14 @@ DEFAULT_DOMAIN = {'description': @dependency.provider('assignment_api') -@dependency.requires('identity_api') +@dependency.requires('credential_api', 'identity_api', 'token_api') class Manager(manager.Manager): """Default pivot point for the Assignment backend. See :mod:`keystone.common.manager.Manager` for more details on how this dynamically calls the backend. assignment.Manager() and identity.Manager() have a circular dependency. - The late import works around this. THe if block prevents creation of the + The late import works around this. The if block prevents creation of the api object by both managers. """ @@ -81,6 +81,10 @@ class Manager(manager.Manager): tenant = tenant.copy() if 'enabled' in tenant: tenant['enabled'] = clean.project_enabled(tenant['enabled']) + if not tenant.get('enabled', True): + self.token_api.delete_tokens_for_users( + self.list_user_ids_for_project(tenant_id), + project_id=tenant_id) ret = self.driver.update_project(tenant_id, tenant) self.get_project.invalidate(self, tenant_id) self.get_project_by_name.invalidate(self, ret['name'], @@ -90,10 +94,13 @@ class Manager(manager.Manager): @notifications.deleted('project') def delete_project(self, tenant_id): project = self.driver.get_project(tenant_id) + user_ids = self.list_user_ids_for_project(tenant_id) + self.token_api.delete_tokens_for_users(user_ids, project_id=tenant_id) ret = self.driver.delete_project(tenant_id) self.get_project.invalidate(self, tenant_id) self.get_project_by_name.invalidate(self, project['name'], project['domain_id']) + self.credential_api.delete_credentials_for_project(tenant_id) return ret def get_roles_for_user_and_project(self, user_id, tenant_id): @@ -278,16 +285,109 @@ class Manager(manager.Manager): def update_domain(self, domain_id, domain): ret = self.driver.update_domain(domain_id, domain) + # disable owned users & projects when the API user specifically set + # enabled=False + if not domain.get('enabled', True): + self.token_api.delete_tokens_for_domain(domain_id) self.get_domain.invalidate(self, domain_id) self.get_domain_by_name.invalidate(self, ret['name']) return ret def delete_domain(self, domain_id): + # explicitly forbid deleting the default domain (this should be a + # carefully orchestrated manual process involving configuration + # changes, etc) + if domain_id == DEFAULT_DOMAIN['id']: + raise exception.ForbiddenAction(action='delete the default domain') + domain = self.driver.get_domain(domain_id) + + # To help avoid inadvertent deletes, we insist that the domain + # has been previously disabled. This also prevents a user deleting + # their own domain since, once it is disabled, they won't be able + # to get a valid token to issue this delete. + if domain['enabled']: + raise exception.ForbiddenAction( + action='delete a domain that is not disabled') + + self._delete_domain_contents(domain_id) self.driver.delete_domain(domain_id) self.get_domain.invalidate(self, domain_id) self.get_domain_by_name.invalidate(self, domain['name']) + def _delete_domain_contents(self, domain_id): + """Delete the contents of a domain. + + Before we delete a domain, we need to remove all the entities + that are owned by it, i.e. Users, Groups & Projects. To do this we + call the respective delete functions for these entities, which are + themselves responsible for deleting any credentials and role grants + associated with them as well as revoking any relevant tokens. + + The order we delete entities is also important since some types + of backend may need to maintain referential integrity + throughout, and many of the entities have relationship with each + other. The following deletion order is therefore used: + + Projects: Reference user and groups for grants + Groups: Reference users for membership and domains for grants + Users: Reference domains for grants + + """ + # Start by disabling all the users in this domain, to minimize the + # the risk that things are changing under our feet. + # TODO(henry-nash): In theory this step should not be necessary, since + # users of a disabled domain are prevented from authenticating. + # However there are some existing bugs in this area (e.g. 1130236). + # Consider removing this code once these have been fixed. + user_refs = self.identity_api.list_users() + user_refs = [r for r in user_refs if r['domain_id'] == domain_id] + for user in user_refs: + if user['enabled']: + user['enabled'] = False + self.identity_api.update_user(user['id'], user) + + user_refs = self.identity_api.list_users() + proj_refs = self.list_projects() + group_refs = self.identity_api.list_groups() + + # First delete the projects themselves + for project in proj_refs: + if project['domain_id'] == domain_id: + try: + self.delete_project(project['id']) + except exception.ProjectNotFound: + LOG.debug(_('Project %(projectid)s not found when ' + 'deleting domain contents for %(domainid)s, ' + 'continuing with cleanup.'), + {'projectid': project['id'], + 'domainid': domain_id}) + + for group in group_refs: + # NOTE(morganfainberg): Cleanup any existing groups. + if group['domain_id'] == domain_id: + try: + self.identity_api.delete_group(group['id'], + domain_scope=domain_id) + except exception.GroupNotFound: + LOG.debug(_('Group %(groupid)s not found when deleting ' + 'domain contents for %(domainid)s, continuing ' + 'with cleanup.'), + {'groupid': group['id'], 'domainid': domain_id}) + + # And finally, delete the users themselves + for user in user_refs: + if user['domain_id'] == domain_id: + try: + self.identity_api.delete_user(user['id'], + domain_scope=domain_id) + except exception.UserNotFound: + LOG.debug(_('User %(userid)s not found when ' + 'deleting domain contents for %(domainid)s, ' + 'continuing with cleanup.'), + {'userid': user['id'], + 'domainid': domain_id}) + @cache.on_arguments(should_cache_fn=SHOULD_CACHE, expiration_time=CONF.assignment.cache_time) def get_project(self, project_id): @@ -318,6 +418,17 @@ class Manager(manager.Manager): @notifications.deleted('role') def delete_role(self, role_id): + try: + self._delete_tokens_for_role(role_id) + except exception.NotImplemented: + # FIXME(morganfainberg): Not all backends (ldap) implement + # `list_role_assignments_for_role` which would have previously + # caused a NotImplmented error to be raised when called through + # the controller. Now error or proper action will always come from + # the `delete_role` method logic. Work needs to be done to make + # the behavior between drivers consistent (capable of revoking + # tokens for the same circumstances). + pass self.driver.delete_role(role_id) self.get_role.invalidate(self, role_id) @@ -332,6 +443,94 @@ class Manager(manager.Manager): return [r for r in self.driver.list_role_assignments() if r['role_id'] == role_id] + def remove_role_from_user_and_project(self, user_id, tenant_id, role_id): + self.driver.remove_role_from_user_and_project(user_id, tenant_id, + role_id) + self.token_api.delete_tokens_for_user(user_id) + + def delete_grant(self, role_id, user_id=None, group_id=None, + domain_id=None, project_id=None, + inherited_to_projects=False): + user_ids = [] + if group_id is not None: + # NOTE(morganfainberg): The user ids are the important part for + # invalidating tokens below, so extract them here. + try: + for user in self.identity_api.list_users_in_group(group_id, + domain_id): + if user['id'] != user_id: + user_ids.append(user['id']) + except exception.GroupNotFound: + LOG.debug(_('Group %s not found, no tokens to invalidate.'), + group_id) + + self.driver.delete_grant(role_id, user_id, group_id, domain_id, + project_id, inherited_to_projects) + if user_id is not None: + user_ids.append(user_id) + self.token_api.delete_tokens_for_users(user_ids) + + def _delete_tokens_for_role(self, role_id): + assignments = self.list_role_assignments_for_role(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 'user_id' in assignment: + if 'project_id' in assignment: + user_and_project_ids.append( + (assignment['user_id'], assignment['project_id'])) + elif 'domain_id' in assignment: + user_ids.add(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 = self.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' in assignment: + for user in users: + user_ids.add(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) + + self.token_api.delete_tokens_for_users(user_ids) + for user_id, project_id in user_and_project_ids_to_action: + self.token_api.delete_tokens_for_user(user_id, project_id) + @six.add_metaclass(abc.ABCMeta) class Driver(object): diff --git a/keystone/auth/controllers.py b/keystone/auth/controllers.py index 72c13cb2f..bb9b420f6 100644 --- a/keystone/auth/controllers.py +++ b/keystone/auth/controllers.py @@ -275,7 +275,8 @@ class AuthInfo(object): self._scope_data = (domain_id, project_id, trust) -@dependency.requires('identity_api', 'token_provider_api') +@dependency.requires('assignment_api', 'identity_api', 'token_api', + 'token_provider_api') class Auth(controller.V3Controller): # Note(atiwari): From V3 auth controller code we are diff --git a/keystone/common/controller.py b/keystone/common/controller.py index 85d646dd5..da917858e 100644 --- a/keystone/common/controller.py +++ b/keystone/common/controller.py @@ -218,93 +218,8 @@ def filterprotected(*filters): return _filterprotected -@dependency.requires('assignment_api', 'identity_api', 'policy_api', - 'token_api', 'trust_api') class V2Controller(wsgi.Application): """Base controller class for Identity API v2.""" - - def _delete_tokens_for_trust(self, user_id, trust_id): - self.token_api.delete_tokens(user_id, trust_id=trust_id) - - def _delete_tokens_for_user(self, user_id, project_id=None): - #First delete tokens that could get other tokens. - self.token_api.delete_tokens(user_id, tenant_id=project_id) - - #delete tokens generated from trusts - for trust in self.trust_api.list_trusts_for_trustee(user_id): - self._delete_tokens_for_trust(user_id, trust['id']) - for trust in self.trust_api.list_trusts_for_trustor(user_id): - self._delete_tokens_for_trust(trust['trustee_user_id'], - trust['id']) - - def _delete_tokens_for_project(self, project_id): - user_ids = self.assignment_api.list_user_ids_for_project(project_id) - for user_id in user_ids: - self._delete_tokens_for_user(user_id, project_id=project_id) - - def _delete_tokens_for_role(self, role_id): - assignments = self.assignment_api.list_role_assignments_for_role( - 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 'user_id' in assignment: - if 'project_id' in assignment: - user_and_project_ids.append( - (assignment['user_id'], assignment['project_id'])) - elif 'domain_id' in assignment: - user_ids.add(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 = self.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' in assignment: - for user in users: - user_ids.add(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 in user_ids: - self._delete_tokens_for_user(user_id) - for user_id, project_id in user_and_project_ids_to_action: - self._delete_tokens_for_user(user_id, project_id) - def _require_attribute(self, ref, attr): """Ensures the reference contains the specified attribute.""" if ref.get(attr) is None or ref.get(attr) == '': @@ -328,7 +243,7 @@ class V2Controller(wsgi.Application): return ref -@dependency.requires('identity_api', 'policy_api', 'token_api') +@dependency.requires('policy_api', 'token_api') class V3Controller(V2Controller): """Base controller class for Identity API v3. @@ -343,11 +258,6 @@ class V3Controller(V2Controller): member_name = 'entity' get_member_from_driver = None - def _delete_tokens_for_group(self, group_id): - user_refs = self.identity_api.list_users_in_group(group_id) - for user in user_refs: - self._delete_tokens_for_user(user['id']) - @classmethod def base_url(cls, path=None): endpoint = CONF.public_endpoint % CONF diff --git a/keystone/common/wsgi.py b/keystone/common/wsgi.py index 1ea96ddd3..338dd7c9a 100644 --- a/keystone/common/wsgi.py +++ b/keystone/common/wsgi.py @@ -27,6 +27,7 @@ import webob.dec import webob.exc from keystone.common import config +from keystone.common import dependency from keystone.common import utils from keystone import exception from keystone.openstack.common import gettextutils @@ -177,6 +178,7 @@ class BaseApplication(object): raise NotImplementedError('You must implement __call__') +@dependency.requires('assignment_api', 'policy_api', 'token_api') class Application(BaseApplication): @webob.dec.wsgify(RequestClass=Request) def __call__(self, req): diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py index 306551235..5f64456bc 100644 --- a/keystone/identity/controllers.py +++ b/keystone/identity/controllers.py @@ -182,10 +182,6 @@ class User(controller.V2Controller): user_ref = self.identity_api.v3_to_v2_user( self.identity_api.update_user(user_id, user)) - if user.get('password') or not user.get('enabled', True): - # If the password was changed or the user was disabled we clear tokens - self._delete_tokens_for_user(user_id) - # If 'tenantId' is in either ref, we might need to add or remove the # user from a project. if 'tenantId' in user_ref or 'tenantId' in old_user_ref: @@ -230,7 +226,6 @@ class User(controller.V2Controller): def delete_user(self, context, user_id): self.assert_admin(context) self.identity_api.delete_user(user_id) - self._delete_tokens_for_user(user_id) @controller.v2_deprecated def set_user_enabled(self, context, user_id, user): @@ -254,7 +249,7 @@ class User(controller.V2Controller): return ref -@dependency.requires('identity_api', 'credential_api') +@dependency.requires('identity_api') class UserV3(controller.V3Controller): collection_name = 'users' member_name = 'user' @@ -303,11 +298,6 @@ class UserV3(controller.V3Controller): self._require_matching_id(user_id, user) ref = self.identity_api.update_user( user_id, user, domain_scope=domain_scope) - - if user.get('password') or not user.get('enabled', True): - # revoke all tokens owned by this user - self._delete_tokens_for_user(user_id) - return UserV3.wrap_member(context, ref) @controller.protected() @@ -320,9 +310,6 @@ class UserV3(controller.V3Controller): self.identity_api.add_user_to_group( user_id, group_id, domain_scope=self._get_domain_id_for_request(context)) - # Delete any tokens so that group membership can have an - # immediate effect - self._delete_tokens_for_user(user_id) @controller.protected(callback=_check_user_and_group_protection) def check_user_in_group(self, context, user_id, group_id): @@ -335,15 +322,11 @@ class UserV3(controller.V3Controller): self.identity_api.remove_user_from_group( user_id, group_id, domain_scope=self._get_domain_id_for_request(context)) - self._delete_tokens_for_user(user_id) @controller.protected() def delete_user(self, context, user_id): - # Delete any credentials that reference this user - self.credential_api.delete_credentials_for_user(user_id) # Make sure any tokens are marked as deleted domain_id = self._get_domain_id_for_request(context) - self._delete_tokens_for_user(user_id) # Finally delete the user itself - the backend is # responsible for deleting any role assignments related # to this user @@ -422,17 +405,8 @@ class GroupV3(controller.V3Controller): @controller.protected() def delete_group(self, context, group_id): - # As well as deleting the group, we need to invalidate - # any tokens for the users who are members of the group. - # We get the list of users before we attempt the group - # deletion, so that we can remove these tokens after we know - # the group deletion succeeded. domain_id = self._get_domain_id_for_request(context) - user_refs = self.identity_api.list_users_in_group( - group_id, domain_scope=domain_id) self.identity_api.delete_group(group_id, domain_scope=domain_id) - for user in user_refs: - self._delete_tokens_for_user(user['id']) # TODO(morganfainberg): Remove proxy compat classes once Icehouse is released. diff --git a/keystone/identity/core.py b/keystone/identity/core.py index afdbe1394..8174ed8cb 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -191,7 +191,7 @@ def domains_configured(f): @dependency.provider('identity_api') -@dependency.requires('assignment_api') +@dependency.requires('assignment_api', 'credential_api', 'token_api') class Manager(manager.Manager): """Default pivot point for the Identity backend. @@ -373,6 +373,8 @@ class Manager(manager.Manager): if not driver.is_domain_aware(): user = self._clear_domain_id(user) ref = driver.update_user(user_id, user) + if user.get('enabled') is False or user.get('password') is not None: + self.token_api.delete_tokens_for_user(user_id) if not driver.is_domain_aware(): ref = self._set_domain_id(ref, domain_id) return ref @@ -382,6 +384,8 @@ class Manager(manager.Manager): def delete_user(self, user_id, domain_scope=None): domain_id, driver = self._get_domain_id_and_driver(domain_scope) driver.delete_user(user_id) + self.credential_api.delete_credentials_for_user(user_id) + self.token_api.delete_tokens_for_user(user_id) @notifications.created('group') @domains_configured @@ -422,17 +426,27 @@ class Manager(manager.Manager): @domains_configured def delete_group(self, group_id, domain_scope=None): domain_id, driver = self._get_domain_id_and_driver(domain_scope) + # As well as deleting the group, we need to invalidate + # any tokens for the users who are members of the group. + # We get the list of users before we attempt the group + # deletion, so that we can remove these tokens after we know + # the group deletion succeeded. + user_ids = [u['id'] + for u in self.list_users_in_group(group_id, domain_scope)] + self.token_api.delete_tokens_for_users(user_ids) driver.delete_group(group_id) @domains_configured def add_user_to_group(self, user_id, group_id, domain_scope=None): domain_id, driver = self._get_domain_id_and_driver(domain_scope) driver.add_user_to_group(user_id, group_id) + self.token_api.delete_tokens_for_user(user_id) @domains_configured def remove_user_from_group(self, user_id, group_id, domain_scope=None): domain_id, driver = self._get_domain_id_and_driver(domain_scope) driver.remove_user_from_group(user_id, group_id) + self.token_api.delete_tokens_for_user(user_id) @domains_configured def list_groups_for_user(self, user_id, domain_scope=None): diff --git a/keystone/tests/test_auth.py b/keystone/tests/test_auth.py index 7890db642..72388d74b 100644 --- a/keystone/tests/test_auth.py +++ b/keystone/tests/test_auth.py @@ -796,7 +796,7 @@ class AuthWithTrust(AuthTest): self.assert_token_count_for_trust(0) self.fetch_v2_token_from_trust() self.assert_token_count_for_trust(1) - self.trust_controller._delete_tokens_for_user(self.trustee['id']) + self.token_api.delete_tokens_for_user(self.trustee['id']) self.assert_token_count_for_trust(0) def test_token_from_trust_cant_get_another_token(self): diff --git a/keystone/tests/test_backend.py b/keystone/tests/test_backend.py index 5a66ea026..e24ac54e0 100644 --- a/keystone/tests/test_backend.py +++ b/keystone/tests/test_backend.py @@ -2209,6 +2209,11 @@ class IdentityTests(object): new_group = {'id': uuid.uuid4().hex, 'domain_id': domain['id'], 'name': uuid.uuid4().hex} self.identity_api.create_group(new_group['id'], new_group) + # Make sure we get an empty list back on a new group, not an error. + user_refs = self.identity_api.list_users_in_group(new_group['id']) + self.assertEqual(user_refs, []) + # Make sure we get the correct users back once they have been added + # to the group. new_user = {'id': uuid.uuid4().hex, 'name': 'new_user', 'password': uuid.uuid4().hex, 'enabled': True, 'domain_id': domain['id']} @@ -2470,7 +2475,19 @@ class IdentityTests(object): domain_ref = self.assignment_api.get_domain(domain['id']) self.assertDictEqual(domain_ref, domain) + # Ensure an 'enabled' domain cannot be deleted + self.assertRaises(exception.ForbiddenAction, + self.assignment_api.delete_domain, + domain_id=domain['id']) + + # Disable the domain + domain['enabled'] = False + self.assignment_api.update_domain(domain['id'], domain) + + # Delete the domain self.assignment_api.delete_domain(domain['id']) + + # Make sure the domain no longer exists self.assertRaises(exception.DomainNotFound, self.assignment_api.get_domain, domain['id']) @@ -2640,6 +2657,11 @@ class IdentityTests(object): self.assignment_api.update_domain(domain_id, domain_ref) self.assertDictContainsSubset( domain_ref, self.assignment_api.get_domain(domain_id)) + # Make sure domain is 'disabled', bypass assignment api manager + domain_ref_disabled = domain_ref.copy() + domain_ref_disabled['enabled'] = False + self.assignment_api.driver.update_domain(domain_id, + domain_ref_disabled) # Delete domain, bypassing assignment api manager self.assignment_api.driver.delete_domain(domain_id) # Verify get_domain still returns the domain @@ -2654,6 +2676,9 @@ class IdentityTests(object): # Recreate Domain self.assignment_api.create_domain(domain_id, domain) self.assignment_api.get_domain(domain_id) + # Make sure domain is 'disabled', bypass assignment api manager + domain['enabled'] = False + self.assignment_api.driver.update_domain(domain_id, domain) # Delete domain self.assignment_api.delete_domain(domain_id) # verify DomainNotFound raised diff --git a/keystone/tests/test_keystoneclient.py b/keystone/tests/test_keystoneclient.py index 360bbfb77..51d877150 100644 --- a/keystone/tests/test_keystoneclient.py +++ b/keystone/tests/test_keystoneclient.py @@ -18,7 +18,9 @@ import os import uuid import webob +from keystone.common import sql from keystone import config +from keystone.openstack.common.db.sqlalchemy import session from keystone.openstack.common import jsonutils from keystone.openstack.common import timeutils from keystone import tests @@ -33,13 +35,25 @@ KEYSTONECLIENT_REPO = '%s/python-keystoneclient.git' % OPENSTACK_REPO class CompatTestCase(tests.NoModule, tests.TestCase): - def setUp(self): - super(CompatTestCase, self).setUp() - # The backends should be loaded and initialized before the servers are - # started because the servers use the backends. + def config(self, config_files): + super(CompatTestCase, self).config(config_files) + # FIXME(morganfainberg): Since we are running tests through the + # controllers and some internal api drivers are SQL-only, the correct + # approach is to ensure we have the correct backing store. The + # credential api makes some very SQL specific assumptions that should + # be addressed allowing for non-SQL based testing to occur. self.load_backends() + self.engine = session.get_engine() + sql.ModelBase.metadata.create_all(bind=self.engine) + self.addCleanup(session.cleanup) + self.addCleanup(sql.ModelBase.metadata.drop_all, + bind=self.engine) + + def setUp(self): + super(CompatTestCase, self).setUp() + self.load_fixtures(default_fixtures) # TODO(termie): add an admin user to the fixtures and use that user diff --git a/keystone/tests/test_keystoneclient_sql.py b/keystone/tests/test_keystoneclient_sql.py index 44673a3b2..a8b562abe 100644 --- a/keystone/tests/test_keystoneclient_sql.py +++ b/keystone/tests/test_keystoneclient_sql.py @@ -21,7 +21,6 @@ from keystoneclient.contrib.ec2 import utils as ec2_utils from keystone.common import sql from keystone import config -from keystone.openstack.common.db.sqlalchemy import session from keystone import tests from keystone.tests import test_keystoneclient @@ -36,19 +35,10 @@ class KcMasterSqlTestCase(test_keystoneclient.KcMasterTestCase, sql.Base): tests.dirs.tests('test_overrides.conf'), tests.dirs.tests('backend_sql.conf')]) - self.load_backends() - self.engine = session.get_engine() - sql.ModelBase.metadata.create_all(bind=self.engine) - def setUp(self): super(KcMasterSqlTestCase, self).setUp() self.default_client = self.get_client() - def tearDown(self): - sql.ModelBase.metadata.drop_all(bind=self.engine) - session.cleanup() - super(KcMasterSqlTestCase, self).tearDown() - def test_endpoint_crud(self): from keystoneclient import exceptions as client_exceptions diff --git a/keystone/token/core.py b/keystone/token/core.py index 052ef0892..c2c0cdb65 100644 --- a/keystone/token/core.py +++ b/keystone/token/core.py @@ -95,7 +95,8 @@ def validate_auth_info(self, user_ref, tenant_ref): raise exception.Unauthorized(msg) -@dependency.requires('token_provider_api') +@dependency.requires('assignment_api', 'identity_api', 'token_provider_api', + 'trust_api') @dependency.provider('token_api') class Manager(manager.Manager): """Default pivot point for the Token backend. @@ -182,6 +183,53 @@ class Manager(manager.Manager): # determining cache-keys. self.list_revoked_tokens.invalidate(self) + def delete_tokens_for_domain(self, domain_id): + """Delete all tokens for a given domain.""" + projects = self.assignment_api.list_projects() + for project in projects: + if project['domain_id'] == domain_id: + for user_id in self.assignment_api.list_user_ids_for_project( + project['id']): + self.delete_tokens_for_user(user_id, project['id']) + # TODO(morganfainberg): implement deletion of domain_scoped tokens. + + def delete_tokens_for_user(self, user_id, project_id=None): + """Delete all tokens for a given user or user-project combination. + + This method adds in the extra logic for handling trust-scoped token + revocations in a single call instead of needing to explicitly handle + trusts in the caller's logic. + """ + self.delete_tokens(user_id, tenant_id=project_id) + for trust in self.trust_api.list_trusts_for_trustee(user_id): + # Ensure we revoke tokens associated to the trust / project + # user_id combination. + self.delete_tokens(user_id, trust_id=trust['id'], + tenant_id=project_id) + for trust in self.trust_api.list_trusts_for_trustor(user_id): + # Ensure we revoke tokens associated to the trust / project / + # user_id combination where the user_id is the trustor. + + # NOTE(morganfainberg): This revocation is a bit coarse, but it + # covers a number of cases such as disabling of the trustor user, + # deletion of the trustor user (for any number of reasons). It + # might make sense to refine this and be more surgical on the + # deletions (e.g. don't revoke tokens for the trusts when the + # trustor changes password). For now, to maintain previous + # functionality, this will continue to be a bit overzealous on + # revocations. + self.delete_tokens(trust['trustee_user_id'], trust_id=trust['id'], + tenant_id=project_id) + + def delete_tokens_for_users(self, user_ids, project_id=None): + """Delete all tokens for a list of user_ids. + + :param user_ids: list of user identifiers + :param project_id: optional project identifier + """ + for user_id in user_ids: + self.delete_tokens_for_user(user_id, project_id=project_id) + def _invalidate_individual_token_cache(self, token_id): # NOTE(morganfainberg): invalidate takes the exact same arguments as # the normal method, this means we need to pass "self" in (which gets |