CR suggestions applied

snowflakedb · Dec 5, 2024 · 9365dbf · 9365dbf
1 parent 77a9ce0
commit 9365dbf
Show file tree

Hide file tree

Showing 10 changed files with 124 additions and 111 deletions.
diff --git a/ci/scripts/check_content.sh b/ci/scripts/check_content.sh
@@ -8,7 +8,7 @@ set -o pipefail
 
 DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" >/dev/null && pwd )"
 
-if jar tvf snowflake-jdbc.jar | awk '{print $8}' | grep -v -E "/$" | grep -v -E "^(net|com)/snowflake" | grep -v -E "(com|net)/\$" | grep -v -E "^META-INF" | grep -v -E "^iso3166_" | grep -v -E "^mozilla" | grep -v -E "^com/sun/jna" | grep -v com/sun/ | grep -v mime.types | grep -v -E "^com/github/luben/zstd/" | grep -v -E "^aix/" | grep -v -E "^darwin/" | grep -v -E "^freebsd/" | grep -v -E "^linux/" | grep -v -E "^win/"; then
+if jar $DIR/../../target/snowflake-jdbc${package_modifier}.jar | awk '{print $8}' | grep -v -E "/$" | grep -v -E "^(net|com)/snowflake" | grep -v -E "(com|net)/\$" | grep -v -E "^META-INF" | grep -v -E "^iso3166_" | grep -v -E "^mozilla" | grep -v -E "^com/sun/jna" | grep -v com/sun/ | grep -v mime.types | grep -v -E "^com/github/luben/zstd/" | grep -v -E "^aix/" | grep -v -E "^darwin/" | grep -v -E "^freebsd/" | grep -v -E "^linux/" | grep -v -E "^win/"; then
   echo "[ERROR] JDBC jar includes class not under the snowflake namespace"
   exit 1
 fi

diff --git a/linkage-checker-exclusion-rules.xml b/linkage-checker-exclusion-rules.xml
@@ -19,11 +19,12 @@
         <Source><Package name="com.google.api.client.extensions.appengine"/></Source>
         <Reason>provided appengine</Reason>
     </LinkageError>
-<!--    <LinkageError>-->
-<!--        <Target><Package name="com.oracle"/></Target>-->
-<!--        <Source><Package name="com.google.api.gax"/></Source>-->
-<!--        <Reason>?</Reason>-->
-<!--    </LinkageError>-->
+    <LinkageError>
+        <Target><Package name="com.oracle"/></Target>
+        <Source><Package name="com.google.api.gax"/></Source>
+        <Reason>?</Reason>
+    </LinkageError>
+
     <LinkageError>
         <Target><Package name="org.osgi"/></Target>
         <Source><Package name="org.apache.tika.config"/></Source>
@@ -59,6 +60,11 @@
         <Source><Package name="com.nimbusds"/></Source>
         <Reason>?</Reason>
     </LinkageError>
+    <LinkageError>
+        <Target><Package name="net.shibboleth.utilities"/></Target>
+        <Source><Package name="com.nimbusds"/></Source>
+        <Reason>?</Reason>
+    </LinkageError>
     <!--
     <LinkageError>
         <Target><Package name=""/></Target>

