mikebridge opened a new pull request, #44025:
URL: https://github.com/apache/superset/pull/44025
### SUMMARY
An entity with `is_managed_externally = True` has its source of truth
outside Superset: an in-app write would be overwritten on the next external
sync, which is why the UI hides the edit affordances for such entities. But the
refusal was browser-only — an otherwise-authorized editor could still mutate
the entity by calling `PUT /api/v1/{chart,dashboard,dataset}/<id>` directly.
This is the ordinary-update sibling of the gap #44013 closed for version
restore, and closing it keeps the restore and update surfaces from drifting.
This PR adds a shared `raise_if_managed_externally` helper in
`superset/commands/utils.py` and calls it from all three
`UpdateCommand.validate()` implementations:
- **Placement**: after the editorship check, so a caller with no edit rights
at all keeps receiving the plain editorship denial.
- **Response shape**: the helper raises each entity's existing
editorship-denial exception (`ChartForbiddenError` / `DashboardForbiddenError`
/ `DatasetForbiddenError`), so the HTTP 403 body is identical to a permission
denial and discloses nothing new.
- **Deliberate exemption**: the chart *query-context-only* save path is not
gated. That branch exists so report workers can persist refreshed query context
(it already relaxes editorship for the same reason), and query context is
derived state rather than an edit the external source of truth owns. Blocking
it would break reports on externally managed charts.
Scope note: this is a **data-integrity consistency fix**, not a privilege
escalation — an editor holding edit rights is entitled to mutate these entities
under the role/capability matrix; `is_managed_externally` is an external-sync
integrity concern.
There is precedent for the check in this very layer: `UpdateChartCommand`
already refuses linking a chart to an externally managed *target dashboard*
(`superset/commands/chart/update.py`), and version restore enforces the same
gate server-side (#44013).
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — API-only behavior change. Before: `PUT` on an externally managed
entity returned 200 and mutated it. After: HTTP 403, entity unchanged.
### TESTING INSTRUCTIONS
- `pytest tests/unit_tests/commands/test_update_managed_externally.py` — 7
tests: parametrized across chart/dashboard/dataset, an externally managed
entity is refused with the entity's 403 exception after the editorship check
was consulted; the gate is inert (called, no raise) for locally managed
entities; the helper refuses exactly when `is_managed_externally` is set.
Reverting only the three call sites flips 6 of the 7 tests (the helper-only
test stands), so the pins are non-vacuous.
-
`tests/integration_tests/charts/api_tests.py::TestChartApi::test_update_chart_refuses_externally_managed`
— representative real-endpoint case: admin `PUT` on an externally managed
chart returns 403 and the chart is unchanged.
- Manual: flip `is_managed_externally` on any chart, then `PUT
/api/v1/chart/<id>` with `{"slice_name": "x"}` as an admin → 403; flip it back
→ 200.
### ADDITIONAL INFORMATION
- [ ] Has associated issue:
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
https://claude.ai/code/session_01JRLEJS4mUqKBoPjSjviKUW
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
