From c1e96d42d3446614c2475b5716a075eac67ea73f Mon Sep 17 00:00:00 2001 From: Vishakha Agarwal Date: Tue, 18 Sep 2018 15:17:07 +0530 Subject: [PATCH] LDAP attribute names non-case-sensitive keystone was not able to find any users while the LDAP user name attribute was configured to "samaccountname", but could find users when reconfigured to use "sAMAccountName". LDAP is not supposed to be case-sensitive, so either should work. This patch addresses the above problem by making both the attributes into lower case. Also updated the ldap_result example supporting python3. Change-Id: I51813ac41489baed04f3cadbccd748e03025313e Closes-Bug: #1753585 (cherry picked from commit 816b472a9d20e4e7cfe33f2f40ef5daae590795e) --- keystone/identity/backends/ldap/common.py | 29 ++++++++---- keystone/tests/unit/test_backend_ldap.py | 47 +++++++++++++++++++ .../notes/bug-1753585-7e11213743754999.yaml | 6 +++ 3 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 releasenotes/notes/bug-1753585-7e11213743754999.yaml diff --git a/keystone/identity/backends/ldap/common.py b/keystone/identity/backends/ldap/common.py index 8d632aab74..a7b3e605ff 100644 --- a/keystone/identity/backends/ldap/common.py +++ b/keystone/identity/backends/ldap/common.py @@ -1402,20 +1402,31 @@ class BaseLdap(object): raise ValueError('"%(attr)s" is not a valid value for' ' "%(attr_name)s"' % {'attr': attr, 'attr_name': attr_name}) + + # consider attr = "cn" and + # ldap_result = [{'uid': ['fake_id1']}, , 'cN': ["name"]}] + # doing lower case on both user_name_attribute and ldap users + # attribute 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"]}] + # ldap_result = [(u'cn=fake1,o=ex_domain', {'uid': ['fake_id1']}), + # (u'cn=fake2,o=ex_domain', {'uid': ['fake_id2'], + # 'cn': [' ']}), + # (u'cn=fake3,o=ex_domain', {'uid': ['fake_id3'], + # 'cn': ['']}), + # (u'cn=fake4,o=ex_domain', {'uid': ['fake_id4'], + # 'cn': []}), + # (u'cn=fake5,o=ex_domain', {'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(): + ldap_res_low_keys_dict = {k.lower(): v for k, v in obj[1].items()} + result_attr_vals = ldap_res_low_keys_dict.get(attr.lower()) + # ignore ldap object whose attr value has empty strings or + # contains only whitespaces. + if result_attr_vals: + if result_attr_vals[0] and result_attr_vals[0].strip(): result.append(obj) # except {'uid': ['fake_id5'], 'cn': ["name"]}, all entries # will be ignored in ldap_result diff --git a/keystone/tests/unit/test_backend_ldap.py b/keystone/tests/unit/test_backend_ldap.py index 79f0c04bbe..c41f80f7cc 100644 --- a/keystone/tests/unit/test_backend_ldap.py +++ b/keystone/tests/unit/test_backend_ldap.py @@ -1305,6 +1305,53 @@ class LDAPIdentity(BaseLDAPIdentity): self.assertEqual('junk1', user_refs[0]['name']) self.assertEqual('sn=junk1,dc=example,dc=com', user_refs[0]['dn']) + @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_with_case_sensitive_attr(self, + mock_simple_bind_s, + mock_search_s, + mock_connect): + # Mock the ldap search results to return user entries + # irrespective of lowercase and uppercase characters in + # ldap_result attribute keys e.g. {'Sn': ['junk1']} with + # user_name_attribute('sn') + mock_search_s.return_value = [( + 'sn=junk1,dc=example,dc=com', + { + 'cn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex], + 'sN': ['junk1'] + } + ), + ( + 'sn=junk1,dc=example,dc=com', + { + 'cn': [uuid.uuid4().hex], + 'email': [uuid.uuid4().hex], + 'Sn': ['junk1'] + } + ), + ( + 'sn=junk1,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 keys having case insensitive characters. + self.assertEqual(2, 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-1753585-7e11213743754999.yaml b/releasenotes/notes/bug-1753585-7e11213743754999.yaml new file mode 100644 index 0000000000..83b77a0bf7 --- /dev/null +++ b/releasenotes/notes/bug-1753585-7e11213743754999.yaml @@ -0,0 +1,6 @@ +--- +fixes: + - | + ['bug 1753585 '_] + LDAP attribute names are now matched case insensitively to comply with + LDAP implementations. \ No newline at end of file