cc @HLeithner @wilsonge @C-Lodder

When I contributed the Web Authentication plugin I added the onUserLoginButtons plugin event with the explicit and documented purpose of the core and 3PDs being able to provide additional login buttons to the login module and login form. The idea is that the world is moving away from per-site password authentication and into passwordless authentication and Single Sign On (SSO). The contributed code was explicitly stated to allow 3PDs to provide SSO implementations without having to provide custom login modules and template overrides to com_users.

We had already learned a lesson from Joomla 3.2 and the TFA implementation being hardcoded in com_users and mod_login. Therefore I architectured that code in a way that decouples the non-password login button definitions from their rendering.

The plugins return an array of information which can be used to render one or more buttons through the onUserLoginButtons plugin event. This information is aggregated through AuthenticationHelper::getLoginButtons which is called by the login module or com_users login form. Said login module / form then renders the buttons using code that is appropriate for the context. Code that is generically appropriate for ANY kind of button, not just the WebAuthn one.

Imagine my surprise when I saw that 9d3f7aa has completely reverted this login and made it only possible to render ONE type of login button, the WebAuthn one. It's not @C-Lodder fault. He's not a backend developer and he didn't know the intended architecture. It shouldn't have been merged, though. Supposedly other people reviewed the code. The problem is that nobody saw that the PR broke the architecture which is a failure of the Joomla PR testing model.

I do understand that the change was made for the purpose of sidestepping the onclick handled which is incompatible with CSP. However, it was done in an architecturally incorrect way.

Here is how it should have been done in an architecturally correct manner:

First, AuthenticationHelper::getLoginButtons needs two changes:

1. Line 144 should also allow any key starting with data-* (not unset it)
2. Line 151 should check if there is any data-* attribute and not run the continue line in this case.

This allows passing buttons with data attributes which can work around onclick being forbidden by CSP.

Frontend mod_login, line 112-113 must be removed. Instead, iterate through all the data attributes and emit them verbatim.

Line 120 must NOT use a hardcoded name, for crying out loud! We have the 'title' key in the returned button definition.

The same changes need to be done in the backend mod_login.

The same changes need to be done in the backend com_users, default_login.php template.

In fact, I'd say that rendering additional buttons in login forms should be a layout.

I will start writing a PR about it instead of working on the other issues I have already filed because this issue broke the architecture we had agreed upon. If my PR is not accepted I will have to ask you to remove my WebAuthn plugin instead because I contributed it under the explicit agreement that Joomla would facilitate any 3PD to write an SSO solution, the WebAuthn plugin serving as an example – not as the only hardcoded option.

PS: I am sorry I didn't have the time to go through the merged code that broke the behavior. Unfortunately, I was stuck at home with a toddler and limited time due to the pandemic. I only returned to a semblance of normalcy two weeks ago. I am trying to go through every aspect of J4 I didn't have the time to before, find issues and fix them. This is a big one so it takes priority.