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.

diff --git a/.env.example b/.env.example
index 4bb2555b0b03c30c063dbe82457016bafb862591..d09db93d3a1bab9674ce10679140f959029dd569 100644 (file)
--- a/.env.example
+++ b/.env.example
@@ -71,8 +71,6 @@ LDAP_VERSION=false
 LDAP_USER_TO_GROUPS=false
 # What is the LDAP attribute for group memberships
 LDAP_GROUP_ATTRIBUTE="memberOf"
 LDAP_USER_TO_GROUPS=false
 # 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
-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
 LDAP_REMOVE_FROM_GROUPS=false
 # 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

diff --git a/app/Http/Controllers/PermissionController.php b/app/Http/Controllers/PermissionController.php
index c4c7fe9726b24acf052b0f12098f32cb0200708d..5695705d02b6b1e934a4a3a0b5872f13aa3cd973 100644 (file)
--- a/app/Http/Controllers/PermissionController.php
+++ b/app/Http/Controllers/PermissionController.php
@@ -78,6 +78,7 @@ class PermissionController extends Controller
      * @param $id
      * @param Request $request
      * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector
      * @param $id
      * @param Request $request
      * @return \Illuminate\Http\RedirectResponse|\Illuminate\Routing\Redirector

+     * @throws PermissionsException

      */
     public function updateRole($id, Request $request)
     {
      */
     public function updateRole($id, Request $request)
     {

diff --git a/app/Role.php b/app/Role.php
index e86854e791051a6f8ca2470562437ef4d0b18fbb..54a5bb18016afc69a4d01600372a68add9713f67 100644 (file)
--- a/app/Role.php
+++ b/app/Role.php
@@ -3,7 +3,7 @@
 class Role extends Model
 {
 
 class Role extends Model
 {
 

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

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

diff --git a/app/Services/LdapService.php b/app/Services/LdapService.php
index 7606f1271b68ed15ac292c18ea14dde4c7ad344f..4936b2da87842ccd8c9f8881dd973be7428ba2ea 100644 (file)
--- a/app/Services/LdapService.php
+++ b/app/Services/LdapService.php
@@ -5,6 +5,7 @@ use BookStack\Repos\UserRepo;
 use BookStack\Role;
 use BookStack\User;
 use Illuminate\Contracts\Auth\Authenticatable;
 use BookStack\Role;
 use BookStack\User;
 use Illuminate\Contracts\Auth\Authenticatable;

+use Illuminate\Database\Eloquent\Builder;

 
 /**
  * Class LdapService
 
 /**
  * Class LdapService


@@ -299,15 +300,13 @@ class LdapService
      * Sync the LDAP groups to the user roles for the current user
      * @param \BookStack\User $user
      * @throws LdapException
      * Sync the LDAP groups to the user roles for the current user
      * @param \BookStack\User $user
      * @throws LdapException

-     * @throws \BookStack\Exceptions\NotFoundException

      */
     public function syncGroups(User $user)
     {
         $userLdapGroups = $this->getUserGroups($user->external_auth_id);
      */
     public function syncGroups(User $user)
     {
         $userLdapGroups = $this->getUserGroups($user->external_auth_id);

-        $userLdapGroups = $this->groupNameFilter($userLdapGroups);

 
         // Get the ids for the roles from the names
 
         // Get the ids for the roles from the names

-        $ldapGroupsAsRoles = Role::query()->whereIn('name', $userLdapGroups)->pluck('id');
+        $ldapGroupsAsRoles = $this->matchLdapGroupsToSystemsRoles($userLdapGroups);

 
         // Sync groups
         if ($this->config['remove_from_groups']) {
 
         // Sync groups
         if ($this->config['remove_from_groups']) {


@@ -316,26 +315,55 @@ class LdapService
         } else {
             $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles);
         }
         } else {
             $user->roles()->syncWithoutDetaching($ldapGroupsAsRoles);
         }

+    }

 
 

-        // make the user an admin?
-        // TODO - Remove
-        if (in_array($this->config['admin'], $userLdapGroups)) {
-            $this->userRepo->attachSystemRole($user, 'admin');
+    /**
+     * Match an array of group names from LDAP to BookStack system roles.
+     * Formats LDAP group names to be lower-case and hyphenated.
+     * @param array $groupNames
+     * @return \Illuminate\Support\Collection
+     */
+    protected function matchLdapGroupsToSystemsRoles(array $groupNames)
+    {
+        foreach ($groupNames as $i => $groupName) {
+            $groupNames[$i] = str_replace(' ', '-', trim(strtolower($groupName)));

         }
         }

+
+        $roles = Role::query()->where(function(Builder $query) use ($groupNames) {
+            $query->whereIn('name', $groupNames);
+            foreach ($groupNames as $groupName) {
+                $query->orWhere('external_auth_id', 'LIKE', '%' . $groupName . '%');
+            }
+        })->get();
+
+        $matchedRoles = $roles->filter(function(Role $role) use ($groupNames) {
+            return $this->roleMatchesGroupNames($role, $groupNames);
+        });
+
+        return $matchedRoles->pluck('id');

     }
 
     /**
     }
 
     /**

-     * Filter to convert the groups from ldap to the format of the roles name on BookStack
-     * Spaces replaced with -, all lowercase letters
-     * @param array $groups
-     * @return array
+     * Check a role against an array of group names to see if it matches.
+     * Checked against role 'external_auth_id' if set otherwise the name of the role.
+     * @param Role $role
+     * @param array $groupNames
+     * @return bool

      */
      */

-    private function groupNameFilter(array $groups)
+    protected function roleMatchesGroupNames(Role $role, array $groupNames)

     {
     {

-        $return = [];
-        foreach ($groups as $groupName) {
-            $return[] = str_replace(' ', '-', strtolower($groupName));
+        if ($role->external_auth_id) {
+            $externalAuthIds = explode(',', strtolower($role->external_auth_id));
+            foreach ($externalAuthIds as $externalAuthId) {
+                if (in_array(trim($externalAuthId), $groupNames)) {
+                    return true;
+                }
+            }
+            return false;

         }
         }

-        return $return;
+
+        $roleName = str_replace(' ', '-', trim(strtolower($role->display_name)));
+        return in_array($roleName, $groupNames);

     }
     }

+

 }
 }

diff --git a/config/services.php b/config/services.php
index daa1606437a051e4cfc46ad49eff600a7b28f671..9c550f2fada4bc5074407ff071dcfd83009760c2 100644 (file)
--- a/config/services.php
+++ b/config/services.php
@@ -120,7 +120,6 @@ return [
         'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false),
                'user_to_groups' => env('LDAP_USER_TO_GROUPS',false),
                'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'),
         'follow_referrals' => env('LDAP_FOLLOW_REFERRALS', false),
                'user_to_groups' => env('LDAP_USER_TO_GROUPS',false),
                'group_attribute' => env('LDAP_GROUP_ATTRIBUTE', 'memberOf'),

-               'admin' => env('LDAP_ADMIN_GROUP','Domain Admins'),

                'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS',false),
        ]
 
                'remove_from_groups' => env('LDAP_REMOVE_FROM_GROUPS',false),
        ]
 

