[#38610] - [4.3] Use the current user in MFA
- Fixed in Code Base
- 1 Dec 2022
- Medium
- Build: 4.3-dev
- # 38610
- Diff
- Digital-Peak:mfa/model/currentuser
User tests: Successful: Unsuccessful:
Summary of Changes
Uses the current user in the MFA models instead of from the factory.
ping @nikosdion for review
Testing Instructions
- Create a Joomla user and log into the site with it
- Edit your profile
- There's a Multi-factor Authentication tab
- You will need to enable an authenticator
- The verification code is basically Google Authenticator
- Click it and it gives you on screen instructions and a QR code to scan
- Enter a 6 digit code generated by it into the box and click the button
- Now log out of the site and log back in
- It asks you for the 6 digit code
Actual result BEFORE applying this Pull Request
All works.
Expected result AFTER applying this Pull Request
All works.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Administration com_users Libraries |
| Labels |
Added:
PR-4.3-dev
|
||
Stupid question. Have we ever checked if the MFA system plugin continues to work correctly in the CLI application after this change? The ConsoleApplication returns null for getIdentity (instead of a guest user). The current implementation seems to be going through Factory::getUser() in this case which will return a guest user object but I'd like someone to test it for real.
Good question
As far as I can see, the events are only registered when on site or admin app https://github.com/joomla/joomla-cms/blob/4.2-dev/plugins/system/webauthn/src/Extension/Webauthn.php#L166.
The WebAuthn plugin is for authentication, not MFA. I know it doesn't kick in when we have a CLI application.
I said “system plugin”, huh? Sorry, I had my old LoginGuard code in mind. When I converted it to a core feature half of it went into a Trait (\Joomla\CMS\Application\MultiFactorAuthenticationHandler) which is only implemented in SiteApplication and AdministratorApplication and half of it into the plg_user_joomla plugin.
I was mostly worried about what plg_user_joomla does under the CLI application but I do see that I explicitly checked for the administrator and site application.
The other thing we need to check, though, is whether we break anything in the API application for com_users. We do return which users have MFA enabled but as far as I remember we don't actually call any code you touched, we just check the DB data.
@nikosdion could you check if com_users breaks in API? You have here the best knowledge and can test it correctly.
Sorry for the delay, lots of things going on at the same time.
I tested com_users' API with your patch applied, I could not see any problems. In fact, we do not return anything MFA-related in the API.
@nikosdion would be good if you can mark it then as a success test in the issue tracker.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38610.
Done. You may want to add @viocassel's test back manually as the only commits you've made is merging the dev branch.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38610.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38610.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-12-01 10:59:54 |
| Closed_By | ⇒ | obuisard |
I have tested this item✅ successfully on 004882a
? ? All works.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38610.