From: Dan Brown <redacted>
Date: Sun, 16 Oct 2022 21:15:15 +0000 (+0100)
Subject: Added ldap host failover test, updated error handling
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/392eef82730478000e6a87060f02e88111fd02a1

Added ldap host failover test, updated error handling

Error handling updated after testing, since the ldap 'starttls'
operation can throw an error, or maybe return false. The PHP docs are
quite under-documented in regards to this function.
---

diff --git a/app/Auth/Access/LdapService.php b/app/Auth/Access/LdapService.php
index ddd7c6280..a45c07c2f 100644
--- a/app/Auth/Access/LdapService.php
+++ b/app/Auth/Access/LdapService.php
@@ -247,8 +247,13 @@ class LdapService
 
                 // Start and verify TLS if it's enabled
                 if ($this->config['start_tls']) {
-                    $started = $this->ldap->startTls($ldapConnection);
-                    if (!$started) {
+                    try {
+                        $tlsStarted = $this->ldap->startTls($ldapConnection);
+                    } catch (ErrorException $exception) {
+                        $tlsStarted = false;
+                    }
+
+                    if (!$tlsStarted) {
                         throw new LdapException('Could not start TLS connection');
                     }
                 }
diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 978420f86..3721898bd 100644
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -29,6 +29,7 @@ class LdapTest extends TestCase
         config()->set([
             'auth.method'                          => 'ldap',
             'auth.defaults.guard'                  => 'ldap',
+            'services.ldap.server'                 => 'ldap.example.com',
             'services.ldap.base_dn'                => 'dc=ldap,dc=local',
             'services.ldap.email_attribute'        => 'mail',
             'services.ldap.display_name_attribute' => 'cn',
@@ -581,6 +582,39 @@ class LdapTest extends TestCase
         $this->checkLdapReceivesCorrectDetails('ldap.bookstack.com', 'ldap.bookstack.com', 389);
     }
 
+    public function test_host_fail_over_by_using_semicolon_seperated_hosts()
+    {
+        app('config')->set([
+            'services.ldap.server' => 'ldap-tiger.example.com;ldap-donkey.example.com:8080',
+        ]);
+
+        // Standard mocks
+        $this->commonLdapMocks(0, 1, 1, 2, 1);
+        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)->andReturn(['count' => 1, 0 => [
+            'uid' => [$this->mockUser->name],
+            'cn'  => [$this->mockUser->name],
+            'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+        ]]);
+
+        $this->mockLdap->shouldReceive('connect')->once()->with('ldap-tiger.example.com', 389)->andReturn(false);
+        $this->mockLdap->shouldReceive('connect')->once()->with('ldap-donkey.example.com', 8080)->andReturn($this->resourceId);
+        $this->mockUserLogin();
+    }
+
+    public function test_host_fail_over_by_using_semicolon_seperated_hosts_still_throws_error()
+    {
+        app('config')->set([
+            'services.ldap.server' => 'ldap-tiger.example.com;ldap-donkey.example.com:8080',
+        ]);
+
+        $this->mockLdap->shouldReceive('connect')->once()->with('ldap-tiger.example.com', 389)->andReturn(false);
+        $this->mockLdap->shouldReceive('connect')->once()->with('ldap-donkey.example.com', 8080)->andReturn(false);
+
+        $resp = $this->mockUserLogin();
+        $resp->assertStatus(500);
+        $resp->assertSee('Cannot connect to ldap server, Initial connection failed');
+    }
+
     public function test_forgot_password_routes_inaccessible()
     {
         $resp = $this->get('/password/email');