Improve compatibility with OpenID Connect destination

I'm curious, does this retain the functionality of Redirect After Login for you? OpenID Connect seems to always save a destination to session, even if none is specified in the query destination or in the config (reference on 3.x, but same on 2.x: https://git.drupalcode.org/project/openid_connect/-/blob/3.x/src/OpenIDC...).

I am also integrating these two modules (as well as Anonymous Login) and have suffered a lot of pain to get them to work together (even more so when the site is in a subdirectory); not only in the beginning when I first added OpenID Connect into the mix, but on an ongoing basis as the modules have updates.

I was looking at https://www.drupal.org/project/openid_connect/issues/3276203 ✨ Create a hook to set the destination path after authentication. Needs work , but the proposed solution was not sufficient because it does the alter before the user has authenticated, and (obviously) Redirect After Login logic is based on user roles which can only be determined after login.

My original solution (a few years ago) was to create a custom route and controller that would reproduce the logic in the Redirect After Login user login alter hook and return a redirect; then in the OpenID Connect configuration I set that page as the Redirects > Login path. The downside is that it resulted in an extra (interim) redirect, but honestly after fighting with it for so long to get it working at all (at that point I already had the #25 patch from https://www.drupal.org/project/redirect_after_login/issues/3214949 🐛 Headers have already been sent after upgrade to Drupal 9.2 (can't login) Fixed to use the middleware, and needed to create a custom patch on the OpenID Connect Controller to redirect using the Redirect After Login middleware for the redirect if the module was installed), I was just happy that it worked at all.

However, I have had to rethink the approach since:

• https://www.drupal.org/project/redirect_after_login/issues/3214949 🐛 Headers have already been sent after upgrade to Drupal 9.2 (can't login) Fixed switched to setting the destination on the request query, rather than using the middleware approach that was accepted for so long
• A redirect response is not respected when the destination query parameter is set, due to a Drupal core oddity (as noted in the 10+ year old changelog record for drupal_goto, https://www.drupal.org/node/2023537 → , which there are multiple related issues about including https://www.drupal.org/project/drupal/issues/2950883 🐛 Allow form redirects to ignore ?destination query parameter Fixed , https://www.drupal.org/project/drupal/issues/2640672 🐛 Make redirect.destination service update request Closed: duplicate , and https://www.drupal.org/project/drupal/issues/3269760 → )
• The super heavy-handed Drupal destination redirection logic only happens if you return a redirect response
• One can't easily create a redirect response from the destination parameter, despite the documentation in https://api.drupal.org/api/drupal/core%21lib%21Drupal%21Core%21Routing%2..., because that results in an error (at least when the site is in a subdirectory, due to destinations containing the base path)
• One may be tempted to manually "fix" the destination from the destination service before creating a redirect, such as by using https://www.drupal.org/project/drupal/issues/3044656 ✨ Add a helper method to strip subdirectories from URL paths Needs work or creating your own logic to do the same thing; but that seems rather hacky, and could cause issues such as those noted in https://www.drupal.org/project/drupal/issues/2582295#comment-13038790; 🐛 Confirmation cancel links are incorrect if installed in a subdirectory Needs work this is especially the case with OpenID Connect because the config for the login path does not have the base path in it, but there is no way to know whether the session destination came from the destination parameter or the config

My current thought on what might work well is as follows, but I haven't quite gotten to trying it out:

• In Redirect After Login, create a service to wrap what's currently occurring in redirect_after_login_user_login, so that any contrib/custom module can reuse it without copy/paste (unlike what I did previously)
• In OpenID Connect, do not save the session destination if it wasn't provided in the query
• In OpenID Connect, implement https://www.drupal.org/project/openid_connect/issues/3276203 ✨ Create a hook to set the destination path after authentication. Needs work after login (in the controller), passing (separately) the saved session destination and the OIDC configured login destination
• In OpenID Connect, set the destination on the request to the determined redirect (what is being sent as the response); that way, any modules such as this one or others like it (or in custom code) can respect an existing destination and not override it
• (Possibly optional) In Redirect After Login, implement the OpenID Connect destination alter hook calling the new service; however, it seems like this would be easy enough to implement in custom code; maybe it would work without custom code by setting the module weights (I haven't tested, but my thought is that it wouldn't work)

Very interested in thoughts and feedback.

Sorry, I am clearly not as deep into this issue as you are. I was taking the code in Redirect after Login as it is. Please apply the patch in your setup and try it out. We also use the combination of Redirect after Login, Anonymous Login and OpenID Connect, and it works fine.

Thanks. After further digging into the module's code I did eventually get it to work with this patch by setting the OpenID Connect login destination to "/user/login". However, I don't want to do that because my app's login is not at that path (overriding the "user.login" path in a route subscriber) and it seems likely to cause issues down the road or at the very least confusion; probably a separate issue should be opened for using the login route path rather than the hard-coded "user/login" since that isn't unique to this fix. When debugging I also found that the existing check on the login path is not accounting for the site being in a subdirectory either (this seems to be the issue I run into the most); again, probably a separate issue should be opened for that.

Instead, I submitted a patch on the OpenID Connect issue for the site being in a subdirectory, which also allows it to seamlessly work with Redirect After Login (without this patch or other changes): https://www.drupal.org/project/openid_connect/issues/3463457#comment-160... 🐛 Redirect does not work with base paths Active .

I'm not sure if I will open the two other issues in this module at the moment because, as far as I can tell, it isn't causing any problems for me when I don't need/use the integration patch on this issue.

Thanks. After further digging into the module's code I did eventually get it to work with this patch by setting the OpenID Connect login destination to "/user/login".

Setting the login redirect in OpenID Connect to /user/login should prevent setting the openid_connect_destination. In an unpatched installation, you can see that in OpenIDConnectSession::saveDestination(). Therefore, the change in this issue would also have no effect.

... using the login route path rather than the hard-coded "user/login" ...

You are right. The check whether the destination is the user login could be more verbose. This is not in the scope of this issue, though. After checking the prior logic in OpenIDConnectSession::saveDestination(), I adjusted the changes here.

I submitted a patch on the OpenID Connect issue for the site being in a subdirectory ...

After checking out the patch, I imagine it will be hard to get this in. The OpenID Connect module is not responsible for handling the "pure" destination query parameter. The changes are also not backward-compatible because the behavior of OpenIDConnectSession::saveDestination() changes. I see this as a commonly used extension point. We are also changing the behavior of that method.