Problem/Motivation

Since Drupal 9.2, this module does not work correctly anymore. The troublesome change seems to be https://www.drupal.org/node/3006306

Proposed resolution

There are currently two active avenues:

  • Patch #25; introduce a middleware
  • Patch #107; introduce a destination parameter in the current request

#25

This introduces a middleware to implement the module's functionality. There were some doubts about whether this would not be too heavy, and whether it would not make more sense to only trigger on submission of the login form, which larowlan had this to say about over in #45:

Please see comment #37, this module supports more than form-based login at present.

Is that an acceptable compromise?

It would be nice to find some alternative strategy, maybe event-based.

The only suitable event would be the ResponseEvent from the Kernel, and that also runs on every request, so its a comparable amount either way.

Regardless, all we're adding is one __construct call, one extra function call (to call the inner kernel) and a ternary.

#107

This simply adds a destination parameter to the current request.

Remaining tasks

  • Decide which approach to take.
  • Apply patch/merge
  • Make a release that is actually compatible with the currently supported versions of Drupal 9 (Drupal 10 is a separate matter)

User interface changes

None.

API changes

None.

Data model changes

None.

Original report by Cangurin limpiezas

Hi, i have problems to login when i update from drupal 9.18 to drupal-9.2.0-alpha1
after upgrade i continue in login, all ok
but when i logout and try login again ,
in apache log i get the next error

[Wed May 19 21:49:41.463707 2021] [php7:notice] [pid 45161] [client 127.0.0.1:50796] RuntimeException: Failed to start the session because headers have already been sent by "/var/www/html/c19052021/vendor/symfony/http-foundation/Response.php" at line 1224. in /var/www/html/c19052021/vendor/symfony/http-foundation/Session/Storage/NativeSessionStorage.php on line 152 #0 /var/www/html/c19052021/core/lib/Drupal/Core/Session/SessionManager.php(162): Symfony\\Component\\HttpFoundation\\Session\\Storage\\NativeSessionStorage->start()\n#1 /var/www/html/c19052021/core/lib/Drupal/Core/Session/SessionManager.php(193): Drupal\\Core\\Session\\SessionManager->startNow()\n#2 /var/www/html/c19052021/vendor/symfony/http-foundation/Session/Session.php(189): Drupal\\Core\\Session\\SessionManager->save()\n#3 /var/www/html/c19052021/core/lib/Drupal/Core/StackMiddleware/Session.php(60): Symfony\\Component\\HttpFoundation\\Session\\Session->save()\n#4 /var/www/html/c19052021/core/lib/Drupal/Core/StackMiddleware/KernelPreHandle.php(47): Drupal\\Core\\StackMiddleware\\Session->handle()\n#5 /var/www/html/c19052021/core/modules/page_cache/src/StackMiddleware/PageCache.php(106): Drupal\\Core\\StackMiddleware\\KernelPreHandle->handle()\n#6 /var/www/html/c19052021/core/modules/page_cache/src/StackMiddleware/PageCache.php(85): Drupal\\page_cache\\StackMiddleware\\PageCache->pass()\n#7 /var/www/html/c19052021/core/lib/Drupal/Core/StackMiddleware/ReverseProxyMiddleware.php(47): Drupal\\page_cache\\StackMiddleware\\PageCache->handle()\n#8 /var/www/html/c19052021/core/lib/Drupal/Core/StackMiddleware/NegotiationMiddleware.php(52): Drupal\\Core\\StackMiddleware\\ReverseProxyMiddleware->handle()\n#9 /var/www/html/c19052021/vendor/stack/builder/src/Stack/StackedHttpKernel.php(23): Drupal\\Core\\StackMiddleware\\NegotiationMiddleware->handle()\n#10 /var/www/html/c19052021/core/lib/Drupal/Core/DrupalKernel.php(716): Stack\\StackedHttpKernel->handle()\n#11 /var/www/html/c19052021/index.php(19): Drupal\\Core\\DrupalKernel->handle()\n#12 {main}, referer: http://www.localhost/c19052021/user/login

if i delete the module with
drush pm-uninstall redirect_after_login

i can enter to my account without problems,
any help?
Thanks.

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

Cangurin limpiezas created an issue. See original summary.

sokru’s picture

Title: Headers have already been sent after upgrade to drupal-9.2.0-alpha1 » Headers have already been sent after upgrade to drupal-9.2.0-beta1

Also on Drupal 9.2-beta1

sokru’s picture

parisek’s picture

Priority: Normal » Critical

I have same issue, I think its critical issue

dbielke1986’s picture

smustgrave’s picture

Also getting this error with no workaround other then uninstalling this module.

laborouge’s picture

Hello,
Same problem with Drupal 9.2.0

djg_tram’s picture

It's not directly the problem of the module. I don't use the module but have a similar redirect code in my own code and that fails, too. Something broke with the way TrustedRedirectResponse used to work.

djg_tram’s picture

dbielke1986’s picture

@djg_tram

