Skip to content

Commit 54b94b1

Browse files
authored
Fixed configure method for non-refreshable tokens (#526)
## What changes are proposed in this pull request? We recently added support for disabling auto token refresh (#525). The current code is hardcoded to handle refreshable tokens, and it forcefully refreshes the token on every call of `configure()` This PR proposed using a token (no matter if it is refreshable or not) if it is valid. Once the ExternalBrowserCredentialsProvider is run, it stores the token on disk. If the application is run again, it checks the token and tries to use it. ## How is this tested? Added Unit tests NO_CHANGELOG=true
1 parent 23b4016 commit 54b94b1

File tree

2 files changed

+86
-39
lines changed

2 files changed

+86
-39
lines changed

databricks-sdk-java/src/main/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProvider.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@ public OAuthHeaderFactory configure(DatabricksConfig config) {
6767

6868
// First try to use the cached token if available (will return null if disabled)
6969
Token cachedToken = tokenCache.load();
70-
if (cachedToken != null && cachedToken.getRefreshToken() != null) {
70+
if (cachedToken != null) {
7171
LOGGER.debug("Found cached token for {}:{}", config.getHost(), clientId);
7272

7373
try {
@@ -84,9 +84,10 @@ public OAuthHeaderFactory configure(DatabricksConfig config) {
8484

8585
CachedTokenSource cachedTokenSource =
8686
new CachedTokenSource.Builder(tokenSource)
87+
.setToken(cachedToken)
8788
.setAsyncDisabled(config.getDisableAsyncTokenRefresh())
8889
.build();
89-
LOGGER.debug("Using cached token, will immediately refresh");
90+
LOGGER.debug("Using cached token, will refresh if necessary");
9091
cachedTokenSource.getToken();
9192
return OAuthHeaderFactory.fromTokenSource(cachedTokenSource);
9293
} catch (Exception e) {

databricks-sdk-java/src/test/java/com/databricks/sdk/core/oauth/ExternalBrowserCredentialsProviderTest.java

Lines changed: 83 additions & 37 deletions
Original file line numberDiff line numberDiff line change
@@ -246,83 +246,127 @@ void sessionCredentials() throws IOException {
246246
// Token caching tests
247247

248248
@Test
249-
void cacheWithValidTokenTest() throws IOException {
250-
// Create mock HTTP client for token refresh
249+
void cacheWithValidRefreshableTokenTest() throws IOException {
250+
// Create mock HTTP client (shouldn't be called for valid token).
251251
HttpClient mockHttpClient = Mockito.mock(HttpClient.class);
252-
String refreshResponse =
253-
"{\"access_token\": \"refreshed_access_token\", \"token_type\": \"Bearer\", \"expires_in\": \"3600\", \"refresh_token\": \"new_refresh_token\"}";
254-
URL url = new URL("https://test.databricks.com/");
255-
Mockito.doAnswer(invocation -> new Response(refreshResponse, url))
256-
.when(mockHttpClient)
257-
.execute(any(Request.class));
258252

259-
// Create an valid token with valid refresh token
253+
// Create a valid token with valid refresh token (expires in 1 hour - FRESH state).
260254
Instant futureTime = Instant.now().plusSeconds(3600);
261255
Token validToken = new Token("valid_access_token", "Bearer", "valid_refresh_token", futureTime);
262256

263-
// Create mock token cache that returns the valid token
257+
// Create mock token cache that returns the valid token.
264258
TokenCache mockTokenCache = Mockito.mock(TokenCache.class);
265259
Mockito.doReturn(validToken).when(mockTokenCache).load();
266260

267-
// Create config with HTTP client and mock token cache
261+
// Create config with HTTP client and mock token cache.
268262
DatabricksConfig config =
269263
new DatabricksConfig()
270264
.setAuthType("external-browser")
271265
.setHost("https://test.databricks.com")
272266
.setClientId("test-client-id")
273267
.setHttpClient(mockHttpClient);
274268

275-
// We need to provide OIDC endpoints for token refresh
269+
// We need to provide OIDC endpoints.
276270
OpenIDConnectEndpoints endpoints =
277271
new OpenIDConnectEndpoints(
278-
"https://test.databricks.com/token", "https://test.databricks.com/authorize");
272+
"https://test.databricks.com/oidc/v1/token",
273+
"https://test.databricks.com/oidc/v1/authorize");
279274

280-
// Create our provider with the mock token cache and mock the browser auth method
275+
// Create our provider with the mock token cache.
281276
ExternalBrowserCredentialsProvider provider =
282277
Mockito.spy(new ExternalBrowserCredentialsProvider(mockTokenCache));
283278

284-
// Spy on the config to inject the endpoints
279+
// Spy on the config to inject the endpoints.
285280
DatabricksConfig spyConfig = Mockito.spy(config);
286281
Mockito.doReturn(endpoints).when(spyConfig).getOidcEndpoints();
287282

288-
// Configure provider
283+
// Configure provider.
289284
HeaderFactory headerFactory = provider.configure(spyConfig);
285+
assertNotNull(headerFactory, "HeaderFactory should be created");
290286

291-
// Verify headers contain the refreshed token even though the cached token is valid
287+
// Verify headers contain the CACHED valid token (no refresh needed!).
292288
Map<String, String> headers = headerFactory.headers();
293-
assertEquals("Bearer refreshed_access_token", headers.get("Authorization"));
289+
assertEquals(
290+
"Bearer valid_access_token",
291+
headers.get("Authorization"),
292+
"Should use cached valid token without refreshing");
294293

295294
// Verify token was loaded from cache
296295
Mockito.verify(mockTokenCache, Mockito.times(1)).load();
297296

298-
// Verify HTTP call was made to refresh the token
299-
Mockito.verify(mockHttpClient, Mockito.times(1)).execute(any(Request.class));
297+
// Verify NO HTTP call was made (token is still valid, no refresh needed).
298+
Mockito.verify(mockHttpClient, Mockito.never()).execute(any(Request.class));
300299

301-
// Verify performBrowserAuth was NOT called since refresh succeeded
300+
// Verify performBrowserAuth was NOT called since cached token is valid.
302301
Mockito.verify(provider, Mockito.never())
303302
.performBrowserAuth(
304303
any(DatabricksConfig.class),
305304
any(String.class),
306305
any(String.class),
307306
any(TokenCache.class));
308307

309-
// Verify token was saved back to cache
310-
Mockito.verify(mockTokenCache, Mockito.times(1)).save(any(Token.class));
308+
// Verify token was NOT saved back to cache (we're using the cached one as-is).
309+
Mockito.verify(mockTokenCache, Mockito.never()).save(any(Token.class));
310+
}
311311

312-
// Capture the token that was saved to cache to verify it's the refreshed token
313-
ArgumentCaptor<Token> tokenCaptor = ArgumentCaptor.forClass(Token.class);
314-
Mockito.verify(mockTokenCache).save(tokenCaptor.capture());
315-
Token savedToken = tokenCaptor.getValue();
312+
@Test
313+
void cacheWithValidNonRefreshableTokenTest() throws IOException {
314+
// Create mock HTTP client (shouldn't be called for valid token).
315+
HttpClient mockHttpClient = Mockito.mock(HttpClient.class);
316316

317-
// Verify the saved token contains the refreshed values from the HTTP response
318-
assertEquals(
319-
"refreshed_access_token",
320-
savedToken.getAccessToken(),
321-
"Should save refreshed access token to cache");
317+
// Create a valid token WITHOUT refresh token (expires in 1 hour - FRESH state).
318+
Instant futureTime = Instant.now().plusSeconds(3600);
319+
Token validTokenNoRefresh = new Token("valid_access_token", "Bearer", null, futureTime);
320+
321+
// Create mock token cache that returns the valid token.
322+
TokenCache mockTokenCache = Mockito.mock(TokenCache.class);
323+
Mockito.doReturn(validTokenNoRefresh).when(mockTokenCache).load();
324+
325+
// Create config with HTTP client and mock token cache.
326+
DatabricksConfig config =
327+
new DatabricksConfig()
328+
.setAuthType("external-browser")
329+
.setHost("https://test.databricks.com")
330+
.setClientId("test-client-id")
331+
.setHttpClient(mockHttpClient);
332+
333+
// We need to provide OIDC endpoints.
334+
OpenIDConnectEndpoints endpoints =
335+
new OpenIDConnectEndpoints(
336+
"https://test.databricks.com/oidc/v1/token",
337+
"https://test.databricks.com/oidc/v1/authorize");
338+
339+
// Create our provider with the mock token cache.
340+
ExternalBrowserCredentialsProvider provider =
341+
Mockito.spy(new ExternalBrowserCredentialsProvider(mockTokenCache));
342+
343+
// Spy on the config to inject the endpoints.
344+
DatabricksConfig spyConfig = Mockito.spy(config);
345+
Mockito.doReturn(endpoints).when(spyConfig).getOidcEndpoints();
346+
347+
// Configure provider.
348+
HeaderFactory headerFactory = provider.configure(spyConfig);
349+
assertNotNull(headerFactory, "HeaderFactory should be created");
350+
351+
// Verify headers contain the cached token (NOT browser auth!).
352+
Map<String, String> headers = headerFactory.headers();
322353
assertEquals(
323-
"new_refresh_token",
324-
savedToken.getRefreshToken(),
325-
"Should save new refresh token to cache");
354+
"Bearer valid_access_token",
355+
headers.get("Authorization"),
356+
"Should use cached valid token even without refresh token");
357+
358+
// Verify token was loaded from cache.
359+
Mockito.verify(mockTokenCache, Mockito.times(1)).load();
360+
361+
// Verify NO HTTP call was made (token is still valid, no refresh needed).
362+
Mockito.verify(mockHttpClient, Mockito.never()).execute(any(Request.class));
363+
364+
// Verify performBrowserAuth was NOT called.
365+
Mockito.verify(provider, Mockito.never())
366+
.performBrowserAuth(any(DatabricksConfig.class), any(), any(), any(TokenCache.class));
367+
368+
// Verify no token was saved (we're using the cached one as-is).
369+
Mockito.verify(mockTokenCache, Mockito.never()).save(any(Token.class));
326370
}
327371

328372
@Test
@@ -356,7 +400,8 @@ void cacheWithInvalidAccessTokenValidRefreshTest() throws IOException {
356400
// We need to provide OIDC endpoints for token refresh
357401
OpenIDConnectEndpoints endpoints =
358402
new OpenIDConnectEndpoints(
359-
"https://test.databricks.com/token", "https://test.databricks.com/authorize");
403+
"https://test.databricks.com/oidc/v1/token",
404+
"https://test.databricks.com/oidc/v1/authorize");
360405

361406
// Create our provider with the mock token cache
362407
ExternalBrowserCredentialsProvider provider =
@@ -455,7 +500,8 @@ void cacheWithInvalidAccessTokenRefreshFailingTest() throws IOException {
455500
// We need to provide OIDC endpoints for token refresh attempt
456501
OpenIDConnectEndpoints endpoints =
457502
new OpenIDConnectEndpoints(
458-
"https://test.databricks.com/token", "https://test.databricks.com/authorize");
503+
"https://test.databricks.com/oidc/v1/token",
504+
"https://test.databricks.com/oidc/v1/authorize");
459505

460506
// Create our provider and mock the browser auth method
461507
ExternalBrowserCredentialsProvider provider =

0 commit comments

Comments
 (0)