-
Notifications
You must be signed in to change notification settings - Fork 233
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
docs(samples): added auth samples and tests #927
Conversation
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.
Hi Sita, feel free to grab a slot on my calendar if you want to discuss any of my comments!
samples/snippets/src/main/java/AuthWithCredentialsFromMetadataServer.java
Outdated
Show resolved
Hide resolved
samples/snippets/src/main/java/IdTokenFromImpersonatedCredentialsREST.java
Outdated
Show resolved
Hide resolved
# Conflicts: # samples/snippets/src/main/java/AuthenticateExplicit.java # samples/snippets/src/main/java/AuthenticateImplicitWithAdc.java # samples/snippets/src/main/java/IdTokenFromImpersonatedCredentials.java # samples/snippets/src/main/java/IdTokenFromImpersonatedCredentials_OAuth.java # samples/snippets/src/main/java/IdTokenFromMetadataServer.java # samples/snippets/src/main/java/IdTokenFromServiceAccount.java # samples/snippets/src/main/java/IdTokenFromServiceAccountREST.java # samples/snippets/src/main/java/VerifyNonGoogleIdToken.java # samples/snippets/src/test/java/SnippetsIT.java
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.
Thanks Sita!
samples/snippets/src/main/java/AuthenticateImplicitWithAdc.java
Outdated
Show resolved
Hide resolved
samples/snippets/src/main/java/IdTokenFromImpersonatedCredentials_IAM.java
Outdated
Show resolved
Hide resolved
samples/snippets/src/main/java/IdTokenFromImpersonatedCredentials_OAuth.java
Outdated
Show resolved
Hide resolved
samples/snippets/src/main/java/AuthenticateImplicitWithAdc.java
Outdated
Show resolved
Hide resolved
Here is the summary of changes. You are about to add 5 region tags.
This comment is generated by snippet-bot.
|
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.
LGTM, thanks Sita!
import com.auth0.jwk.JwkException; | ||
import com.google.api.client.googleapis.auth.oauth2.GoogleIdToken; | ||
import com.google.api.client.googleapis.auth.oauth2.GoogleIdToken.Payload; | ||
import com.google.api.client.googleapis.auth.oauth2.GoogleIdTokenVerifier; |
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.
This verifier is from google-api-java-client and was not being maintained. It also had a CVE until recently where we weren't actually validating the signature.
We should instead use the token verifier from this library (like https://github.com/GoogleCloudPlatform/java-docs-samples/blob/96c4f10bf01683d0e0a5b2a44ba4bee1ac5fc507/iap/src/main/java/com/example/iap/VerifyIapRequestHeader.java#L60-L73)
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.
Done. I've made the change you suggested and also removed the third party dependency.
PTAL.
public static void verifyNonGoogleIdToken(String idToken, String targetAudience, | ||
String jwkUrl) | ||
throws IOException, JwkException { | ||
|
||
// Start verification. | ||
DecodedJWT jwt = JWT.decode(idToken); | ||
|
||
// Check if the token has expired. | ||
if (jwt.getExpiresAt().before(Calendar.getInstance().getTime())) { | ||
System.out.println("Token already expired.."); | ||
return; | ||
} | ||
|
||
// Construct the jwkProvider from the provided jwkURL. | ||
JwkProvider jwkProvider = new UrlJwkProvider(new URL(jwkUrl)); | ||
// Get the jwk from the provided key id. | ||
Jwk jwk = jwkProvider.get(jwt.getKeyId()); | ||
// Retrieve the public key and use that to create an instance of the Algorithm. | ||
Algorithm algorithm = Algorithm.RSA256((RSAPublicKey) jwk.getPublicKey(), null); | ||
// Create the verifier with the algorithm and target audience. | ||
JWTVerifier jwtVerifier = JWT.require(algorithm).withAudience(targetAudience).build(); | ||
|
||
boolean isVerified = true; | ||
try { | ||
// Verify the obtained id token. This is done at the receiving end of the OIDC endpoint. | ||
jwt = jwtVerifier.verify(idToken); | ||
} catch (JWTVerificationException e) { | ||
System.out.println("Could not verify Signature: " + e.getMessage()); | ||
isVerified = false; | ||
} | ||
|
||
if (isVerified) { | ||
System.out.println("Id token verified."); | ||
return; | ||
} | ||
System.out.println("Unable to verify 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.
I would recommend that we use our first-party implementation and not require extra third-party dependencies.
// see: https://developers.google.com/identity/protocols/oauth2/scopes | ||
|
||
// Construct the Storage client. | ||
Storage storage = |
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.
This should use a try/catch and include the warning about making sure to close: disingenuous: https://googlecloudplatform.github.io/samples-style-guide/#clients
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 am not sure of this as many of the storage samples look like they don't follow the auto close try-catch..
https://github.com/googleapis/java-storage/blob/main/samples/snippets/src/main/java/com/example/storage/bucket/ListBuckets.java
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.
2 non-blocking comments
# Conflicts: # samples/snippets/src/main/java/VerifyGoogleIdToken.java
Thanks Jeff! Included your comments in the sample for further clarification. |
🤖 I have created a release *beep* *boop* --- ## [1.10.0](v1.9.0...v1.10.0) (2022-08-05) ### Features * workforce identity federation for pluggable auth ([#959](#959)) ([7f2c535](7f2c535)) ### Bug Fixes * updates executable response spec for executable-sourced credentials ([#955](#955)) ([48ff83d](48ff83d)) ### Documentation * **samples:** added auth samples and tests ([#927](#927)) ([32c717f](32c717f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
🤖 I have created a release *beep* *boop* --- ## [1.10.0](v1.9.0...v1.10.0) (2022-08-05) ### Features * workforce identity federation for pluggable auth ([#959](#959)) ([7f2c535](7f2c535)) ### Bug Fixes * updates executable response spec for executable-sourced credentials ([#955](#955)) ([48ff83d](48ff83d)) ### Documentation * **samples:** added auth samples and tests ([#927](#927)) ([32c717f](32c717f)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). See [documentation](https://github.com/googleapis/release-please#release-please).
Adding samples for the authentication docs.