[2.x] Allow authentication via mail and password over OAuth2 module

Nice feature for 1.1 release, needs tests like

Even though this looks like a clean way of doing it, it doesn't really work well with Postman, which doesn't support changing the the grant_type outside of the defined list (password, implicit, authorization code, client credentials). Might we this is not compatible with the OAuth2 standard?

My idea was to simply override the `user.auth` implementation to support authorisation by email as well. This would potentially fix other modules too as we'd then simply use the email where the name is expected.

Sorry, the previous patch was in a wrong format, attaching a new one.

I was able to get OAuth authentication working with the patch and instructions in #2, but the patch in #6 didn't work for me-- I kept getting an error message about an incorrect grant type.

Also it would be great if there is a way to either 1) make this work with Postman, as described in #5 or 2) there are instructions provided for how to debug without Postman.

Updated patch #6 for Drupal 9:

1. +++ b/src/EmailRegistrationServiceProvider.php
   @@ -0,0 +1,18 @@
   +    $definition->setClass('Drupal\email_registration\UserAuth');
   

   Should use UserAuth::class

2. +++ b/src/UserAuth.php
   @@ -0,0 +1,47 @@
   +
   +class UserAuth extends \Drupal\user\UserAuth {
   +
   +  public function authenticate($username, $password) {
   

   needs docblock

Addressed the feedback above and made sure the two new files follow coding standards.

Patch #2 works properly.

I think the last patch not working because they need to set dependencies.

https://www.drupal.org/docs/drupal-apis/services-and-dependency-injectio...

+++ b/src/EmailRegistrationServiceProvider.php
@@ -0,0 +1,23 @@
+    $definition->setClass(UserAuth::class);

I think they need dependencies:

https://www.drupal.org/docs/drupal-apis/services-and-dependency-injectio...

  user.auth:
    class: Drupal\user\UserAuth
    arguments: ['@entity_type.manager', '@password']

The tests are failing because it is expecting protected function setUp(): void in line 38 of tests/src/Functional/Plugin/Commerce/CheckoutPane/EmailRegistrationLoginTest.php.

Not sure if that addition should be included on this patch as the tests are extending "commerce" test base classes.

The dependencies on the new file should be exactly the same ones. As we are extending the core.auth via a plugin I am not sure that we need to set them up again.

Trying to reroll the patch with the previous fix on the test file to see if it runs the tests. I could reproduce the issue of the test runner locally, then applied the patch, and all the tests ran sucessfully.

Note that this patch is the evolution of #5, not #2.

Patch #2 is working properly in D9.2.7, tested in Next Auth authentication context. Great !

Patch from #19 works like a charm on Drupal 9.3.2 in tandem with email_registration module. Thank you all so much for your work on this!

Patch #2 work perfect for simple_oauth Drupal 9.3.3.
Thanks a.dmitriiev

NW for remarks and still no test added, the overriden UserAuth needs better explanation (in code comments) why we need to override it

1. +++ b/src/UserAuth.php
   @@ -0,0 +1,58 @@
   +   * Return whether the given user credentials are valid or not.
   ...
   +   * @param string $username
   ...
   +   * @param string $password
   ...
   +   * @return bool
   ...
   +  public function authenticate($username, $password) {
   

   this method should use @inheritdoc as defined in \Drupal\user\UserAuth::authenticate()

2. +++ b/tests/src/Functional/Plugin/Commerce/CheckoutPane/EmailRegistrationLoginTest.php
   @@ -35,7 +35,7 @@ class EmailRegistrationLoginTest extends CommerceBrowserTestBase {
   -  protected function setUp() {
   +  protected function setUp(): void {
   

   this change is unrelated and fixed via #3228033: Update the EmailRegistrationLoginTest to match the parent CommerceBrowserTestBase class.

I opened an MR with the changes. I addressed the feedback and added a test that bypasses the login form (that this module alters) via http request to check that login with both username and mail does work. I extended core tests and added comments where appropriate.

Marking as need review again.

I just want to thank @fjgarlin for the latest changes in MR!3, I've tested this MR with Drupal 9.3.21 and it works great. Cheers!

This would be a great feature for 2.x! Thanks a lot, I'll see if I can squeeze in some time the next couple of weeks! :)

Needs a reroll for 2.x, please. Then we can proceed.

@Cryt1c - no need to create a new MR. The only difference is $query->accessCheck(TRUE);.
You can collaborate in https://git.drupalcode.org/project/email_registration/-/merge_requests/3

A new MR would make sense if the approach is completely different, but not if it's the same.

@fjgarlin thanks for the info. I had some issues with pushing before. Now it worked and I have added my commit to your MR.

Perfect! Thanks for helping with the issue.

I guess, this can go back to "Needs review"?

needs to add strict types

Fixed my feedback

created https://www.drupal.org/project/email_registration/releases/2.0.0-rc5

Thank you, @andypost! I didn't find the time to review this!