Properly detect when Snowflake is using a local key file when expanding...

Properly detect when Snowflake is using a local key file when expanding connection properties (#46348)

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

@@ -97,30 +97,28 @@
   Setting the Snowflake driver property privatekey would be easier, but that doesn't work
   because clojure.java.jdbc (properly) converts the property values into strings while the
   Snowflake driver expects a java.security.PrivateKey instance."
-  [{:keys [user password account private-key-path]
+  [{:keys [user password account private-key-options]
     :as   details}]
-  (let [base-details (apply dissoc details (vals (secret/->sub-props "private-key")))]
-    (cond
-      password
-      details
-
-      private-key-path
-      (let [secret-map       (secret/db-details-prop->secret-map details "private-key")
-            private-key-file (when (some? (:value secret-map))
-                               (secret/value->file! secret-map :snowflake))]
-        (cond-> base-details
-          private-key-file (handle-conn-uri user account private-key-file)))
-
-      :else
-      ;; Why setting `:private-key-options` to "uploaded"? To fix the issue #41852. Snowflake's database edit gui
-      ;; is designed in a way, that `:private-key-options` are not sent if `hosting` enterprise feature is enabled.
-      ;; The option must be set to "uploaded" for base64 decoding to happen. Setting that option at this point is fine
-      ;; because the alternative ("local") is ruled out already in this `cond` branch.
-      (let [decoded (secret/get-secret-string (assoc details :private-key-options "uploaded") "private-key")
-            file    (secret/value->file! {:connection-property-name "private-key-file"
-                                          :value                    decoded})]
-        (assoc (handle-conn-uri base-details user account file)
-               :private_key_file file)))))
+  (if password
+    details
+    (let [base-details (apply dissoc details (vals (secret/->sub-props "private-key")))
+          secret-map   (secret/db-details-prop->secret-map details "private-key")
+          private-key-file (cond
+                             ;; Local file path
+                             (= private-key-options "local")
+                             (secret/value->file! secret-map :snowflake)
+
+                             ;; Uploaded file stored in `secrets` table
+                             :else
+                             ;; Why setting `:private-key-options` to "uploaded"? To fix the issue #41852. Snowflake's database edit gui
+                             ;; is designed in a way, that `:private-key-options` are not sent if `hosting` enterprise feature is enabled.
+                             ;; The option must be set to "uploaded" for base64 decoding to happen. Setting that option at this point is fine
+                             ;; because the alternative ("local") is ruled out already in this `cond` branch.
+                             (let [decoded (secret/get-secret-string (assoc details :private-key-options "uploaded") "private-key")]
+                               (secret/value->file! {:connection-property-name "private-key-file"
+                                                     :value                    decoded})))]
+      (assoc (handle-conn-uri base-details user account private-key-file)
+             :private_key_file private-key-file))))
 
 (defn- quote-name
   [raw-name]

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

@@ -396,15 +396,24 @@
 
         (when (and pk-key pk-user)
           (mt/with-temp-file [pk-path]
-            (testing "private key authentication"
-              (spit pk-path pk-key)
-              (doseq [to-merge [{:private-key-value pk-key} ;; uploaded string
-                                {:private-key-value (.getBytes pk-key "UTF-8")} ;; uploaded byte array
-                                {:private-key-path pk-path}]] ;; local file path
-                (let [details (-> (:details (mt/db))
-                                  (dissoc :password)
-                                  (merge {:db pk-db :user pk-user} to-merge))]
-                  (is (can-connect? details)))))))))))
+            (mt/with-temp [:model/Secret {secret-id :id} {:name   "Private key for Snowflake"
+                                                          :kind   :pem-cert
+                                                          :source "file-path"
+                                                          :value  pk-path}]
+              (testing "private key authentication via uploaded keys or local key with path stored in a secret"
+                (spit pk-path pk-key)
+                (doseq [to-merge [{:private-key-value pk-key                      ;; uploaded string
+                                   :private-key-options "uploaded"}
+                                  {:private-key-value (.getBytes pk-key "UTF-8")
+                                   :private-key-options "uploaded"}               ;; uploaded byte array
+                                  {:private-key-value (.getBytes pk-key "UTF-8")} ;; uploaded byte array without private-key-options
+                                  {:private-key-options "local"
+                                   :private-key-source "file-path"
+                                   :private-key-id secret-id}]]              ;; local file path
+                  (let [details (-> (:details (mt/db))
+                                    (dissoc :password)
+                                    (merge {:db pk-db :user pk-user} to-merge))]
+                    (is (can-connect? details))))))))))))
 
 (deftest ^:synchronized pk-auth-custom-role-e2e-test
   (mt/test-driver