msyavuz commented on code in PR #45681:
URL: https://github.com/apache/superset/pull/45681#discussion_r4378135192
##########
superset/models/helpers.py:
##########
@@ -4015,7 +4015,9 @@ def get_from_clause(
# Apply RLS filters to virtual dataset SQL to prevent RLS bypass
# For each table referenced in the virtual dataset, apply its RLS
filters
if parsed_script.statements:
- default_schema = self.database.get_default_schema(self.catalog)
+ default_schema = self.schema or self.database.get_default_schema(
Review Comment:
This bypasses RLS on engines that can't switch schema per query
(`supports_dynamic_schema = False`: MSSQL, Oracle, SQLite, and others). On
those engines an unqualified `FROM orders` still runs against the connection's
default schema. Because `self.schema` now takes precedence, `apply_rls` looks
up rules for `<dataset schema>.orders` instead of the table actually queried,
and those rules are silently skipped. Gate this on
`db_engine_spec.supports_dynamic_schema`, or reuse the logic in
`get_default_schema_for_query`.
##########
superset/connectors/sqla/models.py:
##########
@@ -2891,7 +2891,9 @@ class and any keys added via `ExtraCache`.
# For virtual datasets, include RLS predicates in the cache key
if self.is_virtual and self.sql:
- default_schema = self.database.get_default_schema(self.catalog)
+ default_schema = self.schema or self.database.get_default_schema(
Review Comment:
The same mis-scoping applies to the RLS cache key: on engines without
dynamic schema, rules for the real default-schema table are left out of
`extra_cache_keys`. Users with different RLS filters can then share a cached
result.
##########
superset/models/sql_lab.py:
##########
@@ -410,7 +410,7 @@ def get_extra_cache_keys(self, query_obj: QueryObjectDict)
-> list[Hashable]:
collect_rls_predicates_for_sql,
)
- default_schema = self.database.get_default_schema(self.catalog)
+ default_schema = self.schema or
self.database.get_default_schema(self.catalog)
Review Comment:
The same issue affects SQL Lab Explore charts: when the engine can't apply
`Query.schema` at execution time, predicates are collected for the wrong
schema, so the cache key misses the RLS rules that apply.
--
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]
