colin
/
bookstack
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

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.
This commit is contained in:
Dan Brown 2025-05-24 17:56:21 +01:00
parent 9d6bc1ad4d
commit eb47e11916
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
3 changed files with 37 additions and 4 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

4
app/Config/oidc.php
View File

@ -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

13
app/Uploads/UserAvatars.php
View File

@ -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);
$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]));

22
tests/Auth/OidcTest.php
View File

@ -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([