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 ddd7c6280574efa26066bf67ba24f5f13dd1719c..a45c07c2f8109323ece256234ed400d108ee4072 100644 (file)
--- 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 978420f869ede41d480cbe7258f1c6600773e3d5..3721898bdbf1161638226f350ce4da570034b243 100644 (file)
--- 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');