From 05f2ec40cc83f33b05a29d16217358a45fc9ba45 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Mon, 11 Sep 2023 11:50:58 +0100
Subject: [PATCH] OIDC: Moved name claim option handling from config to service

Closes #4494
---
 app/Access/Oidc/OidcService.php |  5 +++--
 app/Config/oidc.php             |  2 +-
 tests/Auth/OidcTest.php         | 19 ++++++++++++++++++-
 3 files changed, 22 insertions(+), 4 deletions(-)

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index d22b26eec..8778cbd98 100644
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -142,10 +142,11 @@ class OidcService
      */
     protected function getUserDisplayName(OidcIdToken $token, string $defaultValue): string
     {
-        $displayNameAttr = $this->config()['display_name_claims'];
+        $displayNameAttrString = $this->config()['display_name_claims'] ?? '';
+        $displayNameAttrs = explode('|', $displayNameAttrString);
 
         $displayName = [];
-        foreach ($displayNameAttr as $dnAttr) {
+        foreach ($displayNameAttrs as $dnAttr) {
             $dnComponent = $token->getClaim($dnAttr) ?? '';
             if ($dnComponent !== '') {
                 $displayName[] = $dnComponent;
diff --git a/app/Config/oidc.php b/app/Config/oidc.php
index 1f73fb688..b28b8a41a 100644
--- a/app/Config/oidc.php
+++ b/app/Config/oidc.php
@@ -9,7 +9,7 @@ return [
     'dump_user_details' => env('OIDC_DUMP_USER_DETAILS', false),
 
     // Claim, within an OpenId token, to find the user's display name
-    'display_name_claims' => explode('|', env('OIDC_DISPLAY_NAME_CLAIMS', 'name')),
+    'display_name_claims' => env('OIDC_DISPLAY_NAME_CLAIMS', 'name'),
 
     // Claim, within an OpenID token, to use to connect a BookStack user to the OIDC user.
     'external_id_claim' => env('OIDC_EXTERNAL_ID_CLAIM', 'sub'),
diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 367e84816..204a3bb5f 100644
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -30,7 +30,7 @@ class OidcTest extends TestCase
             'auth.method'                 => 'oidc',
             'auth.defaults.guard'         => 'oidc',
             'oidc.name'                   => 'SingleSignOn-Testing',
-            'oidc.display_name_claims'    => ['name'],
+            'oidc.display_name_claims'    => 'name',
             'oidc.client_id'              => OidcJwtHelper::defaultClientId(),
             'oidc.client_secret'          => 'testpass',
             'oidc.jwt_public_key'         => $this->keyFilePath,
@@ -408,6 +408,23 @@ class OidcTest extends TestCase
         $this->assertEquals('xXBennyTheGeezXx', $user->external_auth_id);
     }
 
+    public function test_auth_uses_mulitple_display_name_claims_if_configured()
+    {
+        config()->set(['oidc.display_name_claims' => 'first_name|last_name']);
+
+        $this->runLogin([
+            'email'      => 'benny@example.com',
+            'sub'        => 'benny1010101',
+            'first_name' => 'Benny',
+            'last_name'  => 'Jenkins'
+        ]);
+
+        $this->assertDatabaseHas('users', [
+            'name' => 'Benny Jenkins',
+            'email' => 'benny@example.com',
+        ]);
+    }
+
     public function test_login_group_sync()
     {
         config()->set([