From 1f4c9c77a38fb6dfb751447361af9cf00964f96b Mon Sep 17 00:00:00 2001 From: Timur Sadykov Date: Tue, 9 Aug 2022 21:52:52 -0700 Subject: [PATCH] fix: updates IdTokenVerifier so that it does not cache a failed public key response (#967) * fix: add test intermittent issue * fix: fixing the intermittent error when getting public keys and empty key handling * churn: revert gitignore updates --- .../oauth2/ServiceAccountCredentials.java | 1 + .../com/google/auth/oauth2/TokenVerifier.java | 27 +++++----- .../auth/oauth2/MockTokenServerTransport.java | 18 +++++++ .../google/auth/oauth2/TokenVerifierTest.java | 53 ++++++++++++++++++- 4 files changed, 86 insertions(+), 13 deletions(-) diff --git a/oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java b/oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java index 9b9c99c54..123d72dc0 100644 --- a/oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java +++ b/oauth2_http/java/com/google/auth/oauth2/ServiceAccountCredentials.java @@ -181,6 +181,7 @@ static ServiceAccountCredentials fromJson( } catch (URISyntaxException e) { throw new IOException("Token server URI specified in 'token_uri' could not be parsed."); } + if (clientId == null || clientEmail == null || privateKeyPkcs8 == null diff --git a/oauth2_http/java/com/google/auth/oauth2/TokenVerifier.java b/oauth2_http/java/com/google/auth/oauth2/TokenVerifier.java index aed9949b5..12c6af92e 100644 --- a/oauth2_http/java/com/google/auth/oauth2/TokenVerifier.java +++ b/oauth2_http/java/com/google/auth/oauth2/TokenVerifier.java @@ -314,17 +314,13 @@ public static class JsonWebKey { public Map load(String certificateUrl) throws Exception { HttpTransport httpTransport = httpTransportFactory.create(); JsonWebKeySet jwks; - try { - HttpRequest request = - httpTransport - .createRequestFactory() - .buildGetRequest(new GenericUrl(certificateUrl)) - .setParser(OAuth2Utils.JSON_FACTORY.createJsonObjectParser()); - HttpResponse response = request.execute(); - jwks = response.parseAs(JsonWebKeySet.class); - } catch (IOException io) { - return ImmutableMap.of(); - } + HttpRequest request = + httpTransport + .createRequestFactory() + .buildGetRequest(new GenericUrl(certificateUrl)) + .setParser(OAuth2Utils.JSON_FACTORY.createJsonObjectParser()); + HttpResponse response = request.execute(); + jwks = response.parseAs(JsonWebKeySet.class); ImmutableMap.Builder keyCacheBuilder = new ImmutableMap.Builder<>(); if (jwks.keys == null) { @@ -345,7 +341,14 @@ public Map load(String certificateUrl) throws Exception { } } - return keyCacheBuilder.build(); + ImmutableMap keyCache = keyCacheBuilder.build(); + + if (keyCache.isEmpty()) { + throw new VerificationException( + "No valid public key returned by the keystore: " + certificateUrl); + } + + return keyCache; } private PublicKey buildPublicKey(JsonWebKey key) diff --git a/oauth2_http/javatests/com/google/auth/oauth2/MockTokenServerTransport.java b/oauth2_http/javatests/com/google/auth/oauth2/MockTokenServerTransport.java index 7eea7d462..29c8e6337 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/MockTokenServerTransport.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/MockTokenServerTransport.java @@ -133,6 +133,24 @@ public LowLevelHttpRequest buildRequest(String method, String url) throws IOExce int questionMarkPos = url.indexOf('?'); final String urlWithoutQuery = (questionMarkPos > 0) ? url.substring(0, questionMarkPos) : url; final String query = (questionMarkPos > 0) ? url.substring(questionMarkPos + 1) : ""; + + if (!responseSequence.isEmpty()) { + return new MockLowLevelHttpRequest(url) { + @Override + public LowLevelHttpResponse execute() throws IOException { + try { + return responseSequence.poll().get(); + } catch (ExecutionException e) { + Throwable cause = e.getCause(); + throw (IOException) cause; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new RuntimeException("Unexpectedly interrupted"); + } + } + }; + } + if (urlWithoutQuery.equals(tokenServerUri.toString())) { return new MockLowLevelHttpRequest(url) { @Override diff --git a/oauth2_http/javatests/com/google/auth/oauth2/TokenVerifierTest.java b/oauth2_http/javatests/com/google/auth/oauth2/TokenVerifierTest.java index 6c6373267..041fbabe4 100644 --- a/oauth2_http/javatests/com/google/auth/oauth2/TokenVerifierTest.java +++ b/oauth2_http/javatests/com/google/auth/oauth2/TokenVerifierTest.java @@ -43,6 +43,8 @@ import com.google.api.client.testing.http.MockLowLevelHttpResponse; import com.google.api.client.util.Clock; import com.google.auth.http.HttpTransportFactory; +import com.google.auth.oauth2.GoogleCredentialsTest.MockTokenServerTransportFactory; +import com.google.auth.oauth2.TokenVerifier.VerificationException; import com.google.common.io.CharStreams; import java.io.IOException; import java.io.InputStream; @@ -145,7 +147,8 @@ public LowLevelHttpResponse execute() throws IOException { TokenVerifier.VerificationException exception = assertThrows( TokenVerifier.VerificationException.class, () -> tokenVerifier.verify(ES256_TOKEN)); - assertTrue(exception.getMessage().contains("Could not find PublicKey")); + assertTrue( + exception.getMessage().contains("Error fetching PublicKey from certificate location")); } @Test @@ -186,6 +189,54 @@ public LowLevelHttpResponse execute() throws IOException { assertTrue(exception.getMessage().contains("Error fetching PublicKey")); } + @Test + void verifyPublicKeyStoreIntermittentError() throws IOException, VerificationException { + // mock responses + MockLowLevelHttpResponse response404 = + new MockLowLevelHttpResponse() + .setStatusCode(404) + .setContentType("application/json") + .setContent(""); + + MockLowLevelHttpResponse responseEmpty = + new MockLowLevelHttpResponse() + .setStatusCode(200) + .setContentType("application/json") + .setContent("{\"keys\":[]}"); + + MockLowLevelHttpResponse responseGood = + new MockLowLevelHttpResponse() + .setStatusCode(200) + .setContentType("application/json") + .setContent(readResourceAsString("iap_keys.json")); + + // Mock HTTP requests + MockTokenServerTransportFactory transportFactory = new MockTokenServerTransportFactory(); + + transportFactory.transport.addResponseSequence(response404, responseEmpty, responseGood); + + TokenVerifier tokenVerifier = + TokenVerifier.newBuilder() + .setClock(FIXED_CLOCK) + .setHttpTransportFactory(transportFactory) + .build(); + TokenVerifier.VerificationException exception = + assertThrows( + TokenVerifier.VerificationException.class, + () -> tokenVerifier.verify(ES256_TOKEN), + "Should have failed verification"); + assertTrue(exception.getMessage().contains("Error fetching PublicKey")); + + exception = + assertThrows( + TokenVerifier.VerificationException.class, + () -> tokenVerifier.verify(ES256_TOKEN), + "Should have failed verification"); + assertTrue(exception.getCause().getMessage().contains("No valid public key")); + + assertNotNull(tokenVerifier.verify(ES256_TOKEN)); + } + @Test void verifyEs256Token() throws TokenVerifier.VerificationException, IOException { HttpTransportFactory httpTransportFactory =