Only show default plugin when TFA disabled and hide skip validation attempts if disabled

Merge request for 2.x is up. Patch for 1.x is attached.

In my opinion only the default validation plugin should be visible if the user doesn't have TFA set up.

What if the user wishes to use a different plugin? I know in 1.0.0 default plugin was treated as "must be configured" however my recolection is we have mitigated most of that in the string of security releases since 1.0.0 and now work with it not being required, if not that is the direction we are currently trending with the design.

On our site, users are clicking the link to set up recovery codes without first setting up TOTP (which is our default plugin), which results in invalid recovery codes and not-so-happy users.

That may be the root issues we need to fix if that is the case.

At the moment I don't recall exactly how we process "No tfa plugins configured, but recovery codes configured" although I believe we allowed this already (especially since this would have been part of the method for dealing with one of the previous security releases we pushed).

Can you clarify what happens from your testing?

Hide skipped validation attempt information/actions if the user doesn't have TFA enabled.

In a way we actually would want the opposite, TFA skip counts only impact users when TFA is disabled and is required. Once a user enables TFA my recollection from working on previous security issues is that TFA now becomes mandatory (no skips allowed) at least in 1.x, and I believe believe we ported the same into 2.x already too.

Hi @cmlara, thank you for replying so quickly to this issue!

Answers to your feedback:

What if the user wishes to use a different plugin?

I definitely don't think we should force site owners to use one validation plugin over another. Rather, the idea here is to require end users to configure the default validation plugin for their accounts before they can configure secondary plugins. For us, that would mean requiring users to configure TOTP before they can configure recovery codes.

Can you clarify what happens from your testing?

I think I misspoke in the issue summary. Instead of creating invalid codes as I originally thought, it seems one of two things is happening:

1. End users are creating recovery codes and not setting up TOTP
2. End users are creating recovery codes first, then setting up TOTP, which means their default authentication method is recovery codes

In either situation, once they run out of recovery codes, they reach out to us for support thinking the functionality is broken. We could simply disable the recovery code validation plugin, but so far our leadership doesn't want to do that. So part of the motivation behind this change would be to steer end users to the default validation plugin first before allowing them to set up alternate validation methods.

In a way we actually would want the opposite, TFA skip counts only impact users when TFA is not configured for a user(disabled for the user) and is required for the role.

Site owners can configure the number of times end users can skip validation via the "Skip Validation" setting. I've thought about it a bit more and I think you're right, hiding this information would be less than ideal because it could remove urgency on the part of the end user in terms of needing to get TFA set up. So I think I'll update the merge request to remove that part. Once that's done I'll make sure to update the issue summary.

Rather, the idea here is to require end users to configure the default validation plugin for their accounts before they can configure secondary plugins.

I'm inclined to say that is a bit presumptuous from us as a module in the site owner -> user relationship. I'm going to refrence #3323276: Seems the default verification method must be setup → as the original issue it links to actually ended up being released as sa-contrib-2023-030 . I do not want us move back towards insisting that the 'default' plugin is configured.

📌 Convert plugin configuration to Config Entities Active might start laying the ground work for removing the concept of "default" allowing us to move more towards an ordered (weighted) list where we can respect site owner preference. I recall a request for us to allow users to specify their default plugin however I can not find the issue at this time.

End users are creating recovery codes first, then setting up TOTP, which means their default authentication method is recovery codes

This would not override the site owner preference for default, the user would still be prompted to use TOTP and would have to select recovery codes.

End users are creating recovery codes and not setting up TOTP

That sounds very possible. If that is the case I wonder if we need to introduce the concept that "Recovery" tokens do not satisfy "User has configured TFA" which becomes relevant for ✨ Force user to setup TFA when required and there are no remaining skips Needs work and ✨ Redirect to validation setup after login without tfa Needs work . This would be significantly more involved code wise than "just show the default plugin".

Alternatively maybe we can expose extra data out in some manner, possibly via the Recovery Plugin emitting a warning on login when remaining login counts are low or possibly an ECA plugin that invokes when recovery codes are low. Both of these would likely be much easier to implement. Neither of these match your initial request however is it possible they could provide enough notification to avoid the support inquires?

A possible option, though I foresee it too having concerns that would need to be addressed, is that instead of only showing the default plugin we instead do not show recovery plugins until a 'primary' plugin is configured. Similar concept to your original suggestion with just more options available.

At least one edge case I can think of off hand however is all of the users primary plugin types are disabled by an admin a user could be left in a spot where they can not add more recovery codes to keep login working, which poses an issue if the user is unable to provision a new token (for example the primary tokens are physically mailed to a user).

Ah, I didn't realize that was the case. I certainly understand where you're coming from given that.

I like the idea of setting recovery codes as something that doesn't satisfy "User has configured TFA". Although I can see how it'd be more involved than other options, I think it might be the most satisfactory option. One other change I'd suggest is that we at least consider listing the default validation plugin first. In our case this would place TOTP above recovery codes and would alleviate most of the issues.

If this sounds good to you, I'd be happy to give it a try to make the changes.

If this sounds good to you, I'd be happy to give it a try to make the changes.

Overall sounds good.

One other change I'd suggest is that we at least consider listing the default validation plugin first. In our case this would place TOTP above recovery codes and would alleviate most of the issues.

This sounds reasonable. It may go away in the future if we move to ordered list, but until such a time such code happens it sounds like a reasonable change.