From 45080ad694f1d7b5cbdf5ec46951bb1bef0281ec Mon Sep 17 00:00:00 2001
From: Cam Saul <1455846+camsaul@users.noreply.github.com>
Date: Fri, 26 Jan 2024 14:33:52 -0800
Subject: [PATCH] Chain filter search should check block perms (#38213)
* Chain filter search should check block perms
* Simplify test
---
src/metabase/models/params/chain_filter.clj | 22 +++++++++++----------
test/metabase/api/dashboard_test.clj | 6 +++++-
test/metabase/test/data/impl.clj | 6 +++++-
3 files changed, 22 insertions(+), 12 deletions(-)
diff --git a/src/metabase/models/params/chain_filter.clj b/src/metabase/models/params/chain_filter.clj
index 13c09d0c3ef..fba6da56590 100644
--- a/src/metabase/models/params/chain_filter.clj
+++ b/src/metabase/models/params/chain_filter.clj
@@ -679,20 +679,22 @@
v->human-readable (delay (human-readable-remapping-map field-id))
the-remapped-field-id (delay (remapped-field-id field-id))]
(cond
- (str/blank? query)
- (apply chain-filter field-id constraints options)
+ (str/blank? query)
+ (apply chain-filter field-id constraints options)
- (some? @v->human-readable)
- (human-readable-values-remapped-chain-filter-search field-id @v->human-readable constraints query options)
+ (some? @v->human-readable)
+ (human-readable-values-remapped-chain-filter-search field-id @v->human-readable constraints query options)
- (and (search-cached-field-values? field-id constraints) (nil? @the-remapped-field-id))
- (cached-field-values-search field-id query constraints options)
+ (and (search-cached-field-values? field-id constraints) (nil? @the-remapped-field-id))
+ (do
+ (check-field-value-query-permissions field-id constraints options)
+ (cached-field-values-search field-id query constraints options))
- (some? @the-remapped-field-id)
- (unremapped-chain-filter-search @the-remapped-field-id constraints query (assoc options :original-field-id field-id))
+ (some? @the-remapped-field-id)
+ (unremapped-chain-filter-search @the-remapped-field-id constraints query (assoc options :original-field-id field-id))
- :else
- (unremapped-chain-filter-search field-id constraints query options))))
+ :else
+ (unremapped-chain-filter-search field-id constraints query options))))
;;; ------------------ Filterable Field IDs (powers GET /api/dashboard/params/valid-filter-fields) -------------------
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index 61eef165e38..46d55040b5d 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -2842,7 +2842,11 @@
{:schemas :block})
(is (= "You don't have permissions to do that."
(->> (chain-filter-values-url (:id dashboard) (:category-name param-keys))
- (mt/user-http-request :rasta :get 403))))))))))
+ (mt/user-http-request :rasta :get 403))))
+ (testing "search"
+ (is (= "You don't have permissions to do that."
+ (->> (chain-filter-search-url (:id dashboard) (:category-name param-keys) "BBQ")
+ (mt/user-http-request :rasta :get 403)))))))))))
(deftest dashboard-with-static-list-parameters-test
(testing "A dashboard that has parameters that has static values"
diff --git a/test/metabase/test/data/impl.clj b/test/metabase/test/data/impl.clj
index 8e819626845..ba02e926d7e 100644
--- a/test/metabase/test/data/impl.clj
+++ b/test/metabase/test/data/impl.clj
@@ -255,7 +255,11 @@
:let [field-name (get old-field-id->name old-field-id)]]
(-> field-values
(dissoc :id)
- (assoc :field_id (get new-field-name->id field-name)))))))
+ (assoc :field_id (get new-field-name->id field-name))
+ ;; Toucan after-select for FieldValues returns NULL human_readable_values as [] for FE-friendliness..
+ ;; preserve NULL in the app DB copy so we don't end up changing things that rely on checking whether its
+ ;; NULL like [[metabase.models.params.chain-filter/search-cached-field-values?]]
+ (update :human_readable_values not-empty))))))
(defn- copy-db-tables! [old-db-id new-db-id]
(let [old-tables (t2/select Table :db_id old-db-id {:order-by [[:id :asc]]})
--
GitLab