Fixed tests

src/metabase/db/encryption.clj

@@ -1,10 +1,13 @@
 (ns metabase.db.encryption
   (:require
+   [clojure.core :as core]
+   [metabase.models.interface :as mi]
    [metabase.util.encryption :as encryption]
    [metabase.util.honey-sql-2 :as h2x]
-   [metabase.util.i18n :refer [trs]]
-   [metabase.util.json :as json]
-   [toucan2.core :as t2]))
+   [metabase.util.log :as log]
+   [methodical.core :as methodical]
+   [toucan2.core :as t2])
+  (:import (java.sql Blob)))
 
 (set! *warn-on-reflection* true)
 
@@ -16,32 +19,62 @@
   (let [encrypt-str-fn (make-encrypt-fn encryption/maybe-encrypt)
         encrypt-bytes-fn (make-encrypt-fn encryption/maybe-encrypt-bytes)]
     (t2/with-transaction [conn {:datasource data-source}]
-      (doseq [[id details] (t2/select-pk->fn :details :model/Database)]
-        (when (encryption/possibly-encrypted-string? details)
-          (throw (ex-info (trs "Can''t decrypt app db with MB_ENCRYPTION_SECRET_KEY") {:database-id id})))
-        (t2/update! :conn conn :metabase_database
-                    {:id id}
-                    {:details (encrypt-str-fn (json/encode details))}))
-      (doseq [[key value] (t2/select-fn->fn :key :value :model/Setting)]
-        (case key
-          "settings-last-updated" (let [current-timestamp-as-string-honeysql (h2x/cast (if (= db-type :mysql) :char :text)
-                                                                                       [:raw "current_timestamp"])]
-                                    (t2/update! :conn conn :setting {:key key} {:value current-timestamp-as-string-honeysql}))
-          "encryption-check" (t2/update! :conn conn :setting {:key key} {:value (if encrypting? (encrypt-str-fn (str (random-uuid))) "unencrypted")})
-          (t2/update! :conn conn :setting
-                      {:key key}
-                      {:value (encrypt-str-fn value)})))
-      ;; update all secret values according to the new encryption key
-      ;; fortunately, we don't need to fetch the latest secret instance per ID, as we would need to in order to update
-      ;; a secret value through the regular database save API path; instead, ALL secret values in the app DB (regardless
-      ;; of whether they are the "current version" or not), should be updated with the new key
-      (doseq [[id value] (t2/select-pk->fn :value :model/Secret)]
-        (when (encryption/possibly-encrypted-string? value)
-          (throw (ex-info (trs "Can''t decrypt secret value with MB_ENCRYPTION_SECRET_KEY") {:secret-id id})))
-        (t2/update! :conn conn :secret
-                    {:id id}
-                    {:value (encrypt-bytes-fn value)}))
-      (t2/delete! :conn conn :model/QueryCache))))
+      (t2/delete! :conn conn :model/QueryCache)
+      (t2/delete! :conn conn :model/FieldValues :human_readable_values [:= nil])
+
+      (doseq [model (filter #(= (namespace %) "model") (core/descendants :metabase/model))]
+        (cond
+          (= model :model/Setting)
+          (doseq [[key value] (t2/select-fn->fn :key :value :model/Setting)]
+            (case key
+              "settings-last-updated" (let [current-timestamp-as-string-honeysql (h2x/cast (if (= db-type :mysql) :char :text)
+                                                                                           [:raw "current_timestamp"])]
+                                        (t2/update! :conn conn :setting {:key key} {:value current-timestamp-as-string-honeysql}))
+              "encryption-check" (t2/update! :conn conn :model/Setting {:key key} {:value (if encrypting? (encryption/maybe-encrypt (str (random-uuid))) "unencrypted")})
+              (t2/update! :conn conn (t2/table-name :model/Setting)
+                          {:key key}
+                          {:value (encrypt-str-fn value)})))
+
+          :else
+          (when-let [transforms-fn (methodical/effective-method t2/transforms model)]
+            (doseq [[field] (keep
+                             (fn [[field transform]]
+                               (when (or (= transform mi/transform-encrypted-json)
+                                         (= transform mi/transform-encrypted-json-no-keywordization)
+                                         (= transform mi/transform-secret-value))
+                                 [field]))
+
+                             (transforms-fn model))]
+              (log/info (str "Encrypting/decrypting " model " " field))
+              (doseq [{:keys [id value]} (t2/query conn {:select [:id [field :value]]
+                                                         :from   [(t2/table-name model)]
+                                                         :where  [:not= field nil]})]
+                (let [decrypted-value (encryption/maybe-decrypt value)]
+                  (cond
+                    (string? decrypted-value)
+                    (if (encryption/possibly-encrypted-string? decrypted-value)
+                      (throw (ex-info (str "Can't decrypt " (-> model t2/table-name name) "." (name field) " with MB_ENCRYPTION_SECRET_KEY") {:model model :id id}))
+                      (t2/update! :conn conn (t2/table-name model)
+                                  {:id id}
+                                  {field (encrypt-str-fn decrypted-value)}))
+
+                    (instance? Blob decrypted-value)
+                    (let [decrypted-bytes (encryption/maybe-decrypt (.getBytes ^Blob decrypted-value 1 (.length ^Blob decrypted-value)))]
+                      (if (encryption/possibly-encrypted-bytes? decrypted-bytes)
+                        (throw (ex-info (str "Can't decrypt " (-> model t2/table-name name) "." (name field) " with MB_ENCRYPTION_SECRET_KEY") {:model model :id id}))
+                        (t2/update! :conn conn (t2/table-name model)
+                                    {:id id}
+                                    {field (encrypt-bytes-fn decrypted-bytes)})))
+
+                    (bytes? decrypted-value)
+                    (if (encryption/possibly-encrypted-bytes? decrypted-value)
+                      (throw (ex-info (str "Can't decrypt " (-> model t2/table-name name) "." (name field) " with MB_ENCRYPTION_SECRET_KEY") {:model model :id id}))
+                      (t2/update! :conn conn (t2/table-name model)
+                                  {:id id}
+                                  {field (encrypt-bytes-fn decrypted-value)}))
+
+                    :else
+                    (throw (ex-info (str "Unknown value type" decrypted-value) {:model model :field field}))))))))))))
 
 (defn encrypt-db
   "Encrypt the db using the current `MB_ENCRYPTION_SECRET_KEY` to read existing data, and the passed `to-key` to re-encrypt.

src/metabase/models/field_values.clj

@@ -105,8 +105,8 @@
   (derive :hook/timestamped?))
 
 (t2/deftransforms :model/FieldValues
-  {:human_readable_values mi/transform-json-no-keywordization
-   :values                mi/transform-json
+  {:human_readable_values mi/transform-encrypted-json-no-keywordization
+   :values                mi/transform-encrypted-json
    :type                  mi/transform-keyword})
 
 (defn- assert-valid-human-readable-values [{human-readable-values :human_readable_values}]

src/metabase/models/interface.clj

@@ -332,15 +332,40 @@
           (log/error e "Error parsing JSON"))  ; same message as in `json-out`
         v))))
 
+(defn encrypted-json-out-without-keywordization
+  "Deserialize encrypted json."
+  [v]
+  (let [decrypted (encryption/maybe-decrypt v)]
+    (try
+      (json-out decrypted false)
+      (catch Throwable e
+        (if (or (encryption/possibly-encrypted-string? decrypted)
+                (encryption/possibly-encrypted-bytes? decrypted))
+          (log/error e "Could not decrypt encrypted field! Have you forgot to set MB_ENCRYPTION_SECRET_KEY?")
+          (log/error e "Error parsing JSON"))  ; same message as in `json-out`
+        v))))
+
+(def ^:dynamic *enable-json-caching*
+  "Disable when changing the encryption key, so the wrong encrypted version is not used."
+  true)
+
 ;; cache the decryption/JSON parsing because it's somewhat slow (~500µs vs ~100µs on a *fast* computer)
 ;; cache the decrypted JSON for one hour
-(def ^:private cached-encrypted-json-out (memoize/ttl encrypted-json-out :ttl/threshold (* 60 60 1000)))
+(def ^:private cached-encrypted-json-out
+  (if *enable-json-caching*
+    (memoize/ttl encrypted-json-out :ttl/threshold (* 60 60 1000))
+    encrypted-json-out))
 
 (def transform-encrypted-json
   "Transform for encrypted json."
   {:in  encrypted-json-in
    :out cached-encrypted-json-out})
 
