mikebridge opened a new pull request, #43848:
URL: https://github.com/apache/superset/pull/43848
### SUMMARY
Chart-side transplant of the type-aware datasource access pattern from
#43781 (SC-111233), closing SC-119500.
`ChartFilter`'s dataset-access fallback — the only reachable list path —
inner-joined `Slice.datasource_id == SqlaTable.id` with **no `datasource_type`
predicate**, failing in both directions for charts on non-table datasource
types:
- **fail closed**: semantic-view charts were dropped by the inner join, so
users holding the view's `datasource_access` grant (or its parent semantic
layer's, per the layer fallback shipped in #43781) never saw those charts in
the chart list — even though the object gate admits them when opened directly;
- **fail open**: the type-less join could bind a semantic-view chart to an
unrelated `SqlaTable` sharing its numeric id, listing the chart to users
entitled only to the colliding table's database (the same id-collision class as
SC-111089 / #42539, and the exact leak #43781 closed for dashboards).
What this PR does:
- **One shared type-guard predicate** (`table_backed_slice_join` in
`superset/utils/filters.py`): the single authoritative ON-clause consumed by
both `DashboardAccessFilter` and `ChartFilter`, so the id-collision constraint
cannot drift between the filters.
- **`ChartFilter` branch C reshape**: outer, type-guarded joins; access
matches through the perm columns denormalized onto `Slice` by
`set_related_perm`, uniformly across perm-carrying datasource types (no
per-type allowlist; NULL-perm rows can never match). Includes the
semantic-layer grant clause, mirroring the dashboard filter.
- **`_apply_legacy` deleted**: zero callers (`ChartFilter.apply` routes
every request through `_apply_viewers` regardless of `ENABLE_VIEWERS`);
deleting rather than fixing keeps exactly one definition of the access path.
- **Type-aware drill membership** (`Dashboard.has_member_datasource`):
compares `(datasource_type, datasource_id)` pairs — zero queries,
collision-immune by construction. Both drill consumers
(`can_drill_dataset_via_dashboard_access`, `has_drill_access`) use it instead
of the table-shaped `Dashboard.datasources` set (whose export/thumbnail
consumers are untouched). No user-visible drill change today (semantic views
ship with drill disabled); this makes the membership primitive correct before
any provider enables it.
- **Embedded-guest allowlist pinned**: a guest token carrying a `datasets`
allowlist still denies a semantic-view member chart — now asserted by tests as
the *decided* fail-closed semantic (the allowlist is dataset-id space;
resolving other types into it would reintroduce id ambiguity), with a companion
control showing the same token admits the table chart whose id it names.
- **`get_dataset_access_filters` signature**: `*args: Any` becomes named
`*extra_access_clauses` with an explicit `include_all` keyword (deferred-in
from the #43781 review round).
**Stacked on #43781** — this branch is based on its head (`27a1958d14`),
because it consumes that PR's shared fixture, dashboard-filter join shape, and
layer-grant clause. Review the last two commits (`6954a881bb`, `bb95bb664f`);
I'll rebase onto master once #43781 merges.
Reverted-fix control (with `superset/` stashed, the new contract tests flip
— both defect directions demonstrated):
```
FAILED …::test_list_shows_semantic_chart_to_entitled_user
FAILED …::test_list_database_grant_does_not_leak_colliding_semantic_chart
FAILED …::test_list_layer_grant_lists_all_layer_charts
FAILED …::test_list_query_chart_participates_uniformly
4 failed, 5 passed
```
### BEFORE/AFTER SCREENSHOTS OR ANIMATED GIF
N/A — list-visibility change; the contract matrix in
`tests/unit_tests/charts/semantic_view_chart_filter_test.py` is the
behavioural spec.
### TESTING INSTRUCTIONS
```bash
python -m pytest tests/unit_tests/charts tests/unit_tests/dashboards \
tests/unit_tests/subjects tests/unit_tests/security
tests/unit_tests/utils/filters_test.py -q
```
Manual: as a scoped user with no DAR grants the chart list shows no
semantic-view charts; grant `datasource_access` on a semantic view (or its
parent layer) → its charts appear in `/chart/list/`; a user granted only
database access to a database containing a table that shares a semantic view's
numeric id does not see that view's charts; `all_datasource_access` sees
everything.
### ADDITIONAL INFORMATION
- [x] Has associated issue: SC-119500 (follow-up to #43781 / SC-111233)
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Introduces new feature or API
- [x] Removes existing feature or API (dead `ChartFilter._apply_legacy`, no
callers)
🤖 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]
