Skip to content

Fix 2FA admin emails silently suppressed when default store has email disabled - #346

Open
thai2301 wants to merge 2 commits into
magento:developfrom
thai2301:40969-2fa-email-store-scope
Open

Fix 2FA admin emails silently suppressed when default store has email disabled#346
thai2301 wants to merge 2 commits into
magento:developfrom
thai2301:40969-2fa-email-store-scope

Conversation

@thai2301

Copy link
Copy Markdown

Description

EmailUserNotifier::sendConfigRequired() renders its 2FA-instructions email for 'area' => 'adminhtml', 'store' => 0, but that option only affects the template's locale/content. Whether the email actually gets sent is decided by a completely separate, global plugin in Magento core: Magento\Email\Model\Mail\TransportInterfacePlugin::aroundSendMessage(), which checks:

$this->scopeConfig->isSetFlag('system/smtp/disable', ScopeInterface::SCOPE_STORE)

with no explicit scope code. That resolves against whatever store StoreManagerInterface::getStore() considers "current" at that moment — in a live adminhtml request (no frontend store context), Magento's store resolution falls through to the default store of the default website, not store 0. If that store specifically has email communication disabled, every 2FA notification is silently swallowed, even though this email is architecturally meant to be governed by the global/default scope.

This mirrors a very similar, already-fixed bug in Magento\ProductAlert\Model\Email::send() (ticket ACP2E-4870, "Product Alert Consumer ignores store scoped system/smtp/disable"), which wraps its own sendMessage() call in App\Emulation::startEnvironmentEmulation()/stopEnvironmentEmulation() so the ambient store the suppression check reads actually matches the store the email was built for.

This PR applies the same pattern here, emulating Store::DEFAULT_STORE_ID / Area::AREA_ADMINHTML around the sendMessage() call, so the plugin's scope-less check resolves to the same global scope this notification is rendered for, instead of an unrelated frontend store. The TransportInterfacePlugin itself is intentionally left untouched — it's a shared plugin wrapping every outgoing email in Magento, and changing its scope resolution would break legitimate per-store suppression for genuinely store-scoped emails (newsletters, product alerts, etc., including the ACP2E-4870 fix it relies on).

Related Issue

Fixes magento/magento2#40969

Manual testing scenarios

  1. Set system/smtp/disable (Store Email Addresses config or direct core_config_data row) to 1 for a specific store, e.g. the default store of the default website.
  2. Enable 2FA and create/edit an admin user so a "configuration required" 2FA email would normally be triggered.
  3. Before this fix: the email is silently not sent, no error surfaced. After this fix: the email sends normally, since the suppression check now correctly reads the default/global scope rather than the disabled store's scope.

Questions or comments

None.

Contribution checklist

  • Pull request has a meaningful description of its purpose
  • All existing and new tests pass
  • Static tests pass
  • Ensured backward compatibility

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.
@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

1 similar comment
@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

@thai2301

Copy link
Copy Markdown
Author

@magento run Unit Tests, Functional Tests B2B

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.
@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Admin e-mails not sent when e-mail communication is disabled for store id 1

1 participant