Skip to content

Commit 266bc8e

Browse files
Add u2m timeout
1 parent 1ef0e36 commit 266bc8e

File tree

7 files changed

+191
-6
lines changed

7 files changed

+191
-6
lines changed

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: 22 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.
169+
* If set 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
}
@@ -588,6 +596,20 @@ public DatabricksConfig setDisableAsyncTokenRefresh(boolean disableAsyncTokenRef
588596
return this;
589597
}
590598

599+
public Duration getOAuthBrowserAuthTimeout() {
600+
return oauthBrowserAuthTimeout;
601+
}
602+
603+
public DatabricksConfig setOAuthBrowserAuthTimeout(Duration oauthBrowserAuthTimeout) {
604+
this.oauthBrowserAuthTimeout = oauthBrowserAuthTimeout;
605+
return this;
606+
}
607+
608+
public DatabricksConfig setOAuthBrowserAuthTimeout(int seconds) {
609+
this.oauthBrowserAuthTimeout = seconds > 0 ? Duration.ofSeconds(seconds) : null;
610+
return this;
611+
}
612+
591613
public boolean isAzure() {
592614
if (azureWorkspaceResourceId != null) {
593615
return true;

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

Lines changed: 35 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,9 @@
1818
import java.util.HashMap;
1919
import java.util.Map;
2020
import java.util.Objects;
21+
import java.util.Optional;
22+
import java.time.Duration;
23+
2124
import org.apache.commons.io.IOUtils;
2225
import org.slf4j.Logger;
2326
import org.slf4j.LoggerFactory;
@@ -53,6 +56,7 @@ public class Consent implements Serializable {
5356
private final String redirectUrl;
5457
private final String clientId;
5558
private final String clientSecret;
59+
private final Optional<Duration> browserTimeout;
5660

5761
public static class Builder {
5862
private HttpClient hc = new CommonsHttpClient.Builder().withTimeoutSeconds(30).build();
@@ -63,6 +67,7 @@ public static class Builder {
6367
private String redirectUrl;
6468
private String clientId;
6569
private String clientSecret;
70+
private Optional<Duration> browserTimeout = Optional.empty();
6671

6772
public Builder withHttpClient(HttpClient hc) {
6873
this.hc = hc;
@@ -104,6 +109,15 @@ public Builder withClientSecret(String clientSecret) {
104109
return this;
105110
}
106111

112+
public Builder withBrowserTimeout(Optional<Duration> browserTimeout) {
113+
this.browserTimeout = browserTimeout;
114+
return this;
115+
}
116+
117+
public Builder withBrowserTimeout(Duration browserTimeout) {
118+
return withBrowserTimeout(Optional.of(browserTimeout));
119+
}
120+
107121
public Consent build() {
108122
return new Consent(this);
109123
}
@@ -119,6 +133,7 @@ private Consent(Builder builder) {
119133
this.clientId = Objects.requireNonNull(builder.clientId);
120134
// This may be null for native apps or single-page apps.
121135
this.clientSecret = builder.clientSecret;
136+
this.browserTimeout = builder.browserTimeout;
122137
}
123138

124139
public Consent setHttpClient(HttpClient hc) {
@@ -155,6 +170,10 @@ public String getClientSecret() {
155170
return clientSecret;
156171
}
157172

173+
public Optional<Duration> getBrowserTimeout() {
174+
return browserTimeout;
175+
}
176+
158177
/**
159178
* Launch a browser to collect an authorization code and exchange the code for an OAuth token.
160179
*
@@ -219,7 +238,8 @@ private Map<String, String> getOAuthCallbackParameters() throws IOException {
219238
+ redirect.getHost()
220239
+ ", redirectUrl host must be one of: localhost, 127.0.0.1");
221240
}
222-
CallbackResponseHandler handler = new CallbackResponseHandler();
241+
242+
CallbackResponseHandler handler = new CallbackResponseHandler(this.browserTimeout);
223243
HttpServer httpServer =
224244
HttpServer.create(new InetSocketAddress(redirect.getHost(), redirect.getPort()), 0);
225245
httpServer.createContext("/", handler);
@@ -285,6 +305,11 @@ static class CallbackResponseHandler implements HttpHandler {
285305
// Protects params
286306
private final Object lock = new Object();
287307
private volatile Map<String, String> params;
308+
private final Optional<Duration> timeout;
309+
310+
public CallbackResponseHandler(Optional<Duration> timeout) {
311+
this.timeout = timeout;
312+
}
288313

289314
@Override
290315
public void handle(HttpExchange exchange) {
@@ -373,7 +398,15 @@ public Map<String, String> getParams() {
373398
synchronized (lock) {
374399
if (params == null) {
375400
try {
376-
lock.wait();
401+
if (timeout.isPresent()) {
402+
Duration t = timeout.get();
403+
lock.wait(t.toMillis());
404+
if (params == null) {
405+
throw new DatabricksException("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);

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77
import java.nio.file.Path;
88
import java.util.Objects;
99
import java.util.Optional;
10+
import java.time.Duration;
1011
import org.slf4j.Logger;
1112
import org.slf4j.LoggerFactory;
1213

@@ -115,6 +116,7 @@ CachedTokenSource performBrowserAuth(
115116
.withAccountId(config.getAccountId())
116117
.withRedirectUrl(config.getEffectiveOAuthRedirectUrl())
117118
.withScopes(config.getScopes())
119+
.withBrowserTimeout(config.getOAuthBrowserAuthTimeout())
118120
.build();
119121
Consent consent = client.initiateConsent();
120122

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,14 @@
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.Arrays;
14+
import java.util.List;
15+
import java.util.Map;
16+
import java.util.Objects;
17+
import java.util.Optional;
18+
import java.util.Base64;
19+
import java.util.HashMap;
1320
import java.util.stream.Collectors;
1421

1522
/**
@@ -36,6 +43,7 @@ public static class Builder {
3643
private String clientSecret;
3744
private HttpClient hc;
3845
private String accountId;
46+
private Optional<Duration> browserTimeout = Optional.empty();
3947

4048
public Builder() {}
4149

@@ -77,6 +85,11 @@ public Builder withAccountId(String accountId) {
7785
this.accountId = accountId;
7886
return this;
7987
}
88+
89+
public Builder withBrowserTimeout(Duration browserTimeout) {
90+
this.browserTimeout = Optional.of(browserTimeout);
91+
return this;
92+
}
8093
}
8194

8295
private final String clientId;
@@ -90,6 +103,7 @@ public Builder withAccountId(String accountId) {
90103
private final SecureRandom random = new SecureRandom();
91104
private final boolean isAws;
92105
private final boolean isAzure;
106+
private final Optional<Duration> browserTimeout;
93107

94108
private OAuthClient(Builder b) throws IOException {
95109
this.clientId = Objects.requireNonNull(b.clientId);
@@ -120,6 +134,7 @@ private OAuthClient(Builder b) throws IOException {
120134
config.getEffectiveAzureLoginAppId() + "/user_impersonation", "offline_access");
121135
}
122136
this.scopes = scopes;
137+
this.browserTimeout = b.browserTimeout;
123138
}
124139

125140
public String getHost() {
@@ -207,6 +222,7 @@ public Consent initiateConsent() throws MalformedURLException {
207222
.withState(state)
208223
.withVerifier(verifier)
209224
.withHttpClient(hc)
225+
.withBrowserTimeout(browserTimeout.orElse(Duration.ofSeconds(0)))
210226
.build();
211227
}
212228
}

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

Lines changed: 59 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,62 @@ 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+
// Test default value (should be null)
260+
assertNull(config.getOAuthBrowserAuthTimeout());
261+
262+
// Test setting the timeout using Duration
263+
config.setOAuthBrowserAuthTimeout(Duration.ofSeconds(30));
264+
assertEquals(Duration.ofSeconds(30), config.getOAuthBrowserAuthTimeout());
265+
266+
// Test setting the timeout using int (backward compatibility)
267+
config.setOAuthBrowserAuthTimeout(60);
268+
assertEquals(Duration.ofSeconds(60), config.getOAuthBrowserAuthTimeout());
269+
270+
// Test setting to 0 (should be null for indefinite wait)
271+
config.setOAuthBrowserAuthTimeout(0);
272+
assertNull(config.getOAuthBrowserAuthTimeout());
273+
}
274+
275+
@Test
276+
public void testEnvironmentVariableLoading() {
277+
// Test that environment variables are now loaded properly for Duration and Integer
278+
DatabricksConfig config = new DatabricksConfig();
279+
280+
// Set environment variable
281+
Map<String, String> env = new HashMap<>();
282+
env.put("DATABRICKS_OAUTH_BROWSER_AUTH_TIMEOUT", "30");
283+
env.put("DATABRICKS_DEBUG_TRUNCATE_BYTES", "100");
284+
env.put("DATABRICKS_RATE_LIMIT", "50");
285+
286+
// Resolve config with environment
287+
Environment environment = new Environment(env, new ArrayList<>(), "Linux");
288+
config.resolve(environment);
289+
290+
// These should now be loaded properly
291+
assertEquals(Duration.ofSeconds(30), config.getOAuthBrowserAuthTimeout());
292+
assertEquals(Integer.valueOf(100), config.getDebugTruncateBytes());
293+
assertEquals(Integer.valueOf(50), config.getRateLimit());
294+
}
295+
296+
@Test
297+
public void testEnvironmentVariableLoadingZeroTimeout() {
298+
// Test that environment variable with value 0 results in null Duration
299+
DatabricksConfig config = new DatabricksConfig();
300+
301+
// Set environment variable to 0
302+
Map<String, String> env = new HashMap<>();
303+
env.put("DATABRICKS_OAUTH_BROWSER_AUTH_TIMEOUT", "0");
304+
305+
// Resolve config with environment
306+
Environment environment = new Environment(env, new ArrayList<>(), "Linux");
307+
config.resolve(environment);
308+
309+
// Should be null for indefinite wait
310+
assertNull(config.getOAuthBrowserAuthTimeout());
311+
}
253312
}
Lines changed: 43 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,43 @@
1+
package com.databricks.sdk.core.oauth;
2+
3+
import static org.junit.jupiter.api.Assertions.*;
4+
5+
import java.time.Duration;
6+
import java.util.Optional;
7+
import org.junit.jupiter.api.Test;
8+
9+
public class ConsentTest {
10+
11+
@Test
12+
public void testConsentWithBrowserAuthTimeout() {
13+
Consent consent = new Consent.Builder()
14+
.withClientId("test-client-id")
15+
.withClientSecret("test-client-secret")
16+
.withAuthUrl("https://test.com/auth")
17+
.withTokenUrl("https://test.com/token")
18+
.withRedirectUrl("http://localhost:8080/callback")
19+
.withState("test-state")
20+
.withVerifier("test-verifier")
21+
.withBrowserTimeout(Duration.ofSeconds(30))
22+
.build();
23+
24+
// Verify that the timeout is properly set
25+
assertEquals(Optional.of(Duration.ofSeconds(30)), consent.getBrowserTimeout());
26+
}
27+
28+
@Test
29+
public void testConsentWithoutBrowserAuthTimeout() {
30+
Consent consent = new Consent.Builder()
31+
.withClientId("test-client-id")
32+
.withClientSecret("test-client-secret")
33+
.withAuthUrl("https://test.com/auth")
34+
.withTokenUrl("https://test.com/token")
35+
.withRedirectUrl("http://localhost:8080/callback")
36+
.withState("test-state")
37+
.withVerifier("test-verifier")
38+
.build();
39+
40+
// Verify that the timeout is empty when not set
41+
assertEquals(Optional.empty(), consent.getBrowserTimeout());
42+
}
43+
}

0 commit comments

Comments
 (0)