Skip to content

Commit cda5bde

Browse files
Use user provided scopes to all OAuth authentication flow. (#495)
## What changes are proposed in this pull request? This PR updates OAuth related flows to use the scopes user provided scopes if any, or default to `all-apis` otherwize. ## How is this tested? Unit and integration tests.
1 parent 1ef0e36 commit cda5bde

File tree

9 files changed

+50
-19
lines changed

9 files changed

+50
-19
lines changed

NEXT_CHANGELOG.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@
66

77
### Bug Fixes
88

9+
* User provided scopes are now properly propagated in OAuth flows.
10+
* [Warning] Correctly defaults to scope `all-apis` (instead of `clusters sql`) in U2M if no scopes are provided by the users. This change aligns the Java SDK logic with the Python and Go SDK logic.
11+
912
### Documentation
1013

1114
### Internal Changes

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DatabricksConfig.java

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -351,7 +351,16 @@ public DatabricksConfig setOAuthRedirectUrl(String redirectUrl) {
351351
return this;
352352
}
353353

354+
/**
355+
* Returns the scopes to use for the current configuration. If no scopes were provided, returns
356+
* the default "all-apis" scope.
357+
*
358+
* @return The scopes to use for the current configuration
359+
*/
354360
public List<String> getScopes() {
361+
if (scopes == null || scopes.isEmpty()) {
362+
return Arrays.asList("all-apis");
363+
}
355364
return scopes;
356365
}
357366

databricks-sdk-java/src/main/java/com/databricks/sdk/core/DefaultCredentialsProvider.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -139,6 +139,7 @@ private void addOIDCCredentialsProviders(DatabricksConfig config) {
139139
config.getHttpClient())
140140
.audience(config.getTokenAudience())
141141
.accountId(config.isAccountClient() ? config.getAccountId() : null)
142+
.scopes(config.getScopes())
142143
.build();
143144

144145
providers.add(new TokenSourceCredentialsProvider(oauthTokenSource, namedIdTokenSource.name));

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

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
import com.databricks.sdk.core.http.HttpClient;
55
import com.google.common.base.Strings;
66
import java.time.Instant;
7+
import java.util.Arrays;
78
import java.util.HashMap;
9+
import java.util.List;
810
import java.util.Map;
911
import java.util.Objects;
1012
import org.slf4j.Logger;
@@ -31,10 +33,11 @@ public class DatabricksOAuthTokenSource implements TokenSource {
3133
private final IDTokenSource idTokenSource;
3234
/** HTTP client for making token exchange requests. */
3335
private final HttpClient httpClient;
36+
/** Scopes to request during token exchange. */
37+
private final List<String> scopes;
3438

3539
private static final String GRANT_TYPE = "urn:ietf:params:oauth:grant-type:token-exchange";
3640
private static final String SUBJECT_TOKEN_TYPE = "urn:ietf:params:oauth:token-type:jwt";
37-
private static final String SCOPE = "all-apis";
3841
private static final String GRANT_TYPE_PARAM = "grant_type";
3942
private static final String SUBJECT_TOKEN_PARAM = "subject_token";
4043
private static final String SUBJECT_TOKEN_TYPE_PARAM = "subject_token_type";
@@ -49,6 +52,7 @@ private DatabricksOAuthTokenSource(Builder builder) {
4952
this.audience = builder.audience;
5053
this.idTokenSource = builder.idTokenSource;
5154
this.httpClient = builder.httpClient;
55+
this.scopes = builder.scopes == null ? Arrays.asList() : builder.scopes;
5256
}
5357

5458
/**
@@ -63,6 +67,7 @@ public static class Builder {
6367
private final HttpClient httpClient;
6468
private String accountId;
6569
private String audience;
70+
private List<String> scopes;
6671

6772
/**
6873
* Creates a new Builder with required parameters.
@@ -108,6 +113,17 @@ public Builder audience(String audience) {
108113
return this;
109114
}
110115

116+
/**
117+
* Sets the scopes to request during token exchange.
118+
*
119+
* @param scopes The scopes to request.
120+
* @return This builder instance.
121+
*/
122+
public Builder scopes(List<String> scopes) {
123+
this.scopes = scopes;
124+
return this;
125+
}
126+
111127
/**
112128
* Builds a new DatabricksOAuthTokenSource instance.
113129
*
@@ -149,7 +165,7 @@ public Token getToken() {
149165
params.put(GRANT_TYPE_PARAM, GRANT_TYPE);
150166
params.put(SUBJECT_TOKEN_PARAM, idToken.getValue());
151167
params.put(SUBJECT_TOKEN_TYPE_PARAM, SUBJECT_TOKEN_TYPE);
152-
params.put(SCOPE_PARAM, SCOPE);
168+
params.put(SCOPE_PARAM, String.join(" ", scopes));
153169
params.put(CLIENT_ID_PARAM, clientId);
154170

155171
OAuthResponse response;

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

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5,8 +5,11 @@
55
import com.databricks.sdk.core.DatabricksException;
66
import java.io.IOException;
77
import java.nio.file.Path;
8+
import java.util.ArrayList;
9+
import java.util.HashSet;
810
import java.util.Objects;
911
import java.util.Optional;
12+
import java.util.Set;
1013
import org.slf4j.Logger;
1114
import org.slf4j.LoggerFactory;
1215

@@ -106,6 +109,17 @@ CachedTokenSource performBrowserAuth(
106109
DatabricksConfig config, String clientId, String clientSecret, TokenCache tokenCache)
107110
throws IOException {
108111
LOGGER.debug("Performing browser authentication");
112+
113+
// Get user-provided scopes and add required default scopes.
114+
Set<String> scopes = new HashSet<>(config.getScopes());
115+
116+
// Needed to request a refresh token.
117+
scopes.add("offline_access");
118+
119+
if (config.isAzure()) {
120+
scopes.add(config.getEffectiveAzureLoginAppId() + "/user_impersonation");
121+
}
122+
109123
OAuthClient client =
110124
new OAuthClient.Builder()
111125
.withHttpClient(config.getHttpClient())
@@ -114,7 +128,7 @@ CachedTokenSource performBrowserAuth(
114128
.withHost(config.getHost())
115129
.withAccountId(config.getAccountId())
116130
.withRedirectUrl(config.getEffectiveOAuthRedirectUrl())
117-
.withScopes(config.getScopes())
131+
.withScopes(new ArrayList<>(scopes))
118132
.build();
119133
Consent consent = client.initiateConsent();
120134

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66
import com.databricks.sdk.core.HeaderFactory;
77
import com.google.common.collect.ImmutableMap;
88
import java.io.IOException;
9-
import java.util.Collections;
109
import java.util.HashMap;
1110
import java.util.Map;
1211

@@ -47,7 +46,7 @@ public HeaderFactory configure(DatabricksConfig config) throws DatabricksExcepti
4746
.withHttpClient(config.getHttpClient())
4847
.withClientId(config.getClientId())
4948
.withTokenUrl(endpointUrl)
50-
.withScopes(Collections.singletonList("all-apis"))
49+
.withScopes(config.getScopes())
5150
.withAuthParameterPosition(AuthParameterPosition.HEADER)
5251
.withEndpointParametersSupplier(
5352
() ->

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

Lines changed: 1 addition & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -109,17 +109,7 @@ private OAuthClient(Builder b) throws IOException {
109109
this.isAzure = config.isAzure();
110110
this.tokenUrl = oidc.getTokenEndpoint();
111111
this.authUrl = oidc.getAuthorizationEndpoint();
112-
113-
List<String> scopes = b.scopes;
114-
if (scopes == null) {
115-
scopes = Arrays.asList("offline_access", "clusters", "sql");
116-
}
117-
if (config.isAzure()) {
118-
scopes =
119-
Arrays.asList(
120-
config.getEffectiveAzureLoginAppId() + "/user_impersonation", "offline_access");
121-
}
122-
this.scopes = scopes;
112+
this.scopes = b.scopes;
123113
}
124114

125115
public String getHost() {

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

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@
22

33
import com.databricks.sdk.core.*;
44
import java.io.IOException;
5-
import java.util.Collections;
65

76
/**
87
* Adds refreshed Databricks machine-to-machine OAuth Bearer token to every request, if
@@ -33,7 +32,7 @@ public OAuthHeaderFactory configure(DatabricksConfig config) {
3332
.withClientId(config.getClientId())
3433
.withClientSecret(config.getClientSecret())
3534
.withTokenUrl(jsonResponse.getTokenEndpoint())
36-
.withScopes(Collections.singletonList("all-apis"))
35+
.withScopes(config.getScopes())
3736
.withAuthParameterPosition(AuthParameterPosition.HEADER)
3837
.build();
3938

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ void clientAndConsentTest() throws IOException {
6262
assertTrue(authUrl.contains("response_type=code"));
6363
assertTrue(authUrl.contains("client_id=test-client-id"));
6464
assertTrue(authUrl.contains("redirect_uri=http://localhost:8080/callback"));
65-
assertTrue(authUrl.contains("scope=offline_access%20clusters%20sql"));
65+
assertTrue(authUrl.contains("scope=all-apis"));
6666
}
6767
}
6868

0 commit comments

Comments
 (0)