Skip to content

Commit f421d83

Browse files
ssddanbrownssddanbrown
committed
Added ability to set custom ldap group -> role mapping
Added input in role form to allow matching against custom names. Changed default mapping to use role display name instead of the hidden DB name.
1 parent be2ca9d commit f421d83

9 files changed

Lines changed: 146 additions & 26 deletions

File tree

‎.env.example‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -71,8 +71,6 @@ LDAP_VERSION=false
7171
LDAP_USER_TO_GROUPS=false
7272
# What is the LDAP attribute for group memberships
7373
LDAP_GROUP_ATTRIBUTE="memberOf"
74-
# What LDAP group should the user be a part of to be an admin on BookStack
75-
LDAP_ADMIN_GROUP="Domain Admins"
7674
# Would you like to remove users from roles on BookStack if they do not match on LDAP
7775
# If false, the ldap groups-roles sync will only add users to roles
7876
LDAP_REMOVE_FROM_GROUPS=false

‎app/Http/Controllers/PermissionController.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -78,6 +78,7 @@ public function editRole($id)
7878
* @param $id
7979
* @param Request $request
8080
* @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
81+
* @throws PermissionsException
8182
*/
8283
public function updateRole($id, Request $request)
8384
{

‎app/Role.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
class Role extends Model
44
{
55

6-
protected $fillable = ['display_name', 'description'];
6+
protected $fillable = ['display_name', 'description', 'external_auth_id'];
77

88
/**
99
* The roles that belong to the role.

‎app/Services/LdapService.php‎

Lines changed: 44 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use BookStack\Role;
66
use BookStack\User;
77
use Illuminate\Contracts\Auth\Authenticatable;
8+
use Illuminate\Database\Eloquent\Builder;
89

910
/**
1011
* Class LdapService
@@ -299,15 +300,13 @@ protected function groupFilter($ldapSearchReturn)
299300
* Sync the LDAP groups to the user roles for the current user
300301
* @param \BookStack\User $user
301302
* @throws LdapException
302-
* @throws \BookStack\Exceptions\NotFoundException
303303
*/
304304
public function syncGroups(User $user)
305305
{
306306
$userLdapGroups = $this->getUserGroups($user->external_auth_id);
307-
$userLdapGroups = $this->groupNameFilter($userLdapGroups);
308307

309308
// Get the ids for the roles from the names
310-
$ldapGroupsAsRoles = Role::query()->whereIn('name', $userLdapGroups)->pluck('id');
309+
$ldapGroupsAsRoles = $this->matchLdapGroupsToSystemsRoles($userLdapGroups);
311310

312311
// Sync groups
313312
if ($this->config['remove_from_groups']) {
@@ -316,26 +315,55 @@ public function syncGroups(User $user)
316315
} else {
317316
$user->roles()->syncWithoutDetaching($ldapGroupsAsRoles);
318317
}
318+
}
319319

320-
// make the user an admin?
321-
// TODO - Remove
322-
if (in_array($this->config['admin'], $userLdapGroups)) {
323-
$this->userRepo->attachSystemRole($user, 'admin');
320+
/**
321+
* Match an array of group names from LDAP to BookStack system roles.
322+
* Formats LDAP group names to be lower-case and hyphenated.
323+
* @param array $groupNames
324+
* @return \Illuminate\Support\Collection
325+
*/
326+
protected function matchLdapGroupsToSystemsRoles(array $groupNames)
327+
{
328+
foreach ($groupNames as $i => $groupName) {
329+
$groupNames[$i] = str_replace(' ', '-', trim(strtolower($groupName)));
324330
}
331+
332+
$roles = Role::query()->where(function(Builder $query) use ($groupNames) {
333+
$query->whereIn('name', $groupNames);
334+
foreach ($groupNames as $groupName) {
335+
$query->orWhere('external_auth_id', 'LIKE', '%' . $groupName . '%');
336+
}
337+
})->get();
338+
339+
$matchedRoles = $roles->filter(function(Role $role) use ($groupNames) {
340+
return $this->roleMatchesGroupNames($role, $groupNames);
341+
});
342+
343+
return $matchedRoles->pluck('id');
325344
}
326345

327346
/**
328-
* Filter to convert the groups from ldap to the format of the roles name on BookStack
329-
* Spaces replaced with -, all lowercase letters
330-
* @param array $groups
331-
* @return array
347+
* Check a role against an array of group names to see if it matches.
348+
* Checked against role 'external_auth_id' if set otherwise the name of the role.
349+
* @param Role $role
350+
* @param array $groupNames
351+
* @return bool
332352
*/
333-
private function groupNameFilter(array $groups)
353+
protected function roleMatchesGroupNames(Role $role, array $groupNames)
334354
{
335-
$return = [];
336-
foreach ($groups as $groupName) {
337-
$return[] = str_replace(' ', '-', strtolower($groupName));
355+
if ($role->external_auth_id) {
356+
$externalAuthIds = explode(',', strtolower($role->external_auth_id));
357+
foreach ($externalAuthIds as $externalAuthId) {
358+
if (in_array(trim($externalAuthId), $groupNames)) {
359+
return true;
360+
}
361+
}
362+
return false;
338363
}
339-
return $return;
364+
365+
$roleName = str_replace(' ', '-', trim(strtolower($role->display_name)));
366+
return in_array($roleName, $groupNames);
340367
}
368+
341369
}

‎config/services.php‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,6 @@
120120
'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false),
121121
'user_to_groups' => env('LDAP_USER_TO_GROUPS',false),
122122
'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'),
123-
'admin' => env('LDAP_ADMIN_GROUP','Domain Admins'),
124123
'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS',false),
125124
]
126125

