From 17bca662a7757a66538a8074527b43b2ba20a592 Mon Sep 17 00:00:00 2001 From: Dan Brown Date: Sun, 15 Jul 2018 17:57:25 +0100 Subject: [PATCH] Added tests to cover ldap group mapping Also updated .env.example formatting. Updated how LdapRepo uses Ldap so can be mocked by testing. --- .env.example | 10 +- app/Http/Controllers/Auth/LoginController.php | 3 +- app/Repos/LdapRepo.php | 7 +- tests/Auth/LdapTest.php | 119 +++++++++++++++++- 4 files changed, 126 insertions(+), 13 deletions(-) diff --git a/.env.example b/.env.example index 57e6af6a9..4bb2555b0 100644 --- a/.env.example +++ b/.env.example @@ -67,14 +67,14 @@ LDAP_DN=false LDAP_PASS=false LDAP_USER_FILTER=false LDAP_VERSION=false -#do you want to sync LDAP groups to BookStack roles for a user +# Do you want to sync LDAP groups to BookStack roles for a user LDAP_USER_TO_GROUPS=false -#what is the LDAP attribute for group memberships +# What is the LDAP attribute for group memberships LDAP_GROUP_ATTRIBUTE="memberOf" -#what LDAP group should the user be a part of to be an admin on BookStack +# What LDAP group should the user be a part of to be an admin on BookStack LDAP_ADMIN_GROUP="Domain Admins" -#would you like to remove users from roles on bookstack if they do not match on LDAP -#if false, the ldap groups-roles sync will only add users to roles +# Would you like to remove users from roles on BookStack if they do not match on LDAP +# If false, the ldap groups-roles sync will only add users to roles LDAP_REMOVE_FROM_GROUPS=false # Mail settings diff --git a/app/Http/Controllers/Auth/LoginController.php b/app/Http/Controllers/Auth/LoginController.php index 21c9dc4c5..08b1bce67 100644 --- a/app/Http/Controllers/Auth/LoginController.php +++ b/app/Http/Controllers/Auth/LoginController.php @@ -6,6 +6,7 @@ use BookStack\Exceptions\AuthException; use BookStack\Http\Controllers\Controller; use BookStack\Repos\UserRepo; use BookStack\Repos\LdapRepo; +use BookStack\Services\LdapService; use BookStack\Services\SocialAuthService; use Illuminate\Contracts\Auth\Authenticatable; use Illuminate\Foundation\Auth\AuthenticatesUsers; @@ -99,7 +100,7 @@ class LoginController extends Controller // ldap groups refresh if (config('services.ldap.user_to_groups') !== false && $request->filled('username')) { - $ldapRepo = new LdapRepo($this->userRepo); + $ldapRepo = new LdapRepo($this->userRepo, app(LdapService::class)); $ldapRepo->syncGroups($user, $request->input('username')); } diff --git a/app/Repos/LdapRepo.php b/app/Repos/LdapRepo.php index 12bde0cdd..e57872039 100644 --- a/app/Repos/LdapRepo.php +++ b/app/Repos/LdapRepo.php @@ -1,9 +1,7 @@ config = config('services.ldap'); @@ -25,7 +24,7 @@ class LdapRepo return false; } - $this->ldapService = new LdapService(new Ldap); + $this->ldapService = $ldapService; $this->userRepo = $userRepo; } diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php index 8880c7b65..ada33d692 100644 --- a/tests/Auth/LdapTest.php +++ b/tests/Auth/LdapTest.php @@ -1,10 +1,17 @@ set(['auth.method' => 'ldap', 'services.ldap.base_dn' => 'dc=ldap,dc=local', 'auth.providers.users.driver' => 'ldap']); - $this->mockLdap = \Mockery::mock(\BookStack\Services\Ldap::class); - $this->app['BookStack\Services\Ldap'] = $this->mockLdap; + app('config')->set([ + 'auth.method' => 'ldap', + 'services.ldap.base_dn' => 'dc=ldap,dc=local', + 'services.ldap.email_attribute' => 'mail', + 'services.ldap.user_to_groups' => false, + 'auth.providers.users.driver' => 'ldap', + ]); + $this->mockLdap = \Mockery::mock(Ldap::class); + $this->app[Ldap::class] = $this->mockLdap; $this->mockUser = factory(User::class)->make(); } @@ -133,4 +146,104 @@ class LdapTest extends BrowserKitTest ->dontSee('External Authentication'); } + public function test_login_maps_roles_and_retains_existsing_roles() + { + $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']); + $roleToRecieve2 = factory(Role::class)->create(['name' => 'ldaptester-second']); + $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']); + $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save(); + $this->mockUser->attachRole($existingRole); + + app('config')->set([ + 'services.ldap.user_to_groups' => true, + 'services.ldap.group_attribute' => 'memberOf', + 'services.ldap.remove_from_groups' => false, + ]); + $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(2); + $this->mockLdap->shouldReceive('setOption')->times(5); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(5) + ->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')], + 'mail' => [$this->mockUser->email], + 'memberof' => [ + 'count' => 2, + 0 => "cn=ldaptester,ou=groups,dc=example,dc=com", + 1 => "cn=ldaptester-second,ou=groups,dc=example,dc=com", + ] + ]]); + $this->mockLdap->shouldReceive('bind')->times(6)->andReturn(true); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/'); + + $user = User::where('email', $this->mockUser->email)->first(); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToRecieve->id + ]); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToRecieve2->id + ]); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $existingRole->id + ]); + } + + public function test_login_maps_roles_and_removes_old_roles_if_set() + { + $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']); + $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']); + $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save(); + $this->mockUser->attachRole($existingRole); + + app('config')->set([ + 'services.ldap.user_to_groups' => true, + 'services.ldap.group_attribute' => 'memberOf', + 'services.ldap.remove_from_groups' => true, + ]); + $this->mockLdap->shouldReceive('connect')->times(2)->andReturn($this->resourceId); + $this->mockLdap->shouldReceive('setVersion')->times(2); + $this->mockLdap->shouldReceive('setOption')->times(4); + $this->mockLdap->shouldReceive('searchAndGetEntries')->times(4) + ->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')], + 'mail' => [$this->mockUser->email], + 'memberof' => [ + 'count' => 1, + 0 => "cn=ldaptester,ou=groups,dc=example,dc=com", + ] + ]]); + $this->mockLdap->shouldReceive('bind')->times(5)->andReturn(true); + + $this->visit('/login') + ->see('Username') + ->type($this->mockUser->name, '#username') + ->type($this->mockUser->password, '#password') + ->press('Log In') + ->seePageIs('/'); + + $user = User::where('email', $this->mockUser->email)->first(); + $this->seeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $roleToRecieve->id + ]); + $this->dontSeeInDatabase('role_user', [ + 'user_id' => $user->id, + 'role_id' => $existingRole->id + ]); + } + }