From 93433fdb0f87b5da791299335a7863ab04da7335 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Fri, 25 Nov 2022 17:45:06 +0000 Subject: [PATCH] Added more complexity in an attempt to make ldap host failover fit --- app/Auth/Access/Ldap/LdapConfig.php | 60 ++++++++ app/Auth/Access/Ldap/LdapConnection.php | 14 +- .../Access/Ldap/LdapConnectionManager.php | 58 ++----- app/Auth/Access/Ldap/LdapService.php | 40 ++--- tests/Auth/LdapTest.php | 142 ++++++++---------- 5 files changed, 167 insertions(+), 147 deletions(-) create mode 100644 app/Auth/Access/Ldap/LdapConfig.php diff --git a/app/Auth/Access/Ldap/LdapConfig.php b/app/Auth/Access/Ldap/LdapConfig.php new file mode 100644 index 000000000..a96f20713 --- /dev/null +++ b/app/Auth/Access/Ldap/LdapConfig.php @@ -0,0 +1,60 @@ +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 + */ + 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 f9eb9f17e..9702c1e87 100644 --- a/app/Auth/Access/Ldap/LdapConnection.php +++ b/app/Auth/Access/Ldap/LdapConnection.php @@ -16,19 +16,25 @@ class LdapConnection */ protected $connection; + protected string $hostName; + protected 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 */ - 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 e5720a49a..ab16f5b0e 100644 --- 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. */ - public function startSystemBind(array $config): LdapConnection + public function startSystemBind(LdapConfig $config): LdapConnection { // Incoming options are string|false - $dn = $config['dn']; - $pass = $config['pass']; + $dn = $config->get('dn'); + $pass = $config->get('pass'); $isAnonymous = ($dn === false || $pass === false); @@ -39,7 +39,7 @@ class LdapConnectionManager * * @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') { @@ -48,11 +48,11 @@ class LdapConnectionManager // 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); } - $serverDetails = $this->parseMultiServerString($config['server']); + $serverDetails = $config->getServers(); $lastException = null; foreach ($serverDetails as $server) { @@ -85,25 +85,26 @@ class LdapConnectionManager * 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]; } - $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 ($config['version']) { - $ldapConnection->setVersion($config['version']); + if ($config->get('version')) { + $ldapConnection->setVersion($config->get('version')); } // Start and verify TLS if it's enabled - if ($config['start_tls']) { + if ($config->get('start_tls')) { try { $tlsStarted = $ldapConnection->startTls(); } catch (ErrorException $exception) { @@ -117,39 +118,4 @@ class LdapConnectionManager 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 - */ - 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 502e6099a..e483fff04 100644 --- 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 array $config; + protected LdapConfig $config; 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 - $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) { @@ -68,10 +68,10 @@ class LdapService */ 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, @@ -90,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, @@ -170,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) { @@ -180,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, @@ -227,13 +227,13 @@ class LdapService { $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); - $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 []; @@ -248,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; @@ -275,7 +275,7 @@ 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')); } /** @@ -283,7 +283,7 @@ class LdapService */ 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 { - if (is_null($this->config['thumbnail_attribute']) || is_null($ldapUserDetails['avatar'])) { + if (is_null($this->config->get('thumbnail_attribute')) || is_null($ldapUserDetails['avatar'])) { return; } diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 1c575ec86..726595bd5 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -2,6 +2,7 @@ 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; @@ -45,9 +46,6 @@ class LdapTest extends TestCase '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(); @@ -55,26 +53,12 @@ class LdapTest extends TestCase 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', [ @@ -86,20 +70,18 @@ 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('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], @@ -128,9 +110,9 @@ 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], @@ -153,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, @@ -172,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, @@ -191,9 +173,9 @@ 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], @@ -209,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(); @@ -284,9 +266,9 @@ 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], @@ -329,9 +311,9 @@ 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], @@ -371,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'), @@ -430,9 +412,9 @@ 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], @@ -471,9 +453,9 @@ 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], @@ -505,9 +487,9 @@ 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], @@ -530,9 +512,9 @@ 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], @@ -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'; - $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() @@ -595,14 +582,15 @@ class LdapTest extends TestCase ]); // Standard mocks - $this->commonLdapMocks(0, 1, 1, 2, 1); $this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)->andReturn(['count' => 1, 0 => [ 'uid' => [$this->mockUser->name], 'cn' => [$this->mockUser->name], 'dn' => 'dc=test' . config('services.ldap.base_dn'), ]]); - $this->mockLdap->shouldReceive('connect')->once()->with('ldap-tiger.example.com', 389)->andReturn(false); + $this->mockLdap->shouldReceive('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(); } @@ -658,9 +646,9 @@ 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], @@ -690,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); } @@ -699,9 +687,9 @@ 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], @@ -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() { - $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], @@ -750,7 +738,7 @@ 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 => [ @@ -798,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,