feat(oauth2): implement IAM impersonation mTLS transport pinning and 401 recovery by macastelaz · Pull Request #14212 · googleapis/google-cloud-java

macastelaz · 2026-08-30T02:28:22Z

🥞 Stacked PRs

👉 This PR (feat: implement IAM impersonation mTLS transport pinning and 401 recovery)
⏳ Stacked PR test(oauth2): add mTLS in-process socket tests and live GCP WIF integration tests #14220 (test: add mTLS in-process socket tests and live GCP WIF integration tests)

Summary

Propagates the cycle-pinned mTLS transport factory into ImpersonatedCredentials during Workload Identity Federation with Service Account Impersonation, ensuring the IAM generateAccessToken call uses the exact same pinned client certificate as the STS token exchange.
Also extends 401 recovery in IdentityPoolCredentials to catch unauthorized responses across both STS and IAM calls, reloading fresh certificates and re-executing the full refresh cycle.

Test Coverage

1,030 unit tests passing across oauth2_http (including transport pinning lifecycle, multi-cycle rotation, 401 retry on IAM, and suppressed error handling).
Google Java Format: 100% compliant (com.spotify.fmt:fmt-maven-plugin:2.25).

Manual Testing

Sample test "app": https://paste.googleplex.com/5495919190081536
Test results: https://paste.googleplex.com/5757145073713152
See b/542238030 for tracking.

gemini-code-assist

Code Review

This pull request introduces support for actor tokens in IdentityPoolCredentials (specifically for file-based JSON credential sources over mTLS endpoints) and adds certificate pinning with automatic certificate reloading on 401 Unauthorized errors. It also updates other credential classes to support passing a custom HttpTransportFactory during token refresh, masks actor tokens in logs, and adds corresponding unit tests. The review feedback suggests persisting the reloaded transport factory back to this.transportFactory after a successful retry to prevent subsequent refreshes from failing first, and preserving programmatically set actorTokenSupplier instances in the Builder copy constructor.

…401 recovery - Pin mTLS HttpTransportFactory across multi-step STS and IAM token exchanges so both requests use the exact same certificate snapshot within a single refresh cycle. - Add 401 Unauthorized recovery with automatic certificate reload from X509Provider and single-retry coordination in IdentityPoolCredentials and ImpersonatedCredentials. - Preserve custom non-default HttpTransportFactory instances when X509Provider is configured. - Add comprehensive unit tests across IdentityPoolCredentialsTest, ImpersonatedCredentialsTest, and OAuth2UtilsTest.

…dentials and address review findings - Explicitly scope inner sourceCredentials to CLOUD_PLATFORM_SCOPE in ExternalAccountCredentials.buildImpersonatedCredentials and ImpersonatedCredentials.refreshAccessToken so STS issues tokens authorized to call IAM generateAccessToken even when downstream target scopes are configured via createScoped. - Ensure public no-arg ImpersonatedCredentials.refreshAccessToken delegates without overriding source credential transport settings. - Preserve custom actorTokenSupplier in IdentityPoolCredentials.Builder copy constructor when credentialSource is present. - Ensure HTTP response is closed in a finally block in ImpersonatedCredentials.refreshAccessToken. - Attach initial 401 exception as suppressed when the 401 retry attempt fails in IdentityPoolCredentials.refreshWithRetry. - Add unit tests in IdentityPoolCredentialsTest and ImpersonatedCredentialsTest covering scoped impersonation, custom actorTokenSupplier preservation, and retry exception chaining.

