From bff3bd71821344069048b2c319ae0bfb54f3710c Mon Sep 17 00:00:00 2001 From: Bubka <858858+Bubka@users.noreply.github.com> Date: Wed, 4 Jun 2025 13:58:17 +0200 Subject: [PATCH] Refactor logoService to the laravel manager+driver pattern --- app/Api/v1/Controllers/IconController.php | 8 +- app/Facades/LogoLib.php | 16 +++ app/Providers/TwoFAuthServiceProvider.php | 12 +- app/Services/IconService.php | 4 +- app/Services/LogoLib/AbstractLogoLib.php | 56 +++++++++ app/Services/LogoLib/LogoLibInterface.php | 8 ++ app/Services/LogoLib/LogoLibManager.php | 24 ++++ .../TfaLogoLib.php} | 60 +++------- .../Api/v1/Controllers/IconControllerTest.php | 6 +- .../Controllers/TwoFAccountControllerTest.php | 6 +- tests/Feature/Models/TwoFAccountModelTest.php | 6 +- tests/Feature/Services/IconServiceTest.php | 17 +-- .../Services/LogoLib/TfaLogoLibTest.php | 109 ++++++++++++++++++ tests/Feature/Services/LogoServiceTest.php | 109 ------------------ 14 files changed, 260 insertions(+), 181 deletions(-) create mode 100644 app/Facades/LogoLib.php create mode 100644 app/Services/LogoLib/AbstractLogoLib.php create mode 100644 app/Services/LogoLib/LogoLibInterface.php create mode 100644 app/Services/LogoLib/LogoLibManager.php rename app/Services/{LogoService.php => LogoLib/TfaLogoLib.php} (65%) create mode 100644 tests/Feature/Services/LogoLib/TfaLogoLibTest.php delete mode 100644 tests/Feature/Services/LogoServiceTest.php diff --git a/app/Api/v1/Controllers/IconController.php b/app/Api/v1/Controllers/IconController.php index 219e150a..9574ea82 100644 --- a/app/Api/v1/Controllers/IconController.php +++ b/app/Api/v1/Controllers/IconController.php @@ -4,10 +4,10 @@ namespace App\Api\v1\Controllers; use App\Api\v1\Requests\IconFetchRequest; use App\Facades\IconStore; +use App\Facades\LogoLib; use App\Helpers\Helpers; use App\Http\Controllers\Controller; use App\Models\TwoFAccount; -use App\Services\LogoService; use Exception; use Illuminate\Http\Request; use Illuminate\Http\UploadedFile; @@ -48,12 +48,12 @@ class IconController extends Controller * * @return \Illuminate\Http\JsonResponse */ - public function fetch(IconFetchRequest $request, LogoService $logoService) + public function fetch(IconFetchRequest $request) { $validated = $request->validated(); - $icon = $logoService->getIcon($validated['service']); - + $icon = LogoLib::driver('tfa')->getIcon($validated['service']); + return $icon ? response()->json(['filename' => $icon], 201) : response()->json(null, 204); diff --git a/app/Facades/LogoLib.php b/app/Facades/LogoLib.php new file mode 100644 index 00000000..20359a4a --- /dev/null +++ b/app/Facades/LogoLib.php @@ -0,0 +1,16 @@ +make(Sanitizer::class)); }); - $this->app->singleton(LogoService::class, function ($app) { - return new LogoService; - }); - $this->app->singleton(IconService::class, function ($app) { return new IconService; }); @@ -47,6 +43,10 @@ class TwoFAuthServiceProvider extends ServiceProvider implements DeferrableProvi return new ReleaseRadarService; }); + $this->app->singleton('logolib', function ($app) { + return new LogoLibManager($app); + }); + $this->app->bind(QrReader::class, function ($app, array $parameters) { return new QrReader($parameters['imgSource'], $parameters['sourceType']); }); @@ -74,7 +74,7 @@ class TwoFAuthServiceProvider extends ServiceProvider implements DeferrableProvi return [ IconService::class, IconStoreService::class, - LogoService::class, + LogoLibManager::class, QrReader::class, ReleaseRadarService::class, ]; diff --git a/app/Services/IconService.php b/app/Services/IconService.php index acd7d8e9..fdf3770b 100644 --- a/app/Services/IconService.php +++ b/app/Services/IconService.php @@ -3,8 +3,8 @@ namespace App\Services; use App\Facades\IconStore; +use App\Facades\LogoLib; use App\Helpers\Helpers; -use Illuminate\Support\Facades\App; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Log; use Illuminate\Support\Facades\Storage; @@ -18,7 +18,7 @@ class IconService */ public function buildFromOfficialLogo(?string $service) : ?string { - return App::make(LogoService::class)->getIcon($service); + return LogoLib::driver('tfa')->getIcon($service); } /** diff --git a/app/Services/LogoLib/AbstractLogoLib.php b/app/Services/LogoLib/AbstractLogoLib.php new file mode 100644 index 00000000..91aa332d --- /dev/null +++ b/app/Services/LogoLib/AbstractLogoLib.php @@ -0,0 +1,56 @@ + config('2fauth.config.outgoingProxy'), + ])->retry(3, 100)->get($this->logoUrl($logoFilename)); + + if ($response->successful()) { + Storage::disk('logos')->put($logoFilename, $response->body()) + ? Log::info(sprintf('Logo "%s" saved to logos dir.', $logoFilename)) + : Log::notice(sprintf('Cannot save logo "%s" to logos dir', $logoFilename)); + } + } catch (\Exception $exception) { + Log::error(sprintf('Fetching of logo "%s" failed.', $logoFilename)); + } + } + + /** + * Copy a logo file to the icons store with a new name + */ + protected function copyToIconStore(string $logoFilename, string $iconFilename) : bool + { + if ($content = Storage::disk('logos')->get($logoFilename)) { + return IconStore::store($iconFilename, $content); + } + + return false; + } +} diff --git a/app/Services/LogoLib/LogoLibInterface.php b/app/Services/LogoLib/LogoLibInterface.php new file mode 100644 index 00000000..aa694278 --- /dev/null +++ b/app/Services/LogoLib/LogoLibInterface.php @@ -0,0 +1,8 @@ + @@ -28,8 +29,11 @@ class LogoService /** * @var string */ - const TFA_IMG_URL = 'https://raw.githubusercontent.com/2factorauth/twofactorauth/master/img/'; + const IMG_URL = 'https://raw.githubusercontent.com/2factorauth/twofactorauth/master/img/'; + /** + * + */ public function __construct() { $this->setTfaCollection(); @@ -41,7 +45,7 @@ class LogoService * @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(?string $serviceName) + public function getIcon(?string $serviceName) : string|null { $logoFilename = $this->getLogo(strval($serviceName)); @@ -63,7 +67,7 @@ class LogoService */ protected function getLogo(string $serviceName) { - $domain = $this->tfas->get($this->cleanDomain(strval($serviceName))); + $domain = $this->tfas->get($this->sanitizeServiceName(strval($serviceName))); $logoFilename = $domain . '.svg'; if ($domain && ! Storage::disk('logos')->exists($logoFilename)) { @@ -118,50 +122,18 @@ class LogoService } /** - * Fetch and cache a logo from 2fa.Directory repository - * - * @param string $logoFile Logo filename to fetch + * Url to use in http request to get a specific logo from the logo lib */ - protected function fetchLogo(string $logoFile) : void + protected function logoUrl(string $logoFilename) : string { - try { - $response = Http::withOptions([ - 'proxy' => config('2fauth.config.outgoingProxy'), - ])->retry(3, 100)->get(self::TFA_IMG_URL . $logoFile[0] . '/' . $logoFile); - - if ($response->successful()) { - Storage::disk('logos')->put($logoFile, $response->body()) - ? Log::info(sprintf('Logo "%s" saved to logos dir.', $logoFile)) - : Log::notice(sprintf('Cannot save logo "%s" to logos dir', $logoFile)); - } - } catch (\Exception $exception) { - Log::error(sprintf('Fetching of logo "%s" failed.', $logoFile)); - } + return self::IMG_URL . $logoFilename[0] . '/' . $logoFilename; } /** - * Prepare and make some replacement to optimize logo fetching - * - * @return string Optimized domain name + * Prepare service name to match logo libs naming convention */ - protected function cleanDomain(string $domain) : string + protected function sanitizeServiceName(string $service) : string { - return strtolower(str_replace(['+'], ['plus'], $domain)); - } - - /** - * Copy a logo file to the icons store with a new name - * - * @param string $logoFilename - * @param string $iconFilename - * @return bool Whether the copy succeed or not - */ - protected function copyToIconStore($logoFilename, $iconFilename) : bool - { - if ($content = Storage::disk('logos')->get($logoFilename)) { - return IconStore::store($iconFilename, $content); - } - - return false; + return strtolower(str_replace(['+'], ['plus'], $service)); } } diff --git a/tests/Api/v1/Controllers/IconControllerTest.php b/tests/Api/v1/Controllers/IconControllerTest.php index a50ca3d0..de77b214 100644 --- a/tests/Api/v1/Controllers/IconControllerTest.php +++ b/tests/Api/v1/Controllers/IconControllerTest.php @@ -6,7 +6,7 @@ use App\Api\v1\Controllers\IconController; use App\Facades\IconStore; use App\Models\TwoFAccount; use App\Models\User; -use App\Services\LogoService; +use App\Services\LogoLib\TfaLogoLib; use Illuminate\Http\Testing\FileFactory; use Illuminate\Http\UploadedFile; use Illuminate\Support\Facades\Http; @@ -38,8 +38,8 @@ class IconControllerTest extends FeatureTestCase Http::preventStrayRequests(); Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), + TfaLogoLib::IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfalogoLib::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), ]); Http::fake([ OtpTestData::EXTERNAL_IMAGE_URL_DECODED => Http::response((new FileFactory)->image('file.png', 10, 10)->tempFile, 200), diff --git a/tests/Api/v1/Controllers/TwoFAccountControllerTest.php b/tests/Api/v1/Controllers/TwoFAccountControllerTest.php index b6b4924f..5436fb68 100644 --- a/tests/Api/v1/Controllers/TwoFAccountControllerTest.php +++ b/tests/Api/v1/Controllers/TwoFAccountControllerTest.php @@ -16,7 +16,7 @@ use App\Models\User; use App\Policies\TwoFAccountPolicy; use App\Providers\MigrationServiceProvider; use App\Providers\TwoFAuthServiceProvider; -use App\Services\LogoService; +use App\Services\LogoLib\TfaLogoLib; use Illuminate\Http\Testing\FileFactory; use Illuminate\Support\Facades\DB; use Illuminate\Support\Facades\Http; @@ -242,8 +242,8 @@ class TwoFAccountControllerTest extends FeatureTestCase Http::preventStrayRequests(); Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), + TfaLogoLib::IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfaLogoLib::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), OtpTestData::EXTERNAL_IMAGE_URL_DECODED => Http::response((new FileFactory)->image('file.png', 10, 10)->tempFile, 200), OtpTestData::EXTERNAL_INFECTED_IMAGE_URL_DECODED => Http::response((new FileFactory)->createWithContent('infected.svg', OtpTestData::ICON_SVG_DATA_INFECTED)->tempFile, 200), 'example.com/*' => Http::response(null, 400), diff --git a/tests/Feature/Models/TwoFAccountModelTest.php b/tests/Feature/Models/TwoFAccountModelTest.php index 441c9c73..178c159d 100644 --- a/tests/Feature/Models/TwoFAccountModelTest.php +++ b/tests/Feature/Models/TwoFAccountModelTest.php @@ -5,7 +5,7 @@ namespace Tests\Feature\Models; use App\Facades\Icons; use App\Models\TwoFAccount; use App\Models\User; -use App\Services\LogoService; +use App\Services\LogoLib\TfaLogoLib; use Illuminate\Http\Testing\FileFactory; use Illuminate\Support\Facades\Http; use Illuminate\Support\Facades\Storage; @@ -269,8 +269,8 @@ class TwoFAccountModelTest extends FeatureTestCase $this->user['preferences->getOfficialIcons'] = false; $this->user->save(); - $this->mock(LogoService::class, function (MockInterface $logoService) { - $logoService->shouldNotReceive('getIcon'); + $this->mock(TfaLogoLib::class, function (MockInterface $logoLib) { + $logoLib->shouldNotReceive('getIcon'); }); $twofaccount = new TwoFAccount; diff --git a/tests/Feature/Services/IconServiceTest.php b/tests/Feature/Services/IconServiceTest.php index 4142b7c7..eb4190aa 100644 --- a/tests/Feature/Services/IconServiceTest.php +++ b/tests/Feature/Services/IconServiceTest.php @@ -2,8 +2,9 @@ namespace Tests\Feature\Services; +use App\Facades\LogoLib; use App\Services\IconService; -use App\Services\LogoService; +use App\Services\LogoLib\TfaLogoLib; use Illuminate\Foundation\Testing\WithoutMiddleware; use Illuminate\Http\Testing\FileFactory; use Illuminate\Support\Facades\Http; @@ -38,8 +39,8 @@ class IconServiceTest extends FeatureTestCase Http::preventStrayRequests(); Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), + TfaLogoLib::IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfaLogoLib::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), ]); Http::fake([ OtpTestData::EXTERNAL_IMAGE_URL_DECODED => Http::response((new FileFactory)->image('file.png', 10, 10)->tempFile, 200), @@ -47,14 +48,16 @@ class IconServiceTest extends FeatureTestCase } #[Test] - public function test_buildFromOfficialLogo_calls_logoservice_to_get_the_icon() + public function test_buildFromOfficialLogo_calls_logoLib_to_get_the_icon() { - $logoServiceSpy = $this->spy(LogoService::class); + // LogoLib::spy(); + LogoLib::shouldReceive('driver->getIcon') + ->once() + ->with('fakeService') + ->andReturn('value'); $this->iconService = $this->app->make(IconService::class); $this->iconService->buildFromOfficialLogo('fakeService'); - - $logoServiceSpy->shouldHaveReceived('getIcon')->once()->with('fakeService'); } #[Test] diff --git a/tests/Feature/Services/LogoLib/TfaLogoLibTest.php b/tests/Feature/Services/LogoLib/TfaLogoLibTest.php new file mode 100644 index 00000000..950f81c0 --- /dev/null +++ b/tests/Feature/Services/LogoLib/TfaLogoLibTest.php @@ -0,0 +1,109 @@ + Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfalogoLib::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), + ]); + + $this->tfaLogoLib = $this->app->make(TfalogoLib::class); + $icon = $this->tfaLogoLib->getIcon('service'); + + $this->assertNotNull($icon); + Storage::disk('icons')->assertExists($icon); + } + + #[Test] + public function test_getIcon_returns_null_when_github_request_fails() + { + Http::preventStrayRequests(); + Http::fake([ + TfalogoLib::IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfalogoLib::TFA_URL => Http::response('not found', 404), + ]); + + $this->tfaLogoLib = $this->app->make(TfalogoLib::class); + $icon = $this->tfaLogoLib->getIcon('service'); + + $this->assertEquals(null, $icon); + } + + #[Test] + public function test_getIcon_returns_null_when_logo_fetching_fails() + { + Http::preventStrayRequests(); + Http::fake([ + TfalogoLib::IMG_URL . '*' => Http::response('not found', 404), + TfalogoLib::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), + ]); + + $this->tfaLogoLib = $this->app->make(TfalogoLib::class); + $icon = $this->tfaLogoLib->getIcon('service'); + + $this->assertEquals(null, $icon); + } + + #[Test] + public function test_getIcon_returns_null_when_no_logo_exists() + { + Http::preventStrayRequests(); + Http::fake([ + TfalogoLib::IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfalogoLib::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), + ]); + + $this->tfaLogoLib = $this->app->make(TfalogoLib::class); + $icon = $this->tfaLogoLib->getIcon('no_logo_should_exists_with_this_name'); + + $this->assertEquals(null, $icon); + } + + #[Test] + public function test_TfalogoLib_loads_empty_collection_when_tfajson_fetching_fails() + { + Http::preventStrayRequests(); + Http::fake([ + TfalogoLib::IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), + TfalogoLib::TFA_URL => Http::response('not found', 404), + ]); + + $this->tfaLogoLib = $this->app->make(TfalogoLib::class); + $icon = $this->tfaLogoLib->getIcon('service'); + + $this->assertNull($icon); + Storage::disk('logos')->assertMissing(TfalogoLib::TFA_JSON); + } +} diff --git a/tests/Feature/Services/LogoServiceTest.php b/tests/Feature/Services/LogoServiceTest.php deleted file mode 100644 index 25e08398..00000000 --- a/tests/Feature/Services/LogoServiceTest.php +++ /dev/null @@ -1,109 +0,0 @@ - Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), - ]); - - $this->logoService = $this->app->make(LogoService::class); - $icon = $this->logoService->getIcon('service'); - - $this->assertNotNull($icon); - Storage::disk('icons')->assertExists($icon); - } - - #[Test] - public function test_getIcon_returns_null_when_github_request_fails() - { - Http::preventStrayRequests(); - Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response('not found', 404), - ]); - - $this->logoService = $this->app->make(LogoService::class); - $icon = $this->logoService->getIcon('service'); - - $this->assertEquals(null, $icon); - } - - #[Test] - public function test_getIcon_returns_null_when_logo_fetching_fails() - { - Http::preventStrayRequests(); - Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response('not found', 404), - LogoService::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), - ]); - - $this->logoService = $this->app->make(LogoService::class); - $icon = $this->logoService->getIcon('service'); - - $this->assertEquals(null, $icon); - } - - #[Test] - public function test_getIcon_returns_null_when_no_logo_exists() - { - Http::preventStrayRequests(); - Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response(HttpRequestTestData::TFA_JSON_BODY, 200), - ]); - - $this->logoService = $this->app->make(LogoService::class); - $icon = $this->logoService->getIcon('no_logo_should_exists_with_this_name'); - - $this->assertEquals(null, $icon); - } - - #[Test] - public function test_logoService_loads_empty_collection_when_tfajson_fetching_fails() - { - Http::preventStrayRequests(); - Http::fake([ - LogoService::TFA_IMG_URL . '*' => Http::response(HttpRequestTestData::SVG_LOGO_BODY, 200), - LogoService::TFA_URL => Http::response('not found', 404), - ]); - - $this->logoService = $this->app->make(LogoService::class); - $icon = $this->logoService->getIcon('service'); - - $this->assertNull($icon); - Storage::disk('logos')->assertMissing(LogoService::TFA_JSON); - } -}