Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement support for ID tokens #158

Closed
wants to merge 1 commit into from

Conversation

nagisa
Copy link
Contributor

@nagisa nagisa commented Jun 28, 2021

For google stuff these are relevant when trying to invoke e.g. Cloud
Run services. I'm not at all knowledgeable enough with OAuth to be able
to tell if what I'm doing here is correct.

This is a breaking change. AccessToken got renamed to just
Token (since it now encompasses more than just access_token and
there are some changes to the TokenInfo type too.

Sponsored by: standard.ai

@nagisa nagisa force-pushed the nagisa/id_token branch from 6ac0f13 to 6304a3e Compare June 28, 2021 12:30
@dermesser
Copy link
Owner

Thank you very much! I will try to take a look soon; if I'm too slow, please don't hesitate to ping me.

src/types.rs Outdated Show resolved Hide resolved
@nagisa
Copy link
Contributor Author

nagisa commented Jul 9, 2021

I have made some further changes in an effort to make this more readily landable, but it also ended up breaking the API further. For example it is no longer possible to directly obtain the token as str, and instead the user must make a decision whether they want the access or id token, by calling the appropriate API function.

@nagisa nagisa force-pushed the nagisa/id_token branch 2 times, most recently from 25de05b to c926d7d Compare July 9, 2021 11:08
For google stuff these are relevant when trying to invoke e.g. Cloud
Run services. I'm not at all knowledgeable enough with OAuth to be able
to tell if what I'm doing here is correct.

This is a breaking change. `AccessToken` got renamed to just `Token`
(since it now encompasses more than just `access_token` and there are
some changes to the `TokenInfo` type too.

Sponsored by: standard.ai
@nagisa nagisa force-pushed the nagisa/id_token branch from c926d7d to 4e54fba Compare July 19, 2021 13:23
@dermesser
Copy link
Owner

this looks alright, but I didn't see where the ID token is used to make requests. Did I miss something? Or are you just preparing the types in order to later be able to make requests using the ID token? (to be honest, after a bit of googling I am still not quite sure what the distinctive feature of ID tokens is :)

@nagisa
Copy link
Contributor Author

nagisa commented Jul 20, 2021

I didn't see where the ID token is used to make requests. Did I miss something?

I'm not sure what you're asking, so I'll try to describe my use-case, instead:

I have a Google Cloud Run application. This Cloud Run application is set up so that it can only be invoked by certain entities (service accounts). In order to obtain a Bearer token valid for requests to this application it is necessary to use a scope such as https://applicationname-0000000000-uc.a.run.app (where applicationname and 0s are specific to the service).

When a service account is used in combination with the scope above, Google's OAuth returns the ID token only, and the response does not include the access token. With the current yup-oauth2 this response fails to be processed. Anyway, the ID token can be used as a Bearer token to “authenticate” requests to the cloud run application correctly.

And so if the question is “why is there nothing specific done to request ID tokens to be returned”, that's because the scope used decides whether an ID token or an access token (or both?) are returned.

If the question is “how would one obtain the ID token to use to put into the header", that's done through Token::id_token method.

For my own purposes this PR as it is right now has everything I need.

@nagisa
Copy link
Contributor Author

nagisa commented Jul 20, 2021

after a bit of googling I am still not quite sure what the distinctive feature of ID tokens is

I believe ID tokens allow to verify you are who you claim you are, whereas access tokens don't necessarily contain that information.

@bjornwein bjornwein mentioned this pull request Dec 20, 2021
@dermesser
Copy link
Owner

Ping! Can you check #166 w.r.t. whether it helps with this use case, or can be extended to support it?

@nagisa
Copy link
Contributor Author

nagisa commented Jan 2, 2022

I no longer work on the codebase that required this changeset, and so am not able to verify this.

@dermesser dermesser closed this Apr 8, 2022
@blogle blogle mentioned this pull request Sep 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants