You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by mi...@apache.org on 2026/10/08 07:18:15 UTC
(superset) branch master updated: perf(security): batch dashboard fallback datasource resolution (#44993)
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 100f51240bd perf(security): batch dashboard fallback datasource resolution (#44993)
100f51240bd is described below
commit 100f51240bda67dc4536ce101c38a3ff0fcbf309
Author: Mike Bridge <mi...@preset.io>
AuthorDate: Thu Oct 8 01:18:07 2026 -0600
perf(security): batch dashboard fallback datasource resolution (#44993)
Co-authored-by: Mike Bridge <mi...@ext.preset.io>
---
superset/daos/datasource.py | 21 ++-
superset/models/slice.py | 52 ++++++
superset/security/manager.py | 9 +-
.../dashboards/semantic_view_access_test.py | 186 +++++++++++++++++++++
4 files changed, 260 insertions(+), 8 deletions(-)
diff --git a/superset/daos/datasource.py b/superset/daos/datasource.py
index eb3832e8979..7fda06813b3 100644
--- a/superset/daos/datasource.py
+++ b/superset/daos/datasource.py
@@ -20,7 +20,7 @@ import uuid
from typing import Any, cast, Union
from sqlalchemy import and_, func, literal, or_, select
-from sqlalchemy.orm import joinedload
+from sqlalchemy.orm import joinedload, Query as ORMQuery
from sqlalchemy.sql import Select
from superset import db, security_manager
@@ -91,6 +91,25 @@ class DatasourceDAO(BaseDAO[Datasource]):
return datasource
+ @classmethod
+ def get_datasources_by_ids(
+ cls, datasource_type: str, datasource_ids: set[int]
+ ) -> dict[int, Datasource]:
+ """Fetch typed integer references in one query; omit invalid/missing IDs."""
+ if datasource_type not in cls.sources:
+ raise DatasourceTypeNotSupportedError()
+ ids: set[int] = {
+ value for value in datasource_ids if type(value) is int and value > 0
+ }
+ if not ids:
+ return {}
+ model: type[Datasource] = cls.sources[datasource_type]
+ query: ORMQuery[Datasource] = db.session.query(model)
+ if model is Query:
+ # Query.perm and schema checks read the database for every member.
+ query = query.options(joinedload(Query.database))
+ return {item.id: item for item in query.filter(model.id.in_(ids)).all()}
+
@staticmethod
def build_dataset_query(
name_filter: str | None,
diff --git a/superset/models/slice.py b/superset/models/slice.py
index 3dcf10c790e..12b5ffce7f3 100644
--- a/superset/models/slice.py
+++ b/superset/models/slice.py
@@ -17,6 +17,7 @@
from __future__ import annotations
import logging
+from collections.abc import Iterator, Sequence
from typing import Any, TYPE_CHECKING
from urllib import parse
@@ -284,6 +285,57 @@ class Slice( # pylint: disable=too-many-public-methods
# letting the access check crash on it.
return resolved if hasattr(resolved, "perm") else None
+ @staticmethod
+ def iter_resolved_datasources(
+ slices: Sequence[Slice],
+ ) -> Iterator[Datasource | None]:
+ """Resolve unique member references in order, batching each fallback type.
+
+ Relationship-backed references retain their existing resolver. A fallback
+ batch is loaded only when its first member is reached, so callers can
+ short-circuit before any fallback query. All batch state is call-local.
+ """
+ # pylint: disable=import-outside-toplevel
+ from superset.daos.datasource import DatasourceDAO
+ from superset.daos.exceptions import DatasourceTypeNotSupportedError
+
+ seen: set[tuple[str | None, int | None]] = set()
+ batches: dict[str, dict[int, Datasource]] = {}
+ for slc in slices:
+ key: tuple[str | None, int | None] = (
+ slc.datasource_type,
+ slc.datasource_id,
+ )
+ if key in seen:
+ continue
+ seen.add(key)
+ if not slc.datasource_id or slc.datasource_type in (
+ utils.DatasourceType.TABLE,
+ utils.DatasourceType.SEMANTIC_VIEW,
+ ):
+ yield slc.resolved_datasource
+ continue
+ datasource_type: str = slc.datasource_type
+ if datasource_type not in batches:
+ ids: set[int] = {
+ member.datasource_id
+ for member in slices
+ if member.datasource_type == datasource_type
+ and member.datasource_id is not None
+ }
+ try:
+ batches[datasource_type] = DatasourceDAO.get_datasources_by_ids(
+ datasource_type, ids
+ )
+ except DatasourceTypeNotSupportedError:
+ batches[datasource_type] = {}
+ resolved: Datasource | None = batches[datasource_type].get(
+ slc.datasource_id
+ )
+ yield (
+ resolved if resolved is not None and hasattr(resolved, "perm") else None
+ )
+
def clone(self) -> Slice:
return Slice(
slice_name=self.slice_name,
diff --git a/superset/security/manager.py b/superset/security/manager.py
index c20c977e3c3..f7fe9f0063f 100644
--- a/superset/security/manager.py
+++ b/superset/security/manager.py
@@ -5562,13 +5562,8 @@ class SupersetSecurityManager( # pylint: disable=too-many-public-methods
member_slices = dashboard.slices
def member_datasource_accessible() -> bool:
- seen: set[tuple[str | None, int | None]] = set()
- for slc in member_slices:
- key = (slc.datasource_type, slc.datasource_id)
- if key in seen:
- continue
- seen.add(key)
- resolved = slc.resolved_datasource
+ resolved: BaseDatasource | Explorable | None
+ for resolved in Slice.iter_resolved_datasources(member_slices):
if resolved is not None and self.can_access_datasource(
resolved
):
diff --git a/tests/unit_tests/dashboards/semantic_view_access_test.py b/tests/unit_tests/dashboards/semantic_view_access_test.py
index a4a842231ad..7c1a19b032e 100644
--- a/tests/unit_tests/dashboards/semantic_view_access_test.py
+++ b/tests/unit_tests/dashboards/semantic_view_access_test.py
@@ -40,6 +40,8 @@ from typing import cast, Iterator, TYPE_CHECKING
from unittest.mock import MagicMock, patch
import pytest
+from sqlalchemy import event
+from sqlalchemy.engine import Connection, Engine, ExecutionContext
from sqlalchemy.orm.session import Session
if TYPE_CHECKING:
@@ -285,6 +287,190 @@ def test_resolved_datasource_query_resolves(
assert resolved.perm
+@pytest.mark.parametrize("allow_last", [False, True])
+def test_dashboard_batches_query_datasource_resolution(
+ access_fixtures: SimpleNamespace, app_context: None, allow_last: bool
+) -> None:
+ """Distinct query references use one SELECT without changing allow/deny."""
+ from superset.exceptions import SupersetSecurityException
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+ from superset.models.sql_lab import Query
+
+ session: Session = access_fixtures.session
+ queries: list[Query] = [
+ Query(id=idx, client_id=f"batch{idx}", database_id=10, sql="select 1")
+ for idx in (80, 81, 82)
+ ]
+ session.add_all(queries)
+ session.flush()
+ dashboard: Dashboard = Dashboard(
+ dashboard_title="query members",
+ published=True,
+ slices=[
+ Slice(datasource_type="query", datasource_id=idx)
+ for idx in (80, 80, 81, 82)
+ ],
+ )
+ granted_perms: set[str] = {queries[-1].perm} if allow_last else set()
+ statements: list[str] = []
+ engine: Engine = session.get_bind()
+
+ def record_query_select(
+ connection: Connection,
+ cursor: object,
+ statement: str,
+ parameters: object,
+ context: ExecutionContext,
+ executemany: bool,
+ ) -> None:
+ """Count actual datasource SELECTs, not mocked DAO invocations."""
+ if "FROM query " in statement or 'FROM "query" ' in statement:
+ statements.append(statement)
+
+ sm: SupersetSecurityManager = _gate_sm()
+ event.listen(engine, "before_cursor_execute", record_query_select)
+ try:
+ with _gate_patches(sm, granted_perms=granted_perms):
+ if allow_last:
+ sm.raise_for_access(dashboard=dashboard)
+ else:
+ with pytest.raises(SupersetSecurityException):
+ sm.raise_for_access(dashboard=dashboard)
+ finally:
+ event.remove(engine, "before_cursor_execute", record_query_select)
+ assert len(statements) == 1, statements
+
+
+@pytest.mark.parametrize(
+ ("datasource_type", "datasource_id", "expected_access"),
+ [
+ ("table", 1, True),
+ ("semantic_view", 1, True),
+ ("query", 78, True),
+ ("query", 999, False),
+ ("query", -78, False),
+ ("query", True, False),
+ ("saved_query", 77, False),
+ ("unknown", 1, False),
+ (None, 1, False),
+ ("table", None, False),
+ ],
+)
+def test_batched_dashboard_resolution_preserves_access_decisions(
+ access_fixtures: SimpleNamespace,
+ app_context: None,
+ datasource_type: str | None,
+ datasource_id: int | None,
+ expected_access: bool,
+) -> None:
+ """A typed batch preserves the single resolver's fail-closed decisions."""
+ from superset.daos.datasource import Datasource
+ from superset.exceptions import SupersetSecurityException
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+ from superset.models.sql_lab import Query
+
+ session: Session = access_fixtures.session
+ query: Query = session.query(Query).filter(Query.id == 78).one()
+ grants: set[str] = {TABLE_PERM, VIEW_PERM, query.perm}
+ member: Slice
+ if datasource_type == "table" and datasource_id == 1:
+ member = access_fixtures.table_slice
+ elif datasource_type == "semantic_view":
+ member = access_fixtures.semantic_slice
+ else:
+ member = Slice(datasource_type=datasource_type, datasource_id=datasource_id)
+ dashboard: Dashboard = Dashboard(
+ dashboard_title="typed resolution", published=True, slices=[member]
+ )
+ sm: SupersetSecurityManager = _gate_sm()
+ with _gate_patches(sm, granted_perms=grants):
+ original: Datasource | None = member.resolved_datasource
+ assert bool(original is not None and sm.can_access_datasource(original)) == (
+ expected_access
+ )
+ if expected_access:
+ sm.raise_for_access(dashboard=dashboard)
+ else:
+ with pytest.raises(SupersetSecurityException):
+ sm.raise_for_access(dashboard=dashboard)
+
+
+@pytest.mark.parametrize("relationship_type", ["table", "semantic_view"])
+def test_dashboard_relationship_short_circuit_skips_fallback_batch(
+ access_fixtures: SimpleNamespace, app_context: None, relationship_type: str
+) -> None:
+ """An accessible relationship-backed member avoids loading later queries."""
+ from superset.daos.datasource import DatasourceDAO
+ from superset.models.dashboard import Dashboard
+ from superset.models.slice import Slice
+
+ member: Slice = (
+ access_fixtures.table_slice
+ if relationship_type == "table"
+ else access_fixtures.semantic_slice
+ )
+ dashboard: Dashboard = Dashboard(
+ dashboard_title="relationship first",
+ published=True,
+ slices=[member, Slice(datasource_type="query", datasource_id=78)],
+ )
+ sm: SupersetSecurityManager = _gate_sm()
+ with (
+ _gate_patches(sm, granted_perms={TABLE_PERM, VIEW_PERM}),
+ patch.object(DatasourceDAO, "get_datasources_by_ids") as batch,
+ ):
+ sm.raise_for_access(dashboard=dashboard)
+ batch.assert_not_called()
+
+
+def test_batched_resolver_keeps_typed_order_and_relationship_identity(
+ access_fixtures: SimpleNamespace, app_context: None
+) -> None:
+ """Batches neither reorder members nor conflate colliding table/view IDs."""
+ from superset.daos.datasource import Datasource, DatasourceDAO
+ from superset.models.slice import Slice
+ from superset.models.sql_lab import Query
+
+ session: Session = access_fixtures.session
+ query: Query = session.query(Query).filter(Query.id == 78).one()
+ members: list[Slice] = [
+ access_fixtures.semantic_slice,
+ Slice(datasource_type="query", datasource_id=78),
+ access_fixtures.table_slice,
+ Slice(datasource_type="query", datasource_id=78),
+ Slice(datasource_type="saved_query", datasource_id=77),
+ Slice(datasource_type="query", datasource_id=999),
+ ]
+ with patch.object(
+ DatasourceDAO,
+ "get_datasources_by_ids",
+ wraps=DatasourceDAO.get_datasources_by_ids,
+ ) as batch:
+ resolved: list[Datasource | None] = list(
+ Slice.iter_resolved_datasources(members)
+ )
+ assert resolved == [access_fixtures.view, query, access_fixtures.table, None, None]
+ assert batch.call_count == 2
+ batch.assert_any_call("query", {78, 999})
+ batch.assert_any_call("saved_query", {77})
+
+
+def test_batched_resolver_does_not_cache_across_calls(
+ access_fixtures: SimpleNamespace, app_context: None
+) -> None:
+ """A later resolution sees deletion rather than a prior batch's Query."""
+ from superset.models.slice import Slice
+ from superset.models.sql_lab import Query
+
+ session: Session = access_fixtures.session
+ member: Slice = Slice(datasource_type="query", datasource_id=78)
+ assert next(Slice.iter_resolved_datasources([member])) is not None
+ session.query(Query).filter(Query.id == 78).delete(synchronize_session=False)
+ assert list(Slice.iter_resolved_datasources([member])) == [None]
+
+
# ---------------------------------------------------------------------------
# Object gate: security_manager.raise_for_access(dashboard=..., chart=...)
# ---------------------------------------------------------------------------