Introduce ProviderVerifier to clean up OIDC discovery code #1559
Conversation
JoelSpeed
force-pushed
the
refactor-oidc-verifier
branch
2 times, most recently
from
5776779 to
7da9e06
Compare
pkg/providers/oidc/provider.go
Outdated
| // We implement this here as opposed to using oidc.Provider so that we can override the Issuer verification check. | ||
| // As we have our own verifier and fetch the userinfo separately, the rest of the oidc.Provider implementation is not | ||
| // useful to us. | ||
| func NewProvider(ctx context.Context, issuerURL string, skipIssuerVerification bool) (EndpointProvider, error) { |
There was a problem hiding this comment.
Starting with PKCE, there is more information which we may want to pull out of this than just Endpoints. For instance in RFC-8414 implemented by Google, Dex, and Auth0 (to name a few), there is now the code challenge methods that are supported in the payload.
It would be great to see a way to pull additional claims out of this response. We currently were able to do that in providers by using the oidcProvider.Claims(struct) to map extra claims to a struct.
There was a problem hiding this comment.
Good point! Perhaps instead of this being an EndpointsProvider interface it should be a DiscoveryProvider and then we can extend the interface to include a new PKCE method to retrieve the PKCE information, WDYT?
JoelSpeed
force-pushed
the
refactor-oidc-verifier
branch
from
7da9e06 to
dd7b0d5
Compare
|
@braunsonm If you could take a look at the new changes, I think this should now cater for the PKCE stuff you're working on |
JoelSpeed
force-pushed
the
refactor-oidc-verifier
branch
from
dd7b0d5 to
c3158eb
Compare
|
Rebased to resolve changelog conflict, no other changes so will merge based on previous approval |
This change restores the implicit behavior of github.com/coreos/go-oidc/v3/oidc.Provider.Verifier(...) which automatically trusts all algorithms supported by the issuer. Also drops use of github.com/dgrijalva/jwt-go Both of these regressions were caused by oauth2-proxy#1559 Signed-off-by: Monis Khan <[email protected]>
Description
Further to the changes in #1555 this cleans up the OIDC discovery and verifier code into a package so that we are no longer depending on the OIDC discovery from go-oidc
Motivation and Context
This should makes things a lot cleaner in terms of separation of concerns and also, as everything here is now behind interfaces, should allow us to start testing using fake implementations of the interface for providers, which should make things a little easier when unit testing.
How Has This Been Tested?
New Unit tests and old Unit tests still pass
Checklist: