OIDC: Added testing coverage for picture fetching

diff --git a/app/Access/Oidc/OidcService.php b/app/Access/Oidc/OidcService.php
index 452fda3d85fca1a9e677f5ab26294863ec80517e..38eecdff35c8b4f4df3a17d97e5b310173bea2ab 100644 (file)
--- a/app/Access/Oidc/OidcService.php
+++ b/app/Access/Oidc/OidcService.php
@@ -222,6 +222,8 @@ 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) {
             $this->userAvatars->assignToUserFromUrl($user, $userDetails->picture);
         }

diff --git a/app/Uploads/UserAvatars.php b/app/Uploads/UserAvatars.php
index 1763dcbbd5840fe1a01f9d8b6d6e2bb014030979..2c9f7a8482eecb1cd1342df0d3b53f50f28cc578 100644 (file)
--- a/app/Uploads/UserAvatars.php
+++ b/app/Uploads/UserAvatars.php
@@ -65,7 +65,7 @@ class UserAvatars
 
             $mime = (new WebSafeMimeSniffer())->sniff($imageData);
             [$format, $type] = explode('/', $mime, 2);
-            if ($format !== 'image' || ImageService::isExtensionSupported($type)) {
+            if ($format !== 'image' || !ImageService::isExtensionSupported($type)) {
                 return;
             }
 

diff --git a/app/Users/Models/User.php b/app/Users/Models/User.php
index 3797e7cb0279d27cd88902443d9aafd393857178..0d437418bb9f29a6643670d04bc4bef6d56776ef 100644 (file)
--- a/app/Users/Models/User.php
+++ b/app/Users/Models/User.php
@@ -45,6 +45,7 @@ use Illuminate\Support\Collection;
  * @property string     $system_name
  * @property Collection $roles
  * @property Collection $mfaValues
+ * @property ?Image     $avatar
  */
 class User extends Model implements AuthenticatableContract, CanResetPasswordContract, Loggable, Sluggable
 {

diff --git a/tests/Auth/OidcTest.php b/tests/Auth/OidcTest.php
index 205f75a4d62c159c2fefde68712567981c73c693..f4d044bf14dcb243c088854d7e42b92e72ba8b8c 100644 (file)
--- a/tests/Auth/OidcTest.php
+++ b/tests/Auth/OidcTest.php
@@ -41,6 +41,7 @@ class OidcTest extends TestCase
             'oidc.discover'               => false,
             'oidc.dump_user_details'      => false,
             'oidc.additional_scopes'      => '',
+            'odic.fetch_avatar'           => false,
             'oidc.user_to_groups'         => false,
             'oidc.groups_claim'           => 'group',
             'oidc.remove_from_groups'     => false,
@@ -457,6 +458,57 @@ class OidcTest extends TestCase
         ]);
     }
 
+    public function test_user_avatar_fetched_from_picture_on_first_login_if_enabled()
+    {
+        config()->set(['oidc.fetch_avatar' => true]);
+
+        $this->runLogin([
+            'email' => '[email protected]',
+            'picture' => 'https://example.com/my-avatar.jpg',
+        ], [
+            new Response(200, ['Content-Type' => 'image/jpeg'], $this->files->jpegImageData())
+        ]);
+
+        $user = User::query()->where('email', '=', '[email protected]')->first();
+        $this->assertNotNull($user);
+
+        $this->assertTrue($user->avatar()->exists());
+    }
+
+    public function test_user_avatar_not_fetched_if_image_data_format_unknown()
+    {
+        config()->set(['oidc.fetch_avatar' => true]);
+
+        $this->runLogin([
+            'email' => '[email protected]',
+            'picture' => 'https://example.com/my-avatar.jpg',
+        ], [
+            new Response(200, ['Content-Type' => 'image/jpeg'], str_repeat('abc123', 5))
+        ]);
+
+        $user = User::query()->where('email', '=', '[email protected]')->first();
+        $this->assertNotNull($user);
+
+        $this->assertFalse($user->avatar()->exists());
+    }
+
+    public function test_user_avatar_not_fetched_when_user_already_exists()
+    {
+        config()->set(['oidc.fetch_avatar' => true]);
+        $editor = $this->users->editor();
+        $editor->external_auth_id = 'benny509';
+
+        $this->runLogin([
+            'picture' => 'https://example.com/my-avatar.jpg',
+            'sub' => 'benny509',
+        ], [
+            new Response(200, ['Content-Type' => 'image/jpeg'], $this->files->jpegImageData())
+        ]);
+
+        $editor->refresh();
+        $this->assertFalse($editor->avatar()->exists());
+    }
+
     public function test_login_group_sync()
     {
         config()->set([

diff --git a/tests/Helpers/FileProvider.php b/tests/Helpers/FileProvider.php
index 442e036ff245e4dda122b4d7a4bbbaf033293fd2..a455e0fb4ea503641a91d5c018d147db80c628d5 100644 (file)
--- a/tests/Helpers/FileProvider.php
+++ b/tests/Helpers/FileProvider.php
@@ -60,6 +60,14 @@ class FileProvider
         return file_get_contents($this->testFilePath('test-image.png'));
     }
 
+    /**
+     * Get raw data for a Jpeg image test file.
+     */
+    public function jpegImageData(): string
+    {
+        return file_get_contents($this->testFilePath('test-image.jpg'));
+    }
+
     /**
      * Get the expected relative path for an uploaded image of the given type and filename.
      */

diff --git a/tests/test-data/test-image.jpg b/tests/test-data/test-image.jpg
new file mode 100644 (file)
index 0000000..9bb74ff
Binary files /dev/null and b/tests/test-data/test-image.jpg differ