diff --git a/NEXT_CHANGELOG.md b/NEXT_CHANGELOG.md index b4585a98d..5cede78d0 100644 --- a/NEXT_CHANGELOG.md +++ b/NEXT_CHANGELOG.md @@ -1,9 +1,13 @@ # NEXT CHANGELOG -## Release v0.68.0 +## Release v0.67.1 ### New Features and Improvements +* Add a new config attribute `DATABRICKS_DISABLE_OAUTH_REFRESH_TOKEN` to disable requesting + 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. + ### Bug Fixes ### Documentation diff --git a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java index 68cb0fff0..fb763aad2 100644 --- a/databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java +++ b/databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java @@ -175,6 +175,14 @@ public class DatabricksConfig { @ConfigAttribute(env = "DATABRICKS_OAUTH_BROWSER_AUTH_TIMEOUT") private Duration oauthBrowserAuthTimeout; + /** + * Disable automatically adding the offline_access scope to the OAuth authentication request to + * request refresh tokens. Note that this does not remove the scope if it is explicitly provided + * by the user. + */ + @ConfigAttribute(env = "DATABRICKS_DISABLE_OAUTH_REFRESH_TOKEN") + private Boolean disableOauthRefreshToken; + public Environment getEnv() { return env; } @@ -631,6 +639,15 @@ public DatabricksConfig setOAuthBrowserAuthTimeout(Duration oauthBrowserAuthTime return this; } + public boolean getDisableOauthRefreshToken() { + return disableOauthRefreshToken != null && disableOauthRefreshToken; + } + + public DatabricksConfig setDisableOauthRefreshToken(boolean disable) { + this.disableOauthRefreshToken = disable; + return this; + } + public boolean isAzure() { if (azureWorkspaceResourceId != null) { return true; 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 a2821af24..fe89b0a7f 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 @@ -7,6 +7,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.HashSet; +import java.util.List; import java.util.Objects; import java.util.Optional; import java.util.Set; @@ -105,20 +106,25 @@ public OAuthHeaderFactory configure(DatabricksConfig config) { } } - CachedTokenSource performBrowserAuth( - DatabricksConfig config, String clientId, String clientSecret, TokenCache tokenCache) - throws IOException { - LOGGER.debug("Performing browser authentication"); - + protected List getScopes(DatabricksConfig config) { // Get user-provided scopes and add required default scopes. Set scopes = new HashSet<>(config.getScopes()); - - // Needed to request a refresh token. - scopes.add("offline_access"); - + // Requesting a refresh token is most of the time the right thing to do from a + // user perspective to enable long-lived access to the API. However, some Identity + // Providers do not support refresh tokens. + if (!config.getDisableOauthRefreshToken()) { + scopes.add("offline_access"); + } if (config.isAzure()) { scopes.add(config.getEffectiveAzureLoginAppId() + "/user_impersonation"); } + return new ArrayList<>(scopes); + } + + CachedTokenSource performBrowserAuth( + DatabricksConfig config, String clientId, String clientSecret, TokenCache tokenCache) + throws IOException { + LOGGER.debug("Performing browser authentication"); OAuthClient client = new OAuthClient.Builder() @@ -129,7 +135,7 @@ CachedTokenSource performBrowserAuth( .withAccountId(config.getAccountId()) .withRedirectUrl(config.getEffectiveOAuthRedirectUrl()) .withBrowserTimeout(config.getOAuthBrowserAuthTimeout()) - .withScopes(new ArrayList<>(scopes)) + .withScopes(getScopes(config)) .withOpenIDConnectEndpoints(config.getOidcEndpoints()) .build(); Consent consent = client.initiateConsent(); diff --git a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java index 0f3fecf6d..5600f5e51 100644 --- a/databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java +++ b/databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java @@ -299,4 +299,27 @@ public void testDisableRetriesEnvironmentVariable() { assertEquals(true, config.getDisableRetries()); } + + @Test + public void testDisableOauthRefreshTokenDefaultValue() { + DatabricksConfig config = new DatabricksConfig(); + assertEquals(false, config.getDisableOauthRefreshToken()); + } + + @Test + public void testDisableOauthRefreshTokenSetAndGet() { + DatabricksConfig config = new DatabricksConfig().setDisableOauthRefreshToken(true); + assertEquals(true, config.getDisableOauthRefreshToken()); + } + + @Test + public void testDisableOauthRefreshTokenEnvironmentVariable() { + Map env = new HashMap<>(); + env.put("DATABRICKS_DISABLE_OAUTH_REFRESH_TOKEN", "true"); + + DatabricksConfig config = new DatabricksConfig(); + config.resolve(new Environment(env, new ArrayList<>(), System.getProperty("os.name"))); + + assertEquals(true, config.getDisableOauthRefreshToken()); + } } 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 70981a54b..f920f38d6 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 @@ -16,6 +16,7 @@ import java.time.Instant; import java.util.Arrays; import java.util.HashMap; +import java.util.List; import java.util.Map; import java.util.Optional; import org.junit.jupiter.api.Test; @@ -551,4 +552,53 @@ void cacheWithInvalidTokensTest() throws IOException { // Verify token was saved after browser auth (for the new token) Mockito.verify(mockTokenCache, Mockito.times(1)).save(any(Token.class)); } + + @Test + void doNotAddOfflineAccessScopeWhenDisableOauthRefreshTokenIsTrue() { + DatabricksConfig config = + new DatabricksConfig() + .setHost("https://test.databricks.com") + .setClientId("test-client-id") + .setDisableOauthRefreshToken(true) + .setScopes(Arrays.asList("my-test-scope")); + + ExternalBrowserCredentialsProvider provider = new ExternalBrowserCredentialsProvider(); + List scopes = provider.getScopes(config); + + assertEquals(1, scopes.size()); + assertTrue(scopes.contains("my-test-scope")); + } + + @Test + void doNotRemoveUserProvidedScopesWhenDisableOauthRefreshTokenIsTrue() { + DatabricksConfig config = + new DatabricksConfig() + .setHost("https://test.databricks.com") + .setClientId("test-client-id") + .setDisableOauthRefreshToken(true) + .setScopes(Arrays.asList("my-test-scope", "offline_access")); + + ExternalBrowserCredentialsProvider provider = new ExternalBrowserCredentialsProvider(); + List scopes = provider.getScopes(config); + + assertEquals(2, scopes.size()); + assertTrue(scopes.contains("offline_access")); + assertTrue(scopes.contains("my-test-scope")); + } + + @Test + void addOfflineAccessScopeWhenDisableOauthRefreshTokenIsFalse() { + DatabricksConfig config = + new DatabricksConfig() + .setHost("https://test.databricks.com") + .setClientId("test-client-id") + .setScopes(Arrays.asList("my-test-scope")); + + ExternalBrowserCredentialsProvider provider = new ExternalBrowserCredentialsProvider(); + List scopes = provider.getScopes(config); + + assertEquals(2, scopes.size()); + assertTrue(scopes.contains("offline_access")); + assertTrue(scopes.contains("my-test-scope")); + } }