From ce16fb22bd38dee1850592629f615dd766953954 Mon Sep 17 00:00:00 2001 From: Daniel Vu Date: Mon, 20 Jul 2026 15:29:32 +0700 Subject: [PATCH 1/2] 40969: emulate default store/adminhtml scope around 2FA email send EmailUserNotifier renders its emails for store 0/adminhtml, but that only affects template locale/content. The actual send-suppression check lives in Magento_Email's TransportInterfacePlugin, which reads system/smtp/disable from whatever store the environment currently resolves to - which in a live adminhtml request falls through to the default store of the default website, not store 0. If that store has email communication disabled, every 2FA notification silently gets swallowed even though it's meant to be governed by the global scope. Wrapped the sendMessage() call in App\Emulation::startEnvironmentEmulation for Store::DEFAULT_STORE_ID/adminhtml, mirroring the existing fix for the same class of bug in Magento\ProductAlert\Model\Email (ACP2E-4870), which uses the identical pattern for its own (per-store) scope. --- TwoFactorAuth/Model/EmailUserNotifier.php | 25 ++- .../Test/Unit/Model/EmailUserNotifierTest.php | 152 ++++++++++++++++++ 2 files changed, 175 insertions(+), 2 deletions(-) create mode 100644 TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php diff --git a/TwoFactorAuth/Model/EmailUserNotifier.php b/TwoFactorAuth/Model/EmailUserNotifier.php index c53ef6a7..aa6c0484 100644 --- a/TwoFactorAuth/Model/EmailUserNotifier.php +++ b/TwoFactorAuth/Model/EmailUserNotifier.php @@ -8,7 +8,10 @@ namespace Magento\TwoFactorAuth\Model; +use Magento\Framework\App\Area; use Magento\Framework\App\Config\ScopeConfigInterface; +use Magento\Store\Model\App\Emulation; +use Magento\Store\Model\Store; use Magento\Store\Model\StoreManagerInterface; use Magento\TwoFactorAuth\Model\Config\UserNotifier as UserNotifierConfig; use Magento\User\Model\User; @@ -47,25 +50,34 @@ class EmailUserNotifier implements UserNotifierInterface */ private $userNotifierConfig; + /** + * @var Emulation + */ + private $appEmulation; + /** * @param ScopeConfigInterface $scopeConfig * @param TransportBuilder $transportBuilder * @param StoreManagerInterface $storeManager * @param LoggerInterface $logger * @param UserNotifierConfig $userNotifierConfig + * @param Emulation|null $appEmulation */ public function __construct( ScopeConfigInterface $scopeConfig, TransportBuilder $transportBuilder, StoreManagerInterface $storeManager, LoggerInterface $logger, - UserNotifierConfig $userNotifierConfig + UserNotifierConfig $userNotifierConfig, + ?Emulation $appEmulation = null ) { $this->scopeConfig = $scopeConfig; $this->transportBuilder = $transportBuilder; $this->storeManager = $storeManager; $this->logger = $logger; $this->userNotifierConfig = $userNotifierConfig; + $this->appEmulation = $appEmulation + ?? \Magento\Framework\App\ObjectManager::getInstance()->get(Emulation::class); } /** @@ -103,7 +115,16 @@ private function sendConfigRequired( ) ->addTo($user->getEmail(), $user->getFirstName() . ' ' . $user->getLastName()) ->getTransport(); - $transport->sendMessage(); + // The send-suppression check in TransportInterfacePlugin reads system/smtp/disable from + // whatever store the environment currently resolves to, not from the store this email was + // rendered for above. Emulate the default/admin store so it checks the same global scope + // that this notification is actually governed by, rather than an unrelated frontend store. + $this->appEmulation->startEnvironmentEmulation(Store::DEFAULT_STORE_ID, Area::AREA_ADMINHTML); + try { + $transport->sendMessage(); + } finally { + $this->appEmulation->stopEnvironmentEmulation(); + } } catch (\Throwable $exception) { $this->logger->critical($exception); throw new NotificationException('Failed to send 2FA E-mail to a user', 0, $exception); diff --git a/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php b/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php new file mode 100644 index 00000000..e17af5b2 --- /dev/null +++ b/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php @@ -0,0 +1,152 @@ +scopeConfigMock = $this->createMock(ScopeConfigInterface::class); + $this->transportBuilderMock = $this->createMock(TransportBuilder::class); + $this->storeManagerMock = $this->createMock(StoreManagerInterface::class); + $this->userNotifierConfigMock = $this->createMock(UserNotifierConfig::class); + $this->appEmulationMock = $this->createMock(Emulation::class); + $this->transportMock = $this->createMock(TransportInterface::class); + + $this->transportBuilderMock->method('setTemplateIdentifier')->willReturnSelf(); + $this->transportBuilderMock->method('setTemplateOptions')->willReturnSelf(); + $this->transportBuilderMock->method('setTemplateVars')->willReturnSelf(); + $this->transportBuilderMock->method('setFromByScope')->willReturnSelf(); + $this->transportBuilderMock->method('addTo')->willReturnSelf(); + $this->transportBuilderMock->method('getTransport')->willReturn($this->transportMock); + + $storeMock = $this->createMock(\Magento\Store\Api\Data\StoreInterface::class); + $storeMock->method('getFrontendName')->willReturn('Main Website Store'); + $this->storeManagerMock->method('getStore')->willReturn($storeMock); + + $this->notifier = new EmailUserNotifier( + $this->scopeConfigMock, + $this->transportBuilderMock, + $this->storeManagerMock, + $this->createMock(LoggerInterface::class), + $this->userNotifierConfigMock, + $this->appEmulationMock + ); + } + + /** + * The email is built for store 0/adminhtml, but the send-suppression plugin reads + * system/smtp/disable from whatever store the environment currently resolves to. The send call + * must happen inside default-store/adminhtml emulation so that check is scoped correctly too. + */ + public function testSendMessageIsEmulatedAsDefaultStoreAdminhtml(): void + { + $callOrder = []; + $this->appEmulationMock->expects($this->once()) + ->method('startEnvironmentEmulation') + ->with(Store::DEFAULT_STORE_ID, Area::AREA_ADMINHTML) + ->willReturnCallback(function () use (&$callOrder) { + $callOrder[] = 'start'; + }); + $this->transportMock->expects($this->once()) + ->method('sendMessage') + ->willReturnCallback(function () use (&$callOrder) { + $callOrder[] = 'send'; + }); + $this->appEmulationMock->expects($this->once()) + ->method('stopEnvironmentEmulation') + ->willReturnCallback(function () use (&$callOrder) { + $callOrder[] = 'stop'; + }); + + $user = $this->createMock(User::class); + $user->method('getEmail')->willReturn('admin@example.com'); + $user->method('getFirstName')->willReturn('Jane'); + $user->method('getLastName')->willReturn('Doe'); + + $this->userNotifierConfigMock->method('getPersonalRequestConfigUrl')->willReturn('http://example.com/token'); + + $this->notifier->sendUserConfigRequestMessage($user, 'token'); + + self::assertSame(['start', 'send', 'stop'], $callOrder); + } + + /** + * Emulation must be torn down even if sending itself throws, otherwise the admin store context + * leaks into the rest of the request. + */ + public function testEmulationIsStoppedEvenWhenSendMessageThrows(): void + { + $this->appEmulationMock->expects($this->once())->method('startEnvironmentEmulation'); + $this->transportMock->method('sendMessage')->willThrowException(new \RuntimeException('SMTP down')); + $this->appEmulationMock->expects($this->once())->method('stopEnvironmentEmulation'); + + $user = $this->createMock(User::class); + $user->method('getEmail')->willReturn('admin@example.com'); + $user->method('getFirstName')->willReturn('Jane'); + $user->method('getLastName')->willReturn('Doe'); + + $this->userNotifierConfigMock->method('getAppRequestConfigUrl')->willReturn('http://example.com/token'); + + $this->expectException(\Magento\TwoFactorAuth\Model\Exception\NotificationException::class); + $this->notifier->sendAppConfigRequestMessage($user, 'token'); + } +} From eba3db229e3863b24b8d53816565ede6395d6e3f Mon Sep 17 00:00:00 2001 From: Daniel Vu Date: Tue, 21 Jul 2026 13:49:38 +0700 Subject: [PATCH 2/2] 40969: fix unit test - mock concrete Store, not StoreInterface getFrontendName() is only declared on the concrete Store class, not on StoreInterface, so PHPUnit's mock generator rejected configuring it on a StoreInterface mock. Also suppressed the expected CouplingBetweenObjects PHPMD warning on the test class, matching existing test conventions. --- TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php b/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php index e17af5b2..3a950de5 100644 --- a/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php +++ b/TwoFactorAuth/Test/Unit/Model/EmailUserNotifierTest.php @@ -22,6 +22,9 @@ use PHPUnit\Framework\TestCase; use Psr\Log\LoggerInterface; +/** + * @SuppressWarnings(PHPMD.CouplingBetweenObjects) + */ class EmailUserNotifierTest extends TestCase { /** @@ -78,7 +81,7 @@ protected function setUp(): void $this->transportBuilderMock->method('addTo')->willReturnSelf(); $this->transportBuilderMock->method('getTransport')->willReturn($this->transportMock); - $storeMock = $this->createMock(\Magento\Store\Api\Data\StoreInterface::class); + $storeMock = $this->createMock(Store::class); $storeMock->method('getFrontendName')->willReturn('Main Website Store'); $this->storeManagerMock->method('getStore')->willReturn($storeMock);