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

Updated ldap connections to be carried via their own class.
Extracted connection logic to its own class.
Having trouble making this new format testable.

.env.example.complete

@@ -368,4 +368,4 @@ LOG_FAILED_LOGIN_CHANNEL=errorlog_plain_webserver
 # IP address '146.191.42.4' would result in '146.191.x.x' being logged.
 # For the IPv6 address '2001:db8:85a3:8d3:1319:8a2e:370:7348' this would result as:
 # '2001:db8:85a3:8d3:x:x:x:x'
-IP_ADDRESS_PRECISION=4
+IP_ADDRESS_PRECISION=4

app/Auth/Access/Guards/LdapSessionGuard.php

@@ -2,7 +2,7 @@
 
 namespace BookStack\Auth\Access\Guards;
 
-use BookStack\Auth\Access\LdapService;
+use BookStack\Auth\Access\Ldap\LdapService;
 use BookStack\Auth\Access\RegistrationService;
 use BookStack\Auth\User;
 use BookStack\Exceptions\JsonDebugException;

app/Auth/Access/Ldap.php

@@ -1,136 +0,0 @@
-<?php
-
-namespace BookStack\Auth\Access;
-
-/**
- * Class Ldap
- * An object-orientated thin abstraction wrapper for common PHP LDAP functions.
- * Allows the standard LDAP functions to be mocked for testing.
- */
-class Ldap
-{
-    /**
-     * Connect to an LDAP server.
-     *
-     * @return resource
-     */
-    public function connect(string $hostName, int $port)
-    {
-        return ldap_connect($hostName, $port);
-    }
-
-    /**
-     * Set the value of a LDAP option for the given connection.
-     *
-     * @param resource $ldapConnection
-     * @param mixed    $value
-     */
-    public function setOption($ldapConnection, int $option, $value): bool
-    {
-        return ldap_set_option($ldapConnection, $option, $value);
-    }
-
-    /**
-     * Start TLS on the given LDAP connection.
-     */
-    public function startTls($ldapConnection): bool
-    {
-        return ldap_start_tls($ldapConnection);
-    }
-
-    /**
-     * Set the version number for the given ldap connection.
-     *
-     * @param resource $ldapConnection
-     */
-    public function setVersion($ldapConnection, int $version): bool
-    {
-        return $this->setOption($ldapConnection, LDAP_OPT_PROTOCOL_VERSION, $version);
-    }
-
-    /**
-     * Search LDAP tree using the provided filter.
-     *
-     * @param resource   $ldapConnection
-     * @param string     $baseDn
-     * @param string     $filter
-     * @param array|null $attributes
-     *
-     * @return resource
-     */
-    public function search($ldapConnection, $baseDn, $filter, array $attributes = null)
-    {
-        return ldap_search($ldapConnection, $baseDn, $filter, $attributes);
-    }
-
-    /**
-     * Get entries from an ldap search result.
-     *
-     * @param resource $ldapConnection
-     * @param resource $ldapSearchResult
-     *
-     * @return array
-     */
-    public function getEntries($ldapConnection, $ldapSearchResult)
-    {
-        return ldap_get_entries($ldapConnection, $ldapSearchResult);
-    }
-
-    /**
-     * Search and get entries immediately.
-     *
-     * @param resource   $ldapConnection
-     * @param string     $baseDn
-     * @param string     $filter
-     * @param array|null $attributes
-     *
-     * @return resource
-     */
-    public function searchAndGetEntries($ldapConnection, $baseDn, $filter, array $attributes = null)
-    {
-        $search = $this->search($ldapConnection, $baseDn, $filter, $attributes);
-
-        return $this->getEntries($ldapConnection, $search);
-    }
-
-    /**
-     * Bind to LDAP directory.
-     *
-     * @param resource $ldapConnection
-     * @param string   $bindRdn
-     * @param string   $bindPassword
-     *
-     * @return bool
-     */
-    public function bind($ldapConnection, $bindRdn = null, $bindPassword = null)
-    {
-        return ldap_bind($ldapConnection, $bindRdn, $bindPassword);
-    }
-
-    /**
-     * Explode a LDAP dn string into an array of components.
-     *
-     * @param string $dn
-     * @param int    $withAttrib
-     *
-     * @return array
-     */
-    public function explodeDn(string $dn, int $withAttrib)
-    {
-        return ldap_explode_dn($dn, $withAttrib);
-    }
-
-    /**
-     * Escape a string for use in an LDAP filter.
-     *
-     * @param string $value
-     * @param string $ignore
-     * @param int    $flags
-     *
-     * @return string
-     */
-    public function escape(string $value, string $ignore = '', int $flags = 0)
-    {
-        return ldap_escape($value, $ignore, $flags);
-    }
-}

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];
+    }
+}

