Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions NEXT_CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,12 @@

### Bug Fixes

- Make the client ID optional in `DatabricksOAuthTokenSource`. Previously `getToken()` threw a
`NullPointerException` ("ClientID cannot be null") when no client ID was set, which prevented
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.

### Security Vulnerabilities

### Documentation
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,9 @@ public static class Builder {
/**
* Creates a new Builder with required parameters.
*
* @param clientId OAuth client ID.
* @param clientId OAuth client ID. May be null or empty for account-wide token federation (e.g.
* users authenticated through a web browser OAuth flow whose IdP JWT has no client ID); in
* that case the client_id parameter is omitted from the token exchange request.
* @param host Databricks host URL.
* @param endpoints OpenID Connect endpoints configuration.
* @param idTokenSource Source of ID tokens.
Expand Down Expand Up @@ -147,20 +149,17 @@ public DatabricksOAuthTokenSource build() {
*
* @return A Token containing the access token and related information.
* @throws DatabricksException when the token exchange fails.
* @throws IllegalArgumentException when the required string parameters are empty.
* @throws NullPointerException when any of the required parameters are null.
* @throws IllegalArgumentException when the host is empty.
* @throws NullPointerException when any of the required parameters (host, endpoints,
* idTokenSource, httpClient) are null. The client ID is optional.
*/
@Override
public Token getToken() {
Objects.requireNonNull(clientId, "ClientID cannot be null");
Objects.requireNonNull(host, "Host cannot be null");
Objects.requireNonNull(endpoints, "Endpoints cannot be null");
Objects.requireNonNull(idTokenSource, "IDTokenSource cannot be null");
Objects.requireNonNull(httpClient, "HttpClient cannot be null");

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

OAuthResponse response;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -117,6 +117,13 @@ private static Stream<TestCase> provideTestCases() throws MalformedURLException
formParams.put("scope", "all-apis");
FormRequest expectedRequest = new FormRequest(TEST_TOKEN_ENDPOINT, formParams);

// Expected request for account-wide token federation, where no client ID is provided (e.g.
// users authenticated through a web browser OAuth flow). The client_id param is omitted.
Map<String, String> formParamsNoClientId = new HashMap<>(formParams);
formParamsNoClientId.remove("client_id");
FormRequest expectedRequestNoClientId =
new FormRequest(TEST_TOKEN_ENDPOINT, formParamsNoClientId);

return Stream.of(
// Token exchange test cases
new TestCase(
Expand Down Expand Up @@ -198,27 +205,27 @@ private static Stream<TestCase> provideTestCases() throws MalformedURLException
DatabricksException.class),
// Parameter validation test cases
new TestCase(
"Null client ID",
"Null client ID performs account-wide token federation",
null,
TEST_HOST,
testEndpoints,
testIdTokenSource,
createMockHttpClient(expectedRequest, 200, successJson),
null,
createMockHttpClient(expectedRequestNoClientId, 200, successJson),
null,
null,
NullPointerException.class),
TEST_TOKEN_ENDPOINT,
null),
new TestCase(
"Empty client ID",
"Empty client ID performs account-wide token federation",
"",
TEST_HOST,
testEndpoints,
testIdTokenSource,
createMockHttpClient(expectedRequest, 200, successJson),
null,
createMockHttpClient(expectedRequestNoClientId, 200, successJson),
null,
null,
IllegalArgumentException.class),
TEST_TOKEN_ENDPOINT,
null),
new TestCase(
"Null host",
TEST_CLIENT_ID,
Expand Down
Loading