Skip to content
Snippets Groups Projects
Unverified Commit 290165d9 authored by Noah Moss's avatar Noah Moss Committed by GitHub
Browse files

Make sure block perm enforcement allows native queries if powering a sandbox (#49974)

parent 113d75ce
No related branches found
No related tags found
No related merge requests found
......@@ -22,15 +22,17 @@
run if the current User has unrestricted data permissions from another Group. See the namespace documentation for
[[metabase.models.collection]] for more details."
:feature :advanced-permissions
[{database-id :database :as query}]
(let [{:keys [table-ids card-ids]} (query-perms/query->source-ids query)
[{{gtap-perms :gtaps} ::query-perms/perms, database-id :database :as query}]
(let [{:keys [table-ids card-ids native?]} (query-perms/query->source-ids query)
table-permissions (map (partial data-perms/table-permission-for-user api/*current-user-id*
:perms/view-data database-id)
table-ids)]
;; Make sure we don't have block permissions for any individual tables in the query
;; Make sure we don't have block permissions for the entire DB or individual tables referenced by the query.
(or
(not= :blocked (data-perms/full-db-permission-for-user api/*current-user-id* :perms/view-data database-id))
(= #{:unrestricted} (set table-permissions))
;; Don't block a query if we have native access implicitly granted to power a sandbox
(and native? (= :query-builder-and-native (:perms/create-queries gtap-perms)))
(throw-block-permissions-exception))
;; Recursively check block permissions for any Cards referenced by the query
......
......@@ -1215,3 +1215,10 @@
:people {:remappings {"user_id" [:dimension $people.zip]}}}})
(data-perms/set-table-permission! &group (mt/id :people) :perms/view-data :unrestricted)
(is (= 0 (count (mt/rows (qp/process-query (mt/mbql-query orders)))))))))
(deftest native-sandbox-table-level-block-perms-test
(testing "A sandbox powered by a native query source card can be used even when other tables have block perms (#49969)"
(met/with-gtaps! {:gtaps {:venues (venues-category-native-gtap-def)}
:attributes {"cat" 50}}
(data-perms/set-table-permission! &group (mt/id :people) :perms/view-data :blocked)
(is (= 10 (count (mt/rows (qp/process-query (mt/mbql-query venues)))))))))
......@@ -108,11 +108,12 @@
(let [card-id (or *card-id* (:qp/source-card-id outer-query))
required-perms (query-perms/required-perms-for-query outer-query :already-preprocessed? true)
source-card-ids (set/difference (:card-ids required-perms) (:card-ids gtap-perms))]
;; On EE, check block permissions up front for all queries. If block perms are in place, reject all native queries
;; (unless overriden by `gtap-perms`) and any queries that touch blocked tables/DBs
(check-block-permissions outer-query)
(cond
card-id
(do
(query-perms/check-card-read-perms database-id card-id)
(check-block-permissions outer-query))
(query-perms/check-card-read-perms database-id card-id)
;; set when querying for field values of dashboard filters, which only require
;; collection perms for the dashboard and not ad-hoc query perms
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment