Browse Source

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
(cherry picked from commit a1dc21f3d3)
changes/50/672350/1
Raildo Mascena 5 months ago
parent
commit
909cc9fa83

+ 32
- 3
keystone/identity/backends/ldap/common.py View File

@@ -1293,9 +1293,38 @@ class BaseLdap(object):
1293 1293
         else:
1294 1294
             return self._id_to_dn_string(object_id)
1295 1295
 
1296
-    @staticmethod
1297
-    def _dn_to_id(dn):
1298
-        return ldap.dn.str2dn(dn)[0][0][1]
1296
+    def _dn_to_id(self, dn):
1297
+        # Check if the naming attribute in the DN is the same as keystone's
1298
+        # configured 'id' attribute'.  If so, extract the ID value from the DN
1299
+        if self.id_attr == utf8_decode(
1300
+                ldap.dn.str2dn(utf8_encode(dn))[0][0][0].lower()):
1301
+            return utf8_decode(ldap.dn.str2dn(utf8_encode(dn))[0][0][1])
1302
+        else:
1303
+            # The 'ID' attribute is NOT in the DN, so we need to perform an
1304
+            # LDAP search to look it up from the user entry itself.
1305
+            with self.get_connection() as conn:
1306
+                search_result = conn.search_s(dn, ldap.SCOPE_BASE)
1307
+
1308
+            if search_result:
1309
+                try:
1310
+                    id_list = search_result[0][1][self.id_attr]
1311
+                except KeyError:
1312
+                    message = ('ID attribute %(id_attr)s not found in LDAP '
1313
+                               'object %(dn)s.') % ({'id_attr': self.id_attr,
1314
+                                                     'dn': search_result})
1315
+                    LOG.warning(message)
1316
+                    raise exception.NotFound(message=message)
1317
+                if len(id_list) > 1:
1318
+                    message = ('In order to keep backward compatibility, in '
1319
+                               'the case of multivalued ids, we are '
1320
+                               'returning the first id %(id_attr) in the '
1321
+                               'DN.') % ({'id_attr': id_list[0]})
1322
+                    LOG.warning(message)
1323
+                return id_list[0]
1324
+            else:
1325
+                message = _('DN attribute %(dn)s not found in LDAP') % (
1326
+                    {'dn': dn})
1327
+                raise exception.NotFound(message=message)
1299 1328
 
1300 1329
     def _ldap_res_to_model(self, res):
1301 1330
         # LDAP attribute names may be returned in a different case than

+ 5
- 2
keystone/identity/backends/ldap/core.py View File

@@ -311,8 +311,11 @@ class UserApi(common_ldap.EnabledEmuMixIn, common_ldap.BaseLdap):
311 311
         return obj
312 312
 
313 313
     def get_filtered(self, user_id):
314
-        user = self.get(user_id)
315
-        return self.filter_attributes(user)
314
+        try:
315
+            user = self.get(user_id)
316
+            return self.filter_attributes(user)
317
+        except ldap.NO_SUCH_OBJECT:
318
+            raise self.NotFound(user_id=user_id)
316 319
 
317 320
     def get_all(self, ldap_filter=None, hints=None):
318 321
         objs = super(UserApi, self).get_all(ldap_filter=ldap_filter,

+ 50
- 5
keystone/tests/unit/test_backend_ldap.py View File

@@ -1819,7 +1819,33 @@ class LDAPIdentity(BaseLDAPIdentity):
1819 1819
         self.assertEqual(self.user_foo['email'], user_ref['id'])
1820 1820
 
1821 1821
     @mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
1822
-    def test_get_id_from_dn_for_multivalued_attribute_id(self, mock_ldap_get):
1822
+    def test_get_multivalued_attribute_id_from_dn(self,
1823
+                                                  mock_ldap_get):
1824
+        driver = PROVIDERS.identity_api._select_identity_driver(
1825
+            CONF.identity.default_domain_id)
1826
+        driver.user.id_attr = 'mail'
1827
+
1828
+        # make 'email' multivalued so we can test the error condition
1829
+        email1 = uuid.uuid4().hex
1830
+        email2 = uuid.uuid4().hex
1831
+        # Mock the ldap search results to return user entries with
1832
+        # user_name_attribute('sn') value has emptyspaces, emptystring
1833
+        # and attibute itself is not set.
1834
+        mock_ldap_get.return_value = (
1835
+            'cn=users,dc=example,dc=com',
1836
+            {
1837
+                'mail': [email1, email2],
1838
+            }
1839
+        )
1840
+
1841
+        # This is not a valid scenario, since we do not support multiple value
1842
+        # attribute id on DN.
1843
+        self.assertRaises(exception.NotFound,
1844
+                          PROVIDERS.identity_api.get_user, email1)
1845
+
1846
+    @mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
1847
+    def test_raise_not_found_dn_for_multivalued_attribute_id(self,
1848
+                                                             mock_ldap_get):
1823 1849
         driver = PROVIDERS.identity_api._select_identity_driver(
1824 1850
             CONF.identity.default_domain_id)
1825 1851
         driver.user.id_attr = 'mail'
@@ -1836,10 +1862,29 @@ class LDAPIdentity(BaseLDAPIdentity):
1836 1862
             }
1837 1863
         )
1838 1864
 
1839
-        user_ref = PROVIDERS.identity_api.get_user(email1)
1840
-        # make sure we get the ID from DN (old behavior) if the ID attribute
1841
-        # has multiple values
1842
-        self.assertEqual('nobodycares', user_ref['id'])
1865
+        # This is not a valid scenario, since we do not support multiple value
1866
+        # attribute id on DN.
1867
+        self.assertRaises(exception.NotFound,
1868
+                          PROVIDERS.identity_api.get_user, email1)
1869
+
1870
+    @mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
1871
+    def test_get_id_not_in_dn(self,
1872
+                              mock_ldap_get):
1873
+        driver = PROVIDERS.identity_api._select_identity_driver(
1874
+            CONF.identity.default_domain_id)
1875
+        driver.user.id_attr = 'sAMAccountName'
1876
+
1877
+        user_id = uuid.uuid4().hex
1878
+        mock_ldap_get.return_value = (
1879
+            'cn=someuser,dc=example,dc=com',
1880
+            {
1881
+                'cn': 'someuser',
1882
+                'sn': [uuid.uuid4().hex],
1883
+                'sAMAccountName': [user_id],
1884
+            }
1885
+        )
1886
+        user_ref = PROVIDERS.identity_api.get_user(user_id)
1887
+        self.assertEqual(user_id, user_ref['id'])
1843 1888
 
1844 1889
     @mock.patch.object(common_ldap.BaseLdap, '_ldap_get')
1845 1890
     def test_id_attribute_not_found(self, mock_ldap_get):

+ 10
- 0
releasenotes/notes/bug-1782922-db822fda486ac773.yaml View File

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

Loading…
Cancel
Save