You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by va...@apache.org on 2026/04/09 05:24:41 UTC
(superset) 01/01: fix: Drill to Detail for Embedded
This is an automated email from the ASF dual-hosted git repository.
vavila pushed a commit to branch fix/d2d-embedded
in repository https://gitbox.apache.org/repos/asf/superset.git
commit 6f0821eb9fcbd0522caa57c9a3d54982e4aaeee4
Author: Vitor Avila <vi...@gmail.com>
AuthorDate: Thu Apr 9 02:24:18 2026 -0300
fix: Drill to Detail for Embedded
---
superset/security/manager.py | 82 +++++++++-------
superset/views/datasource/utils.py | 7 +-
superset/views/datasource/views.py | 8 +-
tests/integration_tests/datasource_tests.py | 20 +++-
.../security/guest_token_security_tests.py | 103 ++++++++++++++++++++-
5 files changed, 180 insertions(+), 40 deletions(-)
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 2c10cd8ce02..acf44c925a9 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -725,27 +725,36 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
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 (
+ not form_data.get("slice_id")
+ and not form_data.get("chart_id")
+ 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 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
)
)
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_)
)
diff --git a/superset/views/datasource/utils.py b/superset/views/datasource/utils.py
index 096b8482c59..deec4067a56 100644
--- a/superset/views/datasource/utils.py
+++ b/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,
)
diff --git a/superset/views/datasource/views.py b/superset/views/datasource/views.py
index 6796fd4cdc5..c7b130a4b9a 100644
--- a/superset/views/datasource/views.py
+++ b/superset/views/datasource/views.py
@@ -212,16 +212,15 @@ class Datasource(BaseSupersetView):
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 @@ class Datasource(BaseSupersetView):
page=params["page"],
per_page=params["per_page"],
payload=payload,
+ dashboard_id=dashboard_id,
)
return self.json_response({"result": rv})
diff --git a/tests/integration_tests/datasource_tests.py b/tests/integration_tests/datasource_tests.py
index 319b8573720..22ab6fbc608 100644
--- a/tests/integration_tests/datasource_tests.py
+++ b/tests/integration_tests/datasource_tests.py
@@ -24,7 +24,7 @@ import prison
import pytest
from flask import current_app
-from superset import db
+from superset import db, security_manager
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,9 +531,19 @@ class TestDatasource(SupersetTestCase):
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
+ self.login(GAMMA_USERNAME)
+
+ perm_view = security_manager.find_permission_view_menu(
+ "can_samples", "Datasource"
+ )
+ security_manager.add_permission_role(
+ security_manager.find_role("Gamma"), perm_view
+ )
mock_is_guest_user.return_value = True
mock_has_guest_access.return_value = True
mock_rls.return_value = []
@@ -543,6 +553,10 @@ class TestDatasource(SupersetTestCase):
resp = self.client.post(uri, json={})
assert resp.status_code == 200
+ security_manager.del_permission_role(
+ security_manager.find_role("Gamma"), perm_view
+ )
+
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
@mock.patch(
"superset.security.manager.SupersetSecurityManager.get_guest_rls_filters"
diff --git a/tests/integration_tests/security/guest_token_security_tests.py b/tests/integration_tests/security/guest_token_security_tests.py
index edc1e0a649f..fdfed3c720a 100644
--- a/tests/integration_tests/security/guest_token_security_tests.py
+++ b/tests/integration_tests/security/guest_token_security_tests.py
@@ -356,7 +356,103 @@ class TestGuestUserDatasourceAccess(SupersetTestCase):
}
)
- 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,
+ },
+ # D2D QueryContexts are built without a stored chart.
+ # slice_=None prevents query_context_modified() from
+ # incorrectly flagging this as a guest payload modification.
+ 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 +462,12 @@ class TestGuestUserDatasourceAccess(SupersetTestCase):
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=[],
)
}
)