Added more complexity in an attempt to make ldap host failover fit

diff --git a/app/Auth/Access/Ldap/LdapConfig.php b/app/Auth/Access/Ldap/LdapConfig.php
new file mode 100644 (file)
index 0000000..a96f207
--- /dev/null
+++ b/app/Auth/Access/Ldap/LdapConfig.php
@@ -0,0 +1,60 @@
+<?php
+
+namespace BookStack\Auth\Access\Ldap;
+
+class LdapConfig
+{
+    /**
+     * App provided config array.
+     * @var array
+     */
+    protected array $config;
+
+    public function __construct(array $config)
+    {
+        $this->config = $config;
+    }
+
+    /**
+     * Get a value from the config.
+     */
+    public function get(string $key)
+    {
+        return $this->config[$key] ?? null;
+    }
+
+    /**
+     * Parse the potentially multi-value LDAP server host string and return an array of host/port detail pairs.
+     * Multiple hosts are separated with a semicolon, for example: 'ldap.example.com:8069;ldaps://ldap.example.com'
+     *
+     * @return array<array{host: string, port: int}>
+     */
+    public function getServers(): array
+    {
+        $serverStringList = explode(';', $this->get('server'));
+
+        return array_map(fn ($serverStr) => $this->parseSingleServerString($serverStr), $serverStringList);
+    }
+
+    /**
+     * Parse an LDAP server string and return the host and port for a connection.
+     * Is flexible to formats such as 'ldap.example.com:8069' or 'ldaps://ldap.example.com'.
+     *
+     * @return array{host: string, port: int}
+     */
+    protected function parseSingleServerString(string $serverString): array
+    {
+        $serverNameParts = explode(':', trim($serverString));
+
+        // If we have a protocol just return the full string since PHP will ignore a separate port.
+        if ($serverNameParts[0] === 'ldaps' || $serverNameParts[0] === 'ldap') {
+            return ['host' => $serverString, 'port' => 389];
+        }
+
+        // Otherwise, extract the port out
+        $hostName = $serverNameParts[0];
+        $ldapPort = (count($serverNameParts) > 1) ? intval($serverNameParts[1]) : 389;
+
+        return ['host' => $hostName, 'port' => $ldapPort];
+    }
+}

diff --git a/app/Auth/Access/Ldap/LdapConnection.php b/app/Auth/Access/Ldap/LdapConnection.php
index f9eb9f17ea49d361994a34ca6d411fda862cb64a..9702c1e8703ef61ea1be385c1372f1384ba707bd 100644 (file)
--- a/app/Auth/Access/Ldap/LdapConnection.php
+++ b/app/Auth/Access/Ldap/LdapConnection.php
@@ -16,19 +16,25 @@ class LdapConnection
      */
     protected $connection;
 
      */
     protected $connection;
 

