This is an automated email from the ASF dual-hosted git repository.
vincbeck pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/main by this push:
new bb213a30692 Honor next when an authenticated user reaches the FAB
login page (#75665)
bb213a30692 is described below
commit bb213a306922ba527b1c7677f3a66ba269732abf
Author: Mike Barry <[email protected]>
AuthorDate: Mon Oct 26 11:20:29 2026 -0400
Honor next when an authenticated user reaches the FAB login page (#75665)
---
.../fab/auth_manager/security_manager/override.py | 7 +-
.../providers/fab/auth_manager/views/auth_login.py | 67 ++++++++++
.../providers/fab/auth_manager/views/auth_oauth.py | 9 ++
.../unit/fab/auth_manager/views/test_auth_login.py | 146 +++++++++++++++++++++
.../tests/unit/fab/www/views/test_views_login.py | 90 +++++++++++++
5 files changed, 315 insertions(+), 4 deletions(-)
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 8f193335136..40d00b05293 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
@@ -53,8 +53,6 @@ from flask_appbuilder.security.registerviews import (
RegisterUserOAuthView,
)
from flask_appbuilder.security.views import (
- AuthDBView,
- AuthLDAPView,
AuthRemoteUserView,
AuthView,
RegisterUserModelView,
@@ -87,6 +85,7 @@ from airflow.providers.fab.auth_manager.models import (
)
from airflow.providers.fab.auth_manager.models.anonymous_user import
AnonymousUser
from airflow.providers.fab.auth_manager.security_manager.constants import
EXISTING_ROLES
+from airflow.providers.fab.auth_manager.views.auth_login import
CustomAuthDBView, CustomAuthLDAPView
from airflow.providers.fab.auth_manager.views.auth_oauth import
CustomAuthOAuthView
from airflow.providers.fab.auth_manager.views.permissions import (
ActionModelView,
@@ -213,9 +212,9 @@ class
FabAirflowSecurityManagerOverride(AirflowSecurityManagerV2):
permission_model = Permission
""" Views """
- authdbview = AuthDBView
+ authdbview = CustomAuthDBView
""" Override if you want your own Authentication DB view """
- authldapview = AuthLDAPView
+ authldapview = CustomAuthLDAPView
""" Override if you want your own Authentication LDAP view """
authoauthview = CustomAuthOAuthView
""" Custom Authentication OAuth view with session commit fix for #57981 """
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_login.py
b/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_login.py
new file mode 100644
index 00000000000..9dc2bba91cf
--- /dev/null
+++ b/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_login.py
@@ -0,0 +1,67 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+"""Login views that honor ``next`` when the user is already authenticated."""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+
+from flask import g, request
+from flask_appbuilder import expose
+from flask_appbuilder.security.decorators import no_cache
+from flask_appbuilder.security.views import AuthDBView, AuthLDAPView
+
+from airflow.providers.fab.www.views import get_safe_url, redirect
+
+if TYPE_CHECKING:
+ from werkzeug.wrappers import Response
+
+
+def redirect_if_authenticated() -> Response | None:
+ """
+ Redirect an already-authenticated user to the requested ``next`` URL.
+
+ The UI sends the user back to the login page when its API token has
expired, even though the
+ Flask session is still valid. Flask-AppBuilder's login views then redirect
to the index page and
+ discard ``next``, so the user loses the page they were opening.
``redirect`` sets a fresh API
+ token cookie, so the user lands on the requested page authenticated.
+ """
+ if g.user is not None and g.user.is_authenticated:
+ return redirect(get_safe_url(request.args.get("next")))
+ return None
+
+
+class CustomAuthDBView(AuthDBView):
+ """Database login view that honors ``next`` for already-authenticated
users."""
+
+ @expose("/login/", methods=["GET", "POST"])
+ @no_cache
+ def login(self):
+ if (response := redirect_if_authenticated()) is not None:
+ return response
+ return super().login()
+
+
+class CustomAuthLDAPView(AuthLDAPView):
+ """LDAP login view that honors ``next`` for already-authenticated users."""
+
+ @expose("/login/", methods=["GET", "POST"])
+ @no_cache
+ def login(self):
+ if (response := redirect_if_authenticated()) is not None:
+ return response
+ return super().login()
diff --git
a/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_oauth.py
b/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_oauth.py
index a5d4e520d07..e9140152ff8 100644
--- a/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_oauth.py
+++ b/providers/fab/src/airflow/providers/fab/auth_manager/views/auth_oauth.py
@@ -26,6 +26,7 @@ from flask_appbuilder import expose
from flask_appbuilder.security.views import AuthOAuthView
from airflow.providers.common.compat.sdk import conf
+from airflow.providers.fab.auth_manager.views.auth_login import
redirect_if_authenticated
log = logging.getLogger(__name__)
@@ -38,6 +39,14 @@ class CustomAuthOAuthView(AuthOAuthView):
the Flask session is not yet committed when the redirect response is sent.
"""
+ @expose("/login/")
+ @expose("/login/<provider>")
+ def login(self, provider=None):
+ """Redirect already-authenticated users to ``next`` instead of the
index page."""
+ if (response := redirect_if_authenticated()) is not None:
+ return response
+ return super().login(provider)
+
@expose("/oauth-authorized/<provider>")
def oauth_authorized(self, provider):
"""
diff --git a/providers/fab/tests/unit/fab/auth_manager/views/test_auth_login.py
b/providers/fab/tests/unit/fab/auth_manager/views/test_auth_login.py
new file mode 100644
index 00000000000..d6cdfa999a3
--- /dev/null
+++ b/providers/fab/tests/unit/fab/auth_manager/views/test_auth_login.py
@@ -0,0 +1,146 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+from unittest import mock
+
+import pytest
+from flask import Flask, Response, g
+from flask_appbuilder.security.views import AuthDBView, AuthLDAPView,
AuthOAuthView
+
+from airflow.providers.fab.auth_manager.views import auth_login
+from airflow.providers.fab.auth_manager.views.auth_login import (
+ CustomAuthDBView,
+ CustomAuthLDAPView,
+ redirect_if_authenticated,
+)
+from airflow.providers.fab.auth_manager.views.auth_oauth import
CustomAuthOAuthView
+
+MODULE = "airflow.providers.fab.auth_manager.views.auth_login"
+
+
[email protected]
+def flask_app():
+ return Flask(__name__)
+
+
[email protected]
+def mock_get_safe_url():
+ with mock.patch(f"{MODULE}.get_safe_url", side_effect=lambda url:
f"safe:{url}") as get_safe_url:
+ yield get_safe_url
+
+
[email protected]
+def mock_redirect():
+ with mock.patch(f"{MODULE}.redirect") as redirect:
+ yield redirect
+
+
+def _set_user(authenticated: bool | None) -> None:
+ g.user = None if authenticated is None else
mock.Mock(is_authenticated=authenticated)
+
+
+class TestRedirectIfAuthenticated:
+ def test_redirects_authenticated_user_to_next(self, flask_app,
mock_get_safe_url, mock_redirect):
+ with flask_app.test_request_context("/login/?next=/dags/my_dag"):
+ _set_user(authenticated=True)
+ response = redirect_if_authenticated()
+
+ mock_get_safe_url.assert_called_once_with("/dags/my_dag")
+ mock_redirect.assert_called_once_with("safe:/dags/my_dag")
+ assert response is mock_redirect.return_value
+
+ def test_falls_back_to_index_without_next(self, flask_app,
mock_get_safe_url, mock_redirect):
+ with flask_app.test_request_context("/login/"):
+ _set_user(authenticated=True)
+ redirect_if_authenticated()
+
+ mock_get_safe_url.assert_called_once_with(None)
+ mock_redirect.assert_called_once_with("safe:None")
+
+ @pytest.mark.parametrize("authenticated", [None, False])
+ def test_does_nothing_for_anonymous_user(
+ self, flask_app, authenticated, mock_get_safe_url, mock_redirect
+ ):
+ with flask_app.test_request_context("/login/?next=/dags/my_dag"):
+ _set_user(authenticated=authenticated)
+ assert redirect_if_authenticated() is None
+
+ mock_redirect.assert_not_called()
+
+
+class TestCustomLoginViews:
+ @pytest.mark.parametrize(
+ ("view_class", "routes"),
+ [
+ (CustomAuthDBView, {"/login/"}),
+ (CustomAuthLDAPView, {"/login/"}),
+ (CustomAuthOAuthView, {"/login/", "/login/<provider>"}),
+ ],
+ )
+ def test_login_routes_are_exposed(self, view_class, routes):
+ assert {route for route, _methods in view_class.login._urls} == routes
+
+ @pytest.mark.parametrize(
+ ("view_class", "parent_class", "args"),
+ [
+ (CustomAuthDBView, AuthDBView, ()),
+ (CustomAuthLDAPView, AuthLDAPView, ()),
+ (CustomAuthOAuthView, AuthOAuthView, ("azure",)),
+ ],
+ )
+ def test_authenticated_login_redirects_to_next(self, flask_app,
view_class, parent_class, args):
+ view = view_class()
+ redirect_response = Response(status=302, headers={"Location":
"/dags/my_dag"})
+ with (
+ flask_app.test_request_context("/login/?next=/dags/my_dag"),
+ mock.patch(f"{view_class.__module__}.redirect_if_authenticated",
return_value=redirect_response),
+ mock.patch.object(parent_class, "login") as parent_login,
+ ):
+ response = view.login(*args)
+
+ assert response.status_code == 302
+ assert response.location == "/dags/my_dag"
+ parent_login.assert_not_called()
+
+ @pytest.mark.parametrize(
+ ("view_class", "parent_class", "args", "expected_args"),
+ [
+ (CustomAuthDBView, AuthDBView, (), ()),
+ (CustomAuthLDAPView, AuthLDAPView, (), ()),
+ (CustomAuthOAuthView, AuthOAuthView, ("azure",), ("azure",)),
+ (CustomAuthOAuthView, AuthOAuthView, (), (None,)),
+ ],
+ )
+ def test_anonymous_login_uses_parent(self, flask_app, view_class,
parent_class, args, expected_args):
+ view = view_class()
+ with (
+ flask_app.test_request_context("/login/"),
+ mock.patch(f"{view_class.__module__}.redirect_if_authenticated",
return_value=None),
+ mock.patch.object(parent_class, "login",
return_value=Response("login page")) as parent_login,
+ ):
+ response = view.login(*args)
+
+ assert response.get_data(as_text=True) == "login page"
+ parent_login.assert_called_once_with(*expected_args)
+
+
+def test_module_uses_token_setting_redirect():
+ from airflow.providers.fab.www import views
+
+ assert auth_login.redirect is views.redirect
diff --git a/providers/fab/tests/unit/fab/www/views/test_views_login.py
b/providers/fab/tests/unit/fab/www/views/test_views_login.py
new file mode 100644
index 00000000000..bb54bcafc93
--- /dev/null
+++ b/providers/fab/tests/unit/fab/www/views/test_views_login.py
@@ -0,0 +1,90 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+from __future__ import annotations
+
+import pytest
+
+from airflow.api_fastapi.auth.managers.base_auth_manager import
COOKIE_NAME_JWT_TOKEN
+from airflow.providers.fab.www import app as application
+from airflow.providers.fab.www.security import permissions
+
+from tests_common.test_utils.config import conf_vars
+from unit.fab.auth_manager.test_utils import create_user, delete_user
+from unit.fab.utils import client_with_login
+
+pytestmark = pytest.mark.db_test
+
+USERNAME = "test_login_next"
+
+
[email protected]
+def app():
+ with conf_vars(
+ {("core", "auth_manager"):
"airflow.providers.fab.auth_manager.fab_auth_manager.FabAuthManager"}
+ ):
+ app = application.create_app(enable_plugins=False)
+ app.config["WTF_CSRF_ENABLED"] = False
+ try:
+ yield app
+ finally:
+ with app.app_context():
+ for fab_engine in
app.extensions["sqlalchemy"].engines.values():
+ fab_engine.dispose()
+
+
[email protected]
+def client(app):
+ with app.app_context():
+ create_user(
+ app,
+ username=USERNAME,
+ role_name="role_login_next",
+ permissions=[(permissions.ACTION_CAN_READ,
permissions.RESOURCE_WEBSITE)],
+ )
+ try:
+ yield client_with_login(app, username=USERNAME, password=USERNAME)
+ finally:
+ delete_user(app, USERNAME)
+
+
+def _sets_token_cookie(response) -> bool:
+ return any(
+ header.startswith(f"{COOKIE_NAME_JWT_TOKEN}=") for header in
response.headers.getlist("Set-Cookie")
+ )
+
+
+class TestLoginWhenAuthenticated:
+ def test_redirects_to_next(self, client):
+ response = client.get("/login/?next=/dags/my_dag/tasks/my_task")
+
+ assert response.status_code == 302
+ assert response.location ==
"http://localhost/dags/my_dag/tasks/my_task";
+ assert _sets_token_cookie(response)
+
+ def test_redirects_to_index_without_next(self, client):
+ response = client.get("/login/")
+
+ assert response.status_code == 302
+ assert response.location == "/"
+ assert _sets_token_cookie(response)
+
+ def test_rejects_external_next(self, client):
+ response = client.get("/login/?next=https://evil.example.com/";)
+
+ assert response.status_code == 302
+ assert "evil.example.com" not in response.location
