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.

Author: ssddanbrown

app/Config/oidc.php

@@ -49,8 +49,8 @@
 
     // 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

app/Uploads/UserAvatars.php

@@ -74,7 +74,7 @@ public function assignToUserFromUrl(User $user, string $avatarUrl): void
             $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 @@ protected function getAvatarImageData(string $url): string
     {
         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]));

tests/Auth/OidcTest.php

@@ -536,6 +536,28 @@ public function test_user_avatar_not_fetched_when_avatar_already_assigned()
         $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' => '[email protected]',
+            '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', '=', '[email protected]')->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([