From b09db3b450047b4b94e6d595ecb1b3763b3ceb78 Mon Sep 17 00:00:00 2001
From: bryan <bryan.maass@gmail.com>
Date: Thu, 29 Feb 2024 15:43:36 -0800
Subject: [PATCH] Tech Debt: teamzero (#39417)
* [AS+BCM] fix first 3 todos
* [AS+BCM] add issues
* add jdbc proxy issue
* driver/driver-features -> driver/features
* killing more todos and remove `supports?`
* add more issues
* dont let people who dont have snippet perms access snippets
* adding more issues
* replace calls to support? in mongo driver
* fix namespace sorting
* whitespace
---
.clj-kondo/config.edn | 2 +-
.../sso/integrations/jwt.clj | 4 +-
.../sso/integrations/saml.clj | 4 +-
.../sandbox/api/gtap_test.clj | 4 +-
.../athena/test/metabase/test/data/athena.clj | 2 +-
.../metabase/driver/bigquery_cloud_sdk.clj | 2 +
.../mongo/src/metabase/driver/mongo.clj | 12 +-----
.../mongo/test/metabase/test/data/mongo.clj | 5 ---
src/metabase/api/database.clj | 6 ++-
src/metabase/api/session.clj | 30 +++++++-------
src/metabase/api/user.clj | 4 +-
src/metabase/driver.clj | 39 +++++--------------
src/metabase/driver/util.clj | 2 +-
src/metabase/lib/join.cljc | 2 +-
src/metabase/lib/js.cljs | 2 +-
src/metabase/lib/schema/metadata.cljc | 2 +-
src/metabase/models/collection.clj | 5 +--
src/metabase/models/dashboard.clj | 2 +
src/metabase/models/humanization.clj | 1 +
src/metabase/models/login_history.clj | 6 +--
src/metabase/models/session.clj | 4 +-
src/metabase/plugins/jdbc_proxy.clj | 1 +
src/metabase/server/middleware/auth.clj | 8 ++--
.../server/middleware/browser_cookie.clj | 4 +-
src/metabase/server/middleware/log.clj | 4 +-
src/metabase/server/middleware/misc.clj | 4 +-
src/metabase/server/middleware/security.clj | 18 ++-------
src/metabase/server/middleware/session.clj | 10 ++---
src/metabase/server/middleware/ssl.clj | 4 +-
src/metabase/server/middleware/util.clj | 5 ---
src/metabase/server/request/util.clj | 9 +++++
test/metabase/api/alert_test.clj | 6 +--
test/metabase/api/card_test.clj | 6 +--
test/metabase/api/dashboard_test.clj | 4 +-
test/metabase/api/metric_test.clj | 6 +--
test/metabase/api/notify_test.clj | 4 +-
test/metabase/api/pulse_test.clj | 6 +--
test/metabase/api/segment_test.clj | 6 +--
test/metabase/api/table_test.clj | 6 +--
test/metabase/api/timeline_event_test.clj | 6 +--
test/metabase/api/timeline_test.clj | 6 +--
test/metabase/api/user_test.clj | 6 +--
test/metabase/models/login_history_test.clj | 4 +-
test/metabase/query_processor/test_util.clj | 4 +-
test/metabase/server/middleware/auth_test.clj | 12 +++---
test/metabase/server/request/util_test.clj | 18 ++++-----
46 files changed, 137 insertions(+), 170 deletions(-)
delete mode 100644 src/metabase/server/middleware/util.clj
diff --git a/.clj-kondo/config.edn b/.clj-kondo/config.edn
index 32c53cecb9a..059b7e9a8ee 100644
--- a/.clj-kondo/config.edn
+++ b/.clj-kondo/config.edn
@@ -340,7 +340,7 @@
metabase.server.middleware.security mw.security
metabase.server.middleware.session mw.session
metabase.server.middleware.ssl mw.ssl
- metabase.server.middleware.util mw.util
+ metabase.server.request.util req.util
metabase.server.protocols server.protocols
metabase.shared.util shared.u
metabase.shared.util.currency currency
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj
index 0fd300419a1..678a6730685 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/jwt.clj
@@ -12,7 +12,7 @@
[metabase.integrations.common :as integrations.common]
[metabase.public-settings.premium-features :as premium-features]
[metabase.server.middleware.session :as mw.session]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util.i18n :refer [tru]]
[ring.util.response :as response])
(:import
@@ -91,7 +91,7 @@
first-name (get jwt-data (jwt-attribute-firstname))
last-name (get jwt-data (jwt-attribute-lastname))
user (fetch-or-create-user! first-name last-name email login-attrs)
- session (api.session/create-session! :sso user (request.u/device-info request))]
+ session (api.session/create-session! :sso user (req.util/device-info request))]
(sync-groups! user jwt-data)
(mw.session/set-session-cookies request (response/redirect redirect-url) session (t/zoned-date-time (t/zone-id "GMT"))))))
diff --git a/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj b/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj
index 5c0c9fa5b34..d28f7db153c 100644
--- a/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj
+++ b/enterprise/backend/src/metabase_enterprise/sso/integrations/saml.clj
@@ -44,7 +44,7 @@
[metabase.public-settings :as public-settings]
[metabase.public-settings.premium-features :as premium-features]
[metabase.server.middleware.session :as mw.session]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util :as u]
[metabase.util.i18n :refer [trs tru]]
[metabase.util.log :as log]
@@ -230,7 +230,7 @@
:email email
:group-names groups
:user-attributes attrs
- :device-info (request.u/device-info request)})
+ :device-info (req.util/device-info request)})
response (response/redirect (or continue-url (public-settings/site-url)))]
(mw.session/set-session-cookies request response session (t/zoned-date-time (t/zone-id "GMT"))))))
diff --git a/enterprise/backend/test/metabase_enterprise/sandbox/api/gtap_test.clj b/enterprise/backend/test/metabase_enterprise/sandbox/api/gtap_test.clj
index 9b83a658979..c63a58f913c 100644
--- a/enterprise/backend/test/metabase_enterprise/sandbox/api/gtap_test.clj
+++ b/enterprise/backend/test/metabase_enterprise/sandbox/api/gtap_test.clj
@@ -6,7 +6,7 @@
[metabase.models :refer [Card Field PermissionsGroup Table]]
[metabase.models.permissions :as perms]
[metabase.public-settings.premium-features :as premium-features]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[toucan2.core :as t2]
[toucan2.tools.with-temp :as t2.with-temp]))
@@ -14,7 +14,7 @@
(deftest require-auth-test
(testing "Must be authenticated to query for GTAPs"
(mt/with-premium-features #{:sandboxes}
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "mt/gtap")))
(is (= "You don't have permissions to do that."
diff --git a/modules/drivers/athena/test/metabase/test/data/athena.clj b/modules/drivers/athena/test/metabase/test/data/athena.clj
index 28659189281..93b126b114b 100644
--- a/modules/drivers/athena/test/metabase/test/data/athena.clj
+++ b/modules/drivers/athena/test/metabase/test/data/athena.clj
@@ -184,7 +184,7 @@
:type/Time "TIMESTAMP"}]
(defmethod sql.tx/field-base-type->sql-type [:athena base-type] [_ _] sql-type))
-;; I'm not sure why `driver/supports?` above doesn't rectify this, but make `add-fk-sql a noop
+;; TODO: Maybe make `add-fk-sql a noop
(defmethod sql.tx/add-fk-sql :athena [& _] nil)
;; Athena can only execute one statement at a time
diff --git a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj
index ae9538a92ca..978cc425ca1 100644
--- a/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj
+++ b/modules/drivers/bigquery-cloud-sdk/src/metabase/driver/bigquery_cloud_sdk.clj
@@ -512,6 +512,8 @@
(t2/update! Database db-id {:details (:details updated-db)})
updated-db)))
+;; TODO: THIS METHOD SHOULD NOT BE UPDATING THE APP-DB (which it does in [convert-dataset-id-to-filters!])
+;; Issue: https://github.com/metabase/metabase/issues/39392
(defmethod driver/normalize-db-details :bigquery-cloud-sdk
[_driver {:keys [details] :as database}]
(when-not (empty? (filter some? ((juxt :auth-code :client-id :client-secret) details)))
diff --git a/modules/drivers/mongo/src/metabase/driver/mongo.clj b/modules/drivers/mongo/src/metabase/driver/mongo.clj
index 30ba5305964..22ef38d019f 100644
--- a/modules/drivers/mongo/src/metabase/driver/mongo.clj
+++ b/modules/drivers/mongo/src/metabase/driver/mongo.clj
@@ -23,8 +23,8 @@
[metabase.util.log :as log]
[taoensso.nippy :as nippy])
(:import
- (org.bson.types ObjectId)
- (com.mongodb.client MongoClient MongoDatabase)))
+ (com.mongodb.client MongoClient MongoDatabase)
+ (org.bson.types ObjectId)))
(set! *warn-on-reflection* true)
@@ -271,14 +271,6 @@
:index-info true}]
(defmethod driver/database-supports? [:mongo feature] [_driver _feature _db] supported?))
-;; We say Mongo supports foreign keys so that the front end can use implicit
-;; joins. In reality, Mongo doesn't support foreign keys.
-;; Only define an implementation for `:foreign-keys` if none exists already.
-;; In test extensions we define an alternate implementation, and we don't want
-;; to stomp over that if it was loaded already.
-(when-not (get (methods driver/supports?) [:mongo :foreign-keys])
- (defmethod driver/supports? [:mongo :foreign-keys] [_ _] true))
-
(defmethod driver/database-supports? [:mongo :schemas] [_driver _feat _db] false)
(defmethod driver/database-supports? [:mongo :expressions]
diff --git a/modules/drivers/mongo/test/metabase/test/data/mongo.clj b/modules/drivers/mongo/test/metabase/test/data/mongo.clj
index b09c1858766..fb5129698cc 100644
--- a/modules/drivers/mongo/test/metabase/test/data/mongo.clj
+++ b/modules/drivers/mongo/test/metabase/test/data/mongo.clj
@@ -6,8 +6,6 @@
[clojure.test :refer :all]
[flatland.ordered.map :as ordered-map]
[medley.core :as m]
- [metabase.config :as config]
- [metabase.driver :as driver]
[metabase.driver.ddl.interface :as ddl.i]
[metabase.driver.mongo.connection :as mongo.connection]
[metabase.driver.mongo.util :as mongo.util]
@@ -23,9 +21,6 @@
(defmethod tx/supports-time-type? :mongo [_driver] false)
(defmethod tx/supports-timestamptz-type? :mongo [_driver] false)
-;; during unit tests don't treat Mongo as having FK support
-(defmethod driver/supports? [:mongo :foreign-keys] [_ _] (not config/is-test?))
-
(defn ssl-required?
"Returns if the mongo server requires an SSL connection."
[]
diff --git a/src/metabase/api/database.clj b/src/metabase/api/database.clj
index 156afd71400..59b3d313c38 100644
--- a/src/metabase/api/database.clj
+++ b/src/metabase/api/database.clj
@@ -448,7 +448,11 @@
(let [db (-> (if include-editable-data-model?
(api/check-404 (t2/select-one Database :id id))
(api/read-check Database id))
- (t2/hydrate [:tables [:fields [:target :has_field_values] :has_field_values] :segments :metrics]))
+ (t2/hydrate [:tables [:fields
+ :has_field_values
+ [:target :has_field_values]]
+ :segments
+ :metrics]))
db (if include-editable-data-model?
;; We need to check data model perms after hydrating tables, since this will also filter out tables for
;; which the *current-user* does not have data model perms
diff --git a/src/metabase/api/session.clj b/src/metabase/api/session.clj
index 53bf8434246..455f78918e2 100644
--- a/src/metabase/api/session.clj
+++ b/src/metabase/api/session.clj
@@ -19,7 +19,7 @@
[metabase.models.user :as user :refer [User]]
[metabase.public-settings :as public-settings]
[metabase.server.middleware.session :as mw.session]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru trs tru]]
[metabase.util.log :as log]
@@ -37,7 +37,7 @@
(mu/defn ^:private record-login-history!
[session-id :- (ms/InstanceOfClass UUID)
user-id :- ms/PositiveInt
- device-info :- request.u/DeviceInfo]
+ device-info :- req.util/DeviceInfo]
(t2/insert! LoginHistory (merge {:user_id user-id
:session_id (str session-id)}
device-info)))
@@ -62,7 +62,7 @@
[:type [:enum :normal :full-app-embed]]]])
(mu/defmethod create-session! :sso :- SessionSchema
- [_ user :- CreateSessionUserInfo device-info :- request.u/DeviceInfo]
+ [_ user :- CreateSessionUserInfo device-info :- req.util/DeviceInfo]
(let [session-uuid (random-uuid)
session (first (t2/insert-returning-instances! Session
:id (str session-uuid)
@@ -80,7 +80,7 @@
(mu/defmethod create-session! :password :- SessionSchema
[session-type
user :- CreateSessionUserInfo
- device-info :- request.u/DeviceInfo]
+ device-info :- req.util/DeviceInfo]
;; this is actually the same as `create-session!` for `:sso` but we check whether password login is enabled.
(when-not (public-settings/enable-password-login)
(throw (ex-info (str (tru "Password login is disabled for this instance.")) {:status-code 400})))
@@ -107,7 +107,7 @@
(mu/defn ^:private ldap-login :- [:maybe [:map [:id (ms/InstanceOfClass UUID)]]]
"If LDAP is enabled and a matching user exists return a new Session for them, or `nil` if they couldn't be
authenticated."
- [username password device-info :- request.u/DeviceInfo]
+ [username password device-info :- req.util/DeviceInfo]
(when (api.ldap/ldap-enabled)
(try
(when-let [user-info (ldap/find-user username)]
@@ -131,7 +131,7 @@
"Find a matching `User` if one exists and return a new Session for them, or `nil` if they couldn't be authenticated."
[username :- ms/NonBlankString
password :- [:maybe ms/NonBlankString]
- device-info :- request.u/DeviceInfo]
+ device-info :- req.util/DeviceInfo]
(if-let [user (t2/select-one [User :id :password_salt :password :last_login :is_active], :%lower.email (u/lower-case-en username))]
(when (u.password/verify-password password (:password_salt user) (:password user))
(if (:is_active user)
@@ -157,7 +157,7 @@
throwing an Exception if login could not be completed."
[username :- ms/NonBlankString
password :- ms/NonBlankString
- device-info :- request.u/DeviceInfo]
+ device-info :- req.util/DeviceInfo]
;; Primitive "strategy implementation", should be reworked for modular providers in #3210
(or (ldap-login username password device-info) ; First try LDAP if it's enabled
(email-login username password device-info) ; Then try local authentication
@@ -185,10 +185,10 @@
[:as {{:keys [username password]} :body, :as request}]
{username ms/NonBlankString
password ms/NonBlankString}
- (let [ip-address (request.u/ip-address request)
+ (let [ip-address (req.util/ip-address request)
request-time (t/zoned-date-time (t/zone-id "GMT"))
do-login (fn []
- (let [{session-uuid :id, :as session} (login username password (request.u/device-info request))
+ (let [{session-uuid :id, :as session} (login username password (req.util/device-info request))
response {:id (str session-uuid)}]
(mw.session/set-session-cookies request response session request-time)))]
(if throttling-disabled?
@@ -240,7 +240,7 @@
[:as {{:keys [email]} :body, :as request}]
{email ms/Email}
;; Don't leak whether the account doesn't exist, just pretend everything is ok
- (let [request-source (request.u/ip-address request)]
+ (let [request-source (req.util/ip-address request)]
(throttle-check (forgot-password-throttlers :ip-address) request-source))
(throttle-check (forgot-password-throttlers :email) email)
(forgot-password-impl email)
@@ -289,7 +289,7 @@
;; Send all the active admins an email :D
(messages/send-user-joined-admin-notification-email! (t2/select-one User :id user-id)))
;; after a successful password update go ahead and offer the client a new session that they can use
- (let [{session-uuid :id, :as session} (create-session! :password user (request.u/device-info request))
+ (let [{session-uuid :id, :as session} (create-session! :password user (req.util/device-info request))
response {:success true
:session_id (str session-uuid)}]
(mw.session/set-session-cookies request response session (t/zoned-date-time (t/zone-id "GMT"))))))
@@ -317,9 +317,9 @@
(if throttling-disabled?
(google/do-google-auth request)
(http-401-on-error
- (throttle/with-throttling [(login-throttlers :ip-address) (request.u/ip-address request)]
+ (throttle/with-throttling [(login-throttlers :ip-address) (req.util/ip-address request)]
(let [user (google/do-google-auth request)
- {session-uuid :id, :as session} (create-session! :sso user (request.u/device-info request))
+ {session-uuid :id, :as session} (create-session! :sso user (req.util/device-info request))
response {:id (str session-uuid)}
user (t2/select-one [User :id :is_active], :email (:email user))]
(if (and user (:is_active user))
@@ -356,7 +356,7 @@
{pulse-id ms/PositiveInt
email :string
hash :string}
- (check-hash pulse-id email hash (request.u/ip-address request))
+ (check-hash pulse-id email hash (req.util/ip-address request))
(t2/with-transaction [_conn]
(api/let-404 [pulse-channel (t2/select-one PulseChannel :pulse_id pulse-id :channel_type "email")]
(let [emails (get-in pulse-channel [:details :emails])]
@@ -374,7 +374,7 @@
{pulse-id ms/PositiveInt
email :string
hash :string}
- (check-hash pulse-id email hash (request.u/ip-address request))
+ (check-hash pulse-id email hash (req.util/ip-address request))
(t2/with-transaction [_conn]
(api/let-404 [pulse-channel (t2/select-one PulseChannel :pulse_id pulse-id :channel_type "email")]
(let [emails (get-in pulse-channel [:details :emails])]
diff --git a/src/metabase/api/user.clj b/src/metabase/api/user.clj
index 775d8489aac..42b7c42ae28 100644
--- a/src/metabase/api/user.clj
+++ b/src/metabase/api/user.clj
@@ -25,7 +25,7 @@
[metabase.public-settings.premium-features :as premium-features]
[metabase.server.middleware.offset-paging :as mw.offset-paging]
[metabase.server.middleware.session :as mw.session]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util :as u]
[metabase.util.i18n :refer [deferred-tru tru]]
[metabase.util.malli.schema :as ms]
@@ -519,7 +519,7 @@
(user/set-password! id password)
;; after a successful password update go ahead and offer the client a new session that they can use
(when (= id api/*current-user-id*)
- (let [{session-uuid :id, :as session} (api.session/create-session! :password user (request.u/device-info request))
+ (let [{session-uuid :id, :as session} (api.session/create-session! :password user (req.util/device-info request))
response {:success true
:session_id (str session-uuid)}]
(mw.session/set-session-cookies request response session (t/zoned-date-time (t/zone-id "GMT")))))))
diff --git a/src/metabase/driver.clj b/src/metabase/driver.clj
index be0bb80b48b..a702340c9f1 100644
--- a/src/metabase/driver.clj
+++ b/src/metabase/driver.clj
@@ -425,8 +425,7 @@
dispatch-on-initialized-driver
:hierarchy #'hierarchy)
-;; TODO -- I think we should rename this to `features` since `driver/driver-features` is a bit redundant.
-(def driver-features
+(def features
"Set of all features a driver can support."
#{
;; Does this database support foreign key relationships?
@@ -550,25 +549,6 @@
;; Does the driver support column(s) support storing index info
:index-info})
-
-(defmulti supports?
- "Does this driver support a certain `feature`? (A feature is a keyword, and can be any of the ones listed above in
- [[driver-features]].)
-
- (supports? :postgres :set-timezone) ; -> true
-
- DEPRECATED — [[database-supports?]] should be used instead. This function will be removed in Metabase version 0.50.0."
- {:added "0.32.0", :arglists '([driver feature]), :deprecated "0.47.0"}
- (fn [driver feature]
- (when-not (driver-features feature)
- (throw (Exception. (tru "Invalid driver feature: {0}" feature))))
- [(dispatch-on-initialized-driver driver) feature])
- :hierarchy #'hierarchy)
-
-(defmethod supports? :default [_ _] false)
-
-(defmethod supports? [::driver :schemas] [_ _] true)
-
(defmulti database-supports?
"Does this driver and specific instance of a database support a certain `feature`?
(A feature is a keyword, and can be any of the ones listed above in `driver-features`.
@@ -587,18 +567,20 @@
(database-supports? :mongo :set-timezone mongo-db) ; -> true"
{:arglists '([driver feature database]), :added "0.41.0"}
(fn [driver feature _database]
- (when-not (driver-features feature)
+ (when-not (features feature)
(throw (Exception. (tru "Invalid driver feature: {0}" feature))))
[(dispatch-on-initialized-driver driver) feature])
:hierarchy #'hierarchy)
-(defmethod database-supports? :default [driver feature _] (supports? driver feature))
+(defmethod database-supports?
+ :default [_driver _feature _] false)
-(doseq [[feature supported?] {:basic-aggregations true
+(doseq [[feature supported?] {:convert-timezone false
+ :basic-aggregations true
:case-sensitivity-string-filter-options true
:date-arithmetics true
:temporal-extract true
- :convert-timezone false
+ :schemas true
:test/jvm-timezone-setting true}]
(defmethod database-supports? [::driver feature] [_driver _feature _db] supported?))
@@ -712,10 +694,8 @@
[_ query]
query)
-;; TODO - we should just have some sort of `core.async` channel to handle DB update notifications instead
-;;
;; TODO -- shouldn't this be called `notify-database-updated!`, since the expectation is that it is done for side
-;; effects?
+;; effects? issue: https://github.com/metabase/metabase/issues/39367
(defmulti notify-database-updated
"Notify the driver that the attributes of a `database` have changed, or that `database was deleted. This is
specifically relevant in the event that the driver was doing some caching or connection pooling; the driver should
@@ -804,6 +784,7 @@
(defmethod default-field-order ::driver [_] :database)
;; TODO -- this can vary based on session variables or connection options
+;; Issue: https://github.com/metabase/metabase/pull/39386
(defmulti db-start-of-week
"Return the day that is considered to be the start of week by `driver`. Should return a keyword such as `:sunday`."
{:added "0.37.0" :arglists '([driver])}
@@ -826,8 +807,8 @@
;;;
;;; 1. We definitely should not be asking drivers to "update the value for `:details`". Drivers shouldn't touch the
;;; application database.
-;;;
;;; 2. Something that is done for side effects like updating the application DB NEEDS TO END IN AN EXCLAMATION MARK!
+;;; Issue: https://github.com/metabase/metabase/issues/39392
(defmulti normalize-db-details
"Normalizes db-details for the given driver. This is to handle migrations that are too difficult to perform via
regular Liquibase queries. This multimethod will be called from a `:post-select` handler within the database model.
diff --git a/src/metabase/driver/util.clj b/src/metabase/driver/util.clj
index 91ce5b7fbe8..77a2e0d2795 100644
--- a/src/metabase/driver/util.clj
+++ b/src/metabase/driver/util.clj
@@ -211,7 +211,7 @@
(defn features
"Return a set of all features supported by `driver` with respect to `database`."
[driver database]
- (set (for [feature driver/driver-features
+ (set (for [feature driver/features
:when (driver/database-supports? driver feature database)]
feature)))
diff --git a/src/metabase/lib/join.cljc b/src/metabase/lib/join.cljc
index 03531e78c40..7d5f36f79cb 100644
--- a/src/metabase/lib/join.cljc
+++ b/src/metabase/lib/join.cljc
@@ -568,7 +568,7 @@
(mu/defn available-join-strategies :- [:sequential ::lib.schema.join/strategy.option]
"Get available join strategies for the current Database (based on the Database's
- supported [[metabase.driver/driver-features]]) as raw keywords like `:left-join`."
+ supported [[metabase.driver/features]]) as raw keywords like `:left-join`."
([query]
(available-join-strategies query -1))
diff --git a/src/metabase/lib/js.cljs b/src/metabase/lib/js.cljs
index 814a7c4633f..d901c1c9f36 100644
--- a/src/metabase/lib/js.cljs
+++ b/src/metabase/lib/js.cljs
@@ -739,7 +739,7 @@
(defn ^:export available-join-strategies
"Get available join strategies for the current Database (based on the Database's
- supported [[metabase.driver/driver-features]]) as opaque JoinStrategy objects."
+ supported [[metabase.driver/features]]) as opaque JoinStrategy objects."
[a-query stage-number]
(to-array (lib.core/available-join-strategies a-query stage-number)))
diff --git a/src/metabase/lib/schema/metadata.cljc b/src/metabase/lib/schema/metadata.cljc
index 1016ca49dc5..181088a00c1 100644
--- a/src/metabase/lib/schema/metadata.cljc
+++ b/src/metabase/lib/schema/metadata.cljc
@@ -280,7 +280,7 @@
{:error/message "Valid Database metadata"}
[:lib/type [:= :metadata/database]]
[:id ::lib.schema.id/database]
- ;; TODO -- this should validate against the driver features list in [[metabase.driver/driver-features]] if we're in
+ ;; TODO -- this should validate against the driver features list in [[metabase.driver/features]] if we're in
;; Clj mode
[:dbms-version {:optional true} [:maybe :map]]
[:details {:optional true} :map]
diff --git a/src/metabase/models/collection.clj b/src/metabase/models/collection.clj
index 9082b4f9d1b..5a565b696e2 100644
--- a/src/metabase/models/collection.clj
+++ b/src/metabase/models/collection.clj
@@ -928,11 +928,8 @@
(let [collection (if (integer? collection-or-id)
(t2/select-one [Collection :id :namespace] :id (collection-or-id))
collection-or-id)]
- ;; HACK Collections in the "snippets" namespace have no-op permissions unless EE enhancements are enabled
- ;;
- ;; TODO -- Pretty sure snippet perms should be feature flagged by `advanced-permissions` instead
(if (and (= (u/qualified-name (:namespace collection)) "snippets")
- (not (premium-features/enable-enhancements?)))
+ (not (premium-features/enable-snippet-collections?)))
#{}
;; This is not entirely accurate as you need to be a superuser to modifiy a collection itself (e.g., changing its
;; name) but if you have write perms you can add/remove cards
diff --git a/src/metabase/models/dashboard.clj b/src/metabase/models/dashboard.clj
index 9dc82ef5249..1aa64c58d2a 100644
--- a/src/metabase/models/dashboard.clj
+++ b/src/metabase/models/dashboard.clj
@@ -448,7 +448,9 @@
(let [new-param-field-ids (params/dashcards->param-field-ids (t2/hydrate new-dashcards :card))]
(update-field-values-for-on-demand-dbs! (params/dashcards->param-field-ids old-dashcards) new-param-field-ids))))
+
;; TODO - we need to actually make this async, but then we'd need to make `save-card!` async, and so forth
+;; Issue: https://github.com/metabase/metabase/issues/39413
(defn- result-metadata-for-query
"Fetch the results metadata for a `query` by running the query and seeing what the `qp` gives us in return."
[query]
diff --git a/src/metabase/models/humanization.clj b/src/metabase/models/humanization.clj
index 37db41a62f8..1c459ec2e3c 100644
--- a/src/metabase/models/humanization.clj
+++ b/src/metabase/models/humanization.clj
@@ -70,6 +70,7 @@
(setting/set-value-of-type! :keyword :humanization-strategy new-value)
;; now rehumanize all the Tables and Fields using the new strategy.
;; TODO: Should we do this in a background thread because it is potentially slow?
+ ;; https://github.com/metabase/metabase/issues/39406
(log/info (trs "Changing Table & Field names humanization strategy from ''{0}'' to ''{1}''"
(name old-strategy) (name new-strategy)))
(re-humanize-table-and-field-names! old-strategy))))
diff --git a/src/metabase/models/login_history.clj b/src/metabase/models/login_history.clj
index 8e43d0a61b2..4fc88985d92 100644
--- a/src/metabase/models/login_history.clj
+++ b/src/metabase/models/login_history.clj
@@ -3,7 +3,7 @@
[java-time.api :as t]
[metabase.email.messages :as messages]
[metabase.models.setting :refer [defsetting]]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util.date-2 :as u.date]
[metabase.util.i18n :as i18n :refer [trs tru]]
[metabase.util.log :as log]
@@ -28,7 +28,7 @@
or another -- keep that in mind when using this."
[history-items]
(let [ip-addresses (map :ip_address history-items)
- ip->info (request.u/geocode-ip-addresses ip-addresses)]
+ ip->info (req.util/geocode-ip-addresses ip-addresses)]
(for [history-item history-items
:let [{location-description :description, timezone :timezone} (get ip->info (:ip_address history-item))]]
(-> history-item
@@ -38,7 +38,7 @@
(if (and timestamp timezone)
(t/zoned-date-time (u.date/with-time-zone-same-instant timestamp timezone) timezone)
timestamp)))
- (update :device_description request.u/describe-user-agent)))))
+ (update :device_description req.util/describe-user-agent)))))
(defsetting send-email-on-first-login-from-new-device
;; no need to i18n -- this isn't user-facing
diff --git a/src/metabase/models/session.clj b/src/metabase/models/session.clj
index 80d0ec10aaf..230210a1e1b 100644
--- a/src/metabase/models/session.clj
+++ b/src/metabase/models/session.clj
@@ -3,7 +3,7 @@
[buddy.core.codecs :as codecs]
[buddy.core.nonce :as nonce]
[metabase.server.middleware.misc :as mw.misc]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[methodical.core :as methodical]
[schema.core :as s]
[toucan2.core :as t2]))
@@ -29,7 +29,7 @@
(t2/define-before-insert :model/Session
[session]
(cond-> session
- (some-> mw.misc/*request* request.u/embedded?) (assoc :anti_csrf_token (random-anti-csrf-token))))
+ (some-> mw.misc/*request* req.util/embedded?) (assoc :anti_csrf_token (random-anti-csrf-token))))
(t2/define-after-insert :model/Session
[{anti-csrf-token :anti_csrf_token, :as session}]
diff --git a/src/metabase/plugins/jdbc_proxy.clj b/src/metabase/plugins/jdbc_proxy.clj
index c4bc5a90d5f..194844d77d7 100644
--- a/src/metabase/plugins/jdbc_proxy.clj
+++ b/src/metabase/plugins/jdbc_proxy.clj
@@ -16,6 +16,7 @@
;;; -------------------------------------------------- Proxy Driver --------------------------------------------------
;; TODO -- why not use `java.sql.Wrapper` here instead of defining a new protocol that basically does the same thing?
+;; issue: https://github.com/metabase/metabase/issues/39357
(p.types/defprotocol+ ^:private ProxyDriver
(wrapped-driver [this]
"Get the JDBC driver wrapped by a Metabase JDBC proxy driver."))
diff --git a/src/metabase/server/middleware/auth.clj b/src/metabase/server/middleware/auth.clj
index 5e3e60df492..a56fcf16f0b 100644
--- a/src/metabase/server/middleware/auth.clj
+++ b/src/metabase/server/middleware/auth.clj
@@ -4,7 +4,7 @@
(:require
[clojure.string :as str]
[metabase.models.setting :refer [defsetting]]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.util.i18n :refer [deferred-trs]]))
(def ^:private ^:const ^String static-metabase-api-key-header "x-metabase-apikey")
@@ -15,7 +15,7 @@
(fn [{:keys [metabase-user-id] :as request} respond raise]
(if metabase-user-id
(handler request respond raise)
- (respond mw.util/response-unauthentic))))
+ (respond req.util/response-unauthentic))))
(defn- wrap-static-api-key* [{:keys [headers], :as request}]
(if-let [api-key (headers static-metabase-api-key-header)]
@@ -64,10 +64,10 @@
(respond key-not-set-response)
(not static-metabase-api-key)
- (respond mw.util/response-forbidden)
+ (respond req.util/response-forbidden)
(= (static-api-key) static-metabase-api-key)
(handler request respond raise)
:else
- (respond mw.util/response-forbidden))))
+ (respond req.util/response-forbidden))))
diff --git a/src/metabase/server/middleware/browser_cookie.clj b/src/metabase/server/middleware/browser_cookie.clj
index bbec17e21c0..4e61a263f77 100644
--- a/src/metabase/server/middleware/browser_cookie.clj
+++ b/src/metabase/server/middleware/browser_cookie.clj
@@ -5,7 +5,7 @@
they log in."
(:require
[java-time.api :as t]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util.malli :as mu]
[metabase.util.malli.schema :as ms]
[ring.util.response :as response]))
@@ -25,7 +25,7 @@
:path "/"
;; Set the cookie to expire 20 years from now. That should be sufficient
:expires (t/format :rfc-1123-date-time (t/plus (t/zoned-date-time) (t/years 20)))}
- (if (request.u/https? request)
+ (if (req.util/https? request)
{:same-site :none, :secure true}
{:same-site :lax})))
diff --git a/src/metabase/server/middleware/log.clj b/src/metabase/server/middleware/log.clj
index 14782e30383..0761cbc0206 100644
--- a/src/metabase/server/middleware/log.clj
+++ b/src/metabase/server/middleware/log.clj
@@ -10,7 +10,7 @@
[metabase.driver.sql-jdbc.execute.diagnostic
:as sql-jdbc.execute.diagnostic]
[metabase.server :as server]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util :as u]
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log]
@@ -193,7 +193,7 @@
(defn- should-log-request? [{:keys [uri], :as request}]
;; don't log calls to /health or /util/logs because they clutter up the logs (especially the window in admin) with
;; useless lines
- (and (request.u/api-call? request)
+ (and (req.util/api-call? request)
(not (#{"/api/util/logs"} uri))))
(defn log-api-call
diff --git a/src/metabase/server/middleware/misc.clj b/src/metabase/server/middleware/misc.clj
index aba0b9bac56..0bd79f890e3 100644
--- a/src/metabase/server/middleware/misc.clj
+++ b/src/metabase/server/middleware/misc.clj
@@ -5,7 +5,7 @@
[metabase.async.streaming-response]
[metabase.db :as mdb]
[metabase.public-settings :as public-settings]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util.i18n :refer [trs]]
[metabase.util.log :as log])
(:import
@@ -27,7 +27,7 @@
[handler]
(fn [request respond raise]
(handler request
- (if-not (request.u/api-call? request)
+ (if-not (req.util/api-call? request)
respond
(comp respond add-content-type*))
raise)))
diff --git a/src/metabase/server/middleware/security.clj b/src/metabase/server/middleware/security.clj
index 9bad6121dfe..fefb48c1fa4 100644
--- a/src/metabase/server/middleware/security.clj
+++ b/src/metabase/server/middleware/security.clj
@@ -7,10 +7,8 @@
[metabase.analytics.snowplow :as snowplow]
[metabase.config :as config]
[metabase.embed.settings :as embed.settings]
- [metabase.models.setting :refer [defsetting]]
[metabase.public-settings :as public-settings]
- [metabase.server.request.util :as request.u]
- [metabase.util.i18n :refer [deferred-tru]]
+ [metabase.server.request.util :as req.util]
[ring.util.codec :refer [base64-encode]])
(:import
(java.security MessageDigest SecureRandom)))
@@ -82,7 +80,6 @@
(when-not config/is-dev?
(map (partial format "'sha256-%s'") inline-js-hashes)))
:child-src ["'self'"
- ;; TODO - double check that we actually need this for Google Auth
"https://accounts.google.com"]
:style-src ["'self'"
;; See [[generate-nonce]]
@@ -129,15 +126,6 @@
"Content-Security-Policy"
#(format "%s frame-ancestors %s;" % (if allow-iframes? "*" (or (embedding-app-origin) "'none'")))))
-(defsetting ssl-certificate-public-key
- (deferred-tru
- (str "Base-64 encoded public key for this site''s SSL certificate. "
- "Specify this to enable HTTP Public Key Pinning. "
- "See {0} for more information.")
- "http://mzl.la/1EnfqBf")
- :audit :getter)
-;; TODO - it would be nice if we could make this a proper link in the UI; consider enabling markdown parsing
-
(defn- first-embedding-app-origin
"Return only the first embedding app origin."
[]
@@ -171,8 +159,8 @@
;; merge is other way around so that handler can override headers
(update response :headers #(merge %2 %1) (security-headers
:nonce (:nonce request)
- :allow-iframes? ((some-fn request.u/public? request.u/embed?) request)
- :allow-cache? (request.u/cacheable? request))))
+ :allow-iframes? ((some-fn req.util/public? req.util/embed?) request)
+ :allow-cache? (req.util/cacheable? request))))
(defn add-security-headers
"Middleware that adds HTTP security and cache-busting headers."
diff --git a/src/metabase/server/middleware/session.clj b/src/metabase/server/middleware/session.clj
index 71519f449e1..ad25d172009 100644
--- a/src/metabase/server/middleware/session.clj
+++ b/src/metabase/server/middleware/session.clj
@@ -34,7 +34,7 @@
[metabase.models.user :as user :refer [User]]
[metabase.public-settings :as public-settings]
[metabase.public-settings.premium-features :as premium-features]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.util :as u]
[metabase.util.i18n :as i18n :refer [deferred-trs deferred-tru trs tru]]
[metabase.util.log :as log]
@@ -121,18 +121,18 @@
(merge
{:same-site (session-cookie-samesite)
;; TODO - we should set `site-path` as well. Don't want to enable this yet so we don't end
- ;; up breaking things
+ ;; up breaking things issue: https://github.com/metabase/metabase/issues/39346
:path "/" #_(site-path)}
;; If the authentication request request was made over HTTPS (hopefully always except for
;; local dev instances) add `Secure` attribute so the cookie is only sent over HTTPS.
- (when (request.u/https? request)
+ (when (req.util/https? request)
{:secure true})))
(defmethod default-session-cookie-attributes :full-app-embed
[_ request]
(merge
{:path "/"}
- (when (request.u/https? request)
+ (when (req.util/https? request)
;; SameSite=None is required for cross-domain full-app embedding. This is safe because
;; security is provided via anti-CSRF token. Note that most browsers will only accept
;; SameSite=None with secure cookies, thus we are setting it only over HTTPS to prevent
@@ -194,7 +194,7 @@
;; max-session age-is in minutes; Max-Age= directive should be in seconds
(when (use-permanent-cookies? request)
{:max-age (* 60 (config/config-int :max-session-age))}))]
- (when (and (= (session-cookie-samesite) :none) (not (request.u/https? request)))
+ (when (and (= (session-cookie-samesite) :none) (not (req.util/https? request)))
(log/warn
(str (deferred-trs "Session cookie's SameSite is configured to \"None\", but site is served over an insecure connection. Some browsers will reject cookies under these conditions.")
" "
diff --git a/src/metabase/server/middleware/ssl.clj b/src/metabase/server/middleware/ssl.clj
index 8918a32e71d..5dc942f2888 100644
--- a/src/metabase/server/middleware/ssl.clj
+++ b/src/metabase/server/middleware/ssl.clj
@@ -3,7 +3,7 @@
(:require
[clojure.string :as str]
[metabase.public-settings :as public-settings]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[ring.util.request :as req]
[ring.util.response :as response]))
@@ -45,7 +45,7 @@
(and
(public-settings/redirect-all-requests-to-https)
- (not (request.u/https? request)))
+ (not (req.util/https? request)))
(respond (ssl-redirect-response request))
:else (handler request respond raise))))
diff --git a/src/metabase/server/middleware/util.clj b/src/metabase/server/middleware/util.clj
deleted file mode 100644
index 803864683a6..00000000000
--- a/src/metabase/server/middleware/util.clj
+++ /dev/null
@@ -1,5 +0,0 @@
-(ns metabase.server.middleware.util
- "Ring middleware utility functions. TODO -- consider renaming this to `metabase.server.request.util`.")
-
-(def response-unauthentic "Generic `401 (Unauthenticated)` Ring response map." {:status 401, :body "Unauthenticated"})
-(def response-forbidden "Generic `403 (Forbidden)` Ring response map." {:status 403, :body "Forbidden"})
diff --git a/src/metabase/server/request/util.clj b/src/metabase/server/request/util.clj
index f091da5eaa0..4b718bde906 100644
--- a/src/metabase/server/request/util.clj
+++ b/src/metabase/server/request/util.clj
@@ -149,6 +149,7 @@
[:timezone [:maybe (ms/InstanceOfClass ZoneId)]]]])
;; TODO -- replace with something better, like built-in database once we find one that's GPL compatible
+;; issue: https://github.com/metabase/metabase/issues/39352
(mu/defn geocode-ip-addresses :- [:maybe IPAddress->Info]
"Geocode multiple IP addresses, returning a map of IP address -> info, with each info map containing human-friendly
`:description` of the location and a `java.time.ZoneId` `:timezone`, if that information is available."
@@ -169,3 +170,11 @@
(catch Throwable e
(log/error e (trs "Error geocoding IP addresses") {:url url})
nil))))))
+
+(def response-unauthentic
+ "Generic `401 (Unauthenticated)` Ring response map."
+ {:status 401, :body "Unauthenticated"})
+
+(def response-forbidden
+ "Generic `403 (Forbidden)` Ring response map."
+ {:status 403, :body "Forbidden"})
diff --git a/test/metabase/api/alert_test.clj b/test/metabase/api/alert_test.clj
index 4451acfda79..1a7e3443ce6 100644
--- a/test/metabase/api/alert_test.clj
+++ b/test/metabase/api/alert_test.clj
@@ -12,7 +12,7 @@
[metabase.models.permissions-group :as perms-group]
[metabase.models.pulse :as pulse]
[metabase.models.pulse-test :as pulse-test]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.test.mock.util :refer [pulse-channel-defaults]]
[metabase.util :as u]
@@ -134,10 +134,10 @@
;; authentication test on every single individual endpoint
(deftest auth-tests
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "alert")))
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :put 401 "alert/13"))))
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/api/card_test.clj b/test/metabase/api/card_test.clj
index 8e36c213698..3f86d38c369 100644
--- a/test/metabase/api/card_test.clj
+++ b/test/metabase/api/card_test.clj
@@ -40,7 +40,7 @@
[metabase.query-processor.card :as qp.card]
[metabase.query-processor.compile :as qp.compile]
[metabase.query-processor.middleware.constraints :as qp.constraints]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.task :as task]
[metabase.task.persist-refresh :as task.persist-refresh]
[metabase.task.sync-databases :as task.sync-databases]
@@ -212,8 +212,8 @@
(card-returned? :database db card-2))))))
(deftest ^:parallel authentication-test
- (is (= (get mw.util/response-unauthentic :body) (client/client :get 401 "card")))
- (is (= (get mw.util/response-unauthentic :body) (client/client :put 401 "card/13"))))
+ (is (= (get req.util/response-unauthentic :body) (client/client :get 401 "card")))
+ (is (= (get req.util/response-unauthentic :body) (client/client :put 401 "card/13"))))
(deftest ^:parallel model-id-requied-when-f-is-database-test
(is (= {:errors {:model_id "model_id is a required parameter when filter mode is 'database'"}}
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 89614c2b9d3..ffe115bb780 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -46,7 +46,7 @@
[metabase.query-processor :as qp]
[metabase.query-processor.middleware.permissions :as qp.perms]
[metabase.query-processor.streaming.test-util :as streaming.test-util]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
@@ -173,7 +173,7 @@
;; authentication test on every single individual endpoint
(deftest auth-test
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "dashboard")
(client/client :put 401 "dashboard/13"))))
diff --git a/test/metabase/api/metric_test.clj b/test/metabase/api/metric_test.clj
index 3357a9773c8..6d507db5dd7 100644
--- a/test/metabase/api/metric_test.clj
+++ b/test/metabase/api/metric_test.clj
@@ -7,7 +7,7 @@
[metabase.models.metric :as metric :refer [Metric]]
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.util :as u]
[toucan2.core :as t2]
@@ -44,10 +44,10 @@
(testing "AUTHENTICATION"
;; We assume that all endpoints for a given context are enforced by the same middleware, so we don't run the same
;; authentication test on every single individual endpoint
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "metric")))
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :put 401 "metric/13")))))
(deftest create-test
diff --git a/test/metabase/api/notify_test.clj b/test/metabase/api/notify_test.clj
index 478ef4f2cd3..ec8ba7c6f04 100644
--- a/test/metabase/api/notify_test.clj
+++ b/test/metabase/api/notify_test.clj
@@ -8,7 +8,7 @@
[metabase.http-client :as client]
[metabase.models.database :as database :refer [Database]]
[metabase.server.middleware.auth :as mw.auth]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.sync :as sync]
[metabase.sync.sync-metadata]
[metabase.test :as mt]
@@ -26,7 +26,7 @@
(client/client :post 403 "notify/db/100")))))
(testing "endpoint requires authentication"
(mt/with-temporary-setting-values [api-key "test-api-key"] ;; set in :test but not in :dev
- (is (= (get mw.util/response-forbidden :body)
+ (is (= (get req.util/response-forbidden :body)
(client/client :post 403 "notify/db/100")))))))
(def ^:private api-headers {:headers {"x-metabase-apikey" "test-api-key"
diff --git a/test/metabase/api/pulse_test.clj b/test/metabase/api/pulse_test.clj
index 7bf73342abf..ecf49c0af9f 100644
--- a/test/metabase/api/pulse_test.clj
+++ b/test/metabase/api/pulse_test.clj
@@ -21,7 +21,7 @@
[metabase.models.pulse-channel :as pulse-channel]
[metabase.models.pulse-test :as pulse-test]
[metabase.pulse.render.style :as style]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.test.mock.util :refer [pulse-channel-defaults]]
[metabase.util :as u]
@@ -99,8 +99,8 @@
;; authentication test on every single individual endpoint
(deftest authentication-test
- (is (= (:body mw.util/response-unauthentic) (client/client :get 401 "pulse")))
- (is (= (:body mw.util/response-unauthentic) (client/client :put 401 "pulse/13"))))
+ (is (= (:body req.util/response-unauthentic) (client/client :get 401 "pulse")))
+ (is (= (:body req.util/response-unauthentic) (client/client :put 401 "pulse/13"))))
;;; +----------------------------------------------------------------------------------------------------------------+
diff --git a/test/metabase/api/segment_test.clj b/test/metabase/api/segment_test.clj
index 746d968d426..847d1cbe6ba 100644
--- a/test/metabase/api/segment_test.clj
+++ b/test/metabase/api/segment_test.clj
@@ -7,7 +7,7 @@
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.models.segment :as segment :refer [Segment]]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.util :as u]
[toucan2.core :as t2]
@@ -33,10 +33,10 @@
;; authentication test on every single individual endpoint
(deftest authentication-test
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "segment")))
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :put 401 "segment/13"))))
;; ## POST /api/segment
diff --git a/test/metabase/api/table_test.clj b/test/metabase/api/table_test.clj
index 1daa635f4fe..119e83ecbe3 100644
--- a/test/metabase/api/table_test.clj
+++ b/test/metabase/api/table_test.clj
@@ -12,7 +12,7 @@
[metabase.models.permissions :as perms]
[metabase.models.permissions-group :as perms-group]
[metabase.models.table :as table]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.timeseries-query-processor-test.util :as tqpt]
[metabase.upload-test :as upload-test]
@@ -27,9 +27,9 @@
;; authentication test on every single individual endpoint
(deftest unauthenticated-test
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "table")))
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 (format "table/%d" (mt/id :users))))))
(defn- db-details []
diff --git a/test/metabase/api/timeline_event_test.clj b/test/metabase/api/timeline_event_test.clj
index d7c72d6f784..dbc3e8480ad 100644
--- a/test/metabase/api/timeline_event_test.clj
+++ b/test/metabase/api/timeline_event_test.clj
@@ -6,7 +6,7 @@
[metabase.models.collection :refer [Collection]]
[metabase.models.timeline :refer [Timeline]]
[metabase.models.timeline-event :refer [TimelineEvent]]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.util :as u]
[toucan2.core :as t2]))
@@ -15,9 +15,9 @@
(deftest auth-tests
(testing "Authentication"
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "/timeline-event")))
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "/timeline-event/1")))))
(deftest get-timeline-event-test
diff --git a/test/metabase/api/timeline_test.clj b/test/metabase/api/timeline_test.clj
index 1ca06466d4f..87c43757bba 100644
--- a/test/metabase/api/timeline_test.clj
+++ b/test/metabase/api/timeline_test.clj
@@ -9,7 +9,7 @@
[metabase.models.permissions-group :as perms-group]
[metabase.models.timeline :refer [Timeline]]
[metabase.models.timeline-event :refer [TimelineEvent]]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.util :as u]
[toucan2.core :as t2]
@@ -17,9 +17,9 @@
(deftest auth-tests
(testing "Authentication"
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "/timeline")))
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "/timeline/1")))))
(deftest list-timelines-test
diff --git a/test/metabase/api/user_test.clj b/test/metabase/api/user_test.clj
index 7ff49347b20..0cb481397f1 100644
--- a/test/metabase/api/user_test.clj
+++ b/test/metabase/api/user_test.clj
@@ -14,7 +14,7 @@
[metabase.models.user :as user]
[metabase.models.user-test :as user-test]
[metabase.public-settings.premium-features :as premium-features]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.test.fixtures :as fixtures]
[metabase.util :as u]
@@ -54,10 +54,10 @@
(deftest user-list-authentication-test
(testing "authentication"
(testing "GET /api/user"
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "user"))))
(testing "GET /api/user/current"
- (is (= (get mw.util/response-unauthentic :body)
+ (is (= (get req.util/response-unauthentic :body)
(client/client :get 401 "user/current"))))))
(deftest user-list-test
diff --git a/test/metabase/models/login_history_test.clj b/test/metabase/models/login_history_test.clj
index 7468833ce85..16acbc1d973 100644
--- a/test/metabase/models/login_history_test.clj
+++ b/test/metabase/models/login_history_test.clj
@@ -6,7 +6,7 @@
[metabase.models :refer [LoginHistory User]]
[metabase.models.login-history :as login-history]
[metabase.public-settings :as public-settings]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.util :as u]
[metabase.util.date-2 :as u.date]
@@ -48,7 +48,7 @@
(mt/with-fake-inbox
;; mock out the IP address geocoding function so we can make sure it handles timezones like PST correctly
;; (#15603)
- (with-redefs [request.u/geocode-ip-addresses (fn [ip-addresses]
+ (with-redefs [req.util/geocode-ip-addresses (fn [ip-addresses]
(into {} (for [ip-address ip-addresses]
[ip-address
{:description "San Francisco, California, United States"
diff --git a/test/metabase/query_processor/test_util.clj b/test/metabase/query_processor/test_util.clj
index cd169b2527a..5d8c9818072 100644
--- a/test/metabase/query_processor/test_util.clj
+++ b/test/metabase/query_processor/test_util.clj
@@ -63,7 +63,7 @@
(every? #(driver/database-supports? driver % db) features)))]
driver))))
-(alter-meta! #'normal-drivers-with-feature assoc :arglists (list (into ['&] (sort driver/driver-features))))
+(alter-meta! #'normal-drivers-with-feature assoc :arglists (list (into ['&] (sort driver/features))))
(defn normal-drivers-without-feature
"Return a set of all non-timeseries engines (e.g., everything except Druid and Google Analytics) that DO NOT support
@@ -71,7 +71,7 @@
[feature]
(set/difference (normal-drivers) (normal-drivers-with-feature feature)))
-(alter-meta! #'normal-drivers-without-feature assoc :arglists (list (into ['&] (sort driver/driver-features))))
+(alter-meta! #'normal-drivers-without-feature assoc :arglists (list (into ['&] (sort driver/features))))
(defn normal-drivers-except
"Return the set of all drivers except Druid, Google Analytics, and those in `excluded-drivers`."
diff --git a/test/metabase/server/middleware/auth_test.clj b/test/metabase/server/middleware/auth_test.clj
index a5caaae147e..42709fcd786 100644
--- a/test/metabase/server/middleware/auth_test.clj
+++ b/test/metabase/server/middleware/auth_test.clj
@@ -5,7 +5,7 @@
[metabase.models.session :refer [Session]]
[metabase.server.middleware.auth :as mw.auth]
[metabase.server.middleware.session :as mw.session]
- [metabase.server.middleware.util :as mw.util]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[metabase.test.data.users :as test.users]
[metabase.test.fixtures :as fixtures]
@@ -49,7 +49,7 @@
(testing "Invalid requests should return unauthed response"
(testing "when no session ID is sent with request"
- (is (= mw.util/response-unauthentic
+ (is (= req.util/response-unauthentic
(auth-enforced-handler
(ring.mock/request :get "/anyurl")))))
@@ -62,7 +62,7 @@
:user_id (test.users/user->id :rasta)})
(t2/update! (t2/table-name Session) {:id session-id}
{:created_at (t/instant 1000)})
- (is (= mw.util/response-unauthentic
+ (is (= req.util/response-unauthentic
(auth-enforced-handler (request-with-session-id session-id))))
(finally (t2/delete! Session :id session-id)))))
@@ -74,7 +74,7 @@
(try
(t2/insert! Session {:id session-id
:user_id (test.users/user->id :trashbird)})
- (is (= mw.util/response-unauthentic
+ (is (= req.util/response-unauthentic
(auth-enforced-handler
(request-with-session-id session-id))))
(finally (t2/delete! Session :id session-id)))))))
@@ -123,7 +123,7 @@
(deftest enforce-static-api-key-request
(mt/with-temporary-setting-values [api-key "test-api-key"]
(testing "no apikey in the request, expect 403"
- (is (= mw.util/response-forbidden
+ (is (= req.util/response-forbidden
(api-key-enforced-handler
(ring.mock/request :get "/anyurl")))))
@@ -133,7 +133,7 @@
(request-with-api-key "test-api-key")))))
(testing "invalid apikey, expect 403"
- (is (= mw.util/response-forbidden
+ (is (= req.util/response-forbidden
(api-key-enforced-handler
(request-with-api-key "foobar"))))))
diff --git a/test/metabase/server/request/util_test.clj b/test/metabase/server/request/util_test.clj
index 1bab8a9e9a4..d17a6aff02b 100644
--- a/test/metabase/server/request/util_test.clj
+++ b/test/metabase/server/request/util_test.clj
@@ -3,7 +3,7 @@
[clojure.test :refer :all]
[clojure.tools.reader.edn :as edn]
[java-time.api :as t]
- [metabase.server.request.util :as request.u]
+ [metabase.server.request.util :as req.util]
[metabase.test :as mt]
[ring.mock.request :as ring.mock]))
@@ -22,7 +22,7 @@
{"origin" "http://mysite.com"} false}]
(testing (pr-str (list 'https? {:headers headers}))
(is (= expected
- (request.u/https? {:headers headers}))))))
+ (req.util/https? {:headers headers}))))))
(def ^:private mock-request
(delay (edn/read-string (slurp "test/metabase/server/request/sample-request.edn"))))
@@ -31,10 +31,10 @@
(is (= {:device_id "129d39d1-6758-4d2c-a751-35b860007002"
:device_description "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/89.0.4389.72 Safari/537.36"
:ip_address "0:0:0:0:0:0:0:1"}
- (request.u/device-info @mock-request))))
+ (req.util/device-info @mock-request))))
(deftest ^:parallel describe-user-agent-test
- (are [user-agent expected] (= expected (request.u/describe-user-agent user-agent))
+ (are [user-agent expected] (= expected (req.util/describe-user-agent user-agent))
"Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML like Gecko) Chrome/89.0.4389.86 Safari/537.36"
"Browser (Chrome/Windows)"
@@ -57,27 +57,27 @@
(let [request (ring.mock/request :get "api/session")]
(testing "request with no forwarding"
(is (= "127.0.0.1"
- (request.u/ip-address request))))
+ (req.util/ip-address request))))
(testing "request with forwarding"
(let [mock-request (-> (ring.mock/request :get "api/session")
(ring.mock/header "X-Forwarded-For" "5.6.7.8"))]
(is (= "5.6.7.8"
- (request.u/ip-address mock-request))))
+ (req.util/ip-address mock-request))))
(testing "multiple IP addresses"
(let [mock-request (-> (ring.mock/request :get "api/session")
(ring.mock/header "X-Forwarded-For" "1.2.3.4, 5.6.7.8"))]
(is (= "1.2.3.4"
- (request.u/ip-address mock-request)))))
+ (req.util/ip-address mock-request)))))
(testing "different header than default X-Forwarded-For"
(mt/with-temporary-setting-values [source-address-header "X-ProxyUser-Ip"]
(let [mock-request (-> (ring.mock/request :get "api/session")
(ring.mock/header "x-proxyuser-ip" "1.2.3.4"))]
(is (= "1.2.3.4"
- (request.u/ip-address mock-request)))))))))
+ (req.util/ip-address mock-request)))))))))
(deftest ^:parallel geocode-ip-addresses-test
(are [ip-addresses expected] (malli= [:maybe expected]
- (request.u/geocode-ip-addresses ip-addresses))
+ (req.util/geocode-ip-addresses ip-addresses))
;; Google DNS
["8.8.8.8"]
[:map
--
GitLab