diff --git a/keystone/identity/core.py b/keystone/identity/core.py index ee5abba3b4..377a8ce226 100644 --- a/keystone/identity/core.py +++ b/keystone/identity/core.py @@ -1114,6 +1114,7 @@ class Manager(manager.Manager): def delete_group(self, group_id, initiator=None): domain_id, driver, entity_id = ( self._get_domain_driver_and_entity_id(group_id)) + roles = self.assignment_api.list_role_assignments(group_id=group_id) user_ids = (u['id'] for u in self.list_users_in_group(group_id)) driver.delete_group(entity_id) self.get_group.invalidate(self, group_id) @@ -1122,8 +1123,12 @@ class Manager(manager.Manager): notifications.Audit.deleted(self._GROUP, group_id, initiator) - for uid in user_ids: - self.emit_invalidate_user_token_persistence(uid) + # If the group has been created and has users but has no role + # 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) # Invalidate user role assignments cache region, as it may be caching # role assignments expanded from the specified group to its users diff --git a/keystone/tests/unit/test_revoke.py b/keystone/tests/unit/test_revoke.py index 26a256a69d..87d79bf0e2 100644 --- a/keystone/tests/unit/test_revoke.py +++ b/keystone/tests/unit/test_revoke.py @@ -17,6 +17,7 @@ import uuid import mock from oslo_utils import timeutils from six.moves import range +from testtools import matchers from keystone.common import utils import keystone.conf @@ -518,6 +519,54 @@ class RevokeTests(object): self.revoke_api.check_token, token_values) + def test_delete_group_without_role_does_not_revoke_users(self): + revocation_backend = sql.Revoke() + domain = unit.new_domain_ref() + self.resource_api.create_domain(domain['id'], domain) + # Create two groups. Group1 will be used to test deleting a group, + # without role assignments and users in the group, doesn't create + # revoked events. Group2 will show that deleting a group with role + # assignment and users in the group does create revoked events + group1 = unit.new_group_ref(domain_id=domain['id']) + group1 = self.identity_api.create_group(group1) + group2 = unit.new_group_ref(domain_id=domain['id']) + group2 = self.identity_api.create_group(group2) + role = unit.new_role_ref() + self.role_api.create_role(role['id'], role) + user1 = unit.new_user_ref(domain_id=domain['id']) + user1 = self.identity_api.create_user(user1) + user2 = unit.new_user_ref(domain_id=domain['id']) + user2 = self.identity_api.create_user(user2) + + # Add two users to the group, verify they are added, delete group, and + # check that the revocaiton events have not been created + self.identity_api.add_user_to_group(user_id=user1['id'], + group_id=group1['id']) + self.identity_api.add_user_to_group(user_id=user2['id'], + group_id=group1['id']) + self.assertEqual( + 2, len(self.identity_api.list_users_in_group(group1['id']))) + self.identity_api.delete_group(group1['id']) + self.assertEqual(0, len(revocation_backend.list_events())) + + # Assign a role to the group, add two users to the group, verify that + # the role has been assigned to the group, verify the users have been + # added to the group, delete the group, check that the revocation + # events have been created + self.assignment_api.create_grant(group_id=group2['id'], + domain_id=domain['id'], + role_id=role['id']) + grants = self.assignment_api.list_role_assignments(role_id=role['id']) + self.assertThat(grants, matchers.HasLength(1)) + self.identity_api.add_user_to_group(user_id=user1['id'], + group_id=group2['id']) + self.identity_api.add_user_to_group(user_id=user2['id'], + group_id=group2['id']) + self.assertEqual( + 2, len(self.identity_api.list_users_in_group(group2['id']))) + self.identity_api.delete_group(group2['id']) + self.assertEqual(2, len(revocation_backend.list_events())) + class UUIDSqlRevokeTests(test_backend_sql.SqlTests, RevokeTests): def config_overrides(self):