#2670118: Make getPostLoginDestination / getPostLogoutDestination configurable started changing redirects too, however since I
* am not 100% comfortable with all aspects of the URL/redirect code
* want to change more than was present in that patch,
that part of the code has been split off into this issue.

If things start breaking, you can scold me here :-)

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

roderik created an issue. See original summary.

roderik’s picture

Issue summary: View changes
Status: Active » Needs work
FileSize
6.72 KB

Baseline: status as of #2670118-24: Make getPostLoginDestination / getPostLogoutDestination configurable with
* no_cache: TRUE added to the routing.yml (for links both before and after the redirect to the IDP);
* a large comment inserted somewhwere.

roderik’s picture

Status: Needs work » Needs review
Issue tags: +DevDaysSeville
FileSize
10.51 KB
7.34 KB

@seanB: can you try out this patch? You'll probably need to patch the -dev version with this issue, in order to get the redirect to your external site to work. (Because I removed the following from the -dev code: #2670118-9: Make getPostLoginDestination / getPostLogoutDestination configurable. So you are the dedicated guinea pig now.)

Done:

  • Do not let the PHP toolkit do redirects; do it ourselves. Then we can have more influence on how to do redirects (even though it might not be necessary). Credit droath.
  • Unify code: replace the other RedirectResponse objects by TrustedRedirectResponse too.
  • Move all that into a helper function with a boatload of comments. (The amount of comments is influenced by the fact that I still don't get all the internal details, but I've stopped caring.)
roderik’s picture

FileSize
10.51 KB
7.34 KB

Renamed redirectResponseFromUrl() to createRedirectResponse(). ('redirect' is also a verb.)

Interdiff is from 1 to 3.

seanB’s picture

Status: Needs review » Reviewed & tested by the community

#3 seems to work for me. Nice work!

pcambra’s picture

+1 to the RTBC, looks great

roderik’s picture

Status: Reviewed & tested by the community » Fixed

Thanks all! Finally committed.

  • roderik committed 56cb294 on 8.x-2.x
    Issue #2863340 by seanB, roderik, droath: Fix/unify usage of redirects
    

Status: Fixed » Closed (fixed)

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