This is an automated email from the ASF dual-hosted git repository.
aminghadersohi 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 fd2740bec1e fix(charts): avoid loading semantic provider metadata
before denying chart data access (#45060)
fd2740bec1e is described below
commit fd2740bec1edf5603d8b78f2abce3ab7f261cbfd
Author: Mike Bridge <[email protected]>
AuthorDate: Sat Oct 10 16:27:16 2026 -0600
fix(charts): avoid loading semantic provider metadata before denying chart
data access (#45060)
Co-authored-by: Mike Bridge <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
---
superset/charts/data/api.py | 7 +-
superset/common/query_context_factory.py | 42 +-
superset/config.py | 6 +
superset/security/manager.py | 4 +
superset/views/api.py | 4 +-
.../charts/semantic_metadata_authz_tests.py | 640 +++++++++++++++++++++
tests/unit_tests/security/manager_test.py | 281 +++++++++
7 files changed, 981 insertions(+), 3 deletions(-)
diff --git a/superset/charts/data/api.py b/superset/charts/data/api.py
index 01e3657d761..1dfdf5c394e 100644
--- a/superset/charts/data/api.py
+++ b/superset/charts/data/api.py
@@ -49,6 +49,7 @@ from superset.commands.chart.exceptions import (
)
from superset.common.chart_data import ChartDataResultFormat,
ChartDataResultType
from superset.common.chart_data_timing import ChartDataExecutionResult
+from superset.common.query_context_factory import QueryContextFactory
from superset.connectors.sqla.models import BaseDatasource
from superset.constants import CACHE_DISABLED_TIMEOUT
from superset.daos.exceptions import DatasourceNotFound
@@ -750,7 +751,11 @@ class ChartDataRestApi(ChartRestApi):
"""
try:
- return ChartDataQueryContextSchema().load(form_data)
+ schema: ChartDataQueryContextSchema = ChartDataQueryContextSchema()
+ schema.query_context_factory = QueryContextFactory(
+ authorize_semantic_before_metadata=True
+ )
+ return schema.load(form_data)
except KeyError as ex:
raise ValidationError("Request is incorrect") from ex
except ValueError as ex:
diff --git a/superset/common/query_context_factory.py
b/superset/common/query_context_factory.py
index 31affea616b..2a4c66f3d8a 100644
--- a/superset/common/query_context_factory.py
+++ b/superset/common/query_context_factory.py
@@ -27,7 +27,9 @@ from superset.common.query_object_factory import
QueryObjectFactory
from superset.daos.chart import ChartDAO
from superset.daos.datasource import DatasourceDAO
from superset.explorables.base import Explorable
+from superset.extensions import security_manager
from superset.models.slice import Slice
+from superset.security.manager import STOCK_RAISE_FOR_ACCESS
from superset.superset_typing import Column
from superset.utils.core import DatasourceDict, DatasourceType, is_adhoc_column
@@ -35,15 +37,29 @@ if TYPE_CHECKING:
from superset.connectors.sqla.models import BaseDatasource
+def _uses_stock_raise_for_access() -> bool:
+ """Check the resolved method against the original stock function.
+
+ The semantic preflight passes empty queries. Instance wrappers and operator
+ reassignment of the base-class method must keep the completed-context path.
+ """
+ return (
+ getattr(security_manager.raise_for_access, "__func__", None)
+ is STOCK_RAISE_FOR_ACCESS
+ )
+
+
def create_query_object_factory() -> QueryObjectFactory:
return QueryObjectFactory(current_app.config, DatasourceDAO())
class QueryContextFactory: # pylint: disable=too-few-public-methods
_query_object_factory: QueryObjectFactory
+ _authorize_semantic_before_metadata: bool
- def __init__(self) -> None:
+ def __init__(self, authorize_semantic_before_metadata: bool = False) ->
None:
self._query_object_factory = create_query_object_factory()
+ self._authorize_semantic_before_metadata =
authorize_semantic_before_metadata
def create( # pylint: disable=too-many-arguments
self,
@@ -72,6 +88,30 @@ class QueryContextFactory: # pylint:
disable=too-few-public-methods
result_type = result_type or ChartDataResultType.FULL
result_format = result_format or ChartDataResultFormat.JSON
+ if (
+ self._authorize_semantic_before_metadata
+ and datasource_model_instance is not None
+ and DatasourceType(datasource["type"]) ==
DatasourceType.SEMANTIC_VIEW
+ ):
+ # Guest dashboard and payload checks need the completed query
context,
+ # and an operator EXTRA_RAISE_FOR_ACCESS_BYPASS hook or a custom
+ # security manager's raise_for_access may read the request's
queries;
+ # keep their authorization path and timing unchanged.
+ if (
+ not security_manager.is_guest_user()
+ and not current_app.config.get("EXTRA_RAISE_FOR_ACCESS_BYPASS")
+ and _uses_stock_raise_for_access()
+ ):
+ QueryContext(
+ datasource=datasource_model_instance,
+ queries=[],
+ slice_=slice_,
+ form_data=form_data,
+ result_type=result_type,
+ result_format=result_format,
+ cache_values={},
+ ).raise_for_access()
+
# The server pagination var is extracted from form data as the
# row limit for server pagination is more
# This particular flag server_pagination only exists for table viz type
diff --git a/superset/config.py b/superset/config.py
index 2ab845461bb..53f86824d2f 100644
--- a/superset/config.py
+++ b/superset/config.py
@@ -248,6 +248,9 @@ SUPERSET_DASHBOARD_PERIODICAL_REFRESH_WARNING_MESSAGE = None
SUPERSET_DASHBOARD_MANUAL_REFRESH_STAGGER_MS: int = 0
SUPERSET_DASHBOARD_POSITION_DATA_LIMIT = 65535
+# A manager that overrides raise_for_access receives the complete query
context,
+# so it turns off the semantic-view chart-data check that otherwise denies
before
+# provider metadata is loaded.
CUSTOM_SECURITY_MANAGER = None
SQLALCHEMY_TRACK_MODIFICATIONS = False
@@ -3473,6 +3476,9 @@ class ExtraAccessQueryFilters(TypedDict, total=False):
# Additional query filters for chart/dashboard list views.
EXTRA_ACCESS_QUERY_FILTERS: ExtraAccessQueryFilters = {}
# Bypass raise_for_access for specific assets. Return True to skip checks.
+# The hook receives the complete query context, so setting it turns off the
+# semantic-view chart-data check that otherwise denies before provider metadata
+# is loaded.
EXTRA_RAISE_FOR_ACCESS_BYPASS: Callable[..., bool] | None = None
# Resolve additional editor subjects for a resource. Also used for editorship
# checks and lockout-prevention logic.
diff --git a/superset/security/manager.py b/superset/security/manager.py
index 1161ce30389..b24d8bf1583 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -6786,3 +6786,7 @@ class SupersetSecurityManager( # pylint:
disable=too-many-public-methods
"User Registrations",
]:
security_menu.childs.remove(item)
+
+
+# Keep the original function identity before operator configuration can
replace it.
+STOCK_RAISE_FOR_ACCESS: Callable[..., None] =
SupersetSecurityManager.raise_for_access
diff --git a/superset/views/api.py b/superset/views/api.py
index e1c36bf71f3..b5d8532c9b0 100644
--- a/superset/views/api.py
+++ b/superset/views/api.py
@@ -138,5 +138,7 @@ class Api(BaseSupersetView):
# pylint: disable=import-outside-toplevel
from superset.common.query_context_factory import
QueryContextFactory
- self.query_context_factory = QueryContextFactory()
+ self.query_context_factory = QueryContextFactory(
+ authorize_semantic_before_metadata=True
+ )
return self.query_context_factory
diff --git a/tests/integration_tests/charts/semantic_metadata_authz_tests.py
b/tests/integration_tests/charts/semantic_metadata_authz_tests.py
new file mode 100644
index 00000000000..f2e34b1d232
--- /dev/null
+++ b/tests/integration_tests/charts/semantic_metadata_authz_tests.py
@@ -0,0 +1,640 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Check whether denied chart-data requests call semantic provider metadata."""
+
+from collections.abc import Callable
+from typing import Any, Literal
+from unittest.mock import Mock, patch, PropertyMock
+
+import pytest
+import sqlalchemy as sa
+from flask import current_app, g, Response
+
+from superset.common.query_context import QueryContext
+from superset.connectors.sqla.models import SqlaTable
+from superset.errors import ErrorLevel, SupersetError, SupersetErrorType
+from superset.exceptions import SupersetSecurityException
+from superset.extensions import db, security_manager
+from superset.models.dashboard import Dashboard
+from superset.security.manager import SupersetSecurityManager
+from superset.semantic_layers.models import SemanticLayer, SemanticView
+from superset.utils import json
+from tests.integration_tests.base_tests import SupersetTestCase
+from tests.integration_tests.conftest import with_feature_flags
+from tests.integration_tests.fixtures.birth_names_dashboard import (
+ load_birth_names_dashboard_with_slices, # noqa: F401
+ load_birth_names_data, # noqa: F401
+)
+
+
+class TestSemanticMetadataAuthorization(SupersetTestCase):
+ """Exercise the chart-data route with a provider that records metadata
calls."""
+
+ def test_denied_chart_data_skips_provider_metadata(self) -> None:
+ """A denied request should not load dimensions or metrics."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="authz-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="authz-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ try:
+ with patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ },
+ )
+ assert response.status_code == 403, response.json
+ provider.get_dimensions.assert_not_called()
+ provider.get_metrics.assert_not_called()
+ finally:
+ db.session.rollback()
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ def test_denied_legacy_query_api_skips_provider_metadata(self) -> None:
+ """The legacy ``/api/v1/query/`` route also denies before metadata."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="legacy-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="legacy-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ try:
+ with patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ):
+ response: Response = self.client.post(
+ "/api/v1/query/",
+ data={
+ "query_context": json.dumps(
+ {
+ "datasource": {
+ "id": view.id,
+ "type": "semantic_view",
+ },
+ "queries": [{"columns": [], "metrics": []}],
+ }
+ )
+ },
+ )
+ assert response.status_code == 403, response.json
+ provider.get_dimensions.assert_not_called()
+ provider.get_metrics.assert_not_called()
+ finally:
+ db.session.rollback()
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ def test_allowed_chart_data_uses_provider_metadata(self) -> None:
+ """An entitled role still builds and validates a semantic query."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="allowed-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="allowed-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ original_can_access: Callable[[str, str], bool] =
security_manager.can_access
+
+ def can_access(permission_name: str, view_name: str) -> bool:
+ """Give Gamma this view's datasource grant for the request."""
+ if permission_name == "datasource_access" and view_name ==
view.perm:
+ return True
+ return original_can_access(permission_name, view_name)
+
+ access_spy: Mock
+ try:
+ with (
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ patch.object(security_manager, "can_access",
side_effect=can_access),
+ patch.object(
+ QueryContext,
+ "raise_for_access",
+ autospec=True,
+ side_effect=QueryContext.raise_for_access,
+ ) as access_spy,
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "result_type": "query",
+ },
+ )
+ assert response.status_code == 200, response.json
+ provider.get_dimensions.assert_called()
+ # The preflight and the final check each decide on a query context.
+ assert access_spy.call_count == 2
+ finally:
+ db.session.rollback()
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ def test_payload_based_access_bypass_still_sees_queries(self) -> None:
+ """An operator bypass hook keeps receiving the request's real
queries."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="bypass-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="bypass-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+
+ def bypass(**kwargs: object) -> bool:
+ """Grant only when the request's queries are visible to the
hook."""
+ query_context: object = kwargs["query_context"]
+ return bool(getattr(query_context, "queries", None))
+
+ try:
+ with (
+ patch.dict(
+ current_app.config, {"EXTRA_RAISE_FOR_ACCESS_BYPASS":
bypass}
+ ),
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "result_type": "query",
+ },
+ )
+ assert response.status_code == 200, response.json
+ finally:
+ db.session.rollback()
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ def test_overridden_access_check_still_sees_queries(self) -> None:
+ """A custom security manager's raise_for_access keeps receiving the
+ request's real queries, so the preflight does not run for it."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(
+ name="override-metadata-layer", type="test"
+ )
+ view: SemanticView = SemanticView(
+ name="override-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ manager: SupersetSecurityManager =
security_manager._get_current_object() # noqa: SLF001
+ stock_class: type[SupersetSecurityManager] = type(manager)
+ original_can_access: Callable[[str, str], bool] =
security_manager.can_access
+
+ class QueryCheckingSecurityManager(stock_class): # type:
ignore[valid-type, misc]
+ """Authorizes per requested query, like a column or metric
check."""
+
+ def raise_for_access(self, **kwargs: Any) -> None:
+ query_context: QueryContext | None =
kwargs.get("query_context")
+ if query_context is not None and not query_context.queries:
+ raise SupersetSecurityException(
+ SupersetError(
+ message="No requested queries to authorize",
+
error_type=SupersetErrorType.DATASOURCE_SECURITY_ACCESS_ERROR,
+ level=ErrorLevel.ERROR,
+ )
+ )
+ super().raise_for_access(**kwargs)
+
+ def can_access(permission_name: str, view_name: str) -> bool:
+ """Give Gamma this view's datasource grant for the request."""
+ if permission_name == "datasource_access" and view_name ==
view.perm:
+ return True
+ return original_can_access(permission_name, view_name)
+
+ # patch.object cannot restore __class__, so swap and restore it here.
+ manager.__class__ = QueryCheckingSecurityManager
+ try:
+ with (
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ patch.object(security_manager, "can_access",
side_effect=can_access),
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "result_type": "query",
+ },
+ )
+ assert response.status_code == 200, response.json
+ finally:
+ manager.__class__ = stock_class
+ db.session.rollback()
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ @with_feature_flags(ENABLE_VIEWERS=True)
+ def test_denied_viewer_filter_skips_provider_metadata(self) -> None:
+ """A denied ordinary dashboard filter must not load provider
metadata."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="viewer-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="viewer-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.flush()
+ metadata: str = json.dumps(
+ {
+ "native_filter_configuration": [
+ {
+ "id": "filter-1",
+ "targets": [
+ {
+ "datasetId": view.id + 1,
+ "datasourceType": "semantic_view",
+ }
+ ],
+ }
+ ]
+ }
+ )
+ dashboard_id: int = db.session.execute(
+ sa.insert(Dashboard.__table__).values(
+ dashboard_title="viewer-metadata-dashboard",
+ published=True,
+ json_metadata=metadata,
+ )
+ ).inserted_primary_key[0]
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ provider.features = frozenset()
+ provider.selection_identity_version = None
+ provider.uid.return_value = "viewer-metadata-view"
+ try:
+ with (
+ patch.dict(current_app.config, {"VIEWER_PROMISCUOUS_MODE":
True}),
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ patch.object(security_manager, "is_viewer", return_value=True),
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "form_data": {
+ "dashboardId": dashboard_id,
+ "type": "NATIVE_FILTER",
+ "native_filter_id": "filter-1",
+ },
+ },
+ )
+ assert response.status_code == 403, response.json
+ provider.get_dimensions.assert_not_called()
+ provider.get_metrics.assert_not_called()
+ finally:
+ db.session.rollback()
+ db.session.execute(
+ sa.delete(Dashboard.__table__).where(Dashboard.id ==
dashboard_id)
+ )
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ @with_feature_flags(ENABLE_VIEWERS=True)
+ def test_allowed_viewer_filter_forwards_form_data(self) -> None:
+ """A dashboard-only native filter grant survives the metadata
preflight."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(
+ name="allowed-viewer-metadata-layer", type="test"
+ )
+ view: SemanticView = SemanticView(
+ name="allowed-viewer-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.flush()
+ metadata: str = json.dumps(
+ {
+ "native_filter_configuration": [
+ {
+ "id": "filter-1",
+ "targets": [
+ {
+ "datasetId": view.id,
+ "datasourceType": "semantic_view",
+ }
+ ],
+ }
+ ]
+ }
+ )
+ dashboard_id: int = db.session.execute(
+ sa.insert(Dashboard.__table__).values(
+ dashboard_title="allowed-viewer-metadata-dashboard",
+ published=True,
+ json_metadata=metadata,
+ )
+ ).inserted_primary_key[0]
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ provider.features = frozenset()
+ provider.selection_identity_version = None
+ provider.uid.return_value = "allowed-viewer-metadata-view"
+ access_spy: Mock
+ try:
+ with (
+ patch.dict(current_app.config, {"VIEWER_PROMISCUOUS_MODE":
True}),
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ patch.object(security_manager, "is_viewer", return_value=True),
+ patch.object(
+ QueryContext,
+ "raise_for_access",
+ autospec=True,
+ side_effect=QueryContext.raise_for_access,
+ ) as access_spy,
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "result_type": "query",
+ "form_data": {
+ "dashboardId": dashboard_id,
+ "type": "NATIVE_FILTER",
+ "native_filter_id": "filter-1",
+ },
+ },
+ )
+ assert response.status_code == 200, response.json
+ provider.get_dimensions.assert_called()
+ assert access_spy.call_count == 2
+ finally:
+ db.session.rollback()
+ db.session.execute(
+ sa.delete(Dashboard.__table__).where(Dashboard.id ==
dashboard_id)
+ )
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ @with_feature_flags(EMBEDDED_SUPERSET=True)
+ def test_guest_dashboard_filter_access_is_unchanged(self) -> None:
+ """A guest can still use a dashboard-scoped semantic native filter."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="guest-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="guest-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.flush()
+ metadata: str = json.dumps(
+ {
+ "native_filter_configuration": [
+ {
+ "id": "filter-1",
+ "targets": [
+ {
+ "datasetId": view.id,
+ "datasourceType": "semantic_view",
+ "column": {"name": "category"},
+ }
+ ],
+ }
+ ]
+ }
+ )
+ dashboard_id: int = db.session.execute(
+ sa.insert(Dashboard.__table__).values(
+ dashboard_title="guest-metadata-dashboard",
+ published=True,
+ json_metadata=metadata,
+ )
+ ).inserted_primary_key[0]
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ provider.features = frozenset()
+ provider.selection_identity_version = None
+ provider.uid.return_value = "guest-metadata-view"
+ access_spy: Mock
+ try:
+ with (
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ patch.object(security_manager, "is_guest_user",
return_value=True),
+ patch.object(security_manager, "has_guest_access",
return_value=True),
+ patch.object(
+ QueryContext,
+ "raise_for_access",
+ autospec=True,
+ side_effect=QueryContext.raise_for_access,
+ ) as access_spy,
+ ):
+ g.user.rls = []
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "result_type": "query",
+ "form_data": {
+ "dashboardId": dashboard_id,
+ "type": "NATIVE_FILTER",
+ "native_filter_id": "filter-1",
+ },
+ },
+ )
+ assert response.status_code == 200, response.json
+ provider.get_dimensions.assert_called()
+ assert access_spy.call_count == 1
+ finally:
+ db.session.rollback()
+ db.session.execute(
+ sa.delete(Dashboard.__table__).where(Dashboard.id ==
dashboard_id)
+ )
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ def test_instance_wrapped_access_check_still_sees_queries(self) -> None:
+ """An instance wrapper receives only the completed query context."""
+ self._assert_wrapped_access_check_still_sees_queries("instance")
+
+ def test_class_wrapped_access_check_still_sees_queries(self) -> None:
+ """A reassigned base-class method receives only the completed
context."""
+ self._assert_wrapped_access_check_still_sees_queries("class")
+
+ def _assert_wrapped_access_check_still_sees_queries(
+ self, override_target: Literal["instance", "class"]
+ ) -> None:
+ """Instance and base-class wrappers receive only the completed
context."""
+ self.login("gamma")
+ layer: SemanticLayer = SemanticLayer(name="wrapped-metadata-layer",
type="test")
+ view: SemanticView = SemanticView(
+ name="wrapped-metadata-view", semantic_layer=layer
+ )
+ db.session.add(view)
+ db.session.commit()
+ provider: Mock = Mock()
+ provider.get_dimensions.return_value = set()
+ provider.get_metrics.return_value = set()
+ manager: SupersetSecurityManager =
security_manager._get_current_object() # noqa: SLF001
+ original_check: Callable[..., None] = manager.raise_for_access
+ original_can_access: Callable[[str, str], bool] = manager.can_access
+ query_counts: list[int] = []
+
+ def wrapped_access(**kwargs: Any) -> None:
+ """Model a configured wrapper that inspects the first requested
query."""
+ context: QueryContext | None = kwargs.get("query_context")
+ if context is not None:
+ query_counts.append(len(context.queries))
+ assert context.queries[0] is not None
+ original_check(**kwargs)
+
+ def class_wrapped_access(
+ manager_self: SupersetSecurityManager, **kwargs: Any
+ ) -> None:
+ """Bind the configured base-class wrapper like an ordinary
method."""
+ assert manager_self is manager
+ wrapped_access(**kwargs)
+
+ def can_access(permission_name: str, view_name: str) -> bool:
+ """Grant Gamma this semantic datasource for the request."""
+ if permission_name == "datasource_access" and view_name ==
view.perm:
+ return True
+ return original_can_access(permission_name, view_name)
+
+ target: SupersetSecurityManager | type[SupersetSecurityManager] = (
+ manager if override_target == "instance" else
SupersetSecurityManager
+ )
+ replacement: Callable[..., None] = (
+ wrapped_access if override_target == "instance" else
class_wrapped_access
+ )
+ try:
+ with (
+ patch.object(target, "raise_for_access", replacement),
+ patch.object(manager, "can_access", side_effect=can_access),
+ patch.object(
+ SemanticView,
+ "implementation",
+ new_callable=PropertyMock,
+ return_value=provider,
+ ),
+ ):
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": view.id, "type": "semantic_view"},
+ "queries": [{"columns": [], "metrics": []}],
+ "result_type": "query",
+ },
+ )
+ assert response.status_code == 200, response.json
+ assert query_counts == [1]
+ finally:
+ db.session.rollback()
+ db.session.delete(view)
+ db.session.delete(layer)
+ db.session.commit()
+
+ @pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
+ def test_sql_dataset_chart_data_is_unchanged(self) -> None:
+ """The semantic preflight does not affect SQL chart-data requests."""
+ self.login("admin")
+ dataset: SqlaTable | None = (
+
db.session.query(SqlaTable).filter_by(table_name="birth_names").first()
+ )
+ assert dataset is not None
+ access_spy: Mock
+ with patch.object(
+ QueryContext,
+ "raise_for_access",
+ autospec=True,
+ side_effect=QueryContext.raise_for_access,
+ ) as access_spy:
+ response: Response = self.client.post(
+ "/api/v1/chart/data",
+ json={
+ "datasource": {"id": dataset.id, "type": "table"},
+ "queries": [{"columns": ["name"], "metrics": []}],
+ "result_type": "query",
+ },
+ )
+ assert response.status_code == 200, response.json
+ # Admin passes either way, so pin that a SQL dataset gets no preflight.
+ assert access_spy.call_count == 1
diff --git a/tests/unit_tests/security/manager_test.py
b/tests/unit_tests/security/manager_test.py
index b5cf80eb966..6101dc4a685 100644
--- a/tests/unit_tests/security/manager_test.py
+++ b/tests/unit_tests/security/manager_test.py
@@ -41,6 +41,7 @@ from werkzeug.exceptions import NotFound
from superset.common.chart_data import ChartDataResultType
from superset.common.query_object import QueryObject
from superset.connectors.sqla.models import Database, SqlaTable
+from superset.errors import SupersetErrorType
from superset.exceptions import SupersetSecurityException
from superset.extensions import appbuilder
from superset.models.slice import Slice
@@ -5627,6 +5628,286 @@ def
test_reset_password_self_service_commits_cleared_flag(
assert mock_commit.call_count == 2
[email protected]("granted", [True, False])
+def test_raise_for_access_never_reads_queries_for_non_guest(
+ mocker: MockerFixture,
+ app_context: None,
+ granted: bool,
+) -> None:
+ """
+ Pin the invariant the chart-data semantic preflight relies on: for a
+ non-guest caller, ``raise_for_access`` decides from the datasource and form
+ data alone and never reads ``query_context.queries``.
+ """
+ sm: SupersetSecurityManager = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "is_guest_user", return_value=False)
+ mocker.patch.object(sm, "can_access", return_value=granted)
+ mocker.patch.object(sm, "can_access_schema", return_value=False)
+ mocker.patch.object(sm, "_semantic_layer_grant_allows", return_value=False)
+ mocker.patch.object(sm, "is_editor", return_value=False)
+
+ query_context: MagicMock = mocker.MagicMock()
+ # A consistent saved chart, so a payload comparison would reach the queries
+ # rather than stopping at a slice mismatch.
+ query_context.slice_.id = 42
+ query_context.slice_.query_context = None
+ query_context.slice_.params_dict = {}
+ query_context.form_data = {"slice_id": 42}
+ type(query_context).queries = mocker.PropertyMock(
+ side_effect=AssertionError("raise_for_access read
query_context.queries")
+ )
+
+ if granted:
+ sm.raise_for_access(query_context=query_context)
+ else:
+ with pytest.raises(SupersetSecurityException):
+ sm.raise_for_access(query_context=query_context)
+
+
+_DASHBOARD_ID: int = 7
+_VIEW_ID: int = 1
+_OTHER_VIEW_ID: int = 2
+
+
+def _chart(mocker: MockerFixture, chart_id: int, datasource_id: int) ->
MagicMock:
+ """A saved chart on the given semantic view."""
+ chart: MagicMock = mocker.MagicMock(
+ id=chart_id, datasource_id=datasource_id,
datasource_type="semantic_view"
+ )
+ return chart
+
+
[email protected](
+ "path",
+ ["native-filter", "chart", "multilayer-child", "drill-to-detail",
"drill-by"],
+)
+def test_raise_for_access_dashboard_paths_never_read_queries_for_non_guest(
+ mocker: MockerFixture,
+ app_context: None,
+ path: str,
+) -> None:
+ """
+ Extend the non-guest invariant to the dashboard, viewer and drill branches.
+
+ Each case passes every earlier check on its branch and is refused only by
+ that branch's last check, which a spy proves was reached, so a read of
+ ``query_context.queries`` anywhere along the branch would fail the test.
+ """
+ from superset.models.dashboard import Dashboard
+ from superset.security import manager as manager_module
+
+ sm: SupersetSecurityManager = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "is_guest_user", return_value=False)
+ mocker.patch.object(sm, "can_access", return_value=False)
+ mocker.patch.object(sm, "can_access_schema", return_value=False)
+ mocker.patch.object(sm, "_semantic_layer_grant_allows", return_value=False)
+ mocker.patch.object(sm, "is_editor", return_value=False)
+ mocker.patch.object(sm, "is_viewer", return_value=True)
+ mocker.patch.object(sm, "can_access_dashboard", return_value=True)
+ mocker.patch.object(sm, "_validate_child_in_parent_multilayer",
return_value=True)
+ mocker.patch("superset.is_feature_enabled", return_value=True)
+ mocker.patch.dict(current_app.config, {"VIEWER_PROMISCUOUS_MODE": True})
+ matches: MagicMock = mocker.patch.object(
+ manager_module,
+ "_datasource_matches",
+ wraps=manager_module._datasource_matches,
+ )
+
+ datasource: MagicMock = mocker.MagicMock(
+ id=_VIEW_ID, type="semantic_view", perm="[view](id:1)"
+ )
+ datasource.has_drill_by_columns.return_value = False
+ own_chart: MagicMock = _chart(mocker, 42, _VIEW_ID)
+ foreign_chart: MagicMock = _chart(mocker, 42, _OTHER_VIEW_ID)
+ parent_chart: MagicMock = _chart(mocker, 43, _OTHER_VIEW_ID)
+ native_filters: list[dict[str, Any]] = [
+ {
+ "id": "f",
+ "targets": [
+ {"datasetId": _OTHER_VIEW_ID, "datasourceType":
"semantic_view"}
+ ],
+ }
+ ]
+
+ form_data: dict[str, Any]
+ charts: dict[int, MagicMock]
+ if path == "native-filter":
+ form_data = {
+ "dashboardId": _DASHBOARD_ID,
+ "type": "NATIVE_FILTER",
+ "native_filter_id": "f",
+ }
+ charts = {}
+ elif path == "chart":
+ form_data = {"dashboardId": _DASHBOARD_ID, "slice_id": 42}
+ charts = {42: foreign_chart}
+ elif path == "multilayer-child":
+ form_data = {
+ "dashboardId": _DASHBOARD_ID,
+ "slice_id": 42,
+ "parent_slice_id": 43,
+ }
+ charts = {42: foreign_chart, 43: parent_chart}
+ elif path == "drill-to-detail":
+ form_data = {"dashboardId": _DASHBOARD_ID}
+ charts = {}
+ else:
+ form_data = {
+ "dashboardId": _DASHBOARD_ID,
+ "slice_id": 0,
+ "chart_id": 42,
+ "groupby": ["a"],
+ }
+ charts = {42: own_chart}
+
+ dashboard: MagicMock = mocker.MagicMock(
+ id=_DASHBOARD_ID,
+ json_metadata=json.dumps({"native_filter_configuration":
native_filters}),
+ slices=list(charts.values()),
+ )
+ dashboard.has_member_datasource.return_value = False
+
+ def query(model: Any) -> MagicMock:
+ """Resolve ``Model.id == <value>`` lookups against the fixtures."""
+ rows: dict[int, MagicMock] = (
+ {_DASHBOARD_ID: dashboard} if model is Dashboard else charts
+ )
+ chain: MagicMock = mocker.MagicMock()
+ chain.filter.side_effect = lambda clause: mocker.MagicMock(
+ one_or_none=lambda: rows.get(clause.right.value)
+ )
+ return chain
+
+ mocker.patch.object(
+ SupersetSecurityManager,
+ "session",
+ new_callable=mocker.PropertyMock,
+ return_value=mocker.MagicMock(query=query),
+ )
+
+ query_context: MagicMock = mocker.MagicMock()
+ query_context.datasource = datasource
+ query_context.form_data = form_data
+ type(query_context).queries = mocker.PropertyMock(
+ side_effect=AssertionError("raise_for_access read
query_context.queries")
+ )
+
+ excinfo: pytest.ExceptionInfo[SupersetSecurityException]
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ sm.raise_for_access(query_context=query_context)
+
+ assert (
+ excinfo.value.error.error_type
+ == SupersetErrorType.DATASOURCE_SECURITY_ACCESS_ERROR
+ )
+ # Prove each case reached its branch's final check.
+ if path == "drill-by":
+ datasource.has_drill_by_columns.assert_called_once_with(["a"])
+ elif path == "drill-to-detail":
+ dashboard.has_member_datasource.assert_called_once_with(datasource)
+ else:
+ matches.assert_any_call(datasource, _OTHER_VIEW_ID, "semantic_view")
+
+
[email protected]("dashboard_granted", [True, False])
[email protected]("path", ["drill-to-detail", "drill-by"])
+def
test_raise_for_access_dashboard_final_check_never_reads_queries_for_non_guest(
+ mocker: MockerFixture,
+ app_context: None,
+ path: str,
+ dashboard_granted: bool,
+) -> None:
+ """
+ Carry the non-guest invariant through a successful drill to the shared
final
+ ``can_access_dashboard`` check.
+
+ The drill's membership or dimension check passes, so the final dashboard
+ check alone decides the outcome; a spy proves it was reached, and a read of
+ ``query_context.queries`` on the way there or around it fails the test.
+ """
+ from superset.models.dashboard import Dashboard
+
+ sm: SupersetSecurityManager = SupersetSecurityManager(appbuilder)
+ mocker.patch.object(sm, "is_guest_user", return_value=False)
+ mocker.patch.object(sm, "can_access", return_value=False)
+ mocker.patch.object(sm, "can_access_schema", return_value=False)
+ mocker.patch.object(sm, "_semantic_layer_grant_allows", return_value=False)
+ mocker.patch.object(sm, "is_editor", return_value=False)
+ mocker.patch.object(sm, "is_viewer", return_value=True)
+ final_check: MagicMock = mocker.patch.object(
+ sm, "can_access_dashboard", return_value=dashboard_granted
+ )
+ mocker.patch("superset.is_feature_enabled", return_value=True)
+ mocker.patch.dict(current_app.config, {"VIEWER_PROMISCUOUS_MODE": True})
+
+ datasource: MagicMock = mocker.MagicMock(
+ id=_VIEW_ID, type="semantic_view", perm="[view](id:1)"
+ )
+ datasource.has_drill_by_columns.return_value = True
+
+ form_data: dict[str, Any]
+ charts: dict[int, MagicMock]
+ if path == "drill-to-detail":
+ form_data = {"dashboardId": _DASHBOARD_ID}
+ charts = {}
+ else:
+ form_data = {
+ "dashboardId": _DASHBOARD_ID,
+ "slice_id": 0,
+ "chart_id": 42,
+ "groupby": ["a"],
+ }
+ charts = {42: _chart(mocker, 42, _VIEW_ID)}
+
+ dashboard: MagicMock = mocker.MagicMock(
+ id=_DASHBOARD_ID, json_metadata=None, slices=list(charts.values())
+ )
+ dashboard.has_member_datasource.return_value = True
+
+ def query(model: Any) -> MagicMock:
+ """Resolve ``Model.id == <value>`` lookups against the fixtures."""
+ rows: dict[int, MagicMock] = (
+ {_DASHBOARD_ID: dashboard} if model is Dashboard else charts
+ )
+ chain: MagicMock = mocker.MagicMock()
+ chain.filter.side_effect = lambda clause: mocker.MagicMock(
+ one_or_none=lambda: rows.get(clause.right.value)
+ )
+ return chain
+
+ mocker.patch.object(
+ SupersetSecurityManager,
+ "session",
+ new_callable=mocker.PropertyMock,
+ return_value=mocker.MagicMock(query=query),
+ )
+
+ query_context: MagicMock = mocker.MagicMock()
+ query_context.datasource = datasource
+ query_context.form_data = form_data
+ type(query_context).queries = mocker.PropertyMock(
+ side_effect=AssertionError("raise_for_access read
query_context.queries")
+ )
+
+ if dashboard_granted:
+ sm.raise_for_access(query_context=query_context)
+ else:
+ excinfo: pytest.ExceptionInfo[SupersetSecurityException]
+ with pytest.raises(SupersetSecurityException) as excinfo:
+ sm.raise_for_access(query_context=query_context)
+ assert (
+ excinfo.value.error.error_type
+ == SupersetErrorType.DATASOURCE_SECURITY_ACCESS_ERROR
+ )
+
+ # The drill succeeded, so the final dashboard check made the decision.
+ if path == "drill-by":
+ datasource.has_drill_by_columns.assert_called_once_with(["a"])
+ else:
+ dashboard.has_member_datasource.assert_called_once_with(datasource)
+ final_check.assert_called_once_with(dashboard)
+
+
def test_dataset_delete_uses_stored_permission_identity(
mocker: MockerFixture, app_context: None
) -> None:
