Multi-Factor Authentication (replaces Two Factor Authentication) #37811
Conversation
Began importing basic plumbing code from LoginGuard
Import backend Models
Import backend Controllers
Import backend Views
Import backend view templates
Shape up the lang strings
Update event names
Little bit of JavaScript
Configuration options
Import TOTP plugin
Yeah, we're removing these from pure class files
Need more coffee
Import YubiKey plugin
Prepare SQL for new plugins
Import Fixed plugin (EXAMPLE)
System plugin
Replace the two factor authentication integration in the core
Fix wrong SQL / table name
Use correct prefix in the TFA helper when getting config UI
Fix a whoopsie or four
Coffee has long stopped working
Format the Methods page
Fix wrong TFA method internal name
Make sure we get the right view in the controllers
Remove yet another integration of the legacy TFA
Automatic migration from old TFA upon first login
Frontend MVC
Frontend routing
Style the method select page
|
Authenticator app - debug?
Tried multiple authenticator apps on my phone and each time the verifcation code is rejected. The time and time zone on my phone are set correctly (its done automatically) Any suggestions how to debug this further |
| <?php else: ?> | ||
| <div class="row mb-3"> | ||
| <?php if ($this->renderOptions['label']): ?> | ||
| <label class="col-sm-3 col-form-label hasTooltip" for="com-users-method-edit-code"> |
There was a problem hiding this comment.
| <label class="col-sm-3 col-form-label hasTooltip" for="com-users-method-edit-code"> | |
| <label class="col-sm-3 col-form-label hasTooltip" for="com-users-method-code"> |
the value of for must match the id
| <?php else: ?> | ||
| <div class="row mb-3"> | ||
| <?php if ($this->renderOptions['label']): ?> | ||
| <label class="col-sm-3 col-form-label hasTooltip" for="com-users-method-edit-code"> |
There was a problem hiding this comment.
| <label class="col-sm-3 col-form-label hasTooltip" for="com-users-method-edit-code"> | |
| <label class="col-sm-3 col-form-label hasTooltip" for="com-users-method-code"> |
the value of for must match the id
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
|
@brianteeman One of your PRs is rejected with prejudice https://github.com/nikosdion/joomla-cms/pull/7 Please refer to the explanation I gave there. What you are proposing is an evidently BAD user experience. Your mental model of what users understand best is diametrically opposite with what dozens of thousands of real world users have demonstrated they understand best over the past 6 years. A user study with real world users on a real world sites is infinitely more important than what one person THINKS it's best. UX is about serving the users, not our preconceptions and prejudices. I am still evaluating the other PR (https://github.com/nikosdion/joomla-cms/pull/8). I do not like this approach. There must be a better way. |
|
lol -if only i was so arrogant to think my code was perfect and could never be improved. You are correct. The ui/ux on your own site for mfa is so bad i can believe that people never see that there are backup codes. But it is awesome to hear thAt you have tens of thousands of users that are using mfa on your site. |
|
@brianteeman Are you for real? The sum total of your experience is doing training for a few hundred people. So let's talk numbers. From 2006 to the present day I have had 100,806 users registering a user account on the site out of which 36,786 have purchased at least one subscription. There are currently 17,172 active users (people who have interacted with the site at least once in the past year). Between 2006 and today I have replied to 37,186 support tickets and an approximately equal number of forum posts, contact requests and other means real world people have contacted me asking for my help. At the time of this writing there are 1,752 users on akeeba.com which use an MFA method. Make that 1,753 because I had to reset someone's MFA three hours ago because they lost their YubiKey and the printout of their backup codes. Over the past 6 years the total number of people on akeeba.com who have used an MFA method is well over 10,000. Your “improvement” was a regression to what the MFA methods management page looked in Akeeba LoginGuard between 2016 and 2017. It was causing a real problem with real world users. Ever since I added this info box the problem disappeared. You are STUBBORNLY insisting that your suggestion, proven wrong in the real world, is right whereas my design, proven better in the real world, is wrong. Who's arrogant now? I am here on a good will gesture, trying to contribute the code I have written and been maintaining for the last 6 years. I have put thousands of hours into it. What I contributed is real world tested. I am not obliged to sit here and take this kind of abuse and attack on my experience and ability to deliver software. Even more so when your PROVEN CATASTROPHIC changes to Joomla are committed without as much as a second look and despite my clear warning that there is a problem. Meanwhile my contributions are met not just with excruciating scrutiny but with outright insults like this. What the heck is your problem with me, huh? Every single time I have warned you that something is wrong and will cause real world problems some arrogant clown who has zero clue of the subject matter tells me I don't know what I am doing, you people know better and so on. EVERY. SINGLE. TIME. I have been proven right!!! EVERY. SINGLE. TIME!!! Do you want me to remind you about the disastrous problems in Joomla Update I warned you about FOUR TIMES between 2020 and August 2021? The problems I was spot on and I even filed a PR to fix your most blatant mistakes which is STILL open since January 2nd, 2022 because you're all too afraid to accept you were wrong? In any case, I have no reason to be here and take this abuse. I am retracting my PR, deleting my branch and revoking my permission to relicense my code from GPLv3 to GPLv2. You are FORBIDDEN from using it until you relicense your own CMS to GPLv3 (which you can't, as you can't get the permission of all code authors, LOL!). If you dare do as much as copy anything from my code I'll sue you to oblivion. I've had enough! |
|
@nikosdion Please don't do that. The Joomla community is more than one single person. |
|
And please guys no swear words. |
|
I am done with this. Joomla should deprecate the bad TFA in Joomla 4 because it's a massive security issue and remove it in Joomla 5. It should also suggest that its users make use of WebAuthn (if you people can figure out how to improve it without copying my code) or a third party TFA/MFA solution. PS: Sorry about the swear word. I will edit and remove it. |
|
Enable the option to onbpoard new users. The reason is that the default permissions for the users component do not allow a manager to access the administrator interface |
|
We all have one aim - to make joomla the best it can be. Thats we work together to try and improve it. Sure we dont always agree on how it can be improved or in which direction those improvements should take. But no one demands anything. I only made suggestions. Some of those suggestions you have accepted. Others not and have demanded empirical evidence such as the skipped headings. That's fine - I am not crying - I just dont agree with you. |
|
@nikosdion Please think it over and restore your branch and reopen this PR. It would be sad to throw away all your work. |
|
My work is far from being thrown away. It's been part of a product called Akeeba LoginGuard which is free of charge and has been around since September 2016. The idea was that I was going to contribute that to Joomla itself. As I have shown in this PR I do not claim that I never make mistakes or know everything. I am open to suggestions which have good, stated reasoning. I will not accept suggestions for what has already been tried and found to be an abject UX failure, no matter what the person making the suggestion believes. Field testing trumps personal belief (not to mention that my PERSONAL belief as to what would have been a better user interface coincides with said suggestion; unfortunately it was proven wrong). My problem is that rejecting a suggestion with an explanation of how field testing already proved it wrong should NEVER be an excuse to be passively-aggressively called a liar and my ability to write software called into question. If anything, I have a track record of brutal honesty and maintaining the top rated extension in JED for 12 years straight. It is very disheartening to see that this kind of toxic behaviour is considered acceptable, if not expected. It's not the first time I have encountered this nor am I the only one. I have talked to other 3PDs who have given up contributing to Joomla and they all had the same horror story to tell. Normalising a toxic contribution environment is the very reason Joomla does not get any new contributors and losing the few it already has. In any case, nobody seems to want to apologise to me for the abuse I received and I'm pretty much told to deal with it. So I am dealing with it by not contributing. I have better things to do with my time. |
|
Nik - I apologize if you were offended by my comment and that you understood my comment in a different way to what was intended. I have to say I felt the same about your comments to me about the headings but I deleted my reply and carried on trying to test this PR because its the best thing for everyone. |
|
@brianteeman Apology accepted and I also apologise for my comments about the headings. I was honestly curious as to what problem occurs and in which tool. I am using VoiceOver. It has the WebRotor which finds and lets you navigate to various places on the page. One of its features is navigating by headings. It announces that the first level is, for example, heading level 3. It's very clear. I even tried using Screen Curtain (which blacks out the screen) and navigate the page only with audio — a feat for me as ADHD and rapid spoken text are naturally born enemies. I had no problem doing that, even on site pages other than MFA. That's why I was perplexed more than anything else about your remarks that a screen reader is not what I should focus about and something would have a problem. I actually went through the entire accessibility documentation for macOS and the only navigation options I found are:
I even tried Safari, Edge and Chrome just in case something renders differently. Nope, they all work the same. I also observed that Joomla itself, both Cassiopeia and Atum, don't have heading continuity. Cassiopeia, for example, has the site top area in a BANNER and the main content area in a MAIN which are announced differently. The sidebar is an ASIDE but not announced differently, it's announced as H3. That was with using structural navigation for WebRotor. When I switched to context navigation which takes into account the rendered layout it was even easier to navigate. I fully understand how the accessibility checkers work, however I am the person who always says that the role of tools is to help us solve real world issues. Being a slave to the tool doesn't improve much. For MFA, the heading levels are going to be arbitrary anyway. Some templates will render the site name inside a BANNER, others will have is as H1. Some templates may even have an H2. Which is the top-level heading we should use in MFA? It should be H1 or H2 in the frontend but which one... I can't know. It's worse for any other frontend component as its top level heading would have to take into account how the HTML before the main content area is structured. The top level heading could be H1, H2, H3 or even H4. You can't really solve that in any way that makes sense and doesn't make the code impossible to read and the component impossible to configure. Based on that, I wanted to know: if we skip a level will the person with disabilities still be able to navigate the site or not? The answers seems to be that yes, they can, there's just a bit of confusion because the first heading level announced is not H1. I hope that now I expanded on it you understand what I was trying to figure out and why I was so insistent on asking what accessibility software other than a screen reader (and the similarly working “joystick control” family of control software, typically for people with motor disabilities) may exist that I don't know of. Anyway, I will reopen this PR when I have some time. Right now I need to do some paid work first as it's been two weeks since I last did that and it's catching up with me :( |
I am not a fan of devloping based on test results either but the reality is that this is something that is immediately flagged as a warning by all testing tools.
This is why the accessibility experts say that we shouldnt try and test screen readers unless we are regular users. For example there are a gazillion hotkeys available to you if you have a screenreader that let you navigate a page in ways that those of us who can see would never imagine. You have to see it in action by an experienced user to understand. For example I can hit a hotkey to list all the links on a page or all the headings. Or even jump to the next heading level etc etc. now that I know about them I really miss not having access to them from my keyboard when I am not running a screen reader which was how I found the skipto script This is one of the reasons thata the generaal advice is to always use headings and to use them in order without skipping a level. As this is something that we can do programatically in many places then we should do it if we can. For example we have already started to modify the code in the article views to change the heading level of the item title programatically depending on if the page title is being displayed or not. |
You underestimate me. I read the documentation and used nothing but the shortcuts.
The first is called WebRotor (VO-U) the latter are the VoiceOver navigation hotkeys (VO-RIGHT, VO-LEFT, VO-SHIFT-RIGHT, VO-SHIFT-LEFT). I also told you, I used the Screen Curtain hotkey to hide the screen. I was only using my ears and my keyboard. I don't do things by half. I am crazy like that. |
|
There are so many many more keys than that. And when you get to other readers like narrator, nvda and jaws then you get even more This article is a good read explaining the issues of testing although its a little outdated. |
|
The link would have helped https://webaim.org/resources/evalquickref/ But also see this from the webaim 2022 (please trust me - if they didnt think it was imporytant then they wouldnt be emphasising itn https://webaim.org/projects/million/#headings |
|
There ARE many more keys. I gave you an example, not a documentation. The documentation can be found at https://support.apple.com/guide/voiceover/welcome/mac For navigating web pages the starting point is https://support.apple.com/en-gb/guide/voiceover/vo27974/mac I briefly talked about the Web Rotor https://support.apple.com/en-gb/guide/voiceover/mchlp2719/10/mac/12.0 and how I navigated using landmarks https://support.apple.com/en-gb/guide/voiceover/vo35709/10/mac/12.0 There is of course Quick Nav which is what you are trying to talk about https://support.apple.com/en-gb/guide/voiceover/vo27943/10/mac/12.0 And that's just one screen reader I happen to be using. In any case, let me tell you something about this:
Congratulations, you are addressing a subcase of a niche case without addressing the problem itself. As I said, one template has the site name / logo inside a banner, the other uses an H1. The top-level heading level on your article would be H1 or H2 respectively. This should ALSO inform the starting heading level in the content which let me remind you is hardcoded HTML in the database. Now let's take into account that the same URL with tmpl=component does not have the site chrome. In the first template there's no change in the headings ranks. In the second template you know need to promote the content's top level rank from H2 to H1. You also need to change the content accordingly. But your content is hardcoded HTML in the database. You cannot change it because it'd break existing sites. Such a breaking change would lead people back to third party content management extensions with all the problems we've encountered. You think that's unlikely to happen? It will happen just using the core: the Agree to ToS plugin would do that, for example. Therefore any automated way to handle this would lead to the same problem if you use a different template than Cassiopeia or tmpl=component. The former is the typical use case of Joomla, the latter is common in the CMS even in core. So this change doesn't actually address any real world problem. It's only meant to have a default Joomla installation satisfy the checklist of an accessibility checking tool so that Joomla can claim it's accessible by design. However, I posit it creates an ever bigger issue. The reality is that the only way to address this problem is having a site integrator who understands how core Joomla and all of its extensions work, curating the content and the template overrides to explicitly avoid this kind of problem. This kind of site wouldn't benefit from your change. However, the average site integrator will now put less effort into accessibility checking because you told them jOoMlA iS aCcEsSiBlE bY dEsIgN. The real solution? Sure, yes, do have the default installation accessible. But also educate the site integrators, explaining that once they start using third party software they need to do accessibility assessments and template overrides. In any case, back to this PR. The reason I was asking all these question is that I know FOR A FACT that I cannot choose the right top-level heading rank for my code as I do NOT know which template it's going to be used in and what is in the module positions the site owner has explicitly allowed in com_users' configuration. If I were to make an accessibility checker happy for the default Joomla installation with Cassiopeia I should be using H1 and H2. However, if the user has installed a different template or used modules which appear in the HTML structure BEFORE my MFA feature's output I can't use H1 and H2, I'd be causing the exact wrong structure problem the article you linked me to is talking about! That's why I chose H3. However, by choosing H3 I now know that the default Joomla installation with Cassiopeia has two skipped heading ranks (H1 and H2). So what I was trying to establish is whether my compromise is actually causing a problem I cannot detect by using a screen reader. This, you have still not answered and it's the only thing I don't know. Everything you shared with me, sure, I know. But that knowledge doesn't answer my question! From my point of view, you seem to only be taking into account the unrepresentative use case of a default Joomla installation or at least a site using Cassiopeia. I am torturing myself with "how I do make something that makes sense for BOTH the default installation / Cassiopeia nobody (except few of us, like my wife and myself) will use in the real world AND the way most people actually use Joomla". It's easy to address an issue for the one use case nobody cares about. It's really effin' hard trying to find a good compromise which works reasonably well across the majority of our users. As you said, we ARE on the same side, we ARE both trying to make it better for everyone. Can you please help me with that objective? That's all I am asking. It's okay to say that you don't know; that's an acceptable answer too. If this is the case we note that as pending a user test from someone who has a visual impairment and uses screen readers as their only way to interact with a site. Fair? |
|
If you remember the point I raised in this pr regarding the skipped heading was that you were skipping a heading in your own code. I agree that its impossible to know what the top heading is etc etc but we cn at least avoid it here. All my comments have been about use within atum. I have not tested anythig at all with this pr and cassiopeia. I am not going to change my opinion about skipped headdings. They are part of the standard tests. People put great value on those test results without looking any further. So if we can fix it (and here it is two seconds to fix) we should. |
|
@brianteeman Yup, I know what you reported and I fixed that skipped heading level immediately upon reading your feedback. That was not put into question. Sorry if there was some misunderstanding about that. Your feedback, however, made me think about whether my compromise about the top level heading would cause a problem or not. That's why I kept nagging you. The way I see it, there are two and a half options about the top level heading:
I personally think that the third (combined) option is the best compromise even though the default frontend with Cassiopeia will be skipping H1 and H2. If you have another practical idea I want to hear it. Does me phrasing it like that make more sense to you? |
What you said at the time suggested you objected to the request to fix the skipped heading. You had already rejected the request for a top level heading on the page in the admin.
With my understanding of what you are clarifying now - it is completely off topic for this PR as you are referring to generic scenarios. Can I suggest that when you are ready to re-open the pr you actually do it as a new pr so it has a cleaner and on-topic comment history |
|
@brianteeman I would appreciate it if you did not lie profusely about what has taken place on the public record. First a screenshot.
Now the links painting this sequence of events which is diametrically opposite to what you allege:
How can you even allege I was objecting to the change of heading order since I had already committed your suggestion — and even carried it forward to the frontend which you had not yet reviewed — before starting the discussion on heading levels?!
Nobody had ever made this kind of suggestion, nor would have it made sense — the front- and backend pages actually use the same templates with very minor changes between them. Same headings are used in both places. What I did reject is adding TOOLBAR BUTTONS instead of inline buttons and that was seven days before the discussion on headings. At no point had you asked to add a heading in any page. You asked about a toolbar button and I explained why it can't be done. At no point did I reject your suggestion to reorder the headings. In fact, I committed it straight away in both front- and backend even though you only suggested one. This is exactly the toxic environment I was talking about. I don't care if you do it on purpose (unlikely) or just because you can't be bothered to test your blatantly wrong recollection against the public record. The fact remains that you are dragging my integrity through the mud when I have NEVER, EVER given anybody an excuse to do so in the 18 years of my career so far. I am deeply insulted by these unfounded allegations. Brian, you're a long-time friend, I appreciate you, but I've had enough of this. If you can't figure out how to communicate without being toxic — regardless of your intention — please stay out of my PRs. Thank you, mate. |
) * Captive TFA Import YubiKey plugin * Captive TFA Prepare SQL for new plugins * Captive TFA Import Fixed plugin (EXAMPLE) * Captive TFA System plugin * Captive TFA Replace the two factor authentication integration in the core * Captive TFA Fix wrong SQL / table name * Captive TFA Use correct prefix in the TFA helper when getting config UI * Captive TFA Fix a whoopsie or four * Captive TFA Coffee has long stopped working * Captive TFA Format the Methods page * Captive TFA Fix wrong TFA method internal name * Captive TFA Make sure we get the right view in the controllers * Captive TFA Remove yet another integration of the legacy TFA * Captive TFA Automatic migration from old TFA upon first login * Captive TFA Frontend MVC * Captive TFA Frontend routing * Captive TFA Style the method select page * Captive TFA Missed a legacy integration which needs removal * Captive TFA Better format of the configuration UI in the profile page * Captive TFA Use language strings when migrating data from legacy TFA * Captive TFA Only show the prompt to add a TFA method if none is already added * Captive TFA YubiKey should allow entry batching This means that you can authenticate with any registered YubiKey in your user profile. * Captive TFA Replace Tfa::triggerEvent * Captive TFA Import WebAuthn plugin * Captive TFA Improve TFA behavior on non-HTML pages. Basically, block them! * Captive TFA Replace alerts with Joomla messages * Captive TFA Move onUserAfterDelete code to the `joomla` user plugin * Captive TFA Remove the System - Two Factor Authentication plugin Use a trait for the application and fold the rest of the code into Joomla's core user plugin. * Captive TFA Remove accidental leftover references to loginguard * Captive TFA Import Code by Email plugin * Captive TFA Post-installation messages * Captive TFA Enable the TFA plugins on NEW installations * Captive TFA XML formatting * Captive TFA Language and grammar in comments * Captive TFA Rearrange XML attributes * Captive TFA Fix typo * Captive TFA Fix wrong language key name * Captive TFA Remove leftover legacy TFA options * Captive TFA Fix wrong CSS class * Captive TFA Merge the padding classes * Captive TFA This lang string should never have had a link * Captive TFA Hide the Key emoji from screen readers * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Accessibility improvements * Captive TFA Use “Two Factor Authentication” / TFA consistently * Captive TFA Tytytytypo * Captive TFA Fixed PHPCS issue unrelated to PR but reported by Drone nonetheless * Captive TFA Lang improvement Co-authored-by: Brian Teeman <brian@teeman.net> * Captive TFA Lang improvement Co-authored-by: Brian Teeman <brian@teeman.net> * Captive TFA Remove no longer valid plugin options * Captive TFA Typo in plugin path * Captive TFA Move TFA options in com_users config next to the password options * Captive TFA Add Show Inline Help button to com_users' options page * Captive TFA Move loading static assets to the view template See #37356 for the reasoning. This should REALLY have been documented somewhere... * Captive TFA Fixed wrong plugin path * Captive TFA Language style guide Co-authored-by: Brian Teeman <brian@teeman.net> * Captive TFA Language style guide * SQL code style and consistency fixes * Add "CAN FAIL" installer hint * Change longtext to mediumtext * Change longtext to mediumtext in update script * No default value for method * Use real null values for last_used * Captive TFA Fix JS linter errors * Captive TFA Fix PHPCS issues after merging @richard67 's PR * Captive TFA Update formatRelative to use JNEVER, simplifying the code in the view templates. * Captive TFA Fix typo * Captive TFA Fix transcription error * Captive TFA Show correct TFA column in the backend Users page * Captive TFA Fix PHPCS errors in UsersModel unrelated to this PR * Captive TFA Add note about supported browsers in TOTP's link * Captive TFA Remove bogus ESLint notice about qrcode * Captive TFA Fix confusing prompt * Captive TFA Consistently change ->qn to ->quoteName * Captive TFA Strict equality check * Captive TFA Move setSiteTemplateStyle to the views * Captive TFA Rename regbackupcodes to regenerateBackupCodes * Captive TFA Rename dontshowthisagain to doNotShowThisAgain * Captive TFA Throw deprecated notices from deprecated methods * Captive TFA Strict comparison * Captive TFA Typo in comment * Captive TFA Rename TwoFactorAuthenticationAware to TwoFactorAuthenticationHandler * Captive TFA Fix comment typo * Captive TFA Remove variables from SQL when not necessary * codestyle changes * Renamed SiteTemplateAware to SiteTemplateTrait Change made against feedback item #37811 (review) for pull request #37811 with title "Improved Two Factor Authentication". Feedback item marked the SiteTemplateAware trait name and had the following content: > Still ...Aware is not a good name for a trait, since it usually denotes interfaces * Remove more instances of "2SV" Per feedback item #37811 (comment) * s/Two Step Verification/Two Step Validation/ Per feedback item #37811 (comment) * Language style Per feedback item #37811 (comment) * Remove unnecessary language string * Remove redundant paragraph tags from PLG_TWOFACTORAUTH_EMAIL_XML_DESCRIPTION Per feedback item #37811 (comment) * Remove redundant paragraph tags from PLG_TWOFACTORAUTH_EMAIL_XML_DESCRIPTION Per feedback item #37811 (comment) The other file with the same language string I forgot to put in the previous commit. * Remove the info tooltip in the methods list Addresses feedback in #37811 (comment) * Simplify the TFA enabled / disabled message Per feedback item #37811 (comment) * Fix layout of backup codes in methods list Per feedback item #37811 (comment) * Fix mail message Per #37811 (comment) * Confirm TFA method deletion Per feedback item #37811 (comment) * Simplify code label in Email plugin Per feedback #37811 (comment) We show short instructions above the field and the field label is simplified. Applied the same change to the Fixed plugin for consistency. * Remove more dead code referencing the legacy TFA * Use concrete events This was the plan all along. Now that #36578 is merged we can FINALLY do it! * WebAuthn support for some Android devices and FIDO keys Backported from #37675 * Rename Tfa to Mfa Ongoing process * Move Joomla\CMS\Event\TwoFactor to Joomla\CMS\Event\MultiFactor Ongoing process * Two Factor Authentication => Multi-factor Authentication Ongoing process * `#__user_tfa` => `#__user_mfa` Ongoing process * twofactorauth => multifactorauth Ongoing process * Change the post-install message Ongoing process * Remove references to “second factor” Ongoing process * Remove the legacy TFA plugins * I missed a few things * I missed a few more things * Wrong redirection from post-installation messages Addresses #37811 (comment) * Fix NotifyActionLog expected event names Addresses feedback item #37811 (comment) * Improve display of Last Used date Addresses feedback item #37811 (comment) * MFA extension helper moves the group to the correct alpha order in the array now that it doesnt begin with T * Remove unused field * Remove no longer used language strings * Undo changes in old SQL scripts * Improve layout and accessibility of the methods list page Based on VoiceOver testing on macOS 12.4 and the feedback from #37811 (comment) and #37811 (comment) * Add missing options to plg_multifactorauth_email * Sort lines alphabetically Why not confuse the translators with out of order labels providing zero context to what they are translating? It's the One True Joomla Way... * Add label to the One Time Emergency Password input Per feedback item #37811 (comment) * Sort lines * Fix PHPCS complaint * Formatting of XML files * Forgot to remove extra CSS class * Apply suggestions from code review Formatting, wrong copyright and version information Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Commit formatting suggestions from code review Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update build/media_source/plg_multifactorauth_webauthn/js/webauthn.es6.js Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Fix update SQL Feedback item #37811 (review) * Onboarding would result in a PHP exception Feedback item #37811 (comment) * Make MFA plugins' publish state consistent between MySQL and PostgreSQL Feedback item #37811 (review) * Update administrator/components/com_users/src/Controller/MethodsController.php Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/mysql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/postgresql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/mysql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/postgresql/4.2.0-2022-05-15.sql Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com> * Update administrator/components/com_admin/sql/updates/postgresql/4.2.0-2022-05-15.sql Co-authored-by: Richard Fath <richard67@users.noreply.github.com> * Restore obsolete language strings Per discussion with @bembelimen I had to rename One Time Emergency Passwords to Backup Codes so as not to make major changes to the obsolete language strings. Having them named One Time Emergency Passwords (OTEPs) was both misleading (they are not passwords, they are second factor authentication codes) and would collide with the `_OTEP_` component of language existing strings. Backup Codes is a good compromise, one that is also field tested for nearly seven years. So, there you go! * Re-add the obsolete plugins' language files Per discussion with @bembelimen Yes, it's pointless, it looks wrong, it is what it is. At least I've put a header that this file needs to be removed. * Remove no longer used twofactor field * Rename CSS class to com-users-profile__multifactor * Update administrator/language/en-GB/plg_multifactorauth_email.sys.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/plg_multifactorauth_email.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/plg_multifactorauth_email.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Update administrator/language/en-GB/com_users.ini Co-authored-by: Brian Teeman <brian@teeman.net> * Accessibility improvement * Improve language * Change the heading level * Fix case of extension registry file Regression after renaming TFA to MFA * Remove accidental double space after echo * Remove BS3 leftovers * Remove BS3 leftovers * Remove BS3 leftovers * Update administrator/components/com_users/tmpl/methods/list.php Co-authored-by: Brian Teeman <brian@teeman.net> * Update components/com_users/tmpl/methods/list.php Co-authored-by: Brian Teeman <brian@teeman.net> * PHP warnings when there are no MFA plugins enabled * MFA onboarding was shown with no MFA plugins enabled * Backup codes alert is narrower than page on super-wide screens * Backup codes alert heading font size fix in backend * Revert wording for JENFORCE_2FA_REDIRECT_MESSAGE * Backend users without `core.manage` on com_users were blocked They were blocked from setting up / manage their on MFA, blocked from the onboarding page and blocked from the captive login page. * Onboarding in backend shouldn't have a Back button * Improve layout of method add/edit page * Remove unnecessary H5 tag from TOTP setup table * Kill that bloody Back button with fire * MFA WebAuthn: use Joomla.Text instead of Joomla.JText * MFA WebAuthn: show meaningful error on HTTP * MFA Email: more sensible email body * MFA WebAuthn: must be able to edit the title * MFA add/edit: remove placeholders, replace with help text * Heading levels We assume an H1 will already be output on the page. This is always true on Atum and never true on Cassiopeia — but very likely on real world sites's frontend templates. So it's a compromise which is at least better than the previous case of starting at h3 or h4. * Editing a user would show the wrong interface When editing a user other than ourselves we need to show the MFA editing interface for the user being edited, not the MFA editing interface for our own user. * Refactor security checks Now they are conforming to the original intention * Add missing Group By to the SQL query * Show MFA enabled when a legacy method is enabled * Users: filter by MFA status * Language clarification * Move the frontend onboarding page header to the top * User Options language clarification * PostgreSQL installation SQL wasn't updated * Adding periods to the end of lines of error messages you will never, ever see * Remove a tab * Remove another tab from a comment * Typo removing junk * Remove useless imports * Busywork * Typo in the INI file * Align comment * Remove redundant SQL for PostgreSQL * Typo in labels' `for` attribute * Move backup codes to the top of the page * Mandatory and forbidden MFA was not taken into account If only one group matched, due to typo. * Show information when MFA is mandatory * Make the buttons smaller * The secondary button looks horrid in the frontend * Redirect users to login page in the frontend When they try to access a captive or methods / method page. * MFA Email: fallback to standard mailer when the mail template isn't installed * Delete backup codes when the last MFA method is deleted * Use text inputs for TOTP With the correct input box attributes * Fix the buttons for WebAuthn * Clarify language strings * Use toolbar buttons in the backend Except for screen size small and extra small. Over there we ALSO display the inline content buttons because the toolbar buttons are hidden behind an unintuitive gears icon. JUST BECAUSE THE DEFAULT JOOMLA WAY IS TO USE A TOOLBAR IT DOES NOT MEAN THAT IT MAKES SENSE ALWAYS, EVERYWHERE. THE USER IS KING. WE SERVE THE USER, NOT OURSELVES! * Change the icon classes * Forgot to copy over the changes to the frontend * Regression: configure existing authenticators We used to set field_type to custom to make the code entry disappear. After the changes to the field type handling we need to instead set input_type to hidden. * Backup codes should never become the default method automatically * Improve methods list layout Now it is more clear which methods are enabled and which are available. * Use toolbar buttons in backend pages Except when the screen size is extra small which is the point where the toolbar is hidden and the interface becomes unintuitive. * Fix return URLs for backend MFA edit pages * Edit / Delete buttons mention the auth method name in the respective button's visually hidden text * RTL aware back buttons * Consistent use of the term Fixed Code * Fix typo Co-authored-by: Brian Teeman <brian@teeman.net> Co-authored-by: Richard Fath <richard67@users.noreply.github.com> Co-authored-by: heelc29 <66922325+heelc29@users.noreply.github.com>
Improved Two Factor Authentication
ATTENTION TESTERS!!!
If you had tested this PR when it was still called Two Factor Authentication you will need to follow these steps:
composer installnpm ciadministrator/cache/autoload_psr4.phpThe latter step is extremely important as all the plugin names have changed, including the internal Joomla extension names.
Executive Summary
This PR implements Multi-Factor Authentication (MFA) using a “Captive Login” approach.
Unlike the temporary Two Factor Authentication solution we've been using until today, it does NOT require you to enter your Security Code with your login information (e.g. username and password). In fact, there is no longer a Security Code field at all in the login module or page. Instead, you login using whichever login method you want to use. Then you enter the “captive login” mode in Joomla where you can only interact with a subset of com_users: you can proceed with MFA validation, choose a different MFA method or log out. Non-HTML views are forbidden.
Tagging @SniperSister @brianteeman @richard67 who were involved in the original discussion in the context of improving Joomla's security, @zero-24 @bembelimen @roland-d @HLeithner as the current and future release leads that need to be in the loop, @laoneo for his keen eye and good suggestions and @rdeutz because he needs to know about flagship features. I don't know if @nibra is interested in the architectural aspect — I tried to stay as true to our August 2015 decision on orthogonality of new features as plausible for this kind of feature (at least you get a Trait instead of Yet Another System Plugin). I hope I am not forgetting anyone. Feel free to tag anyone else who needs to be kept in the loop.
Draft PR
This is a Draft PR. The intention here is for all CMS stakeholders to go through the code AND the process (not to mention the language — Brian, I am counting on you, man), have a discussion about everything I didn't think to put here in writing, things like that. Basically, figure out how to shape this up for prime time hopefully before 4.2 betas start rolling out.
There are also a few less important missing bits and ends, mainly on the code polishing side of things, which I document at the bottom of this PR's description. They do not detract from testing the process of Two Factor Authentication so I decided to go through with the Draft PR anyway.
Extra features implemented and considerations made
I implemented a few extra features to the old TFA since I already had the code for them and it'd be a waste to remove it only for someone else to try and replicate them in the future. I also tried to be very considerate of the security of the solution, keeping it in a fine balance with practicality. Without further ado, here's what I did.
Multiple MFA methods per user. There is no good reason why a user shouldn't have multiple MFA methods. In fact, it is a better approach e.g. having two WebAuthn keys (main and backup) and a classic six-digit code for use in an older smartphone which doesn't support WebAuthn dongles. This makes MFA more usable and more resilient to accidents that lock people out of their site.
Default method. If you have multiple authentication methods you don't want to stop and think which method you will be using to authenticate today, especially since you're likely to use the same method 99% of the time. Therefore you can pick which method will be your default. You can NOT use emergency codes as the default method to reduce the probability of silly mistakes.
Methods batching. Let's say you set up three WebAuthn authenticators. When you log in you want to see a single page asking you for a WebAuthn login, not having to click to select which WebAuthn authenticator you will be using (reduces the PICNIC errors — Problem In Chair, Not In Computer). I call that method batching because you can batch process all instances of a MFA method using a single input. WebAuthn and YubiKey let you set up multiple instances of that method so, indeed, they support method batching. Regular Authentication Code and Authentication Code by Email only allow a single instance so they don't. As simple as that.
Automatic migration of legacy data. To support multiple MFA methods per user we moved the MFA data from the two
#__userscolumns (otpKeyandotep) to its own table (#__user_tfa). Any already set up legacy TFA method is automatically migrated upon first login. This means that MFA will migrate cleanly even on sites with hundreds of thousands of users, something which would NOT be the case had we performed the migration during update (we'd timeout as we have to load each record individually, decrypt it, convert it, re-encrypt it and save it). Important: Until a user goes through a login you will NOT see their TFA/MFA status or configuration settings in com_users in the backend.Data is encrypted. As alluded above, the MFA configuration data is still encrypted using AES-256 using a key derived from the site's
secret. Therefore a simple SQLi vuln in any extension won't divulge the secrets for MFA.New MFA methods. You can now use Authentication Code by Email and WebAuth on top of the existing Authentication Code and YubiKey methods. The former sends you a 6-digit code to your email and can also be set up to be forcibly enabled for anyone (to provide a viable backup if your users are not very tech savvy). The latter supports Windows Hello and Android, bringing in the improvements from my other PR (#37675).
Forced MFA and Forbidden MFA groups. You can optionally set up certain user groups to have MFA forcibly enabled and other groups to have MFA forcibly denied. The former will have to set up MFA and use it to continue using the site. The latter will never be asked to go through MFA, even if they had previously enabled it, nor will they see the MFA configuration section in
com_users.Onboarding to drive adoption. You can optionally have users (who do not belong in the forbidden MFA groups!) be automatically shown an onboarding page if they log in and they have not yet enabled MFA. They can set up MFA, go to a different page or forever dismiss the onboarding page. You can also customise the onboarding URL, e.g. if you want to display your own article instead of showing the default MFA setup page. According to my experience this increases MFA adoption by a factor of ten. Note that onboarding is DISABLED by default; we don't want people coming at us with torches and pitchforks for ruining their site's login experience.
Super Users cannot edit other Super Users. As a Super User you cannot touch another Super User's MFA settings at all. Consequently, if you mess up your MFA as a Super User you need to do some database editing. There's only so much we can do.
Administrators can only remove TFA options for other users. As a privileged user you can only remove MFA options from other (non-Super Users) user accounts, or disable their MFA completely. You cannot edit their TFA configuration or enrol new MFA methods.
Single click disable. The MFA configuration interface includes a Turn Off button which disables MFA completely for your user account. This is useful if you lost access to all MFA methods, you have used an emergency code and you just want to turn the darned thing off until you sort out your other problems.
Post-installation message. The old PIM for MFA has been removed and a new one has taken its place.
Module positions customisation in the captive page. You can choose which module positions to display in the captive MFA page. This lets you display, for example, your site's header and footer to give users a better visual experience.
No MFA on silent logins. By default, MFA is disabled on silent logins. You can toggle that as well as customise which login methods are to be considered a "silent" log in. By default these are the Cookie Authentication (remember me) and Passwordless (WebAuthn). The latter needs some explaining. WebAuthn is phishing-resistant and insanely secure — we only ever store a public key in the database, even if it's stolen it cannot be used to log you in (a stolen password hash can be brute forced MUCH more easily, by several orders of magnitude). This means that WebAuthn is strong enough to NOT require Multi-factor Authentication. That's the whole point of WebAuthn; replace usernames and passwords and the need for multifactor authentication with One Authentication Method To Rule Them All And In Security Bind Them.
Implementation notes
This PR is based on my past work on Akeeba LoginGuard, the first practical Two Step Verification solution for Joomla, which had been in active development since 2016. That component had begun as a proof of concept for the Captive Login concept as I had been told repeatedly until that point in time it's not possible in Joomla. Not only is it possible, after proving it feasible Joomla 4 is now already using it in several places (“agree to terms” and “requires password reset”). Meanwhile, I had been actively developing that extension with an eye on contributing it back to Joomla since this is the Two Factor Authentication we had promised to implement nearly ten years ago. What we still have was meant to be a “temporary solution, for 6 to 12 months tops, until we release Joomla 4 with a REAL Two Factor Authentication with a captive login”. Well, better late than never, right?!
This being a core PR instead of an extension I no longer have to do weird bollocks to make the captive login work, using a system plugin, string and duct tape to hold it all together. I also didn't want to repeat the past mistakes of sprinkling magic code all over the darned place to integrate MFA to Joomla. So I used a Trait instead. Only the SiteApplication and AdministratorApplication use it. This is deliberate.
The CLI application does not have any authentication to speak of, nor it needs to. If you are in a position to either access a shell or define CRON jobs you have more control on the hosting environment than a Super User.
The Api application on the other hand is non-interactive. That's why it has a token authentication instead of relying on a username and password (see #27021 and the handy comic panel that drives the point home in #26925).
So, if you end up writing an interactive custom application you can use the Trait. If not, it's cool, DON'T integrate MFA — you actually MUST NOT.
Missing odds and ends
I have not (yet?) integrated this with Joomla's Action Logs. This is deliberate as there's currently an open PR (#37788) touching it and I don't want to create a Merge Conflict Hell.
Moreover, you will see that the MFA plugins are doing a couple of things which can be improved:
They are usingWe are not using the database object directly in the plugins anymore.$dbinstead of the spanking newJoomla\Database\DatabaseAwareTrait. I will work on that before making the PR final.They are usingThere is no ApplicationAware interface so I'm sticking with the CMSPlugin magic for now.$appinstead of injecting the application. Also something to be worked on before the final PR.There is a repeatedFixed now that the PR is merged; we are using concrete events which are results aware.setResultmethod in them. That's temporary, until Work towards using concrete events everywhere maintaining b/c #36578 is merged and I can use the event'ssetResultmethod as I originally intended. Until then, I needed something that works. Hence the method in question.Remember the WebAuthn claim for Windows Hello and Android compatibility? Yeah, I still need to pull that code in.Already addressed.If you bump into one of this, yup, I know.
PS: Brave adventurer, beware! There be dragons! I crammed a month's work into four days. I'd be mighty surprised if there are no bugs.