‎database/migrations/2018_07_15_173514_add_role_external_auth_id.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
<?php
2+
3+
use Illuminate\Support\Facades\Schema;
4+
use Illuminate\Database\Schema\Blueprint;
5+
use Illuminate\Database\Migrations\Migration;
6+
7+
class AddRoleExternalAuthId extends Migration
8+
{
9+
/**
10+
* Run the migrations.
11+
*
12+
* @return void
13+
*/
14+
public function up()
15+
{
16+
Schema::table('roles', function (Blueprint $table) {
17+
$table->string('external_auth_id', 200)->default('');
18+
$table->index('external_auth_id');
19+
});
20+
}
21+
22+
/**
23+
* Reverse the migrations.
24+
*
25+
* @return void
26+
*/
27+
public function down()
28+
{
29+
Schema::table('roles', function (Blueprint $table) {
30+
$table->dropColumn('external_auth_id');
31+
});
32+
}
33+
}

‎resources/lang/en/settings.php‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@
8282
'role_details' => 'Role Details',
8383
'role_name' => 'Role Name',
8484
'role_desc' => 'Short Description of Role',
85+
'role_external_auth_id' => 'External Authentication IDs',
8586
'role_system' => 'System Permissions',
8687
'role_manage_users' => 'Manage users',
8788
'role_manage_roles' => 'Manage roles & role permissions',

‎resources/views/settings/roles/form.blade.php‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,14 @@
1515
<label for="name">{{ trans('settings.role_desc') }}</label>
1616
@include('form/text', ['name' => 'description'])
1717
</div>
18+
19+
@if(config('auth.method') === 'ldap')
20+
<div class="form-group">
21+
<label for="name">{{ trans('settings.role_external_auth_id') }}</label>
22+
@include('form/text', ['name' => 'external_auth_id'])
23+
</div>
24+
@endif
25+
1826
<h5>{{ trans('settings.role_system') }}</h5>
1927
<label>@include('settings/roles/checkbox', ['permission' => 'users-manage']) {{ trans('settings.role_manage_users') }}</label>
2028
<label>@include('settings/roles/checkbox', ['permission' => 'user-roles-manage']) {{ trans('settings.role_manage_roles') }}</label>

‎tests/Auth/LdapTest.php‎

Lines changed: 58 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -148,8 +148,8 @@ public function test_non_admins_cannot_change_auth_id()
148148

