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.

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
11 changes: 11 additions & 0 deletions src/main/java/net/snowflake/client/core/SFLoginInput.java
Original file line number Diff line number Diff line change
Expand Up @@ -51,11 +51,22 @@ public class SFLoginInput {
private boolean disableConsoleLogin = true;
private boolean disableSamlURLCheck = false;

private Duration browserResponseTimeout;

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

SFLoginInput() {}

Duration getBrowserResponseTimeout() {
return browserResponseTimeout;
}

SFLoginInput setBrowserResponseTimeout(Duration browserResponseTimeout) {
this.browserResponseTimeout = browserResponseTimeout;
return this;
}

public String getServerUrl() {
return serverUrl;
}
Expand Down
21 changes: 18 additions & 3 deletions src/main/java/net/snowflake/client/core/SFSession.java
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 Duration browserResponseTimeout = Duration.ofSeconds(120);

// 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 @@ -489,6 +496,12 @@ public void addSFSessionProperty(String propertyName, Object propertyValue) thro
}
break;

case BROWSER_RESPONSE_TIMEOUT:
if (propertyValue != null) {
browserResponseTimeout = Duration.ofSeconds((Integer) propertyValue);
}
sfc-gh-dprzybysz marked this conversation as resolved.
Show resolved Hide resolved
break;

case JDBC_DEFAULT_FORMAT_DATE_WITH_TIMEZONE:
if (propertyValue != null) {
setDefaultFormatDateWithTimezone(getBooleanValue(propertyValue));
Expand Down Expand Up @@ -542,7 +555,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 @@ -574,7 +587,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 @@ -632,7 +646,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);

logger.info(
"Connecting to {} Snowflake domain",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,9 @@ public enum SFSessionProperty {
"JDBC_DEFAULT_FORMAT_DATE_WITH_TIMEZONE", false, Boolean.class),

// Used as a fix for issue SNOW-354859. Remove with snowflake-jdbc version 4.x with BCR changes.
JDBC_GET_DATE_USE_NULL_TIMEZONE("JDBC_GET_DATE_USE_NULL_TIMEZONE", false, Boolean.class);
JDBC_GET_DATE_USE_NULL_TIMEZONE("JDBC_GET_DATE_USE_NULL_TIMEZONE", 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
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import java.net.InetAddress;
import java.net.ServerSocket;
import java.net.Socket;
import java.net.SocketTimeoutException;
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.Charset;
Expand Down Expand Up @@ -256,6 +257,10 @@ private String generateProofKey() {
return Base64.getEncoder().encodeToString(randomness);
}

private int getBrowserResponseTimeout() {
return (int) loginInput.getBrowserResponseTimeout().toMillis();
}

/**
* Authenticate
*
Expand All @@ -265,6 +270,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 Expand Up @@ -305,6 +311,13 @@ void authenticate() throws SFException, SnowflakeSQLException {
socket.close();
}
}
} catch (SocketTimeoutException e) {
throw new SFException(
e,
ErrorCode.NETWORK_ERROR,
"External browser authentication failed within timeout of "
+ getBrowserResponseTimeout()
+ " milliseconds");
} catch (IOException ex) {
throw new SFException(ex, ErrorCode.NETWORK_ERROR, ex.getMessage());
} finally {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,11 @@ public class SnowflakeBasicDataSource implements DataSource, Serializable {
private static final long serialversionUID = 1L;
private static final String AUTHENTICATOR_SNOWFLAKE_JWT = "SNOWFLAKE_JWT";
private static final String AUTHENTICATOR_OAUTH = "OAUTH";

private static final String AUTHENTICATOR_EXTERNAL_BROWSER = "EXTERNALBROWSER";

private static final String AUTHENTICATOR_USERNAME_PASSWORD_MFA = "USERNAME_PASSWORD_MFA";

private String url;

private String serverName;
Expand Down Expand Up @@ -94,7 +98,8 @@ public Connection getConnection(String username, String password) throws SQLExce
}

// The driver needs password for OAUTH as part of SNOW-533673 feature request.
if (!AUTHENTICATOR_SNOWFLAKE_JWT.equalsIgnoreCase(authenticator)) {
if (!AUTHENTICATOR_SNOWFLAKE_JWT.equalsIgnoreCase(authenticator)
&& !AUTHENTICATOR_EXTERNAL_BROWSER.equalsIgnoreCase(authenticator)) {
properties.put(SFSessionProperty.PASSWORD.getPropertyKey(), password);
}

Expand Down Expand Up @@ -380,4 +385,9 @@ public void setJDBCDefaultFormatDateWithTimezone(Boolean jdbcDefaultFormatDateWi
public void setGetDateUseNullTimezone(Boolean getDateUseNullTimezone) {
this.properties.put("JDBC_GET_DATE_USE_NULL_TIMEZONE", getDateUseNullTimezone);
}

public void setBrowserResponseTimeout(int seconds) {
this.setAuthenticator(AUTHENTICATOR_EXTERNAL_BROWSER);
this.properties.put("BROWSER_RESPONSE_TIMEOUT", Integer.toString(seconds));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,15 @@
import java.net.URI;
import java.net.URISyntaxException;
import java.nio.charset.StandardCharsets;
import java.util.Map;
import net.snowflake.client.AbstractDriverIT;
import net.snowflake.client.jdbc.SnowflakeBasicDataSource;
import net.snowflake.client.jdbc.SnowflakeSQLException;
import net.snowflake.client.jdbc.SnowflakeSQLLoggedException;
import net.snowflake.common.core.ClientAuthnDTO;
import org.apache.http.client.methods.HttpPost;
import org.apache.http.client.methods.HttpRequestBase;
import org.junit.Ignore;
import org.junit.Test;
import org.mockito.MockedStatic;
import org.mockito.Mockito;
Expand Down Expand Up @@ -237,4 +242,25 @@ private SFLoginInput initMockLoginInput() {
when(loginInput.getDisableConsoleLogin()).thenReturn(true);
return loginInput;
}

// Run this test manually to confirm external browser timeout is working. When test runs it will
// open a browser window for authentication, close the window, and you should get the expected
// error message within the set timeout. Valid for driver versions after 3.18.0.
@Test
@Ignore
public void testExternalBrowserTimeout() throws Exception {
Map<String, String> params = AbstractDriverIT.getConnectionParameters();
SnowflakeBasicDataSource ds = new SnowflakeBasicDataSource();
ds.setServerName(params.get("host"));
ds.setAccount(params.get("account"));
ds.setPortNumber(Integer.parseInt(params.get("port")));
ds.setUser(params.get("user"));
ds.setBrowserResponseTimeout(10);
try {
ds.getConnection();
fail();
} catch (SnowflakeSQLLoggedException e) {
assertTrue(e.getMessage().contains("External browser authentication failed"));
}
}
}
Loading