Skip to content

Commit 7f9b0c8

Browse files
Add an option to set a timeout for browser confirmation in the U2M authentication flow. (#496)
## What changes are proposed in this pull request? This PR adds a configurable timeout for OAuth browser authentication in the Databricks SDK for Java. The OAuth browser authentication flow currently waits indefinitely for user interaction, which can cause applications to hang indefinitely if users don't complete the authentication process. The key changes include: - New configuration option: Added `setOAuthBrowserAuthTimeout(Duration timeout)` to `DatabricksConfig` with environment variable support via `DATABRICKS_OAUTH_BROWSER_AUTH_TIMEOUT`. - Timeout implementation: Modified the OAuth consent flow in Consent.CallbackResponseHandler to enforce the configured timeout when waiting for browser callback parameters. - Bug fixes: Fixed ConfigAttributeAccessor to properly handle Duration, Integer, and Boolean types from environment variables. ## How is this tested? Added unit tests to verify that env variables are properly loaded and that the timeout logic works as expected.
1 parent cda5bde commit 7f9b0c8

File tree

8 files changed

+265
-16
lines changed

8 files changed

+265
-16
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 option to add a timeout for browser confirmation in the U2M authentication flow.
8+
79
### Bug Fixes
810

911
* User provided scopes are now properly propagated in OAuth flows.

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package com.databricks.sdk.core;
22

33
import java.lang.reflect.Field;
4+
import java.time.Duration;
45
import java.util.Map;
56
import java.util.Objects;
67

@@ -41,16 +42,25 @@ public void setValueOnConfig(DatabricksConfig cfg, String value) throws IllegalA
4142
// workspace clients or config resolution) are safe
4243
synchronized (field) {
4344
field.setAccessible(true);
45+
if (value == null || value.trim().isEmpty()) {
46+
return;
47+
}
48+
4449
if (field.getType() == String.class) {
4550
field.set(cfg, value);
4651
} else if (field.getType() == int.class) {
4752
field.set(cfg, Integer.parseInt(value));
53+
} else if (field.getType() == Integer.class) {
54+
field.set(cfg, Integer.parseInt(value));
4855
} else if (field.getType() == boolean.class) {
4956
field.set(cfg, Boolean.parseBoolean(value));
57+
} else if (field.getType() == Boolean.class) {
58+
field.set(cfg, Boolean.parseBoolean(value));
59+
} else if (field.getType() == Duration.class) {
60+
int seconds = Integer.parseInt(value);
61+
field.set(cfg, seconds > 0 ? Duration.ofSeconds(seconds) : null);
5062
} else if (field.getType() == ProxyConfig.ProxyAuthType.class) {
51-
if (value != null) {
52-
field.set(cfg, ProxyConfig.ProxyAuthType.valueOf(value));
53-
}
63+
field.set(cfg, ProxyConfig.ProxyAuthType.valueOf(value));
5464
}
5565
field.setAccessible(false);
5666
}

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

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
import java.io.File;
1515
import java.io.IOException;
1616
import java.lang.reflect.Field;
17+
import java.time.Duration;
1718
import java.util.*;
1819
import org.apache.http.HttpMessage;
1920

@@ -163,6 +164,13 @@ public class DatabricksConfig {
163164
@ConfigAttribute(env = "DATABRICKS_DISABLE_ASYNC_TOKEN_REFRESH")
164165
private Boolean disableAsyncTokenRefresh;
165166

167+
/**
168+
* The duration to wait for a browser response during U2M authentication before timing out. If set
169+
* to 0 or null, the connector waits for an indefinite amount of time.
170+
*/
171+
@ConfigAttribute(env = "DATABRICKS_OAUTH_BROWSER_AUTH_TIMEOUT")
172+
private Duration oauthBrowserAuthTimeout;
173+
166174
public Environment getEnv() {
167175
return env;
168176
}
@@ -597,6 +605,15 @@ public DatabricksConfig setDisableAsyncTokenRefresh(boolean disableAsyncTokenRef
597605
return this;
598606
}
599607

608+
public Duration getOAuthBrowserAuthTimeout() {
609+
return oauthBrowserAuthTimeout;
610+
}
611+
612+
public DatabricksConfig setOAuthBrowserAuthTimeout(Duration oauthBrowserAuthTimeout) {
613+
this.oauthBrowserAuthTimeout = oauthBrowserAuthTimeout;
614+
return this;
615+
}
616+
600617
public boolean isAzure() {
601618
if (azureWorkspaceResourceId != null) {
602619
return true;

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

Lines changed: 52 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,12 @@
1414
import java.io.Serializable;
1515
import java.net.*;
1616
import java.nio.charset.StandardCharsets;
17+
import java.time.Duration;
1718
import java.util.Arrays;
1819
import java.util.HashMap;
1920
import java.util.Map;
2021
import java.util.Objects;
22+
import java.util.Optional;
2123
import org.apache.commons.io.IOUtils;
2224
import org.slf4j.Logger;
2325
import org.slf4j.LoggerFactory;
@@ -53,6 +55,7 @@ public class Consent implements Serializable {
5355
private final String redirectUrl;
5456
private final String clientId;
5557
private final String clientSecret;
58+
private final Optional<Duration> browserTimeout;
5659

5760
public static class Builder {
5861
private HttpClient hc = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
@@ -63,6 +66,7 @@ public static class Builder {
6366
private String redirectUrl;
6467
private String clientId;
6568
private String clientSecret;
69+
private Optional<Duration> browserTimeout = Optional.empty();
6670

6771
public Builder withHttpClient(HttpClient hc) {
6872
this.hc = hc;
@@ -104,6 +108,11 @@ public Builder withClientSecret(String clientSecret) {
104108
return this;
105109
}
106110

111+
public Builder withBrowserTimeout(Optional<Duration> browserTimeout) {
112+
this.browserTimeout = browserTimeout;
113+
return this;
114+
}
115+
107116
public Consent build() {
108117
return new Consent(this);
109118
}
@@ -119,6 +128,7 @@ private Consent(Builder builder) {
119128
this.clientId = Objects.requireNonNull(builder.clientId);
120129
// This may be null for native apps or single-page apps.
121130
this.clientSecret = builder.clientSecret;
131+
this.browserTimeout = builder.browserTimeout;
122132
}
123133

124134
public Consent setHttpClient(HttpClient hc) {
@@ -155,6 +165,10 @@ public String getClientSecret() {
155165
return clientSecret;
156166
}
157167

168+
public Optional<Duration> getBrowserTimeout() {
169+
return browserTimeout;
170+
}
171+
158172
/**
159173
* Launch a browser to collect an authorization code and exchange the code for an OAuth token.
160174
*
@@ -219,15 +233,19 @@ private Map<String, String> getOAuthCallbackParameters() throws IOException {
219233
+ redirect.getHost()
220234
+ ", redirectUrl host must be one of: localhost, 127.0.0.1");
221235
}
222-
CallbackResponseHandler handler = new CallbackResponseHandler();
236+
237+
CallbackResponseHandler handler = new CallbackResponseHandler(this.browserTimeout);
223238
HttpServer httpServer =
224239
HttpServer.create(new InetSocketAddress(redirect.getHost(), redirect.getPort()), 0);
225240
httpServer.createContext("/", handler);
226241
httpServer.start();
227-
desktopBrowser();
228-
Map<String, String> params = handler.getParams();
229-
httpServer.stop(0);
230-
return params;
242+
243+
try {
244+
desktopBrowser();
245+
return handler.getParams();
246+
} finally {
247+
httpServer.stop(0);
248+
}
231249
}
232250

233251
/**
@@ -282,9 +300,13 @@ private Token exchange(String code, String state) {
282300

283301
static class CallbackResponseHandler implements HttpHandler {
284302
private final Logger LOG = LoggerFactory.getLogger(getClass().getName());
285-
// Protects params
286-
private final Object lock = new Object();
303+
protected final Object lock = new Object(); // protected for testing
287304
private volatile Map<String, String> params;
305+
private final Optional<Duration> timeout;
306+
307+
public CallbackResponseHandler(Optional<Duration> timeout) {
308+
this.timeout = timeout;
309+
}
288310

289311
@Override
290312
public void handle(HttpExchange exchange) {
@@ -323,10 +345,7 @@ public void handleInner(HttpExchange exchange) throws IOException {
323345
});
324346

325347
sendSuccess(exchange);
326-
synchronized (lock) {
327-
params = theseParams;
328-
lock.notify();
329-
}
348+
setParams(theseParams);
330349
}
331350

332351
private void sendError(
@@ -369,11 +388,25 @@ private void sendSuccess(HttpExchange exchange) throws IOException {
369388
exchange.close();
370389
}
371390

391+
/**
392+
* Wait and return the params.
393+
*
394+
* <p>This method might throw an exception in case of timeout.
395+
*/
372396
public Map<String, String> getParams() {
373397
synchronized (lock) {
374398
if (params == null) {
375399
try {
376-
lock.wait();
400+
if (timeout.isPresent()) {
401+
Duration t = timeout.get();
402+
lock.wait(t.toMillis());
403+
if (params == null) {
404+
throw new DatabricksException(
405+
"OAuth browser authentication timed out after " + t.getSeconds() + " seconds");
406+
}
407+
} else {
408+
lock.wait();
409+
}
377410
} catch (InterruptedException e) {
378411
throw new DatabricksException(
379412
"Interrupted while waiting for parameters: " + e.getMessage(), e);
@@ -382,5 +415,12 @@ public Map<String, String> getParams() {
382415
return params;
383416
}
384417
}
418+
419+
void setParams(Map<String, String> params) {
420+
synchronized (lock) {
421+
this.params = params;
422+
lock.notify();
423+
}
424+
}
385425
}
386426
}

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
@@ -128,6 +128,7 @@ CachedTokenSource performBrowserAuth(
128128
.withHost(config.getHost())
129129
.withAccountId(config.getAccountId())
130130
.withRedirectUrl(config.getEffectiveOAuthRedirectUrl())
131+
.withBrowserTimeout(config.getOAuthBrowserAuthTimeout())
131132
.withScopes(new ArrayList<>(scopes))
132133
.build();
133134
Consent consent = client.initiateConsent();

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

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,13 @@
99
import java.security.MessageDigest;
1010
import java.security.NoSuchAlgorithmException;
1111
import java.security.SecureRandom;
12-
import java.util.*;
12+
import java.time.Duration;
13+
import java.util.Base64;
14+
import java.util.HashMap;
15+
import java.util.List;
16+
import java.util.Map;
17+
import java.util.Objects;
18+
import java.util.Optional;
1319
import java.util.stream.Collectors;
1420

1521
/**
@@ -36,6 +42,7 @@ public static class Builder {
3642
private String clientSecret;
3743
private HttpClient hc;
3844
private String accountId;
45+
private Optional<Duration> browserTimeout = Optional.empty();
3946

4047
public Builder() {}
4148

@@ -77,6 +84,11 @@ public Builder withAccountId(String accountId) {
7784
this.accountId = accountId;
7885
return this;
7986
}
87+
88+
public Builder withBrowserTimeout(Duration browserTimeout) {
89+
this.browserTimeout = Optional.of(browserTimeout);
90+
return this;
91+
}
8092
}
8193

8294
private final String clientId;
@@ -90,6 +102,7 @@ public Builder withAccountId(String accountId) {
90102
private final SecureRandom random = new SecureRandom();
91103
private final boolean isAws;
92104
private final boolean isAzure;
105+
private final Optional<Duration> browserTimeout;
93106

94107
private OAuthClient(Builder b) throws IOException {
95108
this.clientId = Objects.requireNonNull(b.clientId);
@@ -109,6 +122,7 @@ private OAuthClient(Builder b) throws IOException {
109122
this.isAzure = config.isAzure();
110123
this.tokenUrl = oidc.getTokenEndpoint();
111124
this.authUrl = oidc.getAuthorizationEndpoint();
125+
this.browserTimeout = b.browserTimeout;
112126
this.scopes = b.scopes;
113127
}
114128

@@ -197,6 +211,7 @@ public Consent initiateConsent() throws MalformedURLException {
197211
.withState(state)
198212
.withVerifier(verifier)
199213
.withHttpClient(hc)
214+
.withBrowserTimeout(browserTimeout)
200215
.build();
201216
}
202217
}

databricks-sdk-java/src/test/java/com/databricks/sdk/core/DatabricksConfigTest.java

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
import com.databricks.sdk.core.oauth.TokenSource;
1313
import com.databricks.sdk.core.utils.Environment;
1414
import java.io.IOException;
15+
import java.time.Duration;
1516
import java.util.ArrayList;
1617
import java.util.HashMap;
1718
import java.util.List;
@@ -250,4 +251,35 @@ public void testGetTokenSourceWithOAuth() {
250251
assertFalse(tokenSource instanceof ErrorTokenSource);
251252
assertEquals(tokenSource.getToken().getAccessToken(), "test-token");
252253
}
254+
255+
@Test
256+
public void testOAuthBrowserAuthTimeout() {
257+
DatabricksConfig config = new DatabricksConfig();
258+
259+
assertNull(config.getOAuthBrowserAuthTimeout());
260+
261+
config.setOAuthBrowserAuthTimeout(Duration.ofSeconds(30));
262+
assertEquals(Duration.ofSeconds(30), config.getOAuthBrowserAuthTimeout());
263+
264+
config.setOAuthBrowserAuthTimeout(Duration.ofSeconds(60));
265+
assertEquals(Duration.ofSeconds(60), config.getOAuthBrowserAuthTimeout());
266+
267+
config.setOAuthBrowserAuthTimeout(Duration.ofSeconds(0));
268+
assertEquals(Duration.ZERO, config.getOAuthBrowserAuthTimeout());
269+
}
270+
271+
@Test
272+
public void testEnvironmentVariableLoading() {
273+
Map<String, String> env = new HashMap<>();
274+
env.put("DATABRICKS_OAUTH_BROWSER_AUTH_TIMEOUT", "30");
275+
env.put("DATABRICKS_DEBUG_TRUNCATE_BYTES", "100");
276+
env.put("DATABRICKS_RATE_LIMIT", "50");
277+
278+
DatabricksConfig config = new DatabricksConfig();
279+
config.resolve(new Environment(env, new ArrayList<>(), System.getProperty("os.name")));
280+
281+
assertEquals(Duration.ofSeconds(30), config.getOAuthBrowserAuthTimeout());
282+
assertEquals(Integer.valueOf(100), config.getDebugTruncateBytes());
283+
assertEquals(Integer.valueOf(50), config.getRateLimit());
284+
}
253285
}

0 commit comments

Comments
 (0)