This is an automated email from the ASF dual-hosted git repository. EnxDev pushed a commit to branch enxdev/fix/set-related-perm-catalog-perm-attributeerror in repository https://gitbox.apache.org/repos/asf/superset.git
commit 41e4745c92e1cd943762b452272650b4e1ef4c87 Author: Enzo Martellucci <[email protected]> AuthorDate: Tue Sep 8 09:29:46 2026 +0200 fix(charts): don't fail chart save when datasource lacks a perm string Creating a chart from a SQL Lab query returned a 500: AttributeError: 'Query' object has no attribute 'catalog_perm' ``set_related_perm`` denormalizes perm / catalog_perm / schema_perm from the datasource onto the chart, but not every datasource model defines all three. ``catalog_perm`` is specific to SqlaTable and SemanticView, so reading it off a SQL Lab ``Query`` raised. The unconditional assignment arrived in #29840; before it, query-backed charts simply left the column at its null default. The same function had two adjacent failures on datasource types the chart API accepts but the listener never tolerated: ``saved_query`` raised on ``perm`` (one line earlier), and ``dataset`` / ``view`` raised KeyError because they have no entry in ``DatasourceDAO.sources``. Read each perm string with a default of None and skip unmapped types with a warning. Null is fail-closed rather than permissive: the access filter ORs ``<perm>.in_(...)``, which never matches NULL, so a chart cannot become visible through a perm string this leaves unset. Deriving a real catalog_perm for ``Query`` was considered and rejected -- it would widen access, and ``Query.schema_perm`` already returns a legacy ``db.schema`` string that does not match a real view-menu name, so the format question is separate. Adds the first test coverage for this listener, over all six datasource types. Co-Authored-By: Claude Opus 5 (1M context) <[email protected]> --- superset/models/slice.py | 24 ++++++++-- tests/unit_tests/models/slice_test.py | 85 ++++++++++++++++++++++++++++++++++- 2 files changed, 104 insertions(+), 5 deletions(-) diff --git a/superset/models/slice.py b/superset/models/slice.py index 65f4ab44edc..a50bdef14a4 100644 --- a/superset/models/slice.py +++ b/superset/models/slice.py @@ -496,13 +496,29 @@ def set_related_perm(_mapper: Mapper, _connection: Connection, target: Slice) -> # pylint: disable=import-outside-toplevel from superset.daos.datasource import DatasourceDAO - src_class = DatasourceDAO.sources[target.datasource_type] + src_class = DatasourceDAO.sources.get(target.datasource_type) + if src_class is None: + # The chart API accepts every ``DatasourceType``, but only some of them + # map to a model here. Leave the perm strings null rather than failing + # the save; there is no datasource to derive them from. + logger.warning( + "Chart datasource type %s has no datasource model; " + "leaving permission strings unset", + target.datasource_type, + ) + return + if id_ := target.datasource_id: ds = db.session.query(src_class).filter_by(id=int(id_)).first() if ds: - target.perm = ds.perm - target.catalog_perm = ds.catalog_perm - target.schema_perm = ds.schema_perm + # Not every datasource model defines all three perm strings -- + # ``catalog_perm`` is specific to tables and semantic views, and a + # ``SavedQuery`` defines none of them. A missing one stays null, + # which is fail-closed: the access filter ORs ``<perm>.in_(...)``, + # and NULL never matches. + target.perm = getattr(ds, "perm", None) + target.catalog_perm = getattr(ds, "catalog_perm", None) + target.schema_perm = getattr(ds, "schema_perm", None) def event_after_chart_changed( diff --git a/tests/unit_tests/models/slice_test.py b/tests/unit_tests/models/slice_test.py index a92e14e51bd..7e76000552d 100644 --- a/tests/unit_tests/models/slice_test.py +++ b/tests/unit_tests/models/slice_test.py @@ -22,7 +22,7 @@ import pytest from flask import current_app from parameterized import parameterized -from superset.models.slice import id_or_uuid_filter, Slice +from superset.models.slice import id_or_uuid_filter, set_related_perm, Slice class TestSlice: @@ -229,6 +229,89 @@ class TestSlice: assert '"onmouseover' not in html +def _run_set_related_perm(datasource_type: str) -> Slice: + """Run the perm-denormalizing listener against a stand-in datasource. + + The stand-in is specced against the model class registered for + ``datasource_type``, so it exposes exactly the perm attributes that class + really defines -- the whole point being that they differ per type. + """ + # pylint: disable=import-outside-toplevel + from superset.daos.datasource import DatasourceDAO + + target = Slice() + target.datasource_type = datasource_type + target.datasource_id = 1 + + src_class = DatasourceDAO.sources.get(datasource_type) + datasource = MagicMock(spec=src_class) if src_class else None + + with patch("superset.models.slice.db") as mock_db: + query = mock_db.session.query.return_value.filter_by.return_value + query.first.return_value = datasource + set_related_perm(None, None, target) + + return target + + [email protected]("datasource_type", ["table", "semantic_view"]) +def test_set_related_perm_denormalizes_all_perms( + app_context: None, datasource_type: str +) -> None: + """Types whose model defines all three perm strings get all three copied.""" + target = _run_set_related_perm(datasource_type) + + assert target.perm is not None + assert target.catalog_perm is not None + assert target.schema_perm is not None + + +def test_set_related_perm_tolerates_query_without_catalog_perm( + app_context: None, +) -> None: + """A chart on a SQL Lab query saves even though ``Query`` has no catalog_perm. + + ``catalog_perm`` is defined on ``SqlaTable``/``SemanticView`` only, so + copying it unconditionally raised AttributeError and turned chart creation + from SQL Lab into a 500. Leaving it null matches the behavior before the + assignment was introduced, and is fail-closed: the access filter ORs + ``catalog_perm.in_(...)``, which is never true for NULL. + """ + target = _run_set_related_perm("query") + + assert target.perm is not None + assert target.schema_perm is not None + assert target.catalog_perm is None + + +def test_set_related_perm_tolerates_datasource_without_any_perms( + app_context: None, +) -> None: + """``SavedQuery`` defines none of the three, and must not raise either.""" + target = _run_set_related_perm("saved_query") + + assert target.perm is None + assert target.catalog_perm is None + assert target.schema_perm is None + + [email protected]("datasource_type", ["dataset", "view"]) +def test_set_related_perm_tolerates_unmapped_datasource_type( + app_context: None, datasource_type: str +) -> None: + """The chart API accepts every ``DatasourceType``, but only some are mapped. + + ``dataset`` and ``view`` have no entry in ``DatasourceDAO.sources``, so + indexing it raised KeyError. There is no datasource to read perms from, so + the listener leaves them null rather than failing the save. + """ + target = _run_set_related_perm(datasource_type) + + assert target.perm is None + assert target.catalog_perm is None + assert target.schema_perm is None + + def test_thumbnail_url_is_router_relative_at_root(app_context: None) -> None: """thumbnail_url uses url_for, so at root it keeps the legacy shape.""" slc = Slice()
