From 789573a0f17fd3ea8abd1a89034b865035925a8f Mon Sep 17 00:00:00 2001
From: prashkre <prashkre@in.ibm.com>
Date: Thu, 26 Oct 2017 18:47:33 +0530
Subject: [PATCH] Filter users/groups in ldap with whitespaces

All users and groups are required to have a name. With this fix,
Keystone will 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.

Change-Id: Id539e1b7e1cea8b05cd9bb753707e1fc98244d29
Closes-Bug: #1727726
---
 keystone/identity/backends/ldap/common.py     | 20 ++++++-
 keystone/tests/unit/test_backend_ldap.py      | 52 +++++++++++++++++++
 .../notes/bug-1727726-0b47608811a2cd16.yaml   |  9 ++++
 3 files changed, 79 insertions(+), 2 deletions(-)
 create mode 100644 releasenotes/notes/bug-1727726-0b47608811a2cd16.yaml

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 <https://bugs.launchpad.net/keystone/+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.