diff --git a/database/migrations/2018_07_15_173514_add_role_external_auth_id.php b/database/migrations/2018_07_15_173514_add_role_external_auth_id.php
new file mode 100644 (file)
index 0000000..706a883
--- /dev/null
+++ b/database/migrations/2018_07_15_173514_add_role_external_auth_id.php
@@ -0,0 +1,33 @@
+<?php
+
+use Illuminate\Support\Facades\Schema;
+use Illuminate\Database\Schema\Blueprint;
+use Illuminate\Database\Migrations\Migration;
+
+class AddRoleExternalAuthId extends Migration
+{
+    /**
+     * Run the migrations.
+     *
+     * @return void
+     */
+    public function up()
+    {
+        Schema::table('roles', function (Blueprint $table) {
+            $table->string('external_auth_id', 200)->default('');
+            $table->index('external_auth_id');
+        });
+    }
+
+    /**
+     * Reverse the migrations.
+     *
+     * @return void
+     */
+    public function down()
+    {
+        Schema::table('roles', function (Blueprint $table) {
+            $table->dropColumn('external_auth_id');
+        });
+    }
+}

diff --git a/resources/lang/en/settings.php b/resources/lang/en/settings.php
index 30abbc1b983f84e0350346b55853553bb46d1176..d6fbb6107c51454965252c8c605ab00dc81dac3a 100755 (executable)
--- a/resources/lang/en/settings.php
+++ b/resources/lang/en/settings.php
@@ -82,6 +82,7 @@ return [
     'role_details' => 'Role Details',
     'role_name' => 'Role Name',
     'role_desc' => 'Short Description of Role',
     'role_details' => 'Role Details',
     'role_name' => 'Role Name',
     'role_desc' => 'Short Description of Role',

+    'role_external_auth_id' => 'External Authentication IDs',

     'role_system' => 'System Permissions',
     'role_manage_users' => 'Manage users',
     'role_manage_roles' => 'Manage roles & role permissions',
     'role_system' => 'System Permissions',
     'role_manage_users' => 'Manage users',
     'role_manage_roles' => 'Manage roles & role permissions',

diff --git a/resources/views/settings/roles/form.blade.php b/resources/views/settings/roles/form.blade.php
index 15ce7e6d7d715e225884b705e2698189d82635e8..6a8e27487c7d307508f38314cc69269dcaaeb951 100644 (file)
--- a/resources/views/settings/roles/form.blade.php
+++ b/resources/views/settings/roles/form.blade.php
@@ -15,6 +15,14 @@
                         <label for="name">{{ trans('settings.role_desc') }}</label>
                         @include('form/text', ['name' => 'description'])
                     </div>
                         <label for="name">{{ trans('settings.role_desc') }}</label>
                         @include('form/text', ['name' => 'description'])
                     </div>

+
+                    @if(config('auth.method') === 'ldap')
+                        <div class="form-group">
+                            <label for="name">{{ trans('settings.role_external_auth_id') }}</label>
+                            @include('form/text', ['name' => 'external_auth_id'])
+                        </div>
+                    @endif
+

                     <h5>{{ trans('settings.role_system') }}</h5>
                     <label>@include('settings/roles/checkbox', ['permission' => 'users-manage']) {{ trans('settings.role_manage_users') }}</label>
                     <label>@include('settings/roles/checkbox', ['permission' => 'user-roles-manage']) {{ trans('settings.role_manage_roles') }}</label>
                     <h5>{{ trans('settings.role_system') }}</h5>
                     <label>@include('settings/roles/checkbox', ['permission' => 'users-manage']) {{ trans('settings.role_manage_users') }}</label>
                     <label>@include('settings/roles/checkbox', ['permission' => 'user-roles-manage']) {{ trans('settings.role_manage_roles') }}</label>

diff --git a/tests/Auth/LdapTest.php b/tests/Auth/LdapTest.php
index ada33d692cd9d5544fd436fb8fedf1af64f457dd..64f3fd74265142e11038b79e9ce4fd2b2ccef208 100644 (file)
--- a/tests/Auth/LdapTest.php
+++ b/tests/Auth/LdapTest.php
@@ -148,8 +148,8 @@ class LdapTest extends BrowserKitTest
 
     public function test_login_maps_roles_and_retains_existsing_roles()
     {
 
     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']);
+        $roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']);
+        $roleToReceive2 = factory(Role::class)->create(['name' => 'ldaptester-second', 'display_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);
         $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
         $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
         $this->mockUser->attachRole($existingRole);


@@ -187,11 +187,11 @@ class LdapTest extends BrowserKitTest
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,

-            'role_id' => $roleToRecieve->id
+            'role_id' => $roleToReceive->id

         ]);
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,
         ]);
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,