149149
public function test_login_maps_roles_and_retains_existsing_roles()
150150
{
151-
$roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']);
152-
$roleToRecieve2 = factory(Role::class)->create(['name' => 'ldaptester-second']);
151+
$roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']);
152+
$roleToReceive2 = factory(Role::class)->create(['name' => 'ldaptester-second', 'display_name' => 'LdapTester Second']);
153153
$existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
154154
$this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
155155
$this->mockUser->attachRole($existingRole);
@@ -187,11 +187,11 @@ public function test_login_maps_roles_and_retains_existsing_roles()
187187
$user = User::where('email', $this->mockUser->email)->first();
188188
$this->seeInDatabase('role_user', [
189189
'user_id' => $user->id,
190-
'role_id' => $roleToRecieve->id
190+
'role_id' => $roleToReceive->id
191191
]);
192192
$this->seeInDatabase('role_user', [
193193
'user_id' => $user->id,
194-
'role_id' => $roleToRecieve2->id
194+
'role_id' => $roleToReceive2->id
195195
]);
196196
$this->seeInDatabase('role_user', [
197197
'user_id' => $user->id,
@@ -201,7 +201,7 @@ public function test_login_maps_roles_and_retains_existsing_roles()
201201

202202
public function test_login_maps_roles_and_removes_old_roles_if_set()
203203
{
204-
$roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']);
204+
$roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']);
205205
$existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
206206
$this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
207207
$this->mockUser->attachRole($existingRole);
@@ -238,12 +238,64 @@ public function test_login_maps_roles_and_removes_old_roles_if_set()
238238
$user = User::where('email', $this->mockUser->email)->first();
239239
$this->seeInDatabase('role_user', [
240240
'user_id' => $user->id,
241-
'role_id' => $roleToRecieve->id
241+
'role_id' => $roleToReceive->id
242242
]);
243243
$this->dontSeeInDatabase('role_user', [
244244
'user_id' => $user->id,
245245
'role_id' => $existingRole->id
246246
]);
247247
}
248248

249+
public function test_external_auth_id_visible_in_roles_page_when_ldap_active()
250+
{
251+
$role = factory(Role::class)->create(['name' => 'ldaptester', 'external_auth_id' => 'ex-auth-a, test-second-param']);
252+
$this->asAdmin()->visit('/settings/roles/' . $role->id)
253+
->see('ex-auth-a');
254+
}
255+
256+
public function test_login_maps_roles_using_external_auth_ids_if_set()
257+
{
258+
$roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'external_auth_id' => 'test-second-param, ex-auth-a']);
259+
$roleToNotReceive = factory(Role::class)->create(['name' => 'ldaptester-not-receive', 'display_name' => 'ex-auth-a', 'external_auth_id' => 'test-second-param']);
260+
261+
app('config')->set([
262+
'services.ldap.user_to_groups' => true,
263+
'services.ldap.group_attribute' => 'memberOf',
264+
'services.ldap.remove_from_groups' => true,
265+
]);
266+
$this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId);
267+
$this->mockLdap->shouldReceive('setVersion')->times(2);
268+
$this->mockLdap->shouldReceive('setOption')->times(4);
269+
$this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
270+
->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
271+
->andReturn(['count' => 1, 0 => [
272+
'uid' => [$this->mockUser->name],
273+
'cn' => [$this->mockUser->name],
274+
'dn' => ['dc=test' . config('services.ldap.base_dn')],
275+
'mail' => [$this->mockUser->email],
276+
'memberof' => [
277+
'count' => 1,
278+
0 => "cn=ex-auth-a,ou=groups,dc=example,dc=com",
279+
]
280+
]]);
281+
$this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true);
282+
283+
$this->visit('/login')
284+
->see('Username')
285+
->type($this->mockUser->name, '#username')
286+
->type($this->mockUser->password, '#password')
287+
->press('Log In')
288+
->seePageIs('/');
289+
290+
$user = User::where('email', $this->mockUser->email)->first();
291+
$this->seeInDatabase('role_user', [
292+
'user_id' => $user->id,
293+
'role_id' => $roleToReceive->id
294+
]);
295+
$this->dontSeeInDatabase('role_user', [
296+
'user_id' => $user->id,
297+
'role_id' => $roleToNotReceive->id
298+
]);
299+
}
300+
249301
}

0 commit comments

Comments
 (0)