Handle group NotFound in effective assignment list
When keystone is using an external identity backend such as LDAP for
storing users and groups, but storing role assignments in the local db,
and a group that has role assignments is deleted out-of-band, its
assignments will still exist in the keystone database. If, after this,
a user attempts to list effective role assignments, keystone will try
to lookup the group and fail with NotFound.
This catches the NotFound exception of the list_users_in_group call and
returns an empty user list so that the effective assignments list does
not fail.
Closes-Bug: 1693510
Change-Id: Ie5f69b150d59287bd0bc68f1ce9eecfeab04c91a
(cherry picked from commit d09c337619
)
This commit is contained in:
parent
736149c0cd
commit
2fdf89554f
|
@ -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):
|
||||
|
|
|
@ -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(
|
||||
|
|
|
@ -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,
|
||||
}
|
||||
]
|
||||
|
||||
|
|
Loading…
Reference in New Issue