villebro commented on code in PR #44215:
URL: https://github.com/apache/superset/pull/44215#discussion_r4053545931
##########
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:
Great catch, and thanks for confirming it locally — you're exactly right.
The dashboard read gate admits editors with no publication requirement and only
gates viewers on `published` (upstream #44093 spells this out: editors "are
admitted above regardless of published"), so applying the gate to both legs of
`via()` broke the case you describe.
Fixed in f8ac8a3137 — `published` now applies to the viewers leg only.
I also took the same fix through `can_inherit_access_via_dashboard`, since
`is_viewer()` returns True for editors and the predicate had the same
asymmetry: an editor of an unpublished dashboard could open it but was denied
drill-to-detail. Both inheritance paths now agree with the read gate.
Covered by new tests that build real dashboards/charts/users/subjects and
execute the generated SQL across the editor/viewer x published/draft matrix
(`tests/unit_tests/subjects/test_inherited_access.py`). I verified they fail
against the old gate and pass with the fix.
##########
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! That per-chart grant toil is exactly what prompted it — good to know
it lands for your setup too. Your review also caught the editor/unpublished
hole in the same area, which is now fixed in f8ac8a3137.
--
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]
