[GitHub] [airflow] vincbeck commented on a diff in pull request #32819: Handle logout by auth manager

["vincbeck (via GitHub)" <gi...@apache.org>]

vincbeck commented on code in PR #32819:
URL: https://github.com/apache/airflow/pull/32819#discussion_r1273603932


##########
airflow/www/extensions/init_security.py:
##########
@@ -70,5 +69,4 @@ def init_check_user_active(app):
     @app.before_request
     def check_user_active():
         if get_auth_manager().is_logged_in() and not g.user.is_active:
-            logout_user()
-            return redirect(get_auth_manager().get_url_login())
+            return redirect(get_auth_manager().get_url_logout())

Review Comment:
   But this is the right change (at least to me). Explanations: the previous code is actually a copy paste from Flask_appbuilder code, see [here](https://github.com/dpgaspar/Flask-AppBuilder/blob/master/flask_appbuilder/security/views.py#L499). Instead of copying the code from Flask, let's just redirect the user to the logout page and Flask will handle it. Plus, we do not want to expose a method like `logout_user()`in `base_auth_manager`. Some auth manager might not have an api to logout an user but only an url to redirect users to in order to log them out



-- 
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: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org