coreos / go-oidc

A Go OpenID Connect client.
Apache License 2.0
1.98k stars 398 forks source link

Verify azp claim #355

Open manjinder-mckc opened 2 years ago

manjinder-mckc commented 2 years ago

Is there a specific reason, verifier does not support verifying azp claim? It could be possible for some scenarios that client is explicitly required to verify that azp claim matches ClientID. Can we add support for verifying that by any chance, it does not have to be enforced and can be optional (via verifier config) to satisfy certain use cases for say custom providers.

ericchiang commented 2 years ago

There's not a strong reason. I've only seen azp used in relatively niche scenarios where you still want to verify the audience rather than authorizing party.

Do you have a use case in mind? What's the desired security outcome?

manjinder-mckc commented 2 years ago

We have a custom Identity Provider, that for some reason supports other entities as ‘aud’ rather than ClientID, and the authorized party sends ClientID as part of ‘azp’ in the token, and requires us to validate that instead of aud.

I understand that perhaps this not a very common use case, but hoping it can still be beneficial to others. If we can provide optional config attribute to validate ‘azp’ to match ClientID.

ericchiang commented 2 years ago

Today you can pull the 'azp' claim out of the IDToken with the Claims() method and do your validation on that.

https://pkg.go.dev/github.com/coreos/go-oidc/v3/oidc#IDToken.Claims

It does appear that the azp claim is supposed to be validated after the token endpoint call

"If an azp (authorized party) Claim is present, the Client SHOULD verify that its client_id is the Claim Value."

https://openid.net/specs/openid-connect-core-1_0.html#IDTokenValidation

The problem is that this package can be used by both the server requesting the token and other service that might want to validate the token later. For the former, you want to verify 'azp' matches your client_id. For the latter, you'd want to validate 'aud'.

So yeah, maybe we should have an "authorizing party" validation option?

manjinder-mckc commented 2 years ago

Makes sense i believe. We can make it optional to satisfy both server and client implementations, and the verifier config can be adjusted accordingly. Aldo, thanks for suggesting the Claims method.

ericchiang commented 2 years ago

Another thought is that we could have a method like:

func (v *IDTokenVerifier) VerifyToken(ctx context.Context, token *oauth2.Token) (*IDToken, error)

That did the right thing with the authorizing party claim. Might be a bit subtle though... Let me think about this a little