fix(oauth2): address PR 13873 review findings for agent identity token binding

- Prevent 30-second polling delay and IOException on standard GCE/container environments when well-known credentials directory exists without certificate files unless mTLS is explicitly enabled.
- Fail fast on malformed certificate config JSON without retrying.
- Fix URI resource path decoding in AgentIdentityUtilsTest to prevent FileNotFoundException when workspace paths contain spaces.
- Copy matching private key in well-known fallback test to verify full key pair loading.
- Clean up static wellKnownDir state and temporary directories in ComputeEngineCredentialsTest.
- Correct opt-out environment variable value in test setup from 'true' to 'false'.

Author: macastelaz

google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/AgentIdentityUtils.java

@@ -71,6 +71,9 @@ public final class AgentIdentityUtils {
   static final String GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES =
       "GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES";
 
+  /** Javadoc. */
+  static final String GOOGLE_API_USE_CLIENT_CERTIFICATE = "GOOGLE_API_USE_CLIENT_CERTIFICATE";
+
   /** Javadoc. */
   private static final List<Pattern> AGENT_IDENTITY_SPIFFE_PATTERNS =
       ImmutableList.of(
@@ -399,6 +402,10 @@ && checkExistsOrAccessDenied(Paths.get(paths.getCertPath()))) {
                 + " token.");
         return null;
       } catch (IOException e) {
+        if (e.getMessage() != null
+            && e.getMessage().contains("Failed to parse Agent Identity config JSON")) {
+          throw e; // Fail fast on malformed JSON syntax errors
+        }
         // Fall through to retry
       }
       if (!warned) {
@@ -435,6 +442,42 @@ private static String getWellKnownCertificatePathWithRetry() throws IOException
     String bundlePath = Paths.get(wellKnownDir, "credentialbundle.pem").toString();
     String certOnlyPath = Paths.get(wellKnownDir, "certificates.pem").toString();
 
+    // 1) First check immediately without sleeping:
+    try {
+      if (checkExistsOrAccessDenied(Paths.get(bundlePath))) {
+        return bundlePath;
+      }
+      if (checkExistsOrAccessDenied(Paths.get(certOnlyPath))) {
+        return certOnlyPath;
+      }
+    } catch (java.nio.file.AccessDeniedException e) {
+      Slf4jUtils.log(
+          LOGGER,
+          org.slf4j.event.Level.WARN,
+          Collections.emptyMap(),
+          "Permission denied reading well-known certificates. Falling back to unbound"
+              + " token.");
+      return null;
+    } catch (Exception e) {
+      // Fall through
+    }
+
+    // 2) If not found immediately, only enter retry loop if mTLS was explicitly enabled:
+    String useClientCert = envReader.getEnv(GOOGLE_API_USE_CLIENT_CERTIFICATE);
+    if (!"true".equalsIgnoreCase(useClientCert)) {
+      Slf4jUtils.log(
+          LOGGER,
+          org.slf4j.event.Level.DEBUG,
+          Collections.emptyMap(),
+          String.format(
+              "Well-known certificate file not found at %s and %s is not"
+                  + " explicitly enabled; falling back to unbound token without"
+                  + " retrying.",
+              wellKnownDir, GOOGLE_API_USE_CLIENT_CERTIFICATE));
+      return null;
+    }
+
+    // 3) Retry loop for rotation/transient absence when explicitly enabled:
     boolean warned = false;
     for (long sleepInterval : POLLING_INTERVALS) {
       try {
@@ -461,20 +504,28 @@ private static String getWellKnownCertificatePathWithRetry() throws IOException
             org.slf4j.event.Level.WARN,
             Collections.emptyMap(),
             String.format(
-                "Well-known certificate file not found at %s. Retrying for up to %d" + " seconds.",
+                "Well-known certificate file not found at %s. Retrying for up to"
+                    + " %d seconds.",
                 wellKnownDir, TOTAL_TIMEOUT_MS / 1000));
         warned = true;
       }
       try {
         timeService.sleep(sleepInterval);
       } catch (InterruptedException e) {
         Thread.currentThread().interrupt();
-        throw new IOException("Interrupted while waiting for well-known certificate files.", e);
+        throw new IOException(
+            "Interrupted while waiting for well-known certificate files.", e);
       }
     }
-    throw new IOException(
-        "Unable to find well-known certificate file for bound token request after multiple"
-            + " retries.");
+    Slf4jUtils.log(
+        LOGGER,
+        org.slf4j.event.Level.WARN,
+        Collections.emptyMap(),
+        String.format(
+            "Unable to find well-known certificate file at %s after retrying;"
+                + " falling back to unbound token.",
+            wellKnownDir));
+    return null;
   }
 
   /** Reads the full certificate chain from the specified path as a string. */
@@ -530,7 +581,7 @@ static PrivateKey readPrivateKey(final String keyPath, final String algorithm)
    */
   static boolean shouldEnableMtls(final boolean certsPresent, final boolean configExists)
       throws IOException {
-    String useClientCert = envReader.getEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE");
+    String useClientCert = envReader.getEnv(GOOGLE_API_USE_CLIENT_CERTIFICATE);
 
     // Case 1: Explicitly enabled via environment variable
     if ("true".equalsIgnoreCase(useClientCert)) {

google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/AgentIdentityUtilsTest.java

@@ -55,6 +55,7 @@
 import java.util.Collection;
 import java.util.Collections;
 import java.util.List;
+import java.util.concurrent.atomic.AtomicInteger;
 import java.util.concurrent.atomic.AtomicLong;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
@@ -150,22 +151,47 @@ private X509Certificate mockCertWithSanUri(String uri) throws CertificateExcepti
 
   @Test
   public void getAgentIdentityCertificate_optedOut_returnsNullImmediately() throws IOException {
-    envProvider.setEnv("GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES", "true");
+    envProvider.setEnv("GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES", "false");
     envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", "/non/existent/path");
     assertNull(AgentIdentityUtils.getAgentIdentityCertInfo());
   }
 
+  @Test
+  public void getAgentIdentityCertificate_preventTokenSharingTrue_doesNotOptOut()
+      throws Exception {
+    envProvider.setEnv("GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES", "true");
+    AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
+
+    URL certUrl = getClass().getClassLoader().getResource("agent/agent_spiffe_cert.pem");
+    assertNotNull(certUrl, "Test resource agent/agent_spiffe_cert.pem not found");
+    String certPath = Paths.get(certUrl.toURI()).toAbsolutePath().toString();
+    Files.copy(Paths.get(certPath), tempDir.resolve("certificates.pem"));
+
+    URL keyUrl = getClass().getClassLoader().getResource("agent/agent_spiffe_key.pem");
+    assertNotNull(keyUrl, "Test resource agent/agent_spiffe_key.pem not found");
+    String keyPath = Paths.get(keyUrl.toURI()).toAbsolutePath().toString();
+    Files.copy(Paths.get(keyPath), tempDir.resolve("private_key.pem"));
+
+    envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", null);
+
+    AgentIdentityUtils.CertInfo info = AgentIdentityUtils.getAgentIdentityCertInfo();
+    assertNotNull(info);
+    assertEquals(
+        new String(Files.readAllBytes(tempDir.resolve("certificates.pem")), StandardCharsets.UTF_8),
+        info.getCertContent());
+  }
+
   @Test
   public void getAgentIdentityCertificate_noConfigEnvVar_returnsNull() throws IOException {
     AgentIdentityUtils.setTimeService(new FakeTimeService());
     assertNull(AgentIdentityUtils.getAgentIdentityCertInfo());
   }
 
   @Test
-  public void getAgentIdentityCertificate_happyPath_loadsCertificate() throws IOException {
+  public void getAgentIdentityCertificate_happyPath_loadsCertificate() throws Exception {
     URL certUrl = getClass().getClassLoader().getResource("x509_leaf_certificate.pem");
     assertNotNull(certUrl, "Test resource x509_leaf_certificate.pem not found");
-    String certPath = new File(certUrl.getFile()).getAbsolutePath();
+    String certPath = Paths.get(certUrl.toURI()).toAbsolutePath().toString();
     File configFile = tempDir.resolve("config.json").toFile();
     String configJson =
         "{"
@@ -209,6 +235,26 @@ public void getAgentIdentityCertInfo_malformedJson_throwsIOException() throws IO
     }
     envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", configFile.getAbsolutePath());
     AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
+    FakeTimeService fakeTime = new FakeTimeService();
+    AgentIdentityUtils.setTimeService(fakeTime);
+
+    IOException e = assertThrows(IOException.class, AgentIdentityUtils::getAgentIdentityCertInfo);
+    assertTrue(e.getMessage().contains("Failed to parse Agent Identity config JSON"));
+    assertEquals(0, fakeTime.getSleepCount());
+  }
+
+  @Test
+  public void getAgentIdentityCertInfo_configExists_certMissing_throwsIOExceptionAfterRetries()
+      throws IOException {
+    File configFile = tempDir.resolve("config.json").toFile();
+    try (FileOutputStream fos = new FileOutputStream(configFile)) {
+      String json =
+          "{ \"cert_configs\": { \"workload\": { \"cert_path\": \"/non/existent/cert.pem\","
+              + " \"key_path\": \"/non/existent/key.pem\" } } }";
+      fos.write(json.getBytes(StandardCharsets.UTF_8));
+    }
+    envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", configFile.getAbsolutePath());
+    AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
     AgentIdentityUtils.setTimeService(new FakeTimeService());
 
     IOException e = assertThrows(IOException.class, AgentIdentityUtils::getAgentIdentityCertInfo);
@@ -244,15 +290,43 @@ public void shouldEnableMtls_unset_certsPresent_returnsTrue() throws IOException
   }
 
   @Test
-  public void getAgentIdentityCertInfo_fallbackPath_loadsCertificate() throws IOException {
-    AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
+  public void shouldEnableMtls_true_noCertsNoConfig_returnsFalse() throws IOException {
+    envProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true");
+    assertFalse(AgentIdentityUtils.shouldEnableMtls(false, false));
+  }
 
-    URL certUrl = getClass().getClassLoader().getResource("x509_leaf_certificate.pem");
-    assertNotNull(certUrl, "Test resource x509_leaf_certificate.pem not found");
-    String certPath = new File(certUrl.getFile()).getAbsolutePath();
+  @Test
+  public void shouldEnableMtls_false_noCertsNoConfig_returnsFalse() throws IOException {
+    envProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "false");
+    assertFalse(AgentIdentityUtils.shouldEnableMtls(false, false));
+  }
 
+  @Test
+  public void shouldEnableMtls_unset_noCertsNoConfig_returnsFalse() throws IOException {
+    envProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", null);
+    assertFalse(AgentIdentityUtils.shouldEnableMtls(false, false));
+  }
+
+  @Test
+  public void shouldEnableMtls_unset_certsMissing_configExists_throwsIOException() {
+    envProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", null);
+    assertThrows(IOException.class, () -> AgentIdentityUtils.shouldEnableMtls(false, true));
+  }
+
+  @Test
+  public void getAgentIdentityCertInfo_fallbackPath_loadsCertificate() throws Exception {
+    AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
+
+    URL certUrl = getClass().getClassLoader().getResource("agent/agent_spiffe_cert.pem");
+    assertNotNull(certUrl, "Test resource agent/agent_spiffe_cert.pem not found");
+    String certPath = Paths.get(certUrl.toURI()).toAbsolutePath().toString();
     Files.copy(Paths.get(certPath), tempDir.resolve("certificates.pem"));
 
+    URL keyUrl = getClass().getClassLoader().getResource("agent/agent_spiffe_key.pem");
+    assertNotNull(keyUrl, "Test resource agent/agent_spiffe_key.pem not found");
+    String keyPath = Paths.get(keyUrl.toURI()).toAbsolutePath().toString();
+    Files.copy(Paths.get(keyPath), tempDir.resolve("private_key.pem"));
+
     envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", null);
 
     AgentIdentityUtils.CertInfo info = AgentIdentityUtils.getAgentIdentityCertInfo();
@@ -289,9 +363,9 @@ public void verifyKeyPair_mismatch_returnsFalse() throws Exception {
 
   @Test
   public void getAgentIdentityCertInfo_mismatch_throwsIOExceptionAfterRetries() throws Exception {
-    URL certUrl = getClass().getClassLoader().getResource("x509_leaf_certificate.pem");
-    assertNotNull(certUrl, "Test resource x509_leaf_certificate.pem not found");
-    String certPath = new File(certUrl.getFile()).getAbsolutePath();
+    URL certUrl = getClass().getClassLoader().getResource("agent/agent_spiffe_cert.pem");
+    assertNotNull(certUrl, "Test resource agent/agent_spiffe_cert.pem not found");
+    String certPath = Paths.get(certUrl.toURI()).toAbsolutePath().toString();
 
     // Generate a random key that won't match the cert
     KeyPairGenerator kpg = KeyPairGenerator.getInstance("RSA");
@@ -339,8 +413,35 @@ public void getAgentIdentityCertInfo_mismatch_throwsIOExceptionAfterRetries() th
     assertEquals(200, fakeTime.currentTimeMillis()); // 2 retries * 100ms
   }
 
+  @Test
+  public void getAgentIdentityCertInfo_wellKnownDirExistsNoFiles_notExplicitlyEnabled_returnsNullImmediately()
+      throws IOException {
+    AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
+    FakeTimeService fakeTime = new FakeTimeService();
+    AgentIdentityUtils.setTimeService(fakeTime);
+    envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", null);
+    envProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", null);
+
+    assertNull(AgentIdentityUtils.getAgentIdentityCertInfo());
+    assertEquals(0, fakeTime.getSleepCount());
+  }
+
+  @Test
+  public void getAgentIdentityCertInfo_wellKnownDirExistsNoFiles_explicitlyEnabled_retriesAndReturnsNull()
+      throws IOException {
+    AgentIdentityUtils.setWellKnownDir(tempDir.toAbsolutePath().toString() + "/");
+    FakeTimeService fakeTime = new FakeTimeService();
+    AgentIdentityUtils.setTimeService(fakeTime);
+    envProvider.setEnv("GOOGLE_API_CERTIFICATE_CONFIG", null);
+    envProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true");
+
+    assertNull(AgentIdentityUtils.getAgentIdentityCertInfo());
+    assertTrue(fakeTime.getSleepCount() > 0);
+  }
+
   private static class FakeTimeService implements AgentIdentityUtils.TimeService {
     private final AtomicLong currentTime = new AtomicLong(0);
+    private final AtomicInteger sleepCount = new AtomicInteger(0);
 
     @Override
     public long currentTimeMillis() {
@@ -349,8 +450,13 @@ public long currentTimeMillis() {
 
     @Override
     public void sleep(long millis) throws InterruptedException {
+      sleepCount.incrementAndGet();
       currentTime.addAndGet(millis);
     }
+
+    int getSleepCount() {
+      return sleepCount.get();
+    }
   }
 
   private static class TestEnvironmentProvider {

google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/ComputeEngineCredentialsTest.java

@@ -82,21 +82,21 @@
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.io.TempDir;
 
 /** Test case for {@link ComputeEngineCredentials}. */
 class ComputeEngineCredentialsTest extends BaseSerializationTest {
 
   private static final URI CALL_URI = URI.create("http://googleapis.com/testapi/v1/foo");
 
   private TestEnvironmentProvider envProvider;
-  private Path tempDir;
+  @TempDir private Path tempDir;
 
   @BeforeEach
   void setUp() throws IOException {
     envProvider = new TestEnvironmentProvider();
     // Inject our test environment reader into AgentIdentityUtils
     AgentIdentityUtils.setEnvReader(envProvider::getEnv);
-    tempDir = Files.createTempDirectory("compute_engine_creds_test");
 
     // Speed up polling in tests by using a fake time service that advances time immediately
     final AtomicLong currentTime = new AtomicLong(0);
@@ -120,6 +120,7 @@ public void sleep(long millis) {
   void tearDown() {
     // Reset the mocks
     AgentIdentityUtils.resetTimeService();
+    AgentIdentityUtils.setWellKnownDir("/var/run/secrets/workload-spiffe-credentials/");
     AgentIdentityUtils.setEnvReader(System::getenv);
   }
 

google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/LoggingTest.java

@@ -62,9 +62,11 @@
 import com.google.gson.JsonParser;
 import com.google.gson.JsonSyntaxException;
 import java.io.IOException;
+import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.List;
 import java.util.Map;
+import org.junit.jupiter.api.AfterAll;
 import org.junit.jupiter.api.AfterEach;
 import org.junit.jupiter.api.BeforeAll;
 import org.junit.jupiter.api.BeforeEach;
@@ -86,22 +88,33 @@ void setUp() {
     AgentIdentityUtils.setEnvReader(
         name -> {
           if ("GOOGLE_API_PREVENT_TOKEN_SHARING_FOR_GCP_SERVICES".equals(name)) {
-            return "true";
+            return "false";
           }
           return null;
         });
   }
 
+  private final List<ch.qos.logback.classic.Logger> modifiedLoggers = new ArrayList<>();
+  private final List<TestAppender> addedAppenders = new ArrayList<>();
+
   @AfterEach
   void tearDown() {
     AgentIdentityUtils.setEnvReader(System::getenv);
+    for (int i = 0; i < modifiedLoggers.size(); i++) {
+      modifiedLoggers.get(i).detachAppender(addedAppenders.get(i));
+    }
+    modifiedLoggers.clear();
+    addedAppenders.clear();
   }
 
   private TestAppender setupTestLogger(Class<?> clazz) {
     TestAppender testAppender = new TestAppender();
     testAppender.start();
-    Logger logger = LoggerFactory.getLogger(clazz);
-    ((ch.qos.logback.classic.Logger) logger).addAppender(testAppender);
+    ch.qos.logback.classic.Logger logger =
+        (ch.qos.logback.classic.Logger) LoggerFactory.getLogger(clazz);
+    logger.addAppender(testAppender);
+    modifiedLoggers.add(logger);
+    addedAppenders.add(testAppender);
     return testAppender;
   }
 
@@ -113,6 +126,11 @@ static void setup() {
     LoggingUtils.setEnvironmentProvider(testEnvironmentProvider);
   }
 
+  @AfterAll
+  static void tearDownAll() {
+    LoggingUtils.setEnvironmentProvider(LoggingUtils.SystemEnvironmentProvider.getInstance());
+  }
+
   @Test
   void userCredentials_getRequestMetadata_fromRefreshToken_hasAccessToken() throws IOException {
     TestAppender testAppender = setupTestLogger(UserCredentials.class);