sadpandajoe opened a new pull request, #44428:
URL: https://github.com/apache/superset/pull/44428
### SUMMARY
Introduced by d5c0506faa ("fix: OAuth2 in async DBs", #29461), which wrapped
`get_sql_results` in `app.test_request_context()` so Celery workers — which
have
no originating request — can build the OAuth2 redirect URI.
The wrapper is unconditional, but SQL Lab's synchronous executor doesn't go
through Celery: it calls the task object directly
(`SynchronousSqlJsonExecutor._get_sql_results`), from inside the already
authenticated request. So a plain "Run" in SQL Lab pushes a *second*,
fabricated
request context inside the real one. Flask binds `session` to the request
context
and `g` to the app context, so `g.user` keeps working while `flask.session`
is
silently replaced with a brand-new empty one — which is why the reporter saw
an
entirely blank session rather than a partially populated one, and why nothing
else in SQL Lab noticed.
With `RLS_IN_SQLLAB` enabled, `execute_sql_statements` reaches `apply_rls` →
`get_predicates_for_table` → `SqlaTable.get_sqla_row_level_filters()`, which
Jinja-renders each RLS filter's clause. A custom `JINJA_CONTEXT_ADDONS` macro
that reads `flask.session` therefore runs against the empty session and
returns
nothing, so the RLS predicate silently collapses.
This fabricates the request context only when there isn't already one. The
Celery
path is unaffected and still gets its context; the synchronous path now
reuses the
real request, so session-dependent Jinja macros behave the same in SQL Lab
as they
already do when rendering a chart.
Note this fixes the synchronous path, which is the default. A deployment
running
SQL Lab asynchronously still has no browser session inside the worker — that
is an
architectural limitation of async execution, not something this change
addresses.
### TESTING INSTRUCTIONS
1. Set `RLS_IN_SQLLAB: True` in `FEATURE_FLAGS`.
2. Add a Jinja macro to `JINJA_CONTEXT_ADDONS` in `superset_config.py` that
reads
the Flask session, e.g.:
```python
from flask import session
JINJA_CONTEXT_ADDONS = {"rls": lambda: session.get("rls", "(NULL)")}
```
Populate `session["rls"]` during login (or temporarily hardcode a write
to it
in a `before_request` hook to test).
3. Create an RLS rule on a dataset whose clause uses the macro, e.g.
`column_name IN {{ rls() }}`.
4. Open a chart on that dataset, and from its menu choose "View query" /
open it
in SQL Lab. Run the query in SQL Lab (leave "Run async" off).
5. Before this change, the rendered RLS predicate is empty — the macro sees
an
empty session. After it, the predicate renders with the session values,
matching
what the chart already produced.
### ADDITIONAL INFORMATION
- [x] Has associated issue: Fixes #37695
- [ ] Required feature flags:
- [ ] Changes UI
- [ ] Includes DB Migration (follow approval process in
[SIP-59](https://github.com/apache/superset/issues/13351))
- [ ] Migration is atomic, supports rollback & is backwards-compatible
- [ ] Confirm DB migration upgrade and downgrade tested
- [ ] Runtime estimates and downtime expectations provided
- [ ] Introduces new feature or API
- [ ] Removes existing feature or API
🤖 Generated with [Claude Code](https://claude.com/claude-code)
--
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]
