Ujjwaljain16 commented on issue #33346:
URL: https://github.com/apache/superset/issues/33346#issuecomment-5582144712

   I looked into this in detail against current `master` (not #43090, which was 
closed without merging).
   
   the originally reported symptom is already fixed on master
   Building a chart directly from an unsaved SQL Lab query no longer bypasses 
RLS i verified this with a live reproduction (RLS-restricted user vs. 
unrestricted user, on real Postgres): the restricted user's chart query 
correctly gets the underlying table's RLS predicate injected, the unrestricted 
user's doesn't.
   
   What actually fixed it: not #43090 (never merged). The fix is in 
`ExploreMixin.get_from_clause` (shared by `SqlaTable` virtual datasets and the 
SQL-Lab-ad-hoc `Query` datasource alike)  it parses the chart's raw SQL for 
referenced tables and injects those tables' RLS predicates at 
query-construction time, added generically for virtual datasets in #36061 and 
hardened to fail closed (aborts instead of running unfiltered on a parse error) 
in #42929. Neither commit references this issue, which is presumably why it's 
stayed open despite being resolved. There was no regression test covering the 
SQL-Lab-ad-hoc path specifically so i added one.
   
   One adjacent gap i found while verifying this, distinct from the original 
report: 
   the query-result *cache key* for a SQL-Lab-ad-hoc chart never varied by the 
viewer's RLS (unlike every other datasource type), so a shared Explore 
permalink viewed by two differently-RLS-scoped users could, with query-result 
caching enabled, serve one viewer's cached result to the other. Filed as 
[[#43985](https://github.com/apache/superset/pull/43985)], separate from this 
issue since it's a caching-isolation gap, not the query-execution bypass this 
issue describes.
   
   Happy to add anything here that would help close this out, or if a 
maintainer wants the regression test coverage broken out as its own PR against 
this issue specifically.


-- 
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]

Reply via email to