Add back get_url_logout in auth managers but make it optional

airflow/api_fastapi/auth/managers/base_auth_manager.py

@@ -109,12 +109,13 @@ def get_jwt_token(
     def get_url_login(self, **kwargs) -> str:
         """Return the login page url."""
 
-    def logout(self) -> None:
+    def get_url_logout(self) -> str | None:
         """
-        Logout the user.
+        Return the logout page url.
 
-        This method is called when the user is logging out. By default, it does nothing. Override it to
-        invalidate resources when logging out, such as a session.
+        The user is redirected to this URL when logging out. If None is returned (by default), no redirection
+        is performed. This redirection is usually needed to invalidate resources when logging out, such as a
+        session.
         """
         return None
 

newsfragments/aip-79.significant.rst

@@ -12,6 +12,8 @@ As part of this change the following breaking changes have occurred:
 
   - The property ``security_manager`` has been removed from the interface
 
+  - The method ``get_url_logout`` is now optional
+
   - All these methods have been removed from the interface:
 
     - ``filter_permitted_menu_items``
@@ -20,12 +22,9 @@ As part of this change the following breaking changes have occurred:
     - ``get_user``
     - ``get_user_id``
     - ``is_logged_in``
-    - ``get_url_logout``
     - ``get_api_endpoints``
     - ``register_views``
 
-  - A new optional method ``logout`` has been added to the interface
-
   - All the following method signatures changed to make the parameter ``user`` required (it was optional)
 
     - ``is_authorized_configuration``

providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py

@@ -26,8 +26,7 @@
 import packaging.version
 from connexion import FlaskApi
 from fastapi import FastAPI
-from flask import Blueprint, g, url_for
-from flask_login import logout_user
+from flask import Blueprint, g
 from sqlalchemy import select
 from sqlalchemy.orm import Session, joinedload
 from starlette.middleware.wsgi import WSGIMiddleware
@@ -427,15 +426,9 @@ def get_url_login(self, **kwargs) -> str:
         """Return the login page url."""
         return urljoin(self.apiserver_endpoint, f"{AUTH_MANAGER_FASTAPI_APP_PREFIX}/login/")
 
-    def get_url_logout(self):
+    def get_url_logout(self) -> str | None:
         """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 logout(self) -> None:
-        """Logout the user."""
-        logout_user()
+        return urljoin(self.apiserver_endpoint, f"{AUTH_MANAGER_FASTAPI_APP_PREFIX}/logout/")
 
     def register_views(self) -> None:
         self.security_manager.register_views()

providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py

@@ -28,7 +28,6 @@
 from typing import TYPE_CHECKING, Any
 
 import jwt
-import packaging.version
 from flask import flash, g, has_request_context, session
 from flask_appbuilder import const
 from flask_appbuilder.const import (
@@ -57,10 +56,8 @@
     AuthOAuthView,
     AuthOIDView,
     AuthRemoteUserView,
-    AuthView,
     RegisterUserModelView,
 )
-from flask_appbuilder.views import expose
 from flask_babel import lazy_gettext
 from flask_jwt_extended import JWTManager
 from flask_login import LoginManager
@@ -71,7 +68,6 @@
 from sqlalchemy.orm import joinedload
 from werkzeug.security import check_password_hash, generate_password_hash
 
-from airflow import __version__ as airflow_version
 from airflow.configuration import conf
 from airflow.exceptions import AirflowException
 from airflow.models import DagBag
@@ -135,35 +131,6 @@
 MAX_NUM_DATABASE_USER_SESSIONS = 50000
 
 
-# The following logic patches the logout method within AuthView, so it supports POST method
-# to make CSRF protection effective. It is backward-compatible with Airflow versions <= 2.9.2 as it still
-# allows utilizing the GET method for them.
-# You could remove the patch and configure it when it is supported
-# natively by Flask-AppBuilder (https://github.com/dpgaspar/Flask-AppBuilder/issues/2248)
-if packaging.version.parse(packaging.version.parse(airflow_version).base_version) < packaging.version.parse(
-    "2.10.0"
-):
-    _methods = ["GET", "POST"]
-else:
-    _methods = ["POST"]
-
-
-class _ModifiedAuthView(AuthView):
-    @expose("/logout/", methods=_methods)
-    def logout(self):
-        return super().logout()
-
-
-for auth_view in [
-    AuthDBView,
-    AuthLDAPView,
-    AuthOAuthView,
-    AuthOIDView,
-    AuthRemoteUserView,
-]:
-    auth_view.__bases__ = (_ModifiedAuthView,)
-
-
 class FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
     """
     This security manager overrides the default AirflowSecurityManager security manager.

providers/fab/src/airflow/providers/fab/www/views.py

@@ -72,7 +72,7 @@ def index(self):
             token = get_auth_manager().get_jwt_token(g.user)
             return redirect(urljoin(conf.get("api", "base_url"), f"?token={token}"), code=302)
         else:
-            return super().index()
+            return redirect(conf.get("api", "base_url"), code=302)
 
 
 def show_traceback(error):
@@ -130,3 +130,15 @@ def get_safe_url(url):
 
     # This will ensure we only redirect to the right scheme/netloc
     return redirect_url.geturl()
+
+
+def method_not_allowed(error):
+    """Show Method Not Allowed on screen for any error in the Webserver."""
+    return (
+        render_template(
+            "airflow/error.html",
+            status_code=405,
+            error_message="Received an invalid request.",
+        ),
+        405,
+    )

providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py

@@ -26,7 +26,7 @@
 from flask import Flask, g
 
 from airflow.api_fastapi.app import AUTH_MANAGER_FASTAPI_APP_PREFIX
-from airflow.exceptions import AirflowConfigException, AirflowException
+from airflow.exceptions import AirflowConfigException
 from airflow.providers.fab.www.extensions.init_appbuilder import init_appbuilder
 from airflow.providers.standard.operators.empty import EmptyOperator
 from unit.fab.auth_manager.api_endpoints.api_connexion_utils import create_user, delete_user
@@ -571,24 +571,9 @@ def test_get_url_login(self, auth_manager):
         result = auth_manager.get_url_login()
         assert result == f"http://localhost:8080{AUTH_MANAGER_FASTAPI_APP_PREFIX}/login/"
 
-    @pytest.mark.db_test
-    def test_get_url_logout_when_auth_view_not_defined(self, auth_manager_with_appbuilder):
-        with pytest.raises(AirflowException, match="`auth_view` not defined in the security manager."):
-            auth_manager_with_appbuilder.get_url_logout()
-
-    @pytest.mark.db_test
-    @mock.patch("airflow.providers.fab.auth_manager.fab_auth_manager.url_for")
-    def test_get_url_logout(self, mock_url_for, auth_manager_with_appbuilder):
-        auth_manager_with_appbuilder.security_manager.auth_view = Mock()
-        auth_manager_with_appbuilder.security_manager.auth_view.endpoint = "test_endpoint"
-        auth_manager_with_appbuilder.get_url_logout()
-        mock_url_for.assert_called_once_with("test_endpoint.logout")
-
-    @pytest.mark.db_test
-    @mock.patch("airflow.providers.fab.auth_manager.fab_auth_manager.logout_user")
-    def test_logout(self, mock_logout_user, auth_manager_with_appbuilder):
-        auth_manager_with_appbuilder.logout()
-        mock_logout_user.assert_called_once()
+    def test_get_url_logout(self, auth_manager):
+        result = auth_manager.get_url_logout()
+        assert result == f"http://localhost:8080{AUTH_MANAGER_FASTAPI_APP_PREFIX}/logout/"
 
     @mock.patch.object(FabAuthManager, "_is_authorized", return_value=True)
     def test_get_menu_items(self, _, auth_manager_with_appbuilder, flask_app):

tests/api_fastapi/auth/managers/test_base_auth_manager.py

@@ -159,8 +159,8 @@ def test_get_cli_commands_return_empty_list(self, auth_manager):
     def test_get_fastapi_app_return_none(self, auth_manager):
         assert auth_manager.get_fastapi_app() is None
 
-    def test_logout_return_none(self, auth_manager):
-        assert auth_manager.logout() is None
+    def test_get_url_logout_return_none(self, auth_manager):
+        assert auth_manager.get_url_logout() is None
 
     def test_get_menu_items_return_empty_list(self, auth_manager):
         assert auth_manager.get_menu_items(user=BaseAuthManagerUserTest(name="test")) == []