+(def transform-encrypted-json-no-keywordization
+  "Transform for json-no-keywordization"
+  {:in  encrypted-json-in
+   :out encrypted-json-out-without-keywordization})
+
 (defn normalize-visualization-settings
   "The frontend uses JSON-serialized versions of MBQL clauses as keys in `:column_settings`. This normalizes them
    to modern MBQL clauses so things work correctly."

src/metabase/util/encryption.clj

@@ -79,9 +79,11 @@
   (^String [^String s]
    (encrypt default-secret-key s))
   (^String [^String secret-key, ^String s]
-   (->> (codecs/to-bytes s)
-        (encrypt-bytes secret-key)
-        codec/base64-encode)))
+   (if (nil? s)
+     nil
+     (->> (codecs/to-bytes s)
+          (encrypt-bytes secret-key)
+          codec/base64-encode))))
 
 (defn decrypt-bytes
   "Decrypt bytes `b` using a `secret-key` (a 64-byte byte array), which by default is the hashed value of
@@ -150,7 +152,8 @@
   (^String [^String s]
    (decrypt default-secret-key s))
   (^String [secret-key, ^String s]
-   (codecs/bytes->str (decrypt-bytes secret-key (codec/base64-decode s)))))
+   (if (nil? s) nil
+       (codecs/bytes->str (decrypt-bytes secret-key (codec/base64-decode s))))))
 
 (defn maybe-encrypt
   "If `MB_ENCRYPTION_SECRET_KEY` is set, return an encrypted version of `s`; otherwise return `s` as-is."
@@ -217,7 +220,7 @@
   {:arglists '([secret-key? s])}
   [& args]
   ;; secret-key as an argument so that tests can pass it directly without using `with-redefs` to run in parallel
-  (let [[secret-key v]     (if (and (bytes? (first args)) (string? (second args)))
+  (let [[secret-key v]     (if (= 2 (count args))
                              args
                              (cons default-secret-key args))
         log-error-fn (fn [kind ^Throwable e]
@@ -228,6 +231,9 @@
     (cond (nil? secret-key)
           v
 
+          (nil? v)
+          v
+
           (possibly-encrypted-string? v)
           (try
             (decrypt secret-key v)

test/metabase/cmd/rotate_encryption_key_test.clj

@@ -148,7 +148,7 @@
                             db)))
                   (is (thrown-with-msg?
                        clojure.lang.ExceptionInfo
-                       #"Can't decrypt app db with MB_ENCRYPTION_SECRET_KEY"
+                       #"Can't decrypt metabase_database.details with MB_ENCRYPTION_SECRET_KEY"
                        (rotate-encryption-key! k3))))
                 (encryption-test/with-secret-key k3
                   (is (not= {:db "/tmp/k2.db"} (t2/select-one-fn :details :model/Database :name "k2")))

test/metabase/setup/core_test.clj

@@ -114,27 +114,27 @@
           (let [cache-backend (i/cache-backend :db)]
             (i/save-results! cache-backend (codecs/to-bytes "cache-key") (codecs/to-bytes "cache-value"))
             (is (= "unencrypted" (t2/select-one-fn :value "setting" :key "encryption-check")))
-            (is (not (encryption/possibly-encrypted-string? (t2/select-one-fn :details "metabase_database"))))
+            (is (not (encryption/possibly-encrypted-string? (t2/select-one-fn :details (t2/table-name :model/Database)))))
             (is (= 1 (t2/count :model/QueryCache)))
 
             (testing "Adding encryption encrypts database on restart"
               (encryption-test/with-secret-key "key1"
                 (reset! (:status mdb.connection/*application-db*) ::setup-finished)
                 (mdb/setup-db! :create-sample-content? false)
-                (is (encryption/possibly-encrypted-string? (:value (t2/select-one "setting" :key "encryption-check"))))
-                (is (encryption/possibly-encrypted-string? (:details (t2/select-one "metabase_database"))))
+                (is (encryption/possibly-encrypted-string? (:value (t2/select-one (t2/table-name :model/Setting) :key "encryption-check"))))
+                (is (encryption/possibly-encrypted-string? (:details (t2/select-one (t2/table-name :model/Database)))))
                 (testing "Cache is cleared on encryption"
                   (is (= 0 (t2/count :model/QueryCache))))))))))
     (testing "Database created with encryption configured is encrypted"
       (encryption-test/with-secret-key "key2"
         (mt/with-temp-empty-app-db [_conn driver/*driver*]
           (mdb/setup-db! :create-sample-content? true)
-          (is (encryption/possibly-encrypted-string? (t2/select-one-fn :value "setting" :key "encryption-check")))
-          (is (encryption/possibly-encrypted-string? (t2/select-one-fn :details "metabase_database")))
+          (is (encryption/possibly-encrypted-string? (t2/select-one-fn :value (t2/table-name :model/Setting) :key "encryption-check")))
+          (is (encryption/possibly-encrypted-string? (t2/select-one-fn :details (t2/table-name :model/Database))))
           (testing "Re-running server works"
             (reset! (:status mdb.connection/*application-db*) ::setup-finished)
             (mdb/setup-db! :create-sample-content? false)
-            (is (encryption/possibly-encrypted-string? (:value (t2/select-one "setting" :key "encryption-check")))))
+            (is (encryption/possibly-encrypted-string? (:value (t2/select-one (t2/table-name :model/Setting) :key "encryption-check")))))
           (testing "Different encryption key throws an error"
             (encryption-test/with-secret-key "different-key"
               (reset! (:status mdb.connection/*application-db*) ::setup-finished)

test/metabase/util/encryption_test.clj

@@ -164,3 +164,13 @@
     (testing "When secret is not set, it does not encrypt the stream"
       (let [encrypted (encryption/maybe-encrypt-for-stream nil (codecs/to-bytes "test string"))]
         (is (= "test string" (codecs/bytes->str encrypted)))))))
+
+(deftest ^:parallel encrypt-nil
+  (testing "encrypting nil should return nil"
+    (is (nil? (encryption/encrypt secret nil)))
+    (is (nil? (encryption/maybe-encrypt secret nil)))))
+
+(deftest ^:parallel decrypt-nil
+  (testing "decrypting nil should return nil"
+    (is (nil? (encryption/decrypt secret nil)))
+    (is (nil? (encryption/maybe-decrypt secret nil)))))