feat(oauth2): support non-mTLS token URLs and unbound actor tokens in IdentityPoolCredentials by macastelaz · Pull Request #14430 · googleapis/google-cloud-java

macastelaz · 2026-09-18T02:22:55Z

Summary

Allows actor_token and actor_token_type to be used in IdentityPoolCredentials with standard (non-mTLS) STS and IAM impersonation endpoints and without requiring client certificate (certificate_config) configuration.

Context & Rationale

In #13955, client-side guardrails (isMtlsConfigured() and validateMtlsEndpoint()) were enforced in the IdentityPoolCredentials constructor because Google STS initially required actor tokens to be paired with certificate-bound tokens over mTLS. Per the original design discussion, this was intentionally designed as a one-way door that could be loosened in a non-breaking manner once backend support for non-mTLS actor token exchanges was ready.

Changes

Removed Constructor mTLS Restrictions:
- Removed isMtlsConfigured() check in IdentityPoolCredentials(Builder) so actor tokens can be configured with standard HttpTransportFactory instances and without a certificate block.
- Removed validateMtlsEndpoint() checks on tokenUrl and serviceAccountImpersonationUrl, allowing standard public endpoints (e.g., https://sts.googleapis.com/v1/token) to be used with actor tokens.
- Preserved strict pairing validation between actorTokenSupplier and actorTokenType, as well as JSON format checks for file-based actor token extraction.
401 Handling & Cert Rotation:
- When x509Provider == null (non-mTLS credentials), refreshAccessToken() uses standard transport without snapshotting a KeyStore, and 401 Unauthorized errors propagate immediately without retry.
- When x509Provider != null && transportFactory instanceof MtlsHttpTransportFactory (mTLS credentials), per-cycle certificate pinning and single-retry cert reload on 401 remain unchanged.
Javadoc & Test Coverage:
- Updated class-level and builder Javadocs on IdentityPoolCredentials.
- Updated negative constructor tests to positive verification tests (*_succeeds).
- Added refreshAccessToken_401WithActorTokenAndNonMtlsTransport_bubblesUpWithoutRetry and fromStream_fileCredentialSource_withoutCertificateConfig_andActorToken_withNonMtlsUrl_refreshesSuccessfully to verify end-to-end non-mTLS actor token exchanges.

Verification

All 1,035 unit tests passing in oauth2_http module (including all 88 tests in IdentityPoolCredentialsTest).
100% compliant with fmt-maven-plugin:2.25:check.

… IdentityPoolCredentials

gemini-code-assist

Code Review

This pull request removes the restriction requiring mTLS endpoints and transport configuration for actor token exchanges in IdentityPoolCredentials. Validation checks enforcing mTLS are removed, and unit tests are updated to verify that configuring actor tokens without mTLS now succeeds. Feedback is provided regarding a cross-platform issue in a new test where unescaped backslashes in a file path can cause JSON parsing failures on Windows.

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

lsirac · 2026-09-19T14:47:36Z

-    return this.x509Provider != null
-        || (this.transportFactory instanceof MtlsHttpTransportFactory
-            && ((MtlsHttpTransportFactory) this.transportFactory).hasKeyStore());
  }


Now that isMtlsConfigured() is removed here, MtlsHttpTransportFactory.hasKeyStore() and checkHasKeyStore(KeyStore) have no production callers left in the repo, but new MtlsHttpTransportFactory(keyStore) still scans keyStore.aliases() and certificate chains on every refresh and 401 retry. Should we remove hasKeyStore() and the duplicate mtlsHttpTransportFactory_hasKeyStore_* tests in IdentityPoolCredentialsTest?

Removed the 3 duplicate mtlsHttpTransportFactory_hasKeyStore_* tests from IdentityPoolCredentialsTest since they are already covered in MtlsHttpTransportFactoryTest. Note, however, that I kept MtlsHttpTransportFactory.hasKeyStore() itself because PR #14212 actively calls ((MtlsHttpTransportFactory) this.transportFactory).hasKeyStore() in IdentityPoolCredentials.shouldUseMtlsTransportFactory()
and readObject().

lsirac · 2026-09-19T14:47:36Z

