From fedd722574ccb858243c4c5eb02837f5c3c6b80f Mon Sep 17 00:00:00 2001
From: Bryan Maass <bryan.maass@gmail.com>
Date: Wed, 7 Aug 2024 10:21:17 -0600
Subject: [PATCH] Revert "WIP: Pairing on making perms checking less wild"
Keep the same behavior, but stick with the saner flow control
This reverts commit 63bcb5b401138555a081378574b15d6506469e60.
---
.../models/permissions/block_permissions.clj | 4 ++--
src/metabase/models/query/permissions.clj | 16 ++++++++--------
.../query_processor/middleware/permissions.clj | 4 ++--
3 files changed, 12 insertions(+), 12 deletions(-)
diff --git a/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/block_permissions.clj b/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/block_permissions.clj
index aa76cc13110..e2a20d1148a 100644
--- a/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/block_permissions.clj
+++ b/enterprise/backend/src/metabase_enterprise/advanced_permissions/models/permissions/block_permissions.clj
@@ -15,8 +15,8 @@
:feature :advanced-permissions
[{database-id :database :as query}]
(or
- #_(not= :blocked (data-perms/full-db-permission-for-user api/*current-user-id* :perms/view-data database-id))
- (let [table-ids #p (query-perms/query->source-table-ids query)]
+ (not= :blocked (data-perms/full-db-permission-for-user api/*current-user-id* :perms/view-data database-id))
+ (let [table-ids (query-perms/query->source-table-ids query)]
(= #{:unrestricted}
(set
(map (partial data-perms/table-permission-for-user api/*current-user-id* :perms/view-data database-id)
diff --git a/src/metabase/models/query/permissions.clj b/src/metabase/models/query/permissions.clj
index 0e09f5d1d89..c2e8c514ac2 100644
--- a/src/metabase/models/query/permissions.clj
+++ b/src/metabase/models/query/permissions.clj
@@ -186,11 +186,12 @@
"Checks that the current user has at least `required-perm` for the entire DB specified by `db-id`."
[perm-type required-perm gtap-perms db-id]
(or
- (data-perms/at-least-as-permissive? perm-type
- (data-perms/full-db-permission-for-user api/*current-user-id* perm-type db-id)
- required-perm)
- (when gtap-perms
- (data-perms/at-least-as-permissive? perm-type gtap-perms required-perm))))
+ (data-perms/at-least-as-permissive? perm-type
+ (data-perms/full-db-permission-for-user api/*current-user-id* perm-type db-id)
+ required-perm)
+ (when gtap-perms
+ (data-perms/at-least-as-permissive? perm-type gtap-perms required-perm))
+ (throw (perms-exception {db-id {perm-type required-perm}}))))
(defn- has-perm-for-table?
"Checks that the current user has the permissions for tables specified in `table-id->perm`. This can be satisfied via
@@ -236,9 +237,8 @@
`throw-exceptions?` to `false`).
If the [:gtap ::perms] path is present in the query, these perms are implicitly granted to the current user."
- [{{gtap-perms :gtaps} ::perms, :as query}
- required-perms & {:keys [throw-exceptions?]
- :or {throw-exceptions? true}}]
+ [{{gtap-perms :gtaps} ::perms, :as query} required-perms & {:keys [throw-exceptions?]
+ :or {throw-exceptions? true}}]
(try
;; Check any required v1 paths
(when-let [paths (:paths required-perms)]
diff --git a/src/metabase/query_processor/middleware/permissions.clj b/src/metabase/query_processor/middleware/permissions.clj
index ab6ee26f786..28baa672172 100644
--- a/src/metabase/query_processor/middleware/permissions.clj
+++ b/src/metabase/query_processor/middleware/permissions.clj
@@ -97,8 +97,8 @@
;; set when querying for field values of dashboard filters, which only require
;; collection perms for the dashboard and not ad-hoc query perms
*param-values-query*
- (when-not (query-perms/has-perm-for-query? outer-query :perms/view-data required-perms)
- (throw (query-perms/perms-exception required-perms)))
+ (when-not (query-perms/check-data-perms outer-query required-perms :throw-exceptions? false)
+ (check-block-permissions outer-query))
:else
(do
--
GitLab