UserAuth BC layer is not working for modules that use it to provide email based logins

catch → changed the visibility of the branch 3444978-userauth-bc-layer to hidden.

Short on time but did the first bit.

Adding a new class and deprecating the entire UserAuth class sounds good, ideally we'd land this before the beta release but that's not long, however a deprecation during beta is better than completely failed logins.

catch → changed the visibility of the branch 3444978-userauth-bc-layer to active.

Did the second step, and a separate 11.x to match the 10.4.x/10.3.x changes.

catch → changed the visibility of the branch 3444978-userauth-bc-layer to hidden.

Realised we don't actually want separate 10.x and 11.x MRs because the deprecations are for removal in 12.x meaning all the same code is in 11.x too, so hid the 11.x MR.

Can we deprecate in 10.3 but add to 10.4?

@smustgrave not sure what you mean here, this is fixing a bc layer we already added to 10.3 so it (hopefully) works for real contrib modules.

It's a bit of a strange one because we're providing bc for the user login form behaviour, which normally we don't provide bc for form behaviour, but because it's the user login form and there isn't a proper API for this stuff, lots of modules actually interact with it. However the previous issue adds an API which contrib modules can implement, and that should hopefully make bc easier to implement if we change anything else in the future.

Once the bc layer works, we can leave it as-is in 10.x and 11.x, and then just drop it in 12.x

What I mean is we are adding something to 10.4.x first it seems but saying it was already added/deprecated in 10.3.x

The same MR will need to go into 11.x, 10.4.x and 10.3.x

That was my only question.

This is not yet changing UserAuth to not implement the new interface nor is it deprecating it. And it's not yet updating the UserLoginForm (and UserAuthController I guess).

I'm not sure that #17 is a review of the correct MR, it does all those things except UserController.

I think I clicked on the wrong MR link, was on my phone, sorry :)

Changes look good, I manually confirmed that this fixes the test fail I had in my project.

Also confirmed that the code flow in UserAuthenticationController wasn't changed and should also be OK. That said, I think some of the flood related optimizations were apparently not applied here, flood control still does an extra username-based lookup here, which also means that it doesn't support flood control for an account returned by \Drupal\user\UserAuthenticationInterface::lookupAccount(). But that's not a regression and can be looked at in another issue, possibly the one that does the work to unify user login flood control.

Committed and pushed 26208e91c0 to 11.x and 079b65a232 to 11.0.x and 517e75283e to 10.4.x and 77d19f7abb to 10.3.x. Thanks!

longwave → closed merge request !7922

Automatically closed - issue fixed for 2 weeks with no activity.

Opened 🐛 UserAuth BC layer not working for modules that use username Active as this change appears to have broken the login form when a decorator of the UserAuth services uses real account names that exist (the authenticate method is never called)

I can confirm this is an urgent BC break in 10.3.0. Maybe even a "simple" bug, please see the issue from #28.