Skip to content

Commit

Permalink
Added dump_user_details option to LDAP and added binary attribute dec…
Browse files Browse the repository at this point in the history
…ode option

Related to #1872
  • Loading branch information
ssddanbrown committed Feb 15, 2020
1 parent 6caedc7 commit 29cc35a
Show file tree
Hide file tree
Showing 4 changed files with 78 additions and 4 deletions.
1 change: 1 addition & 0 deletions .env.example.complete
Expand Up @@ -200,6 +200,7 @@ LDAP_ID_ATTRIBUTE=uid
LDAP_EMAIL_ATTRIBUTE=mail
LDAP_DISPLAY_NAME_ATTRIBUTE=cn
LDAP_FOLLOW_REFERRALS=true
LDAP_DUMP_USER_DETAILS=false

# LDAP group sync configuration
# Refer to https://www.bookstackapp.com/docs/admin/ldap-auth/
Expand Down
28 changes: 25 additions & 3 deletions app/Auth/Access/LdapService.php
@@ -1,6 +1,7 @@
<?php namespace BookStack\Auth\Access;

use BookStack\Auth\User;
use BookStack\Exceptions\JsonDebugException;
use BookStack\Exceptions\LdapException;
use ErrorException;

Expand Down Expand Up @@ -76,26 +77,47 @@ public function getUserDetails(string $userName): ?array
}

$userCn = $this->getUserResponseProperty($user, 'cn', null);
return [
$formatted = [
'uid' => $this->getUserResponseProperty($user, $idAttr, $user['dn']),
'name' => $this->getUserResponseProperty($user, $displayNameAttr, $userCn),
'dn' => $user['dn'],
'email' => $this->getUserResponseProperty($user, $emailAttr, null),
];

if ($this->config['dump_user_details']) {
throw new JsonDebugException([
'details_from_ldap' => $user,
'details_bookstack_parsed' => $formatted,
]);
}

return $formatted;
}

/**
* Get a property from an LDAP user response fetch.
* Handles properties potentially being part of an array.
* If the given key is prefixed with 'BIN;', that indicator will be stripped
* from the key and any fetched values will be converted from binary to hex.
*/
protected function getUserResponseProperty(array $userDetails, string $propertyKey, $defaultValue)
{
$isBinary = strpos($propertyKey, 'BIN;') === 0;
$propertyKey = strtolower($propertyKey);
$value = $defaultValue;

if ($isBinary) {
$propertyKey = substr($propertyKey, strlen('BIN;'));
}

if (isset($userDetails[$propertyKey])) {
return (is_array($userDetails[$propertyKey]) ? $userDetails[$propertyKey][0] : $userDetails[$propertyKey]);
$value = (is_array($userDetails[$propertyKey]) ? $userDetails[$propertyKey][0] : $userDetails[$propertyKey]);
if ($isBinary) {
$value = bin2hex($value);
}
}

return $defaultValue;
return $value;
}

/**
Expand Down
1 change: 1 addition & 0 deletions app/Config/services.php
Expand Up @@ -118,6 +118,7 @@

'ldap' => [
'server' => env('LDAP_SERVER', false),
'dump_user_details' => env('LDAP_DUMP_USER_DETAILS', false),
'dn' => env('LDAP_DN', false),
'pass' => env('LDAP_PASS', false),
'base_dn' => env('LDAP_BASE_DN', false),
Expand Down
52 changes: 51 additions & 1 deletion tests/Auth/LdapTest.php
@@ -1,5 +1,6 @@
<?php namespace Tests;

use BookStack\Auth\Access\LdapService;
use BookStack\Auth\Role;
use BookStack\Auth\Access\Ldap;
use BookStack\Auth\User;
Expand All @@ -20,7 +21,7 @@ public function setUp(): void
{
parent::setUp();
if (!defined('LDAP_OPT_REFERRALS')) define('LDAP_OPT_REFERRALS', 1);
app('config')->set([
config()->set([
'auth.method' => 'ldap',
'auth.defaults.guard' => 'ldap',
'services.ldap.base_dn' => 'dc=ldap,dc=local',
Expand Down Expand Up @@ -560,4 +561,53 @@ public function test_user_register_routes_inaccessible()
$resp = $this->post('/register');
$this->assertPermissionError($resp);
}

public function test_dump_user_details_option_works()
{
config()->set(['services.ldap.dump_user_details' => true]);

$this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
$this->mockLdap->shouldReceive('setVersion')->once();
$this->mockLdap->shouldReceive('setOption')->times(1);
$this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
->with($this->resourceId, 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')]
]]);
$this->mockLdap->shouldReceive('bind')->times(1)->andReturn(true);
$this->mockEscapes(1);

$this->post('/login', [
'username' => $this->mockUser->name,
'password' => $this->mockUser->password,
]);
$this->seeJsonStructure([
'details_from_ldap' => [],
'details_bookstack_parsed' => [],
]);
}

public function test_ldap_attributes_can_be_binary_decoded_if_marked()
{
config()->set(['services.ldap.id_attribute' => 'BIN;uid']);
$ldapService = app()->make(LdapService::class);

$this->mockLdap->shouldReceive('connect')->once()->andReturn($this->resourceId);
$this->mockLdap->shouldReceive('setVersion')->once();
$this->mockLdap->shouldReceive('setOption')->times(1);
$this->mockLdap->shouldReceive('searchAndGetEntries')->times(1)
->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
->andReturn(['count' => 1, 0 => [
'uid' => [hex2bin('FFF8F7')],
'cn' => [$this->mockUser->name],
'dn' => ['dc=test' . config('services.ldap.base_dn')]
]]);
$this->mockLdap->shouldReceive('bind')->times(1)->andReturn(true);
$this->mockEscapes(1);

$details = $ldapService->getUserDetails('test');
$this->assertEquals('fff8f7', $details['uid']);
}
}

0 comments on commit 29cc35a

Please sign in to comment.