-
Notifications
You must be signed in to change notification settings - Fork 229
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
feat: add IDTokenCredential support #303
Conversation
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
ok, i've addressed some of the request and left a bunch unsolved with this commit as they are relaed to the bit mentioned here: The biggest thing is if public class IdToken extends AccessToken is the best way or not The However, customers will also want to evaluate and inspect claims embedded within the token (expiration, the aud: field, etc) public JsonWebSignature getJsonWebSignature() {
return jws;
} for example System.out.println(tokenCredential.getIdToken().getTokenValue());
System.out.println(tokenCredential.getIdToken().getJsonWebSignature().getPayload().getAudienceAsList());
System.out.println(tokenCredential.getIdToken().getJsonWebSignature().getPayload().getExpirationTimeSeconds()); in that way, the FWIW, he only reason i extended // Invoke the API
GenericUrl genericUrl = new GenericUrl("https://myapp-6w42z6vi3q-uc.a.run.app");
HttpCredentialsAdapter adapter = new HttpCredentialsAdapter(tokenCredential);
HttpTransport transport = new NetHttpTransport();
HttpRequest request = transport.createRequestFactory(adapter).buildGetRequest(genericUrl);
request.setThrowExceptionOnExecuteError(false);
HttpResponse response = request.execute();
|
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
The |
As for whether Perhaps |
ok, i changed to public class IdTokenCredentials extends OAuth2Credentials { and verified the credentials are usable in the same way. I also see the raw token accessible in a couple ways: System.out.println(tokenCredential.getAccessToken().getTokenValue());
System.out.println(tokenCredential.getIdToken().getTokenValue());
System.out.println(tokenCredential.getIdToken().getJsonWebSignature().getPayload().getAudienceAsList());
System.out.println(tokenCredential.getIdToken().getJsonWebSignature().getPayload().getExpirationTimeSeconds()); |
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ImpersonatedCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java
Outdated
Show resolved
Hide resolved
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.
Lots of style nits (not all called out) and one substantive comment
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
return getMetadataServerUrl(DefaultCredentialsProvider.DEFAULT) | ||
+ "/computeMetadata/v1/instance/service-accounts/default/identity"; | ||
} | ||
|
||
@Override | ||
public int hashCode() { | ||
return Objects.hash(transportFactoryClassName); |
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.
not for this PR, but this looks really wonky. Note to myself: can this possibly be correct?
String errorMessage = OAuth2Utils.validateString(error, "message", PARSE_ERROR_MESSAGE); | ||
throw new IOException(String.format("Error code %s trying to getIDToken: %s", statusCode, errorMessage)); | ||
} | ||
if (statusCode != HttpStatusCodes.STATUS_CODE_OK) { |
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.
should this be any 200 level response?
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'm not absolutely sure but i've never seen responses range 2xx from this api layer:
ref: https://cloud.google.com/apis/design/errors#handling_errors
but storage mentions a bit of that here: https://cloud.google.com/storage/docs/json_api/v1/status-codes
(the google apis endpoint iamcredentials uses is older one (similar to drive api, calendar api, etc)
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/MockMetadataServerTransport.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/MockMetadataServerTransport.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ServiceAccountCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ServiceAccountCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/IdTokenCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ComputeEngineCredentials.java
Outdated
Show resolved
Hide resolved
oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java
Outdated
Show resolved
Hide resolved
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.
Since we're unlikely to get the API perfect on the first try, we should mark this @beta to give ourselves flexibility to change it in the next couple of releases.
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.
Looking good! Mostly nits, however, can we add some tests on the IAMUtils error handling
oauth2_http/javatests/com/google/auth/oauth2/ServiceAccountCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ImpersonatedCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ImpersonatedCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/IdTokenCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ComputeEngineCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ComputeEngineCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/ComputeEngineCredentialsTest.java
Outdated
Show resolved
Hide resolved
oauth2_http/javatests/com/google/auth/oauth2/IdTokenCredentialsTest.java
Outdated
Show resolved
Hide resolved
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 have any major concerns or objections to this if you're both happy with it. Main thing is that everything is marked @beta so we can and will change it if we find we didn't get it quite right the first time, which we almost never do of course. :-)
ok,i reverified all the credential types work as intended;
Note, the GoogleCredentials adcCreds = GoogleCredentials.getApplicationDefault();
IdTokenCredentials tokenCredential = IdTokenCredentials.newBuilder()
.setIdTokenProvider((IdTokenProvider) adcCreds)
.setTargetAudience(targetAudience).build(); will throw Caused by: java.lang.ClassCastException: class com.google.auth.oauth2.UserCredentials cannot be cast to class com.google.auth.oauth2.IdTokenProvider (com.google.auth.oauth2.UserCredentials and com.google.auth.oauth2.IdTokenProvider are in unnamed module of loader java.net.URLClassLoader @6345dc06) (unless ofcourse the |
thanks; final note, you can use these id_tokens now directly inside gRPC channel credentials (i'll update my sample there once this change is live and shipped in a release) |
Fixes #99
Adds support for IDTokens into
google-auth-java
forServiceAccountCredentials
,ComuteEngineCredentials
andImpersonatedCredentials
GCP users currently have to do these steps manually and outside of
google-auth-java
:ref: https://github.com/salrashid123/salrashid123.github.io/tree/master/google_id_token#java
this change wraps the various flows into the library
I do not have any testcases here. I'm submitting this first for review of the api surface. Usage for this change is shown here:
#99 (comment)