Thanks for reporting!

oldeb’s picture

Issue tags: -drupal-9.2.0-alpha1 +drupal-9.2.0
larowlan’s picture

The issue here is that redirect after login is breaking a basic contract of Drupal hooks, in that it is halting execution.

It is using hook_user_login to hard redirect, preventing any other modules that implement this hook from even firing

My suggestion would be to uninstall the module for now

It needs to find a better way of achieving what it needs to do, most likely using a middleware that had a setter which the hook would call to trigger the redirect

I think the core issue should be changed to a support request, to help the maintainers of this module find the right way to achieve the functionality they need

djg_tram’s picture

Having enumerated all my arguments in https://www.drupal.org/project/drupal/issues/3219320, I also went on and found the "officially sanctioned" way of doing it in the deprecation changelog of drupal_goto(): https://www.drupal.org/node/2023537

$response = new RedirectResponse($url->toString());
$request = \Drupal::request();
// Save the session so things like messages get saved.
$request->getSession()->save();
$response->prepare($request);
// Make sure to trigger kernel events.
\Drupal::service('kernel')->terminate($request, $response);
$response->send();

I would also consider suggesting

$headers = ['Cache-Control' => 'no-cache']; 
$response = new RedirectResponse($url->toString(), 302, $headers);

This seems to solve the problem in my code. But the rest of the problem remains: heaps of admins will be locked out of their sites completely after this breaking change and I can't see how this can be handled easily now.

larowlan’s picture

If you create a new middleware service

And put a method on that service 'setRedirect'

And then have that service send the request into the inner kernel, and grab the response

If setRedirect hasn't been called, just return the response from the inner kernel

If setRedirect has been called, return your redirect response instead

In the user login hook, call setRedirect

That's a rough idea of how you might be able to achieve this without breaking the hooks contract

larowlan’s picture

djg_tram’s picture

StatusFileSize
new3.24 KB

I added the code I play with, it runs without errors but the redirect is always null, so it doesn't happen yet.

By the way, how do you name your middleware service, making sure there will be no name clash?

larowlan’s picture

Looks great 💪

djg_tram’s picture

Looks are not really important if it doesn't work. :-) I guess I should store the url in some other way to make it stick, shouldn't I?

I sprinkled it liberally with logs. I set it all right. The next call to get the url still returns null. Bother.

djg_tram’s picture

So, as the idea was yours to start with. :-))

I set the redirect url. However, the next time around, a *new* Redirect will be __constructed, so my set url will be gone. As you probably know waaay more about the internals than I do, what is the route to be taken in this case? Where am I supposed to store data like this between instantiations? Is \Drupal::state allowed for this? It doesn't feel right... I also tried drupal_static but it doesn't seem to help, either.

djg_tram’s picture

StatusFileSize
new2.69 KB

Let's call it proof-of-concept, it works with the State API (yes, it should be injected, just POC, as said) but it feels wrong to have an added state query (even if it's cached somewhere) for each request. Or is it really cheap enough and correct this way?

Nahh, obviously not a correct way, what about simultaneous requests? Then, how? The private tempstore still doesn't work for anonymous.

cilefen’s picture

The other issue points to https://www.drupal.org/project/drupal/issues/2238561 as a breaking change. If that is accurate, has anyone got an idea of exactly how it broke these use cases?

djg_tram’s picture

I'll be interested in hearing your opinion. This doesn't seem to work the way you envisaged. I'm drawing blanks everyhere.

States API is obviously wrong. It has to be linked to the user. But: I can't use temp storage with an anonymous user, Drupal still doesn't support it, the session isn't started, the store is null. There is code about how to start it, but that won't help us, either. I cannot inject the @current_user and the @tempstore.private because they will change, we're speaking about a login process, actually. I cannot call \Drupal::currentUser() from inside the code either, because this middleware is not the place where such calls go through from; not to mention that it would be rather insane to call that plus a \Drupal::service('tempstore.private')->get() for each and every request but putting that aside for a moment, it plain doesn't work.

I also tried to reach $_SESSION directly, without much ado about the private storage but that doesn't seem to work, either.

So, I'm temporarily out of ideas. While your scheme should theoretically work if there was a way to keep the redirect URL somewhere for the next round to pick it up, there is none I could find so far. Unless I'm missing something, the halting process is still the only one working.

djg_tram’s picture

@cilefen, see #13, this is the full drupal_goto() replacement, modules don't call the full routine, they tend to forget to save the session and pass on the original request. I think that's the reason.

djg_tram’s picture

After spending most of the day with this issue, I finally stumbled upon https://www.thesavvyfew.com/insights/how-redirect-user-after-login-drupa... and, well, it made me think, so much so that I removed my current attempts and modified my code to use this approach. You might want to think about it because, as much as I can tell, Larowlan's original idea doesn't seem to work.

larowlan’s picture

StatusFileSize
new3.47 KB

That link is referring to form submissions.

To be clear form submissions and controllers can continue to use redirects.

The issue is with modules that are forcing redirects in hooks, which is what this module is doing.

This is not how hooks are supposed to work.

The hook it is using is hook_user_login, because this module starts with the letter R it prevents any modules that come after it in the alphabet from being notified of hook_user_login. In core alone that prevents the user.module from running user_user_login.

So there's two ways to fix this, the quick and dirty way, which would retain the broken hook contracts. In that approach, instead of calling ->send() on the created redirects, instead use throw new \Drupal\Core\Form\EnforcedResponseException($response); which will then bubble into the exception handler, where \Drupal\Core\EventSubscriber\EnforcedFormResponseSubscriber will convert it into the actual response. This is abusing the api for form redirects and still prevents user_user_login from ever running.

The correct way is the middleware approach described above. Thanks for starting on the approach @djg_tram, however you only did part of the work, so it didn't work. You need a services.yml file and changes in the module code, in addition the class namespace was in example\StackMiddleware so that wouldn't autoload.

Here's a working patch of the middleware approach. There are no tests in the module for the redirect functionality so this will require manual testing.

Apply this patch, then clear your caches.

Works in my manual testing.

smustgrave’s picture

Can confirm the patch is working. Able to login on 9.2 with this module installed.

djg_tram’s picture

The correct way is the middleware approach described above. Thanks for starting on the approach @djg_tram, however you only did part of the work, so it didn't work. You need a services.yml file and changes in the module code, in addition the class namespace was in example\StackMiddleware so that wouldn't autoload.

Obviously, not. I wasn't patching this module, I was making my own module work and when I reported that it does work with the States API, you can bet your bottom dollar I didn't forget to use services.yml in my own code, it wouldn't have worked otherwise. :-)

Yes, it was "example" when I uploaded here, I edited my file before upload to remove my own references.

The problem was rather different, the inability to store the redirect URL itself between the set call and actually reading it out when needed -- it just went away, not being there when needed. I'll take another look at what you did differently here and report back.

djg_tram’s picture

What I can see at first is that you forgot to clear the stored response when actually doing the redirect. I guess this would be a sanity issue, just to be sure. Also, I wouldn't call the original handler first if all I do is discarding it, anyway. Check for the existence of the redirect first, then decide what to do.

larowlan’s picture

Ah, sorry, I thought you were patching this module, I didnt realise you were working with custom code

There's no need to clear the stored response, this code runs once, the middlewares are layers, like an onion, each is run once and then it returns to the outer layer which ultimately returns to the browser

djg_tram’s picture

Anyway, it still doesn't work for me. If you have a couple of minutes to spare, I show you mine and what I don't understand here. If not, we can leave it as it is, in my own site, the form submit handler works just fine, I'm pursuing this now just for the sake of it.

I won't edit out the name now, my module name is "houses", nothing particularly secret about it. :-)

This is the simple hook:

function houses_user_login(UserInterface $account) {
  $url = Url::fromRoute('<front>', [], ['absolute' => 'true']);
  $home = new RedirectResponse($url->toString(), 302);
//  $home->getCacheableMetadata()->setCacheMaxAge(0); // I keep it here for the Trusted case, not needed now
  \Drupal::service('http_middleware.houses_redirect')->setRedirectResponse($home);
}

And the relevant part of the middleware:

public function handle(Request $request, $type = self::MASTER_REQUEST, $catch = TRUE) {
  \Drupal::logger('houses')->notice($this->redirectResponse ?? 'null');
  $response = $this->redirectResponse ?: $this->httpKernel->handle($request, $type, $catch);
  $this->redirectResponse = NULL;
  return $response;
}

I do clear it before return but I trust you if it isn't needed, although it doesn't hurt anything for sure. I also call the original handler only if needed.

