diff --git a/airflow/api_fastapi/auth/managers/base_auth_manager.py b/airflow/api_fastapi/auth/managers/base_auth_manager.py index 3a3f71e00f577..60084c6d8de05 100644 --- a/airflow/api_fastapi/auth/managers/base_auth_manager.py +++ b/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 diff --git a/newsfragments/aip-79.significant.rst b/newsfragments/aip-79.significant.rst index b22290a45a083..88a419d7a32aa 100644 --- a/newsfragments/aip-79.significant.rst +++ b/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`` diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py b/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py index a793a80d0e637..e47b663741959 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/fab_auth_manager.py +++ b/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() diff --git a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py b/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py index 97eca2a858077..bf8ca6499007a 100644 --- a/providers/fab/src/airflow/providers/fab/auth_manager/security_manager/override.py +++ b/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. diff --git a/providers/fab/src/airflow/providers/fab/www/views.py b/providers/fab/src/airflow/providers/fab/www/views.py index 22cd5def12002..498d6bbbc64dc 100644 --- a/providers/fab/src/airflow/providers/fab/www/views.py +++ b/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, + ) diff --git a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py b/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py index ce7d58e20ec6d..2809c87b1325e 100644 --- a/providers/fab/tests/unit/fab/auth_manager/test_fab_auth_manager.py +++ b/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): diff --git a/tests/api_fastapi/auth/managers/test_base_auth_manager.py b/tests/api_fastapi/auth/managers/test_base_auth_manager.py index bc283de4648a3..7056c586ad554 100644 --- a/tests/api_fastapi/auth/managers/test_base_auth_manager.py +++ b/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")) == []