diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj index ca1b163f876a1179c7143dcf0a37eb933a252ebe..c348cfe763bdefe96fb15f8953f3f191f40d09ee 100644 --- a/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj +++ b/enterprise/backend/src/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions.clj @@ -22,7 +22,7 @@ [metabase.query-processor.middleware.permissions :as qp.perms] [metabase.query-processor.store :as qp.store] [metabase.util :as u] - [metabase.util.i18n :refer [tru]] + [metabase.util.i18n :refer [trs tru]] [metabase.util.schema :as su] [schema.core :as s] [toucan.db :as db])) @@ -324,13 +324,26 @@ (assoc ::original-metadata (expected-cols original-query)) (update-in [::qp.perms/perms :gtaps] (fn [perms] (into (set perms) (gtaps->perms-set (vals table-id->gtap))))))))) +(def ^:private default-recursion-limit 20) +(def ^:private ^:dynamic *recursion-limit* default-recursion-limit) + (defn apply-sandboxing "Pre-processing middleware. Replaces source tables a User was querying against with source queries that (presumably) restrict the rows returned, based on presence of segmented permission GTAPs." [query] (or (when-let [table-id->gtap (when *current-user-id* (query->table-id->gtap query))] - (gtapped-query query table-id->gtap)) + (let [gtapped-query (gtapped-query query table-id->gtap)] + (if (not= query gtapped-query) + ;; Applying GTAPs to the query may have introduced references to tables that are also sandboxed, + ;; so we need to recursively appby the middleware until new queries are not returned. + (if (= *recursion-limit* 0) + (throw (ex-info (trs "Reached recursion limit of {0} in \"apply-sandboxing\" middleware" + default-recursion-limit) + query)) + (binding [*recursion-limit* (dec *recursion-limit*)] + (apply-sandboxing gtapped-query))) + gtapped-query))) query)) diff --git a/frontend/test/metabase/scenarios/permissions/sandboxes.cy.spec.js b/frontend/test/metabase/scenarios/permissions/sandboxes.cy.spec.js index 42fc83c6fc2820ef6860ffe7a80f24bbf4de5fff..b88e4b356a27837f021b55afaa9e2b24b0b09bf3 100644 --- a/frontend/test/metabase/scenarios/permissions/sandboxes.cy.spec.js +++ b/frontend/test/metabase/scenarios/permissions/sandboxes.cy.spec.js @@ -941,7 +941,7 @@ describeEE("formatting > sandboxes", () => { cy.contains("37.65"); }); - it.skip("unsaved/dirty query should work on linked table column with multiple dimensions and remapping (metabase#15106)", () => { + it("unsaved/dirty query should work on linked table column with multiple dimensions and remapping (metabase#15106)", () => { cy.server(); cy.route("POST", "/api/dataset").as("dataset"); diff --git a/src/metabase/models/permissions.clj b/src/metabase/models/permissions.clj index 96a98c7431838097feaaad873b450348058349d2..1b3aac9d16cfec9ffdd57c7d85bc28fd48e03d28 100644 --- a/src/metabase/models/permissions.clj +++ b/src/metabase/models/permissions.clj @@ -75,7 +75,7 @@ permissions but no matching GTAP exists. * Segmented permissions can also be used to enforce column-level permissions -- any column not returned by the - underlying GTAP query is not allowed to be references by the parent query thru other means such as filter clauses. + underlying GTAP query is not allowed to be referenced by the parent query thru other means such as filter clauses. See [[metabase-enterprise.sandbox.query-processor.middleware.column-level-perms-check]]. * GTAPs are not allowed to add columns not present in the original Table, or change their effective type to diff --git a/src/metabase/query_processor.clj b/src/metabase/query_processor.clj index b3be35057859b5fd3690bcf85561cb568c5c47a4..bfff92ebfe74bdf01fbf4be36bda05a8ed398e45 100644 --- a/src/metabase/query_processor.clj +++ b/src/metabase/query_processor.clj @@ -105,6 +105,7 @@ #'resolve-joined-fields/resolve-joined-fields #'fix-bad-refs/fix-bad-references #'escape-join-aliases/escape-join-aliases + ;; yes, this is called a second time, because we need to handle any joins that got added (resolve 'ee.sandbox.rows/apply-sandboxing) #'qp.cumulative-aggregations/rewrite-cumulative-aggregations #'qp.pre-alias-aggregations/pre-alias-aggregations