Skip to content

Commit 17bca66

Browse files
ssddanbrownssddanbrown
committed
Added tests to cover ldap group mapping
Also updated .env.example formatting. Updated how LdapRepo uses Ldap so can be mocked by testing.
1 parent 1776204 commit 17bca66

4 files changed

Lines changed: 126 additions & 13 deletions

File tree

‎.env.example‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -67,14 +67,14 @@ LDAP_DN=false
6767
LDAP_PASS=false
6868
LDAP_USER_FILTER=false
6969
LDAP_VERSION=false
70-
#do you want to sync LDAP groups to BookStack roles for a user
70+
# Do you want to sync LDAP groups to BookStack roles for a user
7171
LDAP_USER_TO_GROUPS=false
72-
#what is the LDAP attribute for group memberships
72+
# 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
74+
# What LDAP group should the user be a part of to be an admin on BookStack
7575
LDAP_ADMIN_GROUP="Domain Admins"
76-
#would you like to remove users from roles on bookstack if they do not match on LDAP
77-
#if false, the ldap groups-roles sync will only add users to roles
76+
# Would you like to remove users from roles on BookStack if they do not match on LDAP
77+
# If false, the ldap groups-roles sync will only add users to roles
7878
LDAP_REMOVE_FROM_GROUPS=false
7979

8080
# Mail settings

‎app/Http/Controllers/Auth/LoginController.php‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use BookStack\Http\Controllers\Controller;
77
use BookStack\Repos\UserRepo;
88
use BookStack\Repos\LdapRepo;
9+
use BookStack\Services\LdapService;
910
use BookStack\Services\SocialAuthService;
1011
use Illuminate\Contracts\Auth\Authenticatable;
1112
use Illuminate\Foundation\Auth\AuthenticatesUsers;
@@ -99,7 +100,7 @@ protected function authenticated(Request $request, Authenticatable $user)
99100

100101
// ldap groups refresh
101102
if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) {
102-
$ldapRepo = new LdapRepo($this->userRepo);
103+
$ldapRepo = new LdapRepo($this->userRepo, app(LdapService::class));
103104
$ldapRepo->syncGroups($user, $request->input('username'));
104105
}
105106

‎app/Repos/LdapRepo.php‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,7 @@
11
<?php namespace BookStack\Repos;
22

3-
use BookStack\Services\Ldap;
43
use BookStack\Services\LdapService;
54
use BookStack\Role;
6-
use BookStack\Repos\UserRepo;
75

