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 56a6083f58c6cf11156482e5304758811348c00e..e2a20d1148a1f71e18c4139603e702982e6a328a 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 @@ -2,6 +2,7 @@ (:require [metabase.api.common :as api] [metabase.models.data-permissions :as data-perms] + [metabase.models.query.permissions :as query-perms] [metabase.public-settings.premium-features :refer [defenterprise]] [metabase.query-processor.error-type :as qp.error-type] [metabase.util.i18n :refer [tru]])) @@ -12,9 +13,14 @@ current User has unrestricted data permissions from another Group. See the namespace documentation for [[metabase.models.collection]] for more details." :feature :advanced-permissions - [{database-id :database}] + [{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 (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) + table-ids)))) (throw (ex-info (tru "Blocked: you are not allowed to run queries against Database {0}." database-id) {:type qp.error-type/missing-required-permissions :actual-permissions @api/*current-user-permissions-set* diff --git a/enterprise/backend/test/metabase_enterprise/advanced_permissions/models/permissions/block_permissions_test.clj b/enterprise/backend/test/metabase_enterprise/advanced_permissions/models/permissions/block_permissions_test.clj index 995012c3eb8585701806c0ef822414523d9d11c3..619518cc41b09e49a749dadacb94a8eb03967d56 100644 --- a/enterprise/backend/test/metabase_enterprise/advanced_permissions/models/permissions/block_permissions_test.clj +++ b/enterprise/backend/test/metabase_enterprise/advanced_permissions/models/permissions/block_permissions_test.clj @@ -215,3 +215,31 @@ clojure.lang.ExceptionInfo #"Blocked: you are not allowed to run queries against Database \d+" (run-saved-question)))))))))))) + +(deftest legacy-no-self-service-test + (mt/with-temp-copy-of-db + (let [query {:database (mt/id) + :type :query + :query {:source-table (mt/id :venues) + :limit 1}}] + (mt/with-temp [User {user-id :id} {} + PermissionsGroup {group-id :id} {} + PermissionsGroupMembership _ {:group_id group-id :user_id user-id}] + (mt/with-premium-features #{:advanced-permissions} + (mt/with-no-data-perms-for-all-users! + (testing "legacy-no-self-service does not override block perms for a table" + (data-perms/set-database-permission! (perms-group/all-users) (mt/id) :perms/view-data :blocked) + (data-perms/set-database-permission! (perms-group/all-users) (mt/id) :perms/create-queries :no) + (data-perms/set-database-permission! group-id (mt/id) :perms/view-data :legacy-no-self-service) + (data-perms/set-database-permission! group-id (mt/id) :perms/create-queries :no) + (is (thrown-with-msg? + clojure.lang.ExceptionInfo + #"Blocked: you are not allowed to run queries against Database \d+" + (mt/with-current-user user-id + (#'qp.perms/check-block-permissions query))))) + + (testing "unrestricted overrides block perms for a table even if other tables have legacy-no-self-service" + (data-perms/set-table-permission! group-id (mt/id :venues) :perms/view-data :unrestricted) + (data-perms/set-table-permission! group-id (mt/id :orders) :perms/view-data :legacy-no-self-service) + (is (true? (mt/with-current-user user-id + (#'qp.perms/check-block-permissions query))))))))))) diff --git a/src/metabase/models/data_permissions.clj b/src/metabase/models/data_permissions.clj index f918f9cf0d43900a9d613f4dfdd5894bb1f254f1..89f64ca69ecf8ce56c38080efa1222c112cece22 100644 --- a/src/metabase/models/data_permissions.clj +++ b/src/metabase/models/data_permissions.clj @@ -351,7 +351,7 @@ (mu/defn full-db-permission-for-user :- PermissionValue "Returns the effective *db-level* permission value for a given user, permission type, and database ID. If the user has multiple permissions for the given type in different groups, they are coalesced into a single value. The - db-level permission is the *most* restrictive table-level permission within that schema." + db-level permission is the *most* restrictive table-level permission within that database." [user-id perm-type database-id] (when (not= :model/Table (model-by-perm-type perm-type)) (throw (ex-info (tru "Permission type {0} is not a table-level permission." perm-type) diff --git a/src/metabase/models/query/permissions.clj b/src/metabase/models/query/permissions.clj index f9ac93904dd9b15d6599988b4cec206103b93949..b5ba05a87422fc78bc13ded283b45ac9296135d0 100644 --- a/src/metabase/models/query/permissions.clj +++ b/src/metabase/models/query/permissions.clj @@ -184,7 +184,7 @@ (defn check-data-perms "Checks whether the current user has sufficient data permissions to run `query`. Returns `true` if the user has data - perms for the query, and throws an exception otherwise (exceptions cna be disabled by setting `throw-exceptions?` to + perms for the query, and throws an exception otherwise (exceptions can be disabled by setting `throw-exceptions?` to `false`). If the [:gtap ::perms] path is present in the query, these perms are implicitly granted to the current user."