The problem is with the logger line. It gives null (yes, I now, it would blow up if it wasn't null, a var_export or similar is needed but it *is* null, unfortunately).

For all intents and purposes, the code should work. This is about the same as I have shown in my first attempt (only that I stored the URL string, not the response itself, small difference). We do lots of services like this and the values we store are, usually, well, stored. But not in my case. That's why I started to tinker with States API, private storage, whatever.

What could make my stored response disappear when I need it?

Oh, an update. If I put a log line into __construct, I can see that it gets __constructed once more. I read from a different instance than what I put my reponse into. This is the immediate reason for the null, of course, but not the underlying cause. Why does it get recreated in the first place?

larowlan’s picture

hi @djg_tram - the logger is null because at that point you're before the inner kernel has been called.

i.e. you're traversing down the layers of the onion, but until you get to the innermost kernel, Drupal hasn't done anything.

It's only when you get to the $this->httpKernel->handle() that you invoke the inner layer.

Eventually that will get to the inner most layer (the actual http kernel) which is when Drupal's request processing occurs.

So, if you look at my patch above I do this

$response = $this->httpKernel->handle($request, $type, $catch);

And then I do my logic. i.e. I make sure to call to the inner kernel(s) so that response processing occurs.

After that I check if my response is set.

larowlan’s picture

I've sent an email to the module maintainers via their contact form asking them to look at this issue.

@smustgrave if this tested fine for you, perhaps you could consider marking it RTBC? Thanks

djg_tram’s picture

Oh, dear, oh, my, that's a tricky one. I was convinced to be the clever one to only call it if the condition fails. :-) You're right, now it works. Thanks.

larowlan’s picture

🎉 No worries, glad you got it sorted

shamsher_alam’s picture

Hi Team,

Sorry, i am busy with other stuff so won't able to track the listed issue for this maintenance. Please let me know if you have any patch for review so if it will works fine I will create the release tag.

Thanks

cristinahart’s picture

I have applied the patch from #25 and I can confirm it's working

alexpott’s picture

Status: Active » Reviewed & tested by the community

The approach in #25 looks good to me. On a client project where I have a similar requirement I moved all the code out of hook_user_login() and into a custom form submit because I realised I was only interested in redirecting logins via the login form and have no interest in redirecting other forms of login like basic auth or json api logins. But implementing #25 will preserve the current functionality so I guess that's good for the module.

laborouge’s picture

I have applied the patch from #25 and I can confirm it's working on Drupal 9.2.0.
Thank you

larowlan’s picture

We have an open issue in core to deprecate EnforcedResponseException, I wonder if there's some inspiration we could take from the redirect middleware here

goldin’s picture

Patch from #25 works for me. Thank you.

Okipa’s picture

Hello,

RuntimeException: Symfony\Component\HttpFoundation\Session\Storage\NativeSessionStorage->start() Failed to start the session because headers have already been sent by "" at line 0.

It gives error. If I download the latest version from git server, will the error be fixed? Or do I have to patch it? I don't know how to patch.

jsutta’s picture

#25 worked for me! Thank you!

manuel.adan’s picture

From the middleware API documentation:

Middlewares run on every request.

#25 adds code that will be executed on every request but actually used only on user login.

It would be nice to find some alternative strategy, maybe event-based.

manuel.adan’s picture

Status: Reviewed & tested by the community » Needs work

Drupal core performs some redirection in the user login form, another way to address this issue could be by overriding such form.

larowlan’s picture

Please see comment #37, this module supports more than form-based login at present.

Is that an acceptable compromise?

It would be nice to find some alternative strategy, maybe event-based.

The only suitable event would be the ResponseEvent from the Kernel, and that also runs on every request, so its a comparable amount either way.

Regardless, all we're adding is one __construct call, one extra function call (to call the inner kernel) and a ternary.

dbielke1986’s picture

https://www.thesavvyfew.com/insights/how-redirect-user-after-login-drupa...

It says:

For Drupal 8 in particular, it is important to note that we no longer use hook_user_login(), because it would stop other implementations of that hook to be invoked. Instead, we’re adding a custom submit handler to the user login form.

Would it be better to go away from the "hook_user_login()" anyway?

ivnish’s picture

Title: Headers have already been sent after upgrade to drupal-9.2.0-beta1 » Headers have already been sent after upgrade to Drupal 9.2 (can't login)
Issue tags: -drupal-9.2.0, -login
djg_tram’s picture

For Drupal 8 in particular, it is important to note that we no longer use hook_user_login(), because it would stop other implementations of that hook to be invoked. Instead, we’re adding a custom submit handler to the user login form.

I tend to agree with Larowlan in this. I also saw that article, mentioned it here (or in the other issue queue) and used it for a day or two but the cleanness of this middleware solution convinced me, too. The quote above is misleading and not really correct: using the hook does nothing like that in itself. Using the hook improperly does but that's nothing specific to hook_user_login(), it would happen with any hook you write.

This hook is and remains the most logical place to do anything related to post login operations. Just do the right thing and don't goto away freely.

dbielke1986’s picture

@djg_tram:

Thank you for your feedback. I completely trust your analysis and Larowlan's experience. You will have chosen the right path, and I look forward to a patch in a stable release.

BR
Daniel

alexpott’s picture

Re the discussion of hook_user_login + middleware vs form submit. I think there is one thing in favour of moving to the form submit. At the moment the hook implementation does:

  if ($request->getRequestFormat() !== 'html') {
    return;
  }

if it was a form submit handler it wouldn't have to do this because forms are only ever submitted via html. Also redirects are the business of forms.

I think currently the module would support basic auth too as per #37 - I was wrong about the module doing anything for jsonapi - but weirdly I'm not 100% convinced that supporting basic auth is actually what you want because with basic auth you get a log on prompt on any page and continue to that page.

rmontero’s picture

Bump! Got a site suddenly breaking and it was very hard to troubleshoot it. Only fixed when we disabled this module. Worked great in the lower environments though.

revati_gawas’s picture

Patch #25 works for me. Thank you.

bjackson’s picture

Bump from me as well. We use this module in several sites and I'm holding back on moving to 9.2 core in the hope of a formal release that integrates this patch.

stopopol’s picture

+1

apparatchik’s picture

+1 for the hook_user_login approach, I suggest to start a survey among users of this module, I bet >95% of us would be served/sufficed by the hook option instead of the much more complex (with perhaps little extra benefit) of the middleware approach.

djg_tram’s picture

Sorry but this has no sense whatsoever. The hook and the middleware are not mutually exclusive, quite the opposite. You use the hook but you're not supposed to just use a goto inside the hook, so you practically *have to* use the middleware *from the hook*.

I don't think the module users are in any way influenced by this. They just want the redirection to happen.

jonmcl’s picture

I came across this issue while researching our own custom code that functions similar to redirect_after_login. Our code uses hook_user_login, but simply dispatches an event. The EventSubscriber class does all the work. We previously had a simple $redirect = new RedirectResponse(...) and then $redirect->send(). I think redirect_after_login had something similar? No longer works with 9.2.

Anyway, my vote is against doing something with a submit handler on the form. Not all logins come from the login form. That's why you need to use the hook_user_login. I'm not sure if there is a core event for user logins, that's why we went the route of creating a custom event and matching subscriber. Just in case we needed additional logic around hook_user_login.

By the way, we based our fix to our custom code on #25. Thank you!

alexpott’s picture

@JonMcL and others, one question that I'm interested in seeing an answer for is "what login that doesn't come via the form do you want a redirect for?". I've tried to come up with one but for both basic auth and some form of RESTful login I'm not sure a redirect is appropriate.

jonmcl’s picture

We have a custom AuthenticationProvider for a proprietary proxy portal (fed gov stuff). I suspect many (all?) of the SSO providers can work without the form. I think core’s basic auth module works without it.

Also, we use the Masquerade module which triggers new logins without the form.

For us, any login is a valid reason to have a redirect. Different roles end up in a different first page.

I should add that our site/app does not have any pages that can be accessed without authentication. We achieved that with the require_login module.

jonmcl’s picture

By the way, I found the change record that I think is causing this issue: https://www.drupal.org/node/3006306

See the "Implications for hook_user_login()" section. That section also links the old change record https://www.drupal.org/node/2023537

Near the end is the following suggestion. This might work here? I have not yet tried it:

  $response = new RedirectResponse($url->toString());
  $request = \Drupal::request();
  // Save the session so things like messages get saved.
  $request->getSession()->save();
  $response->prepare($request);
  // Make sure to trigger kernel events.
  \Drupal::service('kernel')->terminate($request, $response);
  $response->send();
djg_tram’s picture

That was the first move (mentioned either here or in another issue). This is the proper way to do a goto, yes, only that this module didn't follow it. But the main problem is that you're not supposed to do a goto, even if it works. It breaks the chain of hooks, nothing else gets called after yours and this isn't correct.

p4trizio’s picture

Status: Needs work » Reviewed & tested by the community

#25 is working, thank you

aadeshvermaster@gmail.com’s picture

Hi Team,

I am confirming patch#25 is working fine for me for Drupal 9.2 & Redirect After login module version 8.x-2.7

Thanks

orodicio’s picture

I am confirming that patch#25 is working fine for me for Drupal 9.2.4 & Redirect After login module version 8.x-2.7

mb_lewis’s picture

Hi all

Just adding another +1 for Patch#25 tested on Drupal 9.2.5 & module version 8.x-2.7

Thanks

ginobaert’s picture

Hi,

I am confirming that Patch#25 is working on Drupal 9.2.6 and module version 8.x-2.7.
I've tested on multiple websites.

Thank you!

joseft40’s picture

Hi all,
+1 That patch#25 is working fine for me.
Drupal 9.2.6 and module version 8.x-2.7
Thanks

kevinquillen’s picture

Just ran into this on 9.2.6 (upgraded from 8.9.x) with module version 2.7.0. The patch in #25 fixes the issue. Users could not log in and generate a session - now they can.

jorgemontoyab’s picture

Hi @kevinquillen,

I'm working with version 2.7 and with core version 9.2.6. After installing the module with Composer I applied the patch directly in the module folder

web/modules/contrib/redirect_after_login$ patch < 3214949.patch

Now I'm getting the following message:

The website encountered an unexpected error. Please try again later.
Error: Class 'Drupal\redirect_after_login\RedirectMiddleware' not found in Drupal\Component\DependencyInjection\Container->createService() (line 262 of core/lib/Drupal/Component/DependencyInjection/Container.php).

Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 240)
Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 240)
Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 240)
Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 240)
Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 240)
Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 437)
Drupal\Component\DependencyInjection\Container->resolveServicesAndParameters() (Line: 240)
Drupal\Component\DependencyInjection\Container->createService() (Line: 176)
Drupal\Component\DependencyInjection\Container->get() (Line: 1370)
Drupal\Core\DrupalKernel->getHttpKernel() (Line: 717)
Drupal\Core\DrupalKernel->handle() (Line: 19)

