Allow cleaning up non-existant group assignments

If a group gets deleted out-of-band in an LDAP environment, the role
assignments cannot be cleaned as it checks the existence of the group
before triggering the deletion. This fix adds the ability to ignore
non-existant group and clean up stale role assignments. We take the
same approach with user assignments.

Co-Authored-By: Lance Bragstad <lbragstad@gmail.com>

Change-Id: I975c8325f50b412c3aa256e1940a27082c009cce
Closes-Bug: #1751045
This commit is contained in:
Jose Castro Leon 2018-02-22 13:32:23 +01:00 committed by Lance Bragstad
parent 1f477ea3b4
commit 1ab693ced8
3 changed files with 47 additions and 5 deletions

View File

@ -392,7 +392,7 @@ class GrantAssignmentV3(controller.V3Controller):
def _check_grant_protection(self, request, protection, role_id=None, def _check_grant_protection(self, request, protection, role_id=None,
user_id=None, group_id=None, user_id=None, group_id=None,
domain_id=None, project_id=None, domain_id=None, project_id=None,
allow_no_user=False): allow_non_existing=False):
"""Check protection for role grant APIs. """Check protection for role grant APIs.
The policy rule might want to inspect attributes of any of the entities The policy rule might want to inspect attributes of any of the entities
@ -407,10 +407,14 @@ class GrantAssignmentV3(controller.V3Controller):
try: try:
ref['user'] = PROVIDERS.identity_api.get_user(user_id) ref['user'] = PROVIDERS.identity_api.get_user(user_id)
except exception.UserNotFound: except exception.UserNotFound:
if not allow_no_user: if not allow_non_existing:
raise raise
else: else:
ref['group'] = PROVIDERS.identity_api.get_group(group_id) try:
ref['group'] = PROVIDERS.identity_api.get_group(group_id)
except exception.GroupNotFound:
if not allow_non_existing:
raise
# NOTE(lbragstad): This if/else check will need to be expanded in the # NOTE(lbragstad): This if/else check will need to be expanded in the
# future to handle system hierarchies if that is implemented. # future to handle system hierarchies if that is implemented.
@ -465,7 +469,7 @@ class GrantAssignmentV3(controller.V3Controller):
# from the backend in the event the user was removed prior to the role # from the backend in the event the user was removed prior to the role
# assignment being removed. # assignment being removed.
@controller.protected(callback=functools.partial( @controller.protected(callback=functools.partial(
_check_grant_protection, allow_no_user=True)) _check_grant_protection, allow_non_existing=True))
def revoke_grant(self, request, role_id, user_id=None, def revoke_grant(self, request, role_id, user_id=None,
group_id=None, domain_id=None, project_id=None): group_id=None, domain_id=None, project_id=None):
"""Revoke a role from user/group on either a domain or project.""" """Revoke a role from user/group on either a domain or project."""
@ -513,7 +517,7 @@ class GrantAssignmentV3(controller.V3Controller):
PROVIDERS.assignment_api.create_system_grant_for_user(user_id, role_id) PROVIDERS.assignment_api.create_system_grant_for_user(user_id, role_id)
@controller.protected(callback=functools.partial( @controller.protected(callback=functools.partial(
_check_grant_protection, allow_no_user=True)) _check_grant_protection, allow_non_existing=True))
def revoke_system_grant_for_user(self, request, role_id, user_id): def revoke_system_grant_for_user(self, request, role_id, user_id):
"""Revoke a role from user on the system. """Revoke a role from user on the system.

View File

@ -365,6 +365,37 @@ class AssignmentTestCase(test_v3.RestfulTestCase,
# Make sure the role is gone # Make sure the role is gone
self.head(member_url, expected_status=http_client.NOT_FOUND) self.head(member_url, expected_status=http_client.NOT_FOUND)
def test_delete_group_before_removing_role_assignment_succeeds(self):
# Disable the cache so that we perform a fresh check of the identity
# backend when attempting to remove the role assignment.
self.config_fixture.config(group='cache', enabled=False)
# Create a new group
group = unit.new_group_ref(domain_id=self.domain_id)
group_ref = PROVIDERS.identity_api.create_group(group)
# Assign the user a role on the project
collection_url = (
'/projects/%(project_id)s/groups/%(group_id)s/roles' % {
'project_id': self.project_id,
'group_id': group_ref['id']})
member_url = ('%(collection_url)s/%(role_id)s' % {
'collection_url': collection_url,
'role_id': self.role_id})
self.put(member_url)
# Check the user has the role assigned
self.head(member_url)
self.get(member_url, expected_status=http_client.NO_CONTENT)
# Simulate removing the group via LDAP by directly removing it from the
# identity backend.
PROVIDERS.identity_api.driver.delete_group(group_ref['id'])
# Ensure we can clean up the role assignment even though the group
# doesn't exist
self.delete(member_url)
def test_delete_user_before_removing_system_assignments_succeeds(self): def test_delete_user_before_removing_system_assignments_succeeds(self):
system_role = self._create_new_role() system_role = self._create_new_role()
user = self._create_user() user = self._create_user()

View File

@ -0,0 +1,7 @@
---
fixes:
- |
[`bug 1751045 <https://bugs.launchpad.net/keystone/+bug/1751045>`_]
It is now possible to clean up role assignments for groups that don't exist
in the identity backend. This is relevant to deployments that are backed by
LDAP and groups are removed directly by LDAP and not through keystone.