Browse Source

Merge "Fixing dn_to_id function for cases were id is not in the DN" into stable/rocky

stable/rocky
Zuul 2 weeks ago
parent
commit
10a3517042

+ 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

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