diff --git a/enterprise/backend/src/metabase_enterprise/serialization/dump.clj b/enterprise/backend/src/metabase_enterprise/serialization/dump.clj
index 814ff0698eb97a3bdfc7d1885f30b20575fa230c..008caae35c9c8e6317ed0a04cf6d1d5aa5d43d32 100644
--- a/enterprise/backend/src/metabase_enterprise/serialization/dump.clj
+++ b/enterprise/backend/src/metabase_enterprise/serialization/dump.clj
@@ -43,7 +43,7 @@
(log/warn (str filename " is about to be overwritten."))
(log/debug (str "With object: " (pr-str entity))))
- (spit-yaml filename (serialize entity))))
+ (spit-yaml filename (serialize entity))))
(defn dump
"Serialize entities into a directory structure of YAMLs at `path`."
@@ -66,7 +66,7 @@
"Combine all settings into a map and dump it into YAML at `path`."
[path]
(spit-yaml (str path "/settings.yaml")
- (into {} (for [{:keys [key value]} (setting/admin-writable-settings
+ (into {} (for [{:keys [key value]} (setting/admin-writable-site-wide-settings
:getter (partial setting/get-value-of-type :string))]
[key value]))))
diff --git a/resources/migrations/000_migrations.yaml b/resources/migrations/000_migrations.yaml
index b9c775bf01193d4e0fe0810808044c8a82f8161a..1e906e80baf77b75af016a0253ab4a428a999a59 100644
--- a/resources/migrations/000_migrations.yaml
+++ b/resources/migrations/000_migrations.yaml
@@ -10472,6 +10472,21 @@ databaseChangeLog:
- column:
name: timestamp
+ - changeSet:
+ id: v43.00-026
+ author: noahmoss
+ comment: >-
+ Added 0.43.0 - adds User.settings column to implement User-local Settings
+ changes:
+ - addColumn:
+ tableName: core_user
+ columns:
+ - column:
+ name: settings
+ type: ${text.type}
+ remarks: "Serialized JSON containing User-local Settings for this User"
+
+
# >>>>>>>>>> DO NOT ADD NEW MIGRATIONS BELOW THIS LINE! ADD THEM ABOVE <<<<<<<<<<
########################################################################################################################
diff --git a/src/metabase/api/card.clj b/src/metabase/api/card.clj
index 0d6d48d366edfb76c87083f940dfcf8c988d98df..700c6c3b99ef99c96c86bdc0ae256cc875bac0d4 100644
--- a/src/metabase/api/card.clj
+++ b/src/metabase/api/card.clj
@@ -8,6 +8,7 @@
[compojure.core :refer [DELETE GET POST PUT]]
[medley.core :as m]
[metabase.api.common :as api]
+ [metabase.api.common.validation :as validation]
[metabase.api.dataset :as dataset-api]
[metabase.api.timeline :as timeline-api]
[metabase.async.util :as async.u]
@@ -349,7 +350,7 @@
[card-before-updates card-updates]
(when (or (api/column-will-change? :enable_embedding card-before-updates card-updates)
(api/column-will-change? :embedding_params card-before-updates card-updates))
- (api/check-embedding-enabled)
+ (validation/check-embedding-enabled)
(api/check-superuser)))
(defn- publish-card-update!
@@ -732,7 +733,7 @@
be enabled."
[card-id]
(api/check-superuser)
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(api/check-not-archived (api/read-check Card card-id))
{:uuid (or (db/select-one-field :public_uuid Card :id card-id)
(u/prog1 (str (UUID/randomUUID))
@@ -744,7 +745,7 @@
"Delete the publicly-accessible link to this Card."
[card-id]
(api/check-superuser)
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(api/check-exists? Card :id card-id, :public_uuid [:not= nil])
(db/update! Card card-id
:public_uuid nil
@@ -755,7 +756,7 @@
"Fetch a list of Cards with public UUIDs. These cards are publicly-accessible *if* public sharing is enabled."
[]
(api/check-superuser)
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(db/select [Card :name :id :public_uuid], :public_uuid [:not= nil], :archived false))
(api/defendpoint GET "/embeddable"
@@ -763,7 +764,7 @@
and a signed JWT."
[]
(api/check-superuser)
- (api/check-embedding-enabled)
+ (validation/check-embedding-enabled)
(db/select [Card :name :id], :enable_embedding true, :archived false))
(api/defendpoint GET "/:id/related"
diff --git a/src/metabase/api/common.clj b/src/metabase/api/common.clj
index 778ca073ee10c90bb5fb6dc63ad562846a1328e0..c08a68e1a11790136f8fcb567dc77ca0039f1c53 100644
--- a/src/metabase/api/common.clj
+++ b/src/metabase/api/common.clj
@@ -8,7 +8,6 @@
[metabase.api.common.internal :refer [add-route-param-regexes auto-parse route-dox route-fn-name
validate-params wrap-response-if-needed]]
[metabase.models.interface :as mi]
- [metabase.public-settings :as public-settings]
[metabase.util :as u]
[metabase.util.i18n :as ui18n :refer [deferred-tru tru]]
[metabase.util.schema :as su]
@@ -400,18 +399,6 @@
;;; ------------------------------------------------ OTHER HELPER FNS ------------------------------------------------
-(defn check-public-sharing-enabled
- "Check that the `public-sharing-enabled` Setting is `true`, or throw a `400`."
- []
- (check (public-settings/enable-public-sharing)
- [400 (tru "Public sharing is not enabled.")]))
-
-(defn check-embedding-enabled
- "Is embedding of Cards or Objects (secured access via `/api/embed` endpoints with a signed JWT enabled?"
- []
- (check (public-settings/enable-embedding)
- [400 (tru "Embedding is not enabled.")]))
-
(defn check-not-archived
"Check that the `object` exists and is not `:archived`, or throw a `404`. Returns `object` as-is if check passes."
[object]
diff --git a/src/metabase/api/common/validation.clj b/src/metabase/api/common/validation.clj
new file mode 100644
index 0000000000000000000000000000000000000000..2fa6d7d549da0e1fd2b9f21c136f489d017b610b
--- /dev/null
+++ b/src/metabase/api/common/validation.clj
@@ -0,0 +1,19 @@
+(ns metabase.api.common.validation
+ (:require
+ [metabase.api.common :as api]
+ [metabase.public-settings :as public-settings]
+ [metabase.util.i18n :as ui18n :refer [tru]]))
+
+;; TODO: figure out what other functions to move here from metabase.api.common
+
+(defn check-public-sharing-enabled
+ "Check that the `public-sharing-enabled` Setting is `true`, or throw a `400`."
+ []
+ (api/check (public-settings/enable-public-sharing)
+ [400 (tru "Public sharing is not enabled.")]))
+
+(defn check-embedding-enabled
+ "Is embedding of Cards or Objects (secured access via `/api/embed` endpoints with a signed JWT enabled?"
+ []
+ (api/check (public-settings/enable-embedding)
+ [400 (tru "Embedding is not enabled.")]))
diff --git a/src/metabase/api/dashboard.clj b/src/metabase/api/dashboard.clj
index 10c74b768bd907c9c730b0724e78a4b3b0500533..e7bb34a935ee06877050508e068c28eecc0b326f 100644
--- a/src/metabase/api/dashboard.clj
+++ b/src/metabase/api/dashboard.clj
@@ -7,6 +7,7 @@
[medley.core :as m]
[metabase.analytics.snowplow :as snowplow]
[metabase.api.common :as api]
+ [metabase.api.common.validation :as validation]
[metabase.api.dataset :as api.dataset]
[metabase.automagic-dashboards.populate :as magic.populate]
[metabase.events :as events]
@@ -271,7 +272,7 @@
[dash-before-update dash-updates]
(when (or (api/column-will-change? :enable_embedding dash-before-update dash-updates)
(api/column-will-change? :embedding_params dash-before-update dash-updates))
- (api/check-embedding-enabled)
+ (validation/check-embedding-enabled)
(api/check-superuser)))
(api/defendpoint PUT "/:id"
@@ -513,7 +514,7 @@
sharing must be enabled."
[dashboard-id]
(api/check-superuser)
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(api/check-not-archived (api/read-check Dashboard dashboard-id))
{:uuid (or (db/select-one-field :public_uuid Dashboard :id dashboard-id)
(u/prog1 (str (UUID/randomUUID))
@@ -525,7 +526,7 @@
"Delete the publicly-accessible link to this Dashboard."
[dashboard-id]
(api/check-superuser)
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(api/check-exists? Dashboard :id dashboard-id, :public_uuid [:not= nil], :archived false)
(db/update! Dashboard dashboard-id
:public_uuid nil
@@ -537,7 +538,7 @@
enabled."
[]
(api/check-superuser)
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(db/select [Dashboard :name :id :public_uuid], :public_uuid [:not= nil], :archived false))
(api/defendpoint GET "/embeddable"
@@ -545,7 +546,7 @@
endpoints and a signed JWT."
[]
(api/check-superuser)
- (api/check-embedding-enabled)
+ (validation/check-embedding-enabled)
(db/select [Dashboard :name :id], :enable_embedding true, :archived false))
(api/defendpoint GET "/:id/related"
diff --git a/src/metabase/api/embed.clj b/src/metabase/api/embed.clj
index 01e8fd6e9772e68d4c52e5fdb302937672d9c49a..3d2a2c5c7397f88b4053c406cc5237a5afef1fa2 100644
--- a/src/metabase/api/embed.clj
+++ b/src/metabase/api/embed.clj
@@ -20,6 +20,7 @@
[compojure.core :refer [GET]]
[medley.core :as m]
[metabase.api.common :as api]
+ [metabase.api.common.validation :as validation]
[metabase.api.dashboard :as dashboard-api]
[metabase.api.dataset :as dataset-api]
[metabase.api.public :as public-api]
@@ -283,7 +284,7 @@
(check-embedding-enabled-for-object (db/select-one [entity :enable_embedding] :id id)))
([object]
- (api/check-embedding-enabled)
+ (validation/check-embedding-enabled)
(api/check-404 object)
(api/check-not-archived object)
(api/check (:enable_embedding object)
diff --git a/src/metabase/api/preview_embed.clj b/src/metabase/api/preview_embed.clj
index 4e315dd5154d9e59a4ed8a8252a447cc495eaf33..ddb2a6662ba6e920d2d688908c3f573ac1793457 100644
--- a/src/metabase/api/preview_embed.clj
+++ b/src/metabase/api/preview_embed.clj
@@ -10,13 +10,14 @@
Refer to the documentation for those endpoints for further details."
(:require [compojure.core :refer [GET]]
[metabase.api.common :as api]
+ [metabase.api.common.validation :as validation]
[metabase.api.embed :as embed-api]
[metabase.query-processor.pivot :as qp.pivot]
[metabase.util.embed :as eu]))
(defn- check-and-unsign [token]
(api/check-superuser)
- (api/check-embedding-enabled)
+ (validation/check-embedding-enabled)
(eu/unsign token))
(api/defendpoint GET "/card/:token"
diff --git a/src/metabase/api/public.clj b/src/metabase/api/public.clj
index f9eb94bfa5880dfc522b7027911623eda3ad3536..a66a7ab237ec9d172226de81cd1a7889f7d22b77 100644
--- a/src/metabase/api/public.clj
+++ b/src/metabase/api/public.clj
@@ -5,6 +5,7 @@
[compojure.core :refer [GET]]
[medley.core :as m]
[metabase.api.common :as api]
+ [metabase.api.common.validation :as validation]
[metabase.api.dashboard :as dashboard-api]
[metabase.api.dataset :as dataset-api]
[metabase.api.field :as field-api]
@@ -60,7 +61,7 @@
"Fetch a publicly-accessible Card an return query results as well as `:card` information. Does not require auth
credentials. Public sharing must be enabled."
[uuid]
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(card-with-uuid uuid))
(defmulti ^:private transform-results
@@ -131,7 +132,7 @@
"Run query for a *public* Card with UUID. If public sharing is not enabled, this throws an exception. Returns a
`StreamingResponse` object that should be returned as the result of an API endpoint."
[uuid export-format parameters & options]
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [card-id (api/check-404 (db/select-one-id Card :public_uuid uuid, :archived false))]
(apply run-query-for-card-with-id-async card-id export-format parameters options)))
@@ -182,7 +183,7 @@
(api/defendpoint GET "/dashboard/:uuid"
"Fetch a publicly-accessible Dashboard. Does not require auth credentials. Public sharing must be enabled."
[uuid]
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(dashboard-with-uuid uuid))
;; TODO -- this should probably have a name like `run-query-for-dashcard...` so it matches up with
@@ -221,7 +222,7 @@
sharing must be enabled."
[uuid card-id dashcard-id parameters]
{parameters (s/maybe su/JSONString)}
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [dashboard-id (api/check-404 (db/select-one-id Dashboard :public_uuid uuid, :archived false))]
(public-dashcard-results-async
:dashboard-id dashboard-id
@@ -311,7 +312,7 @@
(api/defendpoint GET "/card/:uuid/field/:field-id/values"
"Fetch FieldValues for a Field that is referenced by a public Card."
[uuid field-id]
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [card-id (db/select-one-id Card :public_uuid uuid, :archived false)]
(card-and-field-id->values card-id field-id)))
@@ -325,7 +326,7 @@
(api/defendpoint GET "/dashboard/:uuid/field/:field-id/values"
"Fetch FieldValues for a Field that is referenced by a Card in a public Dashboard."
[uuid field-id]
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [dashboard-id (api/check-404 (db/select-one-id Dashboard :public_uuid uuid, :archived false))]
(dashboard-and-field-id->values dashboard-id field-id)))
@@ -353,7 +354,7 @@
[uuid field-id search-field-id value limit]
{value su/NonBlankString
limit (s/maybe su/IntStringGreaterThanZero)}
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [card-id (db/select-one-id Card :public_uuid uuid, :archived false)]
(search-card-fields card-id field-id search-field-id value (when limit (Integer/parseInt limit)))))
@@ -362,7 +363,7 @@
[uuid field-id search-field-id value limit]
{value su/NonBlankString
limit (s/maybe su/IntStringGreaterThanZero)}
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [dashboard-id (api/check-404 (db/select-one-id Dashboard :public_uuid uuid, :archived false))]
(search-dashboard-fields dashboard-id field-id search-field-id value (when limit (Integer/parseInt limit)))))
@@ -394,7 +395,7 @@
Cards."
[uuid field-id remapped-id value]
{value su/NonBlankString}
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [card-id (api/check-404 (db/select-one-id Card :public_uuid uuid, :archived false))]
(card-field-remapped-values card-id field-id remapped-id value)))
@@ -403,7 +404,7 @@
Dashboards."
[uuid field-id remapped-id value]
{value su/NonBlankString}
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [dashboard-id (db/select-one-id Dashboard :public_uuid uuid, :archived false)]
(dashboard-field-remapped-values dashboard-id field-id remapped-id value)))
@@ -438,7 +439,7 @@
sharing must be enabled."
[uuid card-id dashcard-id parameters]
{parameters (s/maybe su/JSONString)}
- (api/check-public-sharing-enabled)
+ (validation/check-public-sharing-enabled)
(let [dashboard-id (api/check-404 (db/select-one-id Dashboard :public_uuid uuid, :archived false))]
(public-dashcard-results-async
:dashboard-id dashboard-id
@@ -450,7 +451,7 @@
;;; ----------------------------------------- Route Definitions & Complaints -----------------------------------------
;; TODO - why don't we just make these routes have a bit of middleware that includes the
-;; `api/check-public-sharing-enabled` check in each of them? That way we don't need to remember to include the line in
+;; `validation/check-public-sharing-enabled` check in each of them? That way we don't need to remember to include the line in
;; every single endpoint definition here? Wouldn't that be 100x better?!
;;
;; TODO - also a smart person would probably just parse the UUIDs automatically in middleware as appropriate for
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index a4c797fe36e2effe2e598436034117da6e24f486..c68a9239e00cad5ac72018ec7b476d0574d6dc3a 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -8,6 +8,7 @@
[[metabase.driver.sql-jdbc]] for more details."
(:require [clojure.string :as str]
[clojure.tools.logging :as log]
+ [java-time :as t]
[metabase.driver.impl :as impl]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.plugins.classloader :as classloader]
@@ -33,6 +34,15 @@
(catch Throwable e
(log/error e (trs "Failed to notify {0} Database {1} updated" driver id))))))
+(defn- short-timezone-name [timezone-id]
+ (let [^java.time.ZoneId zone (if (seq timezone-id)
+ (t/zone-id timezone-id)
+ (t/zone-id))]
+ (.getDisplayName
+ zone
+ java.time.format.TextStyle/SHORT
+ (java.util.Locale/getDefault))))
+
(defsetting report-timezone
(deferred-tru "Connection timezone to use when executing queries. Defaults to system timezone.")
:setter
@@ -40,6 +50,12 @@
(setting/set-value-of-type! :string :report-timezone new-value)
(notify-all-databases-updated)))
+(defsetting report-timezone-short
+ "Current report timezone abbreviation"
+ :visibility :public
+ :setter :none
+ :getter (fn [] (short-timezone-name (report-timezone))))
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | Current Driver |
diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj
index 786e982ae75a8e9c370a2ff037f8376d63ea97d2..9a19307c62c03384ebdd776ff0102f7f2d2eb877 100644
--- a/src/metabase/driver/util.clj
+++ b/src/metabase/driver/util.clj
@@ -334,6 +334,12 @@
:driver-name (driver/display-name driver)
:superseded-by (driver/superseded-by driver)}])))
+(defsetting engines
+ "Available database engines"
+ :visibility :public
+ :setter :none
+ :getter available-drivers-info)
+
;;; +----------------------------------------------------------------------------------------------------------------+
;;; | TLS Helpers |
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/src/metabase/models/setting.clj b/src/metabase/models/setting.clj
index 411d4eb78188886ef4bd21553281c8ce2baf01ab..7194562985d13ce8149d568fdaabcbf440ffbbeb 100644
--- a/src/metabase/models/setting.clj
+++ b/src/metabase/models/setting.clj
@@ -32,39 +32,45 @@
[[admin-writable-settings]] and [[user-readable-values-map]] can be used to fetch *all* Admin-writable and
User-readable Settings, respectively. See their docstrings for more information.
- ### Database-Local Settings
+ ### User-local and Database-local Settings
Starting in 0.42.0, some Settings are allowed to have Database-specific values that override the normal site-wide
- value. These are similar in concept to buffer-local variables in Emacs Lisp.
+ value. Similarly, starting in 0.43.0, some Settings are allowed to have User-specific values. These are similar in
+ concept to buffer-local variables in Emacs Lisp.
- When a Setting is allowed to be Database-local, any values in [[*database-local-values*]] for that Setting will be
- returned preferentially to site-wide values of that Setting. [[*database-local-values*]] comes from the
- `Database.settings` column in the application DB. `nil` values in [[*database-local-values*]] are ignored, i.e. you
- cannot 'unset' a site-wide value with a Database-local one.
+ When a Setting is allowed to be User or Database local, any values in [[*user-local-values*]] or
+ [[*database-local-values*]] for that Setting will be returned preferentially to site-wide values of that Setting.
+ [[*user-local-values*]] comes from the `User.settings` column in the application DB, and [[*database-local-values*]]
+ comes from the `Database.settings` column. `nil` values in [[*user-local-values*]] and [[*database-local-values*]]
+ are ignored, i.e. you cannot 'unset' a site-wide value with a User- or Database-local one.
- Whether or not a Setting can be Database-local is controlled by the `:database-local` option passed
- to [[defsetting]]. There are three valid values of this option:
+ Whether or not a Setting can be User- or Database-local is controlled by the `:user-local` and `:database-local`
+ options passed to [[defsetting]]. A Setting can only be User-local *or* Database-local, not both; this is enforced
+ when the Setting is defined. There are three valid values of these options:
- * `:only` means this Setting can *only* have a Database-local value and cannot have a 'normal' site-wide value. It
- cannot be set via env var. Default values are still allowed for Database-local-only Settings. Database-local-only
- Settings are never returned by [[admin-writable-settings]] or [[user-readable-values-map]] regardless of
- their [[Visibility]].
+ * `:only` means this Setting can *only* have a User- or Database-local value and cannot have a 'normal' site-wide
+ value. It cannot be set via env var. Default values are still allowed for User- and Database-local-only Settings.
+ User- and Database-local-only Settings are never returned by [[admin-writable-settings]] or
+ [[user-readable-values-map]] regardless of their [[Visibility]].
- * `:allowed` means this Setting can be Database-local and can also have a normal site-wide value; if both are
- specified, the Database-specific value will be returned preferentially when we are in the context of a specific
- Database (i.e., [[*database-local-values*]] is bound).
+ * `:allowed` means this Setting can be User- or Database-local and can also have a normal site-wide value; if both
+ are specified, the User- or Database-specific value will be returned preferentially when we are in the context of a
+ specific User or Database (i.e., [[*user-local-values*]] or [[*database-local-values*]] is bound).
- * `:never` means Database-specific values cannot be set for this Setting. Values in [[*database-local-values*]]
- will be ignored.
+ * `:never` means User- or Database-specific values cannot be set for this Setting. Values in [[*user-local-values*]]
+ and [[*database-local-values*]] will be ignored.
- `:never` is the default value of `:database-local`; to allow Database-local values, the Setting definition must
- explicitly specify `:database-local` `:only` or `:allowed`.
+ `:never` is the default value of both `:user-local` and `:database-local`; to allow User- or Database-local values,
+ the Setting definition must explicitly specify `:only` or `:allowed` for the appropriate option.
- Setting setter functions do not affect Database-local values; they always set the site-wide value. (At the time of
- this writing, there is not yet a FE-client-friendly way to set Database-local values. Just set them manually in the
- application DB until we figure that out.)
+ If a User-local setting is written in the context of an API request (i.e., when [[metabase.api.common/*current-user*]]
+ is bound), the value will be local to the current user. If it is written outside of an API request, a site-wide
+ value will be written. (At the time of this writing, there is not yet a FE-client-friendly way to set Database-local
+ values. Just set them manually in the application DB until we figure that out.)
- See #19399 for more information about and motivation behind Database-local Settings."
+ Custom setter functions do not affect User- or Database-local values; they always set the site-wide value.
+
+ See #14055 and #19399 for more information about and motivation behind User- and Database-local Settings."
(:refer-clojure :exclude [get])
(:require [cheshire.core :as json]
[clojure.core :as core]
@@ -74,6 +80,7 @@
[clojure.tools.logging :as log]
[environ.core :as env]
[medley.core :as m]
+ [metabase.api.common :as api]
[metabase.models.setting.cache :as cache]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
@@ -98,6 +105,15 @@
TODO -- we should probably also bind this in sync contexts e.g. functions in [[metabase.sync]]."
nil)
+(def ^:dynamic *user-local-values*
+ "User-local Settings values (as a map of Setting name -> already-deserialized value). This comes from the value of
+ `User.settings` in the application DB. When bound, any Setting that *can* be User-local will have a value from this
+ map returned preferentially to the site-wide value.
+
+ This is normally bound automatically by session middleware, in
+ [[metabase.server.middleware.session/do-with-current-user]]."
+ (atom nil))
+
(def ^:private retired-setting-names
"A set of setting names which existed in previous versions of Metabase, but are no longer used. New settings may not use
these names to avoid unintended side-effects if an application database still stores values for these settings."
@@ -189,8 +205,9 @@
:cache? s/Bool ; should the getter always fetch this value "fresh" from the DB? (default: false)
:deprecated (s/maybe s/Str) ; if non-nil, contains the Metabase version in which this setting was deprecated
- ;; whether this Setting can be Database-local. See [[metabase.models.setting]] docstring for more info.
+ ;; whether this Setting can be Database-local or User-local. See [[metabase.models.setting]] docstring for more info.
:database-local LocalOption
+ :user-local LocalOption
;; called whenever setting value changes, whether from update-setting! or a cache refresh. used to handle cases
;; where a change to the cache necessitates a change to some value outside the cache, like when a change the
@@ -251,17 +268,42 @@
(setting-name [this]
(name this)))
+(defn- allows-site-wide-values? [setting]
+ (and
+ (not= (:database-local (resolve-setting setting)) :only)
+ (not= (:user-local (resolve-setting setting)) :only)))
+
(defn- allows-database-local-values? [setting]
(#{:only :allowed} (:database-local (resolve-setting setting))))
-(defn- allows-site-wide-values? [setting]
- (not= (:database-local (resolve-setting setting)) :only))
-
(defn- database-local-value [setting-definition-or-name]
(let [{setting-name :name, :as setting} (resolve-setting setting-definition-or-name)]
(when (allows-database-local-values? setting)
(core/get *database-local-values* setting-name))))
+(defn- allows-user-local-values? [setting]
+ (#{:only :allowed} (:user-local (resolve-setting setting))))
+
+(defn- user-local-value [setting-definition-or-name]
+ (let [{setting-name :name, :as setting} (resolve-setting setting-definition-or-name)]
+ (when (allows-user-local-values? setting)
+ (core/get @*user-local-values* setting-name))))
+
+(defn- should-set-user-local-value? [setting-definition-or-name]
+ (let [setting (resolve-setting setting-definition-or-name)]
+ (and (allows-user-local-values? setting)
+ @*user-local-values*)))
+
+(defn- set-user-local-value! [setting-definition-or-name value]
+ (let [{setting-name :name} (resolve-setting setting-definition-or-name)]
+ ;; Update the atom in *user-local-values* with the new value before writing to the DB. This ensures that
+ ;; subsequent setting updates within the same API request will not overwrite this value.
+ (swap! *user-local-values*
+ (fn [old-settings] (if value
+ (assoc old-settings setting-name value)
+ (dissoc old-settings setting-name))))
+ (db/update! 'User api/*current-user-id* {:settings (json/generate-string @*user-local-values*)})))
+
(defn- munge-setting-name
"Munge names so that they are legal for bash. Only allows for alphanumeric characters, underscores, and hyphens."
[setting-nm]
@@ -313,10 +355,11 @@
"Get the raw value of a Setting from wherever it may be specified. Value is fetched by trying the following sources in
order:
- 1. From [[*database-local-values*]] if this Setting is allowed to have a Database-local value
- 2. From the corresponding env var (excluding empty string values)
- 3. From the application database (i.e., set via the admin panel) (excluding empty string values)
- 4. The default value, if one was specified
+ 1. From [[*user-local-values*]] if this Setting is allowed to have User-local values
+ 2. From [[*database-local-values*]] if this Setting is allowed to have Database-local values
+ 3. From the corresponding env var (excluding empty string values)
+ 4. From the application database (i.e., set via the admin panel) (excluding empty string values)
+ 5. The default value, if one was specified
!!!!!!!!!! The value returned MAY OR MAY NOT be a String depending on the source !!!!!!!!!!
@@ -328,7 +371,8 @@
conditions values can be returned directly (`pred`) -- see [[get-value-of-type]] for `:boolean` for example usage."
([setting-definition-or-name]
(let [setting (resolve-setting setting-definition-or-name)
- source-fns [database-local-value
+ source-fns [user-local-value
+ database-local-value
env-var-value
db-or-cache-value
default-value]]
@@ -481,10 +525,7 @@
:as setting} (resolve-setting setting-definition-or-name)
obfuscated? (and sensitive? (obfuscated-value? new-value))
setting-name (setting-name setting)]
- ;; make sure we're not trying to set the value of a Database-local-only Setting or something like that
- (when-not (allows-site-wide-values? setting)
- (throw (ex-info (tru "Site-wide values are not allowed for Setting {0}" (:name setting))
- {: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))
@@ -493,27 +534,36 @@
(log/warn (trs "Setting {0} is deprecated as of Metabase {1} and may be removed in a future version."
setting-name
deprecated)))
- ;; always update the cache entirely when updating a Setting.
- (cache/restore-cache!)
- ;; 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!))
+ (if (should-set-user-local-value? setting)
+ ;; If this is user-local and this is being set in the context of an API call, we don't want to update the
+ ;; site-wide value or write or read from the cache
+ (set-user-local-value! setting-name new-value)
+ (do
+ ;; make sure we're not trying to set the value of a Database-local-only Setting
+ (when-not (allows-site-wide-values? setting)
+ (throw (ex-info (tru "Site-wide values are not allowed for Setting {0}" (:name setting))
+ {:setting (:name setting)})))
+ ;; always update the cache entirely when updating a Setting.
+ (cache/restore-cache!)
+ ;; 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))))
@@ -630,6 +680,7 @@
:sensitive? false
:cache? true
:database-local :never
+ :user-local :never
:deprecated nil}
(dissoc setting :name :type :default)))
(s/validate SettingDefinition <>)
@@ -650,6 +701,10 @@
(throw (ex-info (tru "Setting name ''{0}'' is retired; use a different name instead" (name setting-name))
{:retired-setting-name (name setting-name)
:new-setting (dissoc <> :on-change :getter :setter)})))
+ (when (and (allows-user-local-values? setting) (allows-database-local-values? setting))
+ (throw (ex-info (tru "Setting {0} allows both user-local and database-local values; this is not supported"
+ setting-name)
+ {:setting setting})))
(swap! registered-settings assoc setting-name <>))))
;;; +----------------------------------------------------------------------------------------------------------------+
@@ -905,12 +960,10 @@
`options` are passed to [[user-facing-value]].
This is currently used by `GET /api/setting` ([[metabase.api.setting/GET_]]; admin-only; powers the Admin Settings
- page) so all admin-visible Settings should be included. Also used
- by [[metabase-enterprise.serialization.dump/dump-settings]] which should also have access to everything not
- `:internal`. In either case we *do not* want to return env var values -- we don't want to serialize them regardless
- of whether the value should be readable or not, and admins should not be allowed to modify them."
+ page) so all admin-visible Settings should be included. We *do not* want to return env var values, since admins
+ are not allowed to modify them."
[& {:as options}]
- ;; ignore Database-local values even if this is bound for some reason
+ ;; ignore User-local and Database-local values
(binding [*database-local-values* nil]
(into
[]
@@ -920,6 +973,16 @@
(map #(m/mapply user-facing-info % options)))
(sort-by :name (vals @registered-settings)))))
+(defn admin-writable-site-wide-settings
+ "Returns a sequence of site-wide Settings maps, similar to [[admin-writable-settings]]. However, this function
+ excludes User-local Settings in addition to Database-local Settings. Settings that are optionally user-local will
+ be included with their site-wide value, if a site-wide value is set.
+
+ This is used in [[metabase-enterprise.serialization.dump/dump-settings]] to serialize site-wide Settings."
+ [& options]
+ (binding [*user-local-values* (atom nil)]
+ (apply admin-writable-settings options)))
+
(defn user-readable-values-map
"Returns Settings as a map of setting name -> site-wide value for a given [[Visibility]] e.g. `:public`.
@@ -930,7 +993,7 @@
the frontend client. For that reason, these Settings *should* include values that come back from environment
variables, *unless* they are marked `:sensitive?`."
[visibility]
- ;; ignore Database-local values even if this is bound for some reason
+ ;; ignore Database-local values, but not User-local values
(binding [*database-local-values* nil]
(into
{}
diff --git a/src/metabase/models/user.clj b/src/metabase/models/user.clj
index 9c81b7b178dab9b171c8af796993acac9b21eb31..09f475aad7adbda0ae95796b01fdb175af349bc1 100644
--- a/src/metabase/models/user.clj
+++ b/src/metabase/models/user.clj
@@ -134,7 +134,8 @@
:post-insert post-insert
:pre-update pre-update
:post-select post-select
- :types (constantly {:login_attributes :json-no-keywordization})}))
+ :types (constantly {:login_attributes :json-no-keywordization
+ :settings :encrypted-json})}))
(defn group-ids
"Fetch set of IDs of PermissionsGroup a User belongs to."
diff --git a/src/metabase/public_settings.clj b/src/metabase/public_settings.clj
index 628c3837ed0b3c8b495f305dcae311fd09cd4afb..404ceae1c917f7ebbeeefc8359d3f86570599e78 100644
--- a/src/metabase/public_settings.clj
+++ b/src/metabase/public_settings.clj
@@ -6,8 +6,6 @@
[clojure.tools.logging :as log]
[java-time :as t]
[metabase.config :as config]
- [metabase.driver :as driver]
- [metabase.driver.util :as driver.u]
[metabase.models.setting :as setting :refer [defsetting]]
[metabase.plugins.classloader :as classloader]
[metabase.public-settings.premium-features :as premium-features]
@@ -354,15 +352,6 @@
(assoc object :public_uuid nil)
object))
-(defn- short-timezone-name [timezone-id]
- (let [^java.time.ZoneId zone (if (seq timezone-id)
- (t/zone-id timezone-id)
- (t/zone-id))]
- (.getDisplayName
- zone
- java.time.format.TextStyle/SHORT
- (java.util.Locale/getDefault))))
-
(defsetting available-locales
"Available i18n locales"
:visibility :public
@@ -375,12 +364,6 @@
:setter :none
:getter (comp sort t/available-zone-ids))
-(defsetting engines
- "Available database engines"
- :visibility :public
- :setter :none
- :getter driver.u/available-drivers-info)
-
(defsetting has-sample-database?
"Whether this instance has a Sample Database database"
:visibility :authenticated
@@ -399,12 +382,6 @@
:visibility :public
:default nil)
-(defsetting report-timezone-short
- "Current report timezone abbreviation"
- :visibility :public
- :setter :none
- :getter (fn [] (short-timezone-name (driver/report-timezone))))
-
(defsetting version
"Metabase's version info"
:visibility :public
diff --git a/src/metabase/server/middleware/session.clj b/src/metabase/server/middleware/session.clj
index 03d580fbe07bb4c51846d70d8e527b79be07b5dd..320234b1a71c0569a30a45ee09ade921c9de93dd 100644
--- a/src/metabase/server/middleware/session.clj
+++ b/src/metabase/server/middleware/session.clj
@@ -1,6 +1,7 @@
(ns metabase.server.middleware.session
"Ring middleware related to session (binding current user and permissions)."
- (:require [clojure.java.jdbc :as jdbc]
+ (:require
+ [clojure.java.jdbc :as jdbc]
[clojure.tools.logging :as log]
[honeysql.core :as hsql]
[metabase.api.common :refer [*current-user* *current-user-id* *current-user-permissions-set* *is-superuser?*]]
@@ -9,6 +10,7 @@
[metabase.db :as mdb]
[metabase.driver.sql.query-processor :as sql.qp]
[metabase.models.session :refer [Session]]
+ [metabase.models.setting :refer [*user-local-values*]]
[metabase.models.user :as user :refer [User]]
[metabase.public-settings :as public-settings]
[metabase.server.request.util :as request.u]
@@ -234,12 +236,13 @@
(defn do-with-current-user
"Impl for `with-current-user`."
- [{:keys [metabase-user-id is-superuser? user-locale]} thunk]
+ [{:keys [metabase-user-id is-superuser? user-locale settings]} thunk]
(binding [*current-user-id* metabase-user-id
i18n/*user-locale* user-locale
*is-superuser?* (boolean is-superuser?)
*current-user* (delay (find-user metabase-user-id))
- *current-user-permissions-set* (delay (some-> metabase-user-id user/permissions-set))]
+ *current-user-permissions-set* (delay (some-> metabase-user-id user/permissions-set))
+ *user-local-values* (atom (or settings {}))]
(thunk)))
(defmacro ^:private with-current-user-for-request
@@ -247,15 +250,16 @@
`(do-with-current-user ~request (fn [] ~@body)))
(defn bind-current-user
- "Middleware that binds `metabase.api.common/*current-user*`, `*current-user-id*`, `*is-superuser?*`, and
- `*current-user-permissions-set*`.
+ "Middleware that binds [[metabase.api.common/*current-user*]], [[*current-user-id*]], [[*is-superuser?*]],
+ [[*current-user-permissions-set*]], and [[metabase.models.setting/*user-local-values*]].
* `*current-user-id*` int ID or nil of user associated with request
* `*current-user*` delay that returns current user (or nil) from DB
* `metabase.util.i18n/*user-locale*` ISO locale code e.g `en` or `en-US` to use for the current User.
Overrides `site-locale` if set.
* `*is-superuser?*` Boolean stating whether current user is a superuser.
- * `current-user-permissions-set*` delay that returns the set of permissions granted to the current user from DB"
+ * `current-user-permissions-set*` delay that returns the set of permissions granted to the current user from DB
+ * `*user-local-values*` atom containing a map of user-local settings and values for the current user"
[handler]
(fn [request respond raise]
(with-current-user-for-request request
@@ -265,7 +269,7 @@
"Part of the impl for `with-current-user` -- don't use this directly."
[current-user-id]
(when current-user-id
- (db/select-one [User [:id :metabase-user-id] [:is_superuser :is-superuser?] [:locale :user-locale]]
+ (db/select-one [User [:id :metabase-user-id] [:is_superuser :is-superuser?] [:locale :user-locale] :settings]
:id current-user-id)))
(defmacro with-current-user
diff --git a/test/metabase/models/setting_test.clj b/test/metabase/models/setting_test.clj
index ada4090ee31d46c8936c7ad67698b83279ec3918..4d4db4b0cf4a65aa6ed7c1694140a28b43e05988 100644
--- a/test/metabase/models/setting_test.clj
+++ b/test/metabase/models/setting_test.clj
@@ -708,7 +708,6 @@
(db/delete! Setting :key (name setting-name))
(cache/restore-cache!)))))
(fn [thunk]
- ()
(tu/do-with-temp-env-var-value
(keyword (str "mb-" (name setting-name)))
site-wide-value
@@ -824,3 +823,74 @@
(catch Exception e
(is (= "Setting name 'retired-setting' is retired; use a different name instead"
(ex-message e))))))))
+
+(defsetting ^:private test-user-local-only-setting
+ "test Setting"
+ :visibility :internal
+ :type :integer
+ :user-local :only)
+
+(defsetting ^:private test-user-local-allowed-setting
+ (deferred-tru "test Setting")
+ :visibility :authenticated
+ :type :integer
+ :user-local :allowed)
+
+(defsetting ^:private test-user-local-never-setting
+ "test Setting"
+ :visibility :internal
+ :type :integer) ; `:never` should be the default
+
+(deftest user-local-settings-test
+ (testing "Reading and writing a user-local-only setting in the context of a user uses the user-local value"
+ (mt/with-current-user (mt/user->id :rasta)
+ (test-user-local-only-setting 1)
+ (is (= 1 (test-user-local-only-setting))))
+ (mt/with-current-user (mt/user->id :crowberto)
+ (test-user-local-only-setting 2)
+ (is (= 2 (test-user-local-only-setting))))
+ (mt/with-current-user (mt/user->id :rasta)
+ (is (= 1 (test-user-local-only-setting)))))
+
+ (testing "A user-local-only setting cannot have a site-wide value"
+ (is (thrown-with-msg? Throwable #"Site-wide values are not allowed" (test-user-local-only-setting 1))))
+
+ (testing "Reading and writing a user-local-allowed setting in the context of a user uses the user-local value"
+ ;; TODO: mt/with-temporary-setting-values only affects site-wide value, we should figure out whether it should also
+ ;; affect user-local settings.
+ (mt/with-temporary-setting-values [test-user-local-allowed-setting nil]
+ (mt/with-current-user (mt/user->id :rasta)
+ (test-user-local-allowed-setting 1)
+ (is (= 1 (test-user-local-allowed-setting))))
+ (mt/with-current-user (mt/user->id :crowberto)
+ (test-user-local-allowed-setting 2)
+ (is (= 2 (test-user-local-allowed-setting))))
+ (mt/with-current-user (mt/user->id :rasta)
+ (is (= 1 (test-user-local-allowed-setting))))
+ ;; Calling the setter when not in the context of a user should set the site-wide value
+ (is (nil? (test-user-local-allowed-setting)))
+ (test-user-local-allowed-setting 3)
+ (mt/with-current-user (mt/user->id :crowberto)
+ (is (= 2 (test-user-local-allowed-setting))))
+ (mt/with-current-user (mt/user->id :rasta)
+ (is (= 1 (test-user-local-allowed-setting))))))
+
+ (testing "Reading and writing a user-local-never setting in the context of a user uses the site-wide value"
+ (mt/with-current-user (mt/user->id :rasta)
+ (test-user-local-never-setting 1)
+ (is (= 1 (test-user-local-never-setting))))
+ (mt/with-current-user (mt/user->id :crowberto)
+ (test-user-local-never-setting 2)
+ (is (= 2 (test-user-local-never-setting))))
+ (mt/with-current-user (mt/user->id :rasta)
+ (is (= 2 (test-user-local-never-setting))))
+ (is (= 2 (test-user-local-never-setting))))
+
+ (testing "A setting cannot be defined to allow both user-local and database-local values"
+ (is (thrown-with-msg?
+ Throwable
+ #"Setting .* allows both user-local and database-local values; this is not supported"
+ (defsetting test-user-local-and-db-local-setting
+ (deferred-tru "test Setting")
+ :user-local :allowed
+ :database-local :allowed)))))
diff --git a/test/metabase/server/middleware/session_test.clj b/test/metabase/server/middleware/session_test.clj
index 15686abeed8894696f12dcb0b396271917ae4273..d2854d20e2de54db326371a73342dc2d96297197 100644
--- a/test/metabase/server/middleware/session_test.clj
+++ b/test/metabase/server/middleware/session_test.clj
@@ -166,7 +166,7 @@
(deftest no-session-id-in-request-test
(testing "no session-id in the request"
(is (= nil
- (-> (wrapped-handler (mock/request :get "/anyurl") )
+ (-> (wrapped-handler (mock/request :get "/anyurl"))
:metabase-session-id)))))
(deftest header-test