Is that the proper way to apply the patch or where should I do that?

Edit: I had to install the module from source composer require drupal/redirect_after_login --prefer-source. then apply the patch with git apply -v 3214949.patch inside the module's folder.

#25 works now!

adsyy’s picture

Hi all,
#25 working for me on Drupal 9.2.2 and module version 8.x-2.7
Thanks

@jorgemontoyab you can also apply patch in composer.json with cweagans/composer-patches package, this link explain well :
https://duvien.com/blog/how-apply-patch-file-composer-based-drupal-89
I find this easier and you get a place where you can check all patches use in your projet.

sir_squall’s picture

Tried on Drupal 9.2.7 it's working well, could you please update the module with the patch?

piridium’s picture

It's working here too on Drupal 9.2.7 and module 8.x-2.7.
Thanks @larowlan for the patch in #25.

srihari manepally’s picture

Patch in #25 worked for me on Drupal 9.2.7 and module version 8.x-2.7.
Thanks @larowlan

reloxo95’s picture

Path in #25 worked for me on Drupal 9.2.8!
Thanks! :)

igonzalez’s picture

#25 worked for me on 9.3.2! Thank you!

jbreslow’s picture

+1 for #25 on 9.2.8! Thank you!

mfb’s picture

Just for the record, another relatively simple way to add a redirect is add an anonymous function as a listener for the response event, see e.g. https://git.drupalcode.org/project/securelogin/-/blob/8.x-1.x/src/Secure... (this listener will only be added when it's relevant instead of on every request)

