You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ri...@apache.org on 2026/09/15 22:13:52 UTC
(superset) branch master updated: fix(api): enforce is_managed_externally on chart/dashboard/dataset update (#44025)
This is an automated email from the ASF dual-hosted git repository.
richardfogaca 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 6cfb3319744 fix(api): enforce is_managed_externally on chart/dashboard/dataset update (#44025)
6cfb3319744 is described below
commit 6cfb33197444cbf74590c2803b6dcc06180439f4
Author: Mike Bridge <mi...@preset.io>
AuthorDate: Tue Sep 15 16:13:45 2026 -0600
fix(api): enforce is_managed_externally on chart/dashboard/dataset update (#44025)
Co-authored-by: Claude Fable 5 <no...@anthropic.com>
Co-authored-by: Mike Bridge <mi...@ext.preset.io>
---
UPDATING.md | 4 +
superset/charts/schemas.py | 3 +-
superset/commands/chart/update.py | 11 +
superset/commands/dashboard/update.py | 68 +++-
superset/commands/dataset/refresh.py | 6 +
superset/commands/dataset/update.py | 4 +-
superset/commands/utils.py | 20 +
superset/dashboards/schemas.py | 8 +-
superset/datasets/schemas.py | 4 +-
superset/utils/schema.py | 26 +-
superset/views/core.py | 12 +
tests/integration_tests/charts/api_tests.py | 28 ++
tests/integration_tests/core_tests.py | 50 +++
tests/unit_tests/commands/chart/update_test.py | 47 ++-
tests/unit_tests/commands/dataset/refresh_test.py | 10 +-
tests/unit_tests/commands/dataset/update_test.py | 24 +-
.../commands/test_update_managed_externally.py | 403 +++++++++++++++++++++
17 files changed, 693 insertions(+), 35 deletions(-)
diff --git a/UPDATING.md b/UPDATING.md
index 7d6623dc950..78bf84c7113 100644
--- a/UPDATING.md
+++ b/UPDATING.md
@@ -111,6 +111,10 @@ behavior. Existing tag rows are left untouched.
Version-history and activity endpoints (`GET /api/v1/{chart,dashboard,dataset}/<uuid>/versions/…` and `…/activity/`) are now edit-gated: they require object-level editorship (owner/editor/admin) of the entity, matching the UI's edit-gated Version history menu and the restore endpoint's gate. Read-only users who could previously retrieve the full change log (author identities, field-level before/after diffs) via the API now receive 403. Embedded guest-token principals are always refused o [...]
+### Updates of externally managed entities are refused server-side
+
+`PUT /api/v1/{chart,dashboard,dataset}/<id>` — including the chart query-context-only save, `PUT /api/v1/dataset/<pk>/refresh`, and the legacy Explore chart overwrite (`/superset/explore/`, `action=overwrite`) — now refuses an **externally managed** entity (`is_managed_externally = True`) with HTTP 403, enforcing server-side what the UI already does by hiding the edit affordances. Previously the refusal existed only in the browser, so an otherwise-authorized editor could mutate such an e [...]
+
### Global Async Queries re-platformed onto the Global Task Framework (breaking)
Global Async Queries (GAQ) no longer runs on its own bespoke async-events
diff --git a/superset/charts/schemas.py b/superset/charts/schemas.py
index dc97a05f1cf..df9682a3d0c 100644
--- a/superset/charts/schemas.py
+++ b/superset/charts/schemas.py
@@ -351,7 +351,7 @@ class ChartPostSchema(Schema):
uuid = fields.UUID(allow_none=True)
-class ChartPutSchema(Schema):
+class ChartPutSchema(utils.DiscardIsManagedExternallyMixin, Schema):
"""
Schema to update or patch a chart
"""
@@ -407,7 +407,6 @@ class ChartPutSchema(Schema):
certification_details = fields.String(
metadata={"description": certification_details_description}, allow_none=True
)
- is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
external_url = fields.String(allow_none=True, validate=utils.validate_external_url)
tags = fields.List(fields.Integer(metadata={"description": tags_description}))
uuid = fields.UUID(allow_none=True)
diff --git a/superset/commands/chart/update.py b/superset/commands/chart/update.py
index 069c9829a41..804763be5db 100644
--- a/superset/commands/chart/update.py
+++ b/superset/commands/chart/update.py
@@ -39,6 +39,7 @@ from superset.commands.exceptions import DatasourceTypeInvalidError
from superset.commands.utils import (
compute_subjects,
get_datasource_by_id,
+ raise_if_managed_externally,
update_tags,
validate_tags,
)
@@ -171,11 +172,21 @@ class UpdateChartCommand(UpdateMixin, BaseCommand):
raise ChartForbiddenError() from ex
except ValidationError as ex:
exceptions.append(ex)
+ raise_if_managed_externally(self._model, ChartForbiddenError)
else:
try:
security_manager.raise_for_access(chart=self._model)
except SupersetSecurityException as ex:
raise ChartForbiddenError() from ex
+ # The relaxed-editorship branch refuses externally managed
+ # charts too: the stored query context is executable state
+ # (report execution loads and runs it), so accepting a
+ # client-supplied context here would let an editor change a
+ # managed chart's behavior -- the bypass this gate closes.
+ # Explore fires this save in the background when the chart is
+ # opened; for a managed chart that background call gets a 403
+ # it ignores.
+ raise_if_managed_externally(self._model, ChartForbiddenError)
# Keep the refreshed payload bound to the chart's own datasource so it
# cannot be repointed at an unrelated one.
self._validate_query_context_datasource(exceptions)
diff --git a/superset/commands/dashboard/update.py b/superset/commands/dashboard/update.py
index 16454929ca2..63f0d58915f 100644
--- a/superset/commands/dashboard/update.py
+++ b/superset/commands/dashboard/update.py
@@ -38,6 +38,7 @@ from superset.commands.dashboard.exceptions import (
)
from superset.commands.utils import (
compute_subjects,
+ raise_if_managed_externally,
update_tags,
validate_tags,
)
@@ -56,6 +57,20 @@ logger = logging.getLogger(__name__)
class UpdateDashboardCommand(UpdateMixin, BaseCommand):
+ #: Ordinary edits of an externally managed dashboard are refused
+ #: server-side (see ``raise_if_managed_externally``).
+ #: ``UpdateDashboardColorsConfigCommand`` flips this off so background
+ #: colors sync keeps working while a dashboard is merely viewed -- but
+ #: only for derived color values; its validate() override refuses
+ #: changes to the authoritative inputs.
+ _refuses_externally_managed: bool = True
+
+ #: Superset-local fields not owned by the external source of truth: the
+ #: publish toggle is local visibility state (which authorized viewers
+ #: see the dashboard), not dashboard content, so an update touching
+ #: ONLY these fields passes the managed-externally gate.
+ _MANAGED_LOCAL_ONLY_FIELDS: frozenset[str] = frozenset({"published"})
+
def __init__(self, model_id: int, data: dict[str, Any]):
self._model_id = model_id
self._properties = data.copy()
@@ -119,6 +134,12 @@ class UpdateDashboardCommand(UpdateMixin, BaseCommand):
except SupersetSecurityException as ex:
raise DashboardForbiddenError() from ex
+ if self._refuses_externally_managed and not (
+ self._properties
+ and set(self._properties) <= self._MANAGED_LOCAL_ONLY_FIELDS
+ ):
+ raise_if_managed_externally(self._model, DashboardForbiddenError)
+
# Validate slug uniqueness
if not DashboardDAO.validate_update_slug_uniqueness(self._model_id, slug):
exceptions.append(DashboardSlugExistsValidationError())
@@ -283,17 +304,62 @@ class UpdateDashboardChartCustomizationsCommand(UpdateDashboardCommand):
class UpdateDashboardColorsConfigCommand(UpdateDashboardCommand):
+ # The blanket gate is skipped so background colors sync (fired while a
+ # dashboard is merely viewed) keeps working for externally managed
+ # dashboards -- but only for the DERIVED color values. The authoritative
+ # inputs are the dashboard's real content, owned by the external source
+ # of truth; validate() refuses a payload that would change them.
+ _refuses_externally_managed = False
+
+ #: json_metadata keys a colors-config save may NOT change on an
+ #: externally managed dashboard. The other accepted keys
+ #: (color_scheme_domain, shared_label_colors, map_label_colors) are
+ #: derived from these plus chart state (see
+ #: DashboardDAO.update_colors_config).
+ _AUTHORITATIVE_COLOR_KEYS: tuple[str, ...] = ("color_scheme", "label_colors")
+
def __init__(
self, model_id: int, data: dict[str, Any], mark_updated: bool = True
) -> None:
super().__init__(model_id, data)
self._mark_updated = mark_updated
+ def validate(self) -> None:
+ super().validate()
+ assert self._model
+ if self._model.is_managed_externally and self._changes_authoritative_colors():
+ raise DashboardForbiddenError()
+
+ def _changes_authoritative_colors(self) -> bool:
+ """Whether the payload changes the EFFECTIVE authoritative colors.
+
+ Compared by effective state, not raw metadata bytes: an absent
+ key, an explicit null, and an empty value ("" / {}) all encode
+ the same authoritative color state — none. The background colors
+ sync always sends ``label_colors`` (as ``{}`` when nothing is
+ set) while a dashboard is merely VIEWED, so refusing
+ empty-vs-absent would 403 every view of a managed dashboard
+ whose metadata lacks the key. The DAO may still write the empty
+ key into stored metadata — that changes export bytes, not color
+ state, and the next external sync owns the bytes anyway.
+ """
+ assert self._model
+ metadata = json.loads(self._model.json_metadata or "{}")
+
+ def effective(value: Any) -> Any:
+ return None if value in (None, "", {}) else value
+
+ return any(
+ key in self._properties
+ and effective(self._properties[key]) != effective(metadata.get(key))
+ for key in self._AUTHORITATIVE_COLOR_KEYS
+ )
+
@transaction(
on_error=partial(on_error, reraise=DashboardColorsConfigUpdateFailedError)
)
def run(self) -> Model:
- super().validate()
+ self.validate()
assert self._model
original_changed_on = self._model.changed_on
diff --git a/superset/commands/dataset/refresh.py b/superset/commands/dataset/refresh.py
index 01c089205f3..fffc7de4ff2 100644
--- a/superset/commands/dataset/refresh.py
+++ b/superset/commands/dataset/refresh.py
@@ -28,6 +28,7 @@ from superset.commands.dataset.exceptions import (
DatasetNotFoundError,
DatasetRefreshFailedError,
)
+from superset.commands.utils import raise_if_managed_externally
from superset.connectors.sqla.models import SqlaTable
from superset.daos.dataset import DatasetDAO
from superset.datasets.datetime_format_detector import DatetimeFormatDetector
@@ -93,3 +94,8 @@ class RefreshDatasetCommand(BaseCommand):
security_manager.raise_for_editorship(self._model)
except SupersetSecurityException as ex:
raise DatasetForbiddenError() from ex
+
+ # Refresh persists fetched column metadata onto the dataset; an
+ # externally managed dataset's columns are owned by the external
+ # sync, which would overwrite (or fight) the refresh.
+ raise_if_managed_externally(self._model, DatasetForbiddenError)
diff --git a/superset/commands/dataset/update.py b/superset/commands/dataset/update.py
index b581fa369e0..2352408f202 100644
--- a/superset/commands/dataset/update.py
+++ b/superset/commands/dataset/update.py
@@ -44,7 +44,7 @@ from superset.commands.dataset.exceptions import (
DatasetUpdateFailedError,
MultiCatalogDisabledValidationError,
)
-from superset.commands.utils import compute_subjects
+from superset.commands.utils import compute_subjects, raise_if_managed_externally
from superset.connectors.sqla.models import SqlaTable, validate_stored_expression
from superset.daos.dataset import DatasetDAO
from superset.datasets.schemas import FolderSchema
@@ -110,6 +110,8 @@ class UpdateDatasetCommand(UpdateMixin, BaseCommand):
except SupersetSecurityException as ex:
raise DatasetForbiddenError() from ex
+ raise_if_managed_externally(self._model, DatasetForbiddenError)
+
# Validate/Populate editors
compute_subjects(self._model, self._properties, exceptions)
diff --git a/superset/commands/utils.py b/superset/commands/utils.py
index dd5a404786b..f020c1d103a 100644
--- a/superset/commands/utils.py
+++ b/superset/commands/utils.py
@@ -77,6 +77,26 @@ def current_user_can_modify_object(model: Any) -> bool:
)
+def raise_if_managed_externally(model: Any, forbidden_exc: type[Exception]) -> None:
+ """Refuse a mutation of an externally managed entity.
+
+ An entity with ``is_managed_externally`` set has its source of truth
+ outside Superset: an in-app write would be overwritten on the next
+ external sync, which is why the browser hides the edit affordances for
+ such entities. The refusal must also be enforced server-side -- an
+ otherwise-authorized editor could call the API directly. (Version
+ restore is the same class of mutation; apache/superset#44013 adds the
+ matching gate there.) Raised as the caller's
+ editorship-denial exception (HTTP 403) so the response body is
+ identical to a permission denial and discloses nothing new.
+
+ Call this *after* the editorship check: a caller with no edit rights
+ at all should keep receiving the plain editorship denial.
+ """
+ if model.is_managed_externally:
+ raise forbidden_exc()
+
+
def populate_subject_list(
subject_ids: list[int] | None,
default_to_user: bool,
diff --git a/superset/dashboards/schemas.py b/superset/dashboards/schemas.py
index a71002aa2c7..7514da44bac 100644
--- a/superset/dashboards/schemas.py
+++ b/superset/dashboards/schemas.py
@@ -24,7 +24,10 @@ from superset import security_manager
from superset.subjects.schemas import SubjectResponseSchema
from superset.tags.models import TagType
from superset.utils import json
-from superset.utils.schema import validate_external_url
+from superset.utils.schema import (
+ DiscardIsManagedExternallyMixin,
+ validate_external_url,
+)
get_delete_ids_schema = {
"type": "array",
@@ -480,7 +483,7 @@ class DashboardCopySchema(Schema):
)
-class DashboardPutSchema(BaseDashboardSchema):
+class DashboardPutSchema(DiscardIsManagedExternallyMixin, BaseDashboardSchema):
dashboard_title = fields.String(
metadata={"description": dashboard_title_description},
allow_none=True,
@@ -528,7 +531,6 @@ class DashboardPutSchema(BaseDashboardSchema):
certification_details = fields.String(
metadata={"description": certification_details_description}, allow_none=True
)
- is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
external_url = fields.String(allow_none=True, validate=validate_external_url)
tags = fields.List(
fields.Integer(metadata={"description": tags_description}, allow_none=True)
diff --git a/superset/datasets/schemas.py b/superset/datasets/schemas.py
index cef2fa66006..e41c937175c 100644
--- a/superset/datasets/schemas.py
+++ b/superset/datasets/schemas.py
@@ -34,6 +34,7 @@ from superset import security_manager
from superset.exceptions import SupersetMarshmallowValidationError
from superset.models.sql_types import parse_currency_string
from superset.utils import json
+from superset.utils.schema import DiscardIsManagedExternallyMixin
get_delete_ids_schema = {
"type": "array",
@@ -181,7 +182,7 @@ class DatasetPostSchema(Schema):
uuid = fields.UUID(allow_none=True)
-class DatasetPutSchema(Schema):
+class DatasetPutSchema(DiscardIsManagedExternallyMixin, Schema):
table_name = fields.String(allow_none=True, validate=Length(1, 250))
database_id = fields.Integer()
sql = fields.String(allow_none=True)
@@ -204,7 +205,6 @@ class DatasetPutSchema(Schema):
metrics = fields.List(fields.Nested(DatasetMetricsPutSchema))
folders = fields.List(fields.Nested(FolderSchema), required=False)
extra = fields.String(allow_none=True)
- is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
external_url = fields.String(allow_none=True)
uuid = fields.UUID(allow_none=True)
diff --git a/superset/utils/schema.py b/superset/utils/schema.py
index d2c428cd8da..a2e4f9d3f7d 100644
--- a/superset/utils/schema.py
+++ b/superset/utils/schema.py
@@ -17,7 +17,7 @@
from typing import Any, Optional, Union
from urllib.parse import urlparse
-from marshmallow import validate, ValidationError
+from marshmallow import fields, pre_load, Schema, validate, ValidationError
from superset.utils import json
@@ -83,3 +83,27 @@ def validate_external_url(value: Optional[str]) -> None:
)
if not parsed.netloc:
raise ValidationError("URL must be absolute and include a host.")
+
+
+class DiscardIsManagedExternallyMixin(Schema):
+ """Accept and discard ``is_managed_externally`` for wire compatibility.
+
+ The flag is not client-writable: the managed-externally update gate
+ refuses edits of flagged entities, so a client-set ``True`` would be
+ irreversible via the API. Older clients echo GET payloads back on
+ PUT, and the PUT schemas raise on unknown fields, so the key is
+ dropped here rather than removed from the accepted payload. Shared by
+ the chart, dashboard, and dataset PUT schemas so the discard
+ semantics cannot drift between entities.
+ """
+
+ # pylint: disable=unused-argument
+ @pre_load
+ def _discard_is_managed_externally(
+ self, data: dict[str, Any], **kwargs: Any
+ ) -> dict[str, Any]:
+ if isinstance(data, dict):
+ data.pop("is_managed_externally", None)
+ return data
+
+ is_managed_externally = fields.Boolean(allow_none=True, dump_default=False)
diff --git a/superset/views/core.py b/superset/views/core.py
index 72b3d314aa9..1c5107c181b 100755
--- a/superset/views/core.py
+++ b/superset/views/core.py
@@ -294,6 +294,18 @@ class Superset(BaseSupersetView):
status=403,
)
+ # Externally managed charts refuse the legacy overwrite too: this
+ # path assigns request values (params, query_context, ...) directly
+ # to the loaded chart and persists via ChartDAO.update, bypassing
+ # UpdateChartCommand's raise_if_managed_externally gate. Same 403
+ # message as the editorship denial above, so nothing new is
+ # disclosed (see superset/commands/utils.py).
+ if action == "overwrite" and slc is not None and slc.is_managed_externally:
+ return json_error_response(
+ __("You don't have the rights to alter this chart"),
+ status=403,
+ )
+
if action == "saveas" and not slice_add_perm:
return json_error_response(
__("You don't have the rights to create a chart"),
diff --git a/tests/integration_tests/charts/api_tests.py b/tests/integration_tests/charts/api_tests.py
index 048d5aa75ee..37cfdc7caae 100644
--- a/tests/integration_tests/charts/api_tests.py
+++ b/tests/integration_tests/charts/api_tests.py
@@ -969,6 +969,34 @@ class TestChartApi(ApiEditorsTestCaseMixin, InsertChartMixin, SupersetTestCase):
db.session.delete(user_alpha2)
db.session.commit()
+ def test_update_chart_refuses_externally_managed(self) -> None:
+ """sc-120011: PUT on an externally managed chart is refused with 403.
+
+ Refused server-side even for an admin who could otherwise edit it:
+ the update would be overwritten on the next external sync, and the
+ browser-only gate can be bypassed by calling the endpoint
+ directly. Chart is the representative real-endpoint case; the
+ guard is the shared raise_if_managed_externally helper called by
+ all three update commands, pinned across chart/dashboard/dataset
+ by tests/unit_tests/commands/test_update_managed_externally.py.
+ """
+ admin = self.get_user("admin")
+ chart = self.insert_chart("external source of truth", [admin.id], 1)
+ chart.is_managed_externally = True
+ db.session.commit()
+
+ self.login(ADMIN_USERNAME)
+ try:
+ uri = f"api/v1/chart/{chart.id}"
+ rv = self.put_assert_metric(uri, {"slice_name": "changed"}, "put")
+ assert rv.status_code == 403
+
+ db.session.refresh(chart)
+ assert chart.slice_name == "external source of truth"
+ finally:
+ db.session.delete(chart)
+ db.session.commit()
+
def test_update_chart_linked_with_not_owned_dashboard(self):
"""
Chart API: Test update chart which is linked to not owned dashboard
diff --git a/tests/integration_tests/core_tests.py b/tests/integration_tests/core_tests.py
index 76b3768ee76..fbf82503568 100644
--- a/tests/integration_tests/core_tests.py
+++ b/tests/integration_tests/core_tests.py
@@ -197,6 +197,56 @@ class TestCore(SupersetTestCase):
db.session.delete(slc)
db.session.commit()
+ @pytest.mark.usefixtures("load_energy_table_with_slice")
+ def test_overwrite_refuses_externally_managed_slice(self):
+ """sc-120011: the legacy Explore overwrite is gated server-side.
+
+ /superset/explore/ with action=overwrite assigns request values
+ (params, query_context, ...) directly to the loaded chart and
+ persists via ChartDAO.update, bypassing UpdateChartCommand -- so
+ the managed-externally refusal must exist on this path too, even
+ for an admin who could otherwise edit the chart.
+
+ The refusal message is asserted (not just the status) so a 403
+ from an unrelated check cannot satisfy the pin; environments with
+ broken admin datasource grants fail here with the datasource
+ error instead -- CI is the verifier.
+ """
+ self.login(ADMIN_USERNAME)
+ slc = self.get_slice("Energy Sankey")
+ slice_id = slc.id
+ original_name = slc.slice_name
+ original_query_context = slc.query_context
+ slc.is_managed_externally = True
+ db.session.commit()
+
+ tbl_id = self.table_ids.get("energy_usage")
+ url = (
+ f"/explore/table/{tbl_id}/?slice_name=changed&action=overwrite"
+ "&datasource_name=energy_usage"
+ )
+ form_data = {
+ "adhoc_filters": [],
+ "viz_type": "sankey",
+ "groupby": ["target"],
+ "metric": "sum__value",
+ "row_limit": 5000,
+ "slice_id": slice_id,
+ }
+
+ try:
+ resp = self.client.post(url, data={"form_data": json.dumps(form_data)})
+ assert resp.status_code == 403
+ assert "rights to alter this chart" in resp.get_data(as_text=True)
+
+ slc = db.session.query(Slice).filter_by(id=slice_id).one()
+ assert slc.slice_name == original_name
+ assert slc.query_context == original_query_context
+ finally:
+ slc = db.session.query(Slice).filter_by(id=slice_id).one()
+ slc.is_managed_externally = False
+ db.session.commit()
+
@pytest.mark.usefixtures("load_birth_names_dashboard_with_slices")
def test_slice_data(self):
# slice data should have some required attributes
diff --git a/tests/unit_tests/commands/chart/update_test.py b/tests/unit_tests/commands/chart/update_test.py
index 9626150437b..eef43c037d4 100644
--- a/tests/unit_tests/commands/chart/update_test.py
+++ b/tests/unit_tests/commands/chart/update_test.py
@@ -54,7 +54,9 @@ def test_update_chart_editorship_enforced_for_regular_update(
) -> None:
"""Non-editors must not be able to update a chart via a regular payload."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
raise_for_editorship = mocker.patch(
"superset.commands.chart.update.security_manager.raise_for_editorship",
side_effect=_editorship_exc(),
@@ -72,7 +74,9 @@ def test_update_chart_query_context_skips_editorship_check(
) -> None:
"""Query-context-only updates skip editorship but still require chart access."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
raise_for_editorship = mocker.patch(
"superset.commands.chart.update.security_manager.raise_for_editorship",
side_effect=_editorship_exc(),
@@ -96,7 +100,9 @@ def test_update_chart_query_context_requires_chart_access(
"""A query-context-only update by someone without access to the chart is
rejected, even though the editorship check is relaxed for this path."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
mocker.patch(
"superset.commands.chart.update.security_manager.raise_for_access",
side_effect=_access_exc(),
@@ -115,7 +121,9 @@ def test_update_chart_query_context_non_editor_with_access_allowed(
datasource access, or a report worker) can perform a query-context-only
backfill: editorship is relaxed and ``raise_for_access`` does not deny."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
raise_for_editorship = mocker.patch(
"superset.commands.chart.update.security_manager.raise_for_editorship",
side_effect=_editorship_exc(),
@@ -139,7 +147,9 @@ def test_update_chart_editor_can_perform_regular_update(
"""Chart editors can perform regular updates and pass editor changes."""
editor = mocker.MagicMock(id=1)
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- chart = mocker.MagicMock(id=1, tags=[], dashboards=[], editors=[editor])
+ chart = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[], editors=[editor]
+ )
find_by_id.return_value = chart
raise_for_editorship = mocker.patch(
"superset.commands.chart.update.security_manager.raise_for_editorship"
@@ -179,6 +189,7 @@ def test_update_chart_query_context_matching_datasource_is_allowed(
"""A query context that targets the chart's own datasource is accepted."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False,
id=1,
tags=[],
dashboards=[],
@@ -210,7 +221,12 @@ def test_update_chart_query_context_mismatched_datasource_is_rejected(
"""A query context pointing at a different datasource is rejected with a 4xx."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
find_by_id.return_value = mocker.MagicMock(
- id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
+ is_managed_externally=False,
+ id=1,
+ tags=[],
+ dashboards=[],
+ datasource_id=42,
+ datasource_type="table",
)
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch("superset.commands.chart.update.security_manager.raise_for_access")
@@ -234,7 +250,12 @@ def test_update_chart_query_context_without_datasource_is_allowed(
"""Payloads with no verifiable datasource fall back to the chart's own."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
find_by_id.return_value = mocker.MagicMock(
- id=1, tags=[], dashboards=[], datasource_id=42, datasource_type="table"
+ is_managed_externally=False,
+ id=1,
+ tags=[],
+ dashboards=[],
+ datasource_id=42,
+ datasource_type="table",
)
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch("superset.commands.chart.update.security_manager.raise_for_access")
@@ -259,7 +280,9 @@ def test_update_chart_rejects_repointing_to_non_table_datasource(
through editorship + compute_subjects, unlike the query-context-only
tests above."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch(
"superset.commands.chart.update.compute_subjects",
@@ -290,7 +313,9 @@ def test_update_chart_missing_datasource_type_keeps_required_error(
exceptions key their message under ``datasource_type``, and
normalized_messages() only keeps the last one written for a given key."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch(
"superset.commands.chart.update.compute_subjects",
@@ -323,7 +348,9 @@ def test_update_chart_rejects_type_only_non_table_datasource(
``table`` would still break Slice.datasource, since its relationship
only ever resolves the ``table`` type."""
find_by_id = mocker.patch("superset.commands.chart.update.ChartDAO.find_by_id")
- find_by_id.return_value = mocker.MagicMock(id=1, tags=[], dashboards=[])
+ find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False, id=1, tags=[], dashboards=[]
+ )
mocker.patch("superset.commands.chart.update.security_manager.raise_for_editorship")
mocker.patch(
"superset.commands.chart.update.compute_subjects",
diff --git a/tests/unit_tests/commands/dataset/refresh_test.py b/tests/unit_tests/commands/dataset/refresh_test.py
index 4f61d1eef74..c0c97daa585 100644
--- a/tests/unit_tests/commands/dataset/refresh_test.py
+++ b/tests/unit_tests/commands/dataset/refresh_test.py
@@ -69,7 +69,7 @@ def test_refresh_swallows_virtual_table_parse_exception(
SQL that cannot be validated at save time. See #38012.
"""
mock_dataset_dao = mocker.patch("superset.commands.dataset.refresh.DatasetDAO")
- mock_model = mocker.MagicMock()
+ mock_model = mocker.MagicMock(is_managed_externally=False)
mock_model.table_name = "jinja_dataset"
mock_model.fetch_metadata.side_effect = SupersetVirtualTableParseException(
message="Invalid SQL: unexpected token"
@@ -101,7 +101,7 @@ def test_refresh_still_raises_generic_db_error(mocker: MockerFixture) -> None:
the refresh command must NOT silence these — even when the same
dataset also contains Jinja markers. See #38012."""
mock_dataset_dao = mocker.patch("superset.commands.dataset.refresh.DatasetDAO")
- mock_model = mocker.MagicMock()
+ mock_model = mocker.MagicMock(is_managed_externally=False)
mock_model.table_name = "jinja_dataset_with_bad_connection"
mock_model.sql = (
"SELECT * FROM foo {% if from_dttm %}WHERE ds > '{{ from_dttm }}'{% endif %}"
@@ -129,7 +129,7 @@ def test_refresh_still_raises_on_security_exception(
refresh path must not become a bypass for security checks. See #38012.
"""
mock_dataset_dao = mocker.patch("superset.commands.dataset.refresh.DatasetDAO")
- mock_model = mocker.MagicMock()
+ mock_model = mocker.MagicMock(is_managed_externally=False)
mock_model.table_name = "restricted_dataset"
mock_model.fetch_metadata.side_effect = SupersetSecurityException(
SupersetError(
@@ -170,7 +170,9 @@ def test_refresh_dataset_not_found(mocker: MockerFixture) -> None:
def test_refresh_forbidden(mocker: MockerFixture) -> None:
"""Sanity check that the pre-existing forbidden path still works."""
mock_dataset_dao = mocker.patch("superset.commands.dataset.refresh.DatasetDAO")
- mock_dataset_dao.find_by_id.return_value = mocker.MagicMock()
+ mock_dataset_dao.find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False
+ )
mocker.patch(
"superset.commands.dataset.refresh.security_manager.raise_for_editorship",
side_effect=SupersetSecurityException(
diff --git a/tests/unit_tests/commands/dataset/update_test.py b/tests/unit_tests/commands/dataset/update_test.py
index c5e696c3ce3..ac90207abaf 100644
--- a/tests/unit_tests/commands/dataset/update_test.py
+++ b/tests/unit_tests/commands/dataset/update_test.py
@@ -59,7 +59,9 @@ def test_update_dataset_forbidden(mocker: MockerFixture) -> None:
Test try updating a dataset without permission raises a `DatasetForbiddenError`.
"""
mock_dataset_dao = mocker.patch("superset.commands.dataset.update.DatasetDAO")
- mock_dataset_dao.find_by_id.return_value = mocker.MagicMock()
+ mock_dataset_dao.find_by_id.return_value = mocker.MagicMock(
+ is_managed_externally=False
+ )
mocker.patch(
"superset.commands.dataset.update.security_manager.raise_for_editorship",
@@ -86,7 +88,7 @@ def test_update_dataset_sql_authorized_schema(mocker: MockerFixture) -> None:
mock_database.get_default_catalog.return_value = "catalog"
mock_database.allow_multi_catalog = False
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = "public"
@@ -131,7 +133,7 @@ def test_update_dataset_sql_unauthorized_schema(mocker: MockerFixture) -> None:
mock_database.get_default_catalog.return_value = "catalog"
mock_database.allow_multi_catalog = False
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = "public"
@@ -194,7 +196,7 @@ def test_update_dataset_database_id_change_checks_new_database_access(
mock_new_database.get_default_catalog.return_value = "catalog"
mock_new_database.allow_multi_catalog = False
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_current_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = "public"
@@ -250,7 +252,7 @@ def test_update_dataset_database_id_change_allowed_with_access(
mock_new_database.get_default_catalog.return_value = "catalog"
mock_new_database.allow_multi_catalog = False
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_current_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = "public"
@@ -298,7 +300,7 @@ def test_update_dataset_physical_repoint_requires_table_access(
mock_database.get_default_catalog.return_value = "catalog"
mock_database.allow_multi_catalog = False
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = "public"
@@ -380,7 +382,7 @@ def test_update_dataset_validation_errors(
mock_database.id = 1
mock_database.get_default_catalog.return_value = "catalog"
mock_database.allow_multi_catalog = False
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset_dao.find_by_id.return_value = mock_dataset
@@ -450,7 +452,7 @@ def test_update_dataset_rejects_malicious_expression(
mock_database.backend = "sqlite"
mock_database.allow_multi_catalog = False
mock_database.get_default_catalog.return_value = "catalog"
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = None
@@ -499,7 +501,7 @@ def test_update_dataset_accepts_benign_expression(mocker: MockerFixture) -> None
mock_database.backend = "sqlite"
mock_database.allow_multi_catalog = False
mock_database.get_default_catalog.return_value = "catalog"
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = None
@@ -540,7 +542,7 @@ def test_update_dataset_accepts_jinja_expression(mocker: MockerFixture) -> None:
mock_database.backend = "sqlite"
mock_database.allow_multi_catalog = False
mock_database.get_default_catalog.return_value = "catalog"
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = None
@@ -1301,7 +1303,7 @@ def test_update_dataset_rejects_malicious_fetch_values_predicate(
mock_database.backend = "sqlite"
mock_database.allow_multi_catalog = False
mock_database.get_default_catalog.return_value = "catalog"
- mock_dataset = mocker.MagicMock()
+ mock_dataset = mocker.MagicMock(is_managed_externally=False)
mock_dataset.database = mock_database
mock_dataset.catalog = "catalog"
mock_dataset.schema = None
diff --git a/tests/unit_tests/commands/test_update_managed_externally.py b/tests/unit_tests/commands/test_update_managed_externally.py
new file mode 100644
index 00000000000..342bd926ac4
--- /dev/null
+++ b/tests/unit_tests/commands/test_update_managed_externally.py
@@ -0,0 +1,403 @@
+# 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.
+"""sc-120011: mutating commands refuse externally managed entities.
+
+``is_managed_externally`` marks an entity whose source of truth lives
+outside Superset; an in-app write would be overwritten on the next
+external sync, so the browser hides the edit affordances. These tests pin
+the server-side refusals: the chart/dashboard/dataset update commands
+(including the chart's relaxed-editorship query-context-only save), the
+dataset refresh command, and the narrowing of the dashboard colors-config
+exemption to derived values only. The ``is_managed_externally`` flag
+itself is discarded by the ordinary PUT schemas so it cannot be set (and
+thereby self-locked) through the API. The representative real-endpoint
+case is
+``tests/integration_tests/charts/api_tests.py::test_update_chart_refuses_externally_managed``.
+"""
+
+import contextlib
+from typing import Any, Final
+from unittest.mock import MagicMock
+
+import pytest
+from marshmallow import Schema
+from pytest_mock import MockerFixture
+
+from superset.charts.schemas import ChartPutSchema
+from superset.commands.chart.exceptions import ChartForbiddenError
+from superset.commands.chart.update import UpdateChartCommand
+from superset.commands.dashboard.exceptions import DashboardForbiddenError
+from superset.commands.dashboard.update import (
+ UpdateDashboardColorsConfigCommand,
+ UpdateDashboardCommand,
+)
+from superset.commands.dataset.exceptions import DatasetForbiddenError
+from superset.commands.dataset.refresh import RefreshDatasetCommand
+from superset.commands.dataset.update import UpdateDatasetCommand
+from superset.commands.utils import raise_if_managed_externally
+from superset.dashboards.schemas import DashboardPutSchema
+from superset.datasets.schemas import DatasetPutSchema
+
+ENTITY_CASES: Final = [
+ pytest.param(
+ UpdateChartCommand,
+ "superset.commands.chart.update",
+ "ChartDAO",
+ ChartForbiddenError,
+ id="chart",
+ ),
+ pytest.param(
+ UpdateDashboardCommand,
+ "superset.commands.dashboard.update",
+ "DashboardDAO",
+ DashboardForbiddenError,
+ id="dashboard",
+ ),
+ pytest.param(
+ UpdateDatasetCommand,
+ "superset.commands.dataset.update",
+ "DatasetDAO",
+ DatasetForbiddenError,
+ id="dataset",
+ ),
+]
+
+
+def _wire_module_mocks(
+ mocker: MockerFixture, module: str, dao_name: str, entity: MagicMock
+) -> MagicMock:
+ """Replace the command module's collaborators with explicit MagicMocks.
+
+ ``security_manager`` is a werkzeug LocalProxy at module level; a bare
+ ``mocker.patch`` would replace it with an AsyncMock (``mock.patch``'s
+ async-object detection is fooled by the proxy's attribute forwarding)
+ whose raising side_effects silently never fire, so an explicit
+ ``MagicMock`` is passed as the replacement.
+ """
+ sm = MagicMock()
+ sm.raise_for_editorship.return_value = None
+ mocker.patch(f"{module}.security_manager", new=sm)
+ dao = MagicMock()
+ dao.find_by_id.return_value = entity
+ mocker.patch(f"{module}.{dao_name}", new=dao)
+ mocker.patch(f"{module}.compute_subjects", new=MagicMock())
+ return sm
+
+
+def _managed_entity() -> MagicMock:
+ entity = MagicMock()
+ entity.is_managed_externally = True
+ return entity
+
+
+@pytest.mark.parametrize(("command_cls", "module", "dao_name", "exc"), ENTITY_CASES)
+def test_update_refuses_externally_managed_entity(
+ mocker: MockerFixture,
+ command_cls: type,
+ module: str,
+ dao_name: str,
+ exc: type[Exception],
+) -> None:
+ """An editor-passing update of a managed entity is refused with 403.
+
+ The refusal reuses the entity's editorship-denial exception (mapped to
+ HTTP 403 by the API layer) so the response discloses nothing new.
+ """
+ entity = _managed_entity()
+ sm = _wire_module_mocks(mocker, module, dao_name, entity)
+
+ with pytest.raises(exc):
+ command_cls(1, {"description": "changed"}).validate()
+
+ # This also pins the order: had the gate run first and raised, the
+ # editorship check would never have been reached, so a caller with no
+ # edit rights keeps receiving the plain editorship denial.
+ sm.raise_for_editorship.assert_called_once_with(entity)
+
+
+@pytest.mark.parametrize(("command_cls", "module", "dao_name", "exc"), ENTITY_CASES)
+def test_update_command_passes_entity_and_exception_to_gate(
+ mocker: MockerFixture,
+ command_cls: type,
+ module: str,
+ dao_name: str,
+ exc: type[Exception],
+) -> None:
+ """Pins the call-site wiring of the shared gate.
+
+ validate() hands the loaded entity and the entity's 403 exception class
+ to the gate. The inertness half of the contract (flag False -> no
+ raise) is pinned by test_helper_refuses_only_externally_managed below.
+ """
+ entity = MagicMock()
+ entity.is_managed_externally = False
+ _wire_module_mocks(mocker, module, dao_name, entity)
+ gate = mocker.patch(f"{module}.raise_if_managed_externally")
+
+ # Downstream validation may trip over the MagicMock entity; the gate
+ # call (with the right entity and exception class) is the signal under
+ # test, and the gate itself is patched so it cannot be the raiser.
+ with contextlib.suppress(Exception):
+ command_cls(1, {"description": "changed"}).validate()
+
+ gate.assert_called_once_with(entity, exc)
+
+
+def test_query_context_only_update_refuses_externally_managed_chart(
+ mocker: MockerFixture,
+) -> None:
+ """The relaxed-editorship query-context-only save is gated too.
+
+ Stored query context is executable state -- report execution loads and
+ runs it -- so a client-supplied context on a managed chart is a
+ behavior change, not derived state. Explore's background context save
+ simply receives a 403 for such charts.
+ """
+ entity = _managed_entity()
+ _wire_module_mocks(mocker, "superset.commands.chart.update", "ChartDAO", entity)
+
+ with pytest.raises(ChartForbiddenError):
+ UpdateChartCommand(
+ 1, {"query_context": "{}", "query_context_generation": True}
+ ).validate()
+
+
+def test_dashboard_colors_config_derived_only_update_is_exempt(
+ mocker: MockerFixture,
+) -> None:
+ """Derived-only colors payloads stay writable on managed dashboards.
+
+ UpdateDashboardColorsConfigCommand persists color metadata in the
+ background while a dashboard is merely viewed (fire-and-forget from the
+ frontend); a payload that leaves the authoritative inputs unchanged
+ must not start failing with 403s. Control: the parametrized refusal
+ test above proves the parent UpdateDashboardCommand *does* refuse the
+ same entity, and the test below proves changed authoritative inputs
+ are refused.
+ """
+ entity = _managed_entity()
+ entity.json_metadata = '{"color_scheme": "blues", "label_colors": {"a": "#000"}}'
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+ payload = {
+ "color_scheme": "blues",
+ "label_colors": {"a": "#000"},
+ "color_scheme_domain": ["#000"],
+ "shared_label_colors": ["a"],
+ "map_label_colors": {"a": "#000"},
+ }
+
+ try:
+ UpdateDashboardColorsConfigCommand(1, payload).validate()
+ except DashboardForbiddenError:
+ pytest.fail("derived-only colors save refused for a managed dashboard")
+
+
+@pytest.mark.parametrize(
+ ("key", "empty"),
+ [
+ ("color_scheme", None),
+ ("color_scheme", ""),
+ ("label_colors", None),
+ ("label_colors", {}),
+ ],
+)
+def test_dashboard_colors_config_allows_empty_for_absent_key(
+ mocker: MockerFixture, key: str, empty: object
+) -> None:
+ """Empty-vs-absent is NOT a color change — the routine view-time case.
+
+ The background colors sync always sends ``label_colors`` (as ``{}``
+ when nothing is set) while a dashboard is merely VIEWED; a managed
+ dashboard whose metadata lacks the key must not 403 every view.
+ Absent, explicit null, and empty all encode the same effective color
+ state (writing the empty key changes export bytes, not colors — the
+ next external sync owns the bytes). Supersedes the earlier
+ refuses-null-for-absent-key pin, which overcorrected.
+ """
+ entity = _managed_entity()
+ entity.json_metadata = "{}"
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+
+ try:
+ UpdateDashboardColorsConfigCommand(1, {key: empty}).validate()
+ except DashboardForbiddenError:
+ pytest.fail(f"empty {key} for an absent stored key refused")
+
+
+@pytest.mark.parametrize("key", ["color_scheme", "label_colors"])
+def test_dashboard_colors_config_refuses_clearing_stored_colors(
+ mocker: MockerFixture, key: str
+) -> None:
+ """Emptying a key that HOLDS authoritative state is still a change."""
+ entity = _managed_entity()
+ entity.json_metadata = '{"color_scheme": "blues", "label_colors": {"a": "#000"}}'
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+
+ with pytest.raises(DashboardForbiddenError):
+ UpdateDashboardColorsConfigCommand(1, {key: None}).validate()
+
+
+def test_dashboard_colors_config_refuses_setting_value_on_absent_key(
+ mocker: MockerFixture,
+) -> None:
+ """A real value for an absent key IS a change (the inverse control)."""
+ entity = _managed_entity()
+ entity.json_metadata = "{}"
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+
+ with pytest.raises(DashboardForbiddenError):
+ UpdateDashboardColorsConfigCommand(1, {"color_scheme": "blues"}).validate()
+
+
+@pytest.mark.parametrize("key", ["color_scheme", "label_colors"])
+def test_dashboard_colors_config_refuses_changing_authoritative_colors(
+ mocker: MockerFixture, key: str
+) -> None:
+ """Changing an authoritative color input on a managed dashboard is 403.
+
+ color_scheme and label_colors are the dashboard's real content (the
+ other accepted keys are derived from them); letting the colors PUT
+ rewrite them would reopen the integrity gap the update gate closes.
+ """
+ entity = _managed_entity()
+ entity.json_metadata = '{"color_scheme": "blues", "label_colors": {"a": "#000"}}'
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+
+ with pytest.raises(DashboardForbiddenError):
+ UpdateDashboardColorsConfigCommand(1, {key: "changed"}).validate()
+
+
+def test_refresh_refuses_externally_managed_dataset(mocker: MockerFixture) -> None:
+ """PUT /dataset/<pk>/refresh is gated after the editorship check.
+
+ Refresh persists fetched column metadata onto the dataset; a managed
+ dataset's columns are owned by the external sync.
+ """
+ entity = _managed_entity()
+ sm = MagicMock()
+ sm.raise_for_editorship.return_value = None
+ mocker.patch("superset.commands.dataset.refresh.security_manager", new=sm)
+ dao = MagicMock()
+ dao.find_by_id.return_value = entity
+ mocker.patch("superset.commands.dataset.refresh.DatasetDAO", new=dao)
+
+ with pytest.raises(DatasetForbiddenError):
+ RefreshDatasetCommand(1).validate()
+
+ sm.raise_for_editorship.assert_called_once_with(entity)
+
+
+@pytest.mark.parametrize(
+ "schema_cls", [ChartPutSchema, DashboardPutSchema, DatasetPutSchema]
+)
+def test_put_schemas_discard_is_managed_externally(schema_cls: type[Schema]) -> None:
+ """The flag is not client-writable, and echoing it back is harmless.
+
+ A client-set True would be irreversible via the API once the gate
+ refuses edits of flagged entities, so the ordinary PUT schemas drop
+ the key in a pre_load hook. The field stays DECLARED so the published
+ OpenAPI contract (and generated clients) keep the property, and
+ echoing it back never 422s -- but because pre_load runs before field
+ loading, the key can never reach the loaded payload.
+ """
+ assert "is_managed_externally" in schema_cls().fields
+ loaded: dict[str, Any] = schema_cls().load({"is_managed_externally": True})
+ assert "is_managed_externally" not in loaded
+
+
+def test_helper_refuses_only_externally_managed() -> None:
+ """The helper raises the supplied exception iff the flag is set."""
+ with pytest.raises(ChartForbiddenError):
+ raise_if_managed_externally(_managed_entity(), ChartForbiddenError)
+
+ local = MagicMock()
+ local.is_managed_externally = False
+ raise_if_managed_externally(local, ChartForbiddenError) # must not raise
+
+
+def test_dashboard_published_only_update_is_exempt(mocker: MockerFixture) -> None:
+ """The publish toggle stays writable on a managed dashboard.
+
+ ``published`` is Superset-local visibility state, not dashboard
+ content owned by the external source of truth; the frontend's
+ savePublished PUTs exactly ``{published: bool}``. Control: the mixed
+ test below proves adding any other field re-arms the gate.
+ """
+ entity = _managed_entity()
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+
+ try:
+ UpdateDashboardCommand(1, {"published": False}).validate()
+ except DashboardForbiddenError:
+ pytest.fail("published-only update refused for a managed dashboard")
+
+
+def test_dashboard_published_with_content_field_is_refused(
+ mocker: MockerFixture,
+) -> None:
+ """Bundling content changes with the publish toggle does not slip the gate."""
+ entity = _managed_entity()
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+
+ with pytest.raises(DashboardForbiddenError):
+ UpdateDashboardCommand(
+ 1, {"published": False, "dashboard_title": "renamed"}
+ ).validate()
+
+
+@pytest.mark.parametrize(
+ "command_name",
+ [
+ "UpdateDashboardNativeFiltersCommand",
+ "UpdateDashboardChartCustomizationsCommand",
+ ],
+)
+def test_dashboard_sibling_commands_inherit_the_managed_gate(
+ mocker: MockerFixture, command_name: str
+) -> None:
+ """Native-filters and chart-customizations updates refuse managed dashboards.
+
+ Both subclass UpdateDashboardCommand and call super().validate(), so
+ they inherit the gate. Deliberate: filter configuration and chart
+ customizations are dashboard CONTENT owned by the external source of
+ truth (edit-time writes, not view-time background syncs). This pin
+ turns the silent inheritance into declared behavior.
+ """
+ import superset.commands.dashboard.update as update_module
+
+ entity = _managed_entity()
+ _wire_module_mocks(
+ mocker, "superset.commands.dashboard.update", "DashboardDAO", entity
+ )
+ command_cls = getattr(update_module, command_name)
+
+ with pytest.raises(DashboardForbiddenError):
+ command_cls(1, {"json_metadata": "{}"}).validate()