golang / oauth2

Go OAuth2
https://golang.org/x/oauth2
BSD 3-Clause "New" or "Revised" License
5.33k stars 981 forks source link

golang/oauth2/stsexchange: error handling enhancement proposal #740

Open olefirenque opened 2 weeks ago

olefirenque commented 2 weeks ago

Hello!

The standard implementation of client_credentials grant_type uses a RetrieveError, which is really useful because it provides the raw response body and error code.

https://github.com/golang/oauth2/blob/3e6480915d39dd1a80fa460e56413857f02cc1b9/token.go#L184-L198

As I noticed, the stsexchange implementation wraps the raw error message, which makes it difficult to properly handle custom errors from different OIDC providers.

https://github.com/golang/oauth2/blob/3e6480915d39dd1a80fa460e56413857f02cc1b9/google/internal/stsexchange/sts_exchange.go#L89-L91

It would be great if stsexchange wrapped errors the same way client_credentials do.

codyoss commented 1 week ago

Would you mind raising an issue on https://github.com/googleapis/google-cloud-go instead. This package google package is slowly being phased out in favor of https://pkg.go.dev/cloud.google.com/go/auth/credentials

olefirenque commented 1 week ago

Sure, I can bring this up, but I would like to point out that in my case I use both client_credentials and token-exchange from oauth2 (I don't actually use any Google cloud infrastructure). It would be more convenient to use this enhancement here, since it allows to handle token issuance errors in a more general way (in particular, they will have a common type for the error).

Can the attached MR still be considered?

codyoss commented 1 week ago

(I don't actually use any Google cloud infrastructure)

Your PR and issue mention the google sub-directory that is used for Google clients. This package will soon be deprecated in favor of the one I liked above. We don't plan on adding any more features to this google package at this time.