Skip to content

Commit c436e7f

Browse files
committed
cache userMtlsPolicy so we read only once other comment addressals.
1 parent af9e51f commit c436e7f

6 files changed

Lines changed: 53 additions & 75 deletions

File tree

google-auth-library-java/oauth2_http/java/com/google/auth/mtls/MtlsUtils.java

Lines changed: 26 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,7 @@ private static File getWellKnownCertificateConfigFile(
159159
* @throws IOException if the configuration file is present but contains missing or malformed
160160
* files
161161
*/
162-
public static boolean canMtlsBeEnabled(
162+
public static boolean canBeEnabled(
163163
EnvironmentProvider envProvider, PropertyProvider propProvider, String certConfigPathOverride)
164164
throws IOException {
165165

@@ -286,34 +286,38 @@ public static HttpTransportFactory prepareTransportFactoryIfMtlsEnabled(
286286

287287
MtlsEndpointUsagePolicy mtlsPolicy = getMtlsEndpointUsagePolicy(envProvider);
288288
try {
289-
boolean canMtls = canMtlsBeEnabled(envProvider, propProvider, certConfigPathOverride);
290-
if (canMtls) {
291-
if (baseTransportFactory instanceof MtlsHttpTransportFactory) {
292-
// A custom MtlsHttpTransportFactory was already pre-configured by the user.
293-
// Keep using it as-is without re-initializing.
294-
return baseTransportFactory;
295-
} else if (baseTransportFactory == OAuth2Utils.HTTP_TRANSPORT_FACTORY) {
296-
// This is the default HttpTransportFactory assigned by credentials.
297-
// Automatically discover and load client certificates to construct an mTLS factory.
298-
X509Provider x509Provider =
299-
new X509Provider(envProvider, propProvider, certConfigPathOverride);
300-
KeyStore mtlsKeyStore = x509Provider.getKeyStore();
301-
return new MtlsHttpTransportFactory(mtlsKeyStore);
302-
} else {
303-
// A user configured non-mTLS HttpTransportFactory was explicitly injected.
304-
// Reject it to avoid bypassing mTLS enforcement or overriding the user's factory.
305-
throw new IOException(
306-
"mTLS is enabled on the system, but a user configured non-mTLS HttpTransportFactory was provided: "
307-
+ baseTransportFactory.getClass().getName());
308-
}
289+
if (!canBeEnabled(envProvider, propProvider, certConfigPathOverride)) {
290+
return baseTransportFactory;
291+
}
292+
293+
if (baseTransportFactory instanceof MtlsHttpTransportFactory) {
294+
// A custom MtlsHttpTransportFactory was already pre-configured by the user.
295+
// Keep using it as-is without re-initializing.
296+
return baseTransportFactory;
309297
}
298+
299+
if (baseTransportFactory == OAuth2Utils.HTTP_TRANSPORT_FACTORY) {
300+
// This is the default HttpTransportFactory assigned by credentials.
301+
// Automatically discover and load client certificates to construct an mTLS factory.
302+
X509Provider x509Provider =
303+
new X509Provider(envProvider, propProvider, certConfigPathOverride);
304+
KeyStore mtlsKeyStore = x509Provider.getKeyStore();
305+
return new MtlsHttpTransportFactory(mtlsKeyStore);
306+
}
307+
308+
// A user configured non-mTLS HttpTransportFactory was explicitly injected.
309+
// Reject it to avoid bypassing mTLS enforcement or overriding the user's factory.
310+
throw new IOException(
311+
"mTLS is enabled on the system, but a user configured non-mTLS HttpTransportFactory was provided: "
312+
+ baseTransportFactory.getClass().getName());
313+
310314
} catch (Exception e) {
311315
if (mtlsPolicy == MtlsEndpointUsagePolicy.ALWAYS) {
312316
throw new IOException(
313317
"mTLS is configured to ALWAYS, but initialization failed: " + e.getMessage(), e);
314318
}
315319
// Graceful fallback to standard transport if mTLS initialization fails under AUTO policy
320+
return baseTransportFactory;
316321
}
317-
return baseTransportFactory;
318322
}
319323
}

google-auth-library-java/oauth2_http/java/com/google/auth/mtls/X509Provider.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -131,6 +131,6 @@ public KeyStore getKeyStore() throws CertificateSourceUnavailableException, IOEx
131131

132132
@Override
133133
public boolean isAvailable() throws IOException {
134-
return MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, certConfigPathOverride);
134+
return MtlsUtils.canBeEnabled(envProvider, propProvider, certConfigPathOverride);
135135
}
136136
}

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

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -399,6 +399,7 @@ void refreshRegionalAccessBoundaryIfExpired(@Nullable URI uri, @Nullable AccessT
399399
}
400400

401401
// Automatically discover certificates or enforce mTLS policy if applicable
402+
// TODO: https://github.com/googleapis/google-cloud-java/issues/13461
402403
transportFactory =
403404
MtlsUtils.prepareTransportFactoryIfMtlsEnabled(
404405
transportFactory, getEnvironmentProvider(), getPropertyProvider(), null);
@@ -451,10 +452,6 @@ public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException
451452
// Sets off an async refresh for request-metadata.
452453
refreshRegionalAccessBoundaryIfExpired(uri, getAccessToken());
453454
} catch (IOException e) {
454-
if (MtlsUtils.getMtlsEndpointUsagePolicy(getEnvironmentProvider())
455-
== MtlsUtils.MtlsEndpointUsagePolicy.ALWAYS) {
456-
throw e;
457-
}
458455
// Ignore failure in async refresh trigger.
459456
}
460457
return metadata;
@@ -485,11 +482,6 @@ public void onSuccess(Map<String, List<String>> metadata) {
485482
try {
486483
refreshRegionalAccessBoundaryIfExpired(uri, getAccessToken());
487484
} catch (IOException e) {
488-
if (MtlsUtils.getMtlsEndpointUsagePolicy(getEnvironmentProvider())
489-
== MtlsUtils.MtlsEndpointUsagePolicy.ALWAYS) {
490-
callback.onFailure(e);
491-
return;
492-
}
493485
// Ignore failure in async refresh trigger.
494486
}
495487
callback.onSuccess(metadata);

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

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,8 @@ final class RegionalAccessBoundary implements Serializable {
6969
static final long TTL_MILLIS = 6 * 60 * 60 * 1000L; // 6 hours
7070
static final long REFRESH_THRESHOLD_MILLIS = 1 * 60 * 60 * 1000L; // 1 hour
7171

72+
private static MtlsUtils.MtlsEndpointUsagePolicy userMtlsPolicy = null;
73+
7274
private final String encodedLocations;
7375
private final List<String> locations;
7476
private final long refreshTime;
@@ -190,10 +192,12 @@ static RegionalAccessBoundary refresh(
190192
throw new IllegalArgumentException("The provided access token is expired.");
191193
}
192194

193-
MtlsUtils.MtlsEndpointUsagePolicy mtlsPolicy =
194-
MtlsUtils.getMtlsEndpointUsagePolicy(SystemEnvironmentProvider.getInstance());
195+
if (userMtlsPolicy == null) {
196+
userMtlsPolicy =
197+
MtlsUtils.getMtlsEndpointUsagePolicy(SystemEnvironmentProvider.getInstance());
198+
}
195199
if (transportFactory instanceof com.google.auth.mtls.MtlsHttpTransportFactory
196-
|| mtlsPolicy == MtlsUtils.MtlsEndpointUsagePolicy.ALWAYS) {
200+
|| userMtlsPolicy == MtlsUtils.MtlsEndpointUsagePolicy.ALWAYS) {
197201
url = url.replace("iamcredentials.googleapis.com", "iamcredentials.mtls.googleapis.com");
198202
}
199203

google-auth-library-java/oauth2_http/javatests/com/google/auth/mtls/MtlsUtilsTest.java

Lines changed: 18 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -244,9 +244,9 @@ public String getProperty(String name, String def) {
244244
assertEquals("APPDATA environment variable is not set on Windows.", exception.getMessage());
245245
}
246246

247-
// If client certificate usage is explicitly disabled, canMtlsBeEnabled should return false.
247+
// If client certificate usage is explicitly disabled, canBeEnabled should return false.
248248
@Test
249-
void canMtlsBeEnabled_allowanceExplicitFalse_returnsFalse() throws IOException {
249+
void canBeEnabled_allowanceExplicitFalse_returnsFalse() throws IOException {
250250
EnvironmentProvider envProvider =
251251
new EnvironmentProvider() {
252252
@Override
@@ -259,13 +259,13 @@ public String getEnv(String name) {
259259
};
260260
PropertyProvider propProvider = (name, def) -> def;
261261

262-
assertFalse(MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
262+
assertFalse(MtlsUtils.canBeEnabled(envProvider, propProvider, null));
263263
}
264264

265265
// If client certificate usage is explicitly enabled and a valid configuration is present,
266-
// canMtlsBeEnabled should return true.
266+
// canBeEnabled should return true.
267267
@Test
268-
void canMtlsBeEnabled_allowanceExplicitTrue_withConfig_returnsTrue() throws IOException {
268+
void canBeEnabled_allowanceExplicitTrue_withConfig_returnsTrue() throws IOException {
269269
EnvironmentProvider envProvider =
270270
new EnvironmentProvider() {
271271
@Override
@@ -281,13 +281,13 @@ public String getEnv(String name) {
281281
};
282282
PropertyProvider propProvider = (name, def) -> def;
283283

284-
assertTrue(MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
284+
assertTrue(MtlsUtils.canBeEnabled(envProvider, propProvider, null));
285285
}
286286

287287
// If client certificate usage is unset but a valid configuration is present, mTLS should be
288288
// enabled by default (returns true).
289289
@Test
290-
void canMtlsBeEnabled_allowanceUnset_withConfig_returnsTrue() throws IOException {
290+
void canBeEnabled_allowanceUnset_withConfig_returnsTrue() throws IOException {
291291
EnvironmentProvider envProvider =
292292
new EnvironmentProvider() {
293293
@Override
@@ -300,13 +300,13 @@ public String getEnv(String name) {
300300
};
301301
PropertyProvider propProvider = (name, def) -> def;
302302

303-
assertTrue(MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
303+
assertTrue(MtlsUtils.canBeEnabled(envProvider, propProvider, null));
304304
}
305305

306306
// If the GOOGLE_API_CERTIFICATE_CONFIG environment variable points to a non-existent file,
307-
// canMtlsBeEnabled should throw an IOException.
307+
// canBeEnabled should throw an IOException.
308308
@Test
309-
void canMtlsBeEnabled_envVarConfigMissingFile_throwsIOException() throws IOException {
309+
void canBeEnabled_envVarConfigMissingFile_throwsIOException() throws IOException {
310310
Path nonExistentConfig = tempDir.resolve("non_existent.json");
311311
EnvironmentProvider envProvider =
312312
new EnvironmentProvider() {
@@ -321,13 +321,13 @@ public String getEnv(String name) {
321321
PropertyProvider propProvider = (name, def) -> def;
322322

323323
assertThrows(
324-
IOException.class, () -> MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
324+
IOException.class, () -> MtlsUtils.canBeEnabled(envProvider, propProvider, null));
325325
}
326326

327-
// If the well-known gcloud certificate configuration file exists, canMtlsBeEnabled should return
327+
// If the well-known gcloud certificate configuration file exists, canBeEnabled should return
328328
// true.
329329
@Test
330-
void canMtlsBeEnabled_wellKnownConfigExists_returnsTrue() throws IOException {
330+
void canBeEnabled_wellKnownConfigExists_returnsTrue() throws IOException {
331331
Path gcloudDir = tempDir.resolve(".config/gcloud");
332332
Files.createDirectories(gcloudDir);
333333
Path configFile = gcloudDir.resolve("certificate_config.json");
@@ -348,11 +348,11 @@ public String getProperty(String name, String def) {
348348
}
349349
};
350350

351-
assertTrue(MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
351+
assertTrue(MtlsUtils.canBeEnabled(envProvider, propProvider, null));
352352
}
353353

354354
@Test
355-
void canMtlsBeEnabled_alwaysPolicy_clientCertDisabled_throwsException() {
355+
void canBeEnabled_alwaysPolicy_clientCertDisabled_throwsException() {
356356
EnvironmentProvider envProvider =
357357
name -> {
358358
if ("GOOGLE_API_USE_CLIENT_CERTIFICATE".equals(name)) {
@@ -367,7 +367,7 @@ void canMtlsBeEnabled_alwaysPolicy_clientCertDisabled_throwsException() {
367367

368368
assertThrows(
369369
CertificateSourceUnavailableException.class,
370-
() -> MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
370+
() -> MtlsUtils.canBeEnabled(envProvider, propProvider, null));
371371
}
372372

373373
@Test
@@ -395,7 +395,7 @@ void getMtlsEndpointUsagePolicy_auto() {
395395
}
396396

397397
@Test
398-
void canMtlsBeEnabled_policyNever_returnsFalse() throws IOException {
398+
void canBeEnabled_policyNever_returnsFalse() throws IOException {
399399
EnvironmentProvider envProvider =
400400
new EnvironmentProvider() {
401401
@Override
@@ -411,7 +411,7 @@ public String getEnv(String name) {
411411
};
412412
PropertyProvider propProvider = (name, def) -> def;
413413

414-
assertFalse(MtlsUtils.canMtlsBeEnabled(envProvider, propProvider, null));
414+
assertFalse(MtlsUtils.canBeEnabled(envProvider, propProvider, null));
415415
}
416416

417417
private String createJsonConfigString(Path certPath, Path keyPath) {

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

Lines changed: 0 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -1220,28 +1220,6 @@ public void regionalAccessBoundary_shouldFailOpenWhenRefreshCannotBeStarted() th
12201220
assertNull(headers.get(X_ALLOWED_LOCATIONS_HEADER_KEY));
12211221
}
12221222

1223-
@Test
1224-
public void regionalAccessBoundary_alwaysPolicy_missingCertConfig_throwsException() {
1225-
TestEnvironmentProvider envProvider = new TestEnvironmentProvider();
1226-
envProvider.setEnv("GOOGLE_API_USE_MTLS_ENDPOINT", "always");
1227-
1228-
GoogleCredentials credentials =
1229-
new TestRegionalCredentials(new AccessToken("some-token", null), envProvider);
1230-
1231-
assertThrows(IOException.class, () -> credentials.getRequestMetadata());
1232-
}
1233-
1234-
@Test
1235-
public void regionalAccessBoundary_alwaysPolicy_userConfiguredNonMtlsFactory_throwsException() {
1236-
TestEnvironmentProvider envProvider = new TestEnvironmentProvider();
1237-
envProvider.setEnv("GOOGLE_API_USE_MTLS_ENDPOINT", "always");
1238-
1239-
GoogleCredentials credentials =
1240-
new TestRegionalCredentials(
1241-
new AccessToken("some-token", null), envProvider, DUMMY_TRANSPORT_FACTORY);
1242-
1243-
assertThrows(IOException.class, () -> credentials.getRequestMetadata());
1244-
}
12451223

12461224
@Test
12471225
public void regionalAccessBoundary_deduplicationOfConcurrentRefreshes()

0 commit comments

Comments
 (0)