From 745c0c9463564deae9b7db3a10187db97152ad30 Mon Sep 17 00:00:00 2001
From: metamben <103100869+metamben@users.noreply.github.com>
Date: Thu, 5 Oct 2023 02:19:52 +0300
Subject: [PATCH] Read the latest version of the secret from the DB (#34335)
---
src/metabase/models/secret.clj | 20 +++++++-------
test/metabase/models/secret_test.clj | 41 ++++++++++++++++++++++++++++
2 files changed, 51 insertions(+), 10 deletions(-)
diff --git a/src/metabase/models/secret.clj b/src/metabase/models/secret.clj
index 30d68b4d21f..3853a1e3e66 100644
--- a/src/metabase/models/secret.clj
+++ b/src/metabase/models/secret.clj
@@ -143,6 +143,12 @@
"Regex for parsing base64 encoded file uploads."
#"^data:application/([^;]*);base64,")
+(defn latest-for-id
+ "Returns the latest Secret instance for the given `id` (meaning the one with the highest `version`)."
+ {:added "0.42.0"}
+ [id]
+ (t2/select-one Secret :id id {:order-by [[:version :desc]]}))
+
(defn db-details-prop->secret-map
"Returns a map containing `:value` and `:source` for the given `conn-prop-nm`. `conn-prop-nm` is expected to be the
name of a connection property having `:type` `:secret`, and the relevant sub-properties (ex: -value, -path, etc.) will
@@ -185,14 +191,14 @@
{:invalid-db-details-entry (select-keys details [path-kw])}))))
(id-kw details)
- (:value (t2/select-one Secret :id (id-kw details))))
+ (:value (latest-for-id (id-kw details))))
source (cond
;; set the :source due to the -path suffix (see above))
(and (not= "uploaded" (options-kw details)) (path-kw details))
:file-path
(id-kw details)
- (:source (t2/select-one Secret :id (id-kw details))))]
+ (:source (latest-for-id (id-kw details))))]
(cond-> {:connection-property-name conn-prop-nm, :subprops [path-kw value-kw id-kw]}
value
(assoc :value value
@@ -209,7 +215,7 @@
value
(String. ^bytes value "UTF-8")))
(when id
- (String. ^bytes (:value (t2/select-one Secret :id id)) "UTF-8")))]
+ (String. ^bytes (:value (latest-for-id id)) "UTF-8")))]
(case (options-kw details)
"uploaded" (try
;; When a secret is updated, the value has already been decoded
@@ -226,12 +232,6 @@
bump-version-keys
[:kind :source :value])
-(defn latest-for-id
- "Returns the latest Secret instance for the given `id` (meaning the one with the highest `version`)."
- {:added "0.42.0"}
- [id]
- (t2/select-one Secret :id id {:order-by [[:version :desc]]}))
-
(defn upsert-secret-value!
"Inserts a new secret value, or updates an existing one, for the given parameters.
* if there is no existing Secret instance, inserts with the given field values
@@ -305,7 +305,7 @@
(let [subprop (fn [prop-nm]
(keyword (str conn-prop-nm prop-nm)))
secret* (cond (int? secret-or-id)
- (t2/select-one Secret :id secret-or-id)
+ (latest-for-id secret-or-id)
(mi/instance-of? Secret secret-or-id)
secret-or-id
diff --git a/test/metabase/models/secret_test.clj b/test/metabase/models/secret_test.clj
index cce30abfa0f..fac83f2f604 100644
--- a/test/metabase/models/secret_test.clj
+++ b/test/metabase/models/secret_test.clj
@@ -210,3 +210,44 @@
(is (bytes? decoded))
(is (= content
(String. ^bytes decoded "UTF-8"))))))))))
+
+(deftest use-latest-version-test
+ (testing "when reading a secret from the DB, the latest version is taken (#33116)"
+ (let [initial-value "vnetillo"
+ latest-value (str initial-value "s")
+ initial-source "source"
+ latest-source (str initial-source "s")
+ secret-property "secret"
+ secret-map1 {:name secret-property
+ :kind ::secret/password
+ :value initial-value
+ :source initial-source}
+ secret-map2 {:name secret-property
+ :kind ::secret/password
+ :value latest-value
+ :source latest-source}
+ crowberto-id (mt/user->id :crowberto)
+ by-crowberto #(assoc % :creator_id crowberto-id)
+ timestamp-string? (fn [s]
+ (try
+ (java.time.Instant/parse s)
+ true
+ (catch Exception _
+ false)))]
+ (t2.with-temp/with-temp [Secret {:keys [id version]} (by-crowberto secret-map1)
+ Secret _ (-> secret-map2
+ by-crowberto
+ (assoc :id id :version (inc version)))]
+ (let [details {:secret-id id}]
+ (testing "db-details-prop->secret-map"
+ (let [secret (secret/db-details-prop->secret-map details secret-property)]
+ (is (= latest-value (-> secret :value bytes (String. "UTF8"))))
+ (is (= (keyword latest-source) (:source secret)))))
+ (testing "get-secret-string"
+ (is (= latest-value (secret/get-secret-string details secret-property))))
+ (testing "expand-inferred-secret-values"
+ (is (=? {:secret-id id
+ :secret-source (keyword latest-source)
+ :secret-creator-id crowberto-id
+ :secret-created-at timestamp-string?}
+ (secret/expand-inferred-secret-values details secret-property id)))))))))
--
GitLab