villebro opened a new pull request, #44215: URL: https://github.com/apache/superset/pull/44215
### SUMMARY Fixes #44133. `VIEWER_PROMISCUOUS_MODE` (with `ENABLE_VIEWERS`) is meant to let a viewer who has access to a *published* dashboard render that dashboard's charts and datasets without needing explicit per-chart / per-datasource grants, mirroring the legacy `DASHBOARD_RBAC` behavior. In practice, datasets rendered but charts hung on the loading spinner: a viewer had to be granted access to every chart individually. The enforcement layer already honored the flag everywhere it mattered for *querying* data: the datasource branch of `raise_for_access` grants a promiscuous viewer datasource access when a chart's `form_data` carries the `dashboardId` of a dashboard they can view, and both drill (`get_drill_info`) and `samples` use `security_manager.can_drill_dataset_via_dashboard_access`. The gap was upstream of all of that: to issue a chart data query, the frontend first calls `GET /dashboard/<id>/charts` to obtain each chart's `form_data`. That endpoint's serializer stripped `form_data` based purely on `can_access_chart`, ignoring dashboard access, so a promiscuous viewer received a chart with no `form_data`, never built a query context, and hung on the loading screen even though the query itself would have been authorized. `GET /dashboard/<id>/datasets` had the same narrowing. This wires `can_drill_dataset_via_dashboard_access` into the two dashboard sub-resource serializers (`_serialize_dashboard_chart` and `_serialize_dashboard_dataset`), mirroring the pattern already used in `datasets/api.py` and `views/datasource/views.py`. The two `DashboardDAO` helpers now also return the resolved `Dashboard` so the serializers can pass it to the check (single fetch, no extra query). Because `can_drill_dataset_via_dashboard_access` already enforces the `ENABLE_VIEWERS` + `VIEWER_PROMISCUOUS_MODE` + `dashboard.published` gate (or the `EMBEDDED_SUPERSET` guest gate) and restricts to the dashboard's own datasources, behavior is unchanged when the flags are off, when the dashboard is unpublished, and for datasets/charts not belonging to the dashboard. The scope is deliberately limited to the `/charts` and `/datasets` sub-resource endpoints, which return the dashboard's own members and only need the render payload restored. The main `GET /<id>` endpoint's `charts` field and the general chart-list view are intentionally left unchanged, so implicitly-accessible charts stay hidden from those listings — the fix restores the render payload without broadening chart visibility. ### TESTING INSTRUCTIONS With `ENABLE_VIEWERS` on and `VIEWER_PROMISCUOUS_MODE = True`, grant a viewer role access to a published dashboard (via the Editors/Viewers picker) but no explicit chart or dataset grants. As that viewer, open the dashboard — charts now render instead of hanging on the loading spinner. `GET /api/v1/dashboard/<id>/charts` returns `form_data` and `GET /api/v1/dashboard/<id>/datasets` returns full metadata. Unpublish the dashboard, or turn `VIEWER_PROMISCUOUS_MODE` off, and the metadata is narrowed again. Automated coverage: - `tests/integration_tests/dashboards/api_tests.py` — new cases asserting `/charts` retains `form_data` and `/datasets` retains full metadata when access is granted only via `can_drill_dataset_via_dashboard_access`, alongside the existing narrowing tests. - `tests/unit_tests/subjects/test_raise_for_access.py` — new unit tests for `can_drill_dataset_via_dashboard_access` covering the viewer/promiscuous grant, unpublished-dashboard denial, unrelated-dataset denial, and promiscuous-off denial. ### ADDITIONAL INFORMATION - [x] Has associated issue: #44133 - [x] Required feature flags: `ENABLE_VIEWERS` (with `VIEWER_PROMISCUOUS_MODE`) - [ ] Changes UI - [ ] Includes DB Migration (follow approval process in [SIP-59](https://github.com/apache/superset/issues/13351)) - [ ] Migration is atomic, supports rollback & is backwards-compatible - [ ] Confirm DB migration upgrade and downgrade tested - [ ] Runtime estimates and downtime expectations provided - [ ] Introduces new feature or API - [ ] Removes existing feature or API -- 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]
