msyavuz commented on code in PR #44650:
URL: https://github.com/apache/superset/pull/44650#discussion_r4104555400
##########
superset/utils/rls.py:
##########
@@ -83,24 +85,39 @@ def apply_rls(
# collect all RLS predicates for all tables in the query
default_catalog = database.get_default_catalog()
- predicates: dict[Table, list[Any]] = {}
- for table in parsed_statement.tables:
- table = table.qualify(catalog=catalog, schema=schema)
- predicates[table] = [
- parsed_statement.parse_predicate(predicate)
- for predicate in get_predicates_for_table(
- table,
- database,
- default_catalog,
- exclude_dataset_id=exclude_dataset_id,
- include_global_guest_rls=include_global_guest_rls,
- )
- if predicate
- ]
-
- has_predicates = any(predicates.values())
- parsed_statement.apply_rls(catalog, schema, predicates, method)
- return has_predicates
+
+ def collect_predicates(include_global: bool) -> dict[Table, list[Any]]:
+ predicates: dict[Table, list[Any]] = {}
+ for table in parsed_statement.tables:
+ table = table.qualify(catalog=catalog, schema=schema)
+ predicates[table] = [
+ parsed_statement.parse_predicate(predicate)
+ for predicate in get_predicates_for_table(
+ table,
+ database,
+ default_catalog,
+ exclude_dataset_id=exclude_dataset_id,
+ include_global_guest_rls=include_global,
+ )
+ if predicate
+ ]
+ return predicates
+
+ predicates = collect_predicates(include_global_guest_rls)
+ # The outer query only constrains the rows that reach it, so a table read
+ # inside a sub-query still gets the global guest rules left to the outer
query.
+ # Only a guest token carries such rules, so other users skip the second
lookup.
+ subquery_predicates = (
+ collect_predicates(True)
Review Comment:
Won't this break guest queries on virtual datasets whose sub-query reads a
dataset without the global rule's column? e.g. `SELECT a.*, (SELECT name FROM
lookup WHERE lookup.id = a.lid) FROM a` with global `org_id = 1` and no
`org_id` on `lookup` used to run, and now fails at execution on AS_PREDICATE
engines (after injection, so the fail-closed fallback doesn't catch it). Same
breakage the PR cites for skipping joins.
--
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]
