From 7bc103bb48c6c2280af37cb4cf5ebb70dd160ef0 Mon Sep 17 00:00:00 2001 From: Ryan Hoerr Date: Mon, 16 Mar 2026 21:27:44 -0400 Subject: [PATCH 1/4] refactor: replace template override with plugin-based logo injection Eliminate the fragile full-template override of Magento_Backend::page/header.phtml by using a plugin on Header::toHtml() and Header::getViewFileUrl() instead. The core already differentiates login vs. menu logos via layout handles (default.xml vs admin_login.xml setting logo_image_src). This refactor leverages that mechanism: layout XML passes config path and upload directory as block arguments, and the plugin reads config, builds the media URL, and sets logo_image_src before rendering. A companion afterGetViewFileUrl method passes full URLs through without asset repository resolution. Removes 4 files (template, ViewModel, Model, Scope/Config) and adds 3 (plugin class, di.xml, admin_login layout). The module now works with any theme's default logos and is immune to core template changes. Co-Authored-By: Claude Opus 4.6 (1M context) --- Model/AdminLogo.php | 72 ------------------ Plugin/Backend/Block/Page/HeaderPlugin.php | 73 ++++++++++++++++++ Scope/Config.php | 45 ----------- ViewModel/AdminLogo.php | 52 ------------- etc/adminhtml/di.xml | 14 ++++ etc/module.xml | 2 + view/adminhtml/layout/admin_login.xml | 18 +++++ view/adminhtml/layout/default.xml | 7 +- view/adminhtml/templates/page/header.phtml | 88 ---------------------- 9 files changed, 110 insertions(+), 261 deletions(-) delete mode 100644 Model/AdminLogo.php create mode 100644 Plugin/Backend/Block/Page/HeaderPlugin.php delete mode 100644 Scope/Config.php delete mode 100644 ViewModel/AdminLogo.php create mode 100644 etc/adminhtml/di.xml create mode 100644 view/adminhtml/layout/admin_login.xml delete mode 100644 view/adminhtml/templates/page/header.phtml diff --git a/Model/AdminLogo.php b/Model/AdminLogo.php deleted file mode 100644 index b61dda5..0000000 --- a/Model/AdminLogo.php +++ /dev/null @@ -1,72 +0,0 @@ -moduleConfig = $moduleConfig; - $this->fileDriver = $fileDriver; - $this->urlBuilder = $urlBuilder; - } - - /** - * @return string|null - */ - public function getCustomAdminLoginLogoSrc(): ?string - { - if (!$logoFileName = $this->moduleConfig->getAdminLoginLogoFileName()) { - return null; - } - - return $this->fileDriver->getAbsolutePath( - $this->urlBuilder->getBaseUrl() . DirectoryList::MEDIA . DIRECTORY_SEPARATOR, - AdminLoginLogo::UPLOAD_DIR . DIRECTORY_SEPARATOR . $logoFileName - ); - } - - /** - * @return string|null - */ - public function getCustomAdminMenuLogoSrc(): ?string - { - if (!$logoFileName = $this->moduleConfig->getAdminMenuLogoFileName()) { - return null; - } - - return $this->fileDriver->getAbsolutePath( - $this->urlBuilder->getBaseUrl() . DirectoryList::MEDIA . DIRECTORY_SEPARATOR, - AdminMenuLogo::UPLOAD_DIR . DIRECTORY_SEPARATOR . $logoFileName - ); - } -} diff --git a/Plugin/Backend/Block/Page/HeaderPlugin.php b/Plugin/Backend/Block/Page/HeaderPlugin.php new file mode 100644 index 0000000..feecb36 --- /dev/null +++ b/Plugin/Backend/Block/Page/HeaderPlugin.php @@ -0,0 +1,73 @@ +scopeConfig = $scopeConfig; + $this->storeManager = $storeManager; + } + + /** + * Set custom logo URL on the block before rendering, if configured. + * + * Reads the config path and upload directory from layout XML arguments + * to determine which custom logo (login or menu) applies to this context. + */ + public function beforeToHtml(Header $subject): void + { + $configPath = $subject->getData('custom_logo_config_path'); + $uploadDir = $subject->getData('custom_logo_upload_dir'); + + if (!$configPath || !$uploadDir) { + return; + } + + $filename = $this->scopeConfig->getValue($configPath); + + if (!$filename) { + return; + } + + $mediaUrl = $this->storeManager->getStore() + ->getBaseUrl(UrlInterface::URL_TYPE_MEDIA); + + $subject->setLogoImageSrc($mediaUrl . $uploadDir . '/' . $filename); + } + + /** + * Pass through full URLs without asset repository resolution. + * + * When a custom logo is configured, logo_image_src is set to a full media + * URL. The core template passes this to getViewFileUrl(), which would + * attempt static file resolution. This plugin short-circuits that for URLs. + */ + public function afterGetViewFileUrl( + Header $subject, + string $result, + string $fileId + ): string { + if (str_starts_with($fileId, 'http://') || str_starts_with($fileId, 'https://')) { + return $fileId; + } + + return $result; + } +} diff --git a/Scope/Config.php b/Scope/Config.php deleted file mode 100644 index 391eec0..0000000 --- a/Scope/Config.php +++ /dev/null @@ -1,45 +0,0 @@ -scopeConfig = $scopeConfig; - } - - /** - * @return string|null - */ - public function getAdminLoginLogoFileName(): ?string - { - return $this->scopeConfig->getValue(self::XML_PATH_LOGIN_LOGO, ScopeInterface::SCOPE_STORE); - } - - /** - * @return string|null - */ - public function getAdminMenuLogoFileName(): ?string - { - return $this->scopeConfig->getValue(self::XML_PATH_MENU_LOGO, ScopeInterface::SCOPE_STORE); - } -} diff --git a/ViewModel/AdminLogo.php b/ViewModel/AdminLogo.php deleted file mode 100644 index 71ddea6..0000000 --- a/ViewModel/AdminLogo.php +++ /dev/null @@ -1,52 +0,0 @@ -adminLogo = $adminLogo; - $this->request = $request; - } - - /** - * @return AdminLogoModel - */ - public function getAdminLogoModel(): AdminLogoModel - { - return $this->adminLogo; - } - - /** - * @return bool - */ - public function isAdminLoginPage(): bool - { - return $this->request->getRouteName() === Area::AREA_ADMINHTML - && $this->request->getControllerName() === 'auth' - && $this->request->getActionName() === 'login'; - } -} diff --git a/etc/adminhtml/di.xml b/etc/adminhtml/di.xml new file mode 100644 index 0000000..5d5f0ad --- /dev/null +++ b/etc/adminhtml/di.xml @@ -0,0 +1,14 @@ + + + + + + + diff --git a/etc/module.xml b/etc/module.xml index 49a5f8f..245f8b3 100644 --- a/etc/module.xml +++ b/etc/module.xml @@ -9,7 +9,9 @@ xsi:noNamespaceSchemaLocation="urn:magento:framework:Module/etc/module.xsd"> + + diff --git a/view/adminhtml/layout/admin_login.xml b/view/adminhtml/layout/admin_login.xml new file mode 100644 index 0000000..cae5f39 --- /dev/null +++ b/view/adminhtml/layout/admin_login.xml @@ -0,0 +1,18 @@ + + + + + + + admin/e119_admin_logos/login + admin/logo/custom/login + + + + diff --git a/view/adminhtml/layout/default.xml b/view/adminhtml/layout/default.xml index d1c3773..b374a60 100644 --- a/view/adminhtml/layout/default.xml +++ b/view/adminhtml/layout/default.xml @@ -11,11 +11,10 @@ - + - - Element119\CustomAdminLogo\ViewModel\AdminLogo - + admin/e119_admin_logos/menu + admin/logo/custom/menu diff --git a/view/adminhtml/templates/page/header.phtml b/view/adminhtml/templates/page/header.phtml deleted file mode 100644 index d962375..0000000 --- a/view/adminhtml/templates/page/header.phtml +++ /dev/null @@ -1,88 +0,0 @@ -getShowPart(); - -/** @var AdminLogo $adminLogoViewModel */ -$adminLogoViewModel = $block->getData('admin_logo_view_model'); -?> - - hasEdition() - ? 'data-edition="' . $escaper->escapeHtml($block->getEdition()) . '"' - : ''; ?> - getAdminLogoModel()->getCustomAdminLoginLogoSrc(); ?> - getAdminLogoModel()->getCustomAdminMenuLogoSrc(); ?> - - - class="logo"> - isAdminLoginPage()): ?> - src="escapeUrl( - $adminLoginLogoSrc ?: $block->getViewFileUrl('images/magento-logo.svg') - ); ?>" - - src="escapeUrl( - $adminMenuLogoSrc ?: $block->getViewFileUrl('images/magento-icon.svg') - ); ?>" - - alt="escapeHtmlAttr(__('Magento Admin Panel')); ?>" - title="escapeHtmlAttr(__('Magento Admin Panel')); ?>"/> - - -
- - - - - - -
- - getChildHtml(); ?> - From ba627f72eada978fbce41240b4ec78bdb97507d2 Mon Sep 17 00:00:00 2001 From: Ryan Hoerr Date: Mon, 16 Mar 2026 21:31:34 -0400 Subject: [PATCH 2/4] test: add unit tests for HeaderPlugin Cover all branches of beforeToHtml (no config path, no upload dir, no filename in config, menu logo URL, login logo URL) and afterGetViewFileUrl (https passthrough, http passthrough, view file fallback). Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Backend/Block/Page/HeaderPluginTest.php | 176 ++++++++++++++++++ 1 file changed, 176 insertions(+) create mode 100644 Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php diff --git a/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php b/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php new file mode 100644 index 0000000..dd0f5fa --- /dev/null +++ b/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php @@ -0,0 +1,176 @@ +scopeConfig = $this->createMock(ScopeConfigInterface::class); + $this->storeManager = $this->createMock(StoreManagerInterface::class); + $this->header = $this->getMockBuilder(Header::class) + ->disableOriginalConstructor() + ->addMethods(['setLogoImageSrc']) + ->onlyMethods(['getData']) + ->getMock(); + + $this->plugin = new HeaderPlugin($this->scopeConfig, $this->storeManager); + } + + public function testBeforeToHtmlDoesNothingWhenNoConfigPath(): void + { + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, null], + ['custom_logo_upload_dir', null, null], + ]); + + $this->scopeConfig->expects($this->never())->method('getValue'); + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testBeforeToHtmlDoesNothingWhenNoUploadDir(): void + { + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, 'admin/e119_admin_logos/menu'], + ['custom_logo_upload_dir', null, null], + ]); + + $this->scopeConfig->expects($this->never())->method('getValue'); + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testBeforeToHtmlDoesNothingWhenNoFilenameInConfig(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn(null); + + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testBeforeToHtmlSetsMenuLogoUrl(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + $uploadDir = 'admin/logo/custom/menu'; + $filename = 'my-logo.png'; + $mediaUrl = 'https://example.com/media/'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, $uploadDir], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn($filename); + + $store = $this->createMock(Store::class); + $store->method('getBaseUrl') + ->with(UrlInterface::URL_TYPE_MEDIA) + ->willReturn($mediaUrl); + $this->storeManager->method('getStore')->willReturn($store); + + $this->header->expects($this->once()) + ->method('setLogoImageSrc') + ->with('https://example.com/media/admin/logo/custom/menu/my-logo.png'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testBeforeToHtmlSetsLoginLogoUrl(): void + { + $configPath = 'admin/e119_admin_logos/login'; + $uploadDir = 'admin/logo/custom/login'; + $filename = 'login-logo.jpg'; + $mediaUrl = 'https://example.com/media/'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, $uploadDir], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn($filename); + + $store = $this->createMock(Store::class); + $store->method('getBaseUrl') + ->with(UrlInterface::URL_TYPE_MEDIA) + ->willReturn($mediaUrl); + $this->storeManager->method('getStore')->willReturn($store); + + $this->header->expects($this->once()) + ->method('setLogoImageSrc') + ->with('https://example.com/media/admin/logo/custom/login/login-logo.jpg'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testAfterGetViewFileUrlPassesThroughHttpsUrl(): void + { + $url = 'https://example.com/media/admin/logo/custom/menu/logo.png'; + + $result = $this->plugin->afterGetViewFileUrl($this->header, 'ignored', $url); + + $this->assertSame($url, $result); + } + + public function testAfterGetViewFileUrlPassesThroughHttpUrl(): void + { + $url = 'http://example.com/media/admin/logo/custom/login/logo.png'; + + $result = $this->plugin->afterGetViewFileUrl($this->header, 'ignored', $url); + + $this->assertSame($url, $result); + } + + public function testAfterGetViewFileUrlReturnsOriginalResultForViewFile(): void + { + $resolvedUrl = 'https://example.com/static/adminhtml/Magento/backend/en_US/images/mage-os-icon.svg'; + + $result = $this->plugin->afterGetViewFileUrl( + $this->header, + $resolvedUrl, + 'images/mage-os-icon.svg' + ); + + $this->assertSame($resolvedUrl, $result); + } +} From 31fe85db8bd75850d45ee964dfd85df8294fa3ea Mon Sep 17 00:00:00 2001 From: Ryan Hoerr Date: Mon, 16 Mar 2026 22:03:33 -0400 Subject: [PATCH 3/4] fix: harden plugin with error handling, input validation, and PHPDoc MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Wrap storeManager->getStore() in try-catch for NoSuchEntityException to prevent admin lockout from a cosmetic module - Add is_string() type guard on config value (getValue returns mixed) - Add basename() sanitization to strip path traversal from filename - Inject LoggerInterface and log warning on store resolution failure - Add @param/@return PHPDoc tags for Magento2 PHPCS compliance - Add class-level docblock explaining the plugin design rationale - Fix license header: LICENCE.txt → LICENSE across all module files - Add 4 new test cases: empty string config, non-string config, path traversal stripping, NoSuchEntityException handling Co-Authored-By: Claude Opus 4.6 (1M context) --- Model/Config/Backend/AdminLoginLogo.php | 2 +- Model/Config/Backend/AdminMenuLogo.php | 2 +- Plugin/Backend/Block/Page/HeaderPlugin.php | 57 ++++++++- .../Backend/Block/Page/HeaderPluginTest.php | 117 +++++++++++++++++- etc/adminhtml/di.xml | 2 +- etc/adminhtml/system.xml | 2 +- etc/module.xml | 2 +- registration.php | 2 +- view/adminhtml/layout/admin_login.xml | 2 +- view/adminhtml/layout/default.xml | 2 +- 10 files changed, 174 insertions(+), 16 deletions(-) diff --git a/Model/Config/Backend/AdminLoginLogo.php b/Model/Config/Backend/AdminLoginLogo.php index 9e3eca6..f099aa4 100644 --- a/Model/Config/Backend/AdminLoginLogo.php +++ b/Model/Config/Backend/AdminLoginLogo.php @@ -1,7 +1,7 @@ scopeConfig = $scopeConfig; $this->storeManager = $storeManager; + $this->logger = $logger; } /** @@ -30,6 +52,9 @@ public function __construct( * * Reads the config path and upload directory from layout XML arguments * to determine which custom logo (login or menu) applies to this context. + * + * @param Header $subject + * @return void */ public function beforeToHtml(Header $subject): void { @@ -42,12 +67,26 @@ public function beforeToHtml(Header $subject): void $filename = $this->scopeConfig->getValue($configPath); - if (!$filename) { + if (!is_string($filename) || $filename === '') { return; } - $mediaUrl = $this->storeManager->getStore() - ->getBaseUrl(UrlInterface::URL_TYPE_MEDIA); + $filename = basename($filename); + + if ($filename === '') { + return; + } + + try { + $mediaUrl = $this->storeManager->getStore() + ->getBaseUrl(UrlInterface::URL_TYPE_MEDIA); + } catch (NoSuchEntityException $e) { + $this->logger->warning( + 'Element119_CustomAdminLogo: Unable to resolve store for media URL.', + ['exception' => $e] + ); + return; + } $subject->setLogoImageSrc($mediaUrl . $uploadDir . '/' . $filename); } @@ -57,7 +96,13 @@ public function beforeToHtml(Header $subject): void * * When a custom logo is configured, logo_image_src is set to a full media * URL. The core template passes this to getViewFileUrl(), which would - * attempt static file resolution. This plugin short-circuits that for URLs. + * attempt static file resolution. This plugin short-circuits that for + * absolute URLs, returning the fileId as-is. + * + * @param Header $subject + * @param string $result + * @param string $fileId + * @return string */ public function afterGetViewFileUrl( Header $subject, diff --git a/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php b/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php index dd0f5fa..12205f2 100644 --- a/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php +++ b/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php @@ -1,7 +1,7 @@ scopeConfig = $this->createMock(ScopeConfigInterface::class); $this->storeManager = $this->createMock(StoreManagerInterface::class); + $this->logger = $this->createMock(LoggerInterface::class); $this->header = $this->getMockBuilder(Header::class) ->disableOriginalConstructor() ->addMethods(['setLogoImageSrc']) ->onlyMethods(['getData']) ->getMock(); - $this->plugin = new HeaderPlugin($this->scopeConfig, $this->storeManager); + $this->plugin = new HeaderPlugin( + $this->scopeConfig, + $this->storeManager, + $this->logger + ); } public function testBeforeToHtmlDoesNothingWhenNoConfigPath(): void @@ -83,6 +100,73 @@ public function testBeforeToHtmlDoesNothingWhenNoFilenameInConfig(): void $this->plugin->beforeToHtml($this->header); } + public function testBeforeToHtmlDoesNothingWhenFilenameIsEmptyString(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn(''); + + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testBeforeToHtmlDoesNothingWhenConfigReturnsNonString(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn(['unexpected' => 'array']); + + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testBeforeToHtmlStripsPathTraversalFromFilename(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + $uploadDir = 'admin/logo/custom/menu'; + $mediaUrl = 'https://example.com/media/'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, $uploadDir], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn('../../etc/passwd'); + + $store = $this->createMock(Store::class); + $store->method('getBaseUrl') + ->with(UrlInterface::URL_TYPE_MEDIA) + ->willReturn($mediaUrl); + $this->storeManager->method('getStore')->willReturn($store); + + $this->header->expects($this->once()) + ->method('setLogoImageSrc') + ->with('https://example.com/media/admin/logo/custom/menu/passwd'); + + $this->plugin->beforeToHtml($this->header); + } + public function testBeforeToHtmlSetsMenuLogoUrl(): void { $configPath = 'admin/e119_admin_logos/menu'; @@ -143,6 +227,35 @@ public function testBeforeToHtmlSetsLoginLogoUrl(): void $this->plugin->beforeToHtml($this->header); } + public function testBeforeToHtmlHandlesNoSuchEntityException(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + + $this->header->method('getData') + ->willReturnMap([ + ['custom_logo_config_path', null, $configPath], + ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn('logo.png'); + + $this->storeManager->method('getStore') + ->willThrowException(new NoSuchEntityException(__('Store not found'))); + + $this->logger->expects($this->once()) + ->method('warning') + ->with( + 'Element119_CustomAdminLogo: Unable to resolve store for media URL.', + $this->arrayHasKey('exception') + ); + + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + public function testAfterGetViewFileUrlPassesThroughHttpsUrl(): void { $url = 'https://example.com/media/admin/logo/custom/menu/logo.png'; diff --git a/etc/adminhtml/di.xml b/etc/adminhtml/di.xml index 5d5f0ad..1fec063 100644 --- a/etc/adminhtml/di.xml +++ b/etc/adminhtml/di.xml @@ -2,7 +2,7 @@ Date: Mon, 16 Mar 2026 22:46:50 -0400 Subject: [PATCH 4/4] fix: add show_part guard, broaden exception handling, clean up module config - Add show_part !== 'logo' guard to both plugin methods so they skip non-logo Header block instances (e.g. the 'user' block) - Catch \Throwable in addition to NoSuchEntityException so unexpected errors degrade gracefully instead of breaking the admin header - Fix system.xml group scope to showInWebsite=0/showInStore=0 matching the field visibility (prevents empty group at non-default scopes) - Add missing composer.json dependencies (framework, backend, store) - Remove redundant _getUploadDir() overrides from backend models (parent File class already resolves path from system.xml config) - Fix LESS license header (LICENCE.txt -> LICENSE) - Update tests: add show_part guard and \Throwable coverage (15 tests) Co-Authored-By: Claude Opus 4.6 (1M context) --- Model/Config/Backend/AdminLoginLogo.php | 10 -- Model/Config/Backend/AdminMenuLogo.php | 10 -- Plugin/Backend/Block/Page/HeaderPlugin.php | 16 +- .../Backend/Block/Page/HeaderPluginTest.php | 169 ++++++++++++------ composer.json | 5 +- etc/adminhtml/system.xml | 4 +- view/adminhtml/web/css/module.less | 2 +- 7 files changed, 137 insertions(+), 79 deletions(-) diff --git a/Model/Config/Backend/AdminLoginLogo.php b/Model/Config/Backend/AdminLoginLogo.php index f099aa4..1d232ef 100644 --- a/Model/Config/Backend/AdminLoginLogo.php +++ b/Model/Config/Backend/AdminLoginLogo.php @@ -11,16 +11,6 @@ class AdminLoginLogo extends Image { - public const UPLOAD_DIR = 'admin/logo/custom/login'; - - /** - * @inheritDoc - */ - protected function _getUploadDir(): string - { - return $this->_mediaDirectory->getAbsolutePath(self::UPLOAD_DIR); - } - /** * @inheritDoc */ diff --git a/Model/Config/Backend/AdminMenuLogo.php b/Model/Config/Backend/AdminMenuLogo.php index 1e8ae98..2200598 100644 --- a/Model/Config/Backend/AdminMenuLogo.php +++ b/Model/Config/Backend/AdminMenuLogo.php @@ -11,16 +11,6 @@ class AdminMenuLogo extends Image { - public const UPLOAD_DIR = 'admin/logo/custom/menu'; - - /** - * @inheritDoc - */ - protected function _getUploadDir(): string - { - return $this->_mediaDirectory->getAbsolutePath(self::UPLOAD_DIR); - } - /** * @inheritDoc */ diff --git a/Plugin/Backend/Block/Page/HeaderPlugin.php b/Plugin/Backend/Block/Page/HeaderPlugin.php index 05fcf80..c8ba710 100644 --- a/Plugin/Backend/Block/Page/HeaderPlugin.php +++ b/Plugin/Backend/Block/Page/HeaderPlugin.php @@ -58,6 +58,10 @@ public function __construct( */ public function beforeToHtml(Header $subject): void { + if ($subject->getData('show_part') !== 'logo') { + return; + } + $configPath = $subject->getData('custom_logo_config_path'); $uploadDir = $subject->getData('custom_logo_upload_dir'); @@ -86,6 +90,12 @@ public function beforeToHtml(Header $subject): void ['exception' => $e] ); return; + } catch (\Throwable $e) { + $this->logger->error( + 'Element119_CustomAdminLogo: Unexpected error resolving media URL.', + ['exception' => $e] + ); + return; } $subject->setLogoImageSrc($mediaUrl . $uploadDir . '/' . $filename); @@ -107,8 +117,12 @@ public function beforeToHtml(Header $subject): void public function afterGetViewFileUrl( Header $subject, string $result, - string $fileId + string $fileId = '' ): string { + if ($subject->getData('show_part') !== 'logo') { + return $result; + } + if (str_starts_with($fileId, 'http://') || str_starts_with($fileId, 'https://')) { return $fileId; } diff --git a/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php b/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php index 12205f2..fabce88 100644 --- a/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php +++ b/Test/Unit/Plugin/Backend/Block/Page/HeaderPluginTest.php @@ -20,19 +20,10 @@ class HeaderPluginTest extends TestCase { - /** @var ScopeConfigInterface&MockObject */ private ScopeConfigInterface&MockObject $scopeConfig; - - /** @var StoreManagerInterface&MockObject */ private StoreManagerInterface&MockObject $storeManager; - - /** @var LoggerInterface&MockObject */ private LoggerInterface&MockObject $logger; - - /** @var MockObject */ private MockObject $header; - - /** @var HeaderPlugin */ private HeaderPlugin $plugin; protected function setUp(): void @@ -53,13 +44,52 @@ protected function setUp(): void ); } + /** + * Helper: configure getData to return the given map plus show_part=logo. + */ + private function setHeaderData(array $extra = []): void + { + $map = [['show_part', null, 'logo']]; + foreach ($extra as $key => $value) { + $map[] = [$key, null, $value]; + } + $this->header->method('getData')->willReturnMap($map); + } + + // ── show_part guard ────────────────────────────────────────────── + + public function testBeforeToHtmlSkipsNonLogoBlock(): void + { + $this->header->method('getData')->willReturnMap([ + ['show_part', null, 'user'], + ]); + + $this->scopeConfig->expects($this->never())->method('getValue'); + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + public function testAfterGetViewFileUrlSkipsNonLogoBlock(): void + { + $this->header->method('getData')->willReturnMap([ + ['show_part', null, 'user'], + ]); + + $url = 'https://example.com/media/admin/logo/custom/menu/logo.png'; + $result = $this->plugin->afterGetViewFileUrl($this->header, 'original-result', $url); + + $this->assertSame('original-result', $result); + } + + // ── beforeToHtml guard clauses ─────────────────────────────────── + public function testBeforeToHtmlDoesNothingWhenNoConfigPath(): void { - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, null], - ['custom_logo_upload_dir', null, null], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => null, + 'custom_logo_upload_dir' => null, + ]); $this->scopeConfig->expects($this->never())->method('getValue'); $this->header->expects($this->never())->method('setLogoImageSrc'); @@ -69,11 +99,10 @@ public function testBeforeToHtmlDoesNothingWhenNoConfigPath(): void public function testBeforeToHtmlDoesNothingWhenNoUploadDir(): void { - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, 'admin/e119_admin_logos/menu'], - ['custom_logo_upload_dir', null, null], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => 'admin/e119_admin_logos/menu', + 'custom_logo_upload_dir' => null, + ]); $this->scopeConfig->expects($this->never())->method('getValue'); $this->header->expects($this->never())->method('setLogoImageSrc'); @@ -85,11 +114,10 @@ public function testBeforeToHtmlDoesNothingWhenNoFilenameInConfig(): void { $configPath = 'admin/e119_admin_logos/menu'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => 'admin/logo/custom/menu', + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -104,11 +132,10 @@ public function testBeforeToHtmlDoesNothingWhenFilenameIsEmptyString(): void { $configPath = 'admin/e119_admin_logos/menu'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => 'admin/logo/custom/menu', + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -123,11 +150,10 @@ public function testBeforeToHtmlDoesNothingWhenConfigReturnsNonString(): void { $configPath = 'admin/e119_admin_logos/menu'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => 'admin/logo/custom/menu', + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -138,17 +164,18 @@ public function testBeforeToHtmlDoesNothingWhenConfigReturnsNonString(): void $this->plugin->beforeToHtml($this->header); } + // ── beforeToHtml security ──────────────────────────────────────── + public function testBeforeToHtmlStripsPathTraversalFromFilename(): void { $configPath = 'admin/e119_admin_logos/menu'; $uploadDir = 'admin/logo/custom/menu'; $mediaUrl = 'https://example.com/media/'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, $uploadDir], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => $uploadDir, + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -167,6 +194,8 @@ public function testBeforeToHtmlStripsPathTraversalFromFilename(): void $this->plugin->beforeToHtml($this->header); } + // ── beforeToHtml happy path ────────────────────────────────────── + public function testBeforeToHtmlSetsMenuLogoUrl(): void { $configPath = 'admin/e119_admin_logos/menu'; @@ -174,11 +203,10 @@ public function testBeforeToHtmlSetsMenuLogoUrl(): void $filename = 'my-logo.png'; $mediaUrl = 'https://example.com/media/'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, $uploadDir], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => $uploadDir, + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -204,11 +232,10 @@ public function testBeforeToHtmlSetsLoginLogoUrl(): void $filename = 'login-logo.jpg'; $mediaUrl = 'https://example.com/media/'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, $uploadDir], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => $uploadDir, + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -227,15 +254,16 @@ public function testBeforeToHtmlSetsLoginLogoUrl(): void $this->plugin->beforeToHtml($this->header); } + // ── beforeToHtml error handling ────────────────────────────────── + public function testBeforeToHtmlHandlesNoSuchEntityException(): void { $configPath = 'admin/e119_admin_logos/menu'; - $this->header->method('getData') - ->willReturnMap([ - ['custom_logo_config_path', null, $configPath], - ['custom_logo_upload_dir', null, 'admin/logo/custom/menu'], - ]); + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => 'admin/logo/custom/menu', + ]); $this->scopeConfig->method('getValue') ->with($configPath) @@ -256,8 +284,39 @@ public function testBeforeToHtmlHandlesNoSuchEntityException(): void $this->plugin->beforeToHtml($this->header); } + public function testBeforeToHtmlHandlesUnexpectedThrowable(): void + { + $configPath = 'admin/e119_admin_logos/menu'; + + $this->setHeaderData([ + 'custom_logo_config_path' => $configPath, + 'custom_logo_upload_dir' => 'admin/logo/custom/menu', + ]); + + $this->scopeConfig->method('getValue') + ->with($configPath) + ->willReturn('logo.png'); + + $this->storeManager->method('getStore') + ->willThrowException(new \RuntimeException('Unexpected failure')); + + $this->logger->expects($this->once()) + ->method('error') + ->with( + 'Element119_CustomAdminLogo: Unexpected error resolving media URL.', + $this->arrayHasKey('exception') + ); + + $this->header->expects($this->never())->method('setLogoImageSrc'); + + $this->plugin->beforeToHtml($this->header); + } + + // ── afterGetViewFileUrl ────────────────────────────────────────── + public function testAfterGetViewFileUrlPassesThroughHttpsUrl(): void { + $this->setHeaderData(); $url = 'https://example.com/media/admin/logo/custom/menu/logo.png'; $result = $this->plugin->afterGetViewFileUrl($this->header, 'ignored', $url); @@ -267,6 +326,7 @@ public function testAfterGetViewFileUrlPassesThroughHttpsUrl(): void public function testAfterGetViewFileUrlPassesThroughHttpUrl(): void { + $this->setHeaderData(); $url = 'http://example.com/media/admin/logo/custom/login/logo.png'; $result = $this->plugin->afterGetViewFileUrl($this->header, 'ignored', $url); @@ -276,6 +336,7 @@ public function testAfterGetViewFileUrlPassesThroughHttpUrl(): void public function testAfterGetViewFileUrlReturnsOriginalResultForViewFile(): void { + $this->setHeaderData(); $resolvedUrl = 'https://example.com/static/adminhtml/Magento/backend/en_US/images/mage-os-icon.svg'; $result = $this->plugin->afterGetViewFileUrl( diff --git a/composer.json b/composer.json index d3181dd..c2ae9fd 100644 --- a/composer.json +++ b/composer.json @@ -10,7 +10,10 @@ } ], "require": { - "magento/module-config": "^101.0" + "magento/framework": "*", + "magento/module-backend": "*", + "magento/module-config": "^101.0", + "magento/module-store": "*" }, "autoload": { "psr-4": { diff --git a/etc/adminhtml/system.xml b/etc/adminhtml/system.xml index 90bf2d9..0b1f620 100644 --- a/etc/adminhtml/system.xml +++ b/etc/adminhtml/system.xml @@ -13,8 +13,8 @@ translate="label" sortOrder="100" showInDefault="1" - showInWebsite="1" - showInStore="1"> + showInWebsite="0" + showInStore="0">