app/Auth/Access/Ldap/LdapConnection.php

@@ -0,0 +1,135 @@
+<?php
+
+namespace BookStack\Auth\Access\Ldap;
+
+use ErrorException;
+
+/**
+ * An object-orientated wrapper for core ldap functions,
+ * holding an internal connection instance.
+ */
+class LdapConnection
+{
+    /**
+     * The core ldap connection resource.
+     * @var resource
+     */
+    protected $connection;
+
+    protected string $hostName;
+    protected int $port;
+
+    public function __construct(string $hostName, int $port)
+    {
+        $this->hostName = $hostName;
+        $this->port = $port;
+        $this->connection = $this->connect();
+    }
+
+    /**
+     * Start a connection to an LDAP server.
+     * Does not actually call out to the external server until an action is performed.
+     *
+     * @return resource
+     */
+    protected function connect()
+    {
+        return ldap_connect($this->hostName, $this->port);
+    }
+
+    /**
+     * Set the value of a LDAP option for the current connection.
+     *
+     * @param mixed    $value
+     */
+    public function setOption(int $option, $value): bool
+    {
+        return ldap_set_option($this->connection, $option, $value);
+    }
+
+    /**
+     * Start TLS for this LDAP connection.
+     */
+    public function startTls(): bool
+    {
+        return ldap_start_tls($this->connection);
+    }
+
+    /**
+     * Set the version number for this ldap connection.
+     */
+    public function setVersion(int $version): bool
+    {
+        return $this->setOption(LDAP_OPT_PROTOCOL_VERSION, $version);
+    }
+
+    /**
+     * Search LDAP tree using the provided filter.
+     *
+     * @return resource
+     */
+    public function search(string $baseDn, string $filter, array $attributes = null)
+    {
+        return ldap_search($this->connection, $baseDn, $filter, $attributes);
+    }
+
+    /**
+     * Get entries from an ldap search result.
+     *
+     * @param resource $ldapSearchResult
+     * @return array|false
+     */
+    public function getEntries($ldapSearchResult)
+    {
+        return ldap_get_entries($this->connection, $ldapSearchResult);
+    }
+
+    /**
+     * Search and get entries immediately.
+     *
+     * @return array|false
+     */
+    public function searchAndGetEntries(string $baseDn, string $filter, array $attributes = null)
+    {
+        $search = $this->search($baseDn, $filter, $attributes);
+
+        return $this->getEntries($search);
+    }
+
+    /**
+     * Bind to LDAP directory.
+     *
+     * @throws ErrorException
+     */
+    public function bind(string $bindRdn = null, string $bindPassword = null): bool
+    {
+        return ldap_bind($this->connection, $bindRdn, $bindPassword);
+    }
+
+    /**
+     * Explode a LDAP dn string into an array of components.
+     *
+     * @return array|false
+     */
+    public static function explodeDn(string $dn, int $withAttrib)
+    {
+        return ldap_explode_dn($dn, $withAttrib);
+    }
+
+    /**
+     * Escape a string for use in an LDAP filter.
+     */
+    public static function escape(string $value, string $ignore = '', int $flags = 0): string
+    {
+        return ldap_escape($value, $ignore, $flags);
+    }
+
+    /**
+     * Set a non-connection-specific LDAP option.
+     * @param mixed $value
+     */
+    public static function setGlobalOption(int $option, $value): bool
+    {
+        return ldap_set_option(null, $option, $value);
+    }
+}

app/Auth/Access/Ldap/LdapConnectionManager.php

