Read the latest version of the secret from the DB (#34335)

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

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)))))))))