[#40555] - [4.3] Fix submit button focus on frontend MFA captive login page when using Webauthn
- Fixed in Code Base
- 19 May 2023
- Medium
- Build: 4.3-dev
- # 40555
- Diff
- richard67:4.3-dev-fix-multifactorauth-captive-focus-for-site-2
User tests: Successful: Unsuccessful:
Pull Request for #40330 (comment) .
Summary of Changes
When working on my pull request (PR) #40330 I've noticed that when using MFA at the site (frontend) login and the currently active MFA method is Webauthn, the javascript "com_users/js/two-factor-focus" is not loaded, and so the submit button is not activated, but when doing the same at the admin (backend) login, the script is loaded so the button is activated.
The reason for this is the if condition for loading the javascript in the site captive login here: https://github.com/richard67/joomla-cms/blob/4.3-dev/components/com_users/tmpl/captive/default.php#L25-L28
We do not have this condition for the admin captive login page: https://github.com/richard67/joomla-cms/blob/4.3-dev/administrator/components/com_users/tmpl/captive/default.php#L25-L26
This PR here removes this condition in file components/com_users/tmpl/captive/default.php and does a small code style fix in addition in that file.
In addition it fixes the two-factor-focus.es6.js so that it works with the above change. This basically means a revert of my PR #40330 , which has fixed the issue handled by that PR but broke the thing handled with this PR here, and replaces that wrong fix by the right one, using optional chaining as recommended by @nikosdion during review of this PR here, thanks for that.
Testing Instructions
First test without this PR applied to reproduce the issue.
After applying the changes from this PR, run npm ci or npm run build:js when on a development environment.
When on a regular installation where you have no npm, use the update package or the custom update URL created by drone for this PR.
After having applied the changes, clear your browser cache to make sure you are not testing the old, unmodified javascript.
- Create a user account with MFA active who can login to backend and frontend.
The user should have activated at least one MFA method which requires to enter a numeric verification code, e.g. TOTP, and another one which doesn't require such a code (Webauthn e.g with Windows Hello). - Login to the frontend with this account with username and password.
- Check if depending on the MFA method, the input field for the verification code or the button to validate with the authenticator has focus.
Use the "Select a different method" button in the toolbar and check that for each method.
Watch the browser console with the developer tools of your browser to see if there are javascript errors.
Result: See section "Actual result BEFORE applying this Pull Request" without the PR, and "Expected result AFTER applying this Pull Request" with the PR. - Enter a verification code if necessary and use the validation button to finally log in.
- Login to the backend with this account with username and password.
- Check if depending on the MFA method, the input field for the verification code or the button to validate with the authenticator has focus.
Use the "Select a different method" button in the toolbar and check that for each method.
Watch the browser console with the developer tools of your browser to see if there are javascript errors.
Result: The MFA captive login page for the backend works with this PR on the same way as without this PR, i.e. nothing is broken by this PR, no javascript errors related to the "two-factor-focus" script in both cases. - Enter a verification code if necessary and use the validation button to finally log in.
Actual result BEFORE applying this Pull Request
On the MFA captive login page for the frontend:
- When an MFA method is used which does not require to enter a numeric verification code, i.e. webauthn, no button has focus.
- When an MFA method is used which requires to enter a numeric verification code, e.g. TOTP or backup codes, the field to enter the verification code has focus, i.e. this works as expected.
On the MFA captive login page for the backend everything works as expected:
- When an MFA method is used which does not require to enter a numeric verification code, i.e. webauthn, the button to validate with the authenticator has focus.
- When an MFA method is used which requires to enter a numeric verification code, e.g. TOTP or backup codes, the field to enter the verification code has focus.
There are no javascript errors in the browser console related to the "two-factor-focus" script.
Expected result AFTER applying this Pull Request
On the MFA captive login page for the frontend:
- When an MFA method is used which does not require to enter a numeric verification code, i.e. webauthn, the Submit button has focus.
- When an MFA method is used which requires to enter a numeric verification code, e.g. TOTP or backup codes, everything works as before applying this pull request, i.e. the field to enter the verification code has focus.
On the MFA captive login page for the backend: Everything works as well as before applying this pull request.
There are no javascript errors in the browser console related to the "two-factor-focus" script.
Link to documentations
Please select:
-
No documentation changes for docs.joomla.org needed
-
No documentation changes for manual.joomla.org needed
joomla-cms-bot
- | Category | ⇒ | JavaScript Repository NPM Change Front End com_users |
| Status | New | ⇒ | Pending |
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40555.
@richard67 You asked me this:
It would be more helpful if you would answer my question why there is that difference between the admin and the site php files which I was asking about.
I tried diplomatically avoid answering this because some people say in bad faith that I am difficult to work with. But if you insist, here's the whole story.
Between 2016 and 2022 I developed and made available free of charge my Akeeba LoginGuard extension for Joomla. This was the answer to everyone who was saying that captive MFA is impossible in Joomla — something I had been hearing since 2012 when I contributed the original TFA as a temporary solution, not to be used for more than a year, with the plan all along being captive MFA. I contributed this code last year as Joomla's MFA feature, after the project finally realised that the temporary TFA solution was not all that secure, as I had warned a decade ago and dismissed ever since. Yay for common sense.
Akeeba LoginGuard used the exact same view templates in both the front- and the backend, and for good reasons: 1. a user MUST NOT be expected to log in in two completely different ways depending on whether they are in the front- or the backend of the site; and 2. if the interface is not in the main content area users are confused and start looking around the page in desperation, not knowing what to do next.
However, my PR of a real world proven solution was not going to be accepted unless the backend MFA was made substantially different than the frontend MFA, moving the action button to the bloody toolbar. I vehemently disagree with this approach, it is proven to confuse users, and leads to lower adoption of MFA in Joomla. But I can not merge my own PRs. Therefore, I had to make two different view templates for the front- and backend.
This was asked for in literally the eleventh hour so I hacked it together as best as I could. I didn't have time to do extensive testing, or we'd miss the Joomla! 4.2 merge window, the feature would languish in “PR Hell” and you'd never get MFA in Joomla… It was supposedly tested by the people who asked for this change and approved for merging by one of the maintainers. It does work, minus the focusing of the button in the frontend which is a regression to the original behaviour brought about by the hasty change in view templates and the JavaScript.
If you ask me what should be done, I will reiterate that the toolbar button is a really bad idea and must be removed. If it's removed, the JS can be simplified and the two view templates be the same. Exactly as it was for the 7 years people were using it under the name Akeeba LoginGuard.
If you do not want to remove the toolbar button we just need to look for both the toolbar button and the inline button by element ID, then run .focus() on them if they are defined in the order inline, toolbar.
@nikosdion
Can you have a look at issue #40428, please? It's also about MFA login.
I've added a hint to the testing instructions about the ongoing review so people don't waste time with testing because the PR will receive changes. I haven't set it to draft because I don't want to keep people from reviewing. Thanks so far for all feedback, I've learned new things and will implement changes later today after my paid job.
| Labels |
Added:
NPM Resource Changed
bug
PR-4.3-dev
|
||
@nikosdion I've implemented all your suggestions and updated the description accordingly. Thanks for the review.
@dautrich Could you redo your test with the latest changes? The testing instructions are still the same. The link to the update package and custom update URL is https://ci.joomla.org/artifacts/joomla/joomla-cms/4.3-dev/40555/downloads/65470/ . Thanks in advance.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40555.
@obuisard Why have you set the "Updates requested" label 27 minutes ago? I have implemented all suggestions from Nik 3 days ago.
My bad, I had this PR open in my browser and the page was not updated with the latest comments. I apologize Richard @richard67
I have tested this item
Works as expected!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40555.
HLeithner
- | Status | Pending | ⇒ | Fixed in Code Base |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-05-19 23:24:10 |
| Closed_By | ⇒ | HLeithner |
@nikosdion Hi Nik, could you check this PR and give a quick feedback? Maybe that difference between the admin and the site login captive pages has a reason of which I am just not aware?