RuntimeException: Failed to start the session because headers have already been sent

This is now fixed in the dev release and in the upcoming tagged release.

I came across this issue investigating why the destination parameter is still being ignored #3152353: Redirect to destination URL after login and noticed that the session was only saved after headers have been sent already.

I'm surprised by your approach, because how would it be able to save the "destination" in the session/tempstore if you just terminate the request right away? I haven't tried your solution yet to be honest so I might be missing something, but I was able to fix it temporarily by passing the response upstream instead of sending it on the spot.

I love this project and it does a great job providing SSO for our site. But without any test coverage and community review before changes are being pushed, I'm somewhat hesitant to keep using it in production. Let me know if there's any way I could help to improve this.

EDIT: After doing more testing I found out that if I return the response, it's actually redirecting to "destination" without logging in. But in order for the destination to be picked up, I need to save the session before I send the response and terminate the request. Else "destination" is ignored, because a new session is created when returning back to the site.

    $response = new TrustedRedirectResponse($authUrl);
    $this->request->getSession()->save();
    $response->send();
    exit();

Hi,

This does indeed fix the problem.
There are some different approaches found, like here:

https://www.drupal.org/project/drupal/issues/2852657#comment-14151263

Actually I was to fast.
The errors are gone but the access token is not saved to the temp store.

Thanks for the help and the pointers. I've just created a MR which includes the code from @phma. You can review this.

@phma, regarding the help. You are always welcome in helping this module grow both in usage as in maturity. We are planning to add some test coverage to this module, if you are inclined to help, this would be our main focus for the foreseeable future.

@fabianderijk Thanks for creating the MR. I never go around doing it myself. Also, our website no longer needs the destination parameter to work so desperately. Still, I've applied the patch on our site and can confirm that it is working locally on my machine. At least I can help others, and it will be working if our site ever requires it.

As I'm changing roles soon, my focus is going to shift, and I'm probably no longer able to help you guys, sorry. But I hope I can continue to contribute in some ways in the future. In the meantime, keep up the good work!

I'm marking this as RTBC, but feel free to change it back if you want someone else to test it.

This has now been added to the 3.0.x branch dev release, and is added to the new 5.0.0 release tag