Skip to content

Commit a233879

Browse files
authored
[PECOBLR-15] Respect OAuth Discovery URL for ExternalBrowserCredentialsStrategy (#505)
## What changes are proposed in this pull request? - **WHAT** : Discovery URL is currently being honored only in the M2M flow. This PR makes changes to honor it for U2M and other flows as well. Discovery URL was added by me last year in SDK for M2M flow [Link](#336) - ~~I have also added a fallback to defaultOIDC flow to make this backward compatible too.~~ skipped the fallback in this PR, will raise in a followup if I get approval from the SDK team - **WHY** : A major customer ask is to extend discovery URL for U2M flow as well. ## How is this tested? Tested that discovery URL is being honored in U2M as well. Tested this using the OSS JDBC with the following URL 1. Success flow test : ``` String jdbcUrl = "jdbc:databricks://e2-dogfood.staging.cloud.databricks.com:443/default;transportMode=http;ssl=1;httpPath=/sql/1.0/warehouses/dd43ee29fedd958d;" + "AuthMech=11;Auth_Flow=2;LogLevel=6;UseThriftClient=0;EnableTokenCache=0;OIDCDiscoveryEndpoint=https://e2-dogfood.staging.cloud.databricks.com/oidc/.well-known/oauth-authorization-server"; ``` 2. Failure flow test : ``` String jdbcUrl = "jdbc:databricks://e2-dogfood.staging.cloud.databricks.com:443/default;transportMode=http;ssl=1;httpPath=/sql/1.0/warehouses/dd43ee29fedd958d;" + "AuthMech=11;Auth_Flow=2;LogLevel=6;UseThriftClient=0;EnableTokenCache=0;OIDCDiscoveryEndpoint=https://google.com"; ```
1 parent 82ad269 commit a233879

File tree

4 files changed

+20
-4
lines changed

4 files changed

+20
-4
lines changed

NEXT_CHANGELOG.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@
44

55
### New Features and Improvements
66

7+
* Add support for discovery URL for browser based authentication flow.
8+
79
### Bug Fixes
810

911
### Documentation

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -130,6 +130,7 @@ CachedTokenSource performBrowserAuth(
130130
.withRedirectUrl(config.getEffectiveOAuthRedirectUrl())
131131
.withBrowserTimeout(config.getOAuthBrowserAuthTimeout())
132132
.withScopes(new ArrayList<>(scopes))
133+
.withOpenIDConnectEndpoints(config.getOidcEndpoints())
133134
.build();
134135
Consent consent = client.initiateConsent();
135136

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

Lines changed: 15 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ public static class Builder {
4343
private HttpClient hc;
4444
private String accountId;
4545
private Optional<Duration> browserTimeout = Optional.empty();
46+
private OpenIDConnectEndpoints openIDConnectEndpoints;
4647

4748
public Builder() {}
4849

@@ -51,6 +52,11 @@ public Builder withHttpClient(HttpClient hc) {
5152
return this;
5253
}
5354

55+
public Builder withOpenIDConnectEndpoints(OpenIDConnectEndpoints openIDConnectEndpoints) {
56+
this.openIDConnectEndpoints = openIDConnectEndpoints;
57+
return this;
58+
}
59+
5460
public Builder withHost(String host) {
5561
this.host = host;
5662
return this;
@@ -102,6 +108,7 @@ public Builder withBrowserTimeout(Duration browserTimeout) {
102108
private final SecureRandom random = new SecureRandom();
103109
private final boolean isAws;
104110
private final boolean isAzure;
111+
private final OpenIDConnectEndpoints openIDConnectEndpoints;
105112
private final Optional<Duration> browserTimeout;
106113

107114
private OAuthClient(Builder b) throws IOException {
@@ -113,15 +120,15 @@ private OAuthClient(Builder b) throws IOException {
113120

114121
DatabricksConfig config =
115122
new DatabricksConfig().setHost(b.host).setAccountId(b.accountId).resolve();
116-
OpenIDConnectEndpoints oidc = config.getOidcEndpoints();
117-
if (oidc == null) {
123+
openIDConnectEndpoints = b.openIDConnectEndpoints;
124+
if (openIDConnectEndpoints == null) {
118125
throw new DatabricksException(b.host + " does not support OAuth");
119126
}
120127

121128
this.isAws = config.isAws();
122129
this.isAzure = config.isAzure();
123-
this.tokenUrl = oidc.getTokenEndpoint();
124-
this.authUrl = oidc.getAuthorizationEndpoint();
130+
this.tokenUrl = openIDConnectEndpoints.getTokenEndpoint();
131+
this.authUrl = openIDConnectEndpoints.getAuthorizationEndpoint();
125132
this.browserTimeout = b.browserTimeout;
126133
this.scopes = b.scopes;
127134
}
@@ -138,6 +145,10 @@ public String getClientSecret() {
138145
return clientSecret;
139146
}
140147

148+
public OpenIDConnectEndpoints getOidcEndpoints() {
149+
return openIDConnectEndpoints;
150+
}
151+
141152
public String getRedirectUrl() {
142153
return redirectUrl;
143154
}

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,7 @@ void clientAndConsentTest() throws IOException {
4949
.withClientId(config.getClientId())
5050
.withClientSecret(config.getClientSecret())
5151
.withHost(config.getHost())
52+
.withOpenIDConnectEndpoints(config.getOidcEndpoints())
5253
.withRedirectUrl(config.getEffectiveOAuthRedirectUrl())
5354
.withScopes(config.getScopes())
5455
.build();
@@ -94,6 +95,7 @@ void clientAndConsentTestWithCustomRedirectUrl() throws IOException {
9495
.withClientId(config.getClientId())
9596
.withClientSecret(config.getClientSecret())
9697
.withHost(config.getHost())
98+
.withOpenIDConnectEndpoints(config.getOidcEndpoints())
9799
.withRedirectUrl(config.getEffectiveOAuthRedirectUrl())
98100
.withScopes(config.getScopes())
99101
.build();

0 commit comments

Comments
 (0)