goingforstudying-ctrl opened a new pull request, #45972: URL: https://github.com/apache/superset/pull/45972
### SUMMARY Ran into #45898 while looking at per-user caching gaps: the column-values cache (the one feeding native-filter dropdowns and the Explore filter popover) builds its key from the query, datasource uid, RLS and `changed_on`, but never anything per user. So on a database with user impersonation — or with `CACHE_QUERY_BY_USER` / the database's `per_user_caching` extra on — an upstream engine that filters rows per user still gets one shared entry per column, and whoever warms it first decides what every other user with the same RLS sees in their filter values. Chart data already handles this through `add_impersonation_cache_key_if_needed`; this change routes the column-values key through the same helper for datasets (`SqlaTable` / SQL Lab `Query`). Semantic views have no `database`, so they partition by the server-authenticated principal under `CACHE_QUERY_BY_USER` instead — same convention as the chart result cache side in #45900, which covers a different cache (this one is the column-values path; the two should compose cleanly). Key bytes are untouched when per-user caching is off, verified by the existing legacy-bytes test in `values_endpoint_test.py` plus a new "same RLS, flags off, same key" case. One thing I'm not 100% sure about: guests on semantic views deliberately keep a shared entry (a guest principal can't tell guest tokens apart, and connection mode runs guests under one upstream account anyway) — I mirrored the chart-cache reasoning there, but flag it if you'd rather key guests too. ### BEFORE & AFTER SCREENSHOTS OR ANIMATED GIF N/A (cache-key behavior) ### TESTING INSTRUCTIONS `tests/unit_tests/security/inner_rls_cache_key_test.py` covers it end to end with the existing fixture family: - flags on, identical RLS, two users → different keys, and the cache never serves one user the other's values (patched the engine to answer per user, like an impersonating upstream) - `per_user_caching` in the database extra partitions without any flag - flags off → same key for both users (guard against always partitioning) - semantic views partition by principal, share for guests and with the flag off ``` pytest tests/unit_tests/security/inner_rls_cache_key_test.py pytest tests/unit_tests/semantic_layers/values_endpoint_test.py tests/unit_tests/datasource/ ``` All pass (178 tests across the three files). ### ADDITIONAL INFORMATION - [x] Has associated issue: #45898 - [ ] 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 - [ ] 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]