@@ -0,0 +1,121 @@
+<?php
+
+namespace BookStack\Auth\Access\Ldap;
+
+use BookStack\Exceptions\LdapException;
+use BookStack\Exceptions\LdapFailedBindException;
+use ErrorException;
+use Illuminate\Support\Facades\Log;
+
+class LdapConnectionManager
+{
+    protected array $connectionCache = [];
+
+    /**
+     * Attempt to start and bind to a new LDAP connection as the configured LDAP system user.
+     */
+    public function startSystemBind(LdapConfig $config): LdapConnection
+    {
+        // Incoming options are string|false
+        $dn = $config->get('dn');
+        $pass = $config->get('pass');
+
+        $isAnonymous = ($dn === false || $pass === false);
+
+        try {
+            return $this->startBind($dn ?: null, $pass ?: null, $config);
+        } catch (LdapFailedBindException $exception) {
+            $msg = ($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed'));
+            throw new LdapFailedBindException($msg);
+        }
+    }
+
+    /**
+     * Attempt to start and bind to a new LDAP connection.
+     * Will attempt against multiple defined fail-over hosts if set in the provided config.
+     *
+     * Throws a LdapFailedBindException error if the bind connected but failed.
+     * Otherwise, generic LdapException errors would be thrown.
+     *
+     * @throws LdapException
+     */
+    public function startBind(?string $dn, ?string $password, LdapConfig $config): LdapConnection
+    {
+        // Check LDAP extension in installed
+        if (!function_exists('ldap_connect') && config('app.env') !== 'testing') {
+            throw new LdapException(trans('errors.ldap_extension_not_installed'));
+        }
+
+        // Disable certificate verification.
+        // This option works globally and must be set before a connection is created.
+        if ($config->get('tls_insecure')) {
+            LdapConnection::setGlobalOption(LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
+        }
+
+        $serverDetails = $config->getServers();
+        $lastException = null;
+
+        foreach ($serverDetails as $server) {
+            try {
+                $connection = $this->startServerConnection($server['host'], $server['port'], $config);
+            } catch (LdapException $exception) {
+                $lastException = $exception;
+                continue;
+            }
+
+            try {
+                $bound = $connection->bind($dn, $password);
+                if (!$bound) {
+                    throw new LdapFailedBindException('Failed to perform LDAP bind');
+                }
+            } catch (ErrorException $exception) {
+                Log::error('LDAP bind error: ' . $exception->getMessage());
+                $lastException = new LdapException('Encountered error during LDAP bind');
+                continue;
+            }
+
+            $this->connectionCache[$server['host'] . ':' . $server['port']] = $connection;
+            return $connection;
+        }
+
+        throw $lastException;
+    }
+
+    /**
+     * Attempt to start a server connection from the provided details.
+     * @throws LdapException
+     */
+    protected function startServerConnection(string $host, int $port, LdapConfig $config): LdapConnection
+    {
+        if (isset($this->connectionCache[$host . ':' . $port])) {
+            return $this->connectionCache[$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 ($config->get('version')) {
+            $ldapConnection->setVersion($config->get('version'));
+        }
+
+        // Start and verify TLS if it's enabled
+        if ($config->get('start_tls')) {
+            try {
+                $tlsStarted = $ldapConnection->startTls();
+            } catch (ErrorException $exception) {
+                $tlsStarted = false;
+            }
+
+            if (!$tlsStarted) {
+                throw new LdapException('Could not start TLS connection');
+            }
+        }
+
+        return $ldapConnection;
+    }
+}

app/Auth/Access/Ldap/LdapService.php

@@ -1,12 +1,13 @@
 <?php
 
-namespace BookStack\Auth\Access;
+namespace BookStack\Auth\Access\Ldap;
 
+use BookStack\Auth\Access\GroupSyncService;
 use BookStack\Auth\User;
 use BookStack\Exceptions\JsonDebugException;
 use BookStack\Exceptions\LdapException;
+use BookStack\Exceptions\LdapFailedBindException;
 use BookStack\Uploads\UserAvatars;
-use ErrorException;
 use Illuminate\Support\Facades\Log;
 
 /**
@@ -15,36 +16,18 @@ use Illuminate\Support\Facades\Log;
  */
 class LdapService
 {
-    protected Ldap $ldap;
+    protected LdapConnectionManager $ldap;
     protected GroupSyncService $groupSyncService;
     protected UserAvatars $userAvatars;
 
-    /**
+    protected LdapConfig $config;
-     * @var resource
-     */
-    protected $ldapConnection;
 
-    protected array $config;
+    public function __construct(LdapConnectionManager $ldap, UserAvatars $userAvatars, GroupSyncService $groupSyncService)
-    protected bool $enabled;
-
-    /**
-     * LdapService constructor.
-     */
-    public function __construct(Ldap $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'));
-        $this->enabled = config('auth.method') === 'ldap';
-    }
-
-    /**
-     * Check if groups should be synced.
-     */
-    public function shouldSyncGroups(): bool
-    {
-        return $this->enabled && $this->config['user_to_groups'] !== false;
     }
 
     /**
@@ -52,10 +35,9 @@ class LdapService
      *
      * @throws LdapException
      */
-    private function getUserWithAttributes(string $userName, array $attributes): ?array
+    protected function getUserWithAttributes(string $userName, array $attributes): ?array
     {
-        $ldapConnection = $this->getConnection();
+        $connection = $this->ldap->startSystemBind($this->config);
-        $this->bindSystemUser($ldapConnection);
 
         // Clean attributes
         foreach ($attributes as $index => $attribute) {
@@ -65,12 +47,12 @@ class LdapService
         }
 
         // Find user
-        $userFilter = $this->buildFilter($this->config['user_filter'], ['user' => $userName]);
+        $userFilter = $this->buildFilter($this->config->get('user_filter'), ['user' => $userName]);
-        $baseDn = $this->config['base_dn'];
+        $baseDn = $this->config->get('base_dn');
 
-        $followReferrals = $this->config['follow_referrals'] ? 1 : 0;
+        $followReferrals = $this->config->get('follow_referrals') ? 1 : 0;
-        $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals);
+        $connection->setOption(LDAP_OPT_REFERRALS, $followReferrals);
-        $users = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $userFilter, $attributes);
+        $users = $connection->searchAndGetEntries($baseDn, $userFilter, $attributes);
         if ($users['count'] === 0) {
             return null;
         }
@@ -86,10 +68,10 @@ class LdapService
      */
     public function getUserDetails(string $userName): ?array
     {
-        $idAttr = $this->config['id_attribute'];
+        $idAttr = $this->config->get('id_attribute');
-        $emailAttr = $this->config['email_attribute'];
+        $emailAttr = $this->config->get('email_attribute');
-        $displayNameAttr = $this->config['display_name_attribute'];
+        $displayNameAttr = $this->config->get('display_name_attribute');
-        $thumbnailAttr = $this->config['thumbnail_attribute'];
+        $thumbnailAttr = $this->config->get('thumbnail_attribute');
 
         $user = $this->getUserWithAttributes($userName, array_filter([
             'cn', 'dn', $idAttr, $emailAttr, $displayNameAttr, $thumbnailAttr,
@@ -108,7 +90,7 @@ class LdapService
             '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,
@@ -155,110 +137,15 @@ class LdapService
             return false;
         }
 
-        $ldapConnection = $this->getConnection();
-
         try {
-            $ldapBind = $this->ldap->bind($ldapConnection, $ldapUserDetails['dn'], $password);
+            $this->ldap->startBind($ldapUserDetails['dn'], $password, $this->config);
-        } catch (ErrorException $e) {
+        } catch (LdapFailedBindException $e) {
-            $ldapBind = false;
+            return false;
+        } catch (LdapException $e) {
+            throw $e;
         }
 
-        return $ldapBind;
+        return true;
-    }
-
-    /**
-     * Bind the system user to the LDAP connection using the given credentials
-     * otherwise anonymous access is attempted.
-     *
-     * @param resource $connection
-     *
-     * @throws LdapException
-     */
-    protected function bindSystemUser($connection)
-    {
-        $ldapDn = $this->config['dn'];
-        $ldapPass = $this->config['pass'];
-
-        $isAnonymous = ($ldapDn === false || $ldapPass === false);
-        if ($isAnonymous) {
-            $ldapBind = $this->ldap->bind($connection);
-        } else {
-            $ldapBind = $this->ldap->bind($connection, $ldapDn, $ldapPass);
-        }
-
-        if (!$ldapBind) {
-            throw new LdapException(($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed')));
-        }
-    }
-
-    /**
-     * Get the connection to the LDAP server.
-     * Creates a new connection if one does not exist.
-     *
-     * @throws LdapException
-     *
-     * @return resource
-     */
-    protected function getConnection()
-    {
-        if ($this->ldapConnection !== null) {
-            return $this->ldapConnection;
-        }
-
-        // Check LDAP extension in installed
-        if (!function_exists('ldap_connect') && config('app.env') !== 'testing') {
-            throw new LdapException(trans('errors.ldap_extension_not_installed'));
-        }
-
-        // Disable certificate verification.
-        // This option works globally and must be set before a connection is created.
-        if ($this->config['tls_insecure']) {
-            $this->ldap->setOption(null, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
-        }
-
-        $serverDetails = $this->parseServerString($this->config['server']);
-        $ldapConnection = $this->ldap->connect($serverDetails['host'], $serverDetails['port']);
-
-        if ($ldapConnection === false) {
-            throw new LdapException(trans('errors.ldap_cannot_connect'));
-        }
-
-        // Set any required options
-        if ($this->config['version']) {
-            $this->ldap->setVersion($ldapConnection, $this->config['version']);
-        }
-
-        // Start and verify TLS if it's enabled
-        if ($this->config['start_tls']) {
-            $started = $this->ldap->startTls($ldapConnection);
-            if (!$started) {
-                throw new LdapException('Could not start TLS connection');
-            }
-        }
-
-        $this->ldapConnection = $ldapConnection;
-
-        return $this->ldapConnection;
-    }
-
-    /**
-     * Parse a 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'.
-     */
-    protected function parseServerString(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];
     }
 
     /**
@@ -269,7 +156,7 @@ class LdapService
         $newAttrs = [];
         foreach ($attrs as $key => $attrText) {
             $newKey = '${' . $key . '}';
-            $newAttrs[$newKey] = $this->ldap->escape($attrText);
+            $newAttrs[$newKey] = LdapConnection::escape($attrText);
         }
 
         return strtr($filterString, $newAttrs);
@@ -283,7 +170,7 @@ class LdapService
      */
     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) {
@@ -293,7 +180,7 @@ class LdapService
         $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,
@@ -338,17 +225,16 @@ class LdapService
      */
     private function getGroupGroups(string $groupName): array
     {
-        $ldapConnection = $this->getConnection();
+        $connection = $this->ldap->startSystemBind($this->config);
-        $this->bindSystemUser($ldapConnection);
 
-        $followReferrals = $this->config['follow_referrals'] ? 1 : 0;
+        $followReferrals = $this->config->get('follow_referrals') ? 1 : 0;
-        $this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals);
+        $connection->setOption(LDAP_OPT_REFERRALS, $followReferrals);
 
-        $baseDn = $this->config['base_dn'];
+        $baseDn = $this->config->get('base_dn');
-        $groupsAttr = strtolower($this->config['group_attribute']);
+        $groupsAttr = strtolower($this->config->get('group_attribute'));
 
-        $groupFilter = 'CN=' . $this->ldap->escape($groupName);
+        $groupFilter = 'CN=' . LdapConnection::escape($groupName);
-        $groups = $this->ldap->searchAndGetEntries($ldapConnection, $baseDn, $groupFilter, [$groupsAttr]);
+        $groups = $connection->searchAndGetEntries($baseDn, $groupFilter, [$groupsAttr]);
         if ($groups['count'] === 0) {
             return [];
         }
@@ -362,7 +248,7 @@ class LdapService
      */
     protected function groupFilter(array $userGroupSearchResponse): array
     {
-        $groupsAttr = strtolower($this->config['group_attribute']);
+        $groupsAttr = strtolower($this->config->get('group_attribute'));
         $ldapGroups = [];
         $count = 0;
 
@@ -371,7 +257,7 @@ class LdapService
         }
 
         for ($i = 0; $i < $count; $i++) {
-            $dnComponents = $this->ldap->explodeDn($userGroupSearchResponse[$groupsAttr][$i], 1);
+            $dnComponents = LdapConnection::explodeDn($userGroupSearchResponse[$groupsAttr][$i], 1);
             if (!in_array($dnComponents[0], $ldapGroups)) {
                 $ldapGroups[] = $dnComponents[0];
             }
@@ -389,7 +275,15 @@ class LdapService
     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'));
+    }
+
+    /**
+     * Check if groups should be synced.
+     */
+    public function shouldSyncGroups(): bool
+    {
+        return $this->config->get('user_to_groups') !== false;
     }
 
     /**
@@ -398,7 +292,7 @@ class LdapService
      */
     public function saveAndAttachAvatar(User $user, array $ldapUserDetails): void
     {
-        if (is_null(config('services.ldap.thumbnail_attribute')) || is_null($ldapUserDetails['avatar'])) {
+        if (is_null($this->config->get('thumbnail_attribute')) || is_null($ldapUserDetails['avatar'])) {
             return;
         }
 

app/Exceptions/LdapFailedBindException.php

@@ -0,0 +1,7 @@
+<?php
+
+namespace BookStack\Exceptions;
+
+class LdapFailedBindException extends LdapException
+{
+}

app/Providers/AuthServiceProvider.php

@@ -6,7 +6,7 @@ use BookStack\Api\ApiTokenGuard;
 use BookStack\Auth\Access\ExternalBaseUserProvider;
 use BookStack\Auth\Access\Guards\AsyncExternalBaseSessionGuard;
 use BookStack\Auth\Access\Guards\LdapSessionGuard;
-use BookStack\Auth\Access\LdapService;
+use BookStack\Auth\Access\Ldap\LdapService;
 use BookStack\Auth\Access\LoginService;
 use BookStack\Auth\Access\RegistrationService;
 use Illuminate\Support\Facades\Auth;

tests/Auth/LdapTest.php

@@ -2,8 +2,10 @@
 
 namespace Tests\Auth;
 
-use BookStack\Auth\Access\Ldap;
+use BookStack\Auth\Access\Ldap\LdapConfig;
-use BookStack\Auth\Access\LdapService;
+use BookStack\Auth\Access\Ldap\LdapConnection;
+use BookStack\Auth\Access\Ldap\LdapConnectionManager;
+use BookStack\Auth\Access\Ldap\LdapService;
 use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use Illuminate\Testing\TestResponse;
@@ -23,12 +25,15 @@ class LdapTest extends TestCase
     protected function setUp(): void
     {
         parent::setUp();
+
         if (!defined('LDAP_OPT_REFERRALS')) {
             define('LDAP_OPT_REFERRALS', 1);
         }
+
         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',
@@ -40,33 +45,20 @@ class LdapTest extends TestCase
             'services.ldap.tls_insecure'           => false,
             'services.ldap.thumbnail_attribute'    => null,
         ]);
-        $this->mockLdap = \Mockery::mock(Ldap::class);
+
-        $this->app[Ldap::class] = $this->mockLdap;
+        $this->mockLdap = \Mockery::mock(LdapConnection::class);
+        $this->app[LdapConnection::class] = $this->mockLdap;
         $this->mockUser = User::factory()->make();
     }
 
     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']);
     }
 
-    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', [
@@ -78,25 +70,22 @@ class LdapTest extends TestCase
     /**
      * 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('connect')->times($connects)->andReturn($this->resourceId);
         $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()
     {
-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);
         $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],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'  => 'dc=test' . config('services.ldap.base_dn'),
             ]]);
 
         $resp = $this->mockUserLogin();
@@ -121,13 +110,13 @@ class LdapTest extends TestCase
             'registration-restrict' => 'testing.com',
         ]);
 
-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);
         $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],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'  => 'dc=test' . config('services.ldap.base_dn'),
             ]]);
 
         $resp = $this->mockUserLogin();
@@ -146,9 +135,9 @@ class LdapTest extends TestCase
     {
         $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)
-            ->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,
@@ -165,10 +154,10 @@ class LdapTest extends TestCase
     {
         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)
-            ->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,
@@ -184,13 +173,13 @@ class LdapTest extends TestCase
 
     public function test_initial_incorrect_credentials()
     {
-        $this->commonLdapMocks(1, 1, 1, 0, 1);
+        $this->commonLdapMocks(1, 1, 0);
         $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],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'  => 'dc=test' . config('services.ldap.base_dn'),
             ]]);
         $this->mockLdap->shouldReceive('bind')->times(2)->andReturn(true, false);
 
@@ -202,9 +191,9 @@ class LdapTest extends TestCase
 
     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)
-            ->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();
@@ -277,13 +266,13 @@ class LdapTest extends TestCase
             '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)
-            ->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],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'       => 'dc=test' . config('services.ldap.base_dn'),
                 'mail'     => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 2,
@@ -322,13 +311,13 @@ class LdapTest extends TestCase
             '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)
-            ->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],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'       => 'dc=test' . config('services.ldap.base_dn'),
                 'mail'     => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 1,
@@ -364,9 +353,9 @@ class LdapTest extends TestCase
             '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)
-            ->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'),
@@ -423,13 +412,13 @@ class LdapTest extends TestCase
             '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)
-            ->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],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'       => 'dc=test' . config('services.ldap.base_dn'),
                 'mail'     => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 1,
@@ -464,13 +453,13 @@ class LdapTest extends TestCase
             '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)
-            ->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],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'       => 'dc=test' . config('services.ldap.base_dn'),
                 'mail'     => [$this->mockUser->email],
                 'memberof' => [
                     'count' => 2,
@@ -498,13 +487,13 @@ class LdapTest extends TestCase
             'services.ldap.display_name_attribute' => 'displayName',
         ]);
 
-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);
         $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],
-                'dn'          => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'          => 'dc=test' . config('services.ldap.base_dn'),
                 'displayname' => 'displayNameAttribute',
             ]]);
 
@@ -523,13 +512,13 @@ class LdapTest extends TestCase
             'services.ldap.display_name_attribute' => 'displayName',
         ]);
 
-        $this->commonLdapMocks(1, 1, 2, 4, 2);
+        $this->commonLdapMocks(1, 2, 4);
         $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],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'  => 'dc=test' . config('services.ldap.base_dn'),
             ]]);
 
         $this->mockUserLogin()->assertRedirect('/login');
@@ -546,39 +535,78 @@ class LdapTest extends TestCase
         ]);
     }
 
-    protected function checkLdapReceivesCorrectDetails($serverString, $expectedHost, $expectedPort)
+    protected function checkLdapConfigHostParsing($serverString, ...$expectedHostPortPairs)
     {
-        app('config')->set([
+        config()->set(['services.ldap.server' => $serverString]);
-            'services.ldap.server' => $serverString,
+        $ldapConfig = new LdapConfig(config('services.ldap'));
-        ]);
 
-        // Standard mocks
+        $servers = $ldapConfig->getServers();
-        $this->commonLdapMocks(0, 1, 1, 2, 1);
+        $this->assertCount(count($expectedHostPortPairs), $servers);
-        $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)->andReturn(['count' => 1, 0 => [
+        foreach ($expectedHostPortPairs as $i => $expected) {
-            'uid' => [$this->mockUser->name],
+            $server = $servers[$i];
-            'cn'  => [$this->mockUser->name],
+            $this->assertEquals($expected[0], $server['host']);
-            'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+            $this->assertEquals($expected[1], $server['port']);
-        ]]);
+        }
-
-        $this->mockLdap->shouldReceive('connect')->once()
-            ->with($expectedHost, $expectedPort)->andReturn($this->resourceId);
-        $this->mockUserLogin();
     }
 
     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()
     {
-        $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()
     {
-        $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()
+    {
+        app('config')->set([
+            'services.ldap.server' => 'ldap-tiger.example.com;ldap-donkey.example.com:8080',
+        ]);
+
+        // Standard mocks
+        $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('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();
+    }
+
+    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()
@@ -618,15 +646,15 @@ class LdapTest extends TestCase
     {
         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)
-            ->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],
                 // Test dumping binary data for avatar responses
                 'jpegphoto' => base64_decode('/9j/4AAQSkZJRg=='),
-                'dn'        => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'        => 'dc=test' . config('services.ldap.base_dn'),
             ]]);
 
         $resp = $this->post('/login', [
@@ -650,7 +678,7 @@ class LdapTest extends TestCase
     {
         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);
     }
@@ -659,13 +687,13 @@ class LdapTest extends TestCase
     {
         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)
-            ->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],
-                'dn'  => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'  => 'dc=test' . config('services.ldap.base_dn'),
             ]]);
 
         $details = $ldapService->getUserDetails('test');
@@ -674,18 +702,18 @@ class LdapTest extends TestCase
 
     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)
-            ->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],
-                'dn'   => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'   => 'dc=test' . config('services.ldap.base_dn'),
                 'mail' => 'tester@example.com',
             ]], ['count' => 1, 0 => [
                 'uid'  => ['Barry'],
                 'cn'   => ['Scott'],
-                'dn'   => ['dc=bscott' . config('services.ldap.base_dn')],
+                'dn'   => 'dc=bscott' . config('services.ldap.base_dn'),
                 'mail' => 'tester@example.com',
             ]]);
 
@@ -710,13 +738,13 @@ class LdapTest extends TestCase
             '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 => [
                 'uid'      => [$user->name],
                 'cn'       => [$user->name],
-                'dn'       => ['dc=test' . config('services.ldap.base_dn')],
+                'dn'       => 'dc=test' . config('services.ldap.base_dn'),
                 'mail'     => [$user->email],
                 'memberof' => [
                     'count' => 1,
@@ -758,10 +786,10 @@ class LdapTest extends TestCase
     {
         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)
-            ->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,