lsirac · 2026-09-17T15:39:04Z

    }
    if (this.impersonatedCredentials != null) {
-      return this.impersonatedCredentials.refreshAccessToken();
+      return this.impersonatedCredentials.refreshAccessToken(cycleTransportFactory);


When serviceAccountImpersonationUrl is set, IdentityPoolCredentials.refreshWithRetry fetches the subject and actor tokens to build stsTokenExchangeRequest, calls exchangeExternalCredentialForAccessToken, and then immediately delegates to impersonatedCredentials.refreshAccessToken here while discarding stsTokenExchangeRequest. The inner credential then fetches the subject and actor tokens a second time in the same refresh cycle, or four times on a 401 retry. Can we short-circuit before fetching tokens when serviceAccountImpersonationUrl is present?

Done! Added ImpersonatedCredentials impersonated = getImpersonatedCredentials();
if (impersonated != null) {
return impersonated.refreshAccessToken(cycleTransportFactory);
}

at the start of: IdentityPoolCredentials.refreshWithRetry, AwsCredentials.refreshAccessToken, and
PluggableAuthCredentials.refreshAccessToken before retrieveSubjectToken() / getActorToken() are called.

Subject and actor tokens (and AWS metadata requests) are now fetched only once per refresh attempt (or twice on a 401 retry). Kept the check inside exchangeExternalCredentialForAccessToken as a defensive fallback if called directly.

lsirac · 2026-09-17T15:39:04Z

+   * @return the refreshed access token
+   * @throws IOException if the token refresh fails
+   */
+  public AccessToken refreshAccessToken(HttpTransportFactory transportFactory) throws IOException {


The default refreshAccessToken(HttpTransportFactory) delegates to refreshAccessToken() and silently drops the transport factory. All internal subclasses override it, but a third-party subclass would lose transport pinning without warning. Should we annotate this method or class with @InternalExtensionOnly per the repository versioning guidelines?

Annotated public AccessToken refreshAccessToken(HttpTransportFactory cycleTransportFactory) with @InternalExtensionOnly across ExternalAccountCredentials, IdentityPoolCredentials, AwsCredentials, and PluggableAuthCredentials (and kept ImpersonatedCredentials.refreshAccessToken(HttpTransportFactory) package-private).

lsirac · 2026-09-17T15:39:04Z

  }

+  /**
+   * Refreshes the access token using the specified transport factory. Default implementation


Nit: The Javadoc says the default implementation delegates to refreshAccessToken(), whereas all three subclasses do the reverse by delegating the no-arg method into this overload. Also, naming the parameter transportFactory in AwsCredentials.java:127 and ImpersonatedCredentials.java:601 shadows the class field, whereas IdentityPoolCredentials names it cycleTransportFactory.

Updated the Javadoc on ExternalAccountCredentials.refreshAccessToken(HttpTransportFactory) to clarify that internal subclasses delegate refreshAccessToken() into this method while the default implementation delegates back to refreshAccessToken(), and renamed the parameter to cycleTransportFactory across all classes to avoid shadowing this.transportFactory.

lsirac · 2026-09-17T15:39:04Z

+                  .refreshAccessToken(effectiveTransportFactory);
+      Credentials authCredentials =
+          intermediateAccessToken != null
+              ? OAuth2Credentials.create(intermediateAccessToken)


Wrapping intermediateAccessToken in OAuth2Credentials.create creates a bare credential with a null quotaProjectId, which drops the x-goog-user-project header from the IAM generateAccessToken request. Can we use OAuth2Credentials.newBuilder().setAccessToken(intermediateAccessToken).setQuotaProjectId(this.sourceCredentials.getQuotaProjectId()).build() so quota attribution is preserved?

Replaced OAuth2Credentials.create(intermediateAccessToken) with a GoogleCredentials wrapper initialized via GoogleCredentials.newBuilder().setQuotaProjectId(this.sourceCredentials.getQuotaProjectId()).setUniverseDomain(this.sourceCredentials.getUniverseDomain()) so both quotaProjectId (x-goog-user-project) and universeDomain are preserved on the IAM request.

lsirac · 2026-09-17T15:39:04Z

+      }
+      AccessToken intermediateAccessToken =
+          (transportFactory == null)
+              ? ((ExternalAccountCredentials) this.sourceCredentials).refreshAccessToken()


In the null transport branch, calling refreshAccessToken directly instead of refreshIfExpired bypasses the token cache, lock, and change listeners on sourceCredentials for non-mTLS WIF impersonation. Should line 620 call refreshIfExpired and read getAccessToken, while keeping the uncached refreshAccessToken(effectiveTransportFactory) call on line 621 for pinned mTLS transports?

Done! When cycleTransportFactory == null, we now call this.sourceCredentials.refreshIfExpired() and pass this.sourceCredentials directly to new HttpCredentialsAdapter(this.sourceCredentials) (preserving token caching, locking, and change listeners), while keeping the uncached refreshAccessToken(effectiveTransportFactory) call when cycleTransportFactory != null.

lsirac · 2026-09-17T15:39:04Z

+                    "https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/test@project.iam.gserviceaccount.com:generateAccessToken")) {
+          @Override
+          HttpTransportFactory createMtlsTransportFactory(KeyStore keyStore) {
+            usedKeyStores.add(keyStore);


In the five new retry and impersonation tests, createMtlsTransportFactory records usedKeyStores.add(keyStore) and returns () -> mockTransport, which is the exact same HttpTransport instance regardless of which keystore was passed. The tests would still pass even if refreshWithRetry passed the stale transport on retry. Can we return a distinct transport per keystore so the mock verifies that the retry request ran on the transport created from freshKeyStore, and add tests for a persistent 401 failure and a non-401 IAM error?

Updated createMtlsTransportFactory(KeyStore) in the impersonation mTLS tests to return a distinct HttpTransport wrapper per KeyStore that records which KeyStore executed each request (requestKeyStores), and added "refreshAccessToken_impersonation_persistent401OnIam_throwsWithSuppressed" and "refreshAccessToken_impersonation_non401OnIam_doesNotRetry".

lsirac · 2026-09-17T15:39:04Z

  }

+  @Test
+  void builder_actorToken_plainPublicTokenUrl_throws() throws Exception {


Nit: The five actor-token endpoint validation tests added at lines 1821 through 1928 duplicate the existing five tests at lines 2873 through 2981 in the same file. Can we remove the duplicate block?

Removed the 5 duplicate actor-token endpoint validation tests.

lsirac · 2026-09-17T15:39:04Z

+                errorResponse.put("error", "invalid_token");
+                errorResponse.put("error_description", "Invalid or expired client certificate.");
+                return new MockLowLevelHttpResponse()
+                    .setStatusCode(statusCode)


The early return for non-200 status codes exits before reaching assertEquals(EXPECTED_GRANT_TYPE, grantType), so the failed first attempt never has its request form parameters verified. Can we move the grant_type assertion above the status code check?

Moved the grant_type and subject_token assertions above if (statusCode != 200) so request parameters are verified on simulated error responses as well.

lsirac · 2026-09-17T15:39:04Z

+
+  @Test
+  void isUnauthorizedException_null_returnsFalse() {
+    org.junit.jupiter.api.Assertions.assertFalse(OAuth2Utils.isUnauthorizedException(null));


Nit: This file already static-imports assertEquals and assertThrows at lines 35 and 36, but the new tests write out org.junit.jupiter.api.Assertions.assertFalse, org.junit.jupiter.api.Assertions.assertTrue, java.io.IOException, com.google.api.client.http.HttpResponseException, and com.google.api.client.http.HttpHeaders inline. Can we use static imports and top-level imports here?

Replaced all inline fully qualified names with static and top-level imports in OAuth2UtilsTest.

lsirac · 2026-09-17T15:39:04Z

+        .getTransport()
+        .addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, "");
+
+    java.util.concurrent.atomic.AtomicReference<HttpTransportFactory> capturedSourceTransport =


Nit: The line-wrapping changes to the existing private key string and universe-domain assertion message are unrelated formatting churn, and java.util.concurrent.atomic.AtomicReference is written out inline. Can we revert the unrelated rewrapping to keep the diff clean and add a top-level import for AtomicReference?

Reverted the line-wrapping changes on SA_PRIVATE_KEY_PKCS8 and the universe-domain assertion message, and replaced inline fully qualified names with top-level imports.

…in IdentityPoolCredentials

…ss and FILE cert pinning

…InternalExtensionOnly

lsirac · 2026-09-19T14:10:23Z

      try {
        KeyStore mtlsKeyStore = this.x509Provider.getKeyStore();
-        this.transportFactory = new MtlsHttpTransportFactory(mtlsKeyStore);
+        if (shouldUseMtlsTransportFactory()) {


After deserialization this only rebuilds the factory when shouldUseMtlsTransportFactory() is true, so an MtlsHttpTransportFactory subclass or a ServiceLoader-provided factory comes back keyless without a client certificate, whereas the old readObject always restored the keystore-backed factory. Should this also replace the factory when transportFactory instanceof MtlsHttpTransportFactory && !((MtlsHttpTransportFactory) transportFactory).hasKeyStore()?

lsirac · 2026-09-19T14:10:23Z

+   * @return the refreshed access token
+   * @throws IOException if the token refresh fails
+   */
+  @InternalExtensionOnly


Does this overload need to be public? Commit f2d752e compiled cleanly with it package-private, and making it public @InternalExtensionOnly advertises a call target whose default implementation ignores transportFactory and whose IdentityPoolCredentials override skips 401 cert recovery.

lsirac · 2026-09-19T14:10:24Z

+    iamTransportFactory.getTransport().setExpireTime(getDefaultExpireTime());
+    iamTransportFactory.getTransport().addStatusCodeAndMessage(HttpStatusCodes.STATUS_CODE_OK, "");
+
+    IdentityPoolCredentials sourceCredentials =


The source credential here defaults to cloud-platform, so the new !currentScopes.contains(CLOUD_PLATFORM_SCOPE) branch at ImpersonatedCredentials.java:613 never runs. Can we pass .setScopes(Collections.singletonList("https://www.googleapis.com/auth/devstorage.read_only")) on the source builder so the test covers that branch?

lsirac · 2026-09-19T14:10:24Z

+            .setTokenUrl("https://sts.mtls.googleapis.com/v1/token")
+            .build();
+    IdentityPoolCredentials deserialized = serializeAndDeserialize(regularCredentials);
+    assertTrue(


Nit: Can this assert ((MtlsHttpTransportFactory) deserialized.getTransportFactory()).hasKeyStore() too? Right now instanceof MtlsHttpTransportFactory passes even when readObject leaves the deserialized subclass with a null KeyStore.

lsirac · 2026-09-19T14:10:24Z

+          @Override
+          HttpTransportFactory createMtlsTransportFactory(KeyStore keyStore) {
+            usedKeyStores.add(keyStore);
+            return () -> transport;


Nit: createMtlsTransportFactory returns the same () -> transport instance for both ksA and ksB, so this test passes even if the 401 retry reuses ksA's transport. Could it return a per-keystore recording transport like the impersonation retry tests do?

lsirac · 2026-09-19T14:10:24Z

      throw new IllegalArgumentException(
          "Actor tokens are only supported for mTLS token exchanges. Please configure a certificate"
-              + " source or MtlsHttpTransportFactory.");
+              + " configuration in the credential source or provide an mTLS-enabled transport.");


Nit: The exception message says "or provide an mTLS-enabled transport", but isMtlsConfigured() at line 230 only accepts x509Provider != null or an MtlsHttpTransportFactory with hasKeyStore(). Should the message say "or provide an MtlsHttpTransportFactory constructed with a KeyStore"?

lsirac · 2026-09-19T14:10:24Z

-                return credential.refreshAccessToken();
-              });
+      Future<AccessToken> futureA = executor.submit(() -> credential.refreshAccessToken());
+      Future<AccessToken> futureB = executor.submit(() -> credential.refreshAccessToken());


