webapp/bookstack
1
0
Fork 0
mirror of https://github.com/BookStackApp/BookStack.git synced 2025-08-07 23:03:00 +03:00
Code Activity

Combined LDAP connect and bind for failover handling

Browse Source 
Not totally happy with the complexity in approach, could maybe do with
refactoring some of this out, but getting a little tired spending time
on this.
This commit is contained in:
Dan Brown
2022-10-17 17:46:14 +01:00
parent 303dbf9b01
commit 3d8df952b7
3 changed files with 94 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

11
app/Auth/Access/Ldap.php
View File

@@ -2,6 +2,8 @@
namespace BookStack\Auth\Access;
use ErrorException;
/**
* Class Ldap
* An object-orientated thin abstraction wrapper for common PHP LDAP functions.
@@ -32,6 +34,8 @@ class Ldap
/**
* Start TLS on the given LDAP connection.
*
* @throws ErrorException
*/
public function startTls($ldapConnection): bool
{
@@ -97,12 +101,9 @@ class Ldap
* Bind to LDAP directory.
*
* @param resource $ldapConnection
* @param string $bindRdn
* @param string $bindPassword
*
* @return bool
* @throws ErrorException
*/
public function bind($ldapConnection, $bindRdn = null, $bindPassword = null)
public function bind($ldapConnection, string $bindRdn = null, string $bindPassword = null): bool
{
return ldap_bind($ldapConnection, $bindRdn, $bindPassword);
}

143
app/Auth/Access/LdapService.php
View File

@@ -5,6 +5,7 @@ namespace BookStack\Auth\Access;
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;
@@ -19,11 +20,6 @@ class LdapService
protected GroupSyncService $groupSyncService;
protected UserAvatars $userAvatars;
/**
* @var resource
*/
protected $ldapConnection;
protected array $config;
protected bool $enabled;
@@ -52,10 +48,9 @@ class LdapService
*
* @throws LdapException
*/
private function getUserWithAttributes(string $userName, array $attributes): ?array
protected function getUserWithAttributes(string $userName, array $attributes): ?array
{
$ldapConnection = $this->getConnection();
$this->bindSystemUser($ldapConnection);
$ldapConnection = $this->bindConnection();
// Clean attributes
foreach ($attributes as $index => $attribute) {
@@ -155,55 +150,31 @@ class LdapService
return false;
}
$ldapConnection = $this->getConnection();
try {
$ldapBind = $this->ldap->bind($ldapConnection, $ldapUserDetails['dn'], $password);
} catch (ErrorException $e) {
$ldapBind = false;
$this->bindConnection($ldapUserDetails['dn'], $password);
} catch (LdapFailedBindException $e) {
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.
* Attempted to start and bind to a new LDAP connection.
* Will attempt against multiple defined fail-over hosts if set.
*
* @throws LdapException
* Throws a LdapFailedBindException error if the bind connected but failed.
* Otherwise, generic LdapException errors would be thrown.
*
* @return resource
* @throws LdapException
*/
protected function getConnection()
protected function bindConnection(string $dn = null, string $password = null)
{
if ($this->ldapConnection !== null) {
return $this->ldapConnection;
}
$systemBind = ($dn === null && $password === null);
// Check LDAP extension in installed
if (!function_exists('ldap_connect') && config('app.env') !== 'testing') {
@@ -217,32 +188,81 @@ class LdapService
}
$serverDetails = $this->parseMultiServerString($this->config['server']);
$this->ldapConnection = $this->prepareServerConnection($serverDetails);
return $this->ldapConnection;
}
/**
* Processes an array of received servers and returns the first working connection.
*
* @param array $serverDetails array<array{host: string, port: int}>
* @return resource
* @throws LdapException
*/
protected function prepareServerConnection(array $serverDetails)
{
$lastException = null;
foreach ($serverDetails as $server) {
try {
return $this->startServerConnection($server);
$connection = $this->startServerConnection($server);
} catch (LdapException $exception) {
$lastException = $exception;
continue;
}
try {
if ($systemBind) {
$this->bindSystemUser($connection);
} else {
$this->bindGivenUser($connection, $dn, $password);
}
} catch (LdapFailedBindException $exception) {
// Rethrow simply to indicate the importance of handling this exception case
// to indicate auth status. We skip past attempting fail-over hosts in this case since it's
// likely the connection worked here but the bind was unauthorised.
throw $exception;
} catch (ErrorException $exception) {
Log::error('LDAP bind error: ' . $exception->getMessage());
$lastException = new LdapException('Encountered error during LDAP bind');
continue;
}
return $connection;
}
throw $lastException;
}
/**
* Bind to the given LDAP connection using the given credentials.
* MUST throw an exception on failure.
*
* @param resource $connection
*
* @throws LdapFailedBindException
*/
protected function bindGivenUser($connection, string $dn = null, string $password = null): void
{
$ldapBind = $this->ldap->bind($connection, $dn, $password);
if (!$ldapBind) {
throw new LdapFailedBindException('Failed to bind with given user details');
}
}
/**
* Bind the system user to the LDAP connection using the configured credentials otherwise anonymous
* access is attempted. MUST throw an exception on failure.
*
* @param resource $connection
*
* @throws LdapFailedBindException
*/
protected function bindSystemUser($connection): void
{
$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 LdapFailedBindException(($isAnonymous ? trans('errors.ldap_fail_anonymous') : trans('errors.ldap_fail_authed')));
}
}
/**
* Attempt to start a server connection from the provided details.
*
@@ -391,8 +411,7 @@ class LdapService
*/
private function getGroupGroups(string $groupName): array
{
$ldapConnection = $this->getConnection();
$this->bindSystemUser($ldapConnection);
$ldapConnection = $this->bindConnection();
$followReferrals = $this->config['follow_referrals'] ? 1 : 0;
$this->ldap->setOption($ldapConnection, LDAP_OPT_REFERRALS, $followReferrals);

7
app/Exceptions/LdapFailedBindException.php Normal file
View File

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