86
class LdapRepo
97
{
@@ -16,16 +14,17 @@ class LdapRepo
1614
/**
1715
* LdapRepo constructor.
1816
* @param \BookStack\Repos\UserRepo $userRepo
17+
* @param LdapService $ldapService
1918
*/
20-
public function __construct(UserRepo $userRepo)
19+
public function __construct(UserRepo $userRepo, LdapService $ldapService)
2120
{
2221
$this->config = config('services.ldap');
2322

2423
if (config('auth.method') !== 'ldap') {
2524
return false;
2625
}
2726

28-
$this->ldapService = new LdapService(new Ldap);
27+
$this->ldapService = $ldapService;
2928
$this->userRepo = $userRepo;
3029
}
3130

‎tests/Auth/LdapTest.php‎

Lines changed: 116 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,33 @@
11
<?php namespace Tests;
2+
use BookStack\Role;
3+
use BookStack\Services\Ldap;
24
use BookStack\User;
5+
use Mockery\MockInterface;
36

47
class LdapTest extends BrowserKitTest
58
{
69

10+
/**
11+
* @var MockInterface
12+
*/
713
protected $mockLdap;
14+
815
protected $mockUser;
916
protected $resourceId = 'resource-test';
1017

1118
public function setUp()
1219
{
1320
parent::setUp();
1421
if (!defined('LDAP_OPT_REFERRALS')) define('LDAP_OPT_REFERRALS', 1);
15-
app('config')->set(['auth.method' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'auth.providers.users.driver' => 'ldap']);
16-
$this->mockLdap = \Mockery::mock(\BookStack\Services\Ldap::class);
17-
$this->app['BookStack\Services\Ldap'] = $this->mockLdap;
22+
app('config')->set([
23+
'auth.method' => 'ldap',
24+
'services.ldap.base_dn' => 'dc=ldap,dc=local',
25+
'services.ldap.email_attribute' => 'mail',
26+
'services.ldap.user_to_groups' => false,
27+
'auth.providers.users.driver' => 'ldap',
28+
]);
29+
$this->mockLdap = \Mockery::mock(Ldap::class);
30+
$this->app[Ldap::class] = $this->mockLdap;
1831
$this->mockUser = factory(User::class)->make();
1932
}
2033

@@ -133,4 +146,104 @@ public function test_non_admins_cannot_change_auth_id()
133146
->dontSee('External Authentication');
134147
}
135148

149+
public function test_login_maps_roles_and_retains_existsing_roles()
150+
{
151+
$roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']);
152+
$roleToRecieve2 = factory(Role::class)->create(['name' => 'ldaptester-second']);
153+
$existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
154+
$this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
155+
$this->mockUser->attachRole($existingRole);
156+
157+
app('config')->set([
158+
'services.ldap.user_to_groups' => true,
159+
'services.ldap.group_attribute' => 'memberOf',
160+
'services.ldap.remove_from_groups' => false,
161+
]);
162+
$this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId);
163+
$this->mockLdap->shouldReceive('setVersion')->times(2);
164+
$this->mockLdap->shouldReceive('setOption')->times(5);
165+
$this->mockLdap->shouldReceive('searchAndGetEntries')->times(5)
166+
->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
167+
->andReturn(['count' => 1, 0 => [
168+
'uid' => [$this->mockUser->name],
169+
'cn' => [$this->mockUser->name],
170+
'dn' => ['dc=test' . config('services.ldap.base_dn')],
171+
'mail' => [$this->mockUser->email],
172+
'memberof' => [
173+
'count' => 2,
174+
0 => "cn=ldaptester,ou=groups,dc=example,dc=com",
175+
1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com",
176+
]
177+
]]);
178+
$this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true);
179+
180+
$this->visit('/login')
181+
->see('Username')
182+
->type($this->mockUser->name, '#username')
183+
->type($this->mockUser->password, '#password')
184+
->press('Log In')
185+
->seePageIs('/');
186+
187+
$user = User::where('email', $this->mockUser->email)->first();
188+
$this->seeInDatabase('role_user', [
189+
'user_id' => $user->id,
190+
'role_id' => $roleToRecieve->id
191+
]);
192+
$this->seeInDatabase('role_user', [
193+
'user_id' => $user->id,
194+
'role_id' => $roleToRecieve2->id
195+
]);
196+
$this->seeInDatabase('role_user', [
197+
'user_id' => $user->id,
198+
'role_id' => $existingRole->id
199+
]);
200+
}
201+
202+
public function test_login_maps_roles_and_removes_old_roles_if_set()
203+
{
204+
$roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']);
205+
$existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
206+
$this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
207+
$this->mockUser->attachRole($existingRole);
208+
209+
app('config')->set([
210+
'services.ldap.user_to_groups' => true,
211+
'services.ldap.group_attribute' => 'memberOf',
212+
'services.ldap.remove_from_groups' => true,
213+
]);
214+
$this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId);
215+
$this->mockLdap->shouldReceive('setVersion')->times(2);
216+
$this->mockLdap->shouldReceive('setOption')->times(4);
217+
$this->mockLdap->shouldReceive('searchAndGetEntries')->times(4)
218+
->with($this->resourceId, config('services.ldap.base_dn'), \Mockery::type('string'), \Mockery::type('array'))
219+
->andReturn(['count' => 1, 0 => [
220+
'uid' => [$this->mockUser->name],
221+
'cn' => [$this->mockUser->name],
222+
'dn' => ['dc=test' . config('services.ldap.base_dn')],
223+
'mail' => [$this->mockUser->email],
224+
'memberof' => [
225+
'count' => 1,
226+
0 => "cn=ldaptester,ou=groups,dc=example,dc=com",
227+
]
228+
]]);
229+
$this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true);
230+
231+
$this->visit('/login')
232+
->see('Username')
233+
->type($this->mockUser->name, '#username')
234+
->type($this->mockUser->password, '#password')
235+
->press('Log In')
236+
->seePageIs('/');
237+
238+
$user = User::where('email', $this->mockUser->email)->first();
239+
$this->seeInDatabase('role_user', [
240+
'user_id' => $user->id,
241+
'role_id' => $roleToRecieve->id
242+
]);
243+
$this->dontSeeInDatabase('role_user', [
244+
'user_id' => $user->id,
245+
'role_id' => $existingRole->id
246+
]);
247+
}
248+
136249
}

0 commit comments

Comments
 (0)