diff --git a/src/metabase/query_processor/middleware/permissions.clj b/src/metabase/query_processor/middleware/permissions.clj
index a302fcdce3e450b9c9ddf914d4614d1ef132d556..a442ed2bb45ff85a5a291d87b24a3a324e02d8b4 100644
--- a/src/metabase/query_processor/middleware/permissions.clj
+++ b/src/metabase/query_processor/middleware/permissions.clj
@@ -124,7 +124,7 @@
(defn- query-action-perms
[{:keys [database]}]
- #{(perms/execute-query-perms-path database)})
+ #{(perms/data-perms-path database)})
(s/defn check-query-action-permissions*
"Check that User with `user-id` has permissions to run query action `query`, or throw an exception."
@@ -135,7 +135,7 @@
(when-not (has-data-perms? (required-perms outer-query))
(check-block-permissions outer-query))
(when-not (has-data-perms? (query-action-perms outer-query))
- (throw (perms-exception required-perms))))
+ (throw (perms-exception (required-perms outer-query)))))
(defn check-query-action-permissions
"Middleware that check that the current user has permissions to run the current query action."
diff --git a/test/metabase/api/dashboard_test.clj b/test/metabase/api/dashboard_test.clj
index eb11f348d6cee28a521991633fad0e82416e1540..f846055073702a0162a226310bf455136a6554f1 100644
--- a/test/metabase/api/dashboard_test.clj
+++ b/test/metabase/api/dashboard_test.clj
@@ -2826,13 +2826,14 @@
(:cause
(mt/user-http-request :crowberto :post 400 execute-path
{:parameters {"name" "Birds"}})))))
- (testing "Without execute rights on the DB"
+ (testing "Without read rights on the DB"
+ (perms/revoke-data-perms! (perms-group/all-users) (mt/db))
(mt/with-actions-enabled
(is (partial= {:message "You do not have permissions to run this query."}
(mt/user-http-request :rasta :post 403 execute-path
{:parameters {"name" "Birds"}})))))
(testing "With execute rights on the DB"
- (perms/update-global-execution-permission! (:id (perms-group/all-users)) :all)
+ (perms/grant-full-data-permissions! (perms-group/all-users) (mt/db))
(try
(mt/with-actions-enabled
(is (contains? #{{:ID 76, :NAME "Birds"}
@@ -2841,7 +2842,7 @@
{:parameters {"name" "Birds"}})
:created-row))))
(finally
- (perms/update-global-execution-permission! (:id (perms-group/all-users)) :none)))))))))))
+ (perms/revoke-data-perms! (perms-group/all-users) (mt/db))))))))))))
(deftest dashcard-custom-action-execution-auth-test
(mt/with-temp-copy-of-db
@@ -2860,20 +2861,21 @@
(:cause
(mt/user-http-request :crowberto :post 400 execute-path
{:parameters {"id" 1}})))))
- (testing "Without execute rights on the DB"
+ (testing "Without read rights on the DB"
+ (perms/revoke-data-perms! (perms-group/all-users) (mt/db))
(mt/with-actions-enabled
(is (partial= {:message "You don't have permissions to do that."}
(mt/user-http-request :rasta :post 403 execute-path
{:parameters {"id" 1}})))))
- (testing "With execute rights on the DB"
- (perms/update-global-execution-permission! (:id (perms-group/all-users)) :all)
+ (testing "With read rights on the DB"
+ (perms/grant-full-data-permissions! (perms-group/all-users) (mt/db))
(try
(mt/with-actions-enabled
(is (= {:rows-affected 1}
(mt/user-http-request :rasta :post 200 execute-path
{:parameters {"id" 1}}))))
(finally
- (perms/update-global-execution-permission! (:id (perms-group/all-users)) :none)))))))))))
+ (perms/revoke-data-perms! (perms-group/all-users) (mt/db))))))))))))
(deftest dashcard-execution-fetch-prefill-test
(mt/test-drivers (mt/normal-drivers-with-feature :actions)
@@ -2918,51 +2920,53 @@
(is (partial= {:message "No destination parameter found for #{\"price\"}. Found: #{\"id\" \"name\"}"}
(mt/user-http-request :crowberto :post 400 execute-path {:parameters {"id" 1 "name" "Blueberries" "price" 1234}})))))))))))
-(defn- ee-features-enabled? []
- (u/ignore-exceptions
- (classloader/require 'metabase-enterprise.advanced-permissions.models.permissions)
- (some? (resolve 'metabase-enterprise.advanced-permissions.models.permissions/update-db-execute-permissions!))))
+;; Not relevant for 46; uncomment and make work for 47
+(comment
+ (defn- ee-features-enabled? []
+ (u/ignore-exceptions
+ (classloader/require 'metabase-enterprise.advanced-permissions.models.permissions)
+ (some? (resolve 'metabase-enterprise.advanced-permissions.models.permissions/update-db-execute-permissions!))))
-(deftest dashcard-action-execution-granular-auth-test
- (when (ee-features-enabled?)
- (mt/with-temp-copy-of-db
- (mt/with-actions-test-data-and-actions-enabled
- (mt/with-actions [{:keys [action-id model-id]} {}]
- (testing "Executing dashcard with action"
- (mt/with-temp* [Dashboard [{dashboard-id :id}]
- DashboardCard [{dashcard-id :id}
- {:dashboard_id dashboard-id
- :action_id action-id
- :card_id model-id}]]
- (let [execute-path (format "dashboard/%s/dashcard/%s/execute"
- dashboard-id
- dashcard-id)]
- (testing "with :advanced-permissions feature flag"
- (premium-features-test/with-premium-features #{:advanced-permissions}
- (testing "for non-magic group"
- (mt/with-temp* [PermissionsGroup [{group-id :id}]
- PermissionsGroupMembership [_ {:user_id (mt/user->id :rasta)
- :group_id group-id}]]
- (is (partial= {:message "You don't have permissions to do that."}
- (mt/user-http-request :rasta :post 403 execute-path
- {:parameters {"id" 1}}))
- "Execution permission should be required")
- (mt/user-http-request
- :crowberto :put 200 "permissions/execution/graph"
- (assoc-in (perms/execution-perms-graph) [:groups group-id (mt/id)] :all))
- (is (= :all
- (get-in (perms/execution-perms-graph) [:groups group-id (mt/id)]))
- "Should be able to set execution permission")
- (is (= {:rows-affected 1}
- (mt/user-http-request :rasta :post 200 execute-path
- {:parameters {"id" 1}}))
- "Execution and data permissions should be enough")
-
- (perms/update-data-perms-graph! [group-id (mt/id) :data]
- {:schemas :block})
- (perms/update-data-perms-graph! [(:id (perms-group/all-users)) (mt/id) :data]
- {:schemas :block})
- (is (partial= {:message "You don't have permissions to do that."}
- (mt/user-http-request :rasta :post 403 execute-path
- {:parameters {"id" 1}}))
- "Data permissions should be required")))))))))))))
+ (deftest dashcard-action-execution-granular-auth-test
+ (when (ee-features-enabled?)
+ (mt/with-temp-copy-of-db
+ (mt/with-actions-test-data-and-actions-enabled
+ (mt/with-actions [{:keys [action-id model-id]} {}]
+ (testing "Executing dashcard with action"
+ (mt/with-temp* [Dashboard [{dashboard-id :id}]
+ DashboardCard [{dashcard-id :id}
+ {:dashboard_id dashboard-id
+ :action_id action-id
+ :card_id model-id}]]
+ (let [execute-path (format "dashboard/%s/dashcard/%s/execute"
+ dashboard-id
+ dashcard-id)]
+ (testing "with :advanced-permissions feature flag"
+ (premium-features-test/with-premium-features #{:advanced-permissions}
+ (testing "for non-magic group"
+ (mt/with-temp* [PermissionsGroup [{group-id :id}]
+ PermissionsGroupMembership [_ {:user_id (mt/user->id :rasta)
+ :group_id group-id}]]
+ (is (partial= {:message "You don't have permissions to do that."}
+ (mt/user-http-request :rasta :post 403 execute-path
+ {:parameters {"id" 1}}))
+ "Execution permission should be required")
+ (mt/user-http-request
+ :crowberto :put 200 "permissions/execution/graph"
+ (assoc-in (perms/execution-perms-graph) [:groups group-id (mt/id)] :all))
+ (is (= :all
+ (get-in (perms/execution-perms-graph) [:groups group-id (mt/id)]))
+ "Should be able to set execution permission")
+ (is (= {:rows-affected 1}
+ (mt/user-http-request :rasta :post 200 execute-path
+ {:parameters {"id" 1}}))
+ "Execution and data permissions should be enough")
+
+ (perms/update-data-perms-graph! [group-id (mt/id) :data]
+ {:schemas :block})
+ (perms/update-data-perms-graph! [(:id (perms-group/all-users)) (mt/id) :data]
+ {:schemas :block})
+ (is (partial= {:message "You don't have permissions to do that."}
+ (mt/user-http-request :rasta :post 403 execute-path
+ {:parameters {"id" 1}}))
+ "Data permissions should be required"))))))))))))))