mirror of
https://github.com/BookStackApp/BookStack.git
synced 2025-03-14 22:45:16 +08:00
Updated ldap server option parsing to work with protocol and port
- Aligns with PHP behaviour where ports is ignore for full LDAP URI. - Added tests to check format being passed to LDAP is as expected. - May be related to #1220 - Related to #1386 and #1278
This commit is contained in:
parent
d0db0f8e26
commit
c24764018a
@ -91,7 +91,7 @@ class LdapService
|
||||
$userCn = $this->getUserResponseProperty($user, 'cn', null);
|
||||
return [
|
||||
'uid' => $this->getUserResponseProperty($user, 'uid', $user['dn']),
|
||||
'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),
|
||||
'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),
|
||||
'dn' => $user['dn'],
|
||||
'email' => $this->getUserResponseProperty($user, $emailAttr, null),
|
||||
];
|
||||
@ -116,8 +116,8 @@ class LdapService
|
||||
|
||||
/**
|
||||
* @param Authenticatable $user
|
||||
* @param string $username
|
||||
* @param string $password
|
||||
* @param string $username
|
||||
* @param string $password
|
||||
* @return bool
|
||||
* @throws LdapException
|
||||
*/
|
||||
@ -182,25 +182,14 @@ class LdapService
|
||||
throw new LdapException(trans('errors.ldap_extension_not_installed'));
|
||||
}
|
||||
|
||||
// Get port from server string and protocol if specified.
|
||||
$ldapServer = explode(':', $this->config['server']);
|
||||
$hasProtocol = preg_match('/^ldaps{0,1}\:\/\//', $this->config['server']) === 1;
|
||||
if (!$hasProtocol) {
|
||||
array_unshift($ldapServer, '');
|
||||
}
|
||||
$hostName = $ldapServer[0] . ($hasProtocol?':':'') . $ldapServer[1];
|
||||
$defaultPort = $ldapServer[0] === 'ldaps' ? 636 : 389;
|
||||
|
||||
/*
|
||||
* Check if TLS_INSECURE is set. The handle is set to NULL due to the nature of
|
||||
* the LDAP_OPT_X_TLS_REQUIRE_CERT option. It can only be set globally and not
|
||||
* per handle.
|
||||
*/
|
||||
// Check if TLS_INSECURE is set. The handle is set to NULL due to the nature of
|
||||
// the LDAP_OPT_X_TLS_REQUIRE_CERT option. It can only be set globally and not per handle.
|
||||
if ($this->config['tls_insecure']) {
|
||||
$this->ldap->setOption(null, LDAP_OPT_X_TLS_REQUIRE_CERT, LDAP_OPT_X_TLS_NEVER);
|
||||
}
|
||||
|
||||
$ldapConnection = $this->ldap->connect($hostName, count($ldapServer) > 2 ? intval($ldapServer[2]) : $defaultPort);
|
||||
$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'));
|
||||
@ -215,6 +204,27 @@ class LdapService
|
||||
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'
|
||||
* @param $serverString
|
||||
* @return array
|
||||
*/
|
||||
protected function parseServerString($serverString)
|
||||
{
|
||||
$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];
|
||||
}
|
||||
|
||||
/**
|
||||
* Build a filter string by injecting common variables.
|
||||
* @param string $filterString
|
||||
@ -319,10 +329,10 @@ class LdapService
|
||||
$count = 0;
|
||||
|
||||
if (isset($userGroupSearchResponse[$groupsAttr]['count'])) {
|
||||
$count = (int) $userGroupSearchResponse[$groupsAttr]['count'];
|
||||
$count = (int)$userGroupSearchResponse[$groupsAttr]['count'];
|
||||
}
|
||||
|
||||
for ($i=0; $i<$count; $i++) {
|
||||
for ($i = 0; $i < $count; $i++) {
|
||||
$dnComponents = $this->ldap->explodeDn($userGroupSearchResponse[$groupsAttr][$i], 1);
|
||||
if (!in_array($dnComponents[0], $ldapGroups)) {
|
||||
$ldapGroups[] = $dnComponents[0];
|
||||
|
@ -409,4 +409,41 @@ class LdapTest extends BrowserKitTest
|
||||
->seeInDatabase('users', ['email' => $this->mockUser->email, 'email_confirmed' => false, 'external_auth_id' => $this->mockUser->name, 'name' => $this->mockUser->name]);
|
||||
}
|
||||
|
||||
protected function checkLdapReceivesCorrectDetails($serverString, $expectedHost, $expectedPort)
|
||||
{
|
||||
app('config')->set([
|
||||
'services.ldap.server' => $serverString
|
||||
]);
|
||||
|
||||
// Standard mocks
|
||||
$this->mockLdap->shouldReceive('setVersion')->once();
|
||||
$this->mockLdap->shouldReceive('setOption')->times(2);
|
||||
$this->mockLdap->shouldReceive('searchAndGetEntries')->times(2)->andReturn(['count' => 1, 0 => [
|
||||
'uid' => [$this->mockUser->name],
|
||||
'cn' => [$this->mockUser->name],
|
||||
'dn' => ['dc=test' . config('services.ldap.base_dn')]
|
||||
]]);
|
||||
$this->mockLdap->shouldReceive('bind')->times(3)->andReturn(true);
|
||||
$this->mockEscapes(2);
|
||||
|
||||
$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);
|
||||
}
|
||||
|
||||
public function test_ldap_port_parsed_from_server_if_host_is_not_full_uri()
|
||||
{
|
||||
$this->checkLdapReceivesCorrectDetails('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);
|
||||
}
|
||||
}
|
||||
|
Loading…
x
Reference in New Issue
Block a user