dermesser / yup-oauth2

An oauth2 client implementation providing the Device, Installed, Service Account, and several more flows.
https://docs.rs/yup-oauth2/
Apache License 2.0
219 stars 114 forks source link

Support id tokens from metadata service #209

Open jneem opened 11 months ago

jneem commented 11 months ago

This adds support for getting an ID token (as opposed to an oauth2 access token) from Google's metadata service.

Edit: it's a breaking change because it added a field to a public struct. Maybe it's worth marking the option structs as non_exhaustive to make such changes less disruptive in the future?

fiadliel commented 11 months ago

(as a drive-by commentator who would in theory find this useful)

Functionality to get an ID token would be welcome, but I don't think this is the right approach. Fundamentally, that TokenInfo represents an OAuth2 access token, and an ID token is quite explicitly not an access token (even though Cloud Run/Functions/etc. use them to validate access).

What ID tokens and access tokens have in common, is that they care about similar things (e.g. is there a metadata server, vs running on local workstation, or using impersonated credentials, or workforce federation). But both the exact method of getting the token, and the use of them, varies after that. One might also want to get ID tokens that include the service account email, or licensing details (requires extra parameters to get this information).

I would prefer that, if implemented, this should be a separate method, e.g. async fn id_token(...) -> Result<IdToken, Error>

jneem commented 11 months ago

Fundamentally, that TokenInfo represents an OAuth2 access token

I agree that it would be nice to have a better separation between ID tokens and OAuth2 access tokens, but as far as this crate stands today, TokenInfo already (optionally) contains both of them.

I would prefer that, if implemented, this should be a separate method, e.g. async fn id_token(...) -> Result<IdToken, Error>

Where would you put this method? One of the difficulties I had here was that the actual token-fetching logic is a few layers down from the public API, and I wasn't really sure about the best way of "threading" things through.

fiadliel commented 11 months ago

I had a closer look at the support for ID tokens in the library - and it's unfortunate. The id_token() call provided is a convenience method for accessing the ID token from an OAuth2 client; but it has nothing to do with the authentication required by services in Google Cloud - and the interface provided doesn't match the concepts needed to provide that authentication (for example, scopes are not defined for ID tokens, but it's currently a required parameter to retrieve one).

So I see why your code is the way it is, as the code is already very fuzzy here, and fixing that would take a much larger rework. Renaming "scope" as "audience" allows you to leverage the existing misunderstandings :)

But the larger view is that getting an ID token isn't an authentication flow; it (maybe) uses an already successful authentication flow. If we look at the available methods of getting an ID token (https://cloud.google.com/docs/authentication/get-id-token), we have:

In the second case in particular, we would want a pre-existing Authenticator to make that call.

So this points to the idea that getting an ID token is layered on top of existing code to get a Google access token, rather than being at the same level. That code could exist in this, or another library (in general, it has been bundled together in the same library, if one looks at examples from other languages).