Nit: With the new CyclicBarrier, getKeyStoreCount is deterministically 3, so line 2442 can assert assertEquals(3, ...) instead of >= 3 and verify that credential.getTransportFactory() is still mtlsTransport after the retry.

lsirac · 2026-09-19T14:10:24Z

+  }
+
+  @Test
+  void hasCertificateChanged_nullOrSameReference_returnsFalse() throws Exception {


Nit: hasCertificateChanged_nullOrSameReference_returnsFalse asserts assertTrue at lines 162-163 and only checks identical references. Could we also compare two distinct KeyStore instances loaded with the same cert so oldCerts.equals(newCerts) is tested directly?

lsirac · 2026-09-19T14:10:24Z

+            return new AccessToken("intermediate-sts-token-" + count, null);
+          }
+
+          @Override


Nit: The createScoped override here is never called because line 1554 already sets cloud-platform scope, and the 401 retry comes from HttpCredentialsAdapter.handleResponse on the null-transport path. Can we drop the unused createScoped override and rename the test to reflect that?

lsirac · 2026-09-19T14:10:24Z

      sourceCredentials =
          PluggableAuthCredentials.newBuilder((PluggableAuthCredentials) this)
              .setServiceAccountImpersonationUrl(null)
+              .setScopes(Collections.singletonList(OAuth2Utils.CLOUD_PLATFORM_SCOPE))


