Skip to content
This repository has been archived by the owner on Jan 24, 2019. It is now read-only.

Redirect to uri even if --skip-provider-button #676

Closed
wants to merge 1 commit into from
Closed

Redirect to uri even if --skip-provider-button #676

wants to merge 1 commit into from

Conversation

leon-barrett
Copy link

The normal behavior is that if a user is going to page
https://hostname/x but is not authenticated, they are sent to login and
then redirected back to https://hostname/x.

I discovered that if the flag --skip-provider-button is given, then
after login the user ends up at https://hostname/.

I believe this to be a bug. That behavior seems to happen because the
current URI is only extracted when redirecting to the sign-in page form,
not when the OAuth flow is started, so if that form is skipped, the
current URL is lost. Therefore, I made a change to extract the current
page to send into the OAuth flow.

This change fixes that bug and adds a test.

The normal behavior is that if a user is going to page
https://hostname/x but is not authenticated, they are sent to login and
then redirected back to https://hostname/x.

I discovered that if the flag --skip-provider-button is given, then
after login the user ends up at https://hostname/.

I believe this to be a bug. That behavior seems to happen because the
current URI is only extracted when redirecting to the sign-in page form,
not when the OAuth flow is started, so if that form is skipped, the
current URL is lost. Therefore, I made a change to extract the current
page to send into the OAuth flow.

This change fixes that bug and adds a test.
@leon-barrett
Copy link
Author

While the tests failed, they seemed to fail on dependencies (none of which I touched).

https://travis-ci.org/bitly/oauth2_proxy/jobs/462032033#L588

ensure Solve(): No versions of gopkg.in/fsnotify.v1 met constraints:

https://travis-ci.org/bitly/oauth2_proxy/jobs/462032032#L570

../../../golang.org/x/oauth2/google/default.go:38: syntax error: unexpected = in type declaration

I can investigate what's needed to update those dependencies, but I'd naively expect that you'd have more luck :)

@leon-barrett
Copy link
Author

Ah, it looks like #610 is at least a partial fix.

@ploxiln
Copy link
Contributor

ploxiln commented Dec 3, 2018

this repo is unmaintained, see #628 (comment)

@leon-barrett
Copy link
Author

Thanks. I'll just close this down.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants