Skip to content

Commit b64c9b3

Browse files
committed
OIDC: Added testing coverage for picture fetching
1 parent f9dbbe5 commit b64c9b3

File tree

6 files changed

+64
-1
lines changed

6 files changed

+64
-1
lines changed

app/Access/Oidc/OidcService.php

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -222,6 +222,8 @@ protected function processAccessTokenCallback(OidcAccessToken $accessToken, Oidc
222222
throw new OidcException($exception->getMessage());
223223
}
224224

225+
// TODO - Update this (and tests and config option comments) to actually align with LDAP system
226+
// which syncs whenever on login or registration, where there's no existing avatar.
225227
if ($this->config()['fetch_avatar'] && $user->wasRecentlyCreated && $userDetails->picture) {
226228
$this->userAvatars->assignToUserFromUrl($user, $userDetails->picture);
227229
}

app/Uploads/UserAvatars.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ public function assignToUserFromUrl(User $user, string $avatarUrl): void
6565

6666
$mime = (new WebSafeMimeSniffer())->sniff($imageData);
6767
[$format, $type] = explode('/', $mime, 2);
68-
if ($format !== 'image' || ImageService::isExtensionSupported($type)) {
68+
if ($format !== 'image' || !ImageService::isExtensionSupported($type)) {
6969
return;
7070
}
7171

app/Users/Models/User.php

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@
4545
* @property string $system_name
4646
* @property Collection $roles
4747
* @property Collection $mfaValues
48+
* @property ?Image $avatar
4849
*/
4950
class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable
5051
{

tests/Auth/OidcTest.php

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@ protected function setUp(): void
4141
'oidc.discover' => false,
4242
'oidc.dump_user_details' => false,
4343
'oidc.additional_scopes' => '',
44+
'odic.fetch_avatar' => false,
4445
'oidc.user_to_groups' => false,
4546
'oidc.groups_claim' => 'group',
4647
'oidc.remove_from_groups' => false,
@@ -457,6 +458,57 @@ public function test_auth_uses_mulitple_display_name_claims_if_configured()
457458
]);
458459
}
459460

461+
public function test_user_avatar_fetched_from_picture_on_first_login_if_enabled()
462+
{
463+
config()->set(['oidc.fetch_avatar' => true]);
464+
465+
$this->runLogin([
466+
'email' => 'avatar@example.com',
467+
'picture' => 'https://example.com/my-avatar.jpg',
468+
], [
469+
new Response(200, ['Content-Type' => 'image/jpeg'], $this->files->jpegImageData())
470+
]);
471+
472+
$user = User::query()->where('email', '=', 'avatar@example.com')->first();
473+
$this->assertNotNull($user);
474+
475+
$this->assertTrue($user->avatar()->exists());
476+
}
477+
478+
public function test_user_avatar_not_fetched_if_image_data_format_unknown()
479+
{
480+
config()->set(['oidc.fetch_avatar' => true]);
481+
482+
$this->runLogin([
483+
'email' => 'avatar-format@example.com',
484+
'picture' => 'https://example.com/my-avatar.jpg',
485+
], [
486+
new Response(200, ['Content-Type' => 'image/jpeg'], str_repeat('abc123', 5))
487+
]);
488+
489+
$user = User::query()->where('email', '=', 'avatar-format@example.com')->first();
490+
$this->assertNotNull($user);
491+
492+
$this->assertFalse($user->avatar()->exists());
493+
}
494+
495+
public function test_user_avatar_not_fetched_when_user_already_exists()
496+
{
497+
config()->set(['oidc.fetch_avatar' => true]);
498+
$editor = $this->users->editor();
499+
$editor->external_auth_id = 'benny509';
500+
501+
$this->runLogin([
502+
'picture' => 'https://example.com/my-avatar.jpg',
503+
'sub' => 'benny509',
504+
], [
505+
new Response(200, ['Content-Type' => 'image/jpeg'], $this->files->jpegImageData())
506+
]);
507+
508+
$editor->refresh();
509+
$this->assertFalse($editor->avatar()->exists());
510+
}
511+
460512
public function test_login_group_sync()
461513
{
462514
config()->set([

tests/Helpers/FileProvider.php

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,14 @@ public function pngImageData(): string
6060
return file_get_contents($this->testFilePath('test-image.png'));
6161
}
6262

63+
/**
64+
* Get raw data for a Jpeg image test file.
65+
*/
66+
public function jpegImageData(): string
67+
{
68+
return file_get_contents($this->testFilePath('test-image.jpg'));
69+
}
70+
6371
/**
6472
* Get the expected relative path for an uploaded image of the given type and filename.
6573
*/

tests/test-data/test-image.jpg

268 Bytes
Loading

0 commit comments

Comments
 (0)