Move icons registration logic from TwoFAccount to a dedicated service

app/Models/TwoFAccount.php

@@ -11,7 +11,7 @@
 use App\Helpers\Helpers;
 use App\Models\Dto\HotpDto;
 use App\Models\Dto\TotpDto;
-use App\Services\LogoService;
+use App\Services\IconService;
 use Database\Factories\TwoFAccountFactory;
 use Exception;
 use Illuminate\Database\Eloquent\Factories\HasFactory;
@@ -20,11 +20,7 @@
 use Illuminate\Support\Facades\App;
 use Illuminate\Support\Facades\Auth;
 use Illuminate\Support\Facades\Crypt;
-use Illuminate\Support\Facades\Http;
 use Illuminate\Support\Facades\Log;
-use Illuminate\Support\Facades\Storage;
-use Illuminate\Support\Facades\Validator;
-use Illuminate\Support\Str;
 use Illuminate\Validation\ValidationException;
 use OTPHP\Factory;
 use OTPHP\HOTP;
@@ -127,17 +123,7 @@ class TwoFAccount extends Model implements Sortable
      *
      * @var array<int, string>
      */
-    protected $fillable = [
+    protected $fillable = [];
-        // 'service',
-        // 'account',
-        // 'otp_type',
-        // 'digits',
-        // 'secret',
-        // 'algorithm',
-        // 'counter',
-        // 'period',
-        // 'icon'
-    ];
 
     /**
      * The table associated with the model.
@@ -154,7 +140,7 @@ class TwoFAccount extends Model implements Sortable
     public $appends = [];
 
     /**
-     * The model's default values for attributes.
+     * The model's attributes.
      *
      * @var array
      */
@@ -480,8 +466,8 @@ public function fillWithOtpParameters(array $parameters, bool $skipIconFetching
             $this->enforceAsSteam();
         }
 
-        if (! $this->icon && ! $skipIconFetching) {
+        if (! $this->icon && ! $skipIconFetching && Auth::user()?->preferences['getOfficialIcons']) {
-            $this->icon = $this->getDefaultIcon();
+            $this->icon = App::make(IconService::class)->buildFromOfficialLogo($this->service);
         }
 
         Log::info(sprintf('TwoFAccount filled with OTP parameters'));
@@ -548,11 +534,11 @@ public function fillWithURI(string $uri, bool $isSteamTotp = false, bool $skipIc
             $this->enforceAsSteam();
         }
         if ($this->generator->hasParameter('image')) {
-            self::setIcon($this->generator->getParameter('image'));
+            $this->icon = App::make(IconService::class)->buildFromRemoteImage($this->generator->getParameter('image'));
         }
 
-        if (! $this->icon && ! $skipIconFetching) {
+        if (! $this->icon && ! $skipIconFetching && Auth::user()?->preferences['getOfficialIcons']) {
-            $this->icon = $this->getDefaultIcon();
+            $this->icon = App::make(IconService::class)->buildFromOfficialLogo($this->service);
         }
 
         Log::info(sprintf('TwoFAccount filled with an URI'));
@@ -660,140 +646,6 @@ private function initGenerator() : void
         }
     }
 