-            'role_id' => $roleToRecieve2->id
+            'role_id' => $roleToReceive2->id

         ]);
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,
         ]);
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,


@@ -201,7 +201,7 @@ class LdapTest extends BrowserKitTest
 
     public function test_login_maps_roles_and_removes_old_roles_if_set()
     {
 
     public function test_login_maps_roles_and_removes_old_roles_if_set()
     {

-        $roleToRecieve = factory(Role::class)->create(['name' => 'ldaptester']);
+        $roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'display_name' => 'LdapTester']);

         $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
         $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
         $this->mockUser->attachRole($existingRole);
         $existingRole = factory(Role::class)->create(['name' => 'ldaptester-existing']);
         $this->mockUser->forceFill(['external_auth_id' => $this->mockUser->name])->save();
         $this->mockUser->attachRole($existingRole);


@@ -238,7 +238,7 @@ class LdapTest extends BrowserKitTest
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,
         $user = User::where('email', $this->mockUser->email)->first();
         $this->seeInDatabase('role_user', [
             'user_id' => $user->id,

-            'role_id' => $roleToRecieve->id
+            'role_id' => $roleToReceive->id

         ]);
         $this->dontSeeInDatabase('role_user', [
             'user_id' => $user->id,
         ]);
         $this->dontSeeInDatabase('role_user', [
             'user_id' => $user->id,


@@ -246,4 +246,56 @@ class LdapTest extends BrowserKitTest
         ]);
     }
 
         ]);
     }
 

+    public function test_external_auth_id_visible_in_roles_page_when_ldap_active()
+    {
+        $role = factory(Role::class)->create(['name' => 'ldaptester', 'external_auth_id' => 'ex-auth-a, test-second-param']);
+        $this->asAdmin()->visit('/settings/roles/' . $role->id)
+            ->see('ex-auth-a');
+    }
+
+    public function test_login_maps_roles_using_external_auth_ids_if_set()
+    {
+        $roleToReceive = factory(Role::class)->create(['name' => 'ldaptester', 'external_auth_id' => 'test-second-param, ex-auth-a']);
+        $roleToNotReceive = factory(Role::class)->create(['name' => 'ldaptester-not-receive', 'display_name' => 'ex-auth-a', 'external_auth_id' => 'test-second-param']);
+
+        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=ex-auth-a,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' => $roleToReceive->id
+        ]);
+        $this->dontSeeInDatabase('role_user', [
+            'user_id' => $user->id,
+            'role_id' => $roleToNotReceive->id
+        ]);
+    }
+

 }
 }