-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Current OIDC implementation requires provider to implement the /.well-known/openid-configuration endpoint #27
Comments
Hi @tlawrie, I definitely think this is something we should add support for! I would say keep it within the main OIDC provider though since all the rest of the logic would be duplicated and we would end up (probably) having some drift between the two implementations. I would suggest we add a |
Hi there, to chime in, we would also like to support this one, as we use Azure AD B2C as our OIDC provider, however this expects an additional policy parameter in the URL. So we also currently have our own fork of go-oidc adding this URL parameter to the discovery endpoint, but a manual one would also drop the need for this fork :) |
@marratj Do you have any more information on this policy parameter? We might be able to include it in a PR to fix this issue or follow up with a separate PR after that is merged |
It's a simple URL paramater called So the auth & token endpoints would be
instead of simply
That's all that is different. However, in go-oidc the |
So in which direction would you want to go? I just reread this thread and think that a However, I need dig a bit further into how much ID token validation and the token refresh currently depend on the discovery endpoint. From a first glance it might be enough to replace the oidc.NewProvider() function with one that returns a oidc.Provider struct with arbitrary auth/token endpoints. (just as @tlawrie has mentioned in his NewManualProvider). I will try this out coming next week. |
Apologies for the delay in getting back to this. Happy to merge in my previous code as a flag via PR. @JoelSpeed with regards to the NewManualProvider method, what do you suggest is the best approach. Currently it is stored in a fork of the go-oidc library. https://github.com/tlawrie/go-oidc/blob/v2/oidc.go Should I submit a PR to go-oidc as well for that to be merged in? Or is there a better way to handle this? My knowledge of Go is reasonably limited. |
Just had a quick look, the So the two quick options I see for now are either opening a PR with go-oidc to include manual setting of the endpoints or forking go-oidc completely into oauth2_proxy (which also isn't ideal). |
Thanks to @ploxiln I found a way how we can feed the go-oidc Provider arbitrary Authorization & Token Endpoint URLs :-) I will create a PR later today for you to test and review. |
I have submitted the issue to go-oidc (coreos/go-oidc#192) If that gets resolved / PR submitted then we can fall back on that. For now, your solution seems to be a good work around. I can try testing it from my end either end of this week or start of next week. @JoelSpeed what are your thoughts? |
I think it would be cleaner to implement this as suggested using a manually created To me constructing a manual transport seems kinda hacky and as all we need from the Instead, @marratj, can we make the |
Each block would need to set |
I will most probably take a deeper look into the Verifier instead of "smuggling in" the fake HTTP client next week. I will update this issue and the PR #41 accordingly as I go along. |
@JoelSpeed agreed. I think the suggestion from go-oidc is good. And agree with the approach of @marratj I can test out start of this week as soon as the change is made. |
@marratj let us know if you want us to push a PR for the change. |
@tlawrie I will most probably not be able to work on it myself until beginning of next week, so if you want to go ahead before that, feel free :) |
I have updated #41 to use the recommended solution by go-oidc. |
Update master.yaml
Expected Behavior
Current Behavior
The current implementation expects the OIDC provider to implement the discovery endpoint, however this is optional in the spec and unfortunately some enterprises do not implement this, such as IBM.
Possible Solution
On my fork of the bitly version, I implemented an additional provider with a fork of the go-oidc package that had an implementation for NewManualProvider, skipping the call to the discovery endpoint.
We could implement as a new provider or use a flag to let oauth2_proxy know that we are trying to integrate to a provider that does not have the discovery endpoint available.
Alternatively we could handle the error and proceed.
Steps to Reproduce (for bugs)
Configure the oauth2_proxy against an OIDC provider that does not have the discovery endpoint.
Context
Some enterprises do not implement or expose the /.well-known/openid-configuration endpoint
Your Environment
The text was updated successfully, but these errors were encountered: