[2.x] Login fails with Drupal 10.3

It's definitely broken on 10.3, and unfortunately there's no obvious error in the logs showing what is wrong.

All phpunit tests are failing on 10.3 and 10.4 as well, so I modified GitLab CI to test previous and next minors.

I haven't been able to figure out what's wrong here.

There have been several changes to authentication in 10.3:

• New interface method UserAuthenticationInterface →
• UserAuthenticationController::LOGGED_IN and ::LOGGED_OUT are now strings →
• UserLoginForm::validateName() is deprecated →
• user_validate_name() is deprecated and replaced with a UserNameValidator service →

Thanks for the help testing and looking for what has changed and any help you can provide in fixing it. So it works with 10.2, but not 10.3?

And what about the 8.x-1.x branch - does that work with 10.3?

We may want to add a note to the project page until this is fixed.

I think this is failing because of this:

        if ($this->userAuth instanceof UserAuthenticationInterface) {
          $form_state->set('uid', $this->userAuth->authenticateAccount($account, $password) ? $account->id() : FALSE);
        }

From
UserLoginForm->validateAuthentication();

When this module is enabled, userAuth is an instanceOf email_registration/userAuth so the above condition doesn't fire.

From what I can tell, the uuid is never set with the above condition and thus validateFinal fails.

Ive pushed some WIP thoughts for this, which does get the login functionality working locally, however im unsure of any potential impact outside of this change as I see userAuth used elsewhere, in sub modules. Should at least point us in the right direction (if it's the right approach to take!)

Great find @aaron.ferris!

It seems your changes fix 10.3 and break 10.2. If there's no way to support both, then in theory there should be a major new version, but the next major version (2.x) is already in development... That's tricky.

Had to create a new branch to test prev + next minors on 1.x.

The good news is that 1.x seems fine according to the tests, so I'm updating the issue title to reflect that this is a 2.x problem and downgrading to major since 2.x is not stable.

We could always look to switch the definition out depending on version, if creating a new major release is a problem

IE, reinstate userAuth for < D10.3 and use userAuthenticate for >= 10.3. Feels like there should be a nicer way of doing it mind.

/**
 * Defines a new registration service provider.
 */
class EmailRegistrationServiceProvider extends ServiceProviderBase {

  /**
   * {@inheritdoc}
   */
  public function alter(ContainerBuilder $container) {
    // Replace the core authentication service with our version
    // which supports authorization through email.
    $definition = $container->getDefinition('user.auth');

    if (floatval(\Drupal::VERSION) >= 10.3) {
      $definition->setClass(UserAuthentication::class);
    }
    else {
      $definition->setClass(UserAuth::class);
    }
  }

}

This is by definition a non-backwards-compatible breaking change, warranting a new semantic version branch.

Cut 2.x off at 10.2, and make a new branch called 3.x that only supports 10.3 up. 2.x seems to be working well enough for it to have a release and then be marked as not recommended for dev to continue in 3.x.

The suggestion from @aaron.ferris in #3456461-14: [2.x] Login fails with Drupal 10.3 → would allow the 2.x branch to continue on, however tools like phpcs/phpstan will complain about a non-existent interface in the older versions. And it means maintaining 2x versions of core in the one version which might prop up other issues as things move forward.

There are no doubt going to be more breaking changes heading into D11, although this one is a doosey! Nobody able to log in this morning, and no errors.

It would be better if unit testing was setup to test against supported cores? At present it is only testing against current point release, and not previous or next, which hid this issue from turning in testing.

I removed conflicting require-dev entries from composer.json as it would not build on Drupal 9.5. This just pushed the error over to the unit tests which failed to display the correct output. It looks very much that since Drupal Commerce had an incompatible release with Drupal 9.5, that it simply doesn't install correctly or work with such an old version. This would indicate to me that attempting to test against Drupal 9.5 would require a different branch to be setup to install the last working version of Commerce, and not just the last compatible release.

All of this would suggest to me that the 2.x branch needs to not support Drupal 9. The minimum supported version of Drupal is current 10.1. Perhaps it is time to update the core support for the 2.x branch. With MR!43, it can be ^10.1

The functionality those two removed requirements are adding could easily be duplicate with a "logIn" function written inside the tests themselves, bypassing the need for such complexity as doing an extension replacement just for testing. Ironically, this already exists in tests/src/Traits/EmailRegistrationTestTrait where it provides a replacement drupalLogin which uses the email + pass to log into Drupal. Is the behat extension replacement really needed for GitlabCI testing?

The updated MR!43 courtesy of @arron.ferris is effective for me on a 10.2 and 10.3 site.

Can confirm the issue.
Just recently updated 2 of our projects from Drupal 10.2.7 to drupal 10.3.0.
After update, I was unable to log in to both projects. I could only login using drush user:login but after resetting the password, saving and then logging out, I could still not login. Search half a day for a fix first in the Drupal issues, the flood and flood_control issues, Redis issues, ...
Finally after lots of debugging found that the bug came from the email_registration module. Directly found this issue and tested MR43.
Applied without problem and then the login problem was fixed in both projects.

Weird, we did several updates in core to improve BC around this, and it should detect the case of an override implementing the old interface, would need to look at that more closely.

I have opened 🐛 BC break in login auth changes from #3444978 Active for the core bug that is causing this. Specifically, this was broken as part of trying to fix a different regression 🐛 UserAuth BC layer is not working for modules that use it to provide email based logins Active from the original change in 📌 Optimize user logins by avoiding duplicate entity queries Needs work .

I'm not 100% sure I would consider the fix a BC break - the code works with both branches. I wonder if there are ways to resolve the static analysis (which could be as simple as ignoring them).

@Berdir do you feel this module should be fixed or core or both?

@berdir email registration is hitting code branch that was covered by the original version of the code https://git.drupalcode.org/project/drupal/-/commit/77d19f7abb3875c41fb38...

By moving to an elseif on the parent if, neither branch are hit by email registration.

mail_login is also broken: 🐛 Error: Call to undefined method Drupal\mail_login\AuthDecorator::authenticateAccount() with Drupal 10.3.0 Active

Patch #43 works fine on 10.3

To try to catch issues like this in the future, I moved my MR to expand testing to next and previous minors to a new issue: 📌 Test against next minor and previous minor Needs review

ptmkenny → changed the visibility of the branch 10.3_breakage_1.x to hidden.

ptmkenny → changed the visibility of the branch 10.3_breakage to hidden.

@trickfun where is the patch you mentioned?

There is no #43 in this issue.

Click "plain diff" link near MR !43 mergeable on top of the page.

I figured out later that you referred to the merge :) My bad.

MR !43 fixed it for us! Thanks!

Confirming that. MR!43 LGTM, thanks!!

Merging this into 2.x-dev now based on the feedback, thank you!

Going to tag -rc6 in some days, waiting for further feedback here.

It's been a week - want to go ahead with that release?

Thanks @greggles yes, let's do this, as we don't have any bad reports here.

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

Hi guys,
under Affected versions @teddyvermeulin claims that
(1.x is not affected)

I'm affraid it's not true.
In Drupal\user\Form\UserLoginForm::validateAuthentication() method
login also fails on line
if ($this->userAuth instanceof UserAuthenticationInterface) {

$this->userAuth is actually Drupal\user\UserAuth which implements Drupal\user\UserAuthInterface

So patch should be backported to version 1.x (?)