Implement Drupal\user\UserAuthenticationInterface

Until Drupal 12 is released contrib modules need to accept both interfaces and test which they have been provided. See 🐛 BC break in login auth changes from #3444978 Fixed comments #22 and #24.

Mail_login should indicate it is only compatible with Drupal12+ if it accepts only a single interface otherwise it needs to accept both. You likely will want to raise a bug report in mail_login to inform them of this issue.

Setting to a task as we will eventually need to do this. Moving to needs-work as D.O. has fully moved over to an MR only workflow, patches can not be tested only MR’s.

Thanks for your quick reply and detailed summary on the topic.

I assign the ticket to myself, and I will create an MR soon.
I will take a look what else can be done in mail_login and will create the corresponding ticket if necessary.

I tried to address the same issue in mail_login.

Unfortunately the old interface works like a wrapper for the new interface, therefore without applying this patch I got an other error:
Error: Call to undefined method Drupal\tfa\TfaUserAuth::authenticateAccount() in Drupal\mail_login\AuthDecorator->authenticateAccount() (line 137 of modules/contrib/mail_login/src/AuthDecorator.php).

In addition to implementing the interface, I also refactored the authentication logic into the new method, because at some point the old method is going to be removed.

I created the MR.

I Unassign and set status to Needs review.

From the MR

UserAuthenticationInterface::lookupAccount() does not exist in decorators not implementing UserAuthenticationInterface.

Core's user login form assumes that if the User Auth service implements UserAuthenticationInterface it is safe to call lookupAccount.
This works fine when you have only the two sceanrios:

• Core Login Form -> Core UserAuth service (implements UserAuthenticationInterface)
• Core Login Form -> Decorator1(UserAuthenticationInterface) -> Core UserAuth(UserAuthenticationInterface)
• Core Login Form -> Decorator1(UserAuthenticationInterface) -> Decorator2(UserAuthenticationInterface) -> Core UserAuth(UserAuthenticationInterface)

This logic however fails when you have

• Core Login Form -> Decorator1(UserAuthenticationInterface) -> Decorator2(UserAuthInterface) -> Core UserAuth(UserAuthenticationInterface)
• Core Login Form -> Decorator1(UserAuthenticationInterface) -> Decorator2(UserAuthInterface) -> Decorator3(UserAuthenticationInterface) -> Core UserAuth(UserAuthenticationInterface)

The outcomes in this scenario are that core calls Decorator1 and:

• Decorator 1 calls non-existent method on Decorator2 (throws an exception) or
• Decorator 1 attempts to resolve the data itself using Cores logic ( this is a breach of decorator design, and would mask that Decorator3 is being ignored)

I will note the first scenario is essentialy what you describe in #6, where mail_login was re-written to (incorrectly) assume every decorator will already implement UserAuthenticationInterface prior to D12, possibly because core made the same assumption.

On a cursory glance I believe the only way we can use UserAuthenticationInterface is if we are on D12, OR we have a setting that manually enables the use of a new class when a site owner ensures 100% of their stack supports the UserAuthenticationInterface (in which case it is safe for exceptions to be thrown).

Drupal Core tried to do this in a manner to retain BC with minimal immediate contrib breakage and ended up creating a scenario that the probability of site breakage increased each day the commit remained in the repository by pushing design failures onto the contrib ecosystem. When we look at the entire issue as a whole we are essentially left with a non BC capable API addition that would have been easier to implement if it was mandatory.

I was able to solve the issue in mail_login code, and created the ticket for it as well.
https://www.drupal.org/project/mail_login/issues/3497643 🐛 Backward compatibility fix for UserAuthenticationInterface Needs work