-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
redirect to original path after login #24
Conversation
I like the idea behind this PR, but at the moment it would break people using the If I'm using the |
pseudocode:
? |
I've just realised that's what
is for... Ignore my previous comment |
Could you possibly add a test for the |
Will do. Probably to come on Monday |
@JoelSpeed - tests added. Let me know if you need anything else |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
One minor nit then it's good to go! Thanks
oauthproxy_test.go
Outdated
expectedRedirect: "/foo/bar", | ||
}, | ||
{ | ||
name: "request under of ProxyPrefix redirects to original URL", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This comment isn't quite right
name: "request under of ProxyPrefix redirects to original URL", | |
name: "request under ProxyPrefix redirects to root", |
fixed |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add a note to the Changelog noting this bug fix?
Description
After the user logs in, redirect them to their original URL rather than just /
Motivation and Context
The user wants to go to their destination, no the root URL.
How Has This Been Tested?
Run it against an upstream. Trash all cookies, then try to access it with a non-root URL
Checklist: