diff --git a/enterprise/backend/src/metabase_enterprise/sandbox/api/util.clj b/enterprise/backend/src/metabase_enterprise/sandbox/api/util.clj index f6671742e6b92d43a8f058a3998bc5d3d1e8a782..4d5111318906a8f3416df4fbe72f3f1c22b01a38 100644 --- a/enterprise/backend/src/metabase_enterprise/sandbox/api/util.clj +++ b/enterprise/backend/src/metabase_enterprise/sandbox/api/util.clj @@ -23,11 +23,11 @@ (not (perms/set-has-full-permissions? perms-set (perms/table-query-path table-id))))) (defn enforced-sandboxes - "Given a list of sandboxes, filters it to only include sandboxes that should be enforced for the current user. - A sandbox is not enforced if the user is in a different permissions group that grants full access to the table." - [sandboxes] - (let [group-ids (dedupe (map :group_id sandboxes)) - perms (when (seq group-ids) + "Given a list of sandboxes and a list of permission group IDs that the current user is in, filter the sandboxes to + only include ones that should be enforced for the current user. A sandbox is not enforced if the user is in a + different permissions group that grants full access to the table." + [sandboxes group-ids] + (let [perms (when (seq group-ids) (db/select Permissions {:where [:in :group_id group-ids]})) group-id->perms-set (-> (group-by :group_id perms) (update-vals (fn [perms] (into #{} (map :object) perms))))] @@ -44,7 +44,7 @@ (let [group-ids (db/select-field :group_id PermissionsGroupMembership :user_id *current-user-id*) sandboxes (when (seq group-ids) (db/select GroupTableAccessPolicy :group_id [:in group-ids]))] - (seq (enforced-sandboxes sandboxes))) + (seq (enforced-sandboxes sandboxes group-ids))) ;; If no *current-user-id* is bound we can't check for sandboxes, so we should throw in this case to avoid ;; returning `false` for users who should actually be sandboxes. (throw (ex-info (str (tru "No current user found")) 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 7bf7dc2076e2dae30b650cd099361f2b98d912ce..b22b36858b175dc61e9bc871ced7e050bab67e19 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 @@ -70,11 +70,11 @@ (defn- tables->sandboxes [table-ids] (qp.store/cached [*current-user-id* table-ids] (let [group-ids (qp.store/cached *current-user-id* - (db/select-field :group_id PermissionsGroupMembership :user_id *current-user-id*)) + (db/select-field :group_id PermissionsGroupMembership :user_id *current-user-id*)) sandboxes (when (seq group-ids) (db/select GroupTableAccessPolicy :group_id [:in group-ids] :table_id [:in table-ids])) - enforced-sandboxes (mt.api.u/enforced-sandboxes sandboxes)] + enforced-sandboxes (mt.api.u/enforced-sandboxes sandboxes group-ids)] (when (seq enforced-sandboxes) (assert-one-gtap-per-table enforced-sandboxes) enforced-sandboxes)))) diff --git a/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj b/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj index 9b74d5654753afc56560d6f60b188a49b296de31..4c02429baf1c0b83e3ebefabe4d7bd5f5ba08390 100644 --- a/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj +++ b/enterprise/backend/test/metabase_enterprise/sandbox/query_processor/middleware/row_level_restrictions_test.clj @@ -324,6 +324,12 @@ (is (= [[10]] (run-venues-count-query))))) + (testing "Admins always bypass sandboxes, even if they are in a sandboxed group" + (met/with-gtaps-for-user :crowberto {:gtaps {:venues (venues-category-mbql-gtap-def)} + :attributes {"cat" 50}} + (is (= [[100]] + (run-venues-count-query))))) + (testing "Users with view access to the related collection should bypass segmented permissions" (mt/with-temp-copy-of-db (mt/with-temp* [Collection [collection]