Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SNOW-847342: Add connection property for setting browser timeout value #1824

Merged
merged 9 commits into from
Jul 31, 2024
Merged

SNOW-847342: Add connection property for setting browser timeout value #1824

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
a76367a
Add connection property for setting browser timeout value
sfc-gh-ext-simba-jf Jul 11, 2024
2f115c9
Code review changes
sfc-gh-ext-simba-jf Jul 18, 2024
d8d355f
code review changes
sfc-gh-ext-simba-jf Jul 24, 2024
046d8a6
Merge branch 'master' into Snow-847342-Add-Timeout-for-external-brows…
sfc-gh-ext-simba-jf Jul 24, 2024
ffd2089
check style fix
sfc-gh-ext-simba-jf Jul 24, 2024
01faa5b
Added manual test and better error messaging
sfc-gh-ext-simba-jf Jul 25, 2024
d3f4d0e
check style fix
sfc-gh-ext-simba-jf Jul 25, 2024
e7efe6d
Code review changes
sfc-gh-ext-simba-jf Jul 25, 2024
adcbf2b
Removed old comment
sfc-gh-ext-simba-jf Jul 30, 2024
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
10 changes: 10 additions & 0 deletions src/main/java/net/snowflake/client/core/SFLoginInput.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,21 @@ public class SFLoginInput {
private boolean disableConsoleLogin = true;
private boolean disableSamlURLCheck = false;

private int browserResponseTimeout = 120;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

// Additional headers to add for Snowsight.
Map<String, String> additionalHttpHeadersForSnowsight;

SFLoginInput() {}

public int getBrowserResponseTimeout() {
return browserResponseTimeout;
}

public void setBrowserResponseTimeout(int browserResponseTimeout) {
this.browserResponseTimeout = browserResponseTimeout;
}

public String getServerUrl() {
return serverUrl;
}
Expand Down
19 changes: 16 additions & 3 deletions src/main/java/net/snowflake/client/core/SFSession.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,13 @@ public class SFSession extends SFBaseSession {
*/
private int retryTimeout = 300;

/**
* Max timeout for external browser authentication in seconds
*
* <p>Default: 120
*/
private int browserResponseTimeout = 120;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

// This constructor is used only by tests with no real connection.
// For real connections, the other constructor is always used.
@VisibleForTesting
Expand Down Expand Up @@ -485,6 +492,10 @@ public void addSFSessionProperty(String propertyName, Object propertyValue) thro
setJdbcArrowTreatDecimalAsInt(getBooleanValue(propertyValue));
}
break;
case BROWSER_RESPONSE_TIMEOUT:
if (propertyValue != null) {
browserResponseTimeout = (Integer) propertyValue;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
}
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

default:
break;
Expand Down Expand Up @@ -527,7 +538,7 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
+ " application: {}, app id: {}, app version: {}, login timeout: {}, retry timeout: {}, network timeout: {},"
+ " query timeout: {}, tracing: {}, private key file: {}, private key file pwd is {},"
+ " enable_diagnostics: {}, diagnostics_allowlist_path: {},"
+ " session parameters: client store temporary credential: {}, gzip disabled: {}",
+ " session parameters: client store temporary credential: {}, gzip disabled: {}, browser response timeout: {}",
connectionPropertiesMap.get(SFSessionProperty.SERVER_URL),
connectionPropertiesMap.get(SFSessionProperty.ACCOUNT),
connectionPropertiesMap.get(SFSessionProperty.USER),
Expand Down Expand Up @@ -559,7 +570,8 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
connectionPropertiesMap.get(SFSessionProperty.ENABLE_DIAGNOSTICS),
connectionPropertiesMap.get(SFSessionProperty.DIAGNOSTICS_ALLOWLIST_FILE),
sessionParametersMap.get(CLIENT_STORE_TEMPORARY_CREDENTIAL),
connectionPropertiesMap.get(SFSessionProperty.GZIP_DISABLED));
connectionPropertiesMap.get(SFSessionProperty.GZIP_DISABLED),
connectionPropertiesMap.get(SFSessionProperty.BROWSER_RESPONSE_TIMEOUT));

HttpClientSettingsKey httpClientSettingsKey = getHttpClientKey();
logger.debug(
Expand Down Expand Up @@ -617,7 +629,8 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
connectionPropertiesMap.get(SFSessionProperty.DISABLE_SAML_URL_CHECK) != null
? getBooleanValue(
connectionPropertiesMap.get(SFSessionProperty.DISABLE_SAML_URL_CHECK))
: false);
: false)
.setBrowserResponseTimeout(browserResponseTimeout);

// Enable or disable OOB telemetry based on connection parameter. Default is disabled.
// The value may still change later when session parameters from the server are read.
Expand Down
4 changes: 3 additions & 1 deletion src/main/java/net/snowflake/client/core/SFSessionProperty.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,9 @@ public enum SFSessionProperty {
DISABLE_GCS_DEFAULT_CREDENTIALS("disableGcsDefaultCredentials", false, Boolean.class),

JDBC_ARROW_TREAT_DECIMAL_AS_INT("JDBC_ARROW_TREAT_DECIMAL_AS_INT", false, Boolean.class),
DISABLE_SAML_URL_CHECK("disableSamlURLCheck", false, Boolean.class);
DISABLE_SAML_URL_CHECK("disableSamlURLCheck", false, Boolean.class),

BROWSER_RESPONSE_TIMEOUT("BROWSER_RESPONSE_TIMEOUT", false, Integer.class);
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved

// property key in string
private String propertyKey;
Expand Down
5 changes: 5 additions & 0 deletions src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -256,6 +256,10 @@ private String generateProofKey() {
return Base64.getEncoder().encodeToString(randomness);
}

private int getBrowserResponseTimeout() {
return loginInput.getBrowserResponseTimeout() * 1000;
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
}

/**
* Authenticate
*
Expand All @@ -265,6 +269,7 @@ private String generateProofKey() {
void authenticate() throws SFException, SnowflakeSQLException {
ServerSocket ssocket = this.getServerSocket();
try {
ssocket.setSoTimeout(getBrowserResponseTimeout());
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
// main procedure
int port = this.getLocalPort(ssocket);
logger.debug("Listening localhost: {}", port);
Expand Down
Loading