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

Add support for differing internal vs. public URLs. #1722

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

coderanger
Copy link

This is helpful with keeping token management traffic purely internal in Kubernetes while still sending back a public-facing link for auth and callbacks.

This is helpful with keeping token management traffic purely internal in Kubernetes while still sending back a public-facing link for auth and callbacks.
@JoelSpeed
Copy link
Contributor

@coderanger Can you elaborate on the use case for this and how it works? As far as I was aware, the issuer URL should match the endpoint that the user hits at all times as part of the OIDC spec, is this not the case? Is there precedence for this being done elsewhere?

@coderanger
Copy link
Author

The spec does say that and it makes sense when the issuer is Google, but when trying to set things up for an internal-only service it's nice for internal-only traffic like /tokens to go via the Service, it's not zigzagging all the way out to my external ingress layer. This helps with both availability planning (fewer things in the path between the two services) and latency. But at the same time, the user-facing bits need to have non-cluster hostnames for the OAuth redirection dance.

@sagikazarmark
Copy link
Member

@coderanger I can see why this is potentially useful, but I'd like to get more context about what the spec says about such implementation, so if you could quote or point us to the relevant section of the spec, that would be great. Thanks!

@coderanger
Copy link
Author

As I read through the spec to try and answer that question, I guess actually the spec never specifically states that the authentication endpoint has to be "under" the issuer URL. In fact while the issuer ID needs to be a valid URI, it has no actual connection to the network address of the IdP or any of the endpoints. https://openid.net/specs/openid-connect-discovery-1_0.html#IssuerDiscovery makes this explicit for Issuer Discovery, but the only specific requirement on authorization endpoints is that they use TLS (https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint).

@JoelSpeed
Copy link
Contributor

Thanks for those links and doing the research. I think you're right, this doesn't break the spec.

It does raise the question to me though, can you not manually configure the clients? Since discovery is an optional implementation, your clients should be able to manually configure the token, key and userinfo endpoints right? Does this maybe break the serve mux that Dex has built in 🤔

I guess this is only useful in scenarios where all clients in your configuration can access the internal endpoints? Eg, you couldn't use something like a command line login for users with this?

I think it would be good to update the description of the PR to explain in a bit more detail the motivation and usage of this change, plus we should ideally update the docs to explain the new feature

@janchochol
Copy link

We are trying to solve similar problem, where this PR can help.
I will try to explain our use case, so it is more clear, why we need this.

We are developing application (consisting of several services), where we need authentication. We decided, that we will only support OIDC to make implementation more simple.
There are several situations, where we can not use customer-provided OIDC (customers which does not have OIDC or during development), so we would like to include integrated OIDC (we are mainly considering dex).
Our application will provide multiple deployment options - k8s (for production use), all-in-one docker image (all services in one container - for simple application evaluation) and docker compose based (used by majority of our developers and some automated tests).
Problem is that OIDC must be accessible from internal services and from user browser.
We can currently achieve this in k8s (internal services can use public endpoint) and all-in-one image (everything is localhost), but we have problem with docker compose.
OIDC must be accessible in docker compose network (using service name DNS name) and in user browser (we can not use internal docker compose network, so we need to use localhost and exposed port).

This is not possible to achieve using single issuer URL. It is possible to workaround most of problems using OIDC client configuration, but we can not influence URLs generated by dex itself (e.g. /callback URL), which are concatenated with issuer URL. And we need issuer URL usable for internal communication (we are using Spring Security client, and we can not change it there).
Thus we will us this feature to generate different URLs for external uses (from browser).

@janchochol
Copy link

Never-mind - we were able to achieve required behavior only with Spring Security configuration - it was incorrectly documented, so some reverse engineering showed possible approach.

Auth: s.absURL(s.publicURL, "/auth"),
Token: s.absURL(s.issuerURL, "/token"),
Keys: s.absURL(s.issuerURL, "/keys"),
UserInfo: s.absURL(s.issuerURL, "/userinfo"),
Copy link

@heidemn heidemn Dec 31, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't this quite arbitrary? The userinfo endpoint can be called by public clients as well.

Seems to me that the entire feature would be better handled on client side, and not in Dex.
E.g. in our backend code, we manually rewrite the host of the endpoint, to directly connect to the Dex container hostname instead of the public endpoint.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what you mean, /auth is the only thing that needs to be "public" because that's what is used when bouncing users to the underlying backend. The rest are using by authentication consumers, which in many cases are entirely internal.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If there is a way to do this client-side without violating the spec, I'm all ears.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E.g. a single-page app (using PKCE or Implicit Flow) may call the userinfo endpoint from the browser.
I was assuming that "public URL" = called from browser, "internal URL" = called from backend.

If there is a way to do this client-side without violating the spec

Not that I'm aware of... Rewriting the URL was just a pragmatic solution that worked for us.

What could also work, if public and internal URLs use the same scheme (HTTP / HTTPS): Add an /etc/hosts entry for the backend that should use the internal endpoint, like <public_dex_hostname> <internal_dex_ip>.

Copy link
Author

@coderanger coderanger Jan 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sure but that SPA would be a client so you wouldn’t enable this mode. This is specifically for the case of all clients are internal but the IdP isn’t (google in my case). This is common in the use case of using Dex for Kubernetes authentication.

@tybritten
Copy link

This is specifically for the case of all clients are internal but the IdP isn’t

This is exactly our situation. Could this be reconsidered?

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.

6 participants