It looks like #3275760: Redirect during authentication/authorization error → may be a duplicate of this, related to 💬 Infinite redirect loop when using Keycloak SSO and authorization fails Postponed: needs info . I had this issue as well, and for the past couple years I've been using a local custom patch that invokes an alter hook on the destination in the redirect controller (with context of the client id, success, and id token if available); I never submitted it because of where things ended up in #2953289: Add option to alter the destination during authenticate in the redirect controller. → and didn't think to open a ticket for this specific use case. There is already an alter hook on logout in the redirect controller, so it seems reasonable to have one on login IMHO.
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.
I was struggling to integrate this module with Anonymous Login and Redirect After Login in combination with the site being in a subdirectory. I also had to retain the prefixing "/" from the destination in the session, but came up with a slightly alternate approach in the redirect controller that let me seamlessly integrate with Redirect After Login by distinguishing between a user-provided destination and a default destination (to do this, I set this module's login destination to nothing / empty string).
I previously put a comment on a Redirect After Login issue for compatibility with this module, which details more of the things I had to work through and why/how I came up with this solution, in case it's of interest/use: https://www.drupal.org/project/redirect_after_login/issues/3513324#comme... ✨ Improve compatibility with OpenID Connect destination Active .
A potential benefit of this approach to set the destination in the query is that it doesn't require copying Drupal core code from RedirectResponseSubscriber (because it will be triggered by use of the query destination).
After testing I found that aliases were not quite working with subdirectory; pushed an additional commit to the MR that works for me.
I'm not sure, but it certainly seems plausible. I've had this patched locally for a couple years so I'm not positive, but that does seem vaguely familiar and might be one of the things I ran into. I'm planning to open a merge request with the changes.
Patching the commits on the merge request works for me, thanks.
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.
I am seeing the same Error executable_php, access was denied to the file. error message, both when I run security review in the browser or via drush (and I do have a drush file with the site URI specified); the difference is that I actually see the security review results in the browser (in addition to the error message), whereas via drush there is no review output.
Just wondering if there is a plan for how to target the element to style it? I am needing to style the new button, and I'm not seeing anything very targetable; I also tried a theme preprocess but it's pretty messy because of the template preprocessing that's done on the form that puts it into a table and the buttons are in individual rows in cells that do not have an identifier either. Is a separate issue needed to add some targetable CSS to the markup (I searched but wasn't able to find one)?
I agree, a change record or at least having it listed in whichever 10.2.x release it actually landed in would have been super helpful. This change was unexpected, and I do (and did) read the release pages to look for new features and other items of interest. After finding the problem in my app (as someone else suggested, there was a custom solution in place), I literally searched every 10.2.x release page (all 10 of them, at time of writing) for "remove", "multi", and (once I found it) this issue number "1038316" - and there were no references to this change that I could see. I only found this issue by searching core code for part of the resulting HTML, googling based on what I found, and following a link in a contrib issue (for a module I'm not using) that was in the search results. Can/should this issue be retroactively added to whichever 10.2.x release page it belongs to?
I'm not sure why there aren't any messages above about it, but this was committed and I'm seeing the permission in the 3.0.0 release (it looks like it was released in 8.x-2.7). I think this can be marked as fixed.
https://git.drupalcode.org/project/redirect_after_login/-/commit/debd493...
I can't say I looked to understand the implication of the tag attribute name not being set, but #3 works for me to get rid of the warnings.
It looks like #62 has a few more changes than #61, both are applying cleanly to 2.0.0-rc4 for me though I chose #62 since it seems more up-to-date with the merge request at quick glance. Thanks!