sadpandajoe commented on code in PR #44215:
URL: https://github.com/apache/superset/pull/44215#discussion_r4460906406
##########
superset/security/manager.py:
##########
@@ -5715,154 +6013,34 @@ def raise_for_access( # noqa: C901
assert datasource
self.raise_for_unsupported_guest_rls(datasource)
- def has_promiscuous_chart_access() -> bool:
- if not (
- form_data
- and is_feature_enabled("ENABLE_VIEWERS")
- and current_app.config.get("VIEWER_PROMISCUOUS_MODE")
- and (viewer_slice_id := form_data.get("slice_id"))
- and (
- viewer_slc := self.session.query(Slice)
- .filter(Slice.id == viewer_slice_id)
- .one_or_none()
- )
- ):
- return False
-
- if not _datasource_matches(
- datasource, viewer_slc.datasource_id,
viewer_slc.datasource_type
- ):
- return False
-
- return self.is_viewer(viewer_slc) or self.is_editor(viewer_slc)
-
if not (
self.can_access_schema(datasource)
or self.can_access("datasource_access", datasource.perm or "")
# A grant on a semantic view's parent layer covers the view,
# matching SemanticView.raise_for_access (sc-119501).
or self._semantic_layer_grant_allows(datasource)
- # ``allow_editorship`` is honored here too so a caller that
opts
- # out of the editorship accept (e.g. dataset define/repoint
- # validation) gets consistent behavior across both branches,
- # rather than the flag silently applying only to the
- # database+table / query branch above.
+ # ``allow_editorship`` is honored on both branches so a caller
+ # that opts out of the editorship accept (e.g. dataset
+ # define/repoint validation) gets consistent behavior.
or (allow_editorship and self.is_editor(datasource))
+ # Embedded guest / promiscuous viewer, bound to the specific
+ # dashboard request (native filter, member chart, multi-layer
+ # child, or drill). Keeps guests strictly scoped to the
request.
+ or self._has_request_bound_dashboard_datasource_access(
+ datasource, form_data
+ )
+ # Chart-viewer/editor promiscuous mode: a viewer or editor of
the
+ # chart the request references may query that chart's
datasource.
+ or self._has_promiscuous_chart_access(datasource, form_data)
+ # Dashboard-viewer promiscuous mode: a viewer or editor of a
+ # dashboard that uses this datasource inherits access to it,
+ # independent of the request's ``form_data``. This is what
+ # lets ``GET /dashboard/<id>/datasets`` serve full metadata to
a
+ # promiscuous viewer.
or (
- # Grant access to the datasource only if dashboard RBAC is
enabled
- # or the user is an embedded guest user with access to the
dashboard
- # and said datasource is associated with the dashboard
chart in
- # question.
- form_data
- and (dashboard_id := form_data.get("dashboardId"))
- and (
- dashboard_ := self.session.query(Dashboard)
- .filter(Dashboard.id == dashboard_id)
- .one_or_none()
- )
- and (
- (
- is_feature_enabled("EMBEDDED_SUPERSET")
- and self.is_guest_user()
- )
- or (
- is_feature_enabled("ENABLE_VIEWERS")
- and
current_app.config.get("VIEWER_PROMISCUOUS_MODE")
- and self.is_viewer(dashboard_)
- )
- )
- and (
- (
- # Native filter.
- form_data.get("type") == "NATIVE_FILTER"
- and (native_filter_id :=
form_data.get("native_filter_id"))
- and dashboard_.json_metadata
- and (json_metadata :=
json.loads(dashboard_.json_metadata))
- and any(
- _datasource_matches(
- datasource,
- target.get("datasetId"),
- target.get("datasourceType")
- or DatasourceType.TABLE,
- )
- for fltr in json_metadata.get(
- "native_filter_configuration",
- [],
- )
- for target in fltr.get("targets", [])
- if native_filter_id == fltr.get("id")
- )
- )
- or (
- form_data.get("type") != "NATIVE_FILTER"
- and (
- (
- # Chart.
- (slice_id := form_data.get("slice_id"))
- and (
- # Direct chart access (no parent)
- (
- form_data.get("parent_slice_id")
is None
- and (
- slc :=
self.session.query(Slice)
- .filter(Slice.id == slice_id)
- .one_or_none()
- )
- and slc in dashboard_.slices
- and _datasource_matches(
- datasource,
- slc.datasource_id,
- slc.datasource_type,
- )
- )
- or
- # Multi-layer chart child access (has
parent)
- (
- (
- parent_id := form_data.get(
- "parent_slice_id"
- )
- )
- and (
- parent_slc :=
self.session.query(Slice)
- .filter(Slice.id == parent_id)
- .one_or_none()
- )
- and parent_slc in dashboard_.slices
- # Validate child is actually part
of parent's config # noqa: E501
- and
self._validate_child_in_parent_multilayer( # noqa: E501
- child_slice_id=slice_id,
- parent_slice=parent_slc,
- )
- # Bind the request to the child
- # chart's own datasource, mirroring
- # the direct-chart leg above.
- and (
- child_slc :=
self.session.query(Slice)
- .filter(Slice.id == slice_id)
- .one_or_none()
- )
- and _datasource_matches(
- datasource,
- child_slc.datasource_id,
- child_slc.datasource_type,
- )
- )
- )
- )
- # D2D or Drill By
- or self.has_drill_access(
- form_data, dashboard_, datasource
- )
- )
- )
- )
- and self.can_access_dashboard(dashboard_)
+ allow_dashboard_inheritance
+ and
self._promiscuous_viewer_inherits_datasource(datasource)
Review Comment:
Create and repoint are covered now, but the Save as path this thread also
named is not: `POST /explore/?action=saveas` checks the dataset with
inheritance still allowed (`views/core.py:248`) and writes the chart through
`ChartDAO.create`, so a dashboard viewer still becomes the chart's editor and
keeps dataset access after losing the dashboard. Should that path run the same
dataset check inside `dashboard_inheritance_disabled()`? The inline comment on
`commands/chart/create.py` has the details.
--
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]
