Manual backport of #28961 to 46 release branch (#29007)

* Bind *card-id* when executing a dashboard subscription to fix perm checks (#28961)

* bind *card-id* when executing a dashboard subscription so that perm checks work properly

* refactor & address test comment

* fix spacing

* unskip repro that this also fixes

* fix lint

e2e/test/scenarios/sharing/reproductions/18009-nodata-creates-subscription-receives-error.cy.spec.js

@@ -6,7 +6,7 @@ import {
   sendEmailAndAssert,
 } from "e2e/support/helpers";
 
-describe.skip("issue 18009", { tags: "@external" }, () => {
+describe("issue 18009", { tags: "@external" }, () => {
   beforeEach(() => {
     restore();
     cy.signInAsAdmin();

src/metabase/pulse.clj

@@ -8,7 +8,9 @@
    [metabase.integrations.slack :as slack]
    [metabase.models.card :refer [Card]]
    [metabase.models.dashboard :refer [Dashboard]]
-   [metabase.models.dashboard-card :refer [DashboardCard]]
+   [metabase.models.dashboard-card
+    :as dashboard-card
+    :refer [DashboardCard]]
    [metabase.models.database :refer [Database]]
    [metabase.models.interface :as mi]
    [metabase.models.pulse :as pulse :refer [Pulse]]

src/metabase/query_processor/card.clj

@@ -189,8 +189,7 @@
                   ;; customize the `context` passed to the QP
                   (^:once fn* [query info]
                    (qp.streaming/streaming-response [context export-format (u/slugify (:card-name info))]
-                     (binding [qp.perms/*card-id* card-id]
-                       (qp-runner query info context)))))
+                     (qp-runner query info context))))
         card  (api/read-check (db/select-one [Card :id :name :dataset_query :database_id
                                               :cache_ttl :collection_id :dataset :result_metadata]
                                              :id card-id))
@@ -211,4 +210,5 @@
       (validate-card-parameters card-id (mbql.normalize/normalize-fragment [:parameters] parameters)))
     (log/tracef "Running query for Card %d:\n%s" card-id
                 (u/pprint-to-str query))
-    (run query info)))
+    (binding [qp.perms/*card-id* card-id]
+     (run query info))))

test/metabase/dashboard_subscription_test.clj

@@ -10,6 +10,8 @@
             PulseChannel
             PulseChannelRecipient
             User]]
+   [metabase.models.permissions :as perms]
+   [metabase.models.permissions-group :as perms-group]
    [metabase.models.pulse :as pulse]
    [metabase.pulse]
    [metabase.pulse.render.body :as body]
@@ -451,6 +453,25 @@
       (is (= [{:text "Doohickey and Gizmo"}]
              (@#'metabase.pulse/execute-dashboard {:creator_id (mt/user->id :rasta)} dashboard))))))
 
+(deftest no-native-perms-test
+  (testing "A native query on a dashboard executes succesfully even if the subscription creator does not have native
+           query permissions (#28947)"
+    (let [native-perm-path     [:groups (u/the-id (perms-group/all-users)) (mt/id) :data :native]
+          original-native-perm (get-in (perms/data-perms-graph) native-perm-path)]
+      (try
+        (mt/with-temp* [Dashboard [{dashboard-id :id, :as dashboard} {:name "Dashboard"}]
+                        Card      [{card-id :id} {:name          "Products (SQL)"
+                                                  :dataset_query (mt/native-query
+                                                                  {:query "SELECT * FROM venues LIMIT 1"})}]
+                        DashboardCard [_ {:dashboard_id dashboard-id
+                                          :card_id      card-id}]]
+          (perms/update-data-perms-graph! (assoc-in (perms/data-perms-graph) native-perm-path :none))
+          (is (= [[1 "Red Medicine" 4 10.0646 -165.374 3]]
+                 (-> (@#'metabase.pulse/execute-dashboard {:creator_id (mt/user->id :rasta)} dashboard)
+                     first :result :data :rows))))
+        (finally
+          (perms/update-data-perms-graph! (assoc-in (perms/data-perms-graph) native-perm-path original-native-perm)))))))
+
 (deftest link-cards-and-actions-are-skipped-test
   (testing "Actions and link cards should be filtered out"
     (t2.with-temp/with-temp

test/metabase/models/permissions_test.clj

@@ -872,8 +872,8 @@
   (is (= :non-scoped      (perms/classify-path "/application/monitoring/")))
 
   (is (= :query-v2        (perms/classify-path "/query/db/0/native/")))
-  (is (= :data-v2         (perms/classify-path "/data/db/3/schema/something/table/3/")))
-  )
+  (is (= :data-v2         (perms/classify-path "/data/db/3/schema/something/table/3/"))))
+
 
 (deftest data-permissions-classify-path
   (is (= :data (perms/classify-path "/db/3/")))