Skip to content

Rename 2FA content blocks off the globally-unique 'content' name (#343)#345

Open
thai2301 wants to merge 4 commits into
magento:developfrom
thai2301:343-fix-content-block-name-collision
Open

Rename 2FA content blocks off the globally-unique 'content' name (#343)#345
thai2301 wants to merge 4 commits into
magento:developfrom
thai2301:343-fix-content-block-name-collision

Conversation

@thai2301

Copy link
Copy Markdown

Description (*)

Each of the affected adminhtml layout files declares its 2FA provider's own configuration/verification block with name="content". Magento block names are globally unique across the merged layout regardless of container nesting, and the admin layout already registers a block/container named content elsewhere in the merged tree. Whichever declaration is processed last during layout merge wins the registry slot for that name; on any instance where the ordinary content block/container also participates in these handles, the 2FA provider's own block gets silently dropped from the registry (overwritten), and the section where the configuration form should render is blank — with the provider's block class never even being instantiated.

Renamed each block to a handle-specific name (e.g. tfa-google-configure-content, derived from the layout handle it lives in) that cannot collide with anything else in the merged layout. Verified none of these blocks are referenced by name from any other layout file or template (getChildBlock('content'), referenceBlock name="content", etc.) — this is a pure rename with no other behavior change.

Affected files/handles:

  • tfa_google_auth, tfa_google_configure, tfa_google_verifyidentity
  • tfa_authy_auth, tfa_authy_configure, tfa_authy_verifyidentity
  • tfa_u2f_auth, tfa_u2f_configure, tfa_u2f_verifyidentity
  • tfa_duo_auth, tfa_duo_verifyidentity
  • tfa_tfa_configure, tfa_tfa_verifyidentity, tfa_tfa_requestconfig, tfa_tfa_providerselection

Fixed Issues (if relevant)

  1. 2FA modules using "content" as block name results in blank pages #343: 2FA modules using "content" as block name results in blank pages

Manual testing scenarios (*)

  1. Add a custom block named content to the default adminhtml layout in any module (as described in the issue), or otherwise arrange for a block/container named content to participate in one of the tfa_* handles above.
  2. Open any 2FA configure/auth/verify-identity page, e.g. tfa/google/configure.
  3. Before this fix: the configuration form area is blank; the provider's block class is never instantiated (confirmed via breakpoint in the reporter's investigation).
  4. After this fix: the provider's configuration/verification form renders normally, unaffected by whatever else in the instance happens to be named content.
  5. Regression check: on a clean instance (no colliding content block), all 2FA provider screens (Google, Authy, U2F, Duo, generic TFA) still render identically to before this change.

Questions or comments

None.

Contribution checklist (*)

  • Author has signed the Adobe CLA
  • Pull request has a meaningful description of its purpose
  • All commits are accompanied by meaningful commit messages
  • All new or changed code is covered with unit/integration tests (not applicable — pure layout XML block rename, no testable logic changed)
  • All automated tests passed successfully (all builds are green)

…ent' name

Each of these adminhtml layout files declared its provider-specific content
block as name="content". Magento block names are globally unique across the
merged layout regardless of container nesting, and the admin layout already
registers a block/container named "content" elsewhere. Whichever declaration
is merged last wins the registry slot, so on any instance where the ordinary
"content" block/container also participates in these handles, the 2FA
provider's own block gets silently dropped and the configuration screen
renders blank.

Renamed each block to a handle-specific name (e.g. tfa-google-configure-content)
that can't collide. No template or layout file referenced these blocks by
their old name, so this is a pure rename with no other behavior change.
@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

@thai2301

Copy link
Copy Markdown
Author

Note on the Semantic Version Checker result: it correctly flags this as a MAJOR change for magento/module-two-factor-auth ("Block was removed" for each renamed content block).

That's expected and unavoidable here — the root cause is that content is not a safe, collision-free name to begin with (Magento block names are globally unique across the merged layout), so any fix has to change that name on the TwoFactorAuth side. Third-party code that happens to reference these specific blocks by their old content name (e.g. via referenceBlock name="content" scoped to one of these tfa_* handles) would need to update to the new name — but referencing it that way was never reliable to begin with, since the same block was liable to silently disappear whenever anything else in the merged layout also used content, which is the bug this PR fixes.

Happy to bump the module's version accordingly if that's the convention here, just let me know.

Six of the touched layout files carried 'Copyright 2025 Adobe.' with a
trailing period, which the public-copyright sanity check rejects on any
modified file. Dropped the period to match the required template.
@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

The copyright sanity check rejected the lowercase 'All rights reserved.'
variant on the previous run despite the template describing the match as
case-insensitive; align the six headers byte-for-byte with the canonical
form used by every other file in the module.
@thai2301

Copy link
Copy Markdown
Author

@magento run Static Tests

@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

@thai2301

Copy link
Copy Markdown
Author

@magento run Functional Tests B2B, Functional Tests CE, Functional Tests EE

@thai2301

Copy link
Copy Markdown
Author

Updated with latest develop. @magento run all tests

@thai2301

Copy link
Copy Markdown
Author

@magento run all tests

@thai2301

Copy link
Copy Markdown
Author

@magento run Functional Tests CE

@thai2301

Copy link
Copy Markdown
Author

@magento run Functional Tests B2B, Functional Tests EE, Static Tests, Unit Tests, Integration Tests, WebAPI Tests, Semantic Version Checker

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.

1 participant