LogicException & RuntimeException on user login

When using social_auth_google the errors reported here prevented logging in. I had to downgrade back to beta-4.

A little patch review:

At $response = \Drupal::service('renderer')->executeInRenderContext($context, function() {
The class property $renderer should be used.

Thank you for this! I will be reviewing the provided documentation and patch this week. Even if this is not what I wanted, we might not have other options than to change the API to include the renderer.

@ShaunDychko, you got errors when using Social Auth Google? I've been testing Social Auth Google, and it works fine for me. What Drupal version are you using?

@gvso I'm using the latest 8.6.9, and was also using 8.6.7. I included more DB log output in the duplicate issue I originally reported at #3033225: Logic Exception and RuntimeException after upgrade to beta-4. I'm also running PHP 7.2.

@gvso I got errors with Social Auth Google, too. These are hard to clarify. Completely new (for Google) users do work (on registration), some old Google users do not work (null Exception on registration and login), some old users do work (on login).
Version is 8.6.9 (upgraded from working 8.6.2), drupal/social_api 2.0.0-beta5, drupal/social_auth 2.0.0-beta5, drupal/social_auth_google 2.0.0-beta4, league/oauth2-google 3.0.1.

I updated the patch. No much changes, but I fix coding standard violations.

I also upload a patch for Social Auth Google for testing. Please, let me know if this works since I consider this a major issue if Social Auth Google stopped working.

The two patches above fixed the 2 DB log errors that occurred upon redirect to Google.
However, returning back to the site at /user/login/google/callback results in the following fatal error:

Referrer	https://accounts.google.ca/accounts/SetSID
Message	LogicException: The controller must return a response (null given). Did you forget to add a return statement somewhere in your controller? in Symfony\Component\HttpKernel\HttpKernel->handleRaw() (line 169 of /app/vendor/symfony/http-kernel/HttpKernel.php).

"fatal error" means white screen of death with the message

The website encountered an unexpected error. Please try again later.

Hmm. Would you mind sharing your set up? I am using Drupal 8.6 and Apache when testing this.

The issue is a conflict with another module I have installed, but despite hours coming to that conclusion I haven't figured out which one. With the patches in #13, things work fine on a clean install.

As for your question about my setup, I created https://github.com/ShaunDychko/test-social-auth so that you could have the exact same setup on Platform.sh. I made that repo with the intention for you to see the error, but in fact it works fine.

I don't have an issue with module conflicts on the beta-3 version of social_auth_google (using my fork at https://github.com/ShaunDychko/social_auth_google to have league/oauth2-google:^3.0, and corresponding beta version of the other social_* modules), so I'll stick to that for now, but maybe find time later to investigate further which module is conflicting.

Cool. It'd be nice to see if this works for others as well.

Latest update is that there's no module conflict. The issue is solved when I use Devel's "Reinstall Module" for social_auth. It's not enough to reinstall social_auth_google. Since this requires deleting all social_auth entities, it's not a viable solution for the production site, so I suppose the mystery continues...

@wells, I've tested a fresh install with the same setup I'm currently using in production (matching beta versions), succeeded in creating an account via Google, then updated to the latest dev. along with patches from #13, and had bitter-sweet success. The upgrade path seems to be fine, but upgrading nevertheless fails on my production site.

FWIW:

- I've successfully applied both patches in #13.
- Both Exceptions didn't appear on both registering and log-in after patching, they where there before.
- Didn't experience the Fatal Error mentioned by @ShaunDychko
- Obviously the patch on social_auth breaks other depending modules like social_auth_facebook and social_auth_twitter with a nice WSOD on ArgumentCountError: Too few arguments to function Drupal\social_auth\Controller\OAuth2ControllerBase::__construct(), 8 passed in /app/web/modules/contrib/social_auth_facebook/src/Controller/FacebookAuthController.php on line 42 and exactly 9 expected in Drupal\social_auth\Controller\OAuth2ControllerBase->__construct() (line 107 of /app/web/modules/contrib/social_auth/src/Controller/OAuth2ControllerBase.php)
(The above is a social_auth_facebook example)

Note:

I applied the patches on "drupal/social_auth": "2.0-beta5" and "drupal/social_auth_google": "2.0-beta4" so _not_ on the dev versions.

#23 That WSOD ArgumentCountError: Too few arguments should be fixed here I think too. With semantic versioning, this is probably a BC-break that needs a higher version.

+++ b/src/Controller/OAuth2ControllerBase.php
@@ -92,6 +101,8 @@ class OAuth2ControllerBase extends ControllerBase {
@@ -100,7 +111,8 @@ class OAuth2ControllerBase extends ControllerBase {

@@ -100,7 +111,8 @@ class OAuth2ControllerBase extends ControllerBase {
                               UserAuthenticator $user_authenticator,
                               OAuth2ManagerInterface $provider_manager,
                               RequestStack $request,
-                              SocialAuthDataHandler $data_handler) {
+                              SocialAuthDataHandler $data_handler,
+                              RendererInterface $renderer) {

For backwards compatibility RendererInterface $renderer = NULL is better here.

+++ b/src/Controller/OAuth2ControllerBase.php
@@ -125,49 +138,62 @@ class OAuth2ControllerBase extends ControllerBase {
+    /** @var \Drupal\Core\Routing\TrustedRedirectResponse|\Symfony\Component\HttpFoundation\RedirectResponse $response */
+    $response = $this->renderer->executeInRenderContext($context, function () {
...
-      $response = new TrustedRedirectResponse($auth_url);

The removed line could be the default value of $response if $renderer is NULL.

And I think a proper automated test would help a lot here to prevent issues like this in the future.

Does the approach suggested in #24 make sense if the exceptions are breaking logins in some cases anyway (see #7)?

I was thinking that this might be an issue, but I was also thinking of adding backward compatibility by using \Drupal::service() if NULL. We can then remove this change later.

Hi! I have a problem when a auth path has value "/user/login/google?destination=/some". The destination parameter does not allow to auth. I used patches #13 and #27.

Also I use require_login module. After apply the patch patch #13 or #27 I started getting problems. If I delete require_login module then auth works as usual.

I found that earlier...

  public function redirectToProvider() {

    try {
      /* @var \League\OAuth2\Client\Provider\AbstractProvider|false $client */
      $client = $this->networkManager->createInstance($this->pluginId)->getSdk();
....
      $response->send(); // << --- this was the reason to go to auth

      return $response;
    }

@m.lebedev, does this happens with Nginx? That was introduced in #2981737: Destination parameter forbids user to be redirected for authentication

Yes, it does. I use nginx + php-fpm. There is the same problem

I also applied #28 and tested with social_auth_google 8.x-2.0-beta4
Messages are gone, works perfect.

I also read through social_auth/src/Controller/OAuth2ControllerBase.php
Although I don't understand everything the class seems very well documented and formatted.

So here we go: RTBC

Thanks everyone! We still need to fix the issue with Nginx. I don't know if that would break the current solution. I'll have to try

I think this might solve all the warnings and errors.

It turned out, I couldn't reproduce the bug as I used to do with the issue I referenced to. I removed the save method anyways since it's no longer used.

@m.lebedev, can you test this patch? I tested it with Nginx + PHP-FPM and by adding destination parameters.

I can now login with an existing user, but cannot create users as it used to do before updating.

I've tried patches #13 #28 #35

I do use composer.

        "drupal/social_api": "^2.0",
        "drupal/social_auth": "^2.0",
        "drupal/social_auth_google": "^2.0",

           "drupal/social_auth": {
            "fix google login": "https://www.drupal.org/files/issues/2019-03-07/3033444-35.patch"
          },

I have turned on that visitors can create accounts. I've uninstalled require_login and swiftmailer.

When I try to login with Google, I get to enter in my google user/pass, and when it goes back to my site, it gives me the below (only on non-existing users)

The website encountered an unexpected error. Please try again later.</br></br><em class="placeholder">LogicException</em>: The controller must return a response (null given). Did you forget to add a return statement somewhere in your controller? in <em class="placeholder">Symfony\Component\HttpKernel\HttpKernel-&gt;handleRaw()</em> (line <em class="placeholder">169</em> of <em class="placeholder">/home/[user]/sites/main/vendor/symfony/http-kernel/HttpKernel.php</em>).

If I delete require_login module then auth works as usual.

I've uninstalled require_login and swiftmailer.

It seems like there's a conflict with require_login and Social Auth

it still does not work with the destionation parameter. (nginx)

The request is sent when I go to the auth path. The request is correct.
after that, redirection occurs to the destination.
the require_login checks the path that is forbidden to go to anonymous users and the require_login redirects on the login page.

Update from me. I am running up to date versions of

        "drupal/social_api": "^2.0",
        "drupal/social_auth": "^2.0",
        "drupal/social_auth_google": "^2.0",

And the patch from #37

"drupal/social_auth": {
            "fix google login": "https://www.drupal.org/files/issues/2019-03-07/3033444-37.patch"
          },

And it works!

I was having issues with creating accounts... but turns out my database was messed up from previous tests months ago and this solved the new user creation issue:

https://www.drupal.org/project/social_auth/issues/2914598#comment-13004018

My swiftmailer issue was unrelated and is now resolved.

I am still working to get require_login to work and that will be my focus this morning. I have been running require_auth and social_auth_google for many months with clients... so am looking forward to fixing this as it's the last issue before I can roll out all updates to production. And I don't know if the problem is related to social_auth at all... I'm guessing not.

The request is sent when I go to the auth path. The request is correct.
after that, redirection occurs to the destination.

Does this mean Social Auth works for you on PHP-FPM if require_login is not enabled?

I am trying to get require_login:2.1 working with it.

Under /admin/config/people/require-login , I use these Path exclusions

/user/login/google/callback
/user/login/google/callback?state=*
/user/login/google
/user/login/google?destination=*
/user/login/google?destination=/
/user/login
/user/logout
/user/registration
/legal_accept
/user/reset*
/privacy-policy

I've also tried this patch: https://www.drupal.org/project/require_login/issues/3024495

I've tried looking more closely at /src/EventSubscriber/RequireLoginSubscriber.php (same area as targeted by patch above) and even just commenting out the return false that I thought was triggering it not to work... but still can't get past it.

-----

To better answer your question @wells, if I wanted to get a working version going of all these things, I run this command (I use the drupal composer project)

rm composer.lock; rm -Rf ./vendor/*; rm -Rf ./web/modules/contrib/*; composer require 'drupal/social_auth_google:2.x-dev#e3d47a3bab8eae2bc39b5167e9526135cd02f4e2' 'drupal/social_auth:2.x-dev#7ad52ff8eb621c5a7c4d4dbf0e8fb4c0070fbd60' 'drupal/social_api:2.x-dev#f92b77aa0cbffce13c16c7f7ffc56e4e47498b86'; drush cr

I then install require_login 1.12

With these patches:

"drupal/social_auth": {
            "fix stringtranlationtrait to userauthenticator": "https://www.drupal.org/files/issues/2018-11-10/3012823-4.patch"
},
"drupal/require_login": {
            "fixes redirect bug for pages guest cannot see": "https://www.drupal.org/files/issues/2018-11-07/2986046-18.patch"
},

@eahonet, Did Social Auth (and require_login) work for you before this patch or before beta 5?

@gvso The only working require_login I've seen work was the older 1.12 with the older versions of social_auth 2.0-dev. require_login went through an overhaul recently and is now on 2.1.

I did make some progress and posted it here:
https://www.drupal.org/project/require_login/issues/3038652

It seems that when I go to /search... require_login says "no, you need to login first". Then when I try to login with social_auth_google, it starts the process, but ends up redirecting to /search before things finish... so no login finishes and then the user is taken back to the login page.

I don't think the error with require_login should be fixed here (not even that it should be fixed in Social Auth). Anyways, that issue should be handled separately, especially because this issue is about a major bug in Social Auth.

The only thing I'm concerned about is the compatibility with PHP FPM. I could run the authentication with destination successfully in my local server, but I would like to have input from @m.lebedev whether Social Auth works if require_login is disabled.

Yes. social_auth works If require_login is disabled.

Sorry. I checked one time again. When require_login is disabled:
1) social_auth 8.x-2.0-beta5 without patches
Auth works
2) social_auth 8.x-2.0-beta5 with patches.
Auth doesn't work

Source: /core/lib/Drupal/Core/EventSubscriber/RedirectResponseSubscriber.php
There is a change of target url.

Quick fix.

Upd: perhaps needing to create a static variable and after need to check if it exists in response filtering
Availability of oauth2state validation does not provide sufficient accuracy

That might work, but I wonder if we really need to go through all that "trouble" to solve it. Any thoughts?

I do have an update. My path to social_auth_google + require_login + swiftmailer has made for a long week. I do have all of them fully working together now (we're testing this week with client). Some of the things I tried/thought were wrong. The final piece for me was removing the social_auth patch from this thread. My system had same/similar error... but it was something else... something older. I am running a more traditional LAMP stack with no nginx.

1. Swiftmailer - my own stupidity in composer
2. Require_login - changed how it handles the excluded paths. I did add a patch to ignore ?get parameters
3. social_api - 2.0 beta5 - no patches
4. social_auth - 2.0 beta5 - no patches needed... I did have to fix the database from a much older bug.
5. social_auth_google - 2.0 beta4 - no patches needed

I wanted to at least update this thread and wish everyone luck. Also wanted to say thank you to those smarter than I that are working on these patches and these modules.

@wells - social_auth + require_login breaks with the #37 patch. It all started working when I removed the patch and rebuilt /vender and /modules/contrib. I'm not sure why or where... but it felt like something having to do with redirects. (https://www.drupal.org/project/require_login/issues/3038652)

this would be an issue for Social Auth because I disabled require login last time

@wells

require_login + the patch from this https://www.drupal.org/project/social_auth/issues/3033444 will ignore any ? get parameters on exclusion paths. That's what I use. It still works to have

/user/login?destination=/search

redirect to /search after login (drupal login). But social_auth_google has a parameter of where to go and is set to just go to the homepage.

It's when I tried using the above patch (which I guess I don't need on my setup now) that logging in with Google via Social Auth breaks.... or works as long as require_login is uninstalled.

My configs:
Nginx
PHP-FPM

I checked the problem on the fresh drupal installation. I also checked last dev version of Social Auth.
The problem occurs if OAuth2ControllerBase doesn't run $response->send();

I think this issue has become very complex while the initial motivation was to avoid the warnings and errors. I propose the following

1. Let's use the patch in #35. It doesn't produce any warnings/errors and still maintains the logic the current code has
2. Let's solve @wells's concern about $response->send() in a different issue that might include something like the patch in #53
3. In a different issue, we should check the compatibility problem with require_login

Does this sound good? Any suggestions?

I still think #35 is the correct patch to address this issue and #37, in my test, doesn't even get me logged in.

Can you confirm you actually mean 35 and 37 in the correct order? The detail about what works and what doesn't seem to contradict this conclusion

It looks like my problem is not related to this module. I found an issue that describes what is happening.
I also created a module that reproduces the problem. After installing the module, go to "/drupal-test?destination=/" and there will be a redirect to the main page, but in fact there should be a redirect to google com.

I think we can close this issue and use patch #37.

Thanks.

UPD: I see that people solve this problem in their modules. For example.
We can do a similar solution.

// If destination parameter is set, save it.
$destination = $this->request->getCurrentRequest()->get('destination');
if ($destination) {
  $this->userAuthenticator->setDestination($destination);
  $this->request->getCurrentRequest()->query->remove('destination'); // <<--- THIS
}

It works for me. Auth works and redirect to destination works after auth.

Thank you everyone! I'll create new release later tonight

I created a new release. Hope everything works fine now. Thank you once more. We have to update the implementers now.

Thanks guys!

had the similar problem with social_auth_twitter module, since TwitterAuthController doesn't extend OAuth2ControllerBase need separate patch to solve the problem https://www.drupal.org/files/issues/2020-08-12/3068326.patch, related issue https://www.drupal.org/project/social_auth_twitter/issues/3068326