Skip to content

Commit 728d654

Browse files
committed
Address review comments for cert-bound OAuth Part 2
- 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.
1 parent 48ee795 commit 728d654

7 files changed

Lines changed: 653 additions & 185 deletions

File tree

‎google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsHttpTransportFactory.java‎

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,12 @@
3131

3232
package com.google.auth.mtls;
3333

34-
import com.google.api.client.http.HttpTransport;
3534
import com.google.api.client.http.javanet.NetHttpTransport;
3635
import com.google.api.core.InternalApi;
3736
import com.google.auth.http.HttpTransportFactory;
3837
import java.security.GeneralSecurityException;
3938
import java.security.KeyStore;
39+
import java.security.KeyStoreException;
4040
import java.util.Objects;
4141
import org.jspecify.annotations.NullMarked;
4242
import org.jspecify.annotations.Nullable;
@@ -78,14 +78,21 @@ public MtlsHttpTransportFactory(KeyStore mtlsKeyStore) {
7878
/**
7979
* Returns whether this factory was constructed with a non-null {@link KeyStore} containing client
8080
* certificates for mTLS. A factory created via the no-arg constructor (e.g. during
81-
* deserialization) will return {@code false}.
81+
* deserialization) or with an empty KeyStore will return {@code false}.
8282
*/
8383
public boolean hasKeyStore() {
84-
return this.mtlsKeyStore != null;
84+
if (this.mtlsKeyStore == null) {
85+
return false;
86+
}
87+
try {
88+
return this.mtlsKeyStore.size() > 0;
89+
} catch (KeyStoreException e) {
90+
return false;
91+
}
8592
}
8693

8794
@Override
88-
public HttpTransport create() {
95+
public NetHttpTransport create() {
8996
try {
9097
// Build the mTLS transport using the provided KeyStore.
9198
return new NetHttpTransport.Builder().trustCertificates(null, mtlsKeyStore, "").build();

‎google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/FileIdentityPoolSubjectTokenSupplier.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -61,7 +61,7 @@
6161
class FileIdentityPoolSubjectTokenSupplier
6262
implements IdentityPoolSubjectTokenSupplier, IdentityPoolActorTokenSupplier {
6363

64-
private static final long serialVersionUID = 2475549052347431992L;
64+
private static final long serialVersionUID = 7152208690659890358L;
6565

6666
private final IdentityPoolCredentialSource credentialSource;
6767

‎google-auth-library-java/oauth2_http/java/com/google/auth/oauth2/IdentityPoolCredentials.java‎

Lines changed: 32 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@
3939
import com.google.common.annotations.VisibleForTesting;
4040
import com.google.errorprone.annotations.CanIgnoreReturnValue;
4141
import java.io.IOException;
42+
import java.io.ObjectInputStream;
4243
import java.security.KeyStore;
4344
import java.util.ArrayList;
4445
import java.util.Collection;
@@ -66,9 +67,9 @@ public class IdentityPoolCredentials extends ExternalAccountCredentials {
6667
private final IdentityPoolSubjectTokenSupplier subjectTokenSupplier;
6768
@Nullable private final IdentityPoolActorTokenSupplier actorTokenSupplier;
6869
@Nullable private final String actorTokenType;
69-
// Transient: not serialized. After deserialization, per-cycle cert pinning and 401 retry
70-
// are disabled; the credential falls back to the class-level transportFactory for mTLS.
71-
@Nullable private final transient X509Provider x509Provider;
70+
// Transient: not serialized directly. Reconstructed in readObject() from the credentialSource
71+
// certificate config so deserialized credentials remain usable for mTLS and refresh.
72+
@Nullable private transient X509Provider x509Provider;
7273
private final ExternalAccountSupplierContext supplierContext;
7374
private final String metricsHeaderValue;
7475

@@ -105,6 +106,7 @@ public class IdentityPoolCredentials extends ExternalAccountCredentials {
105106
if (credentialSource.getCertificateConfig() != null) {
106107
try {
107108
X509Provider x509Provider = getX509Provider(builder, credentialSource);
109+
this.x509Provider = x509Provider;
108110
KeyStore mtlsKeyStore = x509Provider.getKeyStore();
109111
this.transportFactory = new MtlsHttpTransportFactory(mtlsKeyStore);
110112
} catch (Exception e) {
@@ -270,6 +272,11 @@ HttpTransportFactory getTransportFactory() {
270272
return this.transportFactory;
271273
}
272274

275+
@VisibleForTesting
276+
@Nullable X509Provider getX509Provider() {
277+
return this.x509Provider;
278+
}
279+
273280
/** Clones the IdentityPoolCredentials with the specified scopes. */
274281
@Override
275282
public IdentityPoolCredentials createScoped(Collection<String> newScopes) {
@@ -293,6 +300,7 @@ private IdentityPoolSubjectTokenSupplier createCertificateSubjectTokenSupplier(
293300
Builder builder, IdentityPoolCredentialSource credentialSource) throws IOException {
294301
// Configure the mTLS transport with the x509 keystore.
295302
X509Provider x509Provider = getX509Provider(builder, credentialSource);
303+
this.x509Provider = x509Provider;
296304
KeyStore mtlsKeyStore = x509Provider.getKeyStore();
297305
this.transportFactory = new MtlsHttpTransportFactory(mtlsKeyStore);
298306

@@ -304,6 +312,27 @@ private IdentityPoolSubjectTokenSupplier createCertificateSubjectTokenSupplier(
304312
return new CertificateIdentityPoolSubjectTokenSupplier(credentialSource);
305313
}
306314

315+
@SuppressWarnings("unused")
316+
private void readObject(ObjectInputStream input) throws IOException, ClassNotFoundException {
317+
input.defaultReadObject();
318+
IdentityPoolCredentialSource credentialSource =
319+
(IdentityPoolCredentialSource) getCredentialSource();
320+
if (credentialSource != null
321+
&& (credentialSource.getCertificateConfig() != null
322+
|| credentialSource.credentialSourceType
323+
== IdentityPoolCredentialSourceType.CERTIFICATE)) {
324+
String explicitCertConfigPath = getExplicitCertConfigPath(credentialSource);
325+
this.x509Provider =
326+
new X509Provider(getEnvironmentProvider(), getPropertyProvider(), explicitCertConfigPath);
327+
try {
328+
KeyStore mtlsKeyStore = this.x509Provider.getKeyStore();
329+
this.transportFactory = new MtlsHttpTransportFactory(mtlsKeyStore);
330+
} catch (Exception e) {
331+
// Cert loading failure will be handled on refreshAccessToken()
332+
}
333+
}
334+
}
335+
307336
private X509Provider getX509Provider(
308337
Builder builder, IdentityPoolCredentialSource credentialSource) {
309338
// Use the provided X509Provider if available, otherwise initialize a default one.
Lines changed: 106 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,106 @@
1+
/*
2+
* Copyright 2026 Google LLC
3+
*
4+
* Redistribution and use in source and binary forms, with or without
5+
* modification, are permitted provided that the following conditions are
6+
* met:
7+
*
8+
* * Redistributions of source code must retain the above copyright
9+
* notice, this list of conditions and the following disclaimer.
10+
* * Redistributions in binary form must reproduce the above
11+
* copyright notice, this list of conditions and the following disclaimer
12+
* in the documentation and/or other materials provided with the
13+
* distribution.
14+
*
15+
* * Neither the name of Google LLC nor the names of its
16+
* contributors may be used to endorse or promote products derived from
17+
* this software without specific prior written permission.
18+
*
19+
* THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
20+
* "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
21+
* LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
22+
* A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
23+
* OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
24+
* SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
25+
* LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
26+
* DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
27+
* THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
28+
* (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
29+
* OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
30+
*/
31+
32+
package com.google.auth.mtls;
33+
34+
import static org.junit.jupiter.api.Assertions.assertEquals;
35+
import static org.junit.jupiter.api.Assertions.assertFalse;
36+
import static org.junit.jupiter.api.Assertions.assertNotNull;
37+
import static org.junit.jupiter.api.Assertions.assertThrows;
38+
import static org.junit.jupiter.api.Assertions.assertTrue;
39+
40+
import com.google.api.client.http.javanet.NetHttpTransport;
41+
import java.io.File;
42+
import java.io.FileInputStream;
43+
import java.security.KeyStore;
44+
import java.security.cert.Certificate;
45+
import java.security.cert.CertificateFactory;
46+
import org.junit.jupiter.api.Test;
47+
48+
class MtlsHttpTransportFactoryTest {
49+
50+
private static final String TEST_CERT_PATH = "testresources/mtls/test_cert.pem";
51+
52+
@Test
53+
void hasKeyStore_noArgConstructor_returnsFalse() {
54+
MtlsHttpTransportFactory factory = new MtlsHttpTransportFactory();
55+
assertFalse(factory.hasKeyStore());
56+
}
57+
58+
@Test
59+
void hasKeyStore_emptyKeyStore_returnsFalse() throws Exception {
60+
KeyStore emptyKeyStore = KeyStore.getInstance(KeyStore.getDefaultType());
61+
emptyKeyStore.load(null, null);
62+
assertEquals(0, emptyKeyStore.size());
63+
64+
MtlsHttpTransportFactory factory = new MtlsHttpTransportFactory(emptyKeyStore);
65+
assertFalse(factory.hasKeyStore());
66+
}
67+
68+
@Test
69+
void hasKeyStore_populatedKeyStore_returnsTrue() throws Exception {
70+
KeyStore populatedKeyStore = KeyStore.getInstance(KeyStore.getDefaultType());
71+
populatedKeyStore.load(null, null);
72+
73+
CertificateFactory cf = CertificateFactory.getInstance("X.509");
74+
try (FileInputStream fis = new FileInputStream(new File(TEST_CERT_PATH))) {
75+
Certificate cert = cf.generateCertificate(fis);
76+
populatedKeyStore.setCertificateEntry("test-alias", cert);
77+
}
78+
assertEquals(1, populatedKeyStore.size());
79+
80+
MtlsHttpTransportFactory factory = new MtlsHttpTransportFactory(populatedKeyStore);
81+
assertTrue(factory.hasKeyStore());
82+
}
83+
84+
@Test
85+
void hasKeyStore_uninitializedKeyStore_returnsFalse() throws Exception {
86+
KeyStore uninitializedKeyStore = KeyStore.getInstance(KeyStore.getDefaultType());
87+
// KeyStore.size() on uninitialized KeyStore throws KeyStoreException
88+
MtlsHttpTransportFactory factory = new MtlsHttpTransportFactory(uninitializedKeyStore);
89+
assertFalse(factory.hasKeyStore());
90+
}
91+
92+
@Test
93+
void constructor_nullKeyStore_throwsNullPointerException() {
94+
assertThrows(NullPointerException.class, () -> new MtlsHttpTransportFactory(null));
95+
}
96+
97+
@Test
98+
void create_returnsNetHttpTransport() throws Exception {
99+
KeyStore keyStore = KeyStore.getInstance(KeyStore.getDefaultType());
100+
keyStore.load(null, null);
101+
102+
MtlsHttpTransportFactory factory = new MtlsHttpTransportFactory(keyStore);
103+
NetHttpTransport transport = factory.create();
104+
assertNotNull(transport);
105+
}
106+
}

‎google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/ExternalAccountCredentialsTest.java‎

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -50,9 +50,14 @@
5050
import com.google.auth.oauth2.ExternalAccountCredentials.SubjectTokenTypes;
5151
import com.google.auth.oauth2.ExternalAccountCredentialsTest.TestExternalAccountCredentials.TestCredentialSource;
5252
import java.io.ByteArrayInputStream;
53+
import java.io.File;
54+
import java.io.FileInputStream;
5355
import java.io.IOException;
5456
import java.math.BigDecimal;
5557
import java.net.URI;
58+
import java.security.KeyStore;
59+
import java.security.cert.Certificate;
60+
import java.security.cert.CertificateFactory;
5661
import java.util.Arrays;
5762
import java.util.Date;
5863
import java.util.HashMap;
@@ -68,6 +73,22 @@ class ExternalAccountCredentialsTest extends BaseSerializationTest {
6873
private static final String STS_URL = "https://sts.googleapis.com/v1/token";
6974
private static final String GOOGLE_DEFAULT_UNIVERSE = "googleapis.com";
7075

76+
private static KeyStore createPopulatedKeyStore() {
77+
try {
78+
KeyStore ks = KeyStore.getInstance(KeyStore.getDefaultType());
79+
ks.load(null, null);
80+
CertificateFactory cf = CertificateFactory.getInstance("X.509");
81+
try (FileInputStream fis =
82+
new FileInputStream(new File("testresources/mtls/test_cert.pem"))) {
83+
Certificate cert = cf.generateCertificate(fis);
84+
ks.setCertificateEntry("test-alias", cert);
85+
}
86+
return ks;
87+
} catch (Exception e) {
88+
throw new RuntimeException("Failed to create test KeyStore", e);
89+
}
90+
}
91+
7192
private static final Map<String, Object> FILE_CREDENTIAL_SOURCE_MAP =
7293
new HashMap<String, Object>() {
7394
{
@@ -212,9 +233,7 @@ void fromJson_identityPoolCredentials_withActorTokenType() throws Exception {
212233
formatMap.put("subject_token_field_name", "subject_token");
213234
credentialSource.put("format", formatMap);
214235

215-
java.security.KeyStore ks =
216-
java.security.KeyStore.getInstance(java.security.KeyStore.getDefaultType());
217-
ks.load(null, null);
236+
java.security.KeyStore ks = createPopulatedKeyStore();
218237
com.google.auth.mtls.MtlsHttpTransportFactory mockTransportFactory =
219238
new com.google.auth.mtls.MtlsHttpTransportFactory(ks);
220239

‎google-auth-library-java/oauth2_http/javatests/com/google/auth/oauth2/FileIdentityPoolSubjectTokenSupplierTest.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -260,6 +260,14 @@ void serialization_roundTrip_succeeds(@TempDir Path tempDir) throws Exception {
260260
}
261261
}
262262

263+
@Test
264+
void serialVersionUID_matchesPrePrSyntheticSuid() {
265+
assertEquals(
266+
7152208690659890358L,
267+
java.io.ObjectStreamClass.lookup(FileIdentityPoolSubjectTokenSupplier.class)
268+
.getSerialVersionUID());
269+
}
270+
263271
@Test
264272
void getToken_missingFile_throws(@TempDir Path tempDir) {
265273
Path credentialFile = tempDir.resolve("missing_file.txt");

0 commit comments

Comments
 (0)