diff options
| author | Henry Nash <henryn@linux.vnet.ibm.com> | 2013-03-15 22:48:50 +0000 |
|---|---|---|
| committer | Henry Nash <henryn@linux.vnet.ibm.com> | 2013-03-18 16:14:16 +0000 |
| commit | 90fcb996cb2acf7b829b2cccfa485c09e4c0d0e8 (patch) | |
| tree | f73dff8715ffa872dc634a8e1195f22dd04d7c1d | |
| parent | 16b464376ea5d2a056cb0a2a9ea946eefa372af6 (diff) | |
Ensure delete domain removes all owned entities
Deleting a domain should delete all Users, Groups and Projects
that are owned by that domain. This is intertwined with making sure
that deleting Users/Projects clean up their relevant Tokens and
Credentials (raised as a separate bug, bug fixed here).
To help avoid inadvertent deletion, we insist that a domain must
be disabled before it can be deleted.
In implementing this change, it was discovered that the exception
CredentialNotFound is referenced in the identity backend, but
never defined - this was needed here for the unit tests. This is raised
as a separate bug, and fixed here. A further bug has been raised
that this indicates we are lacking in negative testing for
Credentials (not fixed in this change)
Fixes Bug #1097995
Fixes Bug #1155921
Fixes Bug #1155924
Change-Id: Ibc926f8212fb9bd4426088339a21002a07c86984
| -rw-r--r-- | keystone/exception.py | 4 | ||||
| -rw-r--r-- | keystone/identity/controllers.py | 109 | ||||
| -rw-r--r-- | tests/test_v3_identity.py | 172 |
3 files changed, 276 insertions, 9 deletions
diff --git a/keystone/exception.py b/keystone/exception.py index 09c43585..5b519cd4 100644 --- a/keystone/exception.py +++ b/keystone/exception.py @@ -194,6 +194,10 @@ class TrustNotFound(NotFound): """Could not find trust: %(trust_id)s""" +class CredentialNotFound(NotFound): + """Could not find credential: %(credential_id)s""" + + class Conflict(Error): """Conflict occurred attempting to store %(type)s. diff --git a/keystone/identity/controllers.py b/keystone/identity/controllers.py index 8d361ce9..e82b81f4 100644 --- a/keystone/identity/controllers.py +++ b/keystone/identity/controllers.py @@ -444,6 +444,65 @@ class DomainV3(controller.V3Controller): 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(context) + 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(context, user['id'], user) + self._delete_tokens_for_user(context, 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(context) + user_ids = [r['id'] for r in user_refs if r['domain_id'] == domain_id] + + proj_refs = self.identity_api.list_projects(context) + proj_ids = [r['id'] for r in proj_refs if r['domain_id'] == domain_id] + + # First delete the projects themselves + project_cntl = ProjectV3() + for project in proj_ids: + project_cntl._delete_project(context, project) + + # Get the list of groups owned by this domain and delete them + group_refs = self.identity_api.list_groups(context) + group_ids = ([r['id'] for r in group_refs + if r['domain_id'] == domain_id]) + group_cntl = GroupV3() + for group in group_ids: + group_cntl._delete_group(context, group) + + # And finally, delete the users themselves + user_cntl = UserV3() + for user in user_ids: + user_cntl._delete_user(context, user) + @controller.protected def delete_domain(self, context, domain_id): # explicitly forbid deleting the default domain (this should be a @@ -452,6 +511,17 @@ class DomainV3(controller.V3Controller): 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.identity_api.get_domain(context, 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.identity_api.delete_domain(context, domain_id) def _get_domain_by_name(self, context, domain_name): @@ -499,9 +569,19 @@ class ProjectV3(controller.V3Controller): ref = self.identity_api.update_project(context, project_id, project) return ProjectV3.wrap_member(context, ref) + def _delete_project(self, context, project_id): + # Delete any credentials that reference this project + for cred in self.identity_api.list_credentials(context): + if cred['project_id'] == project_id: + self.identity_api.delete_credential(context, cred['id']) + # Finally delete the project itself - the backend is + # responsible for deleting any role assignments related + # to this project + return self.identity_api.delete_project(context, project_id) + @controller.protected def delete_project(self, context, project_id): - return self.identity_api.delete_project(context, project_id) + return self._delete_project(context, project_id) class UserV3(controller.V3Controller): @@ -560,9 +640,22 @@ class UserV3(controller.V3Controller): context, user_id, group_id) self._delete_tokens_for_user(context, user_id) + def _delete_user(self, context, user_id): + # Delete any credentials that reference this user + for cred in self.identity_api.list_credentials(context): + if cred['user_id'] == user_id: + self.identity_api.delete_credential(context, cred['id']) + + # Make sure any tokens are marked as deleted + self._delete_tokens_for_user(context, user_id) + # Finally delete the user itself - the backend is + # responsible for deleting any role assignments related + # to this user + return self.identity_api.delete_user(context, user_id) + @controller.protected def delete_user(self, context, user_id): - return self.identity_api.delete_user(context, user_id) + return self._delete_user(context, user_id) class GroupV3(controller.V3Controller): @@ -598,8 +691,7 @@ class GroupV3(controller.V3Controller): ref = self.identity_api.update_group(context, group_id, group) return GroupV3.wrap_member(context, ref) - @controller.protected - def delete_group(self, context, group_id): + 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 @@ -611,6 +703,10 @@ class GroupV3(controller.V3Controller): for user in user_refs: self._delete_tokens_for_user(context, user['id']) + @controller.protected + def delete_group(self, context, group_id): + return self._delete_group(context, group_id) + class CredentialV3(controller.V3Controller): collection_name = 'credentials' @@ -642,9 +738,12 @@ class CredentialV3(controller.V3Controller): credential) return CredentialV3.wrap_member(context, ref) + def _delete_credential(self, context, credential_id): + return self.identity_api.delete_credential(context, credential_id) + @controller.protected def delete_credential(self, context, credential_id): - return self.identity_api.delete_credential(context, credential_id) + return self._delete_credential(context, credential_id) class RoleV3(controller.V3Controller): diff --git a/tests/test_v3_identity.py b/tests/test_v3_identity.py index 162c41e3..05d65d89 100644 --- a/tests/test_v3_identity.py +++ b/tests/test_v3_identity.py @@ -16,6 +16,8 @@ import uuid +from keystone import exception + import test_v3 @@ -98,10 +100,95 @@ class IdentityTestCase(test_v3.RestfulTestCase): # TODO(dolph): assert that v2 & v3 auth return 401 + def test_delete_enabled_domain_fails(self): + """DELETE /domains/{domain_id}...(when domain enabled)""" + + # Try deleting an enabled domain, which should fail + self.delete('/domains/%(domain_id)s' % { + 'domain_id': self.domain['id']}, + expected_status=exception.ForbiddenAction.code) + def test_delete_domain(self): - """DELETE /domains/{domain_id}""" + """DELETE /domains/{domain_id} + + The sample data set up already has a user, group, project + and credential that is part of self.domain. Since the user + we will authenticate with is in this domain, we create a + another set of entities in a second domain. Deleting this + second domain should delete all these new entities. In addition, + all the entities in the regular self.domain should be unaffected + by the delete. + + Test Plan: + - Create domain2 and a 2nd set of entities + - Disable domain2 + - Delete domain2 + - Check entities in domain2 have been deleted + - Check entities in self.domain are unaffected + + """ + # Create a 2nd set of entities in a 2nd domain + self.domain2 = self.new_domain_ref() + self.identity_api.create_domain(self.domain2['id'], self.domain2) + + self.project2 = self.new_project_ref( + domain_id=self.domain2['id']) + self.identity_api.create_project(self.project2['id'], self.project2) + + self.user2 = self.new_user_ref( + domain_id=self.domain2['id'], + project_id=self.project2['id']) + self.identity_api.create_user(self.user2['id'], self.user2) + + self.group2 = self.new_group_ref( + domain_id=self.domain2['id']) + self.identity_api.create_group(self.group2['id'], self.group2) + + self.credential2 = self.new_credential_ref( + user_id=self.user2['id'], + project_id=self.project2['id']) + self.identity_api.create_credential( + self.credential2['id'], + self.credential2) + + # Now disable the new domain and delete it + self.domain2['enabled'] = False + r = self.patch('/domains/%(domain_id)s' % { + 'domain_id': self.domain2['id']}, + body={'domain': {'enabled': False}}) + self.assertValidDomainResponse(r, self.domain2) self.delete('/domains/%(domain_id)s' % { - 'domain_id': self.domain_id}) + 'domain_id': self.domain2['id']}) + + # Check all the domain2 relevant entities are gone + self.assertRaises(exception.DomainNotFound, + self.identity_api.get_domain, + domain_id=self.domain2['id']) + self.assertRaises(exception.ProjectNotFound, + self.identity_api.get_project, + tenant_id=self.project2['id']) + self.assertRaises(exception.GroupNotFound, + self.identity_api.get_group, + group_id=self.group2['id']) + self.assertRaises(exception.UserNotFound, + self.identity_api.get_user, + user_id=self.user2['id']) + self.assertRaises(exception.CredentialNotFound, + self.identity_api.get_credential, + credential_id=self.credential2['id']) + + # ...and that all self.domain entities are still here + r = self.identity_api.get_domain(self.domain['id']) + self.assertDictEqual(r, self.domain) + r = self.identity_api.get_project(self.project['id']) + self.assertDictEqual(r, self.project) + r = self.identity_api.get_group(self.group['id']) + self.assertDictEqual(r, self.group) + r = self.identity_api.get_user(self.user['id']) + self.user.pop('password') + self.assertDictEqual(r, self.user) + r = self.identity_api.get_credential(self.credential['id']) + self.assertDictEqual(r, self.credential) # project crud tests @@ -141,11 +228,41 @@ class IdentityTestCase(test_v3.RestfulTestCase): self.assertValidProjectResponse(r, ref) def test_delete_project(self): - """DELETE /projects/{project_id}""" + """DELETE /projects/{project_id} + + As well as making sure the delete succeeds, we ensure + that any credentials that reference this projects are + also deleted, while other credentials are unaffected. + + """ + # First check the credential for this project is present + r = self.identity_api.get_credential(self.credential['id']) + self.assertDictEqual(r, self.credential) + # Create a second credential with a different project + self.project2 = self.new_project_ref( + domain_id=self.domain['id']) + self.identity_api.create_project(self.project2['id'], self.project2) + self.credential2 = self.new_credential_ref( + user_id=self.user['id'], + project_id=self.project2['id']) + self.identity_api.create_credential( + self.credential2['id'], + self.credential2) + + # Now delete the project self.delete( '/projects/%(project_id)s' % { 'project_id': self.project_id}) + # Deleting the project should have deleted any credentials + # that reference this project + self.assertRaises(exception.CredentialNotFound, + self.identity_api.get_credential, + credential_id=self.credential['id']) + # But the credential for project2 is unaffected + r = self.identity_api.get_credential(self.credential2['id']) + self.assertDictEqual(r, self.credential2) + # user crud tests def test_create_user(self): @@ -211,10 +328,57 @@ class IdentityTestCase(test_v3.RestfulTestCase): self.assertValidUserResponse(r, user) def test_delete_user(self): - """DELETE /users/{user_id}""" + """DELETE /users/{user_id} + + As well as making sure the delete succeeds, we ensure + that any credentials that reference this user are + also deleted, while other credentials are unaffected. + In addition, no tokens should remain valid for this user. + + """ + # First check the credential for this user is present + r = self.identity_api.get_credential(self.credential['id']) + self.assertDictEqual(r, self.credential) + # Create a second credential with a different user + self.user2 = self.new_user_ref( + domain_id=self.domain['id'], + project_id=self.project['id']) + self.identity_api.create_user(self.user2['id'], self.user2) + self.credential2 = self.new_credential_ref( + user_id=self.user2['id'], + project_id=self.project['id']) + self.identity_api.create_credential( + self.credential2['id'], + self.credential2) + # Create a token for this user which we can check later + # gets deleted + auth_data = self.build_authentication_request( + user_id=self.user['id'], + password=self.user['password'], + project_id=self.project['id']) + resp = self.post('/auth/tokens', body=auth_data) + token = resp.getheader('X-Subject-Token') + # Confirm token is valid for now + self.head('/auth/tokens', + headers={'X-Subject-Token': token}, + expected_status=204) + + # Now delete the user self.delete('/users/%(user_id)s' % { 'user_id': self.user['id']}) + # Deleting the user should have deleted any credentials + # that reference this project + self.assertRaises(exception.CredentialNotFound, + self.identity_api.get_credential, + credential_id=self.credential['id']) + # And the no tokens we remain valid + tokens = self.token_api.list_tokens(self.user['id']) + self.assertEquals(len(tokens), 0) + # But the credential for user2 is unaffected + r = self.identity_api.get_credential(self.credential2['id']) + self.assertDictEqual(r, self.credential2) + # group crud tests def test_create_group(self): |
