diff --git a/src/metabase/models/secret.clj b/src/metabase/models/secret.clj
index bda70deb5847fa936c44159d48b79c3d5acb9056..b75121da4d2c57b798cf0c1fdeb60756c501838f 100644
--- a/src/metabase/models/secret.clj
+++ b/src/metabase/models/secret.clj
@@ -1,5 +1,6 @@
(ns metabase.models.secret
(:require [cheshire.generate :refer [add-encoder encode-map]]
+ [clojure.core.memoize :as memoize]
[clojure.java.io :as io]
[clojure.string :as str]
[clojure.tools.logging :as log]
@@ -54,7 +55,7 @@
(->> (filter #(= :secret (keyword (:type %))) conn-props)
(reduce (fn [acc prop] (assoc acc (:name prop) prop)) {})))
-(defn value->file!
+(defn value->file!*
"Returns the value of the given `secret` instance in the form of a file. If the given instance has a `:file-path` as
its source, a `File` referring to that is returned. Otherwise, the `:value` is written to a temporary file, which is
then returned.
@@ -100,6 +101,24 @@
(.write out v)))
tmp-file)))
+(def
+ ^java.io.File
+ ^{:arglists '([{:keys [connection-property-name id value] :as secret} driver?])}
+ value->file!
+ "Returns the value of the given `secret` instance in the form of a file. If the given instance has a `:file-path` as
+ its source, a `File` referring to that is returned. Otherwise, the `:value` is written to a temporary file, which is
+ then returned.
+
+ `driver?` is an optional argument that is only used if an ostensibly existing file value (i.e. `:file-path`) can't be
+ resolved, in order to render a more user-friendly error message (by looking up the display names of the connection
+ properties involved)."
+ (memoize/memo
+ (with-meta value->file!*
+ {::memoize/args-fn (fn [[secret _driver?]]
+ ;; not clear if value->string could return nil due to the cond so we'll just cache on a key
+ ;; that is unique
+ [(vec (:value secret))])})))
+
(defn get-sub-props
"Return a map of secret subproperties for the property `connection-property-name`."
[connection-property-name]
diff --git a/test/metabase/driver/sql_jdbc/connection_test.clj b/test/metabase/driver/sql_jdbc/connection_test.clj
index 678783f67df78b7a8878b6d671556e2a7d19c85e..56a25a4b82745cec7775044ed9347adeee6cfed1 100644
--- a/test/metabase/driver/sql_jdbc/connection_test.clj
+++ b/test/metabase/driver/sql_jdbc/connection_test.clj
@@ -6,7 +6,7 @@
[metabase.driver.sql-jdbc.connection :as sql-jdbc.conn]
[metabase.driver.sql-jdbc.test-util :as sql-jdbc.tu]
[metabase.driver.util :as driver.u]
- [metabase.models.database :refer [Database]]
+ [metabase.models :refer [Database Secret]]
[metabase.sync :as sync]
[metabase.test :as mt]
[metabase.test.data :as data]
@@ -153,4 +153,24 @@
(is (not= db-hash-1 db-hash-2)))))))
(finally
;; restore the original test DB details, no matter what just happened
- (db/update! Database (mt/id) :details (:details db))))))))
+ (db/update! Database (mt/id) :details (:details db)))))))
+ (testing "postgres secrets are stable (#23034)"
+ (mt/with-temp* [Secret [secret {:name "file based secret"
+ :kind :perm-cert
+ :source nil
+ :value (.getBytes "super secret")}]]
+ (let [db {:engine :postgres
+ :details {:ssl true
+ :ssl-mode "verify-ca"
+ :ssl-root-cert-options "uploaded"
+ :ssl-root-cert-creator-id (mt/user->id :crowberto)
+ :ssl-root-cert-source nil
+ :ssl-root-cert-id (:id secret)
+ :ssl-root-cert-created-at "2022-07-25T15:57:51.556-05:00"}}]
+ (is (instance? java.io.File
+ (:sslrootcert (#'sql-jdbc.conn/connection-details->spec :postgres
+ (:details db))))
+ "Secrets not loaded for db connections")
+ (is (= (#'sql-jdbc.conn/jdbc-spec-hash db)
+ (#'sql-jdbc.conn/jdbc-spec-hash db))
+ "Same db produced different hashes due to secrets")))))
diff --git a/test/metabase/models/secret_test.clj b/test/metabase/models/secret_test.clj
index 49a04bbb62ac6d33e000b005b32ca3b979fa626a..43f3e80290241517f7d26b231a461ce18cfbf79d 100644
--- a/test/metabase/models/secret_test.clj
+++ b/test/metabase/models/secret_test.clj
@@ -120,7 +120,30 @@
(let [result (byte-array (.length val-file))]
(with-open [in (DataInputStream. (io/input-stream val-file))]
(.readFully in result))
- result))))))))))
+ result)))))))))
+ (testing "value->file! returns the same file for secrets"
+ (testing "for file paths"
+ (let [file-secret-val "dingbat"
+ ^File tmp-file (doto (File/createTempFile "value-to-file-test_" ".txt")
+ (.deleteOnExit))]
+ (spit tmp-file file-secret-val)
+ (mt/with-temp* [Secret [secret {:name "file based secret"
+ :kind :perm-cert
+ :source "file-path"
+ :value (.getAbsolutePath tmp-file)}]]
+ (is (instance? java.io.File (secret/value->file! secret nil)))
+ (is (= (secret/value->file! secret nil)
+ (secret/value->file! secret nil))
+ "Secret did not return the same file"))))
+ (testing "for upload files (#23034)"
+ (mt/with-temp* [Secret [secret {:name "file based secret"
+ :kind :perm-cert
+ :source nil
+ :value (.getBytes "super secret")}]]
+ (is (instance? java.io.File (secret/value->file! secret nil)))
+ (is (= (secret/value->file! secret nil)
+ (secret/value->file! secret nil))
+ "Secret did not return the same file")))))
(defn- decode-ssl-db-property [content mime-type property]
(let [value-key (keyword (str property "-value"))