+    protected string $hostName;
+    protected int $port;
+

     public function __construct(string $hostName, int $port)
     {
     public function __construct(string $hostName, int $port)
     {

-        $this->connection = $this->connect($hostName, $port);
+        $this->hostName = $hostName;
+        $this->port = $port;
+        $this->connection = $this->connect();

     }
 
     /**
     }
 
     /**

-     * Connect to an LDAP server.
+     * Start a connection to an LDAP server.
+     * Does not actually call out to the external server until an action is performed.

      *
      * @return resource
      */
      *
      * @return resource
      */

-    protected function connect(string $hostName, int $port)
+    protected function connect()

     {
     {

-        return ldap_connect($hostName, $port);
+        return ldap_connect($this->hostName, $this->port);

     }
 
     /**
     }
 
     /**

diff --git a/app/Auth/Access/Ldap/LdapConnectionManager.php b/app/Auth/Access/Ldap/LdapConnectionManager.php
index e5720a49a267a19e62119cd7f21f0a0fbd5e6117..ab16f5b0e2d73b412bce033437374041a0f1a6ce 100644 (file)
--- a/app/Auth/Access/Ldap/LdapConnectionManager.php
+++ b/app/Auth/Access/Ldap/LdapConnectionManager.php
@@ -14,11 +14,11 @@ class LdapConnectionManager
     /**
      * Attempt to start and bind to a new LDAP connection as the configured LDAP system user.
      */
     /**
      * Attempt to start and bind to a new LDAP connection as the configured LDAP system user.
      */

-    public function startSystemBind(array $config): LdapConnection
+    public function startSystemBind(LdapConfig $config): LdapConnection

     {
         // Incoming options are string|false
     {
         // Incoming options are string|false

-        $dn = $config['dn'];
-        $pass = $config['pass'];
+        $dn = $config->get('dn');
+        $pass = $config->get('pass');

 
         $isAnonymous = ($dn === false || $pass === false);
 
 
         $isAnonymous = ($dn === false || $pass === false);
 


@@ -39,7 +39,7 @@ class LdapConnectionManager
      *
      * @throws LdapException
      */
      *
      * @throws LdapException
      */

-    public function startBind(?string $dn, ?string $password, array $config): LdapConnection
+    public function startBind(?string $dn, ?string $password, LdapConfig $config): LdapConnection

     {
         // Check LDAP extension in installed
         if (!function_exists('ldap_connect') && config('app.env') !== 'testing') {
     {
         // Check LDAP extension in installed
         if (!function_exists('ldap_connect') && config('app.env') !== 'testing') {


@@ -48,11 +48,11 @@ class LdapConnectionManager
 
         // Disable certificate verification.
         // This option works globally and must be set before a connection is created.
 
         // Disable certificate verification.
         // This option works globally and must be set before a connection is created.

-        if ($config['tls_insecure']) {
+        if ($config->get('tls_insecure')) {

             LdapConnection::setGlobalOption(LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
         }
 
             LdapConnection::setGlobalOption(LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
         }
 

-        $serverDetails = $this->parseMultiServerString($config['server']);
+        $serverDetails = $config->getServers();

         $lastException = null;
 
         foreach ($serverDetails as $server) {
         $lastException = null;
 
         foreach ($serverDetails as $server) {


@@ -85,25 +85,26 @@ class LdapConnectionManager
      * Attempt to start a server connection from the provided details.
      * @throws LdapException
      */
      * Attempt to start a server connection from the provided details.
      * @throws LdapException
      */

-    protected function startServerConnection(string $host, int $port, array $config): LdapConnection
+    protected function startServerConnection(string $host, int $port, LdapConfig $config): LdapConnection

     {
         if (isset($this->connectionCache[$host . ':' . $port])) {
             return $this->connectionCache[$host . ':' . $port];
         }
 
     {
         if (isset($this->connectionCache[$host . ':' . $port])) {
             return $this->connectionCache[$host . ':' . $port];
         }
 

-        $ldapConnection = new LdapConnection($host, $port);
+        /** @var LdapConnection $ldapConnection */
+        $ldapConnection = app()->make(LdapConnection::class, [$host, $port]);

 
         if (!$ldapConnection) {
             throw new LdapException(trans('errors.ldap_cannot_connect'));
         }
 
         // Set any required options
 
         if (!$ldapConnection) {
             throw new LdapException(trans('errors.ldap_cannot_connect'));
         }
 
         // Set any required options

-        if ($config['version']) {
-            $ldapConnection->setVersion($config['version']);
+        if ($config->get('version')) {
+            $ldapConnection->setVersion($config->get('version'));

         }
 
         // Start and verify TLS if it's enabled
         }
 
         // Start and verify TLS if it's enabled

-        if ($config['start_tls']) {
+        if ($config->get('start_tls')) {

             try {
                 $tlsStarted = $ldapConnection->startTls();
             } catch (ErrorException $exception) {
             try {
                 $tlsStarted = $ldapConnection->startTls();
             } catch (ErrorException $exception) {


@@ -117,39 +118,4 @@ class LdapConnectionManager
 
         return $ldapConnection;
     }
 
         return $ldapConnection;
     }

-
-    /**
-     * Parse a potentially multi-value LDAP server host string and return an array of host/port detail pairs.
-     * Multiple hosts are separated with a semicolon, for example: 'ldap.example.com:8069;ldaps://ldap.example.com'
-     *
-     * @return array<array{host: string, port: int}>
-     */
-    protected function parseMultiServerString(string $serversString): array
-    {
-        $serverStringList = explode(';', $serversString);
-
-        return array_map(fn ($serverStr) => $this->parseSingleServerString($serverStr), $serverStringList);
-    }
-
-    /**
-     * Parse an LDAP server string and return the host and port for a connection.
-     * Is flexible to formats such as 'ldap.example.com:8069' or 'ldaps://ldap.example.com'.
-     *
-     * @return array{host: string, port: int}
-     */
-    protected function parseSingleServerString(string $serverString): array
-    {
-        $serverNameParts = explode(':', $serverString);
-
-        // If we have a protocol just return the full string since PHP will ignore a separate port.
-        if ($serverNameParts[0] === 'ldaps' || $serverNameParts[0] === 'ldap') {
-            return ['host' => $serverString, 'port' => 389];
-        }
-
-        // Otherwise, extract the port out
-        $hostName = $serverNameParts[0];
-        $ldapPort = (count($serverNameParts) > 1) ? intval($serverNameParts[1]) : 389;
-
-        return ['host' => $hostName, 'port' => $ldapPort];
-    }

 }
 }

diff --git a/app/Auth/Access/Ldap/LdapService.php b/app/Auth/Access/Ldap/LdapService.php
index 502e6099a5e124c8f409ac6c1996fbf54392ee20..e483fff041e80b370b2a0077a96770c9e4a61133 100644 (file)
--- a/app/Auth/Access/Ldap/LdapService.php
+++ b/app/Auth/Access/Ldap/LdapService.php
@@ -20,14 +20,14 @@ class LdapService
     protected GroupSyncService $groupSyncService;
     protected UserAvatars $userAvatars;
 
     protected GroupSyncService $groupSyncService;
     protected UserAvatars $userAvatars;
 

-    protected array $config;
+    protected LdapConfig $config;

 
     public function __construct(LdapConnectionManager $ldap, UserAvatars $userAvatars, GroupSyncService $groupSyncService)
     {
         $this->ldap = $ldap;
         $this->userAvatars = $userAvatars;
         $this->groupSyncService = $groupSyncService;
 
     public function __construct(LdapConnectionManager $ldap, UserAvatars $userAvatars, GroupSyncService $groupSyncService)
     {
         $this->ldap = $ldap;
         $this->userAvatars = $userAvatars;
         $this->groupSyncService = $groupSyncService;

-        $this->config = config('services.ldap');
+        $this->config = new LdapConfig(config('services.ldap'));

     }
 
     /**
     }
 
     /**


@@ -47,10 +47,10 @@ class LdapService
         }
 
         // Find user
         }
 
         // Find user

-        $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]);
-        $baseDn = $this->config['base_dn'];
+        $userFilter = $this->buildFilter($this->config->get('user_filter'), ['user' => $userName]);
+        $baseDn = $this->config->get('base_dn');

 
 

-        $followReferrals = $this->config['follow_referrals'] ? 1 : 0;
+        $followReferrals = $this->config->get('follow_referrals') ? 1 : 0;

         $connection->setOption(LDAP_OPT_REFERRALS, $followReferrals);
         $users = $connection->searchAndGetEntries($baseDn, $userFilter, $attributes);
         if ($users['count'] === 0) {
         $connection->setOption(LDAP_OPT_REFERRALS, $followReferrals);
         $users = $connection->searchAndGetEntries($baseDn, $userFilter, $attributes);
         if ($users['count'] === 0) {


@@ -68,10 +68,10 @@ class LdapService
      */
     public function getUserDetails(string $userName): ?array
     {
      */
     public function getUserDetails(string $userName): ?array
     {

-        $idAttr = $this->config['id_attribute'];
-        $emailAttr = $this->config['email_attribute'];
-        $displayNameAttr = $this->config['display_name_attribute'];
-        $thumbnailAttr = $this->config['thumbnail_attribute'];
+        $idAttr = $this->config->get('id_attribute');
+        $emailAttr = $this->config->get('email_attribute');
+        $displayNameAttr = $this->config->get('display_name_attribute');
+        $thumbnailAttr = $this->config->get('thumbnail_attribute');

 
         $user = $this->getUserWithAttributes($userName, array_filter([
             'cn', 'dn', $idAttr, $emailAttr, $displayNameAttr, $thumbnailAttr,
 
         $user = $this->getUserWithAttributes($userName, array_filter([
             'cn', 'dn', $idAttr, $emailAttr, $displayNameAttr, $thumbnailAttr,


@@ -90,7 +90,7 @@ class LdapService
             'avatar' => $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null,
         ];
 
             'avatar' => $thumbnailAttr ? $this->getUserResponseProperty($user, $thumbnailAttr, null) : null,
         ];
 

-        if ($this->config['dump_user_details']) {
+        if ($this->config->get('dump_user_details')) {

             throw new JsonDebugException([
                 'details_from_ldap'        => $user,
                 'details_bookstack_parsed' => $formatted,
             throw new JsonDebugException([
                 'details_from_ldap'        => $user,
                 'details_bookstack_parsed' => $formatted,


@@ -170,7 +170,7 @@ class LdapService
      */
     public function getUserGroups(string $userName): array
     {
      */
     public function getUserGroups(string $userName): array
     {

-        $groupsAttr = $this->config['group_attribute'];
+        $groupsAttr = $this->config->get('group_attribute');

         $user = $this->getUserWithAttributes($userName, [$groupsAttr]);
 
         if ($user === null) {
         $user = $this->getUserWithAttributes($userName, [$groupsAttr]);
 
         if ($user === null) {


@@ -180,7 +180,7 @@ class LdapService
         $userGroups = $this->groupFilter($user);
         $allGroups = $this->getGroupsRecursive($userGroups, []);
 
         $userGroups = $this->groupFilter($user);
         $allGroups = $this->getGroupsRecursive($userGroups, []);
 

-        if ($this->config['dump_user_groups']) {
+        if ($this->config->get('dump_user_groups')) {

             throw new JsonDebugException([
                 'details_from_ldap'             => $user,
                 'parsed_direct_user_groups'     => $userGroups,
             throw new JsonDebugException([
                 'details_from_ldap'             => $user,
                 'parsed_direct_user_groups'     => $userGroups,


@@ -227,13 +227,13 @@ class LdapService
     {
         $connection = $this->ldap->startSystemBind($this->config);
 
     {
         $connection = $this->ldap->startSystemBind($this->config);
 

-        $followReferrals = $this->config['follow_referrals'] ? 1 : 0;
+        $followReferrals = $this->config->get('follow_referrals') ? 1 : 0;

         $connection->setOption(LDAP_OPT_REFERRALS, $followReferrals);
 
         $connection->setOption(LDAP_OPT_REFERRALS, $followReferrals);
 

-        $baseDn = $this->config['base_dn'];
-        $groupsAttr = strtolower($this->config['group_attribute']);
+        $baseDn = $this->config->get('base_dn');
+        $groupsAttr = strtolower($this->config->get('group_attribute'));

 
 

-        $groupFilter = 'CN=' . $connection->escape($groupName);
+        $groupFilter = 'CN=' . LdapConnection::escape($groupName);

         $groups = $connection->searchAndGetEntries($baseDn, $groupFilter, [$groupsAttr]);
         if ($groups['count'] === 0) {
             return [];
         $groups = $connection->searchAndGetEntries($baseDn, $groupFilter, [$groupsAttr]);
         if ($groups['count'] === 0) {
             return [];


@@ -248,7 +248,7 @@ class LdapService
      */
     protected function groupFilter(array $userGroupSearchResponse): array
     {
      */
     protected function groupFilter(array $userGroupSearchResponse): array
     {

-        $groupsAttr = strtolower($this->config['group_attribute']);
+        $groupsAttr = strtolower($this->config->get('group_attribute'));

         $ldapGroups = [];
         $count = 0;
 
         $ldapGroups = [];
         $count = 0;
 


@@ -275,7 +275,7 @@ class LdapService
     public function syncGroups(User $user, string $username)
     {
         $userLdapGroups = $this->getUserGroups($username);
     public function syncGroups(User $user, string $username)
     {
         $userLdapGroups = $this->getUserGroups($username);

-        $this->groupSyncService->syncUserWithFoundGroups($user, $userLdapGroups, $this->config['remove_from_groups']);
+        $this->groupSyncService->syncUserWithFoundGroups($user, $userLdapGroups, $this->config->get('remove_from_groups'));

     }
 
     /**
     }
 
     /**


@@ -283,7 +283,7 @@ class LdapService
      */
     public function shouldSyncGroups(): bool
     {
      */
     public function shouldSyncGroups(): bool
     {

-        return $this->config['user_to_groups'] !== false;
+        return $this->config->get('user_to_groups') !== false;

     }
 
     /**
     }
 
     /**


@@ -292,7 +292,7 @@ class LdapService
      */
     public function saveAndAttachAvatar(User $user, array $ldapUserDetails): void
     {
      */
     public function saveAndAttachAvatar(User $user, array $ldapUserDetails): void
     {

-        if (is_null($this->config['thumbnail_attribute']) || is_null($ldapUserDetails['avatar'])) {
+        if (is_null($this->config->get('thumbnail_attribute')) || is_null($ldapUserDetails['avatar'])) {

             return;
         }
 
             return;
         }
 

diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index 1c575ec86a7a6b1404917d94b43dcb73e9ff2b57..726595bd58c28f832ff73069cb9cfaea90594dfa 100644 (file)
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -2,6 +2,7 @@
 
 namespace Tests\Auth;
 
 
 namespace Tests\Auth;
 

+use BookStack\Auth\Access\Ldap\LdapConfig;

 use BookStack\Auth\Access\Ldap\LdapConnection;
 use BookStack\Auth\Access\Ldap\LdapConnectionManager;
 use BookStack\Auth\Access\Ldap\LdapService;
 use BookStack\Auth\Access\Ldap\LdapConnection;
 use BookStack\Auth\Access\Ldap\LdapConnectionManager;
 use BookStack\Auth\Access\Ldap\LdapService;


@@ -45,9 +46,6 @@ class LdapTest extends TestCase
             'services.ldap.thumbnail_attribute'    => null,
         ]);
 
             'services.ldap.thumbnail_attribute'    => null,
         ]);
 

-        $lcm = $this->mock(LdapConnectionManager::class);
-        // TODO - Properly mock
-

         $this->mockLdap = \Mockery::mock(LdapConnection::class);
         $this->app[LdapConnection::class] = $this->mockLdap;
         $this->mockUser = User::factory()->make();
         $this->mockLdap = \Mockery::mock(LdapConnection::class);
         $this->app[LdapConnection::class] = $this->mockLdap;
         $this->mockUser = User::factory()->make();


@@ -55,26 +53,12 @@ class LdapTest extends TestCase
 
     protected function runFailedAuthLogin()
     {
 
     protected function runFailedAuthLogin()
     {

-        $this->commonLdapMocks(1, 1, 1, 1, 1);
+        $this->commonLdapMocks(1, 1, 1);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
             ->andReturn(['count' => 0]);
         $this->post('/login', ['username' => 'timmyjenkins', 'password' => 'cattreedog']);
     }
 
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
             ->andReturn(['count' => 0]);
         $this->post('/login', ['username' => 'timmyjenkins', 'password' => 'cattreedog']);
     }
 

-    protected function mockEscapes($times = 1)
-    {
-        $this->mockLdap->shouldReceive('escape')->times($times)->andReturnUsing(function ($val) {
-            return ldap_escape($val);
-        });
-    }
-
-    protected function mockExplodes($times = 1)
-    {
-        $this->mockLdap->shouldReceive('explodeDn')->times($times)->andReturnUsing(function ($dn, $withAttrib) {
-            return ldap_explode_dn($dn, $withAttrib);
-        });
-    }
-

     protected function mockUserLogin(?string $email = null): TestResponse
     {
         return $this->post('/login', [
     protected function mockUserLogin(?string $email = null): TestResponse
     {
         return $this->post('/login', [


@@ -86,20 +70,18 @@ class LdapTest extends TestCase
     /**
      * Set LDAP method mocks for things we commonly call without altering.
      */
     /**
      * Set LDAP method mocks for things we commonly call without altering.
      */

-    protected function commonLdapMocks(int $connects = 1, int $versions = 1, int $options = 2, int $binds = 4, int $escapes = 2, int $explodes = 0)
+    protected function commonLdapMocks(int $versions = 1, int $options = 2, int $binds = 4)

     {
         $this->mockLdap->shouldReceive('setVersion')->times($versions);
         $this->mockLdap->shouldReceive('setOption')->times($options);
         $this->mockLdap->shouldReceive('bind')->times($binds)->andReturn(true);
     {
         $this->mockLdap->shouldReceive('setVersion')->times($versions);
         $this->mockLdap->shouldReceive('setOption')->times($options);
         $this->mockLdap->shouldReceive('bind')->times($binds)->andReturn(true);

-        $this->mockEscapes($escapes);
-        $this->mockExplodes($explodes);

     }
 
     public function test_login()
     {
     }
 
     public function test_login()
     {

-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],


@@ -128,9 +110,9 @@ class LdapTest extends TestCase
             'registration-restrict' => 'testing.com',
         ]);
 
             'registration-restrict' => 'testing.com',
         ]);
 

-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],


@@ -153,9 +135,9 @@ class LdapTest extends TestCase
     {
         $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
 
     {
         $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
 

-        $this->commonLdapMocks(1, 1, 1, 2, 1);
+        $this->commonLdapMocks(1, 1, 2);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'cn'   => [$this->mockUser->name],
                 'dn'   => $ldapDn,
             ->andReturn(['count' => 1, 0 => [
                 'cn'   => [$this->mockUser->name],
                 'dn'   => $ldapDn,


@@ -172,10 +154,10 @@ class LdapTest extends TestCase
     {
         config()->set(['services.ldap.id_attribute' => 'my_custom_id']);
 
     {
         config()->set(['services.ldap.id_attribute' => 'my_custom_id']);
 

-        $this->commonLdapMocks(1, 1, 1, 2, 1);
+        $this->commonLdapMocks(1, 1, 2);

         $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'cn'           => [$this->mockUser->name],
                 'dn'           => $ldapDn,
             ->andReturn(['count' => 1, 0 => [
                 'cn'           => [$this->mockUser->name],
                 'dn'           => $ldapDn,


@@ -191,9 +173,9 @@ class LdapTest extends TestCase
 
     public function test_initial_incorrect_credentials()
     {
 
     public function test_initial_incorrect_credentials()
     {

-        $this->commonLdapMocks(1, 1, 1, 0, 1);
+        $this->commonLdapMocks(1, 1, 0);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],


@@ -209,9 +191,9 @@ class LdapTest extends TestCase
 
     public function test_login_not_found_username()
     {
 
     public function test_login_not_found_username()
     {

-        $this->commonLdapMocks(1, 1, 1, 1, 1);
+        $this->commonLdapMocks(1, 1, 1);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 0]);
 
         $resp = $this->mockUserLogin();
             ->andReturn(['count' => 0]);
 
         $resp = $this->mockUserLogin();


@@ -284,9 +266,9 @@ class LdapTest extends TestCase
             'services.ldap.remove_from_groups' => false,
         ]);
 
             'services.ldap.remove_from_groups' => false,
         ]);
 

-        $this->commonLdapMocks(1, 1, 4, 5, 4, 6);
+        $this->commonLdapMocks(1, 4, 5);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],


@@ -329,9 +311,9 @@ class LdapTest extends TestCase
             'services.ldap.remove_from_groups' => true,
         ]);
 
             'services.ldap.remove_from_groups' => true,
         ]);
 

-        $this->commonLdapMocks(1, 1, 3, 4, 3, 2);
+        $this->commonLdapMocks(1, 3, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],


@@ -371,9 +353,9 @@ class LdapTest extends TestCase
             'dn'       => 'dc=test,' . config('services.ldap.base_dn'),
             'mail'     => [$this->mockUser->email],
         ]];
             'dn'       => 'dc=test,' . config('services.ldap.base_dn'),
             'mail'     => [$this->mockUser->email],
         ]];

-        $this->commonLdapMocks(1, 1, 4, 5, 4, 2);
+        $this->commonLdapMocks(1, 4, 5);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn($userResp, ['count' => 1,
                 0                           => [
                     'dn'       => 'dc=test,' . config('services.ldap.base_dn'),
             ->andReturn($userResp, ['count' => 1,
                 0                           => [
                     'dn'       => 'dc=test,' . config('services.ldap.base_dn'),


@@ -430,9 +412,9 @@ class LdapTest extends TestCase
             'services.ldap.remove_from_groups' => true,
         ]);
 
             'services.ldap.remove_from_groups' => true,
         ]);
 

-        $this->commonLdapMocks(1, 1, 3, 4, 3, 2);
+        $this->commonLdapMocks(1, 3, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(3)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],


@@ -471,9 +453,9 @@ class LdapTest extends TestCase
             'services.ldap.remove_from_groups' => true,
         ]);
 
             'services.ldap.remove_from_groups' => true,
         ]);
 

-        $this->commonLdapMocks(1, 1, 4, 5, 4, 6);
+        $this->commonLdapMocks(1, 4, 5);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid'      => [$this->mockUser->name],
                 'cn'       => [$this->mockUser->name],


@@ -505,9 +487,9 @@ class LdapTest extends TestCase
             'services.ldap.display_name_attribute' => 'displayName',
         ]);
 
             'services.ldap.display_name_attribute' => 'displayName',
         ]);
 

-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid'         => [$this->mockUser->name],
                 'cn'          => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid'         => [$this->mockUser->name],
                 'cn'          => [$this->mockUser->name],


@@ -530,9 +512,9 @@ class LdapTest extends TestCase
             'services.ldap.display_name_attribute' => 'displayName',
         ]);
 
             'services.ldap.display_name_attribute' => 'displayName',
         ]);
 

-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],


@@ -553,39 +535,44 @@ class LdapTest extends TestCase
         ]);
     }
 
         ]);
     }
 

-    protected function checkLdapReceivesCorrectDetails($serverString, $expectedHost, $expectedPort)
+    protected function checkLdapConfigHostParsing($serverString, ...$expectedHostPortPairs)

     {
     {

-        app('config')->set([
-            'services.ldap.server' => $serverString,
-        ]);
+        config()->set(['services.ldap.server' => $serverString]);
+        $ldapConfig = new LdapConfig(config('services.ldap'));

 
 

-        // 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($expectedHost, $expectedPort)->andReturn($this->resourceId);
-        $this->mockUserLogin();
+        $servers = $ldapConfig->getServers();
+        $this->assertCount(count($expectedHostPortPairs), $servers);
+        foreach ($expectedHostPortPairs as $i => $expected) {
+            $server = $servers[$i];
+            $this->assertEquals($expected[0], $server['host']);
+            $this->assertEquals($expected[1], $server['port']);
+        }

     }
 
     public function test_ldap_port_provided_on_host_if_host_is_full_uri()
     {
         $hostName = 'ldaps://bookstack:8080';
     }
 
     public function test_ldap_port_provided_on_host_if_host_is_full_uri()
     {
         $hostName = 'ldaps://bookstack:8080';

-        $this->checkLdapReceivesCorrectDetails($hostName, $hostName, 389);
+        $this->checkLdapConfigHostParsing($hostName, [$hostName, 389]);

     }
 
     public function test_ldap_port_parsed_from_server_if_host_is_not_full_uri()
     {
     }
 
     public function test_ldap_port_parsed_from_server_if_host_is_not_full_uri()
     {

-        $this->checkLdapReceivesCorrectDetails('ldap.bookstack.com:8080', 'ldap.bookstack.com', 8080);
+        $this->checkLdapConfigHostParsing('ldap.bookstack.com:8080', ['ldap.bookstack.com', 8080]);

     }
 
     public function test_default_ldap_port_used_if_not_in_server_string_and_not_uri()
     {
     }
 
     public function test_default_ldap_port_used_if_not_in_server_string_and_not_uri()
     {

-        $this->checkLdapReceivesCorrectDetails('ldap.bookstack.com', 'ldap.bookstack.com', 389);
+        $this->checkLdapConfigHostParsing('ldap.bookstack.com', ['ldap.bookstack.com', 389]);
+    }
+
+    public function test_multiple_hosts_parsed_from_config_if_semicolon_seperated()
+    {
+        $this->checkLdapConfigHostParsing(
+            'ldap.bookstack.com:8080; l.bookstackapp.com; b.bookstackapp.com:8081',
+            ['ldap.bookstack.com', 8080],
+            ['l.bookstackapp.com', 389],
+            ['b.bookstackapp.com', 8081],
+        );

     }
 
     public function test_host_fail_over_by_using_semicolon_seperated_hosts()
     }
 
     public function test_host_fail_over_by_using_semicolon_seperated_hosts()


@@ -595,14 +582,15 @@ class LdapTest extends TestCase
         ]);
 
         // Standard mocks
         ]);
 
         // 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('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('bind')->once()->with('ldap-tiger.example.com', 389)->andReturn(false);
+        $this->commonLdapMocks(0, 1, 1);
+

         $this->mockLdap->shouldReceive('connect')->once()->with('ldap-donkey.example.com', 8080)->andReturn($this->resourceId);
         $this->mockUserLogin();
     }
         $this->mockLdap->shouldReceive('connect')->once()->with('ldap-donkey.example.com', 8080)->andReturn($this->resourceId);
         $this->mockUserLogin();
     }


@@ -658,9 +646,9 @@ class LdapTest extends TestCase
     {
         config()->set(['services.ldap.dump_user_details' => true, 'services.ldap.thumbnail_attribute' => 'jpegphoto']);
 
     {
         config()->set(['services.ldap.dump_user_details' => true, 'services.ldap.thumbnail_attribute' => 'jpegphoto']);
 

-        $this->commonLdapMocks(1, 1, 1, 1, 1);
+        $this->commonLdapMocks(1, 1, 1);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [$this->mockUser->name],
                 'cn'  => [$this->mockUser->name],


@@ -690,7 +678,7 @@ class LdapTest extends TestCase
     {
         config()->set(['services.ldap.start_tls' => true]);
         $this->mockLdap->shouldReceive('startTls')->once()->andReturn(false);
     {
         config()->set(['services.ldap.start_tls' => true]);
         $this->mockLdap->shouldReceive('startTls')->once()->andReturn(false);

-        $this->commonLdapMocks(1, 1, 0, 0, 0);
+        $this->commonLdapMocks(1, 0, 0);

         $resp = $this->post('/login', ['username' => 'timmyjenkins', 'password' => 'cattreedog']);
         $resp->assertStatus(500);
     }
         $resp = $this->post('/login', ['username' => 'timmyjenkins', 'password' => 'cattreedog']);
         $resp->assertStatus(500);
     }


@@ -699,9 +687,9 @@ class LdapTest extends TestCase
     {
         config()->set(['services.ldap.id_attribute' => 'BIN;uid']);
         $ldapService = app()->make(LdapService::class);
     {
         config()->set(['services.ldap.id_attribute' => 'BIN;uid']);
         $ldapService = app()->make(LdapService::class);

-        $this->commonLdapMocks(1, 1, 1, 1, 1);
+        $this->commonLdapMocks(1, 1, 1);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), ['cn', 'dn', 'uid', 'mail', 'cn'])
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), ['cn', 'dn', 'uid', 'mail', 'cn'])

             ->andReturn(['count' => 1, 0 => [
                 'uid' => [hex2bin('FFF8F7')],
                 'cn'  => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid' => [hex2bin('FFF8F7')],
                 'cn'  => [$this->mockUser->name],


@@ -714,9 +702,9 @@ class LdapTest extends TestCase
 
     public function test_new_ldap_user_login_with_already_used_email_address_shows_error_message_to_user()
     {
 
     public function test_new_ldap_user_login_with_already_used_email_address_shows_error_message_to_user()
     {

-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);

         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'uid'  => [$this->mockUser->name],
                 'cn'   => [$this->mockUser->name],
             ->andReturn(['count' => 1, 0 => [
                 'uid'  => [$this->mockUser->name],
                 'cn'   => [$this->mockUser->name],


@@ -750,7 +738,7 @@ class LdapTest extends TestCase
             'services.ldap.remove_from_groups' => true,
         ]);
 
             'services.ldap.remove_from_groups' => true,
         ]);
 

-        $this->commonLdapMocks(1, 1, 6, 8, 6, 4);
+        $this->commonLdapMocks(1, 6, 8);

         $this->mockLdap->shouldReceive('searchAndGetEntries')
             ->times(6)
             ->andReturn(['count' => 1, 0 => [
         $this->mockLdap->shouldReceive('searchAndGetEntries')
             ->times(6)
             ->andReturn(['count' => 1, 0 => [


@@ -798,10 +786,10 @@ class LdapTest extends TestCase
     {
         config()->set(['services.ldap.thumbnail_attribute' => 'jpegPhoto']);
 
     {
         config()->set(['services.ldap.thumbnail_attribute' => 'jpegPhoto']);
 

-        $this->commonLdapMocks(1, 1, 1, 2, 1);
+        $this->commonLdapMocks(1, 1, 2);

         $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
         $ldapDn = 'cn=test-user,dc=test' . config('services.ldap.base_dn');
         $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)

-            ->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
+            ->with(config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))

             ->andReturn(['count' => 1, 0 => [
                 'cn'        => [$this->mockUser->name],
                 'dn'        => $ldapDn,
             ->andReturn(['count' => 1, 0 => [
                 'cn'        => [$this->mockUser->name],
                 'dn'        => $ldapDn,