Skip to content

Commit ba2d3ca

Browse files
committed
Improves regional access boundary cooldown on task submission failures, adds 3-minute token clock skew buffer, and fixes mTLS fallback logic.
1 parent 13c69af commit ba2d3ca

4 files changed

Lines changed: 125 additions & 14 deletions

File tree

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

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,8 +56,10 @@
5656
import java.util.Collections;
5757
import java.util.HashMap;
5858
import java.util.List;
59+
import java.util.Locale;
5960
import java.util.Map;
6061
import java.util.Objects;
62+
import java.util.concurrent.Executor;
6163
import javax.annotation.Nullable;
6264

6365
/** Base type for credentials for authorizing calls to Google APIs using OAuth2. */
@@ -379,7 +381,7 @@ void refreshRegionalAccessBoundaryIfExpired(@Nullable URI uri, @Nullable AccessT
379381

380382
// Skip refresh for regional endpoints.
381383
if (uri != null && uri.getHost() != null) {
382-
String host = uri.getHost().toLowerCase(java.util.Locale.US);
384+
String host = uri.getHost().toLowerCase(Locale.US);
383385
if (host.endsWith(".rep.googleapis.com") || host.endsWith(".rep.sandbox.googleapis.com")) {
384386
return;
385387
}
@@ -388,7 +390,7 @@ void refreshRegionalAccessBoundaryIfExpired(@Nullable URI uri, @Nullable AccessT
388390
// We need a valid access token for the refresh.
389391
if (token == null
390392
|| (token.getExpirationTimeMillis() != null
391-
&& token.getExpirationTimeMillis() < clock.currentTimeMillis())) {
393+
&& token.getExpirationTimeMillis() <= clock.currentTimeMillis() + 180_000L)) {
392394
return;
393395
}
394396

@@ -466,9 +468,7 @@ public Map<String, List<String>> getRequestMetadata(URI uri) throws IOException
466468
*/
467469
@Override
468470
public void getRequestMetadata(
469-
final URI uri,
470-
final java.util.concurrent.Executor executor,
471-
final RequestMetadataCallback callback) {
471+
final URI uri, final Executor executor, final RequestMetadataCallback callback) {
472472
super.getRequestMetadata(
473473
uri,
474474
executor,
@@ -556,7 +556,7 @@ Map<String, List<String>> addRegionalAccessBoundaryToRequestMetadata(
556556
Preconditions.checkNotNull(requestMetadata);
557557

558558
if (uri != null && uri.getHost() != null) {
559-
String host = uri.getHost().toLowerCase(java.util.Locale.US);
559+
String host = uri.getHost().toLowerCase(Locale.US);
560560
if (host.endsWith(".rep.googleapis.com") || host.endsWith(".rep.sandbox.googleapis.com")) {
561561
return requestMetadata;
562562
}

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -185,8 +185,8 @@ static RegionalAccessBoundary refresh(
185185
throws IOException {
186186
Preconditions.checkNotNull(accessToken, "The provided access token is null.");
187187
if (accessToken.getExpirationTimeMillis() != null
188-
&& accessToken.getExpirationTimeMillis() < clock.currentTimeMillis()) {
189-
throw new IllegalArgumentException("The provided access token is expired.");
188+
&& accessToken.getExpirationTimeMillis() <= clock.currentTimeMillis() + 180_000L) {
189+
throw new IOException("The provided access token is expired or expiring within skew buffer.");
190190
}
191191

192192
HttpRequestFactory requestFactory = transportFactory.create().createRequestFactory();

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

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -36,7 +36,9 @@
3636
import com.google.api.client.util.Clock;
3737
import com.google.api.core.InternalApi;
3838
import com.google.auth.http.HttpTransportFactory;
39+
import com.google.auth.mtls.MtlsUtils;
3940
import com.google.common.annotations.VisibleForTesting;
41+
import java.io.IOException;
4042
import java.util.concurrent.ExecutorService;
4143
import java.util.concurrent.LinkedBlockingQueue;
4244
import java.util.concurrent.ThreadPoolExecutor;
@@ -68,6 +70,9 @@ final class RegionalAccessBoundaryManager {
6870
*/
6971
static final int DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS = 60000;
7072

73+
static final String IAM_ENDPOINT = "iamcredentials.googleapis.com";
74+
static final String MTLS_IAM_ENDPOINT = "iamcredentials.mtls.googleapis.com";
75+
7176
/**
7277
* cachedRAB uses AtomicReference to provide thread-safe, lock-free access to the cached data for
7378
* high-concurrency request threads.
@@ -201,14 +206,13 @@ void triggerAsyncRefresh(
201206
skipRAB.set(true);
202207
return;
203208
}
204-
if (com.google.auth.mtls.MtlsUtils.canBeEnabled(envProvider, propProvider, null)) {
205-
url =
206-
url.replace(
207-
"iamcredentials.googleapis.com", "iamcredentials.mtls.googleapis.com");
208-
}
209209
HttpTransportFactory upgradedTransportFactory =
210-
com.google.auth.mtls.MtlsUtils.prepareTransportFactoryIfMtlsEnabled(
210+
MtlsUtils.prepareTransportFactoryIfMtlsEnabled(
211211
transportFactory, envProvider, propProvider, null);
212+
if (MtlsUtils.canBeEnabled(envProvider, propProvider, null)
213+
&& upgradedTransportFactory != OAuth2Utils.HTTP_TRANSPORT_FACTORY) {
214+
url = url.replace(IAM_ENDPOINT, MTLS_IAM_ENDPOINT);
215+
}
212216
RegionalAccessBoundary newRAB =
213217
RegionalAccessBoundary.refresh(
214218
upgradedTransportFactory, url, accessToken, clock, maxRetryElapsedTimeMillis);
@@ -227,6 +231,8 @@ void triggerAsyncRefresh(
227231
} catch (Exception | Error e) {
228232
// If scheduling fails (e.g., RejectedExecutionException, OutOfMemoryError for threads),
229233
// the task's finally block will never execute. We must release the lock here.
234+
handleRefreshFailure(
235+
new IOException("Failed to submit background refresh task: " + e.getMessage(), e));
230236
log(
231237
LOGGER_PROVIDER,
232238
Level.FINE,

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

Lines changed: 105 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@
3535
import static org.junit.jupiter.api.Assertions.assertEquals;
3636
import static org.junit.jupiter.api.Assertions.assertFalse;
3737
import static org.junit.jupiter.api.Assertions.assertNull;
38+
import static org.junit.jupiter.api.Assertions.assertThrows;
3839
import static org.junit.jupiter.api.Assertions.assertTrue;
3940

4041
import com.google.api.client.testing.http.MockHttpTransport;
@@ -156,6 +157,23 @@ public void testRefreshClosesResponse() throws Exception {
156157
assertTrue(mockResponse.isDisconnected(), "Response should have been disconnected");
157158
}
158159

160+
@Test
161+
public void testRefreshThrowsIOExceptionOnExpiringToken() {
162+
final String url = "https://example.com/rab";
163+
final AccessToken token =
164+
new AccessToken(
165+
"token", new java.util.Date(testClock.currentTimeMillis() + 120_000L)); // 2 minutes, within 3 min skew
166+
167+
MockHttpTransport transport = new MockHttpTransport.Builder().build();
168+
HttpTransportFactory transportFactory = () -> transport;
169+
170+
assertThrows(
171+
IOException.class,
172+
() -> {
173+
RegionalAccessBoundary.refresh(transportFactory, url, token, testClock, 1000);
174+
});
175+
}
176+
159177
@Test
160178
public void testManagerTriggersRefreshInGracePeriod() throws InterruptedException {
161179
final String url =
@@ -410,4 +428,91 @@ public boolean isDisconnected() {
410428
// Verify that MtlsHttpTransportFactory.create() was called to retrieve the mTLS transport
411429
Mockito.verify(mockMtlsFactory, Mockito.times(2)).create();
412430
}
431+
432+
@Test
433+
public void
434+
regionalAccessBoundary_withMtlsEnabledButInitializationFailed_shouldFallbackToNonMtlsEndpoint()
435+
throws IOException, InterruptedException {
436+
437+
MockExternalAccountCredentialsTransport transport =
438+
new MockExternalAccountCredentialsTransport();
439+
440+
// Configure the environment provider to enable mTLS.
441+
// X509Provider will use the invalid certificate config.
442+
TestEnvironmentProvider testEnvProvider = new TestEnvironmentProvider();
443+
testEnvProvider.setEnv("GOOGLE_API_USE_CLIENT_CERTIFICATE", "true");
444+
testEnvProvider.setEnv(
445+
"GOOGLE_API_CERTIFICATE_CONFIG",
446+
new File("testresources/mtls/invalid_certificate_config.json").getAbsolutePath());
447+
448+
final String url =
449+
"https://iamcredentials.googleapis.com/v1/projects/-/serviceAccounts/default:allowedLocations";
450+
final AccessToken token =
451+
new AccessToken(
452+
"token", new java.util.Date(System.currentTimeMillis() + 10 * 3600000L));
453+
454+
RegionalAccessBoundaryProvider provider = () -> url;
455+
456+
// Use default OAuth2Utils.HTTP_TRANSPORT_FACTORY
457+
HttpTransportFactory transportFactory = OAuth2Utils.HTTP_TRANSPORT_FACTORY;
458+
459+
RegionalAccessBoundaryManager manager =
460+
new RegionalAccessBoundaryManager(
461+
testClock,
462+
RegionalAccessBoundaryManager.DEFAULT_MAX_RETRY_ELAPSED_TIME_MILLIS,
463+
com.google.common.util.concurrent.MoreExecutors.newDirectExecutorService());
464+
465+
// Mock static RegionalAccessBoundary.refresh method to intercept the call
466+
try (org.mockito.MockedStatic<RegionalAccessBoundary> rabMock =
467+
org.mockito.Mockito.mockStatic(RegionalAccessBoundary.class)) {
468+
469+
RegionalAccessBoundary mockRab =
470+
new RegionalAccessBoundary(
471+
"fallback-encoded", Arrays.asList("fallback-loc"), testClock.currentTimeMillis(), testClock);
472+
473+
rabMock
474+
.when(
475+
() ->
476+
RegionalAccessBoundary.refresh(
477+
org.mockito.ArgumentMatchers.any(),
478+
org.mockito.ArgumentMatchers.any(),
479+
org.mockito.ArgumentMatchers.any(),
480+
org.mockito.ArgumentMatchers.any(),
481+
org.mockito.ArgumentMatchers.anyInt()))
482+
.thenReturn(mockRab);
483+
484+
// Trigger refresh
485+
manager.triggerAsyncRefresh(
486+
transportFactory,
487+
provider,
488+
token,
489+
testEnvProvider,
490+
SystemPropertyProvider.getInstance());
491+
492+
// Verify it was cached
493+
assertEquals("fallback-encoded", manager.getCachedRAB().getEncodedLocations());
494+
495+
final org.mockito.ArgumentCaptor<String> urlCaptor =
496+
org.mockito.ArgumentCaptor.forClass(String.class);
497+
final org.mockito.ArgumentCaptor<HttpTransportFactory> factoryCaptor =
498+
org.mockito.ArgumentCaptor.forClass(HttpTransportFactory.class);
499+
500+
// Verify static call and capture arguments
501+
rabMock.verify(
502+
() ->
503+
RegionalAccessBoundary.refresh(
504+
factoryCaptor.capture(),
505+
urlCaptor.capture(),
506+
org.mockito.ArgumentMatchers.any(),
507+
org.mockito.ArgumentMatchers.any(),
508+
org.mockito.ArgumentMatchers.anyInt()),
509+
org.mockito.Mockito.times(1));
510+
511+
// Verify the URL passed to refresh did NOT get changed to the mTLS endpoint
512+
assertEquals(url, urlCaptor.getValue());
513+
// Verify that the transport factory used is the default one
514+
assertEquals(OAuth2Utils.HTTP_TRANSPORT_FACTORY, factoryCaptor.getValue());
515+
}
516+
}
413517
}
518+

0 commit comments

Comments
 (0)