Usiel opened a new pull request, #22462:
URL: https://github.com/apache/superset/pull/22462

   ### SUMMARY
   
   Due to a breaking change in Flask-Login 
(https://github.com/maxcountryman/flask-login/pull/378) the code for logging in 
an `AnonymousUser` breaks for dashboard embedding. 
   
   Unfortunately, Flask-Login not only renames the method we need, but also 
makes it quasi-private. We can switch to a different public util function 
Flask-Login offers since at least version 0.3.0. In all versions I checked it 
essentially executes the same steps as `reload_user(...)` did (it additionally 
signals the login event internally, which shouldn't cause issues).
   
   Fixes https://github.com/apache/superset/issues/21987
   
   ### TESTING INSTRUCTIONS
   
   For any Flask-Login>=0.3.0,<0.7.0:
   1. Enable `EMBEDDED_SUPERSET` in `superset/config.py`
   2. Run Superset locally
   3. Go to any dashboard, get a embed UUID (see 
https://github.com/apache/superset/pull/19364 for detailed instructions)
   4. Execute the following
   ```
   curl "http://localhost:8088/embedded/<embed_uuid>"
   ```
   **Expected**: 200 OK with some valid HTML
   (Before we would fail with a 500 for any Flask-Login>=0.5.0)
   
   ### ADDITIONAL INFORMATION
   <!--- Check any relevant boxes with "x" -->
   <!--- HINT: Include "Fixes #nnn" if you are fixing an existing issue -->
   - [x] Has associated issue: https://github.com/apache/superset/issues/21987
   - [x] Required feature flags: EMBEDDED_SUPERSET
   - [ ] 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
   
   cc @suddjian: Thanks for building this! Tagging you for better visibility 
and in case I missed anything. Btw, I noticed that `g.user` and the `user` on 
the `ctx` is actually set to an `AnonymousUser` instance before doing anything 
in the endpoint; I believe that's some Flask-Login fallback logic. I'm assuming 
there is some circumstance, some case, where this might not happen and that is 
why we added the bit of code previously? That's why I propose to replace it 
instead of removing it in this PR.


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