test(oauth2): add mTLS in-process socket tests and live GCP WIF integration tests - #14220
Draft
macastelaz wants to merge 32 commits into
Draft
macastelaz wants to merge 32 commits into
macastelaz wants to merge 32 commits into
Conversation
Implementation of Phase 1-3 of the Cert-Bound Oauth2 Design Document: 1. Extend IdentityPoolCredentialSource to parse actorTokenFieldName. 2. Relax mutual exclusivity to allow BOTH file and certificate configurations. 3. Parse actor_token_type in ExternalAccountCredentials. 4. Refactor FileIdentityPoolTokenSupplier and track file timestamp via volatile CachedFile for the parsed JSON payload. 5. Inject actor_token and actor_token_type into StsTokenExchangeRequest using ActingParty. 6. Enforce that actor token extraction requires an mTLS STS configuration.
Fixes test failures and thread synchronization bugs regarding actor token credentials from https://paste.googleplex.com/5381957298028544
- Mark CachedFile and X509Provider transient to ensure clean serialization. - Add static modifier to FileIdentityPoolTokenSupplier serialVersionUID. - Make IdentityPoolActorTokenSupplier public with @NullMarked annotation. - Preserve actorTokenSupplier in IdentityPoolCredentials Builder copy constructor. - Mask actor_token in Slf4jLoggingHelpers sensitive keys. - Add no-arg constructor to MtlsHttpTransportFactory for serialization support. - Handle Data.isNull in FileIdentityPoolTokenSupplier JSON parsing. - Add comprehensive test coverage for supplier caching, builder, serialization, and log masking.
…uilder copy constructor - Guard actorTokenSupplier assignment with if (this.credentialSource == null) in Builder copy constructor. - Add getIdentityPoolActorTokenSupplier getter for test assertions. - Add createScoped tests for both file-sourced and supplier-sourced credentials with actor tokens.
…s and FileIdentityPoolTokenSupplier - Add builder_actorTokenTypeWithoutSupplier_throws testing missing supplier validation. - Add builder_fileWithCertificateConfig_initializesMtlsTransport testing mTLS initialization for composite file + cert sources. - Add toBuilder_preservesConfiguration testing builder reconstruction. - Add parseToken_textFormat_succeeds and parseToken_jsonFormat_missingFieldName_throws testing static token parsing methods.
Summary of changes: - Comment 1: Fix Javadoc referencing package-private API - Comment 2: Replace instanceof with isMtlsConfigured() check - Comment 3: Per-cycle cert pinning with KeyStore snapshot, 401 retry - Comment 4: Atomic subject+actor token read via readTokens() - Comment 5: Make setActorTokenSupplier/Type package-private - Comment 6: Add Javadocs to builder setter methods - Comment 7: Fix Builder copy constructor (always copy actorTokenType) - Comment 8: Annotate no-arg MtlsHttpTransportFactory with @internalapi - Comment 9: Revert class rename to FileIdentityPoolSubjectTokenSupplier - Comment 10: Remove CachedFile/volatile caching mechanism - Comment 11: Add actorTokenFieldName validation - Comment 12: Integration tests noted for follow-up PR Added overload exchangeExternalCredentialForAccessToken(request, factory) for per-cycle transport factory threading. Added 12 new unit tests covering readTokens(), validation, and mTLS. All 982 existing + new tests pass.
- Fix copyright year (2024 -> 2026) in FileIdentityPoolSubjectTokenSupplier - Add Javadoc explaining class name retained for serialization compatibility - Add hasKeyStore() to MtlsHttpTransportFactory for watertight mTLS validation - Update isMtlsConfigured() to verify KeyStore is non-null via hasKeyStore() - Update no-arg constructor Javadoc to explain serialization requirement - Add comment to Builder copy constructor explaining supplier reconstruction - Add 3 unit tests for hasKeyStore() and no-arg factory validation
- Set explicit static serialVersionUID in FileIdentityPoolSubjectTokenSupplier matching pre-PR synthetic SUID (7152208690659890358L). - Revert MtlsHttpTransportFactory.create() return type to NetHttpTransport for binary compatibility. - Update MtlsHttpTransportFactory.hasKeyStore() to verify mtlsKeyStore is non-null and size > 0. - Make IdentityPoolCredentials.x509Provider non-final transient and implement custom readObject to restore x509Provider and transportFactory on deserialization. - Add comprehensive unit tests for MtlsHttpTransportFactory, backward-compatible deserialization, readObject restoration, and fromStream production paths.
…freshAccessToken_pinsTransportForStsExchange
…nfigured - Add validation in IdentityPoolCredentials constructor to reject plain public googleapis.com endpoints when actor tokens are configured - Allow mTLS (.mtls.), Private Service Connect (.p.), and custom domain endpoints - Add comprehensive unit tests covering plain public, mTLS, PSC, and custom domain endpoints
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request introduces support for actor token extraction and certificate-bound OAuth 2.0 token exchanges (mTLS) within IdentityPoolCredentials. Key updates include implementing an atomic reader for both subject and actor tokens from JSON files, adding 401 retry logic with certificate reloading, and enhancing ImpersonatedCredentials to propagate custom transport factories. The feedback highlights two important issues: first, MtlsHttpTransportFactory must implement Serializable and mark its non-serializable KeyStore field as transient to avoid serialization failures; second, the raw file paths concatenated into JSON strings in MtlsPipelineLocalTest must escape backslashes to prevent JSON parsing errors on Windows systems.
Comment on lines
+56
to
+57
| public class MtlsHttpTransportFactory implements HttpTransportFactory { | ||
| private final KeyStore mtlsKeyStore; | ||
| @Nullable private final KeyStore mtlsKeyStore; |
Contributor
There was a problem hiding this comment.
Since MtlsHttpTransportFactory is stored in the serializable transportFactory field of IdentityPoolCredentials and is expected to be serialized, it must implement java.io.Serializable. Additionally, java.security.KeyStore is not serializable, so the mtlsKeyStore field must be marked as transient to avoid NotSerializableException during serialization.
Suggested change
| public class MtlsHttpTransportFactory implements HttpTransportFactory { | |
| private final KeyStore mtlsKeyStore; | |
| @Nullable private final KeyStore mtlsKeyStore; | |
| public class MtlsHttpTransportFactory implements HttpTransportFactory, java.io.Serializable { | |
| private static final long serialVersionUID = 1L; | |
| @Nullable private transient final KeyStore mtlsKeyStore; |
| + "/v1/token\",\n" | ||
| + " \"credential_source\": {\n" | ||
| + " \"file\": \"" | ||
| + tokenFile.toString() |
Contributor
There was a problem hiding this comment.
On Windows systems, tokenFile.toString() (and other file paths like certConfigFile, dynamicCertFile, dynamicKeyFile) will contain backslashes (\). When concatenated directly into a JSON string, these backslashes act as unescaped control characters, resulting in invalid JSON and causing parsing failures. To ensure cross-platform compatibility, escape the backslashes by using .replace("\\", "\\\\") or construct the JSON programmatically using GenericJson.
Suggested change
| + tokenFile.toString() | |
| + tokenFile.toString().replace("\\", "\\\\") |
…ration tests - Add MtlsPipelineLocalTest providing hermetic in-process socket tests over JDK HttpsServer with client certificate authentication (peer cert verification, 401 retry with cert rotation, concurrent refreshes, atomic token read, IAM impersonation mTLS transport pinning, and 401 retry with fresh cert). - Add ITWorkloadIdentityFederationTest extensions for certificate-bound workload + actor token JSON config and programmatic mTLS token suppliers covering both direct STS and Service Account Impersonation. - Fix OAuthException to safely handle null HTTP error response content.
macastelaz
force-pushed
the
cert-bound-oauth-integration-tests
branch
from
September 1, 2026 02:09
64b221e to
fe6d3ca
Compare
…port pinning and 401 recovery - IdentityPoolCredentials: - Isolate X509Provider.getKeyStore() inside its own try-catch during 401 retry to prevent token exchange errors from being miswrapped as certificate reload failures. - Preserve custom HttpTransportFactory instances during deserialization in readObject(). - Add @VisibleForTesting createMtlsTransportFactory(KeyStore) hook for testability without altering public API. - Secure validateMtlsEndpoint with URI parsing and strict .googleapis.com domain matching. - Clean up public exception messages to remove references to internal @internalapi classes. - Document intentional allowRetry=false contract in refreshAccessToken(HttpTransportFactory). - ImpersonatedCredentials: - Add fallback guard in refreshAccessToken(HttpTransportFactory) to delegate to sourceCredentials.refreshAccessToken() when default/null transport is supplied, preserving source transport settings. - Ensure null transportFactory defaults to configured or default transport factory to prevent NullPointerException. - Document transport pinning contract and refer users to setServiceAccountImpersonationUrl. - FileIdentityPoolSubjectTokenSupplier: - Throw explicit IOException when credential file is empty. - Wrap JsonObjectParser.parseAndClose in parseToken to convert unchecked runtime exceptions into IOExceptions. - MtlsHttpTransportFactory: - Throw IllegalStateException in create() if mtlsKeyStore is null to prevent unauthenticated plain TLS downgrade. - Document reflection usage for OAuth2Credentials.newInstance(className). - Test Suite: - Enhance IdentityPoolCredentialsTest with same-instance certificate rotation, endpoint validation tests, and custom transport serialization verification. - Intercept KeyStore assertions in Section E tests using createMtlsTransportFactory hook. - Add end-to-end STS 401 HTTP transport retry test. - Add MockExternalAccountCredentialsTransport STS status code queue for 401 simulation. - Add FileIdentityPoolSubjectTokenSupplierTest edge case tests for null and non-string JSON fields. - Add ImpersonatedCredentialsTest unit test verifying null transport factory fallback. - Add @AfterEach tearDown in Slf4jUtilsLogbackTest to restore global logging environment provider.
…ubjectTokenSupplierTest
…token suppliers
…nd add @nullable to getActorTokenType
…ng and add serialVersionUID to OAuthException
…integration tests
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This PR adds comprehensive integration tests for Certificate-Bound OAuth 2.0 (mTLS), Actor Tokens, and Service Account Impersonation transport pinning across both hermetic in-process socket tests and live GCP Workload Identity Federation suites.
Summary of Changes:
MtlsPipelineLocalTest.java):HttpsServeronlocalhostrequiring client authentication (setNeedClientAuth(true)).testMtlsPipeline_verifiesPeerCertAndPayload: Verifies server extracts client peer certificate fromSSLSessionand parses token exchange body (grant_type,subject_token,actor_token,actor_token_type).testMtlsPipeline_withImpersonation_usesSameCertForStsAndIam: Verifies both STS and IAM calls execute on the exact same pinned mTLS transport and verifies the intermediateAuthorization: Bearer <token>header.testMtlsPipeline_withImpersonation_401OnIam_retriesWithFreshCert: Verifies full-cycle recovery when IAM returns 401 (reloads fresh KeyStore from disk, re-exchanges at STS with new cert, and calls IAM with new token + cert).testMtlsPipeline_concurrentRefreshes: Multi-threaded refresh verifying thread-isolated transport snapshots.testMtlsPipeline_atomicTokenRead: Verifies single-pass disk I/O for subject + actor tokens.ITWorkloadIdentityFederationTest.java):identityPoolCredentials_withCertificateBoundWorkloadAndActorToken: E2E verification of JSON config with certificate config and actor tokens.identityPoolCredentials_withProgrammaticMtlsAndActorToken: E2E verification of programmatic builder configuration.OAuthException.java:Verification:
mvn com.spotify.fmt:fmt-maven-plugin:2.25:check).Stacked on feat(oauth2): implement IAM impersonation mTLS transport pinning and 401 recovery #14212
Source-Link: b/542238030