This is an automated email from the ASF dual-hosted git repository.
shahar1 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 79bc6439a9c Always clear the session cookie on logout (#73788)
79bc6439a9c is described below
commit 79bc6439a9c12f00aa9569a1493ac37f7d5f78aa
Author: Jarek Potiuk <[email protected]>
AuthorDate: Sat Oct 3 22:09:02 2026 +0200
Always clear the session cookie on logout (#73788)
When the auth manager supplies an external logout URL (FAB, Keycloak),
``/auth/logout`` revoked the token and redirected without deleting the
``_token`` cookie, so the revocation was the only thing ending the local
session. ``revoke_token`` also caught every failure -- including a
failure to record the revocation -- and logged it as a warning, so a
revocation that did not happen looked the same as one that did.
Delete the cookie on every logout path, and log a failure to record the
revocation of a valid token as an error, separately from a token that
does not validate and needs no revocation.
Generated-by: Claude Opus 5
---
.../src/airflow/api_fastapi/auth/tokens.py | 24 +++++++++---
.../api_fastapi/core_api/routes/public/auth.py | 9 ++---
.../tests/unit/api_fastapi/auth/test_tokens.py | 43 +++++++++++++++++++++
.../core_api/routes/public/test_auth.py | 45 +++++++++++++++++++++-
4 files changed, 110 insertions(+), 11 deletions(-)
diff --git a/airflow-core/src/airflow/api_fastapi/auth/tokens.py
b/airflow-core/src/airflow/api_fastapi/auth/tokens.py
index 91518d5c72a..29681eaefa0 100644
--- a/airflow-core/src/airflow/api_fastapi/auth/tokens.py
+++ b/airflow-core/src/airflow/api_fastapi/auth/tokens.py
@@ -353,13 +353,27 @@ class JWTValidator:
return claims
def revoke_token(self, token: str) -> None:
- """Validate the token, extract jti and exp, and revoke it in the
database."""
+ """
+ Validate the token, extract jti and exp, and revoke it in the database.
+
+ A token that does not validate is not revoked: it is already unusable,
so this is logged as a
+ warning. A valid token whose revocation cannot be recorded is still
usable until it expires,
+ so that failure is logged as an error rather than being reported the
same way.
+ """
try:
claims = self.validated_claims(token)
- if (jti := claims.get("jti")) and (exp := claims.get("exp")):
- RevokedToken.revoke(jti, datetime.fromtimestamp(exp,
tz=timezone.utc))
- except (jwt.InvalidTokenError, Exception):
- log.warning("Failed to revoke token", exc_info=True)
+ except Exception:
+ log.warning("Not revoking a token that does not validate",
exc_info=True)
+ return
+ if not ((jti := claims.get("jti")) and (exp := claims.get("exp"))):
+ log.warning("Not revoking a token that carries no jti or exp
claim")
+ return
+ try:
+ RevokedToken.revoke(jti, datetime.fromtimestamp(exp,
tz=timezone.utc))
+ except Exception:
+ log.exception(
+ "Failed to record the revocation of token %s; it remains valid
until it expires", jti
+ )
def status(self):
if self.jwks:
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/auth.py
b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/auth.py
index c10fb71d9c8..486ccb7c451 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/routes/public/auth.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/routes/public/auth.py
@@ -78,13 +78,12 @@ def logout(
for token_str in collect_request_tokens(request, bearer_credentials):
auth_manager.revoke_token(token_str)
- logout_url = auth_manager.get_url_logout()
- if logout_url:
- return RedirectResponse(logout_url)
-
+ # The local session cookie is cleared on every path, including the
redirect to an external
+ # logout URL: revocation above is recorded server side and can fail, so it
must not be the
+ # only thing that ends the browser's session.
secure = request.base_url.scheme == "https" or bool(conf.get("api",
"ssl_cert", fallback=""))
cookie_path = get_cookie_path()
- response = RedirectResponse(auth_manager.get_url_login())
+ response = RedirectResponse(auth_manager.get_url_logout() or
auth_manager.get_url_login())
response.delete_cookie(
key=COOKIE_NAME_JWT_TOKEN,
path=cookie_path,
diff --git a/airflow-core/tests/unit/api_fastapi/auth/test_tokens.py
b/airflow-core/tests/unit/api_fastapi/auth/test_tokens.py
index 4dfd186756b..394d96a72e8 100644
--- a/airflow-core/tests/unit/api_fastapi/auth/test_tokens.py
+++ b/airflow-core/tests/unit/api_fastapi/auth/test_tokens.py
@@ -424,6 +424,49 @@ class TestRevokeToken:
):
validator.revoke_token(token)
+ def test_revoke_token_db_error_is_logged_as_error(self):
+ """A valid token that cannot be revoked stays usable, so the failure
is logged as an error."""
+ import time
+ from unittest.mock import patch
+
+ from sqlalchemy.exc import SQLAlchemyError
+
+ now = int(time.time())
+ payload = {
+ "sub": "user",
+ "jti": "db-error-jti",
+ "exp": now + 3600,
+ "iat": now,
+ "nbf": now,
+ "aud": "test",
+ }
+ token = jwt.encode(payload, "secret", algorithm="HS256")
+ validator = JWTValidator(
+ secret_key="secret", audience="test", algorithm=["HS256"],
leeway=0, issuer=None
+ )
+ with (
+ patch("airflow.api_fastapi.auth.tokens.log") as mock_log,
+ patch("airflow.models.revoked_token.RevokedToken.revoke",
side_effect=SQLAlchemyError("db down")),
+ ):
+ validator.revoke_token(token)
+
+ mock_log.exception.assert_called_once()
+ assert "db-error-jti" in mock_log.exception.call_args.args
+
+ def test_revoke_token_invalid_token_is_not_logged_as_error(self):
+ """A token that does not validate is already unusable, so it is only a
warning."""
+ from unittest.mock import patch
+
+ validator = JWTValidator(
+ secret_key="secret", audience="test", algorithm=["HS256"],
leeway=0, issuer=None
+ )
+ with patch("airflow.api_fastapi.auth.tokens.log") as mock_log:
+ validator.revoke_token("invalid-token")
+
+ mock_log.exception.assert_not_called()
+ mock_log.error.assert_not_called()
+ mock_log.warning.assert_called_once()
+
@pytest.fixture(scope="session")
def rsa_private_key():
diff --git
a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_auth.py
b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_auth.py
index a36ab808701..fc4e84621e4 100644
--- a/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_auth.py
+++ b/airflow-core/tests/unit/api_fastapi/core_api/routes/public/test_auth.py
@@ -17,7 +17,7 @@
from __future__ import annotations
import time
-from unittest.mock import MagicMock, patch
+from unittest.mock import AsyncMock, MagicMock, patch
from urllib.parse import parse_qs, urlencode
import jwt
@@ -357,3 +357,46 @@ class TestLogoutTokenRevocation:
assert response.status_code == 307
assert response.headers["location"] == "http://external/logout";
assert RevokedToken.is_revoked("test-jti-redirect-456") is True
+
+ def test_logout_clears_cookie_when_logout_url_redirects(self,
logout_client):
+ """The local session cookie is cleared on the external-logout redirect
too, not only revoked."""
+ auth_manager = logout_client.app.state.auth_manager
+ token_str = self._mint(auth_manager, "test-jti-redirect-cookie")
+
+ logout_client.cookies.set(COOKIE_NAME_JWT_TOKEN, token_str)
+ # The refresh middleware is stubbed out so that only the logout route
can clear the cookie.
+ with (
+ patch(
+
"airflow.api_fastapi.auth.middlewares.refresh_token.JWTRefreshMiddleware._refresh_user",
+ new=AsyncMock(return_value=(None, None)),
+ ),
+ patch.object(auth_manager, "get_url_logout",
return_value="http://external/logout";),
+ ):
+ response = logout_client.get("/auth/logout",
follow_redirects=False)
+
+ assert response.status_code == 307
+ assert response.headers["location"] == "http://external/logout";
+ cookies = response.headers.get_list("set-cookie")
+ assert any(c.startswith(f"{COOKIE_NAME_JWT_TOKEN}=") and "Max-Age=0"
in c for c in cookies)
+
+ def test_logout_clears_cookie_when_revocation_cannot_be_recorded(self,
logout_client):
+ """A failed revocation write does not leave the browser session in
place."""
+ from sqlalchemy.exc import SQLAlchemyError
+
+ auth_manager = logout_client.app.state.auth_manager
+ token_str = self._mint(auth_manager, "test-jti-revoke-fails")
+
+ logout_client.cookies.set(COOKIE_NAME_JWT_TOKEN, token_str)
+ with (
+ patch(
+
"airflow.api_fastapi.auth.middlewares.refresh_token.JWTRefreshMiddleware._refresh_user",
+ new=AsyncMock(return_value=(None, None)),
+ ),
+ patch.object(auth_manager, "get_url_logout",
return_value="http://external/logout";),
+ patch("airflow.models.revoked_token.RevokedToken.revoke",
side_effect=SQLAlchemyError("db down")),
+ ):
+ response = logout_client.get("/auth/logout",
follow_redirects=False)
+
+ assert response.status_code == 307
+ cookies = response.headers.get_list("set-cookie")
+ assert any(c.startswith(f"{COOKIE_NAME_JWT_TOKEN}=") and "Max-Age=0"
in c for c in cookies)
