geido commented on code in PR #45266:
URL: https://github.com/apache/superset/pull/45266#discussion_r4272876801
##########
UPDATING.md:
##########
@@ -24,6 +24,21 @@ assists people when migrating to a new version.
## Next
+- Dataset editorship no longer authorizes raw SQL on the underlying table.
+ Under the strict dataset-match scoping used by SQL Lab raw queries, MetaDB,
Review Comment:
Good catch — added alert/report query execution to the strict-path list in
both `UPDATING.md` and the `force_dataset_match` docstring (23fd487).
##########
superset/security/manager.py:
##########
@@ -5363,10 +5363,24 @@ def raise_for_access( # noqa: C901
catalog=effective_catalog,
)
for datasource_ in datasources:
+ # ``datasource_access`` on a registered dataset is always
+ # sufficient. Editorship of the dataset is accepted only on
+ # the non-strict path (chart-data, dataset CRUD,
+ # ``/table_metadata/``, ``/select_star/``), where it is the
+ # dataset-owner "explore your own dataset" grant from
+ # #20382. Under strict scoping (SQL Lab raw queries,
MetaDB,
Review Comment:
Honest answer: it closes the arbitrary-raw-SQL path (SQL Lab, MCP
`execute_sql`, MetaDB, alert/report), where editorship let you run any SQL
against the warehouse. It deliberately leaves the non-strict `is_editor` accept
(chart-data `query_mode: raw`, `/datasource/samples`) — that's the #20382
"explore your own dataset" grant, which is bounded to the dataset's own
datasource (not arbitrary SQL) and isn't reachable by the decoy-rename vector
(a chart on a virtual decoy runs the decoy's own SQL, not the real table).
So for a *physical* dataset you edit, raw records / samples do still return
that table's rows without `datasource_access` — same survives-revocation shape,
but dataset-scoped and by existing design. Making the non-strict path require
`datasource_access` too would be a broader #20382 behavior change; I kept this
PR scoped to the raw-SQL escalation and can file a follow-up if we want parity.
WDYT?
##########
superset/security/manager.py:
##########
@@ -5363,10 +5363,24 @@ def raise_for_access( # noqa: C901
catalog=effective_catalog,
)
for datasource_ in datasources:
+ # ``datasource_access`` on a registered dataset is always
+ # sufficient. Editorship of the dataset is accepted only on
+ # the non-strict path (chart-data, dataset CRUD,
+ # ``/table_metadata/``, ``/select_star/``), where it is the
+ # dataset-owner "explore your own dataset" grant from
+ # #20382. Under strict scoping (SQL Lab raw queries,
MetaDB,
+ # MCP ``execute_sql``) editorship is deliberately excluded:
+ # returning raw rows from the underlying warehouse table
+ # must follow the user's live data-access grants, not
+ # editorship of a same-named dataset. Any principal able to
+ # create a dataset can make themselves an editor of one
+ # named after an arbitrary table, so accepting editorship
+ # here would let dataset creation stand in for data access
+ # and would outlive revocation of every real grant.
if self.can_access(
"datasource_access",
datasource_.perm or "",
- ) or self.is_editor(datasource_):
+ ) or (not force_dataset_match and
self.is_editor(datasource_)):
Review Comment:
Fixed in 23fd487 — dropped the stale "(or owns)" from the
`force_dataset_match` docstring (manager.py) and the MetaDB comment.
Ownership/editorship no longer authorizes the strict path; it's accepted only
on the non-strict branch.
--
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]