diff --git a/src/main/java/net/snowflake/client/core/SFLoginInput.java b/src/main/java/net/snowflake/client/core/SFLoginInput.java
@@ -49,24 +49,19 @@ public class SFLoginInput {
   private String privateKeyPwd;
   private String inFlightCtx; // Opaque string sent for Snowsight account activation
 
+  private SFOauthLoginInput oauthLoginInput;
+
   private boolean disableConsoleLogin = true;
   private boolean disableSamlURLCheck = false;
   private boolean enableClientStoreTemporaryCredential;
   private boolean enableClientRequestMfaToken;
 
-  // OAuth
-  private String redirectUri;
-  private String clientId;
-  private String clientSecret;
-  private String externalAuthorizationUrl;
-  private String externalTokenRequestUrl;
-  private String scope;
-
   private Duration browserResponseTimeout;
 
   // Additional headers to add for Snowsight.
   Map<String, String> additionalHttpHeadersForSnowsight;
 
+  @SnowflakeJdbcInternalApi
   public SFLoginInput() {}
 
   Duration getBrowserResponseTimeout() {
@@ -82,6 +77,7 @@ public String getServerUrl() {
     return serverUrl;
   }
 
+  @SnowflakeJdbcInternalApi
   public SFLoginInput setServerUrl(String serverUrl) {
     this.serverUrl = serverUrl;
     return this;
@@ -168,6 +164,7 @@ public SFLoginInput setAccountName(String accountName) {
     return this;
   }
 
+  @SnowflakeJdbcInternalApi
   public int getLoginTimeout() {
     return loginTimeout;
   }
@@ -246,10 +243,12 @@ SFLoginInput setConnectionTimeout(Duration connectionTimeout) {
     return this;
   }
 
+  @SnowflakeJdbcInternalApi
   public int getSocketTimeoutInMillis() {
     return (int) socketTimeout.toMillis();
   }
 
+  @SnowflakeJdbcInternalApi
   public SFLoginInput setSocketTimeout(Duration socketTimeout) {
     this.socketTimeout = socketTimeout;
     return this;
@@ -396,10 +395,12 @@ SFLoginInput setOCSPMode(OCSPMode ocspMode) {
     return this;
   }
 
+  @SnowflakeJdbcInternalApi
   public HttpClientSettingsKey getHttpClientSettingsKey() {
     return httpClientKey;
   }
 
+  @SnowflakeJdbcInternalApi
   public SFLoginInput setHttpClientSettingsKey(HttpClientSettingsKey key) {
     this.httpClientKey = key;
     return this;
@@ -425,60 +426,6 @@ SFLoginInput setDisableSamlURLCheck(boolean disableSamlURLCheck) {
     return this;
   }
 
-  public String getRedirectUri() {
-    return redirectUri;
-  }
-
-  public SFLoginInput setRedirectUri(String redirectUri) {
-    this.redirectUri = redirectUri;
-    return this;
-  }
-
-  public String getClientId() {
-    return clientId;
-  }
-
-  public SFLoginInput setClientId(String clientId) {
-    this.clientId = clientId;
-    return this;
-  }
-
-  public String getClientSecret() {
-    return clientSecret;
-  }
-
-  public SFLoginInput setClientSecret(String clientSecret) {
-    this.clientSecret = clientSecret;
-    return this;
-  }
-
-  public String getExternalAuthorizationUrl() {
-    return externalAuthorizationUrl;
-  }
-
-  public SFLoginInput setExternalAuthorizationUrl(String externalAuthorizationUrl) {
-    this.externalAuthorizationUrl = externalAuthorizationUrl;
-    return this;
-  }
-
-  public String getExternalTokenRequestUrl() {
-    return externalTokenRequestUrl;
-  }
-
-  public SFLoginInput setExternalTokenRequestUrl(String externalTokenRequestUrl) {
-    this.externalTokenRequestUrl = externalTokenRequestUrl;
-    return this;
-  }
-
-  public String getScope() {
-    return scope;
-  }
-
-  public SFLoginInput setScope(String scope) {
-    this.scope = scope;
-    return this;
-  }
-
   Map<String, String> getAdditionalHttpHeadersForSnowsight() {
     return additionalHttpHeadersForSnowsight;
   }
@@ -547,4 +494,13 @@ SFLoginInput setEnableClientRequestMfaToken(boolean enableClientRequestMfaToken)
     this.enableClientRequestMfaToken = enableClientRequestMfaToken;
     return this;
   }
+
+  public SFOauthLoginInput getOauthLoginInput() {
+    return oauthLoginInput;
+  }
+
+  public SFLoginInput setOauthLoginInput(SFOauthLoginInput oauthLoginInput) {
+    this.oauthLoginInput = oauthLoginInput;
+    return this;
+  }
 }
diff --git a/src/main/java/net/snowflake/client/core/SFOauthLoginInput.java b/src/main/java/net/snowflake/client/core/SFOauthLoginInput.java
@@ -0,0 +1,45 @@
+package net.snowflake.client.core;
+
+@SnowflakeJdbcInternalApi
+public class SFOauthLoginInput {
+
+    private final String clientId;
+    private final String clientSecret;
+    private final String redirectUri;
+    private final String externalAuthorizationUrl;
+    private final String externalTokenRequestUrl;
+    private final String scope;
+
+    public SFOauthLoginInput(String clientId, String clientSecret, String redirectUri, String externalAuthorizationUrl, String externalTokenRequestUrl, String scope) {
+        this.redirectUri = redirectUri;
+        this.clientId = clientId;
+        this.clientSecret = clientSecret;
+        this.externalAuthorizationUrl = externalAuthorizationUrl;
+        this.externalTokenRequestUrl = externalTokenRequestUrl;
+        this.scope = scope;
+    }
+
+    public String getRedirectUri() {
+        return redirectUri;
+    }
+
+    public String getClientId() {
+        return clientId;
+    }
+
+    public String getClientSecret() {
+        return clientSecret;
+    }
+
+    public String getExternalAuthorizationUrl() {
+        return externalAuthorizationUrl;
+    }
+
+    public String getExternalTokenRequestUrl() {
+        return externalTokenRequestUrl;
+    }
+
+    public String getScope() {
+        return scope;
+    }
+}
diff --git a/src/main/java/net/snowflake/client/core/SFSession.java b/src/main/java/net/snowflake/client/core/SFSession.java
@@ -640,6 +640,14 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
 
     // TODO: temporarily hardcode sessionParameter debug info. will be changed in the future
     SFLoginInput loginInput = new SFLoginInput();
+    SFOauthLoginInput oauthLoginInput = new SFOauthLoginInput(
+        (String) connectionPropertiesMap.get(SFSessionProperty.CLIENT_ID),
+        (String) connectionPropertiesMap.get(SFSessionProperty.CLIENT_SECRET),
+        (String) connectionPropertiesMap.get(SFSessionProperty.OAUTH_REDIRECT_URI),
+        (String) connectionPropertiesMap.get(SFSessionProperty.OAUTH_SCOPE),
+        (String) connectionPropertiesMap.get(SFSessionProperty.EXTERNAL_AUTHORIZATION_URL),
+        (String) connectionPropertiesMap.get(SFSessionProperty.EXTERNAL_TOKEN_REQUEST_URL)
+    );
 
     loginInput
         .setServerUrl((String) connectionPropertiesMap.get(SFSessionProperty.SERVER_URL))
@@ -677,14 +685,7 @@ public synchronized void open() throws SFException, SnowflakeSQLException {
         .setSessionParameters(sessionParametersMap)
         .setPrivateKey((PrivateKey) connectionPropertiesMap.get(SFSessionProperty.PRIVATE_KEY))
         .setPrivateKeyFile((String) connectionPropertiesMap.get(SFSessionProperty.PRIVATE_KEY_FILE))
-        .setClientId((String) connectionPropertiesMap.get(SFSessionProperty.CLIENT_ID))
-        .setClientSecret((String) connectionPropertiesMap.get(SFSessionProperty.CLIENT_SECRET))
-        .setRedirectUri((String) connectionPropertiesMap.get(SFSessionProperty.OAUTH_REDIRECT_URI))
-        .setScope((String) connectionPropertiesMap.get(SFSessionProperty.OAUTH_SCOPE))
-        .setExternalAuthorizationUrl(
-            (String) connectionPropertiesMap.get(SFSessionProperty.EXTERNAL_AUTHORIZATION_URL))
-        .setExternalTokenRequestUrl(
-            (String) connectionPropertiesMap.get(SFSessionProperty.EXTERNAL_TOKEN_REQUEST_URL))
+            .setOauthLoginInput(oauthLoginInput)
         .setPrivateKeyBase64(
             (String) connectionPropertiesMap.get(SFSessionProperty.PRIVATE_KEY_BASE64))
         .setPrivateKeyPwd(

diff --git a/src/main/java/net/snowflake/client/core/SessionUtil.java b/src/main/java/net/snowflake/client/core/SessionUtil.java
@@ -276,15 +276,15 @@ static SFLoginOutput openSession(
 
     if (getAuthenticator(loginInput).equals(AuthenticatorType.OAUTH_AUTHORIZATION_CODE)) {
       AssertUtil.assertTrue(
-          loginInput.getClientId() != null,
+          loginInput.getOauthLoginInput().getClientId() != null,
           "passing clientId is required for OAUTH_AUTHORIZATION_CODE_FLOW authentication");
       AssertUtil.assertTrue(
-          loginInput.getClientSecret() != null,
+          loginInput.getOauthLoginInput().getClientSecret() != null,
           "passing clientSecret is required for OAUTH_AUTHORIZATION_CODE_FLOW authentication");
       OauthAccessTokenProvider accessTokenProvider =
           new AuthorizationCodeFlowAccessTokenProvider(
               new SessionUtilExternalBrowser.DefaultAuthExternalBrowserHandlers(),
-              DEFAULT_BROWSER_AUTHORIZATION_TIMEOUT_SECONDS);
+                  (int) loginInput.getBrowserResponseTimeout().getSeconds());
       String oauthAccessToken = accessTokenProvider.getAccessToken(loginInput);
       loginInput.setAuthenticator(AuthenticatorType.OAUTH.name());
       loginInput.setToken(oauthAccessToken);

diff --git a/src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java b/src/main/java/net/snowflake/client/core/SessionUtilExternalBrowser.java
@@ -61,6 +61,7 @@ public interface AuthExternalBrowserHandlers {
     void output(String msg);
   }
 
+  @SnowflakeJdbcInternalApi
   public static class DefaultAuthExternalBrowserHandlers implements AuthExternalBrowserHandlers {
     @Override
     public HttpPost build(URI uri) {

diff --git a/src/main/java/net/snowflake/client/core/auth/ClientAuthnDTO.java b/src/main/java/net/snowflake/client/core/auth/ClientAuthnDTO.java
@@ -24,12 +24,12 @@ public ClientAuthnDTO(Map<String, Object> data, @Nullable String inFlightCtx) {
     this.inFlightCtx = inFlightCtx;
   }
 
-  // Required by Jackson
+  /** Required by Jackson */
   public Map<String, Object> getData() {
     return data;
   }
 
-  // Required by Jackson
+  /** Required by Jackson */
   public String getInFlightCtx() {
     return inFlightCtx;
   }