trebormc’s picture

+1 for #25 Thank you!

darvanen’s picture

@larowlan, before I try making a patch that attempts the approach given in #77 (anonymous function listener inside hook_login), are there any reasons you can think of that it shouldn't be done that way?

larowlan’s picture

I don't have a dog in the fight sorry. I've never used this module, I only came here from some reports of critical regressions in core for 9.2 that turned out to be because of this module.

shamsher_alam’s picture

Thanks to all. I will create one stable release of this module next week.

stijnhau’s picture

Have patch #25 tested it on 9.3 and i got
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "http_middleware.redirect_after_login". in Drupal\Component\DependencyInjection\Container->get() (line 156 of core/lib/Drupal/Component/DependencyInjection/Container.php).

mrogers’s picture

@stijnhau I got the same issue — clearing the cache resolved it for me.

stijnhau’s picture

@mrogers you are right it seems i forgot to clear the cache ;(
@Shamsher_Alam can you create a release?

lapaty’s picture

i applied #25 patch to the last module version (8.x-2.7) and the last core version (9.3.2) and the problem was finally fixed!!. Thanks!!

almador’s picture

#3 https://www.drupal.org/project/login_destination works with Drupal 9.2-beta1.

Thanks, Sokru!

jglynn’s picture

Is there going to be a new release with this issue fixed?

bburg’s picture

I am using the patch from #25, and upgraded to 9.3 while also working on an effort to move to Single-Sign-On via https://www.drupal.org/project/simplesamlphp_auth, and I appear to be running into this issue again.

My mistake, I was looking at a site without the patch, and a different codebase in my IDE with the patch. This is indeed working.

mindhunter75’s picture

Thank you larowlan! #25 works perfect!

medinasod’s picture

Yes, thank you larowlan, #25 worked for me!

goldin’s picture

It looks like we have substantial confirmation this patch works. Do any of the maintainers have a position on when it can be rolled into a new release? The reason I ask is: when you install a new site with Composer, and you have this module as a requirement in your json file, and you visit your new site and try to log in and mysteriously cannot, and you forgot about this patch since your last new site installation, it can kill some time until you retrace your steps and ... oh yeah, the patch.

darvanen’s picture

At #81 the primary (?) maintainer noted there would be a commit and release three months ago.

OSS can be quite slow on that front, almost everyone is volunteering their own time. If you would like things to move faster, you could potentially offer to co-maintain the module.

goldin’s picture

For those looking for an alternate solution until the patch is merged, you can achieve this functionality with the ECA module. Create a model with an event “Login of a user” a condition “Role of current user” and an action of “Redirect to URL.”

umac_de’s picture

#25 worked for me also on 9.3.8! Thank you!

nevergone’s picture

Status: Reviewed & tested by the community » Needs work
Issue tags: +Needs tests

#25 is works well, but this issue needs tests.

superlolo95’s picture

#25 ok for me too

philipp jor’s picture

I have same Problem! Please stop this module in publishing!
After activating the Module und routing to "front" for Admin user, the Admin-User cannot visit all Admin sites!
Drupal 9
redirect after login version 2.7

kevinquillen’s picture

Use the patch, it will fix it.

mav_fly’s picture

I have the same problem using the module "Login Destination module". When disable the module user can login without any problem.
Can someone help to solve with this redirecting problem.

cilefen’s picture

@Mav_fly I think that is a question for the maintainers of "Login Destination" module.

mav_fly’s picture

@cilefen Ok I will post my question in this module issues. I posted here me question due to I was browsing my problem by the internet.
And seems that both module suffer this same problem.

darvanen’s picture

No worries @Mav_fly, it can take a while to get used to how the Drupal community operates :)

