Skip to content

fix!: IdTokenVerifier now throws IOException if any issue obtaining public keys. Adding retries to public key fetch to cover transient network issues. #934

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

Merged
merged 29 commits into from
Aug 29, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
acb985f
verifier signature fix port
TimurSadykov Mar 23, 2022
1c16f8f
more test cases
TimurSadykov Mar 24, 2022
872f5f6
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 25, 2022
70ac584
fix: more test fixes
TimurSadykov Mar 28, 2022
5dde913
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 28, 2022
5c83d47
Update google-oauth-client/src/main/java/com/google/api/client/auth/o…
TimurSadykov Mar 30, 2022
17affb3
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 30, 2022
4d34201
fix: restored original interface for verifier, added default http fac…
TimurSadykov Mar 31, 2022
9611f53
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Mar 31, 2022
8b55b29
Update google-oauth-client/src/main/java/com/google/api/client/auth/o…
TimurSadykov Mar 31, 2022
813bb20
Update google-oauth-client/src/main/java/com/google/api/client/auth/o…
TimurSadykov Mar 31, 2022
e065d1a
doc fixes
TimurSadykov Mar 31, 2022
1af2370
fix: better docs and logging of validation exceptions
TimurSadykov Apr 3, 2022
22178ca
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Apr 3, 2022
c8d01d3
Update google-oauth-client/src/main/java/com/google/api/client/auth/o…
TimurSadykov Apr 12, 2022
aca11aa
nit and linter fixes
TimurSadykov Apr 12, 2022
1d668f8
Merge remote-tracking branch 'origin/main' into stim-signer
TimurSadykov May 31, 2022
6cd7d8c
fix: add verify without signature verification, remove caching empty …
TimurSadykov May 31, 2022
964c5a8
fix: docs for a new method
TimurSadykov May 31, 2022
d51d221
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] May 31, 2022
8f9f8f1
fix: docs for a new method
TimurSadykov Jun 1, 2022
ff2474e
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Jun 1, 2022
50eb0cc
fix: more test cases
TimurSadykov Jun 2, 2022
cc314d5
nit fixes for IdTokenVerifier
TimurSadykov Aug 12, 2022
7e7c69e
fix\!: IdtokenVerifier now throws IOException if any issue obtaining …
TimurSadykov Aug 19, 2022
3afaeaa
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Aug 19, 2022
f7bcae2
merge with main
TimurSadykov Aug 19, 2022
0fe9873
fix: docs nit fixes
TimurSadykov Aug 25, 2022
c4e8d45
🦉 Updates from OwlBot post-processor
gcf-owl-bot[bot] Aug 25, 2022
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@
package com.google.api.client.auth.openidconnect;

import com.google.api.client.http.GenericUrl;
import com.google.api.client.http.HttpBackOffUnsuccessfulResponseHandler;
import com.google.api.client.http.HttpBackOffUnsuccessfulResponseHandler.BackOffRequired;
import com.google.api.client.http.HttpRequest;
import com.google.api.client.http.HttpResponse;
import com.google.api.client.http.HttpTransport;
Expand All @@ -25,6 +27,7 @@
import com.google.api.client.util.Base64;
import com.google.api.client.util.Beta;
import com.google.api.client.util.Clock;
import com.google.api.client.util.ExponentialBackOff;
import com.google.api.client.util.Key;
import com.google.api.client.util.Preconditions;
import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -231,8 +234,10 @@ public final Collection<String> getAudience() {
*
* @param idToken ID token
* @return {@code true} if verified successfully or {@code false} if failed
* @throws IOException if verification fails to run. For example, if it fails to get public keys
* for signature validation.
*/
public boolean verify(IdToken idToken) {
public boolean verify(IdToken idToken) throws IOException {
boolean payloadValid = verifyPayload(idToken);

if (!payloadValid) {
Expand All @@ -242,11 +247,7 @@ public boolean verify(IdToken idToken) {
try {
return verifySignature(idToken);
} catch (VerificationException ex) {
LOGGER.log(
Level.SEVERE,
"id token signature verification failed. "
+ "Please see docs for IdTokenVerifier for default settings and configuration options",
ex);
LOGGER.log(Level.INFO, "Id token signature verification failed. ", ex);
return false;
}
}
Expand Down Expand Up @@ -281,7 +282,7 @@ protected boolean verifyPayload(IdToken idToken) {
}

@VisibleForTesting
boolean verifySignature(IdToken idToken) throws VerificationException {
boolean verifySignature(IdToken idToken) throws IOException, VerificationException {
if (Boolean.parseBoolean(environment.getVariable(SKIP_SIGNATURE_ENV_VAR))) {
return true;
}
Expand All @@ -297,12 +298,12 @@ boolean verifySignature(IdToken idToken) throws VerificationException {
String certificateLocation = getCertificateLocation(idToken.getHeader());
publicKeyToUse = publicKeyCache.get(certificateLocation).get(idToken.getHeader().getKeyId());
} catch (ExecutionException | UncheckedExecutionException e) {
throw new VerificationException(
throw new IOException(
"Error fetching public key from certificate location " + certificatesLocation, e);
}

if (publicKeyToUse == null) {
throw new VerificationException(
throw new IOException(
"Could not find public key for provided keyId: " + idToken.getHeader().getKeyId());
}

Expand Down Expand Up @@ -508,6 +509,10 @@ public Builder setHttpTransportFactory(HttpTransportFactory httpTransportFactory

/** Custom CacheLoader for mapping certificate urls to the contained public keys. */
static class PublicKeyLoader extends CacheLoader<String, Map<String, PublicKey>> {
private static final int DEFAULT_NUMBER_OF_RETRIES = 2;
private static final int INITIAL_RETRY_INTERVAL_MILLIS = 1000;
private static final double RETRY_RANDOMIZATION_FACTOR = 0.1;
private static final double RETRY_MULTIPLIER = 2;
private final HttpTransportFactory httpTransportFactory;

/**
Expand Down Expand Up @@ -553,6 +558,19 @@ public Map<String, PublicKey> load(String certificateUrl) throws Exception {
.createRequestFactory()
.buildGetRequest(new GenericUrl(certificateUrl))
.setParser(GsonFactory.getDefaultInstance().createJsonObjectParser());
request.setNumberOfRetries(DEFAULT_NUMBER_OF_RETRIES);

ExponentialBackOff backoff =
new ExponentialBackOff.Builder()
.setInitialIntervalMillis(INITIAL_RETRY_INTERVAL_MILLIS)
.setRandomizationFactor(RETRY_RANDOMIZATION_FACTOR)
.setMultiplier(RETRY_MULTIPLIER)
.build();

request.setUnsuccessfulResponseHandler(
new HttpBackOffUnsuccessfulResponseHandler(backoff)
.setBackOffRequired(BackOffRequired.ALWAYS));

HttpResponse response = request.execute();
jwks = response.parseAs(JsonWebKeySet.class);
} catch (IOException io) {
Expand Down Expand Up @@ -589,7 +607,7 @@ public Map<String, PublicKey> load(String certificateUrl) throws Exception {
"No valid public key returned by the keystore: " + certificateUrl);
}

return keyCacheBuilder.build();
return keyCache;
}

private PublicKey buildPublicKey(JsonWebKey key)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -68,9 +68,11 @@ public class IdTokenVerifierTest extends TestCase {
"https://www.googleapis.com/oauth2/v1/certs";

private static final String SERVICE_ACCOUNT_RS256_TOKEN =
"eyJhbGciOiJSUzI1NiIsImtpZCI6IjJlZjc3YjM4YTFiMDM3MDQ4NzA0MzkxNmFjYmYyN2Q3NGVkZDA4YjEiLCJ0eXAiOiJKV1QifQ.eyJhdWQiOiJodHRwczovL2V4YW1wbGUuY29tL2F1ZGllbmNlIiwiZXhwIjoxNTg3NjMwNTQzLCJpYXQiOjE1ODc2MjY5NDMsImlzcyI6InNvbWUgaXNzdWVyIiwic3ViIjoic29tZSBzdWJqZWN0In0.gGOQW0qQgs4jGUmCsgRV83RqsJLaEy89-ZOG6p1u0Y26FyY06b6Odgd7xXLsSTiiSnch62dl0Lfi9D0x2ByxvsGOCbovmBl2ZZ0zHr1wpc4N0XS9lMUq5RJQbonDibxXG4nC2zroDfvD0h7i-L8KMXeJb9pYwW7LkmrM_YwYfJnWnZ4bpcsDjojmPeUBlACg7tjjOgBFbyQZvUtaERJwSRlaWibvNjof7eCVfZChE0PwBpZc_cGqSqKXv544L4ttqdCnmONjqrTATXwC4gYxruevkjHfYI5ojcQmXoWDJJ0-_jzfyPE4MFFdCFgzLgnfIOwe5ve0MtquKuv2O0pgvg";
"eyJhbGciOiJSUzI1NiIsImtpZCI6IjE3MjdiNmI0OTQwMmI5Y2Y5NWJlNGU4ZmQzOGFhN2U3YzExNjQ0YjEiLCJ0eXAiOiJKV1QifQ.eyJhdWQiOiJodHRwczovL2Nsb3VkdGFza3MuZ29vZ2xlYXBpcy5jb20vdjIvcHJvamVjdHMvZ2Nsb3VkLWRldmVsL2xvY2F0aW9ucyIsImF6cCI6InN0aW0tdGVzdEBzdGVsbGFyLWRheS0yNTQyMjIuaWFtLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJlbWFpbCI6InN0aW0tdGVzdEBzdGVsbGFyLWRheS0yNTQyMjIuaWFtLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiZXhwIjoxNjYwODgwNjczLCJpYXQiOjE2NjA4NzcwNzMsImlzcyI6Imh0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbSIsInN1YiI6IjExMjgxMDY3Mjk2MzcyODM2NjQwNiJ9.Q2tG-hN6UHecbzaCIlg58K9msp58nLZWs03CBGO_D6F3cI4LKQEUzsbcztZqmNGWd0ld4zkrKzIP9cQosa_xold4hEzSX_ORRHYQLimLYaQmP3rKqWPMsbIupPdpnGqBDzAYjc7Pw9pQBzuZJj8e3FEG6a5tblDfMcgeklXZIkwzN7ypWCbFDoDP2STSYJYZ-LQIB0-Zlex7dm2KhyB8QSkMQK60YvpXz4L1OtwG7spk3yUCWxul6hYF76klST0iS6DH03YdaDpt4gRXkTUKyTRfB10h-WhCAKKRzmT6d_IT9ApIyqPhimkgkBHhLNyjK8lgAJdk9CLriSEOgVpsow";
private static final String SERVICE_ACCOUNT_RS256_TOKEN_BAD_SIGNATURE =
"eyJhbGciOiJSUzI1NiIsImtpZCI6IjE3MjdiNmI0OTQwMmI5Y2Y5NWJlNGU4ZmQzOGFhN2U3YzExNjQ0YjEiLCJ0eXAiOiJKV1QifQ.eyJhdWQiOiJodHRwczovL2Nsb3VkdGFza3MuZ29vZ2xlYXBpcy5jb20vdjIvcHJvamVjdHMvZ2Nsb3VkLWRldmVsL2xvY2F0aW9ucyIsImF6cCI6InN0aW0tdGVzdEBzdGVsbGFyLWRheS0yNTQyMjIuaWFtLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJlbWFpbCI6InN0aW0tdGVzdEBzdGVsbGFyLWRheS0yNTQyMjIuaWFtLmdzZXJ2aWNlYWNjb3VudC5jb20iLCJlbWFpbF92ZXJpZmllZCI6dHJ1ZSwiZXhwIjoxNjYwODgwNjczLCJpYXQiOjE2NjA4NzcwNzMsImlzcyI6Imh0dHBzOi8vYWNjb3VudHMuZ29vZ2xlLmNvbSIsInN1YiI6IjExMjgxMDY3Mjk2MzcyODM2NjQwNiJ9.Q2tG-hN6UHecbzaCIlg58K9msp58nLZWs03CBGO_D6F3cI4LKQEUzsbcztZqmNGWd0ld4zkrKzIP9cQosa_xold4hEzSX_ORRHYQLimLYaQmP3rKqWPMsbIupPdpnGqBDzAYjc7Pw9pQBzuZJj8e3FEG6a5tblDfMcgeklXZIkwzN7ypWCbFDoDP2STSYJYZ-LQIB0-Zlex7dm2KhyB8QSkMQK60YvpXz4L1OtwG7spk3yUCWxul6hYF76klST0iS6DH03YdaDpt4gRXkTUKyTRfB10h-WhCAKKRzmT6d_IT9ApIyqPhimkgkBHhLNyjK8lgAJdk9CLriSEOgVpruy";
private static final String SERVICE_ACCOUNT_CERT_URL =
"https://www.googleapis.com/robot/v1/metadata/x509/integration-tests%40chingor-test.iam.gserviceaccount.com";
"https://www.googleapis.com/oauth2/v3/certs";

private static final List<String> ALL_TOKENS =
Arrays.asList(ES256_TOKEN, FEDERATED_SIGNON_RS256_TOKEN, SERVICE_ACCOUNT_RS256_TOKEN);
Expand Down Expand Up @@ -184,7 +186,7 @@ public void testBuilderSetNullIssuers() throws Exception {
assertNull(verifier.getIssuer());
}

public void testMissingAudience() throws VerificationException {
public void testMissingAudience() throws IOException {
IdToken idToken = newIdToken(ISSUER, null);

MockClock clock = new MockClock();
Expand All @@ -198,7 +200,7 @@ public void testMissingAudience() throws VerificationException {
assertFalse(verifier.verify(idToken));
}

public void testVerifyEs256TokenPublicKeyMismatch() throws Exception {
public void testPublicKeyStoreIntermittentError() throws Exception {
// Mock HTTP requests
MockLowLevelHttpRequest failedRequest =
new MockLowLevelHttpRequest() {
Expand Down Expand Up @@ -245,7 +247,7 @@ public LowLevelHttpResponse execute() throws IOException {
};

HttpTransportFactory httpTransportFactory =
mockTransport(failedRequest, badRequest, emptyRequest, goodRequest);
mockTransport(failedRequest, badRequest, badRequest, badRequest, emptyRequest, goodRequest);
IdTokenVerifier tokenVerifier =
new IdTokenVerifier.Builder()
.setClock(FIXED_CLOCK)
Expand All @@ -255,28 +257,28 @@ public LowLevelHttpResponse execute() throws IOException {
try {
tokenVerifier.verifySignature(IdToken.parse(JSON_FACTORY, ES256_TOKEN));
fail("Should have failed verification");
} catch (VerificationException ex) {
} catch (IOException ex) {
assertTrue(ex.getMessage().contains("Error fetching public key"));
}

try {
tokenVerifier.verifySignature(IdToken.parse(JSON_FACTORY, ES256_TOKEN));
fail("Should have failed verification");
} catch (VerificationException ex) {
} catch (IOException ex) {
assertTrue(ex.getMessage().contains("Error fetching public key"));
}

try {
tokenVerifier.verifySignature(IdToken.parse(JSON_FACTORY, ES256_TOKEN));
fail("Should have failed verification");
} catch (VerificationException ex) {
} catch (IOException ex) {
assertTrue(ex.getCause().getMessage().contains("No valid public key returned"));
}

Assert.assertTrue(tokenVerifier.verifySignature(IdToken.parse(JSON_FACTORY, ES256_TOKEN)));
}

public void testVerifyEs256Token() throws VerificationException, IOException {
public void testVerifyEs256Token() throws IOException {
HttpTransportFactory httpTransportFactory =
mockTransport(
"https://www.gstatic.com/iap/verify/public_key-jwk",
Expand All @@ -289,7 +291,7 @@ public void testVerifyEs256Token() throws VerificationException, IOException {
assertTrue(tokenVerifier.verify(IdToken.parse(JSON_FACTORY, ES256_TOKEN)));
}

public void testVerifyRs256Token() throws VerificationException, IOException {
public void testVerifyRs256Token() throws IOException {
HttpTransportFactory httpTransportFactory =
mockTransport(
"https://www.googleapis.com/oauth2/v3/certs",
Expand All @@ -304,7 +306,7 @@ public void testVerifyRs256Token() throws VerificationException, IOException {
}

public void testVerifyRs256TokenWithLegacyCertificateUrlFormat()
throws VerificationException, IOException {
throws IOException, VerificationException {
HttpTransportFactory httpTransportFactory =
mockTransport(
LEGACY_FEDERATED_SIGNON_CERT_URL, readResourceAsString("legacy_federated_keys.json"));
Expand All @@ -318,15 +320,23 @@ public void testVerifyRs256TokenWithLegacyCertificateUrlFormat()
assertTrue(tokenVerifier.verify(IdToken.parse(JSON_FACTORY, FEDERATED_SIGNON_RS256_TOKEN)));
}

public void testVerifyServiceAccountRs256Token() throws VerificationException, IOException {
MockClock clock = new MockClock(1587626643000L);
public void testVerifyServiceAccountRs256Token() throws IOException {
MockClock clock = new MockClock(1660880973000L);
IdTokenVerifier tokenVerifier =
new IdTokenVerifier.Builder()
.setClock(clock)
.setCertificatesLocation(SERVICE_ACCOUNT_CERT_URL)
.setHttpTransportFactory(new DefaultHttpTransportFactory())
.build();
assertTrue(tokenVerifier.verify(IdToken.parse(JSON_FACTORY, SERVICE_ACCOUNT_RS256_TOKEN)));

// a token with a bad signature that is expected to fail in verify, but work in verifyPayload
assertFalse(
tokenVerifier.verify(
IdToken.parse(JSON_FACTORY, SERVICE_ACCOUNT_RS256_TOKEN_BAD_SIGNATURE)));
assertTrue(
tokenVerifier.verifyPayload(
IdToken.parse(JSON_FACTORY, SERVICE_ACCOUNT_RS256_TOKEN_BAD_SIGNATURE)));
}

static String readResourceAsString(String resourceName) throws IOException {
Expand Down