Failed login can still redirect to the "Post login path"

The changes are ready for review.

Can/should we add a test to https://git.drupalcode.org/project/social_auth/-/blob/4.1.x/tests/src/Fu...?

Yes, we should include a test for this one.

I'm not sure I'd know how to begin. We'd need to mock remote data, set an integration and settings in it (like disallow registrations) and then test the workflow. I don't see any test doing something similar yet.

For context, I found this bug integrating the new D10 version of www.drupal.org with the new keycloak system. We want registrations to always go via D7, therefore not allowing registrations on D10.

When working with existing users, everything works well, but when the user does not exist yet in D10, then we get the above watchdog messages and behaviour.

If I get some boiler plate code for the above from any of you that know the module better I could finish it off.

Ack. Good point. This would be difficult to test. Happy merge it someone else is able to do some functional review and get it to RTBC. Otherwise, I'll try to do so eventually. Thanks, @fjgarlin.

I actually found another case of infinite loop to "user.login" or a nasty error which is not captured.

The website encountered an unexpected error. Try again later.

TypeError: Stevenmaguire\OAuth2\Client\Provider\Keycloak::getResourceOwner(): Argument #1 ($token) must be of type League\OAuth2\Client\Token\AccessToken, null given, called in /app/web/modules/contrib/social_auth_keycloak/src/KeycloakAuthManager.php on line 62 in Stevenmaguire\OAuth2\Client\Provider\Keycloak->getResourceOwner() (line 269 of /app/vendor/stevenmaguire/oauth2-keycloak/src/Provider/Keycloak.php).
Drupal\social_auth_keycloak\KeycloakAuthManager->getUserInfo() (Line: 308)
Drupal\social_auth\Controller\OAuth2ControllerBase->processCallback() (Line: 197)
Drupal\social_auth\Controller\OAuth2ControllerBase->callback()
call_user_func_array() (Line: 123)

If the credentials are wrong, we don't get a token back, so I'm checking for that before going any further. I discovered that the "processCallback" function does not handle any possible error, so I added a differentiation and multiple error codes that can help when debugging.

So, the current MR avoid two cases of infinite redirects to "user.login" and a case for an exception being thrown, and instead it captures the errors and redirects to the homepage instead.

Thanks for your work on this, @fjgarlin -- I left a question on the MR that I think will ultimately lead to some additional changes needed. Let me know your thoughts.

Maybe you couldn’t see the whole MR code? The changes you were asking about were addressed already and shown in the MR.

I refactored the solution as per the feedback in the MR. It can be reviewed again. The approach is the same, it's about catching the error in the "processCallback" function, and then being able to act on it in the "callback" method to avoid an infinite login loop.

Looks good to go -- thanks!

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