I'm 100% sure @cilefen's comment was only intended to be helpful.

manishdrupaldev’s picture

#25 worked for me on 9.3.13
Thank you!

MCID’s picture

I have the same problem, running without problem in drupal 9.1.5 but in other sites whit drupal 9.3.15 I cant login.

norman.lol’s picture

I wish the patch would get committed, a new release brought on the way and a follow-up issue created for the tests.

joshuautley’s picture

#12 worked for us

uninstall the module for now

drush pm:uninstall redirect_after_login

raynaldmo’s picture

#25 works on Drupal 9.4.4 Used PHP 8.1 (8.1.5) but no reason to think it won't work with PHP 7.4 or 8.0. Many thanks for this fix!

To apply 3214949.patch given in #25

* Check that cweagans/composer-patches project is installed - if not, use composer to install it
composer require "cweagans/composer-patches:^1.6.5"

* I happen to be using version 1.6.5 but other versions probably work too
* For more info see, https://github.com/cweagans/composer-patches and https://www.mediacurrent.com/blog/composer-patches-dependency-your-proje...

* In your composer.json file, add patch information in "extra" → "patches" - see below

...
    "extra": {
        "drupal-scaffold": {
            "locations": {
                "web-root": "web/"
            }
        },
        "patches" : {
            "drupal/redirect_after_login" : {
                "#3214949: Headers have already been sent after upgrade to Drupal 9.2 (can't login)  (https://www.drupal.org/project/redirect_after_login/issues/3214949)":"https://www.drupal.org/files/issues/2021-06-20/3214949.patch"
            }
        },
        "composer-exit-on-patch-failure": true,
...

* Run composer update nothing to apply the patch

composer update nothing
Gathering patches for root package.
Removing package drupal/redirect_after_login so that it can be re-installed and re-patched.
  - Removing drupal/redirect_after_login (2.7.0)
Deleting web/modules/contrib/redirect_after_login - deleted
Loading composer repositories with package information
Info from https://repo.packagist.org: #StandWithUkraine
Updating dependencies
Nothing to modify in lock file
Writing lock file
Installing dependencies from lock file (including require-dev)
Package operations: 1 install, 0 updates, 0 removals
Gathering patches for root package.
Gathering patches for dependencies. This might take a minute.
  - Installing drupal/redirect_after_login (2.7.0): Extracting archive
  - Applying patches for drupal/redirect_after_login
    https://www.drupal.org/files/issues/2021-06-20/3214949.patch (#3214949: Headers have already been sent after upgrade to Drupal 9.2 (can't login)  (https://www.drupal.org/project/redirect_after_login/issues/3214949))

* Run composer validate (ensures composer.json and composer.lock are in sync)
* In my case a few benign warnings are reported - In your case you may not get any warnings at all
* Check that you get no errors - if you do get errors, something went wrong, check composer.json for syntax errors and/or consult the documentation given above

composer validate
./composer.json is valid, but with a few warnings

* That's it. Check that redirect after login now works.

fox mulder’s picture

#25 works form me too.

drupal core version: 9.2.21

candelas’s picture

patch #25 solved this issue. thanks!

anishnirmal’s picture

The patch at #107 works fine. This is tested in Drupal 9.4.4 with PHP 8.1 and MySql 8.

anishnirmal’s picture

Status: Needs work » Needs review
mpadilla’s picture

#25 works form me too.

drupal core version: 9.4.6

joelseguin’s picture

I've been using patch #25 over the past several months on various sites without issues. I have just applied it again to D9.4.8 with PHP 8.1 and worked perfectly. I had actually forgotten to include it and noticed while testing. Any chance of cutting a new release that includes the patch as the module is unusable when using a newer version of Drupal (and obviously can easily be forgotten)?

imclean’s picture

@joelseguin, composer can apply the patches for you. See #108 for details.

david.muffley’s picture

I've been using #25 for quite some time and it's been working perfectly. I've also tried out #107 and it worked too. I'm personally a fan of #107's approach as it's a lighter weight implementation than the middleware created in #25.

p4trizio’s picture

Status: Needs review » Reviewed & tested by the community

#107 solves the problem

dravenk’s picture

#13 work for me.

ivntres’s picture

I had the same problem, but #107 works for me.

raveen_thakur51’s picture

I was having this issue for "Headers are already sent in response.php"
options:
no_cache: 'TRUE'

I added the above line in my routing file and it worked for me.

aaronbauman’s picture

#107 worked for me.
Please commit and push a new release.
This module broke my site.

eelkeblok’s picture

Issue summary: View changes

I updated the issue summary in hopes of giving this a boost; the module is currently incompatible with supported versions of Drupal. There are currently two viable patches, both of which take different approaches. People have reported both of them working (I myself have been running a site with #25 for several months). Can the maintainers please make a choice, with, I guess, some bias towards #25 as it has a longer history.

ahmadhalah’s picture

Patch #25 worked for me

maithili11’s picture

Patch #107 worked for me

larowlan’s picture

For everyone who is commenting here that the patch works, I think its clear this module isn't actively supported anymore, and perhaps you might like to step up to co-maintain.

lonalore’s picture

Patch #107 works fine with Drupal 9.5.7.

igonzalez’s picture

Patch #107 worked for me

prempatel2447’s picture

Hi Everyone,

We will include all the changes in new release with Drupal 10 compatibility.

raveen_thakur51’s picture

++

seantwalsh’s picture

@prempatel2447 Do you have a date for releasing the D10 compatible version?

vladimiraus’s picture

Version: 8.x-2.7 » 8.x-2.x-dev
Status: Reviewed & tested by the community » Needs review

Thanks everyone. Moved to PR and refactored after D10 branch commit #3289274: Automated Drupal 10 compatibility fixes.
Seems like middle ware is a bit of a overkill.
Please review.

vladimiraus’s picture

dcam’s picture

@VladimirAus it seems like your commit didn't get uploaded to the MR.

generalredneck’s picture

Everyone,
So I was upgrading to D10, and we had been using #107.

Obviously when I tried to apply it to the 8.x-2.x branch locally via composer, the patch failed. So I came here to reroll it.

That said, I got to looking at the latest commit, and I've found that the Reason VladimirAus's MR shows up as empty is because #25 has been merged and updated for D10. If you look in #3289274: Automated Drupal 10 compatibility fixes, you will see where VladimirAus is asking everyone to review that commit and validate it function's correctly so we can have a new version.

So in theory, provided my tests work here in a little bit, this could be marked as "Fixed" provided we want to release without tests of the redirect functionality.

generalredneck’s picture

The commit appears to work for D9.5.10. It redirects as it's supposed to and even respects when a different ?destination=/some/path is passed to the form. I'd call this good minus specific tests provided the maintainer wants those to be a part of this work. Given that, the fix for this has already been commited to 8.x-2.x, so I'm inclined to vote we don't make this linger and create a new task for creating the test.

jaapjan’s picture

Agree with comment #136. Fastest way forward seems to be to mark story as fixed and tag new release on current dev branch. Considering Drupal 9 is EOL in a few weeks it would be extremely helpful if a maintainer could tag a new release.

For now one could use version constraint dev-2.x#2fa71b6196c64b93e8377f02e2e6d827bbe67ec8.

dzinkevich’s picture

Verifying that this works. We've been using this patch in production since it was released a year ago.

be-a-helper’s picture

@VladimirAus Tagging you in regarding the request for a prod release. How can the community support you in this?

adrianm6254’s picture

Is there a scheduled date for the D10 release yet?

2pha’s picture

the dev (8.x-2.x) version seems to have worked for me on a D8 to D9 upgrade.
Not sure why there is a specific 8.x-2.x-dev branch in the repo, but doing a composer require drupal/redirect_after_login:2.x-dev`does infact install (8.x-2.x)

jackfoust’s picture

Am I correct in assuming that 2pha's comment above requiring the dev branch fixes both the "header already sent" issue and resolves the D10 compatibility issues?

2pha’s picture

@jackfoust I only moved the site to the latest D9 at this stage, I'll check back in here when I move the site to D10 in the near future.

pvbergen’s picture

Status: Needs review » Reviewed & tested by the community

We had the patches from this issue in use for over 2 years.
I can verify that MR#5 applies correctly and resolves the issue on D9 and D10.

djg_tram’s picture

@pvbergen -- This module is obviously abandoned, I'd say the best solution is to quit using it and if you need the functionality, use your own module or start another one. I never did, as I described two years ago, I came here to chime in because my own code had the same problem.

Two years ago, one of the core members of the Drupal team clearly described that the solution this module had used previously was plainly wrong and provided the proper solution. After that, two days of testing should have been enough to change it for once and for all. This never happened, so, after all that time, there are inevitable conclusions to draw...

cilefen’s picture

nevergone’s picture

Version: 8.x-2.x-dev » 3.0.x-dev

@Prem Patel is somewhat active developer.

  • VladimirAus committed 7371e1c3 on 8.x-2.x
    Issue #3214949 by larowlan, Andrew Answer, djg_tram: Headers have...
vladimiraus’s picture

Great work everyone!
Merged into 2.x and 3.x. Working on the releases.

  • VladimirAus committed 7371e1c3 on 3.0.x
    Issue #3214949 by larowlan, Andrew Answer, djg_tram: Headers have...
vladimiraus’s picture

Version: 3.0.x-dev » 8.x-2.x-dev
Status: Reviewed & tested by the community » Fixed
vladimiraus’s picture

Released in 3.0.0 and 2.8. 🍻

Status: Fixed » Closed (fixed)

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