Because buildImpersonatedCredentials() clears serviceAccountImpersonationUrl on the inner sourceCredentials copy, getServiceAccountEmail() returns null when PluggableAuthCredentials.retrieveSubjectToken() runs on that copy, so GOOGLE_EXTERNAL_ACCOUNT_IMPERSONATED_EMAIL is omitted from envMap. Should buildImpersonatedCredentials() preserve the impersonated email on the PluggableAuthCredentials source copy?

macastelaz requested review from a team as code owners August 30, 2026 02:28

macastelaz marked this pull request as draft August 30, 2026 02:28

gemini-code-assist Bot reviewed Aug 30, 2026

View reviewed changes

Comment thread google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java Outdated

Comment thread google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java Outdated

macastelaz force-pushed the cert-bound-oauth-iam-pinning branch from 73a0301 to 4be79cb Compare August 30, 2026 03:19

macastelaz mentioned this pull request Aug 31, 2026

test(oauth2): add mTLS in-process socket tests and live GCP WIF integration tests #14220

Draft

macastelaz force-pushed the cert-bound-oauth-iam-pinning branch from 4be79cb to ce341c7 Compare September 1, 2026 02:09

macastelaz marked this pull request as ready for review September 4, 2026 16:41

macastelaz requested a review from lsirac September 4, 2026 16:41

macastelaz force-pushed the cert-bound-oauth-iam-pinning branch from e09d76e to dadeda6 Compare September 17, 2026 03:17

lsirac requested changes Sep 17, 2026

View reviewed changes

macastelaz added 5 commits September 17, 2026 18:09

fix(oauth2): address PR review feedback for IAM mTLS transport pinning

73c5a0a

fix(oauth2): address post-review audit edge cases for mTLS pinning

8034525

fix(oauth2): preserve exact class check for MtlsHttpTransportFactory …

17aae1f

…in IdentityPoolCredentials

test(oauth2): add coverage for custom MtlsHttpTransportFactory subcla…

f2d752e

…ss and FILE cert pinning

fix(oauth2): annotate refreshAccessToken(HttpTransportFactory) with @…

a5e69ed

…InternalExtensionOnly

macastelaz requested review from blakeli0 and lsirac September 18, 2026 02:50

lsirac requested changes Sep 19, 2026

View reviewed changes

Conversation

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

Uh oh!

🥞 Stacked PRs

Summary

Test Coverage

Manual Testing

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

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!

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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

macastelaz commented Aug 30, 2026 •

edited

Loading