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/30/674030/2
Raildo Mascena 5 months ago
parent
commit
9d9451e13c

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

@@ -1281,9 +1281,38 @@ class BaseLdap(object):
1281 1281
         else:
1282 1282
             return self._id_to_dn_string(object_id)
1283 1283
 
1284
-    @staticmethod
1285
-    def _dn_to_id(dn):
1286
-        return utf8_decode(ldap.dn.str2dn(utf8_encode(dn))[0][0][1])
1284
+    def _dn_to_id(self, dn):
1285
+        # Check if the naming attribute in the DN is the same as keystone's
1286
+        # configured 'id' attribute'.  If so, extract the ID value from the DN
1287
+        if self.id_attr == utf8_decode(
1288
+                ldap.dn.str2dn(utf8_encode(dn))[0][0][0].lower()):
1289
+            return utf8_decode(ldap.dn.str2dn(utf8_encode(dn))[0][0][1])
1290
+        else:
1291
+            # The 'ID' attribute is NOT in the DN, so we need to perform an
1292
+            # LDAP search to look it up from the user entry itself.
1293
+            with self.get_connection() as conn:
1294
+                search_result = conn.search_s(dn, ldap.SCOPE_BASE)
1295
+
1296
+            if search_result:
1297
+                try:
1298
+                    id_list = search_result[0][1][self.id_attr]
1299
+                except KeyError:
1300
+                    message = ('ID attribute %(id_attr)s not found in LDAP '
1301
+                               'object %(dn)s.') % ({'id_attr': self.id_attr,
1302
+                                                     'dn': search_result})
1303
+                    LOG.warning(message)
1304
+                    raise exception.NotFound(message=message)
1305
+                if len(id_list) > 1:
1306
+                    message = ('In order to keep backward compatibility, in '
1307
+                               'the case of multivalued ids, we are '
1308
+                               'returning the first id %(id_attr) in the '
1309
+                               'DN.') % ({'id_attr': id_list[0]})
1310
+                    LOG.warning(message)
1311
+                return id_list[0]
1312
+            else:
1313
+                message = _('DN attribute %(dn)s not found in LDAP') % (
1314
+                    {'dn': dn})
1315
+                raise exception.NotFound(message=message)
1287 1316
 
1288 1317
     def _ldap_res_to_model(self, res):
1289 1318
         # 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

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