Skip to content
Snippets Groups Projects
Unverified Commit b28675d6 authored by John Swanson's avatar John Swanson Committed by GitHub
Browse files

Allow disabling encryption for settings (#43085) (#46773)

* Allow disabling encryption for settings

For two settings, we want to disable encryption entirely.

On the way out of the data source, we don't need to worry - the
decryption is already backwards-compatible with non-encrypted data.

On the way in, things are a bit less straightforward. The core of the
change is replacing the `t2/deftransform` that *always* runs
`encryption/maybe-encrypt` on the value with:

- a define-after-select that always tries to decrypt the value (so we
can handle existing encrypted values in the db)

- a define-before-insert that encrypts the value if the `defsetting`
told us to, and

- a define-before-update that does the same thing.

For now, set two settings to be never-encrypted: `analytics-uuid` and
`anon-tracking-enabled`.
parent cc4de4bc
No related branches found
No related tags found
No related merge requests found
......@@ -85,6 +85,7 @@
(deferred-tru
(str "Unique identifier to be used in Snowplow analytics, to identify this instance of Metabase. "
"This is a public setting since some analytics events are sent prior to initial setup."))
:encryption :never
:visibility :public
:base setting/uuid-nonce-base
:doc false)
......
......@@ -155,6 +155,7 @@
(ensure-audit-db-installed!)
(init-status/set-progress! 0.95)
(settings/migrate-encrypted-settings!)
;; start scheduler at end of init!
(task/start-scheduler!)
(init-status/set-complete!)
......
......@@ -277,11 +277,6 @@
{:in encrypted-json-in
:out cached-encrypted-json-out})
(def transform-encrypted-text
"Transform for encrypted text."
{:in encryption/maybe-encrypt
:out encryption/maybe-decrypt})
(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."
......
......@@ -84,13 +84,13 @@
[metabase.api.common :as api]
[metabase.config :as config]
[metabase.events :as events]
[metabase.models.interface :as mi]
[metabase.models.serialization :as serdes]
[metabase.models.setting.cache :as setting.cache]
[metabase.plugins.classloader :as classloader]
[metabase.server.middleware.json]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
[metabase.util.encryption :as encryption]
[metabase.util.i18n :refer [deferred-trs deferred-tru trs tru]]
[metabase.util.log :as log]
[metabase.util.malli :as mu]
......@@ -160,9 +160,6 @@
(methodical/defmethod t2/primary-keys :model/Setting [_model] [:key])
(t2/deftransforms :model/Setting
{:value mi/transform-encrypted-text})
(defmethod serdes/hash-fields :model/Setting
[_setting]
[:key])
......@@ -274,6 +271,10 @@
;; where this setting should be visible (default: :admin)
[:visibility Visibility]
;; should this setting be encrypted `:never` or `:maybe` (when `MB_ENCRYPTION_SECRET_KEY` is set).
;; Defaults to `:maybe`
[:encryption [:enum :never :maybe]]
;; should this setting be serialized?
[:export? :boolean]
......@@ -327,7 +328,8 @@
(resolve-setting [k]
(or (@registered-settings k)
(throw (ex-info (tru "Unknown setting: {0}" k)
{:registered-settings
{::unknown-setting-error true
:registered-settings
(sort (keys @registered-settings))})))))
(defn registered?
......@@ -386,6 +388,9 @@
(when (allows-database-local-values? setting)
(core/get *database-local-values* setting-name))))
(defn- prohibits-encryption? [setting]
(#{:never} (:encryption (resolve-setting setting))))
(defn- allows-user-local-values? [setting]
(#{:only :allowed} (:user-local (resolve-setting setting))))
......@@ -974,6 +979,7 @@
:init nil
:tag (default-tag-for-type setting-type)
:visibility :admin
:encryption :maybe
:export? false
:sensitive? false
:cache? true
......@@ -1518,3 +1524,46 @@
(:parse-error invalid-setting)))
(log/warn (:parse-error invalid-setting)
(format "Unable to parse setting %s" (:name invalid-setting))))))
(defn migrate-encrypted-settings!
"We have some settings that may currently be encrypted in the database that we'd like to disable encryption for.
This function just goes through all of them, checks to see if a value exists in the database, and re-saves it if
so. Toucan will handle decryption on the way out (if necessary) and the new value won't be encrypted.
Note that we're completely working around the standard getters/setters here. This should be fine in this case
because:
- we're only doing anything when a value exists in the database, and
- we're setting the value to the exact same value that already exists - just a decrypted version."
[]
(doseq [setting (filter prohibits-encryption? (vals @registered-settings))]
;; use a raw query to use `:for :update`
(t2/with-transaction [_conn]
(when-let [v (t2/select-one-fn :value :setting :key (setting-name setting) {:for :update})]
(when (not= (encryption/maybe-decrypt v) v)
;; similarly, use `:setting` vs `:model/Setting` here to ensure the update is actually run even though Toucan
;; thinks nothing has changed
(t2/update! :setting :key (setting-name setting) {:value (encryption/maybe-decrypt v)}))))))
(defn- maybe-encrypt [setting-model]
;; In tests, sometimes we need to insert/update settings that don't have definitions in the code and therefore can't
;; be resolved. Fall back to maybe-encrypting these.
(let [resolved (try (resolve-setting (:key setting-model))
(catch clojure.lang.ExceptionInfo e
(when (not (::unknown-setting-error (ex-data e)))
(throw e))))]
(cond-> setting-model
(or (nil? resolved)
(not (prohibits-encryption? resolved)))
(update :value encryption/maybe-encrypt))))
(t2/define-before-update :model/Setting
[setting]
(maybe-encrypt setting))
(t2/define-before-insert :model/Setting
[setting]
(maybe-encrypt setting))
(t2/define-after-select :model/Setting
[setting]
(update setting :value encryption/maybe-decrypt))
......@@ -211,6 +211,7 @@
(application-name-for-setting-descriptions))
:type :boolean
:default true
:encryption :never
:visibility :public
:audit :getter)
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment