mikebridge commented on code in PR #45538:
URL: https://github.com/apache/superset/pull/45538#discussion_r4391583712
##########
superset/daos/dashboard.py:
##########
@@ -305,6 +305,59 @@ def _existing_chart_ids(chart_ids: set[int]) -> set[int]:
return {row[0] for row in rows}
+def _member_chart_ids(dashboard_id: int | None) -> set[int]:
+ """IDs of *dashboard_id*'s member charts, archived ones included.
+
+ Read from ``dashboard_slices`` rather than ``Dashboard.slices``, whose load
+ hides archived members.
+ """
+ if dashboard_id is None:
+ return set()
+ return set(
+ db.session.scalars(
+ select(dashboard_slices.c.slice_id).where(
+ dashboard_slices.c.dashboard_id == dashboard_id
+ )
+ )
+ )
+
+
+def _raise_for_unreadable_new_charts(
+ charts: list[Slice], allowed_chart_ids: set[int]
+) -> None:
+ """Refuse a layout that links a chart the caller cannot read.
+
+ A chart already allowed (an existing member, or a copy's source member)
+ is accepted as is, so a save never fails over a chart it does not add.
+ Dashboard membership feeds the datasource grant for dashboard viewers, so
+ a layout must not be a way to attach a chart the caller has no access to.
+ """
+ candidates: list[Slice] = [
+ chart for chart in charts if chart.id not in allowed_chart_ids
+ ]
+ if candidates and not security_manager.is_admin():
+ # The access check reads each chart's editors and viewers; load them
+ # in two queries rather than two lazy loads per chart.
+ db.session.query(Slice).options(
+ lazyload("*"),
+ selectinload(Slice.editors),
+ selectinload(Slice.viewers),
+ ).filter(Slice.id.in_([chart.id for chart in candidates])).all()
+ denied: list[int] = sorted(
+ chart.id for chart in candidates if not
security_manager.can_access_chart(chart)
+ )
+ if denied:
+ raise DashboardLayoutInvalidError(
+ exceptions=[
Review Comment:
Agreed, added in 128be9fb4d. `UPDATING.md` now has a "Dashboard layout chart
access" entry next to the copy-layout one. It covers:
- the endpoints that can now return 422: dashboard `PUT`, `POST` and the
copy endpoint;
- the exemption: charts already on the dashboard (archived ones included)
and a copy's source charts aren't checked;
- the recovery: remove the tile from the layout, or get access to the chart,
and save again.
--
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]
