diff --git a/src/metabase/api/email.clj b/src/metabase/api/email.clj
index d6da9a13a865bf74db111f6e5286dfaac63b0405..984991c0842d2cffe81774700085542e312a50a5 100644
--- a/src/metabase/api/email.clj
+++ b/src/metabase/api/email.clj
@@ -3,7 +3,7 @@
(:require [clojure
[data :as data]
[set :as set]
- [string :as string]]
+ [string :as str]]
[clojure.tools.logging :as log]
[compojure.core :refer [DELETE POST PUT]]
[metabase
@@ -11,9 +11,11 @@
[email :as email]]
[metabase.api.common :as api]
[metabase.models.setting :as setting]
- [metabase.util.schema :as su]))
+ [metabase.util
+ [i18n :refer [tru]]
+ [schema :as su]]))
-(def ^:private ^:const mb-to-smtp-settings
+(def ^:private mb-to-smtp-settings
{:email-smtp-host :host
:email-smtp-username :user
:email-smtp-password :pass
@@ -55,18 +57,18 @@
#".*"
{:message "Sorry, something went wrong. Please try again."}))))
-(defn humanize-email-corrections
- "formats warnings when security settings are autocorrected"
+(defn- humanize-email-corrections
+ "Formats warnings when security settings are autocorrected."
[corrections]
- (into {}
- (mapv (fn [[k v]]
- [k (format "%s was autocorrected to %s"
- (name (mb-to-smtp-settings k))
- (string/upper-case v))])
- corrections)))
+ (into
+ {}
+ (for [[k v] corrections]
+ [k (tru "{0} was autocorrected to {1}"
+ (name (mb-to-smtp-settings k))
+ (str/upper-case v))])))
(api/defendpoint PUT "/"
- "Update multiple `Settings` values. You must be a superuser to do this."
+ "Update multiple email Settings. You must be a superuser to do this."
[:as {settings :body}]
{settings su/Map}
(api/check-superuser)
diff --git a/src/metabase/api/setting.clj b/src/metabase/api/setting.clj
index ab173a82628eee5f998095e923992a7f1d56ea21..5e87b2b4c49494ac081ca31555d1995c09c9620b 100644
--- a/src/metabase/api/setting.clj
+++ b/src/metabase/api/setting.clj
@@ -22,11 +22,7 @@
[key]
{key su/NonBlankString}
(api/check-superuser)
- (let [k (keyword key)
- v (setting/get k)]
- ;; for security purposes, don't return value of a setting if it was defined via env var
- (when (not= v (setting/env-var-value k))
- v)))
+ (setting/user-facing-value key))
(api/defendpoint PUT "/:key"
"Create/update a `Setting`. You must be a superuser to do this.
diff --git a/src/metabase/email.clj b/src/metabase/email.clj
index 2f37d7f67527fe29a15b19a4f98631a8175934f7..72fd5395526dea8e4e04f37efb65af29451794c9 100644
--- a/src/metabase/email.clj
+++ b/src/metabase/email.clj
@@ -12,13 +12,25 @@
(:import javax.mail.Session))
;;; CONFIG
+
+(defsetting email-from-address
+ (tru "Email address you want to use as the sender of Metabase.")
+ :default "notifications@metabase.com")
+
+(defsetting email-smtp-host
+ (tru "The address of the SMTP server that handles your emails."))
+
+(defsetting email-smtp-username
+ (tru "SMTP username."))
+
+(defsetting email-smtp-password
+ (tru "SMTP password.")
+ :sensitive? true)
+
;; TODO - smtp-port should be switched to type :integer
+(defsetting email-smtp-port
+ (tru "The port your SMTP server uses for outgoing emails."))
-(defsetting email-from-address (tru "Email address you want to use as the sender of Metabase.") :default "notifications@metabase.com")
-(defsetting email-smtp-host (tru "The address of the SMTP server that handles your emails."))
-(defsetting email-smtp-username (tru "SMTP username."))
-(defsetting email-smtp-password (tru "SMTP password."))
-(defsetting email-smtp-port (tru "The port your SMTP server uses for outgoing emails."))
(defsetting email-smtp-security
(tru "SMTP secure connection protocol. (tls, ssl, starttls, or none)")
:default (tru "none")
diff --git a/src/metabase/integrations/ldap.clj b/src/metabase/integrations/ldap.clj
index e2a9992b8edd4996e6b0b19b2ab280969d7a1bc2..89453b4844690de0777325b56b636e021f48a3f0 100644
--- a/src/metabase/integrations/ldap.clj
+++ b/src/metabase/integrations/ldap.clj
@@ -39,7 +39,8 @@
(tru "The Distinguished Name to bind as (if any), this user will be used to lookup information about other users."))
(defsetting ldap-password
- (tru "The password to bind with for the lookup user."))
+ (tru "The password to bind with for the lookup user.")
+ :sensitive? true)
(defsetting ldap-user-base
(tru "Search base for users. (Will be searched recursively)"))
diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj
index 2b7213bd0bace7e7e240f3378e832b8bc6392924..836f659b2982973b474dcaf7473ec10024060427 100644
--- a/src/metabase/models/setting.clj
+++ b/src/metabase/models/setting.clj
@@ -33,18 +33,14 @@
[clojure
[core :as core]
[string :as str]]
- [clojure.core.memoize :as memoize]
- [clojure.java.jdbc :as jdbc]
[clojure.tools.logging :as log]
[environ.core :as env]
- [honeysql.core :as hsql]
[metabase
- [db :as mdb]
[events :as events]
[util :as u]]
+ [metabase.models.setting.cache :as cache]
[metabase.util
[date :as du]
- [honeysql-extensions :as hx]
[i18n :as ui18n :refer [trs tru]]]
[schema.core :as s]
[toucan
@@ -81,6 +77,7 @@
:getter clojure.lang.IFn ; different getters/setters take care of parsing/unparsing
:setter clojure.lang.IFn
:tag (s/maybe Class) ; type annotation, e.g. ^String, to be applied. Defaults to tag based on :type
+ :sensitive? s/Bool ; is this sensitive (never show in plaintext), like a password? (default: false)
:internal? s/Bool ; should the API never return this setting? (default: false)
:cache? s/Bool}) ; should the getter always fetch this value "fresh" from the DB? (default: false)
@@ -98,128 +95,6 @@
(str (tru "Setting {0} does not exist.\nFound: {1}" k (sort (keys @registered-settings))))))))))
-;;; +----------------------------------------------------------------------------------------------------------------+
-;;; | cache |
-;;; +----------------------------------------------------------------------------------------------------------------+
-
-;; Cache is a 1:1 mapping of what's in the DB
-;; Cached lookup time is ~60µs, compared to ~1800µs for DB lookup
-
-(def ^:private cache
- "Settings cache. Map of Setting key (string) -> Setting value (string)."
- (atom nil))
-
-;; CACHE SYNCHRONIZATION
-;;
-;; When running multiple Metabase instances (horizontal scaling), it is of course possible for one instance to update
-;; a Setting, and, since Settings are cached (to avoid tons of DB calls), for the other instances to then have an
-;; out-of-date cache. Thus we need a way for instances to know when their caches are out of date, so they can update
-;; them accordingly. Here is our solution:
-;;
-;; We will record the last time *any* Setting was updated in a special Setting called `settings-last-updated`.
-;;
-;; Since `settings-last-updated` itself is a Setting, it will get fetched as part of each instance's local cache; we
-;; can then periodically compare the locally cached value of `settings-last-updated` with the value in the DB. If our
-;; locally cached value is older than the one in the DB, we will flush our cache. When the cache is fetched again, it
-;; will have the up-to-date value.
-;;
-;; Because different machines can have out-of-sync clocks, we'll rely entirely on the application DB for caclulating
-;; and comparing values of `settings-last-updated`. Because the Setting table itself only stores text values, we'll
-;; need to cast it between TEXT and TIMESTAMP SQL types as needed.
-
-(def ^:private ^String settings-last-updated-key "settings-last-updated")
-
-(defn- update-settings-last-updated!
- "Update the value of `settings-last-updated` in the DB; if the row does not exist, insert one."
- []
- (log/debug (trs "Updating value of settings-last-updated in DB..."))
- ;; for MySQL, cast(current_timestamp AS char); for H2 & Postgres, cast(current_timestamp AS text)
- (let [current-timestamp-as-string-honeysql (hx/cast (if (= (mdb/db-type) :mysql) :char :text)
- (hsql/raw "current_timestamp"))]
- ;; attempt to UPDATE the existing row. If no row exists, `update-where!` will return false...
- (or (db/update-where! Setting {:key settings-last-updated-key} :value current-timestamp-as-string-honeysql)
- ;; ...at which point we will try to INSERT a new row. Note that it is entirely possible two instances can both
- ;; try to INSERT it at the same time; one instance would fail because it would violate the PK constraint on
- ;; `key`, and throw a SQLException. As long as one instance updates the value, we are fine, so we can go ahead
- ;; and ignore that Exception if one is thrown.
- (try
- ;; Use `simple-insert!` because we do *not* want to trigger pre-insert behavior, such as encrypting `:value`
- (db/simple-insert! Setting :key settings-last-updated-key, :value current-timestamp-as-string-honeysql)
- (catch java.sql.SQLException e
- ;; go ahead and log the Exception anyway on the off chance that it *wasn't* just a race condition issue
- (log/error (trs "Error inserting a new Setting: {0}"
- (with-out-str (jdbc/print-sql-exception-chain e))))))))
- ;; Now that we updated the value in the DB, go ahead and update our cached value as well, because we know about the
- ;; changes
- (swap! cache assoc settings-last-updated-key (db/select-one-field :value Setting :key settings-last-updated-key)))
-
-(defn- cache-out-of-date?
- "Check whether our Settings cache is out of date. We know the cache is out of date if either of the following
- conditions is true:
-
- * The cache is empty (the `cache` atom is `nil`), which of course means it needs to be updated
- * There is a value of `settings-last-updated` in the cache, and it is older than the value of in the DB. (There
- will be no value until the first time a normal Setting is updated; thus if it is not yet set, we do not yet need
- to invalidate our cache.)"
- []
- (log/debug (trs "Checking whether settings cache is out of date (requires DB call)..."))
- (boolean
- (or
- ;; is the cache empty?
- (not @cache)
- ;; if not, get the cached value of `settings-last-updated`, and if it exists...
- (when-let [last-known-update (core/get @cache settings-last-updated-key)]
- ;; compare it to the value in the DB. This is done be seeing whether a row exists
- ;; WHERE value > <local-value>
- (u/prog1 (db/select-one Setting
- {:where [:and
- [:= :key settings-last-updated-key]
- [:> :value last-known-update]]})
- (when <>
- (log/info (u/format-color 'red
- (str (trs "Settings have been changed on another instance, and will be reloaded here."))))))))))
-
-(def ^:private cache-update-check-interval-ms
- "How often we should check whether the Settings cache is out of date (which requires a DB call)?"
- ;; once a minute
- (* 60 1000))
-
-(def ^:private ^{:arglists '([])} should-restore-cache?
- "TTL-memoized version of `cache-out-of-date?`. Call this function to see whether we need to repopulate the cache with
- values from the DB."
- (memoize/ttl cache-out-of-date? :ttl/threshold cache-update-check-interval-ms))
-
-(def ^:private restore-cache-if-needed-lock (Object.))
-
-(defn- restore-cache-if-needed!
- "Check whether we need to repopulate the cache with fresh values from the DB (because the cache is either empty or
- known to be out-of-date), and do so if needed. This is intended to be called every time a Setting value is
- retrieved, so it should be efficient; thus the calculation (`should-restore-cache?`) is itself TTL-memoized."
- []
- ;; There's a potential race condition here where two threads both call this at the exact same moment, and both get
- ;; `true` when they call `should-restore-cache`, and then both simultaneously try to update the cache (or, one
- ;; updates the cache, but the other calls `should-restore-cache?` and gets `true` before the other calls
- ;; `memo-swap!` (see below))
- ;;
- ;; This is not desirable, since either situation would result in duplicate work. Better to just add a quick lock
- ;; here so only one of them does it, since at any rate waiting for the other thread to finish the task in progress is
- ;; certainly quicker than starting the task ourselves from scratch
- (locking restore-cache-if-needed-lock
- (when (should-restore-cache?)
- (log/debug (trs "Refreshing Settings cache..."))
- (reset! cache (db/select-field->field :key :value Setting))
- ;; Now the cache is up-to-date. That is all good, but if we call `should-restore-cache?` again in a second it
- ;; will still return `true`, because its result is memoized, and we would be on the hook to (again) update the
- ;; cache. So go ahead and clear the memozied results for `should-restore-cache?`. The next time around when
- ;; someone calls this it will cache the latest value (which should be `false`)
- ;;
- ;; NOTE: I tried using `memo-swap!` instead to set the cached response to `false` here, avoiding the extra DB
- ;; call the next fn call would make, but it didn't seem to work correctly (I think it was still discarding the
- ;; new value because of the TTL). So we will just stick with `memo-clear!` for now. (One extra DB call whenever
- ;; the cache gets invalidated shouldn't be a huge deal)
- (memoize/memo-clear! should-restore-cache?))))
-
-
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | get |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -252,8 +127,8 @@
(if *disable-cache*
(db/select-one-field :value Setting :key (setting-name setting-or-name))
(do
- (restore-cache-if-needed!)
- (clojure.core/get @cache (setting-name setting-or-name)))))
+ (cache/restore-cache-if-needed!)
+ (clojure.core/get (cache/cache) (setting-name setting-or-name)))))
(defn get-string
@@ -336,7 +211,7 @@
(defn- update-setting!
"Update an existing Setting. Used internally by `set-string!` below; do not use directly."
[setting-name new-value]
- (assert (not= setting-name settings-last-updated-key)
+ (assert (not= setting-name cache/settings-last-updated-key)
(tru "You cannot update `settings-last-updated` yourself! This is done automatically."))
;; This is indeed a very annoying way of having to do things, but `update-where!` doesn't call `pre-update` (in case
;; it updates thousands of objects). So we need to manually trigger `pre-update` behavior by calling `do-pre-update`
@@ -361,34 +236,42 @@
(tru "Assuming Setting already exists in DB and updating existing value."))
(update-setting! setting-name new-value))))
+(defn- obfuscated-value? [v]
+ (when (seq v)
+ (boolean (re-matches #"^\*{10}.{2}$" v))))
+
(s/defn set-string!
"Set string value of `setting-or-name`. A `nil` or empty `new-value` can be passed to unset (i.e., delete)
`setting-or-name`. String-type settings use this function directly; all other types ultimately call this (e.g.
`set-boolean!` eventually calls `set-string!`). Returns the `new-value`."
[setting-or-name, new-value :- (s/maybe s/Str)]
- (let [new-value (when (seq new-value)
- new-value)
- setting (resolve-setting setting-or-name)
- setting-name (setting-name setting)]
- (restore-cache-if-needed!)
- ;; write to DB
- (cond
- (not new-value) (db/simple-delete! Setting :key setting-name)
- ;; if there's a value in the cache then the row already exists in the DB; update that
- (contains? @cache setting-name) (update-setting! setting-name new-value)
- ;; if there's nothing in the cache then the row doesn't exist, insert a new one
- :else (set-new-setting! setting-name new-value))
- ;; update cached value
- (if new-value
- (swap! cache assoc setting-name new-value)
- (swap! cache dissoc setting-name))
- ;; Record the fact that a Setting has been updated so eventaully other instances (if applicable) find out about it
- ;; (For Settings that don't use the Cache, don't update the `last-updated` value, because it will cause other
- ;; instances to do needless reloading of the cache from the DB)
- (when-not *disable-cache*
- (update-settings-last-updated!))
- ;; Now return the `new-value`.
- new-value))
+ (let [new-value (when (seq new-value)
+ new-value)
+ {:keys [sensitive?], :as setting} (resolve-setting setting-or-name)
+ obfuscated? (and sensitive? (obfuscated-value? new-value))
+ setting-name (setting-name setting)]
+ ;; if someone attempts to set a sensitive setting to an obfuscated value (probably via a misuse of the `set-many!`
+ ;; function, setting values that have not changed), ignore the change. Log a message that we are ignoring it.
+ (if obfuscated?
+ (log/info (trs "Attempted to set Setting {0} to obfuscated value. Ignoring change." setting-name))
+ (do
+ (cache/restore-cache-if-needed!)
+ ;; write to DB
+ (cond
+ (nil? new-value) (db/simple-delete! Setting :key setting-name)
+ ;; if there's a value in the cache then the row already exists in the DB; update that
+ (contains? (cache/cache) setting-name) (update-setting! setting-name new-value)
+ ;; if there's nothing in the cache then the row doesn't exist, insert a new one
+ :else (set-new-setting! setting-name new-value))
+ ;; update cached value
+ (cache/update-cache! setting-name new-value)
+ ;; Record the fact that a Setting has been updated so eventaully other instances (if applicable) find out about it
+ ;; (For Settings that don't use the Cache, don't update the `last-updated` value, because it will cause other
+ ;; instances to do needless reloading of the cache from the DB)
+ (when-not *disable-cache*
+ (cache/update-settings-last-updated!))
+ ;; Now return the `new-value`.
+ new-value))))
(defn set-boolean!
"Set the value of boolean `setting-or-name`. `new-value` can be nil, a boolean, or a string representation of one,
@@ -461,21 +344,22 @@
This is used internally be `defsetting`; you shouldn't need to use it yourself."
[{setting-name :name, setting-type :type, default :default, :as setting}]
(u/prog1 (let [setting-type (s/validate Type (or setting-type :string))]
- (merge {:name setting-name
- :description nil
- :type setting-type
- :default default
- :getter (partial (default-getter-for-type setting-type) setting-name)
- :setter (partial (default-setter-for-type setting-type) setting-name)
- :tag (default-tag-for-type setting-type)
- :internal? false
- :cache? true}
+ (merge
+ {:name setting-name
+ :description nil
+ :type setting-type
+ :default default
+ :getter (partial (default-getter-for-type setting-type) setting-name)
+ :setter (partial (default-setter-for-type setting-type) setting-name)
+ :tag (default-tag-for-type setting-type)
+ :internal? false
+ :sensitive? false
+ :cache? true}
(dissoc setting :name :type :default)))
(s/validate SettingDefinition <>)
(swap! registered-settings assoc setting-name <>)))
-
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | defsetting macro |
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -519,16 +403,15 @@
;; :refer-clojure :exclude doesn't seem to work in this case
(metabase.models.setting/set! setting new-value))))
-(defn- expr-of-sym? [symbols expr]
- (when-let [first-sym (and (coll? expr)
- (first expr))]
- (some #(= first-sym %) symbols)))
+(defn- is-expression? [symbols expression]
+ (when (list? expression)
+ ((set symbols) (first expression))))
(defn- valid-trs-or-tru? [desc]
- (expr-of-sym? ['trs 'tru `trs `tru] desc))
+ (is-expression? #{'trs 'tru `trs `tru} desc))
(defn- valid-str-of-trs-or-tru? [maybe-str-expr]
- (when (expr-of-sym? ['str `str] maybe-str-expr)
+ (when (is-expression? #{'str `str} maybe-str-expr)
;; When there are several i18n'd sentences, there will probably be a surrounding `str` invocation and a space in
;; between the sentences, remove those to validate the i18n clauses
(let [exprs-without-strs (remove (every-pred string? str/blank?) (rest maybe-str-expr))]
@@ -560,20 +443,25 @@
You may optionally pass any of the OPTIONS below:
- * `:default` - The default value of the setting. (default: `nil`)
- * `:type` - `:string` (default), `:boolean`, `:integer`, or `:json`. Non-`:string` settings have special
- default getters and setters that automatically coerce values to the correct types.
- * `:internal?` - This Setting is for internal use and shouldn't be exposed in the UI (i.e., not returned by the
- corresponding endpoints). Default: `false`
- * `:getter` - A custom getter fn, which takes no arguments. Overrides the default implementation. (This can in
- turn call functions in this namespace like `get-string` or `get-boolean` to invoke the default
- getter behavior.)
- * `:setter` - A custom setter fn, which takes a single argument. Overrides the default implementation. (This
- can in turn call functions in this namespace like `set-string!` or `set-boolean!` to invoke the
- default setter behavior. Keep in mind that the custom setter may be passed `nil`, which should
- clear the values of the Setting.)
- * `:cache?` - Should this Setting be cached? (default `true`)? Be careful when disabling this, because it could
- have a very negative performance impact."
+ * `:default` - The default value of the setting. (default: `nil`)
+ * `:type` - `:string` (default), `:boolean`, `:integer`, or `:json`. Non-`:string` settings have special
+ default getters and setters that automatically coerce values to the correct types.
+ * `:internal?` - This Setting is for internal use and shouldn't be exposed in the UI (i.e., not returned by the
+ corresponding endpoints). Default: `false`
+ * `:getter` - A custom getter fn, which takes no arguments. Overrides the default implementation. (This can in
+ turn call functions in this namespace like `get-string` or `get-boolean` to invoke the default
+ getter behavior.)
+ * `:setter` - A custom setter fn, which takes a single argument. Overrides the default implementation. (This
+ can in turn call functions in this namespace like `set-string!` or `set-boolean!` to invoke the
+ default setter behavior. Keep in mind that the custom setter may be passed `nil`, which should
+ clear the values of the Setting.)
+ * `:cache?` - Should this Setting be cached? (default `true`)? Be careful when disabling this, because it could
+ have a very negative performance impact.
+ * `:sensitive?` - Is this a sensitive setting, such as a password, that we should never return in plaintext?
+ (Default: `false`). Obfuscation is not done by getter functions, but instead by functions that
+ ultimately return these values via the API, such as `all` below. (In other words, code in the
+ backend can continute to consume sensitive Settings normally; sensitivity is a purely user-facing
+ option.)"
{:style/indent 1}
[setting-symb description & {:as options}]
{:pre [(symbol? setting-symb)]}
@@ -606,21 +494,46 @@
;; changed, and doesn't run when irrelevant changes (to other settings) are made.
(events/publish-event! :settings-update settings))
+(defn- obfuscate-value
+ "Obfuscate the value of sensitive Setting. We'll still show the last 2 characters so admins can still check that the
+ value is what's expected (e.g. the correct password).
+
+ (obfuscate-value \"sensitivePASSWORD123\") ;; -> \"**********23\""
+ [s]
+ (str "**********" (str/join (take-last 2 (str s)))))
+
+(defn user-facing-value
+ "Get the value of a Setting that should be displayed to a User (i.e. via `/api/setting/` endpoints): for Settings set
+ via env vars, or Settings whose value has not been set (i.e., Settings whose value is the same as the default value)
+ no value is displayed; for sensitive Settings, the value is obfuscated."
+ [setting-or-name]
+ (let [{:keys [sensitive? default], k :name, :as setting} (resolve-setting setting-or-name)
+ v (get k)
+ value-is-default? (= v default)
+ value-is-from-env-var? (= v (env-var-value setting))]
+ (cond
+ ;; TODO - Settings set via an env var aren't returned for security purposes. It is an open question whether we
+ ;; should obfuscate them and still show the last two characters like we do for sensitive values that are set via
+ ;; the UI.
+ (or value-is-default? value-is-from-env-var?)
+ nil
+
+ sensitive?
+ (obfuscate-value v)
+
+ :else
+ v)))
-(defn- user-facing-info [setting]
- (let [k (:name setting)
- v (get k)
- env-value (env-var-value setting)]
+(defn- user-facing-info [{:keys [sensitive? default description], k :name, :as setting}]
+ (let [set-via-env-var? (boolean (env-var-value setting))]
{:key k
- :value (when (and (not= v env-value)
- (not= v (:default setting)))
- v)
- :is_env_setting (boolean env-value)
+ :value (user-facing-value setting)
+ :is_env_setting set-via-env-var?
:env_name (env-var-name setting)
- :description (str (:description setting))
- :default (or (when env-value
- (format "Using $%s" (env-var-name setting)))
- (:default setting))}))
+ :description (str description)
+ :default (if set-via-env-var?
+ (str (tru "Using value of env var {0}" (str \$ (env-var-name setting))))
+ default)}))
(defn all
"Return a sequence of Settings maps in a format suitable for consumption by the frontend.
diff --git a/src/metabase/models/setting/cache.clj b/src/metabase/models/setting/cache.clj
new file mode 100644
index 0000000000000000000000000000000000000000..35334a2ff91b2ecc73eb767b495b499f6451cd41
--- /dev/null
+++ b/src/metabase/models/setting/cache.clj
@@ -0,0 +1,143 @@
+(ns metabase.models.setting.cache
+ "Settings cache. Cache is a 1:1 mapping of what's in the DB. Cached lookup time is ~60µs, compared to ~1800µs for DB
+ lookup."
+ (:require [clojure.core :as core]
+ [clojure.core.memoize :as memoize]
+ [clojure.java.jdbc :as jdbc]
+ [clojure.tools.logging :as log]
+ [honeysql.core :as hsql]
+ [metabase
+ [db :as mdb]
+ [util :as u]]
+ [metabase.util
+ [honeysql-extensions :as hx]
+ [i18n :as ui18n :refer [trs]]]
+ [toucan.db :as db]))
+
+(def ^:private cache*
+ "Settings cache. Map of Setting key (string) -> Setting value (string)."
+ (atom nil))
+
+(defn cache
+ "Fetch the current contents of the Settings cache, a map of key (string) -> value (string)."
+ []
+ @cache*)
+
+(defn update-cache!
+ "Update the String value of a Setting in the Settings cache."
+ [setting-name, ^String new-value]
+ (if (seq new-value)
+ (swap! cache* assoc setting-name new-value)
+ (swap! cache* dissoc setting-name)))
+
+;; CACHE SYNCHRONIZATION
+;;
+;; When running multiple Metabase instances (horizontal scaling), it is of course possible for one instance to update
+;; a Setting, and, since Settings are cached (to avoid tons of DB calls), for the other instances to then have an
+;; out-of-date cache. Thus we need a way for instances to know when their caches are out of date, so they can update
+;; them accordingly. Here is our solution:
+;;
+;; We will record the last time *any* Setting was updated in a special Setting called `settings-last-updated`.
+;;
+;; Since `settings-last-updated` itself is a Setting, it will get fetched as part of each instance's local cache; we
+;; can then periodically compare the locally cached value of `settings-last-updated` with the value in the DB. If our
+;; locally cached value is older than the one in the DB, we will flush our cache. When the cache is fetched again, it
+;; will have the up-to-date value.
+;;
+;; Because different machines can have out-of-sync clocks, we'll rely entirely on the application DB for caclulating
+;; and comparing values of `settings-last-updated`. Because the Setting table itself only stores text values, we'll
+;; need to cast it between TEXT and TIMESTAMP SQL types as needed.
+
+(def ^String settings-last-updated-key
+ "Internal key used to store the last updated timestamp for Settings."
+ "settings-last-updated")
+
+(defn update-settings-last-updated!
+ "Update the value of `settings-last-updated` in the DB; if the row does not exist, insert one."
+ []
+ (log/debug (trs "Updating value of settings-last-updated in DB..."))
+ ;; for MySQL, cast(current_timestamp AS char); for H2 & Postgres, cast(current_timestamp AS text)
+ (let [current-timestamp-as-string-honeysql (hx/cast (if (= (mdb/db-type) :mysql) :char :text)
+ (hsql/raw "current_timestamp"))]
+ ;; attempt to UPDATE the existing row. If no row exists, `update-where!` will return false...
+ (or (db/update-where! 'Setting {:key settings-last-updated-key} :value current-timestamp-as-string-honeysql)
+ ;; ...at which point we will try to INSERT a new row. Note that it is entirely possible two instances can both
+ ;; try to INSERT it at the same time; one instance would fail because it would violate the PK constraint on
+ ;; `key`, and throw a SQLException. As long as one instance updates the value, we are fine, so we can go ahead
+ ;; and ignore that Exception if one is thrown.
+ (try
+ ;; Use `simple-insert!` because we do *not* want to trigger pre-insert behavior, such as encrypting `:value`
+ (db/simple-insert! 'Setting :key settings-last-updated-key, :value current-timestamp-as-string-honeysql)
+ (catch java.sql.SQLException e
+ ;; go ahead and log the Exception anyway on the off chance that it *wasn't* just a race condition issue
+ (log/error (trs "Error inserting a new Setting: {0}"
+ (with-out-str (jdbc/print-sql-exception-chain e))))))))
+ ;; Now that we updated the value in the DB, go ahead and update our cached value as well, because we know about the
+ ;; changes
+ (swap! cache* assoc settings-last-updated-key (db/select-one-field :value 'Setting :key settings-last-updated-key)))
+
+(defn- cache-out-of-date?
+ "Check whether our Settings cache is out of date. We know the cache is out of date if either of the following
+ conditions is true:
+
+ * The cache is empty (the `cache*` atom is `nil`), which of course means it needs to be updated
+ * There is a value of `settings-last-updated` in the cache, and it is older than the value of in the DB. (There
+ will be no value until the first time a normal Setting is updated; thus if it is not yet set, we do not yet need
+ to invalidate our cache.)"
+ []
+ (log/debug (trs "Checking whether settings cache is out of date (requires DB call)..."))
+ (boolean
+ (or
+ ;; is the cache empty?
+ (not @cache*)
+ ;; if not, get the cached value of `settings-last-updated`, and if it exists...
+ (when-let [last-known-update (core/get @cache* settings-last-updated-key)]
+ ;; compare it to the value in the DB. This is done be seeing whether a row exists
+ ;; WHERE value > <local-value>
+ (u/prog1 (db/select-one 'Setting
+ {:where [:and
+ [:= :key settings-last-updated-key]
+ [:> :value last-known-update]]})
+ (when <>
+ (log/info (u/format-color 'red
+ (str (trs "Settings have been changed on another instance, and will be reloaded here."))))))))))
+
+(def ^:private cache-update-check-interval-ms
+ "How often we should check whether the Settings cache is out of date (which requires a DB call)?"
+ ;; once a minute
+ (* 60 1000))
+
+(def ^:private ^{:arglists '([])} should-restore-cache?
+ "TTL-memoized version of `cache-out-of-date?`. Call this function to see whether we need to repopulate the cache with
+ values from the DB."
+ (memoize/ttl cache-out-of-date? :ttl/threshold cache-update-check-interval-ms))
+
+(def ^:private restore-cache-if-needed-lock (Object.))
+
+(defn restore-cache-if-needed!
+ "Check whether we need to repopulate the cache with fresh values from the DB (because the cache is either empty or
+ known to be out-of-date), and do so if needed. This is intended to be called every time a Setting value is
+ retrieved, so it should be efficient; thus the calculation (`should-restore-cache?`) is itself TTL-memoized."
+ []
+ ;; There's a potential race condition here where two threads both call this at the exact same moment, and both get
+ ;; `true` when they call `should-restore-cache`, and then both simultaneously try to update the cache (or, one
+ ;; updates the cache, but the other calls `should-restore-cache?` and gets `true` before the other calls
+ ;; `memo-swap!` (see below))
+ ;;
+ ;; This is not desirable, since either situation would result in duplicate work. Better to just add a quick lock
+ ;; here so only one of them does it, since at any rate waiting for the other thread to finish the task in progress is
+ ;; certainly quicker than starting the task ourselves from scratch
+ (locking restore-cache-if-needed-lock
+ (when (should-restore-cache?)
+ (log/debug (trs "Refreshing Settings cache..."))
+ (reset! cache* (db/select-field->field :key :value 'Setting))
+ ;; Now the cache is up-to-date. That is all good, but if we call `should-restore-cache?` again in a second it
+ ;; will still return `true`, because its result is memoized, and we would be on the hook to (again) update the
+ ;; cache. So go ahead and clear the memozied results for `should-restore-cache?`. The next time around when
+ ;; someone calls this it will cache the latest value (which should be `false`)
+ ;;
+ ;; NOTE: I tried using `memo-swap!` instead to set the cached response to `false` here, avoiding the extra DB
+ ;; call the next fn call would make, but it didn't seem to work correctly (I think it was still discarding the
+ ;; new value because of the TTL). So we will just stick with `memo-clear!` for now. (One extra DB call whenever
+ ;; the cache gets invalidated shouldn't be a huge deal)
+ (memoize/memo-clear! should-restore-cache?))))
diff --git a/test/metabase/api/setting_test.clj b/test/metabase/api/setting_test.clj
index 1608d91bf8026ebf17e170ff925674834bdf9687..8dfadd6e4e9588aa1264e876b8f2aed2663c0019 100644
--- a/test/metabase/api/setting_test.clj
+++ b/test/metabase/api/setting_test.clj
@@ -1,7 +1,7 @@
(ns metabase.api.setting-test
- (:require [expectations :refer :all]
- [metabase.models.setting-test :refer [set-settings! test-setting-1 test-setting-2]]
- [metabase.test.data.users :refer :all]))
+ (:require [expectations :refer [expect]]
+ [metabase.models.setting-test :refer [set-settings! test-sensitive-setting test-setting-1 test-setting-2]]
+ [metabase.test.data.users :refer [user->client]]))
;; ## Helper Fns
(defn- fetch-test-settings []
@@ -15,8 +15,18 @@
;; ## GET /api/setting
;; Check that we can fetch all Settings for Org
(expect
- [{:key "test-setting-1", :value nil, :is_env_setting true, :env_name "MB_TEST_SETTING_1", :description "Test setting - this only shows up in dev (1)", :default "Using $MB_TEST_SETTING_1"}
- {:key "test-setting-2", :value "FANCY", :is_env_setting false, :env_name "MB_TEST_SETTING_2", :description "Test setting - this only shows up in dev (2)", :default "[Default Value]"}]
+ [{:key "test-setting-1"
+ :value nil
+ :is_env_setting true
+ :env_name "MB_TEST_SETTING_1"
+ :description "Test setting - this only shows up in dev (1)"
+ :default "Using value of env var $MB_TEST_SETTING_1"}
+ {:key "test-setting-2"
+ :value "FANCY"
+ :is_env_setting false
+ :env_name "MB_TEST_SETTING_2"
+ :description "Test setting - this only shows up in dev (2)"
+ :default "[Default Value]"}]
(do (set-settings! nil "FANCY")
(fetch-test-settings)))
@@ -44,3 +54,52 @@
;; ## Check non-superuser can't set a Setting
(expect "You don't have permissions to do that."
((user->client :rasta) :put 403 "setting/test-setting-1" {:value "NICE!"}))
+
+
+;;; ----------------------------------------------- Sensitive Settings -----------------------------------------------
+
+;; Sensitive settings should always come back obfuscated
+
+;; GET /api/setting/:name should obfuscate sensitive settings
+(expect
+ "**********EF"
+ (do
+ (test-sensitive-setting "ABCDEF")
+ (fetch-setting :test-sensitive-setting)))
+
+;; GET /api/setting should obfuscate sensitive settings
+(expect
+ {:key "test-sensitive-setting"
+ :value "**********LM"
+ :is_env_setting false
+ :env_name "MB_TEST_SENSITIVE_SETTING"
+ :description "This is a sample sensitive Setting."
+ :default nil}
+ (do
+ (test-sensitive-setting "GHIJKLM")
+ (some (fn [{setting-name :key, :as setting}]
+ (when (= setting-name "test-sensitive-setting")
+ setting))
+ ((user->client :crowberto) :get 200 "setting"))))
+
+;; Setting the Setting via an endpoint should still work as expected; the normal getter functions should *not*
+;; obfuscate sensitive Setting values -- that should be done by the API
+(expect
+ "123456"
+ (do
+ ((user->client :crowberto) :put 200 "setting/test-sensitive-setting" {:value "123456"})
+ (test-sensitive-setting)))
+
+;; Attempts to set the Setting to an obfuscated value should be ignored
+(expect
+ "123456"
+ (do
+ (test-sensitive-setting "123456")
+ ((user->client :crowberto) :put 200 "setting/test-sensitive-setting" {:value "**********56"})
+ (test-sensitive-setting)))
+
+(expect
+ (do
+ (test-sensitive-setting "123456")
+ ((user->client :crowberto) :put 200 "setting" {:test-sensitive-setting "**********56"})
+ (test-sensitive-setting)))
diff --git a/test/metabase/models/setting/cache_test.clj b/test/metabase/models/setting/cache_test.clj
new file mode 100644
index 0000000000000000000000000000000000000000..1cc922cd229fc47255d985ebe037ef1b75e2d32d
--- /dev/null
+++ b/test/metabase/models/setting/cache_test.clj
@@ -0,0 +1,124 @@
+(ns metabase.models.setting.cache-test
+ (:require [clojure.core.memoize :as memoize]
+ [expectations :refer [expect]]
+ [honeysql.core :as hsql]
+ [metabase.db :as mdb]
+ [metabase.models
+ [setting :refer [Setting]]
+ [setting-test :as setting-test]]
+ [metabase.models.setting.cache :as cache]
+ [toucan.db :as db]))
+
+;;; --------------------------------------------- Cache Synchronization ----------------------------------------------
+
+(defn- clear-cache! []
+ (reset! @#'metabase.models.setting.cache/cache* nil))
+
+(defn- settings-last-updated-value-in-cache []
+ (get (cache/cache) cache/settings-last-updated-key))
+
+(defn- update-settings-last-updated-value-in-db!
+ "Simulate a different instance updating the value of `settings-last-updated` in the DB by updating its value without
+ updating our locally cached value.."
+ []
+ (db/update-where! Setting {:key cache/settings-last-updated-key}
+ :value (hsql/raw (case (mdb/db-type)
+ ;; make it one second in the future so we don't end up getting an exact match when we try to test
+ ;; to see if things update below
+ :h2 "cast(dateadd('second', 1, current_timestamp) AS text)"
+ :mysql "cast((current_timestamp + interval 1 second) AS char)"
+ :postgres "cast((current_timestamp + interval '1 second') AS text)"))))
+
+(defn- simulate-another-instance-updating-setting! [setting-name new-value]
+ (db/update-where! Setting {:key (name setting-name)} :value new-value)
+ (update-settings-last-updated-value-in-db!))
+
+(defn- flush-memoized-results-for-should-restore-cache!
+ "Remove any memoized results for `should-restore-cache?`, so we can test `restore-cache-if-needed!` works the way we'd
+ expect."
+ []
+ (memoize/memo-clear! @#'cache/should-restore-cache?))
+
+;; When I update a Setting, does it set/update `settings-last-updated`?
+(expect
+ (do
+ (setting-test/clear-settings-last-updated-value-in-db!)
+ (setting-test/toucan-name "Bird Can")
+ (string? (setting-test/settings-last-updated-value-in-db))))
+
+;; ...and is the value updated in the cache as well?
+(expect
+ (do
+ (clear-cache!)
+ (setting-test/toucan-name "Bird Can")
+ (string? (settings-last-updated-value-in-cache))))
+
+;; ...and if I update it again, will the value be updated?
+(expect
+ (do
+ (setting-test/clear-settings-last-updated-value-in-db!)
+ (setting-test/toucan-name "Bird Can")
+ (let [first-value (setting-test/settings-last-updated-value-in-db)]
+ ;; MySQL only has the resolution of one second on the timestamps here so we should wait that long to make sure
+ ;; the second-value actually ends up being greater than the first
+ (Thread/sleep 1200)
+ (setting-test/toucan-name "Bird Can")
+ (let [second-value (setting-test/settings-last-updated-value-in-db)]
+ ;; first & second values should be different, and first value should be "less than" the second value
+ (and (not= first-value second-value)
+ (neg? (compare first-value second-value)))))))
+
+;; If there is no cache, it should be considered out of date!`
+(expect
+ (do
+ (clear-cache!)
+ (#'cache/cache-out-of-date?)))
+
+;; But if I set a setting, it should cause the cache to be populated, and be up-to-date
+(expect
+ false
+ (do
+ (clear-cache!)
+ (setting-test/toucan-name "Reggae Toucan")
+ (#'cache/cache-out-of-date?)))
+
+;; If another instance updates a Setting, `cache-out-of-date?` should return `true` based on DB comparisons...
+;; be true!
+(expect
+ (do
+ (clear-cache!)
+ (setting-test/toucan-name "Reggae Toucan")
+ (simulate-another-instance-updating-setting! :setting-test/toucan-name "Bird Can")
+ (#'cache/cache-out-of-date?)))
+
+;; of course, `restore-cache-if-needed!` should use TTL memoization, and the cache should not get updated right away
+;; even if another instance updates a value...
+(expect
+ "Sam"
+ (do
+ (flush-memoized-results-for-should-restore-cache!)
+ (clear-cache!)
+ (setting-test/toucan-name "Sam") ; should restore cache, and put in {"setting-test/toucan-name" "Sam"}
+ ;; since we cleared the memoized value of `should-restore-cache?` call it again to make sure it gets set to
+ ;; `false` as it would IRL if we were calling it again from the same instance
+ (#'cache/should-restore-cache?)
+ ;; now have another instance change the value
+ (simulate-another-instance-updating-setting! :setting-test/toucan-name "Bird Can")
+ ;; our cache should not be updated yet because it's on a TTL
+ (setting-test/toucan-name)))
+
+;; ...and when it comes time to check our cache for updating (when calling `restore-cache-if-needed!`, it should get
+;; the updated value. (we're not actually going to wait a minute for the memoized values of `should-restore-cache?` to
+;; be invalidated, so we will manually flush the memoization cache to simulate it happening)
+(expect
+ "Bird Can"
+ (do
+ (clear-cache!)
+ (setting-test/toucan-name "Reggae Toucan")
+ (simulate-another-instance-updating-setting! :setting-test/toucan-name "Bird Can")
+ (flush-memoized-results-for-should-restore-cache!)
+ ;; calling `setting-test/toucan-name` will call `restore-cache-if-needed!`, which will in turn call `should-restore-cache?`.
+ ;; Since memoized value is no longer present, this should call `cache-out-of-date?`, which checks the DB; it will
+ ;; detect a cache out-of-date situation and flush the cache as appropriate, giving us the updated value when we
+ ;; call! :wow:
+ (setting-test/toucan-name)))
diff --git a/test/metabase/models/setting_test.clj b/test/metabase/models/setting_test.clj
index 006752d48d590058e6051c4f5769ea8f13f4dc34..db4548dde099ff633beb1d8d9c0b9e3f0e913353 100644
--- a/test/metabase/models/setting_test.clj
+++ b/test/metabase/models/setting_test.clj
@@ -1,12 +1,9 @@
(ns metabase.models.setting-test
- (:require [clojure.core.memoize :as memoize]
- [expectations :refer :all]
- [honeysql.core :as hsql]
- [metabase
- [db :as mdb]
- [util :as u]]
+ (:require [expectations :refer [expect]]
[metabase.models.setting :as setting :refer [defsetting Setting]]
+ [metabase.models.setting.cache :as cache]
[metabase.test.util :refer :all]
+ [metabase.util :as u]
[metabase.util
[encryption :as encryption]
[encryption-test :as encryption-test]
@@ -151,12 +148,12 @@
;; #'setting/user-facing-info w/ no db value, env var value, no default value -- shouldn't leak env var value
(expect
- {:value nil, :is_env_setting true, :env_name "MB_TEST_SETTING_1", :default "Using $MB_TEST_SETTING_1"}
+ {:value nil, :is_env_setting true, :env_name "MB_TEST_SETTING_1", :default "Using value of env var $MB_TEST_SETTING_1"}
(user-facing-info-with-db-and-env-var-values :test-setting-1 nil "TOUCANS"))
;; #'setting/user-facing-info w/ no db value, env var value, default value
(expect
- {:value nil, :is_env_setting true, :env_name "MB_TEST_SETTING_2", :default "Using $MB_TEST_SETTING_2"}
+ {:value nil, :is_env_setting true, :env_name "MB_TEST_SETTING_2", :default "Using value of env var $MB_TEST_SETTING_2"}
(user-facing-info-with-db-and-env-var-values :test-setting-2 nil "TOUCANS"))
;; #'setting/user-facing-info w/ db value, no env var value, no default value
@@ -172,13 +169,13 @@
;; #'setting/user-facing-info w/ db value, env var value, no default value -- the DB value should take precedence over
;; #the env var
(expect
- {:value "WOW", :is_env_setting true, :env_name "MB_TEST_SETTING_1", :default "Using $MB_TEST_SETTING_1"}
+ {:value "WOW", :is_env_setting true, :env_name "MB_TEST_SETTING_1", :default "Using value of env var $MB_TEST_SETTING_1"}
(user-facing-info-with-db-and-env-var-values :test-setting-1 "WOW" "ENV VAR"))
;; #'setting/user-facing-info w/ db value, env var value, default value -- env var should take precedence over default
;; #value
(expect
- {:value "WOW", :is_env_setting true, :env_name "MB_TEST_SETTING_2", :default "Using $MB_TEST_SETTING_2"}
+ {:value "WOW", :is_env_setting true, :env_name "MB_TEST_SETTING_2", :default "Using value of env var $MB_TEST_SETTING_2"}
(user-facing-info-with-db-and-env-var-values :test-setting-2 "WOW" "ENV VAR"))
;; all
@@ -202,7 +199,7 @@
:is_env_setting true
:env_name "MB_TEST_SETTING_1"
:description "Test setting - this only shows up in dev (1)"
- :default "Using $MB_TEST_SETTING_1"}
+ :default "Using value of env var $MB_TEST_SETTING_1"}
{:key :test-setting-2
:value "S2"
:is_env_setting false,
@@ -240,7 +237,7 @@
;; boolean settings shouldn't be obfuscated when set by env var
(expect
- {:value true, :is_env_setting true, :env_name "MB_TEST_BOOLEAN_SETTING", :default "Using $MB_TEST_BOOLEAN_SETTING"}
+ {:value true, :is_env_setting true, :env_name "MB_TEST_BOOLEAN_SETTING", :default "Using value of env var $MB_TEST_BOOLEAN_SETTING"}
(user-facing-info-with-db-and-env-var-values :test-boolean-setting nil "true"))
;; env var values should be case-insensitive
@@ -291,7 +288,7 @@
;; make sure that if for some reason the cache gets out of sync it will reset so we can still set new settings values
;; (#4178)
-(setting/defsetting ^:private toucan-name
+(setting/defsetting toucan-name
"Name for the Metabase Toucan mascot."
:internal? true)
@@ -301,7 +298,7 @@
;; clear out any existing values of `toucan-name`
(db/simple-delete! setting/Setting {:key "toucan-name"})
;; restore the cache
- ((resolve 'metabase.models.setting/restore-cache-if-needed!))
+ (cache/restore-cache-if-needed!)
;; now set a value for the `toucan-name` setting the wrong way
(db/insert! setting/Setting {:key "toucan-name", :value "Reggae"})
;; ok, now try to set the Setting the correct way
@@ -361,130 +358,13 @@
(test-timestamp-setting)))
-;;; --------------------------------------------- Cache Synchronization ----------------------------------------------
-
-(def ^:private settings-last-updated-key @(resolve 'metabase.models.setting/settings-last-updated-key))
-
-(defn- clear-settings-last-updated-value-in-db! []
- (db/simple-delete! Setting {:key settings-last-updated-key}))
-
-(defn- settings-last-updated-value-in-db []
- (db/select-one-field :value Setting :key settings-last-updated-key))
-
-(defn- clear-cache! []
- (reset! @(resolve 'metabase.models.setting/cache) nil))
-
-(defn- settings-last-updated-value-in-cache []
- (get @@(resolve 'metabase.models.setting/cache) settings-last-updated-key))
-
-(defn- update-settings-last-updated-value-in-db!
- "Simulate a different instance updating the value of `settings-last-updated` in the DB by updating its value without
- updating our locally cached value.."
- []
- (db/update-where! Setting {:key settings-last-updated-key}
- :value (hsql/raw (case (mdb/db-type)
- ;; make it one second in the future so we don't end up getting an exact match when we try to test
- ;; to see if things update below
- :h2 "cast(dateadd('second', 1, current_timestamp) AS text)"
- :mysql "cast((current_timestamp + interval 1 second) AS char)"
- :postgres "cast((current_timestamp + interval '1 second') AS text)"))))
-
-(defn- simulate-another-instance-updating-setting! [setting-name new-value]
- (db/update-where! Setting {:key (name setting-name)} :value new-value)
- (update-settings-last-updated-value-in-db!))
-
-(defn- flush-memoized-results-for-should-restore-cache!
- "Remove any memoized results for `should-restore-cache?`, so we can test `restore-cache-if-needed!` works the way we'd
- expect."
- []
- (memoize/memo-clear! @(resolve 'metabase.models.setting/should-restore-cache?)))
-
-;; When I update a Setting, does it set/update `settings-last-updated`?
-(expect
- (do
- (clear-settings-last-updated-value-in-db!)
- (toucan-name "Bird Can")
- (string? (settings-last-updated-value-in-db))))
-
-;; ...and is the value updated in the cache as well?
-(expect
- (do
- (clear-cache!)
- (toucan-name "Bird Can")
- (string? (settings-last-updated-value-in-cache))))
-
-;; ...and if I update it again, will the value be updated?
-(expect
- (do
- (clear-settings-last-updated-value-in-db!)
- (toucan-name "Bird Can")
- (let [first-value (settings-last-updated-value-in-db)]
- ;; MySQL only has the resolution of one second on the timestamps here so we should wait that long to make sure
- ;; the second-value actually ends up being greater than the first
- (Thread/sleep 1200)
- (toucan-name "Bird Can")
- (let [second-value (settings-last-updated-value-in-db)]
- ;; first & second values should be different, and first value should be "less than" the second value
- (and (not= first-value second-value)
- (neg? (compare first-value second-value)))))))
-
-;; If there is no cache, it should be considered out of date!`
-(expect
- (do
- (clear-cache!)
- (#'setting/cache-out-of-date?)))
-
-;; But if I set a setting, it should cause the cache to be populated, and be up-to-date
-(expect
- false
- (do
- (clear-cache!)
- (toucan-name "Reggae Toucan")
- (#'setting/cache-out-of-date?)))
-
-;; If another instance updates a Setting, `cache-out-of-date?` should return `true` based on DB comparisons...
-;; be true!
-(expect
- (do
- (clear-cache!)
- (toucan-name "Reggae Toucan")
- (simulate-another-instance-updating-setting! :toucan-name "Bird Can")
- (#'setting/cache-out-of-date?)))
-
-;; of course, `restore-cache-if-needed!` should use TTL memoization, and the cache should not get updated right away
-;; even if another instance updates a value...
-(expect
- "Sam"
- (do
- (flush-memoized-results-for-should-restore-cache!)
- (clear-cache!)
- (toucan-name "Sam") ; should restore cache, and put in {"toucan-name" "Sam"}
- ;; since we cleared the memoized value of `should-restore-cache?` call it again to make sure it gets set to
- ;; `false` as it would IRL if we were calling it again from the same instance
- (#'setting/should-restore-cache?)
- ;; now have another instance change the value
- (simulate-another-instance-updating-setting! :toucan-name "Bird Can")
- ;; our cache should not be updated yet because it's on a TTL
- (toucan-name)))
-
-;; ...and when it comes time to check our cache for updating (when calling `restore-cache-if-needed!`, it should get
-;; the updated value. (we're not actually going to wait a minute for the memoized values of `should-restore-cache?` to
-;; be invalidated, so we will manually flush the memoization cache to simulate it happening)
-(expect
- "Bird Can"
- (do
- (clear-cache!)
- (toucan-name "Reggae Toucan")
- (simulate-another-instance-updating-setting! :toucan-name "Bird Can")
- (flush-memoized-results-for-should-restore-cache!)
- ;; calling `toucan-name` will call `restore-cache-if-needed!`, which will in turn call `should-restore-cache?`.
- ;; Since memoized value is no longer present, this should call `cache-out-of-date?`, which checks the DB; it will
- ;; detect a cache out-of-date situation and flush the cache as appropriate, giving us the updated value when we
- ;; call! :wow:
- (toucan-name)))
+;;; ----------------------------------------------- Uncached Settings ------------------------------------------------
+(defn clear-settings-last-updated-value-in-db! []
+ (db/simple-delete! Setting {:key cache/settings-last-updated-key}))
-;;; ----------------------------------------------- Uncached Settings ------------------------------------------------
+(defn settings-last-updated-value-in-db []
+ (db/select-one-field :value Setting :key cache/settings-last-updated-key))
(defsetting ^:private uncached-setting
"A test setting that should *not* be cached."
@@ -514,3 +394,25 @@
(clear-settings-last-updated-value-in-db!)
(uncached-setting "abcdef")
(settings-last-updated-value-in-db)))
+
+
+;;; ----------------------------------------------- Sensitive Settings -----------------------------------------------
+
+(defsetting test-sensitive-setting
+ (tru "This is a sample sensitive Setting.")
+ :sensitive? true)
+
+;; `user-facing-value` should obfuscate sensitive settings
+(expect
+ "**********23"
+ (do
+ (test-sensitive-setting "ABC123")
+ (setting/user-facing-value "test-sensitive-setting")))
+
+;; Attempting to set a sensitive setting to an obfuscated value should be ignored -- it was probably done accidentally
+(expect
+ "123456"
+ (do
+ (test-sensitive-setting "123456")
+ (test-sensitive-setting "**********56")
+ (test-sensitive-setting)))