Move database role to url parameters from connection properties on Snowflake (#43602)

* Move role to url parameters from connection properties

* Fix test

modules/drivers/snowflake/resources/metabase-plugin.yaml

@@ -42,7 +42,7 @@ driver:
       type: schema-filters
       display-name: Schemas
     - name: role
-      display-name: Role (required for connection impersonation)
+      display-name: Role (optional, required for connection impersonation)
       helper-text: Specify a role to override the database user’s default role.
       placeholder: user
     - cloud-ip-address-info

modules/drivers/snowflake/src/metabase/driver/snowflake.clj

@@ -125,6 +125,22 @@
   (when raw-name
     (str "\"" (str/replace raw-name "\"" "\"\"") "\"")))
 
+(defn- maybe-add-role-to-spec-url
+  "Maybe add role to `spec`'s `:connection-uri`. This is necessary for rsa auth to work, because at the time of writing
+  Snowflake jdbc driver ignores `:role` connection property when `:connection-uri` (presumably containing
+  private_key_file) is used."
+  [spec details]
+  (if (and (string? (not-empty (:connection-uri spec)))
+           (string? (not-empty (:role details)))
+           (not (str/includes? (:connection-uri spec) "role=")))
+    (let [role-opts-str (sql-jdbc.common/additional-opts->string :url {:role (codec/url-encode (:role details))})]
+      (-> spec
+          ;; It is advised to use either connection property or url parameter, not both. Eg. in the following link.
+          ;; https://docs.oracle.com/javase/8/docs/api/java/sql/DriverManager.html#getConnection-java.lang.String-java.util.Properties-
+          (dissoc :role)
+          (update :connection-uri sql-jdbc.common/conn-str-with-additional-opts :url role-opts-str)))
+    spec))
+
 (defmethod sql-jdbc.conn/connection-details->spec :snowflake
   [_ {:keys [account additional-options host use-hostname], :as details}]
   (when (get "week_start" (sql-jdbc.common/additional-options->map additional-options :url))
@@ -165,7 +181,10 @@
                    (update :schema upcase-not-nil)
                    resolve-private-key
                    (dissoc :host :port :timezone)))
-        (sql-jdbc.common/handle-additional-options details))))
+        (sql-jdbc.common/handle-additional-options details)
+        ;; Role is not respected when used as connection property if connection string is present with private key
+        ;; file. Hence it is moved to connection url. https://github.com/metabase/metabase/issues/43600
+        (maybe-add-role-to-spec-url details))))
 
 (defmethod sql-jdbc.sync/database-type->base-type :snowflake
   [_ base-type]

modules/drivers/snowflake/test/metabase/driver/snowflake_test.clj

@@ -422,7 +422,14 @@
                        (assoc :dbname pk-db
                               :private-key-value (u/encode-base64 pk-key)
                               :user pk-user))]
-       (is (driver/can-connect? :snowflake details))))))
+       (testing "Can connect with base64 encoded `:private-key-value` and no `:private-key-options` set (#41852)"
+         (is (driver/can-connect? :snowflake details)))
+       ;; Following is required when private key and role are used together. See
+       ;; the [[metabase.driver.snowflake/maybe-add-role-to-spec-url]] for the details.
+       (testing "Role is added to connection url, if url is present (#43600)"
+         (is (str/includes?
+              (:connection-uri (sql-jdbc.conn/connection-details->spec :snowflake (assoc details :role "SOME_ROLE")))
+              "role=SOME_ROLE")))))))
 
 (deftest ^:parallel replacement-snippet-date-param-test
   (mt/test-driver :snowflake