-            + " source or MtlsHttpTransportFactory.",
-        e.getMessage());
+    assertNotNull(credentials);
+    assertSame(actorSupplier, credentials.getIdentityPoolActorTokenSupplier());


In Builder(IdentityPoolCredentials) at line 391, this.actorTokenSupplier is only copied when this.credentialSource == null, while this.actorTokenType is copied unconditionally. For a credential built like this test with both credentialSource and .setActorTokenSupplier(actorSupplier), calling credentials.createScoped(...) or credentials.toBuilder().build() drops actorTokenSupplier and throws IllegalArgumentException. Should Builder(IdentityPoolCredentials) preserve credentials.actorTokenSupplier whenever credentials.actorTokenSupplier != credentials.subjectTokenSupplier?

Great catch! Updated Builder(IdentityPoolCredentials) to preserve credentials.actorTokenSupplier when credentials.actorTokenSupplier != credentials.subjectTokenSupplier (even when credentialSource != null), and added a credentials.createScoped(...) assertion to builder_actorTokenWithNonMtlsTransportFactory_succeeds to verify both actorTokenSupplier and actorTokenType are preserved.

lsirac · 2026-09-19T14:47:36Z

+            .build();
+    assertNotNull(cred);
+    assertEquals("urn:ietf:params:oauth:token-type:jwt", cred.getActorTokenType());
+    assertEquals(MockExternalAccountCredentialsTransport.STS_MTLS_URL, cred.getTokenUrl());


Can we assert cred.getServiceAccountImpersonationUrl() here? The test configures the impersonation URL and verifies tokenUrl and actorTokenType, but it omits checking the impersonation URL on the built instance.

Added assertEquals for cred.getServiceAccountImpersonationUrl().

lsirac · 2026-09-19T14:47:36Z

+
+    // Verify Java serialization/deserialization round-trip preserves actor token config
+    IdentityPoolCredentials deserialized = serializeAndDeserialize(idp);
+    assertEquals("urn:ietf:params:oauth:token-type:jwt", deserialized.getActorTokenType());


Can we add assertNull(deserialized.getX509Provider()) and assertFalse(deserialized.getTransportFactory() instanceof MtlsHttpTransportFactory) to this deserialization check? We should confirm readObject() did not trigger unintended mTLS reconstruction on non-mTLS credentials.

Added assertNull(deserialized.getX509Provider()) and assertFalse(deserialized.getTransportFactory() instanceof MtlsHttpTransportFactory) after serializeAndDeserialize(idp).

lsirac · 2026-09-19T14:47:36Z

