diff --git a/keystone/assignment/core.py b/keystone/assignment/core.py index a47e0f2999..29bb3db56a 100644 --- a/keystone/assignment/core.py +++ b/keystone/assignment/core.py @@ -469,9 +469,18 @@ class Manager(manager.Manager): if user_id: return [create_group_assignment(ref, user_id=user_id)] + # Note(prashkre): Try to get the users in a group, + # if a group wasn't found in the backend, users are set + # as empty list. + try: + users = self.identity_api.list_users_in_group(ref['group_id']) + except exception.GroupNotFound: + LOG.warning('Group %(group)s was not found but still has role ' + 'assignments.', {'group': ref['group_id']}) + users = [] + return [create_group_assignment(ref, user_id=m['id']) - for m in self.identity_api.list_users_in_group( - ref['group_id'])] + for m in users] def expand_inherited_assignment(ref, user_id, project_id, subtree_ids, expand_groups): diff --git a/keystone/tests/unit/assignment/test_backends.py b/keystone/tests/unit/assignment/test_backends.py index fbcc248547..c585ed1935 100644 --- a/keystone/tests/unit/assignment/test_backends.py +++ b/keystone/tests/unit/assignment/test_backends.py @@ -19,6 +19,7 @@ from testtools import matchers import keystone.conf from keystone import exception from keystone.tests import unit +from keystone.tests.unit import default_fixtures CONF = keystone.conf.CONF @@ -605,24 +606,73 @@ class AssignmentTests(AssignmentTestHelperMixin): def _group_not_found(value): raise exception.GroupNotFound(group_id=value) - # Note(knikolla): Patch get_group to return GroupNotFound - # this simulates the case of a group being deleted - # directly in the backend and still having lingering role - # assignments. + # Setup + # 1) Remove any pre-existing assignments so we control what's there + for a in self.assignment_api.list_role_assignments(): + self.assignment_api.delete_grant(**a) + # 2) create a group and 2 users in that group + domain_id = CONF.identity.default_domain_id + group = self.identity_api.create_group( + unit.new_group_ref(domain_id=domain_id)) + user1 = self.identity_api.create_user( + unit.new_user_ref(domain_id=domain_id)) + user2 = self.identity_api.create_user( + unit.new_user_ref(domain_id=domain_id)) + self.identity_api.add_user_to_group(user1['id'], group['id']) + self.identity_api.add_user_to_group(user2['id'], group['id']) + # 3) create a role assignment for the group + self.assignment_api.create_grant( + group_id=group['id'], + domain_id=domain_id, + role_id=default_fixtures.MEMBER_ROLE_ID) + + num_assignments = len(self.assignment_api.list_role_assignments()) + self.assertEqual(1, num_assignments) + + # Patch get_group to return GroupNotFound, allowing us to confirm + # that the exception is handled properly when include_names processing + # attempts to lookup a group that has been deleted in the backend with mock.patch.object(self.identity_api, 'get_group', _group_not_found): assignment_list = self.assignment_api.list_role_assignments( include_names=True ) - self.assertNotEqual([], assignment_list) + self.assertEqual(num_assignments, len(assignment_list)) for assignment in assignment_list: + includes_group_assignments = False if 'group_name' in assignment: - # Note(knikolla): In the case of a not found group we + includes_group_assignments = True + # Note(knikolla): In the case of a not-found group we # populate the values with empty strings. self.assertEqual('', assignment['group_name']) self.assertEqual('', assignment['group_domain_id']) self.assertEqual('', assignment['group_domain_name']) + self.assertTrue(includes_group_assignments) + + num_effective = len(self.assignment_api.list_role_assignments( + effective=True)) + self.assertGreater(num_effective, len(assignment_list)) + + # Patch list_users_in_group to return GroupNotFound allowing us to + # confirm that the exception is handled properly when effective + # processing attempts to lookup users for a group that has been deleted + # in the backend + with mock.patch.object(self.identity_api, 'list_users_in_group', + _group_not_found): + assignment_list = self.assignment_api.list_role_assignments( + effective=True + ) + + self.assertGreater(num_effective, len(assignment_list)) + + # cleanup + self.assignment_api.delete_grant( + group_id=group['id'], + domain_id=domain_id, + role_id=default_fixtures.MEMBER_ROLE_ID) + # TODO(edmondsw) should cleanup users/groups as well, but that raises + # LDAP read-only issues def test_add_duplicate_role_grant(self): roles_ref = self.assignment_api.get_roles_for_user_and_project( diff --git a/keystone/tests/unit/default_fixtures.py b/keystone/tests/unit/default_fixtures.py index 1fd48f028e..784b1dd90d 100644 --- a/keystone/tests/unit/default_fixtures.py +++ b/keystone/tests/unit/default_fixtures.py @@ -20,6 +20,7 @@ BAR_TENANT_ID = uuid.uuid4().hex BAZ_TENANT_ID = uuid.uuid4().hex MTU_TENANT_ID = uuid.uuid4().hex SERVICE_TENANT_ID = uuid.uuid4().hex +MEMBER_ROLE_ID = uuid.uuid4().hex DEFAULT_DOMAIN_ID = 'default' TENANTS = [ @@ -140,6 +141,10 @@ ROLES = [ 'id': 'service', 'name': 'Service', 'domain_id': None, + }, { + 'id': MEMBER_ROLE_ID, + 'name': 'member', + 'domain_id': None, } ]