This is an automated email from the ASF dual-hosted git repository.
vatsrahul1001 pushed a commit to branch v3-3-test
in repository https://gitbox.apache.org/repos/asf/airflow.git
The following commit(s) were added to refs/heads/v3-3-test by this push:
new 7a8c7207ecc [v3-3-test] Revoke every credential presented to the Core
API logout endpoint (#72649) (#72933)
7a8c7207ecc is described below
commit 7a8c7207ecca3e8c021a1ee573708bb6be5c4d9b
Author: github-actions[bot]
<41898282+github-actions[bot]@users.noreply.github.com>
AuthorDate: Fri Sep 11 16:24:04 2026 +0530
[v3-3-test] Revoke every credential presented to the Core API logout
endpoint (#72649) (#72933)
logout() read only the _token cookie. A client authenticating with an
Authorization: Bearer header -- the documented way to call the Core API --
therefore received a normal logout response while its token was never
revoked,
and that token stayed valid until it expired. The default lifetime is 24
hours.
A copy of the token held by anyone else survived the holder's logout, so
asking
to be logged out did not end the session it was presented for.
Logout uses collect_request_tokens() and revokes all of them, not just the
precedence-selected one. It walks the same order get_user() authenticates
by, so
logout does not hard-code its own separate answer to what a request's
credentials
are. Revoking only the winner would leave any other credential the caller
presented valid after they asked to be logged out, and which credential
wins is a
question about authentication that should not decide what a logout
terminates.
Revocation still happens before any redirect or cookie deletion, so an
external
auth-manager logout URL cannot skip it.
The bearer tests fail against unpatched sources with 'assert False is True'
--
the token is simply not revoked.
(cherry picked from commit 3f0a5d61fa8e6d8c0c5f6f48559ec350c026903a)
Generated-by: Claude Code (Opus 5)
Claude-Session: https://claude.ai/code/session_01XS3bodTDYYGrPmorhtLsjP
Co-authored-by: Jarek Potiuk <[email protected]>
---
.../core_api/openapi/v2-rest-api-generated.yaml | 3 +
.../api_fastapi/core_api/routes/public/auth.py | 29 ++++--
.../src/airflow/api_fastapi/core_api/security.py | 27 ++++++
.../core_api/routes/public/test_auth.py | 107 +++++++++++++++++++++
4 files changed, 160 insertions(+), 6 deletions(-)
diff --git
a/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
b/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
index 196d1f429e6..ebc0dad8576 100644
---
a/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
+++
b/airflow-core/src/airflow/api_fastapi/core_api/openapi/v2-rest-api-generated.yaml
@@ -10795,6 +10795,9 @@ paths:
application/json:
schema:
$ref: '#/components/schemas/HTTPExceptionResponse'
+ security:
+ - OAuth2PasswordBearer: []
+ - HTTPBearer: []
components:
schemas:
AppBuilderMenuItemResponse:
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 2237a68c055..890be3849fd 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
@@ -19,14 +19,21 @@ from __future__ import annotations
from urllib.parse import urlencode
import structlog
-from fastapi import HTTPException, Request, status
+from fastapi import Depends, HTTPException, Request, status
from fastapi.responses import RedirectResponse
+from fastapi.security import HTTPAuthorizationCredentials
from airflow.api_fastapi.app import get_cookie_path
from airflow.api_fastapi.auth.managers.base_auth_manager import
COOKIE_NAME_JWT_TOKEN
from airflow.api_fastapi.common.router import AirflowRouter
from airflow.api_fastapi.core_api.openapi.exceptions import
create_openapi_http_exception_doc
-from airflow.api_fastapi.core_api.security import AuthManagerDep, is_safe_url
+from airflow.api_fastapi.core_api.security import (
+ AuthManagerDep,
+ bearer_scheme,
+ collect_request_tokens,
+ is_safe_url,
+ oauth2_scheme,
+)
from airflow.configuration import conf
log = structlog.get_logger(logger_name=__name__)
@@ -57,11 +64,21 @@ def login(request: Request, auth_manager: AuthManagerDep,
next: None | str = Non
"/logout",
responses=create_openapi_http_exception_doc([status.HTTP_307_TEMPORARY_REDIRECT]),
)
-def logout(request: Request, auth_manager: AuthManagerDep) -> RedirectResponse:
+def logout(
+ request: Request,
+ auth_manager: AuthManagerDep,
+ oauth_token: str | None = Depends(oauth2_scheme),
+ bearer_credentials: HTTPAuthorizationCredentials | None =
Depends(bearer_scheme),
+) -> RedirectResponse:
"""Logout the user."""
- # Revoke the current token before any redirect or cookie deletion so the
JWT
- # is invalidated even when the auth manager redirects to an external
logout URL.
- if token_str := request.cookies.get(COOKIE_NAME_JWT_TOKEN):
+ # Revoke every credential presented before any redirect or cookie
deletion, so the
+ # JWT is invalidated even when the auth manager redirects to an external
logout URL.
+ #
+ # This previously read only the `_token` cookie. A client that
authenticates with an
+ # `Authorization: Bearer` header -- the documented way to call the API --
therefore
+ # got a successful logout response while its token was never revoked, and
the token
+ # stayed valid until it expired.
+ for token_str in collect_request_tokens(request, oauth_token,
bearer_credentials):
auth_manager.revoke_token(token_str)
logout_url = auth_manager.get_url_logout()
diff --git a/airflow-core/src/airflow/api_fastapi/core_api/security.py
b/airflow-core/src/airflow/api_fastapi/core_api/security.py
index 5a333caadbf..6a357789757 100644
--- a/airflow-core/src/airflow/api_fastapi/core_api/security.py
+++ b/airflow-core/src/airflow/api_fastapi/core_api/security.py
@@ -166,6 +166,33 @@ async def get_user(
return await
resolve_user_from_token(request.cookies.get(COOKIE_NAME_JWT_TOKEN))
+def collect_request_tokens(
+ request: Request,
+ oauth_token: str | None,
+ bearer_credentials: HTTPAuthorizationCredentials | None,
+) -> list[str]:
+ """
+ Return every distinct credential presented on this request, in precedence
order.
+
+ Logout uses this rather than reproducing the single-credential choice
+ :func:`get_user` makes. Revoking only the precedence-selected credential
would leave
+ any other one the caller presented still valid after they asked to be
logged out,
+ and which credential "wins" is a question about *authentication* that
should not
+ decide what a logout terminates.
+ """
+ candidates: list[str | None] = []
+ if bearer_credentials and bearer_credentials.scheme.lower() == "bearer":
+ candidates.append(bearer_credentials.credentials)
+ candidates.append(oauth_token)
+ candidates.append(request.cookies.get(COOKIE_NAME_JWT_TOKEN))
+
+ tokens: list[str] = []
+ for candidate in candidates:
+ if candidate and candidate not in tokens:
+ tokens.append(candidate)
+ return tokens
+
+
GetUserDep = Annotated[BaseUser, Depends(get_user)]
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 0379640f887..b2d616129e4 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
@@ -221,6 +221,113 @@ class TestLogoutTokenRevocation:
assert response.status_code == 307
assert RevokedToken.is_revoked("nonexistent-jti") is False
+ @staticmethod
+ def _mint(auth_manager, jti: str) -> str:
+ now = int(time.time())
+ signer = auth_manager._get_token_signer()
+ return jwt.encode(
+ {
+ "sub": "admin",
+ "jti": jti,
+ "exp": now + 3600,
+ "iat": now,
+ "nbf": now,
+ "aud": "apache-airflow",
+ "iss": signer.issuer,
+ },
+ signer._secret_key,
+ algorithm=signer.algorithm,
+ )
+
+ def test_logout_revokes_a_bearer_token(self, logout_client):
+ """A bearer-only logout must revoke the token it presented.
+
+ Clients calling the API authenticate with `Authorization: Bearer`, and
there is
+ no cookie on such a request. Logout previously read only the cookie,
so it
+ returned its normal response while revoking nothing and the token
stayed valid
+ until it expired.
+ """
+ auth_manager = logout_client.app.state.auth_manager
+ token_str = self._mint(auth_manager, "test-jti-bearer")
+
+ with patch.object(auth_manager, "get_url_logout", return_value=None):
+ response = logout_client.get(
+ "/auth/logout",
+ headers={"Authorization": f"Bearer {token_str}"},
+ follow_redirects=False,
+ )
+
+ assert response.status_code == 307
+ assert RevokedToken.is_revoked("test-jti-bearer") is True
+
+ def test_logout_revokes_every_credential_presented(self, logout_client):
+ """Both a bearer and a cookie presented together must both be revoked.
+
+ Revoking only the precedence-selected one would leave the other valid
after the
+ caller asked to be logged out.
+ """
+ auth_manager = logout_client.app.state.auth_manager
+ bearer_token = self._mint(auth_manager, "test-jti-both-bearer")
+ cookie_token = self._mint(auth_manager, "test-jti-both-cookie")
+
+ logout_client.cookies.set(COOKIE_NAME_JWT_TOKEN, cookie_token)
+ with patch.object(auth_manager, "get_url_logout", return_value=None):
+ response = logout_client.get(
+ "/auth/logout",
+ headers={"Authorization": f"Bearer {bearer_token}"},
+ follow_redirects=False,
+ )
+
+ assert response.status_code == 307
+ assert RevokedToken.is_revoked("test-jti-both-bearer") is True
+ assert RevokedToken.is_revoked("test-jti-both-cookie") is True
+
+ def test_logout_revokes_both_even_when_a_trusted_user_is_cached(self,
logout_client):
+ """The trusted-middleware shortcut must not change what logout revokes.
+
+ On protected routes `get_user()` can return a user cached by
JWTRefreshMiddleware
+ without consulting the request's credentials at all. Logout must still
act on the
+ credentials actually presented, so revocation cannot be skipped by
that shortcut.
+ """
+ from airflow.api_fastapi.core_api.security import
USER_INJECTED_BY_TRUSTED_MIDDLEWARE
+
+ auth_manager = logout_client.app.state.auth_manager
+ bearer_token = self._mint(auth_manager, "test-jti-trusted-bearer")
+ cookie_token = self._mint(auth_manager, "test-jti-trusted-cookie")
+
+ async def _inject(request, call_next):
+ request.state.user = object()
+ request.state.user_authenticated_via =
USER_INJECTED_BY_TRUSTED_MIDDLEWARE
+ return await call_next(request)
+
+ logout_client.app.middleware("http")(_inject)
+ logout_client.cookies.set(COOKIE_NAME_JWT_TOKEN, cookie_token)
+ with patch.object(auth_manager, "get_url_logout", return_value=None):
+ response = logout_client.get(
+ "/auth/logout",
+ headers={"Authorization": f"Bearer {bearer_token}"},
+ follow_redirects=False,
+ )
+
+ assert response.status_code == 307
+ assert RevokedToken.is_revoked("test-jti-trusted-bearer") is True
+ assert RevokedToken.is_revoked("test-jti-trusted-cookie") is True
+
+ def test_logout_revokes_a_bearer_token_before_an_external_redirect(self,
logout_client):
+ """The bearer must be revoked even when the auth manager redirects
away."""
+ auth_manager = logout_client.app.state.auth_manager
+ token_str = self._mint(auth_manager, "test-jti-bearer-redirect")
+
+ with patch.object(auth_manager, "get_url_logout",
return_value="https://idp.example/logout";):
+ response = logout_client.get(
+ "/auth/logout",
+ headers={"Authorization": f"Bearer {token_str}"},
+ follow_redirects=False,
+ )
+
+ assert response.status_code == 307
+ assert RevokedToken.is_revoked("test-jti-bearer-redirect") is True
+
def test_logout_revokes_token_when_logout_url_redirects(self,
logout_client):
"""Token must be revoked before the redirect when get_url_logout
returns a URL."""
now = int(time.time())
