aminghadersohi commented on code in PR #45681:
URL: https://github.com/apache/superset/pull/45681#discussion_r4379941097
##########
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:
Correction to my reply above: this is still open at
5c707e415e9db47ddaa89e40503fbe656c882feb.
The virtual dataset SQL path (injection, the fail-closed retry, and both
cache-key paths) now resolves unqualified tables against the connection schema
on non-dynamic engines. Adhoc SQL expressions do not. `validate_adhoc_subquery`
still receives the dataset schema (`context.schema` / `schema or ""` from
`self.schema`) via `_process_select_expression`,
`_process_metric_sql_expression` and `validate_stored_expression_at_query_time`
in `superset/models/helpers.py`, plus the `validate_adhoc_subquery` call in
`superset/connectors/sqla/models.py`. A column, metric, WHERE or HAVING
subquery that reads the connection-default table therefore runs without that
table's RLS predicate. My sibling list above left this path out.
Repro, using the PR's `static_schema_datasource` fixture (SQLite, dataset
schema `not_the_default`, rule `col1 = 1` on `main.physical`), with
`ALLOW_ADHOC_SUBQUERY` on and the adhoc column `(SELECT MAX(col1) FROM
physical)`:
- At 5c707e415e9db47ddaa89e40503fbe656c882feb, for both SqlaTable and Query:
the outer query is filtered to `col1 = 1`, but the subquery returns 2 instead
of 1. The generated SQL is `(SELECT MAX(col1) FROM physical)` with no predicate.
- With the dataset schema set to `main`, the predicate is injected and the
subquery returns 1.
- Master shows the same behavior, so this PR did not introduce it. Note that
the adhoc denylist resolver (`_resolve_denylist_schema`) was changed for this
case, but adhoc RLS scoping was not.
Remaining: adhoc-expression subquery RLS on non-dynamic-schema engines, for
SqlaTable and Query. There is no regression test for it yet.
--
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]
