Handle logout by auth manager (#32819)

* Handle logout by auth manager

airflow/auth/managers/base_auth_manager.py

@@ -41,32 +41,30 @@ def __init__(self):
     @abstractmethod
     def get_user_name(self) -> str:
         """Return the username associated to the user in session."""
-        ...
 
     @abstractmethod
     def get_user(self) -> BaseUser:
         """Return the user associated to the user in session."""
-        ...
 
     @abstractmethod
     def get_user_id(self) -> str:
         """Return the user ID associated to the user in session."""
-        ...
 
     @abstractmethod
     def is_logged_in(self) -> bool:
         """Return whether the user is logged in."""
-        ...
 
     @abstractmethod
     def get_url_login(self, **kwargs) -> str:
         """Return the login page url."""
-        ...
+
+    @abstractmethod
+    def get_url_logout(self) -> str:
+        """Return the logout page url."""
 
     @abstractmethod
     def get_url_user_profile(self) -> str | None:
         """Return the url to a page displaying info about the current user."""
-        ...
 
     def get_security_manager_override_class(self) -> type:
         """

airflow/auth/managers/fab/fab_auth_manager.py

@@ -70,6 +70,12 @@ def get_url_login(self, **kwargs) -> str:
         else:
             return url_for(f"{self.security_manager.auth_view.endpoint}.login")
 
+    def get_url_logout(self):
+        """Return the logout page url."""
+        if not self.security_manager.auth_view:
+            raise AirflowException("`auth_view` not defined in the security manager.")
+        return url_for(f"{self.security_manager.auth_view.endpoint}.logout")
+
     def get_url_user_profile(self) -> str | None:
         """Return the url to a page displaying info about the current user."""
         if not self.security_manager.user_view:

airflow/www/auth.py

@@ -54,7 +54,7 @@ def decorated(*args, **kwargs):
                         hostname=get_hostname()
                         if conf.getboolean("webserver", "EXPOSE_HOSTNAME")
                         else "redact",
-                        logout_url=appbuilder.get_url_for_logout,
+                        logout_url=get_auth_manager().get_url_logout(),
                     ),
                     403,
                 )

airflow/www/extensions/init_appbuilder.py

@@ -588,10 +588,6 @@ def security_converge(self, dry=False) -> dict:
     def get_url_for_login_with(self, next_url: str | None = None) -> str:
         return get_auth_manager().get_url_login(next_url=next_url)
 
-    @property
-    def get_url_for_logout(self):
-        return url_for(f"{self.sm.auth_view.endpoint}.logout")
-
     @property
     def get_url_for_index(self):
         return url_for(f"{self.indexview.endpoint}.{self.indexview.default_view}")

airflow/www/extensions/init_security.py

@@ -20,7 +20,6 @@
 from importlib import import_module
 
 from flask import g, redirect
-from flask_login import logout_user
 
 from airflow.configuration import conf
 from airflow.exceptions import AirflowConfigException, AirflowException
@@ -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())

airflow/www/templates/appbuilder/navbar_right.html

@@ -78,7 +78,7 @@
         <li><a href="{{user_profile_url}}"><span class="material-icons">account_circle</span>{{_("Your Profile")}}</a></li>
         <li role="separator" class="divider"></li>
       {% endif %}
-      <li><a href="{{appbuilder.get_url_for_logout}}"><span class="material-icons">exit_to_app</span>{{_("Log Out")}}</a></li>
+      <li><a href="{{auth_manager.get_url_logout()}}"><span class="material-icons">exit_to_app</span>{{_("Log Out")}}</a></li>
     </ul>
   </li>
 {% else %}

tests/auth/managers/fab/test_fab_auth_manager.py

@@ -99,6 +99,17 @@ def test_get_url_login_with_next(self, mock_url_for, auth_manager):
         auth_manager.get_url_login(next_url="next_url")
         mock_url_for.assert_called_once_with("test_endpoint.login", next="next_url")
 
+    def test_get_url_logout_when_auth_view_not_defined(self, auth_manager):
+        with pytest.raises(AirflowException, match="`auth_view` not defined in the security manager."):
+            auth_manager.get_url_logout()
+
+    @mock.patch("airflow.auth.managers.fab.fab_auth_manager.url_for")
+    def test_get_url_logout(self, mock_url_for, auth_manager):
+        auth_manager.security_manager.auth_view = Mock()
+        auth_manager.security_manager.auth_view.endpoint = "test_endpoint"
+        auth_manager.get_url_logout()
+        mock_url_for.assert_called_once_with("test_endpoint.logout")
+
     def test_get_url_user_profile_when_auth_view_not_defined(self, auth_manager):
         assert auth_manager.get_url_user_profile() is None
 

tests/www/views/test_session.py

@@ -95,10 +95,4 @@ def test_check_active_user(app, user_client):
     user.active = False
     resp = user_client.get("/home")
     assert resp.status_code == 302
-    assert "/login" in resp.headers.get("Location")
-
-    # And they were logged out
-    user.active = True
-    resp = user_client.get("/home")
-    assert resp.status_code == 302
-    assert "/login" in resp.headers.get("Location")
+    assert "/logout" in resp.headers.get("Location")