-
Notifications
You must be signed in to change notification settings - Fork 525
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
(NOT FOR SUBMISSION) Prototype OIDC support #1415
Conversation
@@ -92,6 +92,19 @@ public override async Task<bool> RequestAccessTokenAsync(CancellationToken taskC | |||
return true; | |||
} | |||
|
|||
/// <inheritdoc/> | |||
public override async Task<TokenResponse> CreateOidcTokenAsync(string targetAudience, CancellationToken cancellationToken = default(CancellationToken)) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we want want two additional settable parameters here:
-
format
-
license
https://cloud.google.com/compute/docs/instances/verifying-instance-identity#request_signature
license=true only works if format=full is set (my 2c is to auto set format=full if the user indicates license=true)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are there other optional parameters coming? If so, we'd probably want to create a new type for that - it's difficult to add extra parameters later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't know offhand if other parameters will be set but my guess would be yes (for example, applying user-defined claims)
if it helps, we're addign in vararg options fileds here for the other credential types:
also for ref, here is the dotnet support for the 'impersonated' credential type; i'll add a comment in that issue pointing out that for impersonated credential, an option is 'includeEmail=true'
#1312
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see any varargs there - just a builder pattern. I may be missing something. (I definitely don't feel like I've got the complete context of what we're trying to achieve here, but that may just be due to coming back to this two months later.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What i meant is that you can passin arbitrary options to signal the variation of the identity token that the underlying credential type provides. For example, in java i could pass in the .setOptions()
field takes in an array of potential values for the IdTokenCredential
type that will get modified by the true unerlying provider
so in java for compute
.setOptions(Arrays.asList(ComputeEngineCredentials.ID_TOKEN_FORMAT_FULL))
can also have a value like
or
.setOptions(Arrays.asList(ComputeEngineCredentials.ID_TOKEN_FORMAT_FULL,
ComputeEngineCredentials.LICENSE_TRUE))
- interms of why we're getting asking ComputeEngine for these extra values, the instance identity document provides the assertion of extended claims and even license values. Those can be used by remote system to attest certain things for the origin system. for info on generic id_tokens on gcp, see this link
If the pattern we're working with on google-auth-java (eg, appy a source credential like ComputeENgineCredential into IdTokenCredential` and then derive the token, thats even better
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I see - so they're simply on/off options, not individual separate values. It still feels like I need a couple of hours to get back into this spec to work out what we should do. I'll see if I can find time later in the week.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
certainly; pls feel free to ping at anytime; i'd be happy to help where i can.
just note, google-auth-python
does idTokens different than in google-auth-java
(as much as i'd like uniformity, that ship has sailed and was also subject to language constructs already in use).
for ref:
google-auth-python
uses a source credential and then gets an id_token:
from google.auth import compute_engine
creds = compute_engine.IDTokenCredentials( request,
target_audience= target_audience)
(as a side note, google-auth-python doesn't even currently use the metadata server to get an id_token...i'm trying to fix that in a different PR here and at the same time apply whatever options i can (at the moment, theyr'e explicitly passed in but as that pr is open, i'll adopt 'options' technique there as well)
in google-auth-java
, its inverted we get a source credential (eg. ComputeEngineCredentials
), then init an IdTokenCredential
and push the source credential to it to derive the id_token
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Anything i can do to assist here?
(i'm trying to make a version of this land on our official docs and it'd be good to have dotnet support)
btw, i'll also verify if the same changes here would work out of the box if we use the credential in grpc calls:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I need to find the time to get back to understanding this, basically. @chrisdunelm may be able to as well, of course.
Is this ever going to happen? We might just have received this functionality in node (thanks btw), but we're still missing support for it in .net to be able to completely avoid using json keyfiles to communicate with IAP. |
@AndyClausen: It's definitely on our list of things to look at, and I'm hoping we'll get to it this quarter, but potentially not in exactly this form. Apologies for the delay - it's the normal matter of prioritization between many important things to do. |
No worries Jon, it's just because we are going into production soon, and for that it would be nice to not extract any keyfiles at all for any of our services - so I wanted to see if it was possible. We'll wait patiently then. Thanks for the quick answer :) |
@AndyClausen i wrote up a small set of unofficial implementations here which can generate and verify google OIDC tokens: its ofcourse unofficial (plus, i'm ot that profiicent in dotnet) but hopefully it'd be easy to migrate over to the official one in dotnet once its implemented. @jskeet i don' tknow how dotnet bootstraps credentials for gRPC but perhaps another enhancement would be to use the OIDC token in grpc transports....so as another unofficial implementation:
|
Closing in favour of #1531 |
No description provided.