diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index 855ee46ab6..8d632aab74 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -1402,8 +1402,24 @@ class BaseLdap(object): raise ValueError('"%(attr)s" is not a valid value for' ' "%(attr_name)s"' % {'attr': attr, 'attr_name': attr_name}) - return [obj for obj in ldap_result - if obj[1].get(attr) and obj[1].get(attr)[0]] + result = [] + # consider attr = "cn" and + # ldap_result = [{'uid': ['fake_id1']}, + # {'uid': ['fake_id2'], 'cn': [' ']}, + # {'uid': ['fake_id3'], 'cn': ['']}, + # {'uid': ['fake_id4'], 'cn': []}, + # {'uid': ['fake_id5'], 'cn': ["name"]}] + for obj in ldap_result: + # ignore ldap object(user/group entry) which has no attr set + # in it or whose value is empty list. + if obj[1].get(attr): + # ignore ldap object whose attr value has empty strings or + # contains only whitespaces. + if obj[1].get(attr)[0] and obj[1].get(attr)[0].strip(): + result.append(obj) + # except {'uid': ['fake_id5'], 'cn': ["name"]}, all entries + # will be ignored in ldap_result + return result def _ldap_get(self, object_id, ldap_filter=None): query = (u'(&(%(id_attr)s=%(id)s)' diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index b83f39a67e..a113566893 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -1198,6 +1198,58 @@ class LDAPIdentity(BaseLDAPIdentity): # from the resource default. self.assertIs(True, user_ref['enabled']) + @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'connect') + @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'search_s') + @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s') + def test_filter_ldap_result_by_attr(self, mock_simple_bind_s, + mock_search_s, mock_connect): + + # 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_search_s.return_value = [( + 'sn=junk1,dc=example,dc=com', + { + 'cn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex], + 'sn': ['junk1'] + } + ), + ( + '', + { + 'cn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex], + } + ), + ( + 'sn=,dc=example,dc=com', + { + 'cn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex], + 'sn': [''] + } + ), + ( + 'sn= ,dc=example,dc=com', + { + 'cn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex], + 'sn': [' '] + } + )] + + user_api = identity.backends.ldap.UserApi(CONF) + user_refs = user_api.get_all() + # validate that keystone.identity.backends.ldap.common.BaseLdap. + # _filter_ldap_result_by_attr() method filtered the ldap query results + # whose name attribute values has emptyspaces, emptystring + # and attibute itself is not set. + self.assertEqual(1, len(user_refs)) + + self.assertEqual('junk1', user_refs[0]['name']) + self.assertEqual('sn=junk1,dc=example,dc=com', user_refs[0]['dn']) + @mock.patch.object(common_ldap.BaseLdap, '_ldap_get') def test_user_enabled_attribute_handles_expired(self, mock_ldap_get): # If using 'passwordisexpired' as enabled attribute, and inverting it, diff --git a/releasenotes/notes/bug-1727726-0b47608811a2cd16.yaml b/releasenotes/notes/bug-1727726-0b47608811a2cd16.yaml new file mode 100644 index 0000000000..b10285e881 --- /dev/null +++ b/releasenotes/notes/bug-1727726-0b47608811a2cd16.yaml @@ -0,0 +1,9 @@ +--- +fixes: + - | + [`bug 1727726 `_] + All users and groups are required to have a name. Prior to this fix, + Keystone was allowing LDAP users and groups whose name has only empty + white spaces. Keystone will now ignore users and groups that do have + only white spaces as value for the LDAP attribute which Keystone has + been configured to use for that entity's name.