-    /**
-     * Store and set the provided icon
-     *
-     * @param  \Psr\Http\Message\StreamInterface|\Illuminate\Http\File|\Illuminate\Http\UploadedFile|string|resource  $data
-     * @param  string|null  $extension  The resource extension, without the dot
-     */
-    public function setIcon($data, $extension = null) : void
-    {
-        $isRemoteData = Str::startsWith($data, ['http://', 'https://']) && Validator::make(
-            [$data],
-            ['url']
-        )->passes();
-
-        if ($isRemoteData) {
-            $icon = $this->storeRemoteImageAsIcon($data);
-        } else {
-            $icon = $extension ? $this->storeFileDataAsIcon($data, $extension) : null;
-        }
-
-        $this->icon = $icon ?: $this->icon;
-    }
-
-    /**
-     * Store img data as an icon file.
-     *
-     * @param  \Psr\Http\Message\StreamInterface|\Illuminate\Http\File|\Illuminate\Http\UploadedFile|string|resource  $content
-     * @param  string  $extension  The file extension, without the dot
-     * @return string|null The filename of the stored icon or null if the operation fails
-     */
-    private function storeFileDataAsIcon($content, $extension) : ?string
-    {
-        $filename = self::getUniqueFilename($extension);
-
-        if (Storage::disk('icons')->put($filename, $content)) {
-            if (self::isValidIcon($filename, 'icons')) {
-                Log::info(sprintf('Image "%s" successfully stored for import', $filename));
-
-                return $filename;
-            } else {
-                Storage::disk('icons')->delete($filename);
-            }
-        }
-
-        return null;
-    }
-
-    /**
-     * Generate a unique filename
-     *
-     * @return string The filename
-     */
-    private function getUniqueFilename(string $extension) : string
-    {
-        return Str::random(40) . '.' . $extension;
-    }
-
-    /**
-     * Validate a file is a valid image
-     *
-     * @param  string  $filename
-     * @param  string  $disk
-     */
-    private function isValidIcon($filename, $disk) : bool
-    {
-        return in_array(Storage::disk($disk)->mimeType($filename), [
-            'image/png',
-            'image/jpeg',
-            'image/webp',
-            'image/bmp',
-            'image/x-ms-bmp',
-            'image/svg+xml',
-        ]) && (Storage::disk($disk)->mimeType($filename) !== 'image/svg+xml' ? getimagesize(Storage::disk($disk)->path($filename)) : true);
-    }
-
-    /**
-     * Gets the image resource pointed by the image url and store it as an icon
-     *
-     * @return string|null The filename of the stored icon or null if the operation fails
-     */
-    private function storeRemoteImageAsIcon(string $url) : ?string
-    {
-        try {
-            $path_parts  = pathinfo($url);
-            $newFilename = self::getUniqueFilename($path_parts['extension']);
-
-            try {
-                $response = Http::withOptions([
-                    'proxy' => config('2fauth.config.outgoingProxy'),
-                ])->retry(3, 100)->get($url);
-
-                if ($response->successful()) {
-                    Storage::disk('imagesLink')->put($newFilename, $response->body());
-                }
-            } catch (\Exception $exception) {
-                Log::error(sprintf('Cannot fetch imageLink at "%s"', $url));
-            }
-
-            if (self::isValidIcon($newFilename, 'imagesLink')) {
-                // Should be a valid image, we move it to the icons disk
-                if (Storage::disk('icons')->put($newFilename, Storage::disk('imagesLink')->get($newFilename))) {
-                    Storage::disk('imagesLink')->delete($newFilename);
-                }
-
-                Log::info(sprintf('Icon file "%s" stored', $newFilename));
-            } else {
-                Storage::disk('imagesLink')->delete($newFilename);
-                throw new \Exception('Unsupported mimeType or missing image on storage');
-            }
-
-            return Storage::disk('icons')->exists($newFilename) ? $newFilename : null;
-        }
-        // @codeCoverageIgnoreStart
-        catch (\Exception|\Throwable $ex) {
-            Log::error(sprintf('Icon storage failed: %s', $ex->getMessage()));
-
-            return null;
-        }
-        // @codeCoverageIgnoreEnd
-    }
-
-    /**
-     * Triggers logo fetching if necessary
-     *
-     * @return string|null The icon
-     */
-    private function getDefaultIcon()
-    {
-        // $logoService = App::make(LogoService::class);
-
-        return (bool) Auth::user()?->preferences['getOfficialIcons']
-            ? App::make(LogoService::class)->getIcon($this->service)
-            : null;
-    }
-
     /**
      * Returns an acceptable value
      */

app/Providers/TwoFAuthServiceProvider.php

@@ -3,6 +3,7 @@
 namespace App\Providers;
 
 use App\Factories\MigratorFactoryInterface;
+use App\Services\IconService;
 use App\Services\LogoService;
 use App\Services\ReleaseRadarService;
 use App\Services\SettingService;
@@ -32,6 +33,10 @@ public function register()
             return new LogoService;
         });
 
+        $this->app->singleton(IconService::class, function () {
+            return new IconService;
+        });
+
         $this->app->singleton(ReleaseRadarService::class, function () {
             return new ReleaseRadarService;
         });
@@ -61,6 +66,7 @@ public function boot()
     public function provides()
     {
         return [
+            IconService::class,
             LogoService::class,
             ReleaseRadarService::class,
         ];

app/Services/IconService.php

@@ -0,0 +1,137 @@
+<?php
+
+namespace App\Services;
+
+use App\Services\LogoService;
+use Illuminate\Support\Facades\App;
+use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Facades\Log;
+use Illuminate\Support\Facades\Storage;
+use Illuminate\Support\Facades\Validator;
+use Illuminate\Support\Str;
+
+/**
+ * App\Services\IconService
+ */
+class IconService
+{
+    /**
+     * Build an icon by fetching the official logo on the internet
+     */
+    public function buildFromOfficialLogo(?string $service) : ?string
+    {
+        return App::make(LogoService::class)->getIcon($service);
+    }
+
+    /**
+     * Build an icon from an image resource
+     * 
+     * @param  \Psr\Http\Message\StreamInterface|\Illuminate\Http\File|\Illuminate\Http\UploadedFile|string|resource  $resource
+     * @param  string  $extension  The file extension, without the dot
+     */
+    public function buildFromResource($resource, $extension) : ?string
+    {
+        // TODO : controller la valeur de $extension
+        $filename = self::getRandomName($extension);
+
+        if (Storage::disk('icons')->put($filename, $resource)) {
+            if (self::isValidImageFile($filename, 'icons')) {
+                Log::info(sprintf('Image "%s" successfully stored for import', $filename));
+
+                return $filename;
+            } else {
+                Storage::disk('icons')->delete($filename);
+            }
+        }
+
+        return null;
+    }
+
+    /**
+     * Build an icon by fetching an image file on the internet 
+     */
+    public function buildFromRemoteImage(string $url) : ?string
+    {
+        $isRemoteData = Str::startsWith($url, ['http://', 'https://']) && Validator::make(
+            [$url],
+            ['url']
+        )->passes();
+
+        return $isRemoteData ? $this->storeRemoteImage($url) : null;
+    }
+
+    /**
+     * Fetch and store an external image file
+     */
+    protected function storeRemoteImage(string $url) : ?string
+    {
+        try {
+            $path_parts  = pathinfo($url);
+            $filename = $this->getRandomName($path_parts['extension']);
+
+            try {
+                $response = Http::withOptions([
+                    'proxy' => config('2fauth.config.outgoingProxy'),
+                ])->retry(3, 100)->get($url);
+
+                if ($response->successful()) {
+                    Storage::disk('imagesLink')->put($filename, $response->body());
+                }
+            } catch (\Exception $exception) {
+                Log::error(sprintf('Cannot fetch imageLink at "%s"', $url));
+                
+                return null;
+            }
+
+            if (self::isValidImageFile($filename, 'imagesLink')) {
+                // Should be a valid image, we move it to the icons disk
+                if (Storage::disk('icons')->put($filename, Storage::disk('imagesLink')->get($filename))) {
+                    Storage::disk('imagesLink')->delete($filename);
+                }
+
+                Log::info(sprintf('Icon file "%s" stored', $filename));
+            } else {
+                Storage::disk('imagesLink')->delete($filename);
+                throw new \Exception('Unsupported mimeType or missing image on storage');
+            }
+
+            if (Storage::disk('icons')->exists($filename)) {
+                return $filename;
+            }
+        }
+        // @codeCoverageIgnoreStart
+        catch (\Exception|\Throwable $ex) {
+            Log::error(sprintf('Icon storage failed: %s', $ex->getMessage()));
+        }
+        // @codeCoverageIgnoreEnd
+
+        return null;
+    }
+
+    /**
+     * Generate a unique filename
+     *
+     */
+    private static function getRandomName(string $extension) : string
+    {
+        return Str::random(40) . '.' . $extension;
+    }
+
+    /**
+     * Validate a file is a valid image
+     *
+     * @param  string  $filename
+     * @param  string  $disk
+     */
+    public static function isValidImageFile($filename, $disk) : bool
+    {
+        return in_array(Storage::disk($disk)->mimeType($filename), [
+            'image/png',
+            'image/jpeg',
+            'image/webp',
+            'image/bmp',
+            'image/x-ms-bmp',
+            'image/svg+xml',
+        ]) && (Storage::disk($disk)->mimeType($filename) !== 'image/svg+xml' ? getimagesize(Storage::disk($disk)->path($filename)) : true);
+    }
+}

app/Services/LogoService.php

@@ -33,10 +33,10 @@ public function __construct()
     /**
      * Fetch a logo for the given service and save it as an icon
      *
-     * @param  string  $serviceName  Name of the service to fetch a logo for
+     * @param  string|null  $serviceName  Name of the service to fetch a logo for
      * @return string|null The icon filename or null if no logo has been found
      */
-    public function getIcon($serviceName)
+    public function getIcon(?string $serviceName)
     {
         $logoFilename = $this->getLogo(strval($serviceName));
 
@@ -55,7 +55,7 @@ public function getIcon($serviceName)
      * @param  string  $serviceName  Name of the service to fetch a logo for
      * @return string|null The logo filename or null if no logo has been found
      */
-    protected function getLogo($serviceName)
+    protected function getLogo(string $serviceName)
     {
         $domain       = $this->tfas->get($this->cleanDomain(strval($serviceName)));
         $logoFilename = $domain . '.svg';

app/Services/Migrators/AegisMigrator.php

@@ -4,9 +4,11 @@
 
 use App\Exceptions\InvalidMigrationDataException;
 use App\Facades\TwoFAccounts;
+use App\Services\IconService;
 use App\Models\TwoFAccount;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\App;
 use Illuminate\Support\Facades\Log;
 
 class AegisMigrator extends Migrator
@@ -37,6 +39,7 @@ class AegisMigrator extends Migrator
      */
     public function migrate(mixed $migrationPayload) : Collection
     {
+        $iconService = App::make(IconService::class);
         $json = json_decode(htmlspecialchars_decode($migrationPayload), true);
 
         if (is_null($json) || Arr::has($json, 'db.entries') == false) {
@@ -88,7 +91,7 @@ public function migrate(mixed $migrationPayload) : Collection
                 $twofaccounts[$key] = new TwoFAccount;
                 $twofaccounts[$key]->fillWithOtpParameters($parameters);
                 if (Arr::has($parameters, 'iconExt') && Arr::has($parameters, 'iconData')) {
-                    $twofaccounts[$key]->setIcon($parameters['iconData'], $parameters['iconExt']);
+                    $twofaccounts[$key]->icon = $iconService->buildFromResource($parameters['iconData'], $parameters['iconExt']);
                 }
             } catch (\Exception $exception) {
                 Log::error(sprintf('Cannot instanciate a TwoFAccount object with OTP parameters from imported item #%s', $key));

app/Services/Migrators/TwoFAuthMigrator.php

@@ -3,9 +3,11 @@
 namespace App\Services\Migrators;
 
 use App\Exceptions\InvalidMigrationDataException;
+use App\Services\IconService;
 use App\Models\TwoFAccount;
 use Illuminate\Support\Arr;
 use Illuminate\Support\Collection;
+use Illuminate\Support\Facades\App;
 use Illuminate\Support\Facades\Log;
 
 class TwoFAuthMigrator extends Migrator
@@ -40,6 +42,7 @@ class TwoFAuthMigrator extends Migrator
      */
     public function migrate(mixed $migrationPayload) : Collection
     {
+        $iconService = App::make(IconService::class);
         $json = json_decode(htmlspecialchars_decode($migrationPayload), true);
 
         if (is_null($json)) {
@@ -105,7 +108,7 @@ public function migrate(mixed $migrationPayload) : Collection
                 $twofaccounts[$key] = new TwoFAccount;
                 $twofaccounts[$key]->fillWithOtpParameters($parameters, Arr::has($parameters, 'iconExt'));
                 if (Arr::has($parameters, 'iconExt')) {
-                    $twofaccounts[$key]->setIcon($parameters['icon_file'], $parameters['iconExt']);
+                    $twofaccounts[$key]->icon = $iconService->buildFromResource($parameters['icon_file'], $parameters['iconExt']);
                 }
             } catch (\Exception $exception) {
                 Log::error(sprintf('Cannot instanciate a TwoFAccount object with 2FAS imported item #%s', $key));

tests/Feature/Models/TwoFAccountModelTest.php

@@ -670,67 +670,6 @@ public function test_equals_returns_false()
         $this->assertFalse($twofaccount->equals($this->customHotpTwofaccount));
     }
 
-    #[Test]
-    #[DataProvider('iconResourceProvider')]
-    public function test_set_icon_stores_and_set_the_icon($res, $ext)
-    {
-        Storage::fake('imagesLink');
-        Storage::fake('icons');
-
-        $previousIcon = $this->customTotpTwofaccount->icon;
-        $this->customTotpTwofaccount->setIcon($res, $ext);
-
-        $this->assertNotEquals($previousIcon, $this->customTotpTwofaccount->icon);
-
-        Storage::disk('icons')->assertExists($this->customTotpTwofaccount->icon);
-        Storage::disk('imagesLink')->assertMissing($this->customTotpTwofaccount->icon);
-    }
-
-    /**
-     * Provide data for Icon store tests
-     */
-    public static function iconResourceProvider()
-    {
-        return [
-            'PNG' => [
-                base64_decode(OtpTestData::ICON_PNG_DATA),
-                'png',
-            ],
-            'JPG' => [
-                base64_decode(OtpTestData::ICON_JPEG_DATA),
-                'jpg',
-            ],
-            'WEBP' => [
-                base64_decode(OtpTestData::ICON_WEBP_DATA),
-                'webp',
-            ],
-            'BMP' => [
-                base64_decode(OtpTestData::ICON_BMP_DATA),
-                'bmp',
-            ],
-            'SVG' => [
-                OtpTestData::ICON_SVG_DATA,
-                'svg',
-            ],
-        ];
-    }
-
-    #[Test]
-    #[DataProvider('invalidIconResourceProvider')]
-    public function test_set_invalid_icon_ends_without_error($res, $ext)
-    {
-        Storage::fake('imagesLink');
-        Storage::fake('icons');
-
-        $previousIcon = $this->customTotpTwofaccount->icon;
-        $this->customTotpTwofaccount->setIcon($res, $ext);
-
-        $this->assertEquals($previousIcon, $this->customTotpTwofaccount->icon);
-
-        Storage::disk('icons')->assertMissing($this->customTotpTwofaccount->icon);
-        Storage::disk('imagesLink')->assertMissing($this->customTotpTwofaccount->icon);
-    }
-
     #[Test]
     public function test_scopeOrphans_retreives_accounts_without_owner()
     {
@@ -743,17 +682,4 @@ public function test_scopeOrphans_retreives_accounts_without_owner()
         $this->assertCount(1, $orphans);
         $this->assertEquals($orphan->id, $orphans[0]->id);
     }
-
-    /**
-     * Provide data for Icon store tests
-     */
-    public static function invalidIconResourceProvider()
-    {
-        return [
-            'INVALID_PNG' => [
-                'lkjdslfkjslkdfjlskdjflksjf',
-                'png',
-            ],
-        ];
-    }
 }

tests/Feature/Services/IconServiceTest.php

@@ -0,0 +1,102 @@
+<?php
+
+namespace Tests\Feature\Services;
+
+use App\Services\IconService;
+use App\Services\LogoService;
+use Illuminate\Foundation\Testing\WithoutMiddleware;
+use Illuminate\Support\Facades\Http;
+use Illuminate\Support\Facades\Storage;
+use PHPUnit\Framework\Attributes\CoversClass;
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Test;
+use Tests\Data\HttpRequestTestData;
+use Tests\TestCase;
+
+/**
+ * IconServiceTest test class
+ */
+#[CoversClass(IconService::class)]
+class IconServiceTest extends TestCase
+{
+    use WithoutMiddleware;
+
+    public function setUp() : void
+    {
+        parent::setUp();
+    }
+
+    // #[Test]
+    // #[DataProvider('iconResourceProvider')]
+    // public function test_set_icon_stores_and_set_the_icon($res, $ext)
+    // {
+    //     Storage::fake('imagesLink');
+    //     Storage::fake('icons');
+
+    //     $previousIcon = $this->customTotpTwofaccount->icon;
+    //     $this->customTotpTwofaccount->setIcon($res, $ext);
+
+    //     $this->assertNotEquals($previousIcon, $this->customTotpTwofaccount->icon);
+
+    //     Storage::disk('icons')->assertExists($this->customTotpTwofaccount->icon);
+    //     Storage::disk('imagesLink')->assertMissing($this->customTotpTwofaccount->icon);
+    // }
+
+    // /**
+    //  * Provide data for Icon store tests
+    //  */
+    // public static function iconResourceProvider()
+    // {
+    //     return [
+    //         'PNG' => [
+    //             base64_decode(OtpTestData::ICON_PNG_DATA),
+    //             'png',
+    //         ],
+    //         'JPG' => [
+    //             base64_decode(OtpTestData::ICON_JPEG_DATA),
+    //             'jpg',
+    //         ],
+    //         'WEBP' => [
+    //             base64_decode(OtpTestData::ICON_WEBP_DATA),
+    //             'webp',
+    //         ],
+    //         'BMP' => [
+    //             base64_decode(OtpTestData::ICON_BMP_DATA),
+    //             'bmp',
+    //         ],
+    //         'SVG' => [
+    //             OtpTestData::ICON_SVG_DATA,
+    //             'svg',
+    //         ],
+    //     ];
+    // }
+
+    // #[Test]
+    // #[DataProvider('invalidIconResourceProvider')]
+    // public function test_set_invalid_icon_ends_without_error($res, $ext)
+    // {
+    //     Storage::fake('imagesLink');
+    //     Storage::fake('icons');
+
+    //     $previousIcon = $this->customTotpTwofaccount->icon;
+    //     $this->customTotpTwofaccount->setIcon($res, $ext);
+
+    //     $this->assertEquals($previousIcon, $this->customTotpTwofaccount->icon);
+
+    //     Storage::disk('icons')->assertMissing($this->customTotpTwofaccount->icon);
+    //     Storage::disk('imagesLink')->assertMissing($this->customTotpTwofaccount->icon);
+    // }
+
+    // /**
+    //  * Provide data for Icon store tests
+    //  */
+    // public static function invalidIconResourceProvider()
+    // {
+    //     return [
+    //         'INVALID_PNG' => [
+    //             'lkjdslfkjslkdfjlskdjflksjf',
+    //             'png',
+    //         ],
+    //     ];
+    // }
+}