From eb47e1191665e75adcbc63876470259685b459e8 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Sat, 24 May 2025 17:56:21 +0100
Subject: [PATCH] Avatars: Added redirect handling image fetching

Up to 3 times.
Can be needed based upon testing with Auth0.
Should be fine as long as it's something clearly documented.
Added test to cover.
---
 app/Config/oidc.php         |  4 ++--
 app/Uploads/UserAvatars.php | 15 +++++++++++++--
 tests/Auth/OidcTest.php     | 22 ++++++++++++++++++++++
 3 files changed, 37 insertions(+), 4 deletions(-)

diff --git a/app/Config/oidc.php b/app/Config/oidc.php
index 1d3193eee..16bec873c 100644
--- a/app/Config/oidc.php
+++ b/app/Config/oidc.php
@@ -49,8 +49,8 @@ return [
 
     // 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.
+    // This can be a security risk due to performing server-side fetching (with up to 3 redirects) 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),
 
     // Group sync options
diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php
index 2c9f7a848..0cc640f22 100644
--- a/app/Uploads/UserAvatars.php
+++ b/app/Uploads/UserAvatars.php
@@ -74,7 +74,7 @@ class UserAvatars
             $user->save();
         } catch (Exception $e) {
             Log::error('Failed to save user avatar image from URL', [
-                'exception' => $e,
+                'exception' => $e->getMessage(),
                 'url'       => $avatarUrl,
                 'user_id'   => $user->id,
             ]);
@@ -141,7 +141,18 @@ class UserAvatars
     {
         try {
             $client = $this->http->buildClient(5);
-            $response = $client->sendRequest(new Request('GET', $url));
+            $responseCount = 0;
+
+            do {
+                $response = $client->sendRequest(new Request('GET', $url));
+                $responseCount++;
+                $isRedirect = ($response->getStatusCode() === 301 || $response->getStatusCode() === 302);
+                $url = $response->getHeader('Location')[0] ?? '';
+            } while ($responseCount < 3 && $isRedirect && is_string($url) && str_starts_with($url, 'http'));
+
+            if ($responseCount === 3) {
+                throw new HttpFetchException("Failed to fetch image, max redirect limit of 3 tries reached. Last fetched URL: {$url}");
+            }
 
             if ($response->getStatusCode() !== 200) {
                 throw new HttpFetchException(trans('errors.cannot_get_image_from_url', ['url' => $url]));
diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 71f883ca6..a0db1c2ba 100644
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -536,6 +536,28 @@ class OidcTest extends TestCase
         $this->assertEquals($originalImageData, $newAvatarData);
     }
 
+    public function test_user_avatar_fetch_follows_up_to_three_redirects()
+    {
+        config()->set(['oidc.fetch_avatar' => true]);
+
+        $logger = $this->withTestLogger();
+
+        $this->runLogin([
+            'email' => 'avatar@example.com',
+            'picture' => 'https://example.com/my-avatar.jpg',
+        ], [
+            new Response(302, ['Location' => 'https://example.com/a']),
+            new Response(302, ['Location' => 'https://example.com/b']),
+            new Response(302, ['Location' => 'https://example.com/c']),
+            new Response(302, ['Location' => 'https://example.com/d']),
+        ]);
+
+        $user = User::query()->where('email', '=', 'avatar@example.com')->first();
+        $this->assertFalse($user->avatar()->exists());
+
+        $this->assertStringContainsString('"Failed to fetch image, max redirect limit of 3 tries reached. Last fetched URL: https://example.com/c"', $logger->getRecords()[0]->formatted);
+    }
+
     public function test_login_group_sync()
     {
         config()->set([