mikebridge opened a new pull request, #43781: URL: https://github.com/apache/superset/pull/43781
### SUMMARY Authorization for dashboards and charts resolved member-chart datasources through table-pinned relationships, with two opposite failures for charts on non-table datasources (semantic views): **Fail open (the reported bug):** `raise_for_access`'s dashboard branch falls back to datasource-based access when a dashboard has no explicit viewers, and treated an empty `Dashboard.datasources` set as "nothing to check → allow". That set is built from `Slice.datasource`, whose relationship joins on `datasource_type == 'table'`, so a dashboard composed **solely of semantic-view charts** presented an empty set and opened for **any authenticated user** — exposing the dashboard shell (title, layout/tabs, chart names, native-filter defaults) to scoped users with no `datasource_access` grant. An equivalent regular-dataset dashboard was correctly denied (403/404). Chart data was never exposed (the query-context branch still denied per chart), which is how QA noticed the inconsistency: the semantic dashboard rendered read-only with per-chart access-error cards while the regular one 404'd. **Fail closed (two siblings, same root cause):** the dashboard list filter's dataset fallback inner-joins `Slice.datasource_id == SqlaTable.id` with no type predicate, so semantic-view charts were dropped and users **with** the grant never saw those dashboards listed (they could still open them by URL) — and the type-less join could bind a semantic-view chart to an unrelated table sharing its numeric id. The standalone chart branch (`chart.datasource and can_access_datasource(...)`) likewise denied entitled semantic-view users. The fix is one mechanism: a `Slice.resolved_datasource` property that resolves the chart's datasource across datasource types via `DatasourceDAO` (semantic views already carry `perm` strings and participate in data-access grants), used by both `raise_for_access` branches — a chart whose datasource cannot be resolved counts as **inaccessible, never as absent**, while dashboards with no charts remain accessible (existing pinned behaviour) — plus a type-constrained outer join in `DashboardAccessFilter` so non-table charts survive into the perm-based access clauses (`Slice.perm` is already denormalized across types by `set_related_perm`). Behaviour changes beyond the semantic-view cases: a dashboard whose charts' datasource rows have all been hard-deleted is now denied by the dataset fallback instead of allowed (fail-closed on dangling references). ### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF N/A — authorization behaviour; see TESTING INSTRUCTIONS. ### TESTING INSTRUCTIONS 1. `pytest tests/unit_tests/dashboards/semantic_view_access_test.py` — new regression suite covering the resolver, both `raise_for_access` branches (deny without grant / allow with grant / dangling / empty dashboard), and the list filter (entitled visibility, no-grant emptiness, id-collision control). Reverting the `superset/` changes makes 8 of the 15 fail (fail-open, fail-closed, collision, and resolver cases). 2. `pytest tests/unit_tests/subjects/test_raise_for_access.py` — existing gate tests, updated to build dashboards with member slices. 3. Manual: with `SEMANTIC_LAYERS` enabled, give a user a Gamma-like role with no data grants, share a dashboard containing only semantic-view charts, and open its URL as that user → access is now denied, matching a regular-dataset dashboard. Grant the user `datasource_access` on the semantic view → the dashboard opens and appears in the Dashboards list. ### ADDITIONAL INFORMATION - [x] Has associated issue: SC-111233 (Preset Shortcut) - [ ] Required feature flags: - [ ] 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 🤖 Generated with [Claude Code](https://claude.com/claude-code) https://claude.ai/code/session_01JRLEJS4mUqKBoPjSjviKUW -- 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]
