Fixing dn_to_id function for cases were id is not in the DN

The more common scenario to return the uid as part of the RDN in a DN,
However, it's a valid case to not have the uid in the RDN, so we need to
search in the LDAP based on the DN and return the uid in the entire object.

Also, we do not support multivalued attribute id on DN, so the test case
covering this case, it was adjusted for raise NotFound.

Closes-Bug: 1782922
Change-Id: I87a3bfa94b5907ce4c6b4eb8e124ec948b390bf2
This commit is contained in:
Raildo Mascena 2019-04-01 16:48:07 -03:00 committed by Guang Yee
parent 7ae65a2f7c
commit a1dc21f3d3
4 changed files with 97 additions and 10 deletions

View File

@ -1293,9 +1293,38 @@ class BaseLdap(object):
else:
return self._id_to_dn_string(object_id)
@staticmethod
def _dn_to_id(dn):
return ldap.dn.str2dn(dn)[0][0][1]
def _dn_to_id(self, dn):
# Check if the naming attribute in the DN is the same as keystone's
# configured 'id' attribute'. If so, extract the ID value from the DN
if self.id_attr == utf8_decode(
ldap.dn.str2dn(utf8_encode(dn))[0][0][0].lower()):
return utf8_decode(ldap.dn.str2dn(utf8_encode(dn))[0][0][1])
else:
# The 'ID' attribute is NOT in the DN, so we need to perform an
# LDAP search to look it up from the user entry itself.
with self.get_connection() as conn:
search_result = conn.search_s(dn, ldap.SCOPE_BASE)
if search_result:
try:
id_list = search_result[0][1][self.id_attr]
except KeyError:
message = ('ID attribute %(id_attr)s not found in LDAP '
'object %(dn)s.') % ({'id_attr': self.id_attr,
'dn': search_result})
LOG.warning(message)
raise exception.NotFound(message=message)
if len(id_list) > 1:
message = ('In order to keep backward compatibility, in '
'the case of multivalued ids, we are '
'returning the first id %(id_attr) in the '
'DN.') % ({'id_attr': id_list[0]})
LOG.warning(message)
return id_list[0]
else:
message = _('DN attribute %(dn)s not found in LDAP') % (
{'dn': dn})
raise exception.NotFound(message=message)
def _ldap_res_to_model(self, res):
# LDAP attribute names may be returned in a different case than

View File

@ -311,8 +311,11 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
return obj
def get_filtered(self, user_id):
user = self.get(user_id)
return self.filter_attributes(user)
try:
user = self.get(user_id)
return self.filter_attributes(user)
except ldap.NO_SUCH_OBJECT:
raise self.NotFound(user_id=user_id)
def get_all(self, ldap_filter=None, hints=None):
objs = super(UserApi, self).get_all(ldap_filter=ldap_filter,

View File

@ -1819,7 +1819,33 @@ class LDAPIdentity(BaseLDAPIdentity):
self.assertEqual(self.user_foo['email'], user_ref['id'])
@mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
def test_get_id_from_dn_for_multivalued_attribute_id(self, mock_ldap_get):
def test_get_multivalued_attribute_id_from_dn(self,
mock_ldap_get):
driver = PROVIDERS.identity_api._select_identity_driver(
CONF.identity.default_domain_id)
driver.user.id_attr = 'mail'
# make 'email' multivalued so we can test the error condition
email1 = uuid.uuid4().hex
email2 = uuid.uuid4().hex
# Mock the ldap search results to return user entries with
# user_name_attribute('sn') value has emptyspaces, emptystring
# and attibute itself is not set.
mock_ldap_get.return_value = (
'cn=users,dc=example,dc=com',
{
'mail': [email1, email2],
}
)
# This is not a valid scenario, since we do not support multiple value
# attribute id on DN.
self.assertRaises(exception.NotFound,
PROVIDERS.identity_api.get_user, email1)
@mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
def test_raise_not_found_dn_for_multivalued_attribute_id(self,
mock_ldap_get):
driver = PROVIDERS.identity_api._select_identity_driver(
CONF.identity.default_domain_id)
driver.user.id_attr = 'mail'
@ -1836,10 +1862,29 @@ class LDAPIdentity(BaseLDAPIdentity):
}
)
user_ref = PROVIDERS.identity_api.get_user(email1)
# make sure we get the ID from DN (old behavior) if the ID attribute
# has multiple values
self.assertEqual('nobodycares', user_ref['id'])
# This is not a valid scenario, since we do not support multiple value
# attribute id on DN.
self.assertRaises(exception.NotFound,
PROVIDERS.identity_api.get_user, email1)
@mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
def test_get_id_not_in_dn(self,
mock_ldap_get):
driver = PROVIDERS.identity_api._select_identity_driver(
CONF.identity.default_domain_id)
driver.user.id_attr = 'sAMAccountName'
user_id = uuid.uuid4().hex
mock_ldap_get.return_value = (
'cn=someuser,dc=example,dc=com',
{
'cn': 'someuser',
'sn': [uuid.uuid4().hex],
'sAMAccountName': [user_id],
}
)
user_ref = PROVIDERS.identity_api.get_user(user_id)
self.assertEqual(user_id, user_ref['id'])
@mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
def test_id_attribute_not_found(self, mock_ldap_get):

View File

@ -0,0 +1,10 @@
---
fixes:
- |
[`bug 1782922 <https://bugs.launchpad.net/keystone/+bug/1782922>`_]
Fixed the problem where Keystone indiscriminately return the first RDN
as the user ID, regardless whether it matches the configured
'user_id_attribute' or not. This will break deployments where
'group_members_are_ids' are set to False and 'user_id_attribute' is not
in the DN. This patch will perform a lookup by DN if the first RND does
not match the configured 'user_id_attribute'.