Add anonymous bind to get_connection method

If no username and password is specified in the keystone ldap
configuration, it may still be possible to bind to an LDAP server
anonymously if the LDAP server is configured to allow it. Currently,
upon creating a connection object, keystone only attempts to bind to
the LDAP server if a username and password has been provided to it.
This would rarely be an issue because pyldap attempts a reconnect upon
executing any ldap command, if necessary, and hence the anonymous bind
just happens later. It is a problem now because logic was added[1] to
check if the server errored during that initial connection, and for it
to work correctly the initial connection needs to happen in a
predictable place. This patch adds an anonymous bind to the
get_connection method so that no matter the credential configuration
the initial connection is consistent.

This required adding mocks to many of the LDAP backend tests since
every LDAP interaction now attempts a simple_bind_s() regardless of
whether credentials are configured in keystone.

[1] https://review.openstack.org/#/c/390948

Closes-bug: #1649138

Change-Id: I193c9537c107092e48f7ea1d25ff9c17f872c15b

keystone/identity/backends/ldap/common.py

@@ -1298,6 +1298,8 @@ class BaseLdap(object):
             # if we don't have any user/pass
             if user and password:
                 conn.simple_bind_s(user, password)
+            else:
+                conn.simple_bind_s()
 
             return conn
         except ldap.SERVER_DOWN:

keystone/tests/unit/fakeldap.py

@@ -305,19 +305,19 @@ class FakeLdap(common.LDAPHandler):
                 common.utf8_decode(cred) in ['password', CONF.ldap.password]):
             return
 
-        try:
-            attrs = self.db[self.key(who)]
-        except KeyError:
-            LOG.debug('bind fail: who=%s not found', common.utf8_decode(who))
-            raise ldap.NO_SUCH_OBJECT
-
-        db_password = None
-        try:
-            db_password = attrs['userPassword'][0]
-        except (KeyError, IndexError):
-            LOG.debug('bind fail: password for who=%s not found',
+        attrs = self.db.get(self.key(who))
+        if not attrs:
+            LOG.debug('who=%s not found, binding anonymously',
                       common.utf8_decode(who))
-            raise ldap.INAPPROPRIATE_AUTH
+
+        db_password = ''
+        if attrs:
+            try:
+                db_password = attrs['userPassword'][0]
+            except (KeyError, IndexError):
+                LOG.debug('bind fail: password for who=%s not found',
+                          common.utf8_decode(who))
+                raise ldap.INAPPROPRIATE_AUTH
 
         if cred != common.utf8_encode(db_password):
             LOG.debug('bind fail: password for who=%s does not match',

keystone/tests/unit/identity/backends/test_ldap_common.py

@@ -283,14 +283,16 @@ class LDAPDeleteTreeTest(unit.TestCase):
 class MultiURLTests(unit.TestCase):
     """Test for setting multiple LDAP URLs."""
 
-    def test_multiple_urls_with_comma_no_conn_pool(self):
+    @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s')
+    def test_multiple_urls_with_comma_no_conn_pool(self, mock_ldap_bind):
         urls = 'ldap://localhost,ldap://backup.localhost'
         self.config_fixture.config(group='ldap', url=urls, use_pool=False)
         base_ldap = common_ldap.BaseLdap(CONF)
         ldap_connection = base_ldap.get_connection()
         self.assertEqual(urls, ldap_connection.conn.conn._uri)
 
-    def test_multiple_urls_with_comma_with_conn_pool(self):
+    @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s')
+    def test_multiple_urls_with_comma_with_conn_pool(self, mock_ldap_bind):
         urls = 'ldap://localhost,ldap://backup.localhost'
         self.config_fixture.config(group='ldap', url=urls, use_pool=True)
         base_ldap = common_ldap.BaseLdap(CONF)
@@ -301,7 +303,8 @@ class MultiURLTests(unit.TestCase):
 class LDAPConnectionTimeoutTest(unit.TestCase):
     """Test for Network Connection timeout on LDAP URL connection."""
 
-    def test_connectivity_timeout_no_conn_pool(self):
+    @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s')
+    def test_connectivity_timeout_no_conn_pool(self, mock_ldap_bind):
         url = 'ldap://localhost'
         conn_timeout = 1  # 1 second
         self.config_fixture.config(group='ldap',
@@ -325,7 +328,8 @@ class LDAPConnectionTimeoutTest(unit.TestCase):
                          ldap.get_option(ldap.OPT_NETWORK_TIMEOUT))
         self.assertEqual(url, ldap_connection.conn.conn._uri)
 
-    def test_connectivity_timeout_with_conn_pool(self):
+    @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s')
+    def test_connectivity_timeout_with_conn_pool(self, mock_ldap_bind):
         url = 'ldap://localhost'
         conn_timeout = 1  # 1 second
         self.config_fixture.config(group='ldap',

keystone/tests/unit/test_backend_ldap.py

@@ -1409,13 +1409,12 @@ class LDAPIdentity(BaseLDAPIdentity, unit.TestCase):
 
     @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'simple_bind_s')
     def test_user_api_get_connection_no_user_password(self, mocked_method):
-        """Don't bind in case the user and password are blank."""
+        """Bind anonymously when the user and password are blank."""
         # Ensure the username/password are in-fact blank
         self.config_fixture.config(group='ldap', user=None, password=None)
         user_api = identity.backends.ldap.UserApi(CONF)
         user_api.get_connection(user=None, password=None)
-        self.assertFalse(mocked_method.called,
-                         msg='`simple_bind_s` method was unexpectedly called')
+        self.assertTrue(mocked_method.called)
 
     @mock.patch.object(common_ldap.KeystoneLDAPHandler, 'connect')
     def test_chase_referrals_off(self, mocked_fakeldap):

releasenotes/notes/bug-1649138-c53974f6bb0eab14.yaml

@@ -0,0 +1,9 @@
+---
+fixes:
+  - >
+    [`bug 1649138 <https://bugs.launchpad.net/keystone/+bug/1649138>`_]
+    When using LDAP as an identity backend, the initial bind will now
+    occur upon creation of a connection object, i.e. early on when
+    performing LDAP queries, no matter whether the bind is
+    authenticated or anonymous, so that any connection errors can be
+    handled correctly and early.