-  void builder_actorTokenWithNoArgMtlsFactory_throws() throws Exception {
-    // A no-arg MtlsHttpTransportFactory (e.g. from deserialization) has no KeyStore,
-    // so isMtlsConfigured() should return false and building should fail.
+  void builder_actorTokenWithNoArgMtlsFactory_succeeds() throws Exception {


Can we assert assertSame(factory, credentials.getTransportFactory()) in this test and builder_actorTokenWithEmptyMtlsFactory_succeeds to verify the configured factory was preserved? We can also drop throws Exception from the method signature.

Added assertSame(noArgFactory, credentials.getTransportFactory()) (and dropped throws Exception) in builder_actorTokenWithNoArgMtlsFactory_succeeds, and added assertSame(emptyFactory, credentials.getTransportFactory()) in builder_actorTokenWithEmptyMtlsFactory_succeeds.

lsirac · 2026-09-19T14:47:36Z

+  void builder_actorTokenWithNonMtlsTransportFactory_succeeds() {
    IdentityPoolCredentialSource credentialSource = createFileCredentialSource();
+    IdentityPoolActorTokenSupplier actorSupplier =
+        new IdentityPoolActorTokenSupplier() {


Nit: Can we use a lambda like context -> "token" or testActorSupplier here instead of the anonymous inner class?

Replaced the anonymous inner class with IdentityPoolActorTokenSupplier actorSupplier = context -> "token";.

lsirac · 2026-09-19T14:47:36Z

+            .setHttpTransportFactory(transportFactory);
+
+    TestableIdentityPoolCredentials testable =
+        new TestableIdentityPoolCredentials(builder, true, false);


Nit: Can we just use new TestableIdentityPoolCredentials(builder, true) here? The two-argument constructor defaults failOnAllExchanges to false already. We can also pass OAuth2Utils.HTTP_TRANSPORT_FACTORY directly instead of instantiating MockExternalAccountCredentialsTransportFactory since TestableIdentityPoolCredentials mocks the exchange.

Updated refreshAccessToken_401WithActorTokenAndNonMtlsTransport_bubblesUpWithoutRetry to pass OAuth2Utils.HTTP_TRANSPORT_FACTORY directly and use the two-argument new TestableIdentityPoolCredentials(builder, true) constructor.

lsirac · 2026-09-22T21:03:41Z

Thanks for the updates I think there are a few issues to think about:

When service account impersonation is configured with an actor token, refreshing the token fetches both the subject and actor tokens twice on the first call and still re-fetches and discards them on every refresh while the intermediate STS token is cached.
This may be pre-existing, but if actor_token is empty or whitespace in a JSON credential file, or if the actor token supplier returns null, we either send an empty actor_token parameter to STS or silently drop the actor token and exchange only the subject token instead of failing.
This also may be pre-existing, but when a custom transport factory is configured alongside a certificate config, deserializing the credentials overwrites the custom transport factory with the mTLS transport.

…sonation caching, and custom transport deserialization

macastelaz · 2026-09-23T03:49:19Z

Thanks for the updates I think there are a few issues to think about:

When service account impersonation is configured with an actor token, refreshing the token fetches both the subject and actor tokens twice on the first call and still re-fetches and discards them on every refresh while the intermediate STS token is cached.

This may be pre-existing, but if actor_token is empty or whitespace in a JSON credential file, or if the actor token supplier returns null, we either send an empty actor_token parameter to STS or silently drop the actor token and exchange only the subject token instead of failing.

This also may be pre-existing, but when a custom transport factory is configured alongside a certificate config, deserializing the credentials overwrites the custom transport factory with the mTLS transport.

Thanks for the thorough review! Addressed all three issues:

Impersonation + actor token double-fetch & caching: IdentityPoolCredentials.refreshAccessToken() now delegates early to getImpersonatedCredentials() (lazily initialized via initializeImpersonatedCredentials(), preserving the actorTokenSupplier on the cloned sourceCredentials) before reading subjectToken and actorToken. Both tokens are now fetched only once on initial refresh and are not re-fetched while the intermediate STS token in sourceCredentials remains
valid.
Empty / whitespace / null actor token validation: FileIdentityPoolSubjectTokenSupplier now validates token.trim().isEmpty() for both JSON fields (extractField) and text files (parseToken), and IdentityPoolCredentials.refreshAccessToken() now verifies that actorToken is non-null and non-empty whenever actorTokenSupplier != null, throwing an IOException instead of sending an empty actor_token parameter or silently dropping the acting party.
Custom HttpTransportFactory preservation on deserialization: Added a serialized useMtlsTransportFactory flag (checked via shouldUseMtlsTransportFactory()) so readObject only creates an MtlsHttpTransportFactory when the credential was using the default or MtlsHttpTransportFactory transport, preserving custom HttpTransportFactory instances across serialization/deserialization.

lsirac · 2026-09-23T19:59:29Z

+  }
+
+  @Test
+  void refreshAccessToken_401WithActorTokenAndNonMtlsTransport_bubblesUpWithoutRetry() {


This builder never sets an X509Provider, so the 401 check in IdentityPoolCredentials short-circuits on this.x509Provider != null before shouldUseMtlsTransportFactory runs, and OAuth2Utils.HTTP_TRANSPORT_FACTORY is treated as mTLS-eligible by isDefaultOrMtlsTransportFactory. Can we configure a TestX509Provider and a custom non-mTLS transport factory here so shouldUseMtlsTransportFactory is what stops the 401 retry?

Done — the test now sets a TestX509Provider and a custom non-mTLS MockExternalAccountCredentialsTransportFactory, so shouldUseMtlsTransportFactory() is what stops the 401 retry.

lsirac · 2026-09-23T19:59:29Z

-    }
+  @VisibleForTesting
+  boolean shouldUseMtlsTransportFactory() {
+    return this.useMtlsTransportFactory


For streams serialized before useMtlsTransportFactory existed, transportFactoryClassName was captured as DefaultHttpTransportFactory before initializeMtlsTransport swapped in MtlsHttpTransportFactory, so useMtlsTransportFactory deserializes as false and readObject skips mTLS reconstruction. Should this return this.useMtlsTransportFactory || isDefaultOrMtlsTransportFactory(this.transportFactory) instead?

Done — shouldUseMtlsTransportFactory() now returns this.useMtlsTransportFactory || isDefaultOrMtlsTransportFactory(this.transportFactory).

lsirac · 2026-09-23T19:59:29Z

+    if (this.serviceAccountImpersonationUrl == null) {
+      return null;
+    }
+    ImpersonatedCredentials local = this.impersonatedCredentials;


Should impersonatedCredentials be declared volatile? Without volatile, the first check in this double-checked locking block can let a concurrent refreshAccessToken caller observe a non-null reference before its constructor writes finish.

Done — declared impersonatedCredentials as volatile.

lsirac · 2026-09-23T19:59:29Z

    @CanIgnoreReturnValue
    public Builder setCredentialSource(IdentityPoolCredentialSource credentialSource) {
      super.setCredentialSource(credentialSource);
+      this.isClonedTransportInitialized = false;


Nit: toBuilder copies x509Provider and getX509Provider prefers that field, so calling setCredentialSource with a new source on a cloned builder still reloads the keystore from the old source's x509Provider. Should setCredentialSource also clear this.x509Provider when the credential source changes?

Done — setCredentialSource now clears x509Provider when the builder already has a different credential source, so a new source on a cloned builder builds its own provider. Added toBuilder_setCredentialSource_withNewSource_clearsCopiedX509Provider to cover it.

lsirac · 2026-09-23T19:59:30Z

+      if (actorToken == null || actorToken.trim().isEmpty()) {
+        throw new IOException("The provided actor token cannot be null or empty.");
+      }
+      if (this.actorTokenType != null) {


Nit: The constructor already checks that actorTokenType is not null or empty whenever actorTokenSupplier is set, so this inner if is dead code and would silently drop the acting party if that invariant ever broke. Can we pass Objects.requireNonNull(this.actorTokenType) to ActingParty instead?

Done — removed the inner if and now pass Objects.requireNonNull(this.actorTokenType) to ActingParty.

lsirac · 2026-09-23T19:59:30Z

            Collections.singletonList("https://www.googleapis.com/auth/cloud-platform"));

-    assertEquals(2, getKeyStoreCount.get());
+    assertEquals(1, getKeyStoreCount.get());


Nit: Now that this test asserts getKeyStoreCount stays at 1 and the transport factory instance is reused across createScoped, can we rename the method to createScoped_withCertificateConfig_preservesMtlsTransportSnapshot?

Done — renamed to createScoped_withCertificateConfig_preservesMtlsTransportSnapshot.

lsirac · 2026-09-23T19:59:30Z

-              + " endpoint. Please use an mTLS endpoint (e.g. containing '.mtls.') or Private"
-              + " Service Connect (containing '.p.').");
-    }
+  @VisibleForTesting


Nit: No test calls shouldUseMtlsTransportFactory, hasInitializedMtlsTransport, or createMtlsTransportFactory. Can we drop @VisibleForTesting and make these helpers private?

Done — dropped @VisibleForTesting and made shouldUseMtlsTransportFactory, hasInitializedMtlsTransport, and createMtlsTransportFactory private.

lsirac · 2026-09-23T19:59:30Z

  }

+  @Test
+  void readTokens_emptyOrWhitespaceActorField_throwsIOException(@TempDir Path tempDir)


Nit: Since extractField now rejects whitespace-only values for subject tokens too, can we add a sibling test for an empty or whitespace subject token field?

Done — added readTokens_emptyOrWhitespaceSubjectField_throwsIOException.

…sport helpers, x509Provider cloning, and actor token tests

lsirac · 2026-09-24T23:51:08Z

+
+    ImpersonatedCredentials impersonated = getImpersonatedCredentials();
+    if (impersonated != null) {
+      return impersonated.refreshAccessToken();


When impersonation is configured and hasInitializedMtlsTransport() is already true, the block at lines 198 to 204 reloads the keystore from disk and allocates cycleTransportFactory, then immediately discards it when impersonated.refreshAccessToken() returns here. Should we move the per-cycle cycleTransportFactory snapshot below this if (impersonated != null) block and only run the !hasInitializedMtlsTransport() lazy initialization before getImpersonatedCredentials()?

Good catch, done. The lazy initialization still runs before getImpersonatedCredentials(),
so the impersonated credential is built with the mTLS factory. The per-cycle snapshot now
runs only on the direct STS path, after the impersonation return. On the first cycle, the
snapshot reuses the KeyStore that the lazy initialization just loaded, so each refresh
reads it at most once.

Added two tests:

refreshAccessToken_withImpersonation_doesNotReloadKeyStoreAfterLazyInit: the first
refresh loads the KeyStore once and getImpersonatedCredentials() sees an
MtlsHttpTransportFactory; the second refresh doesn't load it again.

refreshAccessToken_withoutImpersonation_readsKeyStoreOncePerCycle: the first cycle loads
the KeyStore once and the exchange reuses that factory; each later cycle takes one new
snapshot.

lsirac · 2026-09-24T23:51:08Z

+        new MockExternalAccountCredentialsTransportFactory();
+
+    Map<String, Object> certMap = new HashMap<>();
+    certMap.put("use_default_certificate_config", true);


Because x509Provider is transient, readObject() constructs a new X509Provider with a null path when use_default_certificate_config is true, which throws IOException on getKeyStore() and hits the catch block before this.transportFactory is touched. That makes this test pass even if the shouldUseMtlsTransportFactory() check in readObject() is removed. Can we set use_default_certificate_config to false and point certificate_config_location to "testresources/mtls/certificate_config.json" so getKeyStore() succeeds during deserialization?

The test now sets use_default_certificate_config to false and points
certificate_config_location at testresources/mtls/certificate_config.json, so
getKeyStore() succeeds during deserialization. I confirmed the test now fails if the
shouldUseMtlsTransportFactory() check in readObject() is bypassed.

lsirac · 2026-09-24T23:51:08Z

+        || transportFactory instanceof OAuth2Utils.DefaultHttpTransportFactory
+        || transportFactory.getClass() == MtlsHttpTransportFactory.class
+        || (transportFactory instanceof MtlsHttpTransportFactory
+            && !((MtlsHttpTransportFactory) transportFactory).hasKeyStore());


Nit: OAuth2Utils.HTTP_TRANSPORT_FACTORY is already an instance of OAuth2Utils.DefaultHttpTransportFactory, and transportFactory.getClass() == MtlsHttpTransportFactory.class short-circuits the hasKeyStore() check since MtlsHttpTransportFactory has no subclasses. Can we simplify this to check transportFactory == null || transportFactory instanceof OAuth2Utils.DefaultHttpTransportFactory || transportFactory instanceof MtlsHttpTransportFactory?

Done, simplified to the null / DefaultHttpTransportFactory / MtlsHttpTransportFactory instanceof checks.

lsirac · 2026-09-24T23:51:08Z

+    tokenJson.put("subject_token", "testSubjectToken");
+    OAuth2Utils.writeInputStreamToFile(
+        new ByteArrayInputStream(tokenJson.toPrettyString().getBytes(StandardCharsets.UTF_8)),
+        tokenFile.toString());


Nit: Since this test only builds the credentials and never reads from disk, can we drop @TempDir and pass "credential.json" directly like serialize_deserialize_withCustomTransportFactoryAndCertConfig_preservesCustomFactory does at line 3300?

Done. I dropped @tempdir and the token-file write, and the test now passes "credential.json" directly.

- Skip the per-cycle KeyStore snapshot on the impersonation path; reuse the lazily initialized KeyStore on the first direct cycle. - Simplify isDefaultOrMtlsTransportFactory. - Make the custom-factory serialization test load a real cert config. - Drop the unused TempDir from the toBuilder X509Provider test.

The new constructor and getHttpStatusCode() change the computed serialVersionUID, so instances serialized by released versions would fail to load. Same change as in PR googleapis#14212.

lsirac · 2026-09-29T05:39:11Z

+            return ks;
+          }
+        };
+    ImpersonatedCredentials impersonated = Mockito.mock(ImpersonatedCredentials.class);


Mocking ImpersonatedCredentials without Mockito.withSettings().withoutAnnotations() fails on the JDK 8 CI job with ArrayStoreException: sun.reflect.annotation.EnumConstantNotPresentExceptionProxy because ImpersonatedCredentials is annotated with @NullMarked. Pass Mockito.withSettings().withoutAnnotations() here like IamUtilsTest does.

Updated to pass Mockito.withSettings().withoutAnnotations().

lsirac · 2026-09-29T05:39:11Z

+  static boolean isDefaultOrMtlsTransportFactory(@Nullable HttpTransportFactory transportFactory) {
+    return transportFactory == null
+        || transportFactory instanceof OAuth2Utils.DefaultHttpTransportFactory
+        || transportFactory instanceof MtlsHttpTransportFactory;


Using transportFactory instanceof MtlsHttpTransportFactory here causes shouldUseMtlsTransportFactory() to return true for caller supplied MtlsHttpTransportFactory subclasses, which overwrites custom subclasses with a plain MtlsHttpTransportFactory and conflicts with CustomMtlsHttpTransportFactory in #14212. Check transportFactory.getClass() == MtlsHttpTransportFactory.class || (transportFactory instanceof MtlsHttpTransportFactory && !((MtlsHttpTransportFactory) transportFactory).hasKeyStore()) instead so custom subclasses are preserved.

Updated isDefaultOrMtlsTransportFactory to check transportFactory.getClass() == MtlsHttpTransportFactory.class || (transportFactory instanceof MtlsHttpTransportFactory && !((MtlsHttpTransportFactory) transportFactory).hasKeyStore()), matching #14212.

lsirac · 2026-09-29T05:39:11Z

          KeyStore freshKeyStore = this.x509Provider.getKeyStore();
-          HttpTransportFactory retryTransportFactory = new MtlsHttpTransportFactory(freshKeyStore);
+          HttpTransportFactory retryTransportFactory = createMtlsTransportFactory(freshKeyStore);
+          if (!hasInitializedMtlsTransport()) {


Is if (!hasInitializedMtlsTransport()) ever true here? The lazy init check at the top of refreshAccessToken() already initializes this.transportFactory under the same this.x509Provider != null && shouldUseMtlsTransportFactory() condition before the STS call runs.

Good catch! Since this.transportFactory is already initialized at the start of refreshAccessToken() whenever !hasInitializedMtlsTransport() is true, this check in the catch block was unreachable. Removed

lsirac · 2026-09-29T05:39:11Z

-              + " Service Connect (containing '.p.').");
-    }
+  private boolean shouldUseMtlsTransportFactory() {
+    return this.useMtlsTransportFactory || isDefaultOrMtlsTransportFactory(this.transportFactory);


Can we add a test for the || isDefaultOrMtlsTransportFactory(this.transportFactory) deserialization fallback? Right now serializeAndDeserialize only runs on instances built on this branch where useMtlsTransportFactory is already true, so setting useMtlsTransportFactory to false via reflection before round tripping would cover older serialized streams.

Added CustomMtlsHttpTransportFactory and customMtlsHttpTransportFactorySubclass_preservedInConstructorAndRefresh_rebuiltOnDeserialization (matching #14212).

Passing .setHttpTransportFactory(new CustomMtlsHttpTransportFactory(ks)) sets useMtlsTransportFactory = false, and upon deserialization ServiceCredentials.readObject() instantiates a keyless CustomMtlsHttpTransportFactory() via newInstance(), exercising the || isDefaultOrMtlsTransportFactory(this.transportFactory) fallback branch in shouldUseMtlsTransportFactory().

lsirac · 2026-09-29T05:39:11Z

    @CanIgnoreReturnValue
    public Builder setHttpTransportFactory(HttpTransportFactory transportFactory) {
      super.setHttpTransportFactory(transportFactory);
+      this.useMtlsTransportFactory = isDefaultOrMtlsTransportFactory(transportFactory);


Can we add a test for credential.toBuilder().setHttpTransportFactory(customFactory).build() on a credential with a certificate config? Nothing currently tests that Builder.setHttpTransportFactory updates useMtlsTransportFactory so the custom transport factory is not overwritten by lazy mTLS init.

Added toBuilder_setHttpTransportFactory_withCustomFactoryAndCertConfig_preservesCustomFactory to verify that overriding the transport factory via toBuilder().setHttpTransportFactory(customFactory) preserves customFactory both on build() and across refreshAccessToken().

lsirac · 2026-09-29T05:39:11Z

+
+    assertNotNull(rebuilt.getX509Provider());
+    assertNotSame(trackingProvider, rebuilt.getX509Provider());
+    assertEquals(1, getKeyStoreCount.get());


Can we also assert assertNotSame(credential.getTransportFactory(), rebuilt.getTransportFactory()) here to verify that setCredentialSource resets isClonedTransportInitialized?

Added assertNotSame(credential.getTransportFactory(), rebuilt.getTransportFactory()).

lsirac · 2026-09-29T05:39:11Z

+            .setAudience("audience")
+            .setSubjectTokenType("urn:ietf:params:oauth:token-type:jwt")
+            .setTokenUrl("https://sts.googleapis.com/v1/token")
+            .setHttpTransportFactory(OAuth2Utils.HTTP_TRANSPORT_FACTORY)


Use MockExternalAccountCredentialsTransportFactory and STS_URL in both builders in this test instead of OAuth2Utils.HTTP_TRANSPORT_FACTORY with https://sts.googleapis.com/v1/token so a validation regression does not make a live network call.

Updated refreshAccessToken_withNullOrWhitespaceActorToken_throwsIOException to use MockExternalAccountCredentialsTransportFactory and STS_URL.

lsirac · 2026-09-29T05:39:11Z

-     * Sets the actor token supplier used for certificate-bound OAuth 2.0 token exchanges. The
-     * supplier provides an actor token representing the entity on whose behalf the subject is
-     * acting.
+     * Sets the actor token supplier used for OAuth 2.0 token exchanges. The supplier provides an


In RFC 8693 Section 2.1, the actor token represents the acting party that acts on behalf of the subject rather than the entity on whose behalf the subject is acting. Should we update this Javadoc wording to match?

updated the Javadoc to match RFC 8693 Section 2.1 terminology

lsirac · 2026-09-30T17:14:19Z


    @CanIgnoreReturnValue
    public Builder setCredentialSource(IdentityPoolCredentialSource credentialSource) {
+      if (this.credentialSource != null && this.credentialSource != credentialSource) {


This clears x509Provider, but this.transportFactory still holds the old credential's keyed MtlsHttpTransportFactory, so switching to a credential source without a certificate config leaves the rebuilt credential pinned to the old client certificate. Should we also reset this.transportFactory to null here when useMtlsTransportFactory is true, and add a test for setting a new source without a certificate config?

Done - setCredentialSource now also resets this.transportFactory = null when this.useMtlsTransportFactory is true, and toBuilder_setCredentialSource_withNewSource_clearsCopiedX509Provider now verifies that switching to a credential source without a certificate config resets both x509Provider and MtlsHttpTransportFactory.

lsirac · 2026-09-30T17:14:19Z

-                + value.getClass().getName());
-      }
-      return (String) value;
+      return extractField(fileContents, targetFieldName);


Since parseToken now delegates to extractField and rejects blank JSON token values for URL and single-token file sources too, can we add a test for a blank token field on parseToken or getSubjectToken? Right now only readTokens is covered.

Added parseToken_jsonFormat_emptyOrWhitespaceField_throws covering both supplier.getSubjectToken(null) and FileIdentityPoolSubjectTokenSupplier.parseToken(...) when the JSON token field is whitespace.

lsirac · 2026-09-30T17:14:19Z

-        validateMtlsEndpoint(getServiceAccountImpersonationUrl(), "serviceAccountImpersonationUrl");
-      }
-    }
+  static boolean isDefaultOrMtlsTransportFactory(@Nullable HttpTransportFactory transportFactory) {


Nit: Nothing outside IdentityPoolCredentials calls isDefaultOrMtlsTransportFactory. Can we make it private static?

Done — changed isDefaultOrMtlsTransportFactory to private static.

lsirac · 2026-09-30T17:14:19Z

    private @Nullable IdentityPoolActorTokenSupplier actorTokenSupplier;
    private @Nullable String actorTokenType;
    private @Nullable X509Provider x509Provider;
+    private @Nullable Boolean useMtlsTransportFactory;


Nit: Can we declare this as private boolean useMtlsTransportFactory = true; instead of a nullable Boolean? It is only null on a fresh builder when setHttpTransportFactory was never called, where the constructor fallback always evaluates to true anyway.

Done — changed Builder.useMtlsTransportFactory to private boolean useMtlsTransportFactory = true; and simplified the constructor assignment to this.useMtlsTransportFactory = builder.useMtlsTransportFactory;.

lsirac · 2026-09-30T17:14:19Z

+        Mockito.mock(ImpersonatedCredentials.class, Mockito.withSettings().withoutAnnotations());
+    Mockito.when(impersonated.refreshAccessToken())
+        .thenReturn(new AccessToken("impersonatedAccessToken", null));
+    List<HttpTransportFactory> factoriesSeenByImpersonation = new java.util.ArrayList<>();


Nit: Can we use new ArrayList<>() here since java.util.ArrayList is already imported at line 71?

Updated to new ArrayList<>()

feat(oauth2): support non-mTLS token URLs and unbound actor tokens in…

8e5f005

… IdentityPoolCredentials

macastelaz requested review from a team as code owners September 18, 2026 02:22

gemini-code-assist Bot reviewed Sep 18, 2026

View reviewed changes

Comment thread ...h-library-java/oauth2_http/javatests/com/google/auth/oauth2/IdentityPoolCredentialsTest.java Outdated

macastelaz and others added 2 commits September 17, 2026 21:25

Apply suggestion from @gemini-code-assist[bot]

5b01057

Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>

test(oauth2): strengthen non-mTLS actor token unit tests

82254db

macastelaz requested review from lqiu96 and lsirac September 18, 2026 03:16

lsirac requested changes Sep 19, 2026

View reviewed changes

fix(auth): address PR googleapis#14430 review feedback

961361b

macastelaz requested review from blakeli0 and lsirac September 22, 2026 14:36

fix(oauth2): address review feedback on actor token validation, imper…

78790db

…sonation caching, and custom transport deserialization

lsirac reviewed Sep 23, 2026

View reviewed changes

macastelaz added 2 commits September 24, 2026 16:30

fix(oauth2): address PR googleapis#14430 review feedback on mTLS tran…

3e40f0a

…sport helpers, x509Provider cloning, and actor token tests

style(oauth2): apply google-java-format

23fc045

macastelaz requested a review from lsirac September 24, 2026 16:55

lsirac requested changes Sep 24, 2026

View reviewed changes

macastelaz requested a review from lsirac September 28, 2026 15:38

fix(auth): pin OAuthException serialVersionUID to its released value

e5280d8

The new constructor and getHttpStatusCode() change the computed serialVersionUID, so instances serialized by released versions would fail to load. Same change as in PR googleapis#14212.

lsirac requested changes Sep 29, 2026

View reviewed changes

fix(oauth2): address PR googleapis#14430 review feedback

abfb97c

macastelaz requested a review from lsirac September 30, 2026 03:55

lsirac reviewed Sep 30, 2026

View reviewed changes

fix(oauth2): address additional PR googleapis#14430 review feedback

d12c7ec

macastelaz requested a review from lsirac September 30, 2026 18:25

lsirac approved these changes Oct 2, 2026

View reviewed changes

Conversation

macastelaz commented Sep 18, 2026

Summary

Context & Rationale

Changes

Verification

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lsirac commented Sep 22, 2026

Uh oh!

macastelaz commented Sep 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

macastelaz Sep 28, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

macastelaz Sep 28, 2026 •

edited

Loading