From: Dan Brown <redacted>
Date: Sat, 24 May 2025 15:34:36 +0000 (+0100)
Subject: OIDC: Updated avatar fetching to run on each login
X-Git-Tag: v25.05~1^2~6^2~2
X-Git-Url: http://source.bookstackapp.com/bookstack/commitdiff_plain/30bf0ce632663eb508b02c2f534c817660dd5d34

OIDC: Updated avatar fetching to run on each login

But only where the user does not already have an avatar assigned.
This aligns with the LDAP avatar fetching logic.
---

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index 38eecdff3..d6f6ef156 100644
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -222,9 +222,7 @@ class OidcService
             throw new OidcException($exception->getMessage());
         }
 
-        // TODO - Update this (and tests and config option comments) to actually align with LDAP system
-        //  which syncs whenever on login or registration, where there's no existing avatar.
-        if ($this->config()['fetch_avatar'] && $user->wasRecentlyCreated && $userDetails->picture) {
+        if ($this->config()['fetch_avatar'] && !$user->avatar()->exists() && $userDetails->picture) {
             $this->userAvatars->assignToUserFromUrl($user, $userDetails->picture);
         }
 
diff --git a/app/Config/oidc.php b/app/Config/oidc.php
index 49427cb12..1d3193eee 100644
--- a/app/Config/oidc.php
+++ b/app/Config/oidc.php
@@ -47,7 +47,8 @@ return [
     // Multiple values can be provided comma seperated.
     'additional_scopes' => env('OIDC_ADDITIONAL_SCOPES', null),
 
-    // Enable fetching of the user's avatar from the 'picture' claim on initial login.
+    // Enable fetching of the user's avatar from the 'picture' claim on login.
+    // Will only be fetched if the user doesn't already have an avatar image assigned.
     // This can be a security risk due to performing server-side fetching of data from external URLs.
     // Only enable if you trust the OIDC auth provider to provide safe URLs for user images.
     'fetch_avatar' => env('OIDC_FETCH_AVATAR', false),
diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index f4d044bf1..71f883ca6 100644
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -5,6 +5,7 @@ namespace Tests\Auth;
 use BookStack\Activity\ActivityType;
 use BookStack\Facades\Theme;
 use BookStack\Theming\ThemeEvents;
+use BookStack\Uploads\UserAvatars;
 use BookStack\Users\Models\Role;
 use BookStack\Users\Models\User;
 use GuzzleHttp\Psr7\Response;
@@ -475,6 +476,26 @@ class OidcTest extends TestCase
         $this->assertTrue($user->avatar()->exists());
     }
 
+    public function test_user_avatar_fetched_for_existing_user_when_no_avatar_already_assigned()
+    {
+        config()->set(['oidc.fetch_avatar' => true]);
+        $editor = $this->users->editor();
+        $editor->external_auth_id = 'benny509';
+        $editor->save();
+
+        $this->assertFalse($editor->avatar()->exists());
+
+        $this->runLogin([
+            'picture' => 'https://example.com/my-avatar.jpg',
+            'sub' => 'benny509',
+        ], [
+            new Response(200, ['Content-Type' => 'image/jpeg'], $this->files->jpegImageData())
+        ]);
+
+        $editor->refresh();
+        $this->assertTrue($editor->avatar()->exists());
+    }
+
     public function test_user_avatar_not_fetched_if_image_data_format_unknown()
     {
         config()->set(['oidc.fetch_avatar' => true]);
@@ -492,11 +513,16 @@ class OidcTest extends TestCase
         $this->assertFalse($user->avatar()->exists());
     }
 
-    public function test_user_avatar_not_fetched_when_user_already_exists()
+    public function test_user_avatar_not_fetched_when_avatar_already_assigned()
     {
         config()->set(['oidc.fetch_avatar' => true]);
         $editor = $this->users->editor();
         $editor->external_auth_id = 'benny509';
+        $editor->save();
+
+        $avatars = $this->app->make(UserAvatars::class);
+        $originalImageData = $this->files->pngImageData();
+        $avatars->assignToUserFromExistingData($editor, $originalImageData, 'png');
 
         $this->runLogin([
             'picture' => 'https://example.com/my-avatar.jpg',
@@ -506,7 +532,8 @@ class OidcTest extends TestCase
         ]);
 
         $editor->refresh();
-        $this->assertFalse($editor->avatar()->exists());
+        $newAvatarData = file_get_contents($this->files->relativeToFullPath($editor->avatar->path));
+        $this->assertEquals($originalImageData, $newAvatarData);
     }
 
     public function test_login_group_sync()