Skip to content
Snippets Groups Projects
Unverified Commit 6b64bfe5 authored by github-automation-metabase's avatar github-automation-metabase Committed by GitHub
Browse files

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

parent ce33d649
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