From 51d6a6c6498d3f660943744700b48fba5b35d4a8 Mon Sep 17 00:00:00 2001 From: Bubka <858858+Bubka@users.noreply.github.com> Date: Tue, 1 Oct 2024 15:36:18 +0200 Subject: [PATCH] Move icons registration logic from TwoFAccount to a dedicated service --- app/Models/TwoFAccount.php | 164 +----------------- app/Providers/TwoFAuthServiceProvider.php | 6 + app/Services/IconService.php | 137 +++++++++++++++ app/Services/LogoService.php | 6 +- app/Services/Migrators/AegisMigrator.php | 5 +- app/Services/Migrators/TwoFAuthMigrator.php | 5 +- tests/Feature/Models/TwoFAccountModelTest.php | 74 -------- tests/Feature/Services/IconServiceTest.php | 102 +++++++++++ 8 files changed, 264 insertions(+), 235 deletions(-) create mode 100644 app/Services/IconService.php create mode 100644 tests/Feature/Services/IconServiceTest.php diff --git a/app/Models/TwoFAccount.php b/app/Models/TwoFAccount.php index 74b3590c..5aeff2b6 100644 --- a/app/Models/TwoFAccount.php +++ b/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 */ - protected $fillable = [ - // 'service', - // 'account', - // 'otp_type', - // 'digits', - // 'secret', - // 'algorithm', - // 'counter', - // 'period', - // 'icon' - ]; + protected $fillable = []; /** * 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) { - $this->icon = $this->getDefaultIcon(); + if (! $this->icon && ! $skipIconFetching && Auth::user()?->preferences['getOfficialIcons']) { + $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) { - $this->icon = $this->getDefaultIcon(); + if (! $this->icon && ! $skipIconFetching && Auth::user()?->preferences['getOfficialIcons']) { + $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 */ diff --git a/app/Providers/TwoFAuthServiceProvider.php b/app/Providers/TwoFAuthServiceProvider.php index 6ab40b19..cd9a3124 100644 --- a/app/Providers/TwoFAuthServiceProvider.php +++ b/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, ]; diff --git a/app/Services/IconService.php b/app/Services/IconService.php new file mode 100644 index 00000000..67dd2e0d --- /dev/null +++ b/app/Services/IconService.php @@ -0,0 +1,137 @@ +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); + } +} diff --git a/app/Services/LogoService.php b/app/Services/LogoService.php index 35062d4f..491afc5c 100644 --- a/app/Services/LogoService.php +++ b/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'; diff --git a/app/Services/Migrators/AegisMigrator.php b/app/Services/Migrators/AegisMigrator.php index a2a58504..274f90f1 100644 --- a/app/Services/Migrators/AegisMigrator.php +++ b/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)); diff --git a/app/Services/Migrators/TwoFAuthMigrator.php b/app/Services/Migrators/TwoFAuthMigrator.php index 67b270ce..7aff06d3 100644 --- a/app/Services/Migrators/TwoFAuthMigrator.php +++ b/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)); diff --git a/tests/Feature/Models/TwoFAccountModelTest.php b/tests/Feature/Models/TwoFAccountModelTest.php index f145e157..96db65cf 100644 --- a/tests/Feature/Models/TwoFAccountModelTest.php +++ b/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', - ], - ]; - } } diff --git a/tests/Feature/Services/IconServiceTest.php b/tests/Feature/Services/IconServiceTest.php new file mode 100644 index 00000000..7b5605f3 --- /dev/null +++ b/tests/Feature/Services/IconServiceTest.php @@ -0,0 +1,102 @@ +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', + // ], + // ]; + // } +}