Current OIDC implementation requires provider to implement the /.well-known/openid-configuration endpoint about oauth2-proxy

[marratj]

It's a simple URL paramater called p.

So the auth & token endpoints would be

https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/authorize?p=policyname

instead of simply

https://login.microsoftonline.com/{tenant_id}/oauth2/v2.0/authorize

That's all that is different. However, in go-oidc the /.well-known/openid-configuration is hardcoded without any option to put a URL parameter there.

[JoelSpeed]

I think it would be cleaner to implement this as suggested using a manually created oidc.Verifier.

To me constructing a manual transport seems kinda hacky and as all we need from the oidc.Provider is to construct us a oidc.Verifier, it serves little purpose to use this route.

Instead, @marratj, can we make the if o.SkipOidcDiscovery an if-else, both of which should set o.oidcVerifier with either the discovered or manual verifier?

[JoelSpeed]

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 --skip-oidc-discovery flag plus the manual configuration required and make that part of the main OIDC provider, WDYT?

[marratj]

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 :)

[JoelSpeed]

@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

[marratj]

So in which direction would you want to go? I just reread this thread and think that a --skip-oidc-discovery flag would exactly be what we can use in this case.

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.

[tlawrie]

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.

[marratj]

Just had a quick look, the Provider struct from go-oidc has its types not exported, so we cannot simply create a Provider from the outside without hooking into go-oidc.

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).

[marratj]

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.

[tlawrie]

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?

[JoelSpeed]

Each block would need to set o.AuthURL and o.TokenURL too

[marratj]

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.

[tlawrie]

@JoelSpeed agreed. I think the suggestion from go-oidc is good. And agree with the approach of SkipOIDCDiscovery

@marratj I can test out start of this week as soon as the change is made.

[tlawrie]

@marratj let us know if you want us to push a PR for the change.

[marratj]

@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 :)

[marratj]

I have updated #41 to use the recommended solution by go-oidc.