From 4f3b1e67979badd669212da403b46c25e1e8ca2a Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 17 Oct 2025 15:31:45 +0000 Subject: [PATCH 1/5] fixed case for non-refreshable tokens --- .../sdk/core/oauth/ExternalBrowserCredentialsProvider.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java index fe89b0a7f..fc9556f12 100644 --- a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java +++ b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java @@ -67,7 +67,7 @@ public OAuthHeaderFactory configure(DatabricksConfig config) { // First try to use the cached token if available (will return null if disabled) Token cachedToken = tokenCache.load(); - if (cachedToken != null && cachedToken.getRefreshToken() != null) { + if (cachedToken != null) { LOGGER.debug("Found cached token for {}:{}", config.getHost(), clientId); try { @@ -84,6 +84,7 @@ public OAuthHeaderFactory configure(DatabricksConfig config) { CachedTokenSource cachedTokenSource = new CachedTokenSource.Builder(tokenSource) + .setToken(cachedToken) .setAsyncDisabled(config.getDisableAsyncTokenRefresh()) .build(); LOGGER.debug("Using cached token, will immediately refresh"); From f4ef85cb036e3788efd434db4bb6cef31e2d6ec0 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Fri, 17 Oct 2025 15:58:42 +0000 Subject: [PATCH 2/5] fixed test --- ...xternalBrowserCredentialsProviderTest.java | 49 ++++++------------- 1 file changed, 16 insertions(+), 33 deletions(-) diff --git a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java index f920f38d6..a04f2c611 100644 --- a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java +++ b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java @@ -247,16 +247,10 @@ void sessionCredentials() throws IOException { @Test void cacheWithValidTokenTest() throws IOException { - // Create mock HTTP client for token refresh + // Create mock HTTP client (shouldn't be called for valid token) HttpClient mockHttpClient = Mockito.mock(HttpClient.class); - String refreshResponse = - "{\"access_token\": \"refreshed_access_token\", \"token_type\": \"Bearer\", \"expires_in\": \"3600\", \"refresh_token\": \"new_refresh_token\"}"; - URL url = new URL("https://test.databricks.com/"); - Mockito.doAnswer(invocation -> new Response(refreshResponse, url)) - .when(mockHttpClient) - .execute(any(Request.class)); - // Create an valid token with valid refresh token + // Create a valid token with valid refresh token (expires in 1 hour - FRESH state) Instant futureTime = Instant.now().plusSeconds(3600); Token validToken = new Token("valid_access_token", "Bearer", "valid_refresh_token", futureTime); @@ -272,14 +266,14 @@ void cacheWithValidTokenTest() throws IOException { .setClientId("test-client-id") .setHttpClient(mockHttpClient); - // We need to provide OIDC endpoints for token refresh + // We need to provide OIDC endpoints OpenIDConnectEndpoints endpoints = new OpenIDConnectEndpoints( "https://test.databricks.com/token", "https://test.databricks.com/authorize"); - // Create our provider with the mock token cache and mock the browser auth method + // Create our provider with the mock token cache ExternalBrowserCredentialsProvider provider = - Mockito.spy(new ExternalBrowserCredentialsProvider(mockTokenCache)); + new ExternalBrowserCredentialsProvider(mockTokenCache); // Spy on the config to inject the endpoints DatabricksConfig spyConfig = Mockito.spy(config); @@ -287,18 +281,22 @@ void cacheWithValidTokenTest() throws IOException { // Configure provider HeaderFactory headerFactory = provider.configure(spyConfig); + assertNotNull(headerFactory, "HeaderFactory should be created"); - // Verify headers contain the refreshed token even though the cached token is valid + // Verify headers contain the CACHED valid token (no refresh needed!) Map headers = headerFactory.headers(); - assertEquals("Bearer refreshed_access_token", headers.get("Authorization")); + assertEquals( + "Bearer valid_access_token", + headers.get("Authorization"), + "Should use cached valid token without refreshing"); // Verify token was loaded from cache Mockito.verify(mockTokenCache, Mockito.times(1)).load(); - // Verify HTTP call was made to refresh the token - Mockito.verify(mockHttpClient, Mockito.times(1)).execute(any(Request.class)); + // Verify NO HTTP call was made (token is still valid, no refresh needed) + Mockito.verify(mockHttpClient, Mockito.never()).execute(any(Request.class)); - // Verify performBrowserAuth was NOT called since refresh succeeded + // Verify performBrowserAuth was NOT called since cached token is valid Mockito.verify(provider, Mockito.never()) .performBrowserAuth( any(DatabricksConfig.class), @@ -306,23 +304,8 @@ void cacheWithValidTokenTest() throws IOException { any(String.class), any(TokenCache.class)); - // Verify token was saved back to cache - Mockito.verify(mockTokenCache, Mockito.times(1)).save(any(Token.class)); - - // Capture the token that was saved to cache to verify it's the refreshed token - ArgumentCaptor tokenCaptor = ArgumentCaptor.forClass(Token.class); - Mockito.verify(mockTokenCache).save(tokenCaptor.capture()); - Token savedToken = tokenCaptor.getValue(); - - // Verify the saved token contains the refreshed values from the HTTP response - assertEquals( - "refreshed_access_token", - savedToken.getAccessToken(), - "Should save refreshed access token to cache"); - assertEquals( - "new_refresh_token", - savedToken.getRefreshToken(), - "Should save new refresh token to cache"); + // Verify token was NOT saved back to cache (we're using the cached one as-is) + Mockito.verify(mockTokenCache, Mockito.never()).save(any(Token.class)); } @Test From 1e064bbd2abc5166a715fe7b8eb2457ed76de154 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Mon, 20 Oct 2025 12:00:43 +0000 Subject: [PATCH 3/5] fixed failing existing test --- .../oauth/ExternalBrowserCredentialsProvider.java | 2 +- .../oauth/ExternalBrowserCredentialsProviderTest.java | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java index fc9556f12..b2625f82d 100644 --- a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java +++ b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java @@ -87,7 +87,7 @@ public OAuthHeaderFactory configure(DatabricksConfig config) { .setToken(cachedToken) .setAsyncDisabled(config.getDisableAsyncTokenRefresh()) .build(); - LOGGER.debug("Using cached token, will immediately refresh"); + LOGGER.debug("Using cached token, will refresh if necessary"); cachedTokenSource.getToken(); return OAuthHeaderFactory.fromTokenSource(cachedTokenSource); } catch (Exception e) { diff --git a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java index a04f2c611..716c7694a 100644 --- a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java +++ b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java @@ -269,11 +269,12 @@ void cacheWithValidTokenTest() throws IOException { // We need to provide OIDC endpoints OpenIDConnectEndpoints endpoints = new OpenIDConnectEndpoints( - "https://test.databricks.com/token", "https://test.databricks.com/authorize"); + "https://test.databricks.com/oidc/v1/token", + "https://test.databricks.com/oidc/v1/authorize"); // Create our provider with the mock token cache ExternalBrowserCredentialsProvider provider = - new ExternalBrowserCredentialsProvider(mockTokenCache); + Mockito.spy(new ExternalBrowserCredentialsProvider(mockTokenCache)); // Spy on the config to inject the endpoints DatabricksConfig spyConfig = Mockito.spy(config); @@ -339,7 +340,8 @@ void cacheWithInvalidAccessTokenValidRefreshTest() throws IOException { // We need to provide OIDC endpoints for token refresh OpenIDConnectEndpoints endpoints = new OpenIDConnectEndpoints( - "https://test.databricks.com/token", "https://test.databricks.com/authorize"); + "https://test.databricks.com/oidc/v1/token", + "https://test.databricks.com/oidc/v1/authorize"); // Create our provider with the mock token cache ExternalBrowserCredentialsProvider provider = @@ -438,7 +440,8 @@ void cacheWithInvalidAccessTokenRefreshFailingTest() throws IOException { // We need to provide OIDC endpoints for token refresh attempt OpenIDConnectEndpoints endpoints = new OpenIDConnectEndpoints( - "https://test.databricks.com/token", "https://test.databricks.com/authorize"); + "https://test.databricks.com/oidc/v1/token", + "https://test.databricks.com/oidc/v1/authorize"); // Create our provider and mock the browser auth method ExternalBrowserCredentialsProvider provider = From 4fbff64012939701e248e2fb7a278eef6e191290 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Mon, 20 Oct 2025 12:40:19 +0000 Subject: [PATCH 4/5] added changelog and a test --- NEXT_CHANGELOG.md | 2 + ...xternalBrowserCredentialsProviderTest.java | 86 ++++++++++++++++--- 2 files changed, 75 insertions(+), 13 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 5cede78d0..4cb9399c7 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -8,6 +8,8 @@ refresh tokens by default (by adding the `offline_access` scope) in OAuth exchanges. This option does not remove the scope from the user provided scopes if present. +* Fix 'external-browser' auth type to use the token if it is valid instead of forceful refresh everytime. + ### Bug Fixes ### Documentation diff --git a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java index 716c7694a..dbd1cba9f 100644 --- a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java +++ b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java @@ -246,19 +246,19 @@ void sessionCredentials() throws IOException { // Token caching tests @Test - void cacheWithValidTokenTest() throws IOException { - // Create mock HTTP client (shouldn't be called for valid token) + void cacheWithValidRefreshableTokenTest() throws IOException { + // Create mock HTTP client (shouldn't be called for valid token). HttpClient mockHttpClient = Mockito.mock(HttpClient.class); - // Create a valid token with valid refresh token (expires in 1 hour - FRESH state) + // Create a valid token with valid refresh token (expires in 1 hour - FRESH state). Instant futureTime = Instant.now().plusSeconds(3600); Token validToken = new Token("valid_access_token", "Bearer", "valid_refresh_token", futureTime); - // Create mock token cache that returns the valid token + // Create mock token cache that returns the valid token. TokenCache mockTokenCache = Mockito.mock(TokenCache.class); Mockito.doReturn(validToken).when(mockTokenCache).load(); - // Create config with HTTP client and mock token cache + // Create config with HTTP client and mock token cache. DatabricksConfig config = new DatabricksConfig() .setAuthType("external-browser") @@ -266,25 +266,25 @@ void cacheWithValidTokenTest() throws IOException { .setClientId("test-client-id") .setHttpClient(mockHttpClient); - // We need to provide OIDC endpoints + // We need to provide OIDC endpoints. OpenIDConnectEndpoints endpoints = new OpenIDConnectEndpoints( "https://test.databricks.com/oidc/v1/token", "https://test.databricks.com/oidc/v1/authorize"); - // Create our provider with the mock token cache + // Create our provider with the mock token cache. ExternalBrowserCredentialsProvider provider = Mockito.spy(new ExternalBrowserCredentialsProvider(mockTokenCache)); - // Spy on the config to inject the endpoints + // Spy on the config to inject the endpoints. DatabricksConfig spyConfig = Mockito.spy(config); Mockito.doReturn(endpoints).when(spyConfig).getOidcEndpoints(); - // Configure provider + // Configure provider. HeaderFactory headerFactory = provider.configure(spyConfig); assertNotNull(headerFactory, "HeaderFactory should be created"); - // Verify headers contain the CACHED valid token (no refresh needed!) + // Verify headers contain the CACHED valid token (no refresh needed!). Map headers = headerFactory.headers(); assertEquals( "Bearer valid_access_token", @@ -294,10 +294,10 @@ void cacheWithValidTokenTest() throws IOException { // Verify token was loaded from cache Mockito.verify(mockTokenCache, Mockito.times(1)).load(); - // Verify NO HTTP call was made (token is still valid, no refresh needed) + // Verify NO HTTP call was made (token is still valid, no refresh needed). Mockito.verify(mockHttpClient, Mockito.never()).execute(any(Request.class)); - // Verify performBrowserAuth was NOT called since cached token is valid + // Verify performBrowserAuth was NOT called since cached token is valid. Mockito.verify(provider, Mockito.never()) .performBrowserAuth( any(DatabricksConfig.class), @@ -305,7 +305,67 @@ void cacheWithValidTokenTest() throws IOException { any(String.class), any(TokenCache.class)); - // Verify token was NOT saved back to cache (we're using the cached one as-is) + // Verify token was NOT saved back to cache (we're using the cached one as-is). + Mockito.verify(mockTokenCache, Mockito.never()).save(any(Token.class)); + } + + @Test + void cacheWithValidNonRefreshableTokenTest() throws IOException { + // Create mock HTTP client (shouldn't be called for valid token). + HttpClient mockHttpClient = Mockito.mock(HttpClient.class); + + // Create a valid token WITHOUT refresh token (expires in 1 hour - FRESH state). + Instant futureTime = Instant.now().plusSeconds(3600); + Token validTokenNoRefresh = new Token("valid_access_token", "Bearer", null, futureTime); + + // Create mock token cache that returns the valid token. + TokenCache mockTokenCache = Mockito.mock(TokenCache.class); + Mockito.doReturn(validTokenNoRefresh).when(mockTokenCache).load(); + + // Create config with HTTP client and mock token cache. + DatabricksConfig config = + new DatabricksConfig() + .setAuthType("external-browser") + .setHost("https://test.databricks.com") + .setClientId("test-client-id") + .setHttpClient(mockHttpClient); + + // We need to provide OIDC endpoints. + OpenIDConnectEndpoints endpoints = + new OpenIDConnectEndpoints( + "https://test.databricks.com/oidc/v1/token", + "https://test.databricks.com/oidc/v1/authorize"); + + // Create our provider with the mock token cache. + ExternalBrowserCredentialsProvider provider = + Mockito.spy(new ExternalBrowserCredentialsProvider(mockTokenCache)); + + // Spy on the config to inject the endpoints. + DatabricksConfig spyConfig = Mockito.spy(config); + Mockito.doReturn(endpoints).when(spyConfig).getOidcEndpoints(); + + // Configure provider. + HeaderFactory headerFactory = provider.configure(spyConfig); + assertNotNull(headerFactory, "HeaderFactory should be created"); + + // Verify headers contain the cached token (NOT browser auth!). + Map headers = headerFactory.headers(); + assertEquals( + "Bearer valid_access_token", + headers.get("Authorization"), + "Should use cached valid token even without refresh token"); + + // Verify token was loaded from cache. + Mockito.verify(mockTokenCache, Mockito.times(1)).load(); + + // Verify NO HTTP call was made (token is still valid, no refresh needed). + Mockito.verify(mockHttpClient, Mockito.never()).execute(any(Request.class)); + + // Verify performBrowserAuth was NOT called. + Mockito.verify(provider, Mockito.never()) + .performBrowserAuth(any(DatabricksConfig.class), any(), any(), any(TokenCache.class)); + + // Verify no token was saved (we're using the cached one as-is). Mockito.verify(mockTokenCache, Mockito.never()).save(any(Token.class)); } From 3418f402039e8c82c70f7c32dc5d18f96d0a9397 Mon Sep 17 00:00:00 2001 From: Divyansh Vijayvergia Date: Wed, 22 Oct 2025 09:42:56 +0000 Subject: [PATCH 5/5] removed fix from NEXTCHANGELOG --- NEXT_CHANGELOG.md | 2 -- 1 file changed, 2 deletions(-) diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index 4cb9399c7..5cede78d0 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -8,8 +8,6 @@ refresh tokens by default (by adding the `offline_access` scope) in OAuth exchanges. This option does not remove the scope from the user provided scopes if present. -* Fix 'external-browser' auth type to use the token if it is valid instead of forceful refresh everytime. - ### Bug Fixes ### Documentation