fix: Drill to Detail for Embedded

superset/security/manager.py

@@ -725,27 +725,36 @@ def _validate_child_in_parent_multilayer(
         except (json.JSONDecodeError, TypeError):
             return False
 
-    def has_drill_by_access(
+    def has_drill_access(
         self,
         form_data: dict[str, Any],
         dashboard: "Dashboard",
         datasource: "BaseDatasource | Explorable",
     ) -> bool:
         """
-        Return True if the form_data is performing a supported drill by operation,
-        False otherwise.
+        Return True if the form_data is performing a supported drill operation
+        (Drill to Detail or Drill By), False otherwise.
 
         :param form_data: The form_data included in the request.
         :param dashboard: The dashboard the user is drilling from.
         :param datasource: The datasource being queried
-        :returns: Whether the user has drill by access.
+        :returns: Whether the user has drill access.
         """
 
         from superset.models.slice import Slice
 
+        # Drill to Detail: no slice/chart context, dataset must belong to the dashboard
+        if (
+            form_data.get("slice_id") is None
+            and form_data.get("chart_id") is None
+            and datasource in dashboard.datasources
+        ):
+            return True
+
+        # Drill By: slice_id is 0 (sentinel), chart_id identifies the source chart,
+        # and the requested groupby columns must be drillable
         return bool(
-            form_data.get("type") != "NATIVE_FILTER"
-            and form_data.get("slice_id") == 0
+            form_data.get("slice_id") == 0
             and (chart_id := form_data.get("chart_id"))
             and (
                 slc := self.session.query(Slice)
@@ -2630,40 +2639,51 @@ def raise_for_access(  # noqa: C901
                             )
                         )
                         or (
-                            # Chart.
                             form_data.get("type") != "NATIVE_FILTER"
-                            and (slice_id := form_data.get("slice_id"))
                             and (
-                                # Direct chart access (no parent)
                                 (
-                                    form_data.get("parent_slice_id") is None
+                                    # Chart.
+                                    (slice_id := form_data.get("slice_id"))
                                     and (
-                                        slc := self.session.query(Slice)
-                                        .filter(Slice.id == slice_id)
-                                        .one_or_none()
+                                        # Direct chart access (no parent)
+                                        (
+                                            form_data.get("parent_slice_id") is None
+                                            and (
+                                                slc := self.session.query(Slice)
+                                                .filter(Slice.id == slice_id)
+                                                .one_or_none()
+                                            )
+                                            and slc in dashboard_.slices
+                                            and slc.datasource == datasource
+                                        )
+                                        or
+                                        # Multi-layer chart child access (has parent)
+                                        (
+                                            (
+                                                parent_id := form_data.get(
+                                                    "parent_slice_id"
+                                                )
+                                            )
+                                            and (
+                                                parent_slc := self.session.query(Slice)
+                                                .filter(Slice.id == parent_id)
+                                                .one_or_none()
+                                            )
+                                            and parent_slc in dashboard_.slices
+                                            # Validate child is actually part of parent's config    # noqa: E501
+                                            and self._validate_child_in_parent_multilayer(  # noqa: E501
+                                                child_slice_id=slice_id,
+                                                parent_slice=parent_slc,
+                                            )
+                                        )
                                     )
-                                    and slc in dashboard_.slices
-                                    and slc.datasource == datasource
                                 )
-                                or
-                                # Multi-layer chart child access (has parent)
-                                (
-                                    (parent_id := form_data.get("parent_slice_id"))
-                                    and (
-                                        parent_slc := self.session.query(Slice)
-                                        .filter(Slice.id == parent_id)
-                                        .one_or_none()
-                                    )
-                                    and parent_slc in dashboard_.slices
-                                    # Validate child is actually part of parent's config
-                                    and self._validate_child_in_parent_multilayer(
-                                        child_slice_id=slice_id,
-                                        parent_slice=parent_slc,
-                                    )
+                                # D2D or Drill By
+                                or self.has_drill_access(
+                                    form_data, dashboard_, datasource
                                 )
                             )
                         )
-                        or self.has_drill_by_access(form_data, dashboard_, datasource)
                     )
                     and self.can_access_dashboard(dashboard_)
                 )

superset/views/datasource/utils.py

@@ -94,13 +94,15 @@ def get_samples(  # pylint: disable=too-many-arguments
     force: bool = False,
     page: int = 1,
     per_page: int = 1000,
-    payload: Optional[SamplesPayloadSchema] = None,
+    payload: SamplesPayloadSchema | None = None,
+    dashboard_id: int | None = None,
 ) -> dict[str, Any]:
     datasource = DatasourceDAO.get_datasource(
         datasource_type=datasource_type,
         database_id_or_uuid=str(datasource_id),
     )
 
+    form_data = {"dashboardId": dashboard_id} if dashboard_id else None
     limit_clause = get_limit_clause(page, per_page)
 
     # todo(yongjie): Constructing count(*) and samples in the same query_context,
@@ -112,6 +114,7 @@ def get_samples(  # pylint: disable=too-many-arguments
                 "id": datasource.id,
             },
             queries=[limit_clause],
+            form_data=form_data,
             result_type=ChartDataResultType.SAMPLES,
             force=force,
         )
@@ -128,6 +131,7 @@ def get_samples(  # pylint: disable=too-many-arguments
                 "id": datasource.id,
             },
             queries=[{**payload, **limit_clause}],
+            form_data=form_data,
             result_type=ChartDataResultType.DRILL_DETAIL,
             force=force,
         )
@@ -148,6 +152,7 @@ def get_samples(  # pylint: disable=too-many-arguments
             "id": datasource.id,
         },
         queries=[{**payload, **count_star_metric} if payload else count_star_metric],
+        form_data=form_data,
         result_type=ChartDataResultType.FULL,
         force=force,
     )

superset/views/datasource/views.py

@@ -212,16 +212,15 @@ def samples(self) -> FlaskResponse:
             payload = SamplesPayloadSchema().load(request.json)
         except ValidationError as err:
             return json_error_response(err.messages, status=400)
-
+        dashboard_id = None
         if security_manager.is_guest_user():
             if not params["dashboard_id"]:
                 return json_error_response(_("Forbidden"), status=403)
+            dashboard_id = params["dashboard_id"]
             dataset = DatasetDAO.find_by_id(
                 params["datasource_id"], skip_base_filter=True
             )
-            dashboard = DashboardDAO.find_by_id(
-                params["dashboard_id"], skip_base_filter=True
-            )
+            dashboard = DashboardDAO.find_by_id(dashboard_id, skip_base_filter=True)
             if not (dashboard and dataset):
                 return self.response_404()
             if not security_manager.can_drill_dataset_via_dashboard_access(
@@ -237,6 +236,7 @@ def samples(self) -> FlaskResponse:
             page=params["page"],
             per_page=params["per_page"],
             payload=payload,
+            dashboard_id=dashboard_id,
         )
         return self.json_response({"result": rv})
 

tests/integration_tests/datasource_tests.py

@@ -24,7 +24,7 @@
 import pytest
 from flask import current_app
 
-from superset import db
+from superset import db, security_manager as sm
 from superset.commands.dataset.exceptions import DatasetNotFoundError
 from superset.common.utils.query_cache_manager import QueryCacheManager
 from superset.connectors.sqla.models import (  # noqa: F401
@@ -531,17 +531,26 @@ def test_get_samples_embedded_user(
         self, mock_has_guest_access, mock_is_guest_user, mock_rls
     ):
         """
-        Embedded user can access the /samples view.
+        Embedded guest user can access /samples (for D2D) via the dashboard context
+        passed as form_data to QueryContextFactory.
         """
-        self.login(ADMIN_USERNAME)
+        # Gamma role doesn't have dataset access (mimic embedded role),
+        # but needs access to the /samples endpoint
+        gamma_role = sm.find_role("Gamma")
+        perm_view = sm.find_permission_view_menu("can_samples", "Datasource")
+        sm.add_permission_role(gamma_role, perm_view)
+        self.login(GAMMA_USERNAME)
         mock_is_guest_user.return_value = True
         mock_has_guest_access.return_value = True
         mock_rls.return_value = []
         tbl = self.get_table(name="birth_names")
         dash = self.get_dash_by_slug("births")
-        uri = f"/datasource/samples?datasource_id={tbl.id}&datasource_type=table&dashboard_id={dash.id}"  # noqa: E501
-        resp = self.client.post(uri, json={})
-        assert resp.status_code == 200
+        try:
+            uri = f"/datasource/samples?datasource_id={tbl.id}&datasource_type=table&dashboard_id={dash.id}"  # noqa: E501
+            resp = self.client.post(uri, json={})
+            assert resp.status_code == 200
+        finally:
+            sm.del_permission_role(gamma_role, perm_view)
 
     @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
     @mock.patch(

tests/integration_tests/security/guest_token_security_tests.py

@@ -356,7 +356,100 @@ def test_raise_for_access__no_dashboard_in_form_data(self):
                     }
                 )
 
-    def test_raise_for_access__no_chart_in_form_data(self):
+    def test_raise_for_access__drill_to_detail_happy_path(self):
+        """
+        Drill to Detail: no slice_id in form_data, datasource is on the dashboard
+        the embedded user has access to.
+        """
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            security_manager.raise_for_access(
+                **{
+                    kwarg: Mock(
+                        datasource=self.datasource,
+                        form_data={
+                            "dashboardId": self.dash.id,
+                        },
+                        slice_=None,
+                        queries=[],
+                    )
+                }
+            )
+
+    def test_raise_for_access__drill_to_detail_datasource_not_on_dashboard(self):
+        """
+        Drill to Detail is denied when the target datasource is not associated
+        with the dashboard the embedded user has access to.
+        """
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):  # noqa: PT027
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.other_datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                            },
+                            slice_=None,
+                            queries=[],
+                        )
+                    }
+                )
+
+    def test_raise_for_access__drill_by_happy_path(self):
+        """
+        Drill By: slice_id=0 (sentinel), chart_id points to a chart on the dashboard
+        whose datasource matches, the requested groupby column is drillable and the
+        embedded user has access to.
+        """
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            security_manager.raise_for_access(
+                **{
+                    kwarg: Mock(
+                        datasource=self.datasource,
+                        form_data={
+                            "dashboardId": self.dash.id,
+                            "slice_id": 0,
+                            "chart_id": self.chart.id,
+                            "groupby": ["gender"],
+                        },
+                        slice_=None,
+                        queries=[],
+                    )
+                }
+            )
+
+    def test_raise_for_access__drill_by_chart_not_on_dashboard(self):
+        """
+        Drill By is denied when chart_id refers to a chart that is not on the
+        dashboard the embedded user has access to.
+        """
+        g.user = self.authorized_guest
+        for kwarg in ["viz", "query_context"]:
+            with self.assertRaises(SupersetSecurityException):  # noqa: PT027
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=self.other_datasource,
+                            form_data={
+                                "dashboardId": self.dash.id,
+                                "slice_id": 0,
+                                "chart_id": self.other_chart.id,
+                                "groupby": ["gender"],
+                            },
+                            slice_=None,
+                            queries=[],
+                        )
+                    }
+                )
+
+    def test_raise_for_access__drill_by_columns_not_drillable(self):
+        """
+        Drill By is denied when the requested groupby columns are not marked as
+        drillable (groupby=True) on the datasource.
+        """
         g.user = self.authorized_guest
         for kwarg in ["viz", "query_context"]:
             with self.assertRaises(SupersetSecurityException):  # noqa: PT027
@@ -366,7 +459,12 @@ def test_raise_for_access__no_chart_in_form_data(self):
                             datasource=self.datasource,
                             form_data={
                                 "dashboardId": self.dash.id,
+                                "slice_id": 0,
+                                "chart_id": self.chart.id,
+                                "groupby": ["__not_a_drillable_column__"],
                             },
+                            slice_=None,
+                            queries=[],
                         )
                     }
                 )

tests/integration_tests/security_tests.py

@@ -1895,16 +1895,19 @@ def test_raise_for_access_rbac(
                         }
                     )
 
-                # Undefined dashboard chart.
-                with self.assertRaises(SupersetSecurityException):  # noqa: PT027
-                    security_manager.raise_for_access(
-                        **{
-                            kwarg: Mock(
-                                datasource=birth_names,
-                                form_data={"dashboardId": births.id},
-                            )
-                        }
-                    )
+                # Drill to Detail (no slice_id/chart_id): datasource on dashboard.
+                # Access is granted via DASHBOARD_RBAC — D2D is a valid operation
+                # for users who have dashboard access.
+                security_manager.raise_for_access(
+                    **{
+                        kwarg: Mock(
+                            datasource=birth_names,
+                            form_data={"dashboardId": births.id},
+                            slice_=None,
+                            queries=[],
+                        )
+                    }
+                )
 
                 # Ill-defined dashboard chart.
                 with self.assertRaises(SupersetSecurityException):  # noqa: PT027