madhushreeag commented on code in PR #44215:
URL: https://github.com/apache/superset/pull/44215#discussion_r4008613393
##########
superset/security/manager.py:
##########
@@ -4989,6 +5094,14 @@ def member_datasource_accessible() -> bool:
) is not None and self.can_access_datasource(chart_datasource):
return
+ # Dashboard-viewer promiscuous mode: a viewer or editor of a
+ # published dashboard the chart belongs to inherits access to the
+ # chart itself, so its definition (``form_data``) can be served for
+ # rendering. This is datasource-type agnostic and covers charts
+ # whose datasource cannot be resolved above.
+ if self._promiscuous_viewer_inherits_chart(chart):
Review Comment:
Thanks for picking this up, we have been bitten by needing per-chart grants
on top of dashboard access. So this is very welcome :tada:
##########
superset/subjects/utils.py:
##########
@@ -80,6 +86,46 @@ def get_user_subject_ids_subquery(user_id: int) ->
CompoundSelect:
return union_all(user_subj, role_subj, group_subj, group_role_subj)
+def get_inherited_slice_ids_subquery(user_id: int) -> CompoundSelect:
+ """Return a Select of Slice IDs a user inherits access to via a dashboard.
+
+ A user who is an editor or viewer of a *published* dashboard inherits
+ access to every chart on that dashboard. This expresses the promiscuous
+ ``ENABLE_VIEWERS`` inheritance as SQL so it can back both per-object access
+ checks and (composed opt-in) list filters. The feature-flag gate is the
+ caller's responsibility — this builder is pure and never executed here.
+ """
+ subject_subquery = get_user_subject_ids_subquery(user_id)
+
+ def via(assoc: Any) -> Select:
+ return (
+ select(dashboard_slices.c.slice_id)
+ .select_from(dashboard_slices)
+ .join(Dashboard, Dashboard.id == dashboard_slices.c.dashboard_id)
+ .join(assoc, assoc.c.dashboard_id == Dashboard.id)
+ .where(
+ Dashboard.published.is_(True),
+ assoc.c.subject_id.in_(subject_subquery),
+ )
+ )
+
+ return union_all(via(dashboard_editors), via(dashboard_viewers))
+
+
+def get_inherited_datasource_ids_subquery(user_id: int, datasource_type: str)
-> Select:
Review Comment:
I think `published` is wrongly applied to the editors leg. `via()` applies
`Dashboard.published.is_(True)` to both dashboard_editors and
dashboard_viewers. But `raise_for_access` grants editors dashboard access with
no publication requirement.
So an editor of an unpublished dashboard opens it fine, then
`_promiscuous_viewer_inherits_chart`
returns False and every chart's `form_data` is stripped - the dashboard
renders empty for its own
editor.
Confirmed locally:
`published=false` - `form_data` absent on all charts
flip to
`published=true` with nothing else changed - present on all charts
In practice, since an editor who created the charts passes
`is_editor(chart)` and never reaches the inheritance leg, it bites when you're
added as an editor to someone else's unpublished dashboard. Gate belongs on the
viewers leg only.
--
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]
