Skip to content
Snippets Groups Projects
Unverified Commit c7a5aee1 authored by John Swanson's avatar John Swanson Committed by GitHub
Browse files

Preprocess queries before data perms check (#45601)

When we run queries, we preprocess them, during which the sandboxing
middleware adds information that we use when checking permissions
later on in query processing.

When we checked data permissions before saving a card, we were checking
permissions on the raw query, without preprocessing. Without the data
provided by the sandboxing middleware, the permission check sometimes
failed (depending on the permissions for other tables in the DB).

Note the big hack here: preprocessing can fail, for lots of reasons.
This broke a *lot* of tests that were passing in invalid queries. If an
exception occurs during preprocessing, we catch and just use the
original query like we were before.
parent 45728b08
Branches
Tags v0.9-final
No related merge requests found
......@@ -133,7 +133,9 @@
(try
(let [query (mbql.normalize/normalize query)]
;; if we are using a Card as our source, our perms are that Card's (i.e. that Card's Collection's) read perms
(if-let [source-card-id (qp.util/query->source-card-id query)]
(if-let [source-card-id (if already-preprocessed?
(:qp/source-card-id query)
(qp.util/query->source-card-id query))]
{:paths (source-card-read-perms source-card-id)}
;; otherwise if there's no source card then calculate perms based on the Tables referenced in the query
(let [query (cond-> query
......@@ -263,7 +265,14 @@
(mu/defn can-run-query?
"Return `true` if the current user has sufficient permissions to run `query`, and `false` otherwise."
[query]
(let [required-perms (required-perms-for-query query)]
(let [preprocessed-query (try ((requiring-resolve 'metabase.query-processor.preprocess/preprocess) query)
(catch Exception e
(log/info e "Failed to preprocess query, falling back to unprocessed")
false))
query (or preprocessed-query query)
required-perms (required-perms-for-query query
:already-preprocessed?
(boolean preprocessed-query))]
(check-data-perms query required-perms :throw-exceptions? false)))
(defn can-query-table?
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment