(superset) branch master updated: fix(views): enforce per-chart access check in legacy form_data endpoint (#40497)

[pm...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

sha174n pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new b8a2f925ee4 fix(views): enforce per-chart access check in legacy form_data endpoint (#40497)
b8a2f925ee4 is described below

commit b8a2f925ee4b9401084b4ca238be9909f7a62f4f
Author: Shaitan <10...@users.noreply.github.com>
AuthorDate: Wed Jun 3 12:56:03 2026 +0100

    fix(views): enforce per-chart access check in legacy form_data endpoint (#40497)
    
    Co-authored-by: Claude Opus 4.7 <no...@anthropic.com>
---
 superset/views/api.py                       | 19 ++++++++----
 tests/integration_tests/charts/api_tests.py | 46 +++++++++++++++++++++++++++++
 2 files changed, 59 insertions(+), 6 deletions(-)

diff --git a/superset/views/api.py b/superset/views/api.py
index c24a102d65e..e1c36bf71f3 100644
--- a/superset/views/api.py
+++ b/superset/views/api.py
@@ -24,13 +24,14 @@ from flask_appbuilder.api import rison as parse_rison
 from flask_appbuilder.security.decorators import has_access_api
 from flask_babel import lazy_gettext as _
 
-from superset import db, event_logger
+from superset import event_logger
 from superset.commands.chart.exceptions import (
+    ChartNotFoundError,
     TimeRangeAmbiguousError,
     TimeRangeParseFailError,
 )
+from superset.daos.chart import ChartDAO
 from superset.legacy import update_time_range
-from superset.models.slice import Slice
 from superset.superset_typing import FlaskResponse
 from superset.utils import json
 from superset.utils.date_parser import get_since_until
@@ -85,11 +86,17 @@ class Api(BaseSupersetView):
         Get the form_data stored in the database for existing slice.
         params: slice_id: integer
         """
-        form_data = {}
+        form_data: dict[str, Any] = {}
         if slice_id := request.args.get("slice_id"):
-            slc = db.session.query(Slice).filter_by(id=slice_id).one_or_none()
-            if slc:
-                form_data = slc.form_data.copy()
+            # Reuse ChartDAO.get_by_id_or_uuid so this endpoint applies the
+            # same ChartFilter (dataset-scoped) as ChartRestApi.get. Both a
+            # missing chart and a chart the caller cannot access surface as
+            # ChartNotFoundError, mapped to 404 so the status code cannot be
+            # used to distinguish the two cases.
+            try:
+                form_data = ChartDAO.get_by_id_or_uuid(slice_id).form_data.copy()
+            except ChartNotFoundError:
+                return self.json_response({}, 404)
 
         update_time_range(form_data)
 
diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py
index 699a126f6b8..47fc2712625 100644
--- a/tests/integration_tests/charts/api_tests.py
+++ b/tests/integration_tests/charts/api_tests.py
@@ -1769,6 +1769,52 @@ class TestChartApi(ApiOwnersTestCaseMixin, InsertChartMixin, SupersetTestCase):
         if slice:
             assert data["slice_id"] == slice.id
 
+    @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+    def test_query_form_data_no_data_access(self):
+        """
+        Chart API: query_form_data must refuse callers without
+        datasource_access on the chart's underlying dataset. Mirrors
+        the existing test_get_chart_no_data_access guard on
+        ChartRestApi (which also returns 404 to avoid leaking chart
+        existence to unauthorised callers).
+        """
+        self.login(GAMMA_USERNAME)
+        chart_no_access = (
+            db.session.query(Slice)
+            .filter_by(slice_name="Girl Name Cloud")
+            .one_or_none()
+        )
+        assert chart_no_access is not None, (
+            "fixture load_birth_names_dashboard_with_slices did not "
+            "create the 'Girl Name Cloud' slice"
+        )
+        uri = f"api/v1/form_data/?slice_id={chart_no_access.id}"
+        rv = self.client.get(uri)
+        # Match ChartRestApi.get: 404 for both missing AND forbidden so
+        # the endpoint cannot be used to enumerate chart IDs.
+        assert rv.status_code == 404, (
+            f"Gamma user without datasource_access should get 404 "
+            f"(status={rv.status_code}, body={rv.data[:200]!r})"
+        )
+        # Defence in depth: even if a future regression returns a non-
+        # 200 status with a partially-filled error envelope, ensure the
+        # caller cannot recover form_data fields.
+        assert b"datasource" not in rv.data
+        assert b"adhoc_filters" not in rv.data
+        assert b"viz_type" not in rv.data
+
+    def test_query_form_data_missing_slice(self):
+        """
+        Chart API: a non-existent slice_id must return the same 404 as a
+        forbidden one, so the status code cannot be used to enumerate
+        which slice IDs exist.
+        """
+        self.login(ADMIN_USERNAME)
+        max_id = db.session.query(func.max(Slice.id)).scalar() or 0
+        uri = f"api/v1/form_data/?slice_id={max_id + 10_000}"
+        rv = self.client.get(uri)
+        assert rv.status_code == 404
+
     @pytest.mark.usefixtures(
         "load_unicode_dashboard_with_slice",
         "load_energy_table_with_slice",