Skip to content

Commit

Permalink
Fix role lookup for Active Directory
Browse files Browse the repository at this point in the history
When using Keystone against an Active Directory server, assigned
roles weren't found for users.

When roles are added as DNs in the roleOccupant attribute, an LDAP
server can normalize the value so that when the entry is read later
the roleOccupant isn't exactly the same as it was when added.
Keystone should compare users by ID rather than by DN.
(Note that this is how the comparison is done in Grizzly.)

Keystone's fake LDAP is changed to muck with roleOccupant and
member DNs by uppercasing attribute names (like Active Directory).
The code is fixed to compare users by ID rather than DN.

Change-Id: Iaa41c3ef9febcabef0662f38b13d319a5b5583bc
Resolves-Bug: #1210675
  • Loading branch information
Brant Knudson committed Aug 23, 2013
1 parent 4dbda64 commit e4b1d22
Show file tree
Hide file tree
Showing 2 changed files with 31 additions and 14 deletions.
3 changes: 1 addition & 2 deletions keystone/assignment/backends/ldap.py
Expand Up @@ -93,11 +93,10 @@ def _get_metadata(self, user_id=None, tenant_id=None,
def _get_roles_for_just_user_and_project(user_id, tenant_id):
self.identity_api.get_user(user_id)
self.get_project(tenant_id)
user_dn = self.user._id_to_dn(user_id)
return [self.role._dn_to_id(a.role_dn)
for a in self.role.get_role_assignments
(self.project._id_to_dn(tenant_id))
if a.user_dn == user_dn]
if self.user._dn_to_id(a.user_dn) == user_id]

if domain_id is not None:
msg = 'Domain metadata not supported by LDAP'
Expand Down
42 changes: 30 additions & 12 deletions keystone/common/ldap/fakeldap.py
Expand Up @@ -43,6 +43,27 @@ class definitions. It implements the minimum emulation of the python ldap
LOG = logging.getLogger(__name__)


def _process_attr(attr_name, value_or_values):
attr_fn = lambda x: x

def normalize_dn(dn):
# Capitalize the attribute names as an LDAP server might.
dn = ldap.dn.str2dn(dn)
norm = []
for part in dn:
name, val, i = part[0]
name = name.upper()
norm.append([(name, val, i)])
return ldap.dn.dn2str(norm)

if attr_name in ('member', 'roleOccupant'):
attr_fn = normalize_dn

if isinstance(value_or_values, list):
return [attr_fn(x) for x in value_or_values]
return [attr_fn(value_or_values)]


def _match_query(query, attrs):
"""Match an ldap query to an attribute dictionary.
Expand Down Expand Up @@ -94,7 +115,7 @@ def _match(key, value, attrs):
str_sids = [str(x) for x in attrs[key]]
return str(value) in str_sids
if key != 'objectclass':
return value in attrs[key]
return _process_attr(key, value)[0] in attrs[key]
# it is an objectclass check, so check subclasses
values = _subs(value)
for v in values:
Expand Down Expand Up @@ -190,7 +211,7 @@ def add_s(self, dn, attrs):
' already in store.'), dn)
raise ldap.ALREADY_EXISTS(dn)

self.db[key] = dict([(k, v if isinstance(v, list) else [v])
self.db[key] = dict([(k, _process_attr(k, v))
for k, v in attrs])
self.db.sync()

Expand Down Expand Up @@ -244,14 +265,13 @@ def modify_s(self, dn, attrs):
for cmd, k, v in attrs:
values = entry.setdefault(k, [])
if cmd == ldap.MOD_ADD:
if v in values:
raise ldap.TYPE_OR_VALUE_EXISTS
if isinstance(v, list):
values += v
else:
values.append(v)
v = _process_attr(k, v)
for x in v:
if x in values:
raise ldap.TYPE_OR_VALUE_EXISTS
values += v
elif cmd == ldap.MOD_REPLACE:
values[:] = v if isinstance(v, list) else [v]
values[:] = _process_attr(k, v)
elif cmd == ldap.MOD_DELETE:
if v is None:
if len(values) == 0:
Expand All @@ -260,9 +280,7 @@ def modify_s(self, dn, attrs):
raise ldap.NO_SUCH_ATTRIBUTE
values[:] = []
else:
if not isinstance(v, list):
v = [v]
for val in v:
for val in _process_attr(k, v):
try:
values.remove(val)
except ValueError:
Expand Down

0 comments on commit e4b1d22

Please sign in to comment.