sadpandajoe commented on code in PR #45342:
URL: https://github.com/apache/superset/pull/45342#discussion_r4336854803
##########
superset/commands/explore/get.py:
##########
@@ -83,10 +83,14 @@ def _authorize_datasource(
# the query at all. The author bypass returns before the generic
# ``datasource`` perm check at the tail of ``raise_for_access``,
# so this does not weaken anything for anyone else.
+ #
+ # Editorship stays off for everyone else: a dataset named after a
+ # table the query reads is a decoy any dataset creator can make.
security_manager.raise_for_access(
query=datasource,
datasource=datasource,
allow_query_authorship_bypass=True,
+ allow_editorship=False,
Review Comment:
`_authorize_datasource` returns after `raise_for_access(chart=slc)` whenever
`slc` is set, so this query check, including the new `allow_editorship=False`,
never runs on that path. `get_form_data` lets a caller-supplied `form_data`
override the chart's saved `datasource`, so a user who can read any chart could
request
`/explore/?slice_id=<chart>&form_data={"datasource":"<query_id>__query"}`.
`get_datasource_info` then resolves to another user's SQL Lab `Query`, only the
chart is authorized, and the response carries that query's SQL as
`result.dataset.sql`. So the Explore GET hardening this comment and the
`manager.py` docstring describe doesn't hold when `slice_id` is present. Should
the query datasource be authorized here even when a slice is loaded, or should
the slice and datasource be required to match?
--
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]
