Skip to content

Stats collection must not be shown in captive MFA pages#38533

Merged
roland-d merged 2 commits intojoomla:4.2-devfrom
nikosdion:fix/38476-mfa-stats
Aug 20, 2022
Merged

Stats collection must not be shown in captive MFA pages#38533
roland-d merged 2 commits intojoomla:4.2-devfrom
nikosdion:fix/38476-mfa-stats

Conversation

@nikosdion
Copy link
Copy Markdown
Contributor

Pull Request for Issue #38476 .

Summary of Changes

The Joomla\CMS\Application\MultiFactorAuthenticationHandler::isMultiFactorAuthenticationPage method is now public and has an optional argument to only consider captive pages.

The plg_system_stats plugin uses this method to check whether it is running a captive MFA page and refuse to continue.

A previously hard-coded exception for the stats plugin has been removed from the MultiFactorAuthenticationHandler Trait.

Testing Instructions

  • Install Joomla 4.2.0
  • DO NOT make a choice about the stats collection just yet
  • Set up MFA for your user account
  • Log out
  • Log back in

Actual result BEFORE applying this Pull Request

You see the stats collection interface in the captive MFA page. Trying to use its buttons leads to a broken display experience as per the issue #38476.

Expected result AFTER applying this Pull Request

You do NOT see the stats collection interface in the captive MFA page or any of the captive pages you are allowed access to (basically, selecting an MFA method). It does appear after completing the MFA validation.

Documentation Changes Required

Plugins which render user interfaces in the backend of the site must check whether they are running under the Multi-factor Authentication feature's captive pages using the following code:

method_exists($this->app, 'isMultiFactorAuthenticationPage')
            && $this->app->isMultiFactorAuthenticationPage(true);

If this returns true the plugin MUST NOT render its user interface.

@bayareajenn
Copy link
Copy Markdown

I have tested this item ✅ successfully on 2da7737


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38533.

@softforge
Copy link
Copy Markdown
Contributor

I have tested this item ✅ successfully on 2da7737

I tested using yubikey as my MFA choice and can confirm the stats were NOT showing on the yubikey entry page with this patch applied and once logged in reshowed until I made a choice


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38533.

@richard67
Copy link
Copy Markdown
Member

I have tested this item ✅ successfully on 2da7737

3rd test is not a bad thing.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38533.

@joomla-cms-bot joomla-cms-bot removed this from the Joomla! 4.2.1 milestone Aug 20, 2022
@richard67
Copy link
Copy Markdown
Member

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38533.

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Aug 20, 2022
@richard67 richard67 added this to the Joomla! 4.2.1 milestone Aug 20, 2022
@roland-d roland-d merged commit 63f9f9c into joomla:4.2-dev Aug 20, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Aug 20, 2022
@roland-d
Copy link
Copy Markdown
Contributor

Thank you

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.

8 participants