Skip to content

Commit

Permalink
Fix remove role assignment adds role using LDAP assignment
Browse files Browse the repository at this point in the history
When using the LDAP assignment backend, attempting to remove a
role assignment when the role hadn't been used before would
actually add the role assignment and would not return a
404 Not Found like the SQL backend.

This change makes it so that when attempt to remove a role that
wasn't assigned then 404 Not Found is returned.

Closes-Bug: #1242855
Change-Id: I28ccd26cc4bb1a241d0363d0ab52d2c11410e8b3
  • Loading branch information
Brant Knudson authored and dolph committed Oct 22, 2013
1 parent b17e7be commit c6800ca
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 22 deletions.
18 changes: 4 additions & 14 deletions keystone/assignment/backends/ldap.py
Original file line number Diff line number Diff line change
Expand Up @@ -451,20 +451,10 @@ def delete_user(self, role_dn, user_dn, tenant_dn,
try:
conn.modify_s(role_dn, [(ldap.MOD_DELETE,
self.member_attribute, user_dn)])
except ldap.NO_SUCH_OBJECT:
if tenant_dn is None:
raise exception.RoleNotFound(role_id=role_id)
attrs = [('objectClass', [self.object_class]),
(self.member_attribute, [user_dn])]

if self.use_dumb_member:
attrs[1][1].append(self.dumb_member)
try:
conn.add_s(role_dn, attrs)
except Exception as inst:
raise inst
except ldap.NO_SUCH_ATTRIBUTE:
raise exception.UserNotFound(user_id=user_id)
except (ldap.NO_SUCH_OBJECT, ldap.NO_SUCH_ATTRIBUTE):
raise exception.RoleNotFound(message=_(
'Cannot remove role that has not been granted, %s') %
role_id)
finally:
conn.unbind_s()

Expand Down
8 changes: 0 additions & 8 deletions keystone/tests/test_backend_ldap.py
Original file line number Diff line number Diff line change
Expand Up @@ -870,14 +870,6 @@ def test_list_projects_for_alternate_domain(self):
self.skipTest(
'N/A: LDAP does not support multiple domains')

def test_remove_user_role_not_assigned(self):
# This raises exception as expected with SQL assignment backend but
# not with LDAP (see bug #1242855)
self.identity_api.remove_role_from_user_and_project(
tenant_id=self.tenant_bar['id'],
user_id=self.user_two['id'],
role_id=self.role_other['id'])


class LDAPIdentityEnabledEmulation(LDAPIdentity):
def setUp(self):
Expand Down

0 comments on commit c6800ca

Please sign in to comment.