Added ability to escape role "External Auth ID" commas

- Using a backslash in this field before a comma.
- Could potentially (Although unlikely) be a breaking change.

For #3405
This commit is contained in:
Dan Brown 2022-05-04 21:03:13 +01:00
parent d2ed98d20d
commit d795af04df
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
3 changed files with 74 additions and 4 deletions

View File

@ -28,10 +28,8 @@ class GroupSyncService
*/
protected function externalIdMatchesGroupNames(string $externalId, array $groupNames): bool
{
$externalAuthIds = explode(',', strtolower($externalId));
foreach ($externalAuthIds as $externalAuthId) {
if (in_array(trim($externalAuthId), $groupNames)) {
foreach ($this->parseRoleExternalAuthId($externalId) as $externalAuthId) {
if (in_array($externalAuthId, $groupNames)) {
return true;
}
}
@ -39,6 +37,18 @@ class GroupSyncService
return false;
}
protected function parseRoleExternalAuthId(string $externalId): array
{
$inputIds = preg_split('/(?<!\\\),/', $externalId);
$cleanIds = [];
foreach ($inputIds as $inputId) {
$cleanIds[] = str_replace('\,', ',', trim($inputId));
}
return $cleanIds;
}
/**
* Match an array of group names to BookStack system roles.
* Formats group names to be lower-case and hyphenated.

View File

@ -23,6 +23,7 @@ class RoleFactory extends Factory
return [
'display_name' => $this->faker->sentence(3),
'description' => $this->faker->sentence(10),
'external_auth_id' => '',
];
}
}

View File

@ -0,0 +1,59 @@
<?php
namespace Tests\Auth;
use BookStack\Auth\Access\GroupSyncService;
use BookStack\Auth\Role;
use BookStack\Auth\User;
use Tests\TestCase;
class GroupSyncServiceTest extends TestCase
{
public function test_user_is_assigned_to_matching_roles()
{
$user = $this->getViewer();
$roleA = Role::factory()->create(['display_name' => 'Wizards']);
$roleB = Role::factory()->create(['display_name' => 'Gremlins']);
$roleC = Role::factory()->create(['display_name' => 'ABC123', 'external_auth_id' => 'sales']);
$roleD = Role::factory()->create(['display_name' => 'DEF456', 'external_auth_id' => 'admin-team']);
foreach([$roleA, $roleB, $roleC, $roleD] as $role) {
$this->assertFalse($user->hasRole($role->id));
}
(new GroupSyncService())->syncUserWithFoundGroups($user, ['Wizards', 'Gremlinz', 'Sales', 'Admin Team'], false);
$user = User::query()->find($user->id);
$this->assertTrue($user->hasRole($roleA->id));
$this->assertFalse($user->hasRole($roleB->id));
$this->assertTrue($user->hasRole($roleC->id));
$this->assertTrue($user->hasRole($roleD->id));
}
public function test_multiple_values_in_role_external_auth_id_handled()
{
$user = $this->getViewer();
$role = Role::factory()->create(['display_name' => 'ABC123', 'external_auth_id' => 'sales, engineering, developers, marketers']);
$this->assertFalse($user->hasRole($role->id));
(new GroupSyncService())->syncUserWithFoundGroups($user, ['Developers'], false);
$user = User::query()->find($user->id);
$this->assertTrue($user->hasRole($role->id));
}
public function test_commas_can_be_used_in_external_auth_id_if_escaped()
{
$user = $this->getViewer();
$role = Role::factory()->create(['display_name' => 'ABC123', 'external_auth_id' => 'sales\,-developers, marketers']);
$this->assertFalse($user->hasRole($role->id));
(new GroupSyncService())->syncUserWithFoundGroups($user, ['Sales, Developers'], false);
$user = User::query()->find($user->id);
$this->assertTrue($user->hasRole($role->id));
}
}