Skip to content

Commit 5a6c446

Browse files
committed
Merge remote-tracking branch 'origin/main' into deco-27280-paginator-empty-page
# Conflicts: # NEXT_CHANGELOG.md
2 parents 4ee7901 + 2b44706 commit 5a6c446

3 files changed

Lines changed: 32 additions & 16 deletions

File tree

NEXT_CHANGELOG.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@
1111
### Bug Fixes
1212

1313
* Fixed `Paginator` silently dropping results when a token-paginated response returned an empty page with a non-empty `next_page_token`. List methods (e.g. `tables().list()`) now keep paging until the page token is absent instead of stopping at the first empty page.
14+
- Make the client ID optional in `DatabricksOAuthTokenSource`. Previously `getToken()` threw a
15+
`NullPointerException` ("ClientID cannot be null") when no client ID was set, which prevented
16+
token exchange for users authenticated through a web browser OAuth flow whose IdP JWT does not
17+
contain a client ID. When the client ID is null or empty, the `client_id` parameter is now
18+
omitted from the token exchange request to perform account-wide token federation.
1419

1520
### Security Vulnerabilities
1621

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)