Merge pull request BookStackApp#3616 from BookStackApp/oidc_group_sync

Added OIDC group sync functionality
brexGmbH · Aug 25, 2022 · 401c156 · 401c156
2 parents 760eff3 + b987bea
commit 401c156
Show file tree

Hide file tree

Showing 5 changed files with 170 additions and 4 deletions.
diff --git a/.env.example.complete b/.env.example.complete
@@ -263,7 +263,11 @@ OIDC_ISSUER_DISCOVER=false
 OIDC_PUBLIC_KEY=null
 OIDC_AUTH_ENDPOINT=null
 OIDC_TOKEN_ENDPOINT=null
+OIDC_ADDITIONAL_SCOPES=null
 OIDC_DUMP_USER_DETAILS=false
+OIDC_USER_TO_GROUPS=false
+OIDC_GROUP_ATTRIBUTE=groups
+OIDC_REMOVE_FROM_GROUPS=false
 
 # Disable default third-party services such as Gravatar and Draw.IO
 # Service-specific options will override this option

diff --git a/app/Auth/Access/Oidc/OidcOAuthProvider.php b/app/Auth/Access/Oidc/OidcOAuthProvider.php
@@ -30,6 +30,11 @@ class OidcOAuthProvider extends AbstractProvider
      */
     protected $tokenEndpoint;
 
+    /**
+     * Scopes to use for the OIDC authorization call
+     */
+    protected array $scopes = ['openid', 'profile', 'email'];
+
     /**
      * Returns the base URL for authorizing a client.
      */
@@ -54,6 +59,15 @@ public function getResourceOwnerDetailsUrl(AccessToken $token): string
         return '';
     }
 
+    /**
+     * Add an additional scope to this provider upon the default.
+     */
+    public function addScope(string $scope): void
+    {
+        $this->scopes[] = $scope;
+        $this->scopes = array_unique($this->scopes);
+    }
+
     /**
      * Returns the default scopes used by this provider.
      *
@@ -62,7 +76,7 @@ public function getResourceOwnerDetailsUrl(AccessToken $token): string
      */
     protected function getDefaultScopes(): array
     {
-        return ['openid', 'profile', 'email'];
+        return $this->scopes;
     }
 
     /**

diff --git a/app/Auth/Access/Oidc/OidcService.php b/app/Auth/Access/Oidc/OidcService.php
@@ -2,6 +2,8 @@
 
 namespace BookStack\Auth\Access\Oidc;
 
+use BookStack\Auth\Access\GroupSyncService;
+use Illuminate\Support\Arr;
 use function auth;
 use BookStack\Auth\Access\LoginService;
 use BookStack\Auth\Access\RegistrationService;
@@ -26,15 +28,22 @@ class OidcService
     protected RegistrationService $registrationService;
     protected LoginService $loginService;
     protected HttpClient $httpClient;
+    protected GroupSyncService $groupService;
 
     /**
      * OpenIdService constructor.
      */
-    public function __construct(RegistrationService $registrationService, LoginService $loginService, HttpClient $httpClient)
+    public function __construct(
+        RegistrationService $registrationService,
+        LoginService        $loginService,
+        HttpClient          $httpClient,
+        GroupSyncService    $groupService
+    )
     {
         $this->registrationService = $registrationService;
         $this->loginService = $loginService;
         $this->httpClient = $httpClient;
+        $this->groupService = $groupService;
     }
 
     /**
@@ -117,10 +126,31 @@ protected function getProviderSettings(): OidcProviderSettings
      */
     protected function getProvider(OidcProviderSettings $settings): OidcOAuthProvider
     {
-        return new OidcOAuthProvider($settings->arrayForProvider(), [
+        $provider = new OidcOAuthProvider($settings->arrayForProvider(), [
             'httpClient'     => $this->httpClient,
             'optionProvider' => new HttpBasicAuthOptionProvider(),
         ]);
+
+        foreach ($this->getAdditionalScopes() as $scope) {
+            $provider->addScope($scope);
+        }
+
+        return $provider;
+    }
+
+    /**
+     * Get any user-defined addition/custom scopes to apply to the authentication request.
+     *
+     * @return string[]
+     */
+    protected function getAdditionalScopes(): array
+    {
+        $scopeConfig = $this->config()['additional_scopes'] ?: '';
+
+        $scopeArr = explode(',', $scopeConfig);
+        $scopeArr = array_map(fn(string $scope) => trim($scope), $scopeArr);
+
+        return array_filter($scopeArr);
     }
 
     /**
@@ -145,10 +175,32 @@ protected function getUserDisplayName(OidcIdToken $token, string $defaultValue):
         return implode(' ', $displayName);
     }
 
+    /**
+     * Extract the assigned groups from the id token.
+     *
+     * @return string[]
+     */
+    protected function getUserGroups(OidcIdToken $token): array
+    {
+        $groupsAttr = $this->config()['group_attribute'];
+        if (empty($groupsAttr)) {
+            return [];
+        }
+
+        $groupsList = Arr::get($token->getAllClaims(), $groupsAttr);
+        if (!is_array($groupsList)) {
+            return [];
+        }
+
+        return array_values(array_filter($groupsList, function($val) {
+            return is_string($val);
+        }));
+    }
+
     /**
      * Extract the details of a user from an ID token.
      *
-     * @return array{name: string, email: string, external_id: string}
+     * @return array{name: string, email: string, external_id: string, groups: string[]}
      */
     protected function getUserDetails(OidcIdToken $token): array
     {
@@ -158,6 +210,7 @@ protected function getUserDetails(OidcIdToken $token): array
             'external_id' => $id,
             'email'       => $token->getClaim('email'),
             'name'        => $this->getUserDisplayName($token, $id),
+            'groups'      => $this->getUserGroups($token),
         ];
     }
 
@@ -209,6 +262,12 @@ protected function processAccessTokenCallback(OidcAccessToken $accessToken, Oidc
             throw new OidcException($exception->getMessage());
         }
 
+        if ($this->shouldSyncGroups()) {
+            $groups = $userDetails['groups'];
+            $detachExisting = $this->config()['remove_from_groups'];
+            $this->groupService->syncUserWithFoundGroups($user, $groups, $detachExisting);
+        }
+
         $this->loginService->login($user, 'oidc');
 
         return $user;
@@ -221,4 +280,12 @@ protected function config(): array
     {
         return config('oidc');
     }
+
+    /**
+     * Check if groups should be synced.
+     */
+    protected function shouldSyncGroups(): bool
+    {
+        return $this->config()['user_to_groups'] !== false;
+    }
 }
diff --git a/app/Config/oidc.php b/app/Config/oidc.php
@@ -32,4 +32,16 @@
     // OAuth2 endpoints.
     'authorization_endpoint' => env('OIDC_AUTH_ENDPOINT', null),
     'token_endpoint'         => env('OIDC_TOKEN_ENDPOINT', null),
+
+    // Add extra scopes, upon those required, to the OIDC authentication request
+    // Multiple values can be provided comma seperated.
+    'additional_scopes' => env('OIDC_ADDITIONAL_SCOPES', null),
+
+    // Group sync options
+    // Enable syncing, upon login, of OIDC groups to BookStack roles
+    'user_to_groups' => env('OIDC_USER_TO_GROUPS', false),
+    // Attribute, within a OIDC ID token, to find group names within
+    'group_attribute' => env('OIDC_GROUP_ATTRIBUTE', 'groups'),
+    // When syncing groups, remove any groups that no longer match. Otherwise sync only adds new groups.
+    'remove_from_groups' => env('OIDC_REMOVE_FROM_GROUPS', false),
 ];
diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
@@ -3,6 +3,7 @@
 namespace Tests\Auth;
 
 use BookStack\Actions\ActivityType;
+use BookStack\Auth\Role;
 use BookStack\Auth\User;
 use GuzzleHttp\Psr7\Request;
 use GuzzleHttp\Psr7\Response;
@@ -37,6 +38,10 @@ protected function setUp(): void
             'oidc.token_endpoint'         => 'https://oidc.local/token',
             'oidc.discover'               => false,
             'oidc.dump_user_details'      => false,
+            'oidc.additional_scopes'      => '',
+            'oidc.user_to_groups'         => false,
+            'oidc.group_attribute'        => 'group',
+            'oidc.remove_from_groups'     => false,
         ]);
     }
 
@@ -159,6 +164,17 @@ public function test_login_success_flow()
         $this->assertActivityExists(ActivityType::AUTH_LOGIN, null, "oidc; ({$user->id}) Barry Scott");
     }
 
+    public function test_login_uses_custom_additional_scopes_if_defined()
+    {
+        config()->set([
+            'oidc.additional_scopes' => 'groups, badgers',
+        ]);
+
+        $redirect = $this->post('/oidc/login')->headers->get('location');
+
+        $this->assertStringContainsString('scope=openid%20profile%20email%20groups%20badgers', $redirect);
+    }
+
     public function test_callback_fails_if_no_state_present_or_matching()
     {
         $this->get('/oidc/callback?code=SplxlOBeZQQYbYS6WxSbIA&state=abc124');
@@ -344,6 +360,59 @@ public function test_auth_login_with_autodiscovery_with_keys_that_do_not_have_al
         $this->assertTrue(auth()->check());
     }
 
+    public function test_login_group_sync()
+    {
+        config()->set([
+            'oidc.user_to_groups'     => true,
+            'oidc.group_attribute'    => 'groups',
+            'oidc.remove_from_groups' => false,
+        ]);
+        $roleA = Role::factory()->create(['display_name' => 'Wizards']);
+        $roleB = Role::factory()->create(['display_name' => 'ZooFolks', 'external_auth_id' => 'zookeepers']);
+        $roleC = Role::factory()->create(['display_name' => 'Another Role']);
+
+        $resp = $this->runLogin([
+            'email'  => '[email protected]',
+            'sub'    => 'benny1010101',
+            'groups' => ['Wizards', 'Zookeepers']
+        ]);
+        $resp->assertRedirect('/');
+
+        /** @var User $user */
+        $user = User::query()->where('email', '=', '[email protected]')->first();
+
+        $this->assertTrue($user->hasRole($roleA->id));
+        $this->assertTrue($user->hasRole($roleB->id));
+        $this->assertFalse($user->hasRole($roleC->id));
+    }
+
+    public function test_login_group_sync_with_nested_groups_in_token()
+    {
+        config()->set([
+            'oidc.user_to_groups'     => true,
+            'oidc.group_attribute'    => 'my.custom.groups.attr',
+            'oidc.remove_from_groups' => false,
+        ]);
+        $roleA = Role::factory()->create(['display_name' => 'Wizards']);
+
+        $resp = $this->runLogin([
+            'email'  => '[email protected]',
+            'sub'    => 'benny1010101',
+            'my' => [
+                'custom' => [
+                    'groups' => [
+                        'attr' => ['Wizards']
+                    ]
+                ]
+            ]
+        ]);
+        $resp->assertRedirect('/');
+
+        /** @var User $user */
+        $user = User::query()->where('email', '=', '[email protected]')->first();
+        $this->assertTrue($user->hasRole($roleA->id));
+    }
+
     protected function withAutodiscovery()
     {
         config()->set([