Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Accept a "from" parameter in the urls for redirection #232

Closed

Conversation

Aeon
Copy link
Contributor

@Aeon Aeon commented Dec 3, 2014

the /login url accepts a new parameter that will be combined with service parameter for the user redirection after successful login.

This should fix #194 for real - dynamically setting service_url is a partial solution, but a bad one, because with constantly changing service_urls, the tickets are constantly invalidated (because the new request's service url won't match the previous request's service url).

I'm surprised this wasn't considered in the CAS spec, but I can't find any indication there that the original authors ever considered redirecting the user to a deep url within the project; seems like their assumption was always that the user hits the home page no matter what.

I tried to do this by looking at the Referer header first; that would have been more elegant and not required the users to upgrade their CAS integration gems - but it seems that many browsers do not send the referrer as the source of redirect (or at all) when redirected via 302 status code, so unfortunately we have to be explicit about specifying this.

This replaces pull request #231, not sure why I can't edit that one any more.

this should be a path that will be combined with @service_url parameter
for the user redirection after successful login.
otherwise the stale ticket is still in the return url
and the user gets thrown into an infinite redirect loop
@Aeon
Copy link
Contributor Author

Aeon commented Dec 9, 2015

Hi, it's been a year :) Any interest in merging this?

@mitfik
Copy link
Contributor

mitfik commented Dec 9, 2015

Of course that there is an interest, just lacking of time :)
I will try to take a look on that today.

@mitfik
Copy link
Contributor

mitfik commented Dec 9, 2015

Lack of tests, but for me looks ok. I just squashed those commits to one #253 . Thanks a lot for contribution!

@mitfik mitfik closed this Dec 9, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Dynamically setting service_url?
2 participants