Skip to content

Commit e73b47d

Browse files
committed
Make client ID optional in DatabricksOAuthTokenSource
getToken() previously required a non-null, non-empty client ID and always sent it as the client_id form parameter, throwing NullPointerException ("ClientID cannot be null") otherwise. This broke token exchange for users authenticated through a web browser OAuth flow, whose IdP JWT does not contain a client ID. When the client ID is null or empty, the client_id parameter is now omitted from the token exchange request to perform account-wide token federation, matching the Go SDK behavior. Fixes #757 Signed-off-by: Hector Castejon Diaz <hector.castejon@databricks.com>
1 parent f8c4aa7 commit e73b47d

3 files changed

Lines changed: 33 additions & 16 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,12 @@
88

99
### Bug Fixes
1010

11+
- Make the client ID optional in `DatabricksOAuthTokenSource`. Previously `getToken()` threw a
12+
`NullPointerException` ("ClientID cannot be null") when no client ID was set, which prevented
13+
token exchange for users authenticated through a web browser OAuth flow whose IdP JWT does not
14+
contain a client ID. When the client ID is null or empty, the `client_id` parameter is now
15+
omitted from the token exchange request to perform account-wide token federation.
16+
1117
### Security Vulnerabilities
1218

1319
### Documentation

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

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,9 @@ public static class Builder {
7979
/**
8080
* Creates a new Builder with required parameters.
8181
*
82-
* @param clientId OAuth client ID.
82+
* @param clientId OAuth client ID. May be null or empty for account-wide token federation (e.g.
83+
* users authenticated through a web browser OAuth flow whose IdP JWT has no client ID); in
84+
* that case the client_id parameter is omitted from the token exchange request.
8385
* @param host Databricks host URL.
8486
* @param endpoints OpenID Connect endpoints configuration.
8587
* @param idTokenSource Source of ID tokens.
@@ -147,20 +149,17 @@ public DatabricksOAuthTokenSource build() {
147149
*
148150
* @return A Token containing the access token and related information.
149151
* @throws DatabricksException when the token exchange fails.
150-
* @throws IllegalArgumentException when the required string parameters are empty.
151-
* @throws NullPointerException when any of the required parameters are null.
152+
* @throws IllegalArgumentException when the host is empty.
153+
* @throws NullPointerException when any of the required parameters (host, endpoints,
154+
* idTokenSource, httpClient) are null. The client ID is optional.
152155
*/
153156
@Override
154157
public Token getToken() {
155-
Objects.requireNonNull(clientId, "ClientID cannot be null");
156158
Objects.requireNonNull(host, "Host cannot be null");
157159
Objects.requireNonNull(endpoints, "Endpoints cannot be null");
158160
Objects.requireNonNull(idTokenSource, "IDTokenSource cannot be null");
159161
Objects.requireNonNull(httpClient, "HttpClient cannot be null");
160162

161-
if (clientId.isEmpty()) {
162-
throw new IllegalArgumentException("ClientID cannot be empty");
163-
}
164163
if (host.isEmpty()) {
165164
throw new IllegalArgumentException("Host cannot be empty");
166165
}
@@ -173,7 +172,12 @@ public Token getToken() {
173172
params.put(SUBJECT_TOKEN_PARAM, idToken.getValue());
174173
params.put(SUBJECT_TOKEN_TYPE_PARAM, SUBJECT_TOKEN_TYPE);
175174
params.put(SCOPE_PARAM, String.join(" ", scopes));
176-
params.put(CLIENT_ID_PARAM, clientId);
175+
// The client ID is optional. Service principals (Workload Identity Federation) send it, but
176+
// users authenticated through a web browser OAuth flow have no client ID in their IdP JWT and
177+
// perform account-wide token federation without one.
178+
if (!Strings.isNullOrEmpty(clientId)) {
179+
params.put(CLIENT_ID_PARAM, clientId);
180+
}
177181

178182
OAuthResponse response;
179183
try {

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

Lines changed: 15 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,13 @@ private static Stream<TestCase> provideTestCases() throws MalformedURLException
117117
formParams.put("scope", "all-apis");
118118
FormRequest expectedRequest = new FormRequest(TEST_TOKEN_ENDPOINT, formParams);
119119

120+
// Expected request for account-wide token federation, where no client ID is provided (e.g.
121+
// users authenticated through a web browser OAuth flow). The client_id param is omitted.
122+
Map<String, String> formParamsNoClientId = new HashMap<>(formParams);
123+
formParamsNoClientId.remove("client_id");
124+
FormRequest expectedRequestNoClientId =
125+
new FormRequest(TEST_TOKEN_ENDPOINT, formParamsNoClientId);
126+
120127
return Stream.of(
121128
// Token exchange test cases
122129
new TestCase(
@@ -198,27 +205,27 @@ private static Stream<TestCase> provideTestCases() throws MalformedURLException
198205
DatabricksException.class),
199206
// Parameter validation test cases
200207
new TestCase(
201-
"Null client ID",
208+
"Null client ID performs account-wide token federation",
202209
null,
203210
TEST_HOST,
204211
testEndpoints,
205212
testIdTokenSource,
206-
createMockHttpClient(expectedRequest, 200, successJson),
207-
null,
213+
createMockHttpClient(expectedRequestNoClientId, 200, successJson),
208214
null,
209215
null,
210-
NullPointerException.class),
216+
TEST_TOKEN_ENDPOINT,
217+
null),
211218
new TestCase(
212-
"Empty client ID",
219+
"Empty client ID performs account-wide token federation",
213220
"",
214221
TEST_HOST,
215222
testEndpoints,
216223
testIdTokenSource,
217-
createMockHttpClient(expectedRequest, 200, successJson),
218-
null,
224+
createMockHttpClient(expectedRequestNoClientId, 200, successJson),
219225
null,
220226
null,
221-
IllegalArgumentException.class),
227+
TEST_TOKEN_ENDPOINT,
228+
null),
222229
new TestCase(
223230
"Null host",
224231
TEST_CLIENT_ID,

0 commit comments

Comments
 (0)