From 1af6df3190c0be433031a22eae523fc2b6cc5b04 Mon Sep 17 00:00:00 2001
From: John Bodley <4567245+john-bodley@users.noreply.github.com>
Date: Tue, 22 Aug 2023 09:58:43 -0700
Subject: [PATCH] fix: Native filter dashboard RBAC aware dataset permission
 (#25029)

(cherry picked from commit 60889d27edeeb306cff763743254ca0655faf4b5)
---
 .../components/nativeFilters/utils.ts         |  2 +
 superset/security/manager.py                  | 35 +++++++++--
 tests/integration_tests/security_tests.py     | 63 ++++++++++++++++++-
 3 files changed, 91 insertions(+), 9 deletions(-)

diff --git a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
index b4284a8a63d1c..7194e2435f3d6 100644
--- a/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
+++ b/superset-frontend/src/dashboard/components/nativeFilters/utils.ts
@@ -55,6 +55,7 @@ export const getFormData = ({
   granularity_sqla,
   type,
   dashboardId,
+  id,
 }: Partial<Filter> & {
   dashboardId: number;
   datasetId?: number;
@@ -94,6 +95,7 @@ export const getFormData = ({
     viz_type: filterType,
     type,
     dashboardId,
+    native_filter_id: id,
   };
 };
 
diff --git a/superset/security/manager.py b/superset/security/manager.py
index a32ef9a1b9d57..028fd8762f5f8 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -16,6 +16,7 @@
 # under the License.
 # pylint: disable=too-many-lines
 """A set of constants and methods to manage permissions and security"""
+import json
 import logging
 import re
 import time
@@ -1876,14 +1877,36 @@ def raise_for_access(
                         .one_or_none()
                     )
                     and dashboard_.roles
-                    and (slice_id := form_data.get("slice_id"))
                     and (
-                        slc := self.get_session.query(Slice)
-                        .filter(Slice.id == slice_id)
-                        .one_or_none()
+                        (
+                            # Native filter.
+                            form_data.get("type") == "NATIVE_FILTER"
+                            and (native_filter_id := form_data.get("native_filter_id"))
+                            and dashboard_.json_metadata
+                            and (json_metadata := json.loads(dashboard_.json_metadata))
+                            and any(
+                                target.get("datasetId") == datasource.id
+                                for fltr in json_metadata.get(
+                                    "native_filter_configuration",
+                                    [],
+                                )
+                                for target in fltr.get("targets", [])
+                                if native_filter_id == fltr.get("id")
+                            )
+                        )
+                        or (
+                            # Chart.
+                            form_data.get("type") != "NATIVE_FILTER"
+                            and (slice_id := form_data.get("slice_id"))
+                            and (
+                                slc := self.get_session.query(Slice)
+                                .filter(Slice.id == slice_id)
+                                .one_or_none()
+                            )
+                            and slc in dashboard_.slices
+                            and slc.datasource == datasource
+                        )
                     )
-                    and slc in dashboard_.slices
-                    and slc.datasource == datasource
                     and self.can_access_dashboard(dashboard_)
                 )
             ):
diff --git a/tests/integration_tests/security_tests.py b/tests/integration_tests/security_tests.py
index 4767e5af0e8cb..f741ec43153e2 100644
--- a/tests/integration_tests/security_tests.py
+++ b/tests/integration_tests/security_tests.py
@@ -15,6 +15,7 @@
 # specific language governing permissions and limitations
 # under the License.
 # isort:skip_file
+import json
 import inspect
 import time
 import unittest
@@ -1718,6 +1719,21 @@ def test_raise_for_access_rbac(
         world_health = self.get_dash_by_slug("world_health")
         treemap = self.get_slice("Treemap", db.session, expunge_from_session=False)
 
+        births.json_metadata = json.dumps(
+            {
+                "native_filter_configuration": [
+                    {
+                        "id": "NATIVE_FILTER-ABCDEFGH",
+                        "targets": [{"datasetId": birth_names.id}],
+                    },
+                    {
+                        "id": "NATIVE_FILTER-IJKLMNOP",
+                        "targets": [{"datasetId": treemap.id}],
+                    },
+                ]
+            }
+        )
+
         mock_g.user = security_manager.find_user("gamma")
         mock_is_owner.return_value = False
         mock_can_access.return_value = False
@@ -1725,7 +1741,6 @@ def test_raise_for_access_rbac(
 
         for kwarg in ["query_context", "viz"]:
             births.roles = []
-            db.session.flush()
 
             # No dashboard roles.
             with self.assertRaises(SupersetSecurityException):
@@ -1742,7 +1757,6 @@ def test_raise_for_access_rbac(
                 )
 
             births.roles = [self.get_role("Gamma")]
-            db.session.flush()
 
             # Undefined dashboard.
             with self.assertRaises(SupersetSecurityException):
@@ -1807,7 +1821,50 @@ def test_raise_for_access_rbac(
                 }
             )
 
-        db.session.rollback()
+            # Ill-defined native filter.
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={
+                                "dashboardId": births.id,
+                                "type": "NATIVE_FILTER",
+                            },
+                        )
+                    }
+                )
+
+            # Native filter not associated with said datasource.
+            with self.assertRaises(SupersetSecurityException):
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={
+                                "dashboardId": births.id,
+                                "native_filter_id": "NATIVE_FILTER-IJKLMNOP",
+                                "type": "NATIVE_FILTER",
+                            },
+                        )
+                    }
+                )
+
+            # Native filter associated with said datasource.
+            security_manager.raise_for_access(
+                **{
+                    kwarg: Mock(
+                        datasource=birth_names,
+                        form_data={
+                            "dashboardId": births.id,
+                            "native_filter_id": "NATIVE_FILTER-ABCDEFGH",
+                            "type": "NATIVE_FILTER",
+                        },
+                    )
+                }
+            )
+
+        db.session.expunge_all()
 
     @patch("superset.security.manager.g")
     def test_get_user_roles(self, mock_g):