Skip to content
Snippets Groups Projects
Commit 2987eb8a authored by Cam Saul's avatar Cam Saul
Browse files

Fix occasional test failure

parent 661a3aed
Branches
Tags
No related merge requests found
......@@ -5,9 +5,10 @@
[metabase.mbql.util :as mbql.u]
[metabase.util.i18n :refer [trs tru]]))
(defn- maybe-apply-column-level-perms-check* [{{{source-query-fields :fields} :source-query} :query, :as query}]
(let [restricted-field-ids (and (or (:gtap-perms query)
(get-in query [:query :gtap-perms]))
(defn- maybe-apply-column-level-perms-check*
{:arglists '([query context])}
[{{{source-query-fields :fields} :source-query} :query, :as query} {:keys [gtap-perms]}]
(let [restricted-field-ids (and gtap-perms
(set (mbql.u/match source-query-fields [:field-id id] id)))]
(when (seq restricted-field-ids)
(let [fields-ids-in-query (set (mbql.u/match (m/dissoc-in query [:query :source-query]) [:field-id id] id))]
......@@ -20,5 +21,5 @@
"Check column-level permissions if applicable."
[qp]
(fn [query rff context]
(maybe-apply-column-level-perms-check* query)
(maybe-apply-column-level-perms-check* query context)
(qp query rff context)))
......@@ -271,11 +271,10 @@
(defn- gtapped-query
"Apply GTAPs to `query` and return the updated version of `query`."
[query table-id->gtap]
(-> query
(apply-gtaps table-id->gtap)
(update :gtap-perms (fn [perms]
(into (set perms) (gtaps->perms-set (vals table-id->gtap)))))))
[query table-id->gtap context]
{:query (apply-gtaps query table-id->gtap)
:context (update context :gtap-perms (fn [perms]
(into (set perms) (gtaps->perms-set (vals table-id->gtap)))))})
(defn apply-row-level-permissions
"Does the work of swapping the given table the user was querying against with a nested subquery that restricts the
......@@ -284,9 +283,10 @@
(fn [query rff context]
(if-let [table-id->gtap (when *current-user-id*
(query->table-id->gtap query))]
(qp
(gtapped-query query table-id->gtap)
(fn [metadata]
(rff (merge-metadata query metadata)))
context)
(let [{query' :query, context' :context} (gtapped-query query table-id->gtap context)]
(qp
query'
(fn [metadata]
(rff (merge-metadata query metadata)))
context'))
(qp query rff context))))
......@@ -184,9 +184,7 @@
:alias "v"
:strategy :left-join
:condition [:= $venue_id &v.venues.id]}]
:aggregation [[:count]]}
:gtap-perms #{(perms/table-query-path (Table (mt/id :venues)))
(perms/table-query-path (Table (mt/id :checkins)))}})
:aggregation [[:count]]}})
(apply-row-level-permissions
(mt/mbql-query checkins
{:aggregation [[:count]]
......@@ -205,8 +203,7 @@
:source-query {:native (str "SELECT * FROM \"PUBLIC\".\"VENUES\" "
"WHERE \"PUBLIC\".\"VENUES\".\"CATEGORY_ID\" = 50 "
"ORDER BY \"PUBLIC\".\"VENUES\".\"ID\"")
:params []}}
:gtap-perms #{(perms/adhoc-native-query-path (mt/id))}})
:params []}}})
(apply-row-level-permissions
(mt/mbql-query venues
{:aggregation [[:count]]}))))))))
......
......@@ -37,7 +37,8 @@
(declare check-query-permissions*)
(s/defn ^:private check-ad-hoc-query-perms
[{:keys [gtap-perms], :as outer-query}]
{:arglists '([outer-query context])}
[outer-query {:keys [gtap-perms]}]
;; *If* we're using a GTAP, the User is obviously allowed to run its source query. So subtract the set of
;; perms required to run the source query. (See further discussion in
;; metabase-enterprise.sandbox.query-processor.middleware.row-level-restrictions)
......@@ -52,12 +53,12 @@
(s/defn ^:private check-query-permissions*
"Check that User with `user-id` has permissions to run `query`, or throw an exception."
[{{:keys [card-id]} :info, :as outer-query} :- su/Map]
[{{:keys [card-id]} :info, :as outer-query} :- su/Map context]
(when *current-user-id*
(log/tracef "Checking query permissions. Current user perms set = %s" (pr-str @*current-user-permissions-set*))
(if card-id
(check-card-read-perms card-id)
(check-ad-hoc-query-perms outer-query))))
(check-ad-hoc-query-perms outer-query context))))
(defn check-query-permissions
"Middleware that check that the current user has permissions to run the current query. This only applies if
......@@ -66,7 +67,7 @@
'publishing' a Card)."
[qp]
(fn [query rff context]
(check-query-permissions* query)
(check-query-permissions* query context)
(qp query rff context)))
......
0% Loading or .
You are about to add 0 people to the discussion. Proceed with caution.
Please register or to comment