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

oauth2: use issuer-prefixed auth URL in challenge redirect #509

Merged
merged 1 commit into from
Jun 14, 2017

Conversation

wyattanderson
Copy link
Contributor

In order to support running Hydra with a different path prefix behind
a proxy, issue a challenge token with an issuer-prefixed auth redirect
URL instead of the URL received with the auth request.

Based on discussion in #352

@aeneasr
Copy link
Member

aeneasr commented Jun 13, 2017

Hey thank you for your contribution! Could we maybe add a fallback in case the Issuer is not set?

@wyattanderson
Copy link
Contributor Author

Could we maybe add a fallback in case the Issuer is not set?

Sure, but what are valid uses of Hydra with an empty issuer (versus, say, the default of hydra.localhost)?

@aeneasr
Copy link
Member

aeneasr commented Jun 13, 2017

If you just want to play with it and run it locally, or as part of the tests, such a fallback makes sense. It should be easy to get started with Hydra (it's hard enough already ;) ) so let us make this as easy as possible!

@wyattanderson
Copy link
Contributor Author

100% agree; the ISSUER config option has a default value when not set, though, so this still works. I'd suggest (and was considering sending in a separate PR) changing the issuer logic:

When ISSUER is not set, construct an ISSUER URL from HOST (default localhost), PORT (default 4444), and --dangerous-force-http to set the scheme. The current default of hydra.localhost seems out-of-spec, since it doesn't have a scheme:

An Issuer Identifier is a case sensitive URL using the https scheme that contains scheme, host, and optionally, port number and path components and no query or fragment components.

@aeneasr
Copy link
Member

aeneasr commented Jun 13, 2017

HOST is unfortunately often not the real hostname, but rather the interface that hydra is listening on, this is why I used r.RequestURI in the first place here :)

@wyattanderson
Copy link
Contributor Author

Right, so, when starting out or testing locally it would work (because HOST defaults to localhost), and in production you'd probably be setting the ISSUER explicitly (ISSUER would not be constructed from HOST/PORT etc. when explicitly set), which I think is worth encouraging. This is roughly how Dex behaves.

@aeneasr
Copy link
Member

aeneasr commented Jun 13, 2017

Host defaults to empty catching all interfaces which is why hydra works per default in a docker container

@wyattanderson
Copy link
Contributor Author

🤦‍♂️ sorry; was reading the documentation wrong. I'll add the fallback.

In order to support running Hydra with a different path prefix behind
a proxy, issue a challenge token with an issuer-prefixed auth redirect
URL instead of the URL received with the auth request.

Signed-off-by: Wyatt Anderson <[email protected]>
@aeneasr
Copy link
Member

aeneasr commented Jun 14, 2017

Thank you!

@aeneasr aeneasr merged commit 688103c into ory:master Jun 14, 2017
@wyattanderson wyattanderson deleted the auth-redirect-url branch June 14, 2017 17:17
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.

2 participants