fix: updates IdTokenVerifier so that it does not cache a failed publi…

…c 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
googleapis · Aug 10, 2022 · 1f4c9c7 · 1f4c9c7
1 parent 0f1e085
commit 1f4c9c7
Show file tree

Hide file tree

Showing 4 changed files with 86 additions and 13 deletions.
diff --git 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
@@ -314,17 +314,13 @@ public static class JsonWebKey {
     public Map<String, PublicKey> 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<String, PublicKey> keyCacheBuilder = new ImmutableMap.Builder<>();
       if (jwks.keys == null) {
@@ -345,7 +341,14 @@ public Map<String, PublicKey> load(String certificateUrl) throws Exception {
         }
       }
 
-      return keyCacheBuilder.build();
+      ImmutableMap<String, PublicKey> 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
@@ -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
@@ -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 =