From 3a0d0247df16b54da54ed2e8d3247266b22acf24 Mon Sep 17 00:00:00 2001 From: Ed Summers Date: Tue, 1 Sep 2026 10:23:36 -0400 Subject: [PATCH 1/2] Keycloak auth manager: keep API tokens usable with the Authorization header On Airflow 3.3+, a token from `POST /auth/token` no longer authorized anything. `serialize_user()` omits the Keycloak JWTs from the claims, and the only code that supplies them again, `KeycloakJWTMiddleware`, reads them from cookies. A client authenticating with the `Authorization` header sends no cookies, so `get_user_from_token()` fell through to `return None` and the request failed -- in practice with a 500, because the `None` reaches the authorization layer and raises `AttributeError: 'NoneType' object has no attribute 'get_id'`. The tokens were moved into cookies to keep the browser session cookie under the 4096 byte limit. That constraint does not apply to a token handed to an API client, which is never stored in a cookie, so the two paths can differ: - `generate_api_jwt()` mints tokens that keep the Keycloak JWTs in their claims, and `POST /auth/token` uses it for both the password and client-credentials grants. It builds on `serialize_user()` so an API token cannot silently miss a claim the browser flow gains later. - The browser paths, `routes/login.py` and the middleware, are unchanged and still mint claim-free tokens backed by the cookies. - `get_user_from_token()` falls back to the claims when no cookie-supplied tokens are present. The fallback does not weaken the subject binding added for the cookie flow. That check exists because cookies are not covered by the Airflow JWT signature, so a caller could pair their own session with somebody else's Keycloak token. In the claims both values come from the same signed payload, and both minting sites derive them from one Keycloak response, so a mismatched pair cannot be constructed. A browser token still resolves to `None` without its cookies: it carries no claims to fall back on. Tested alongside the fix so the two paths stay distinct. Fixes #72352 --- .../auth_manager/keycloak_auth_manager.py | 39 ++++++++++++- .../keycloak/auth_manager/services/token.py | 9 ++- .../auth_manager/services/test_token.py | 8 ++- .../test_keycloak_auth_manager.py | 55 +++++++++++++++++++ 4 files changed, 104 insertions(+), 7 deletions(-) diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py index 26fbfcb3868e2..2479c7256fb82 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py @@ -149,7 +149,10 @@ def deserialize_user(self, token: dict[str, Any]) -> KeycloakAuthManagerUser: def serialize_user(self, user: KeycloakAuthManagerUser) -> dict[str, Any]: if AIRFLOW_V_3_3_PLUS: - # Omit Keycloak JWTs from claims, they are stored in separate cookies + # Omit Keycloak JWTs from claims, they are stored in separate cookies. + # That keeps the browser session cookie under the 4096 byte limit. Tokens + # minted for API clients are never stored in a cookie and keep the JWTs in + # their claims instead -- see ``generate_api_jwt``. return { "user_id": user.get_id(), "name": user.get_name(), @@ -161,6 +164,31 @@ def serialize_user(self, user: KeycloakAuthManagerUser) -> dict[str, Any]: "refresh_token": user.refresh_token, } + def generate_api_jwt( + self, + user: KeycloakAuthManagerUser, + *, + expiration_time_in_seconds: int = conf.getint("api_auth", "jwt_expiration_time"), + ) -> str: + """ + Return a JWT for a client that authenticates with the ``Authorization`` header. + + Such a client sends no cookies, so the Keycloak tokens have to travel in the + claims for the request to be authorizable. + + :param user: the user to generate the token for + :param expiration_time_in_seconds: expiration time in seconds of the token + """ + return self._get_token_signer(expiration_time_in_seconds=expiration_time_in_seconds).generate( + { + # Build on serialize_user so an API token cannot silently miss a claim + # that the browser flow gained; only the Keycloak JWTs differ. + **self.serialize_user(user), + "access_token": user.access_token, + "refresh_token": user.refresh_token, + } + ) + async def get_user_from_token( self, token: str, access_token: str | None = None, refresh_token: str | None = None ): @@ -186,8 +214,13 @@ async def get_user_from_token( user.access_token = access_token user.refresh_token = refresh_token return user - # Skip refreshing JWT if Keycloak JWTs are not included. - return None + # No cookie-supplied tokens. A token minted for an API client carries the + # Keycloak JWTs in its own claims, so the user is already complete -- and unlike + # the cookie path those claims are covered by the Airflow JWT signature, so they + # need no separate subject check. A browser token does not carry them and can + # only be completed by KeycloakJWTMiddleware from the cookies; without them + # there is nothing to authorize against. + return user if user.access_token else None def get_url_login(self, **kwargs) -> str: base_url = conf.get("api", "base_url", fallback="/") diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py index a92ec0e04cc76..054416f6fc762 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py @@ -18,6 +18,7 @@ from __future__ import annotations import json +from typing import cast from fastapi import HTTPException, status from keycloak import KeycloakAuthenticationError @@ -58,7 +59,9 @@ def create_token_for( refresh_token=tokens["refresh_token"], ) - return get_auth_manager().generate_jwt(user, expiration_time_in_seconds=expiration_time_in_seconds) + return cast("KeycloakAuthManager", get_auth_manager()).generate_api_jwt( + user, expiration_time_in_seconds=expiration_time_in_seconds + ) def create_client_credentials_token( @@ -118,4 +121,6 @@ def create_client_credentials_token( ), # client_credentials may not return refresh_token (RFC6749 section 4.4.3) ) - return get_auth_manager().generate_jwt(user, expiration_time_in_seconds=expiration_time_in_seconds) + return cast("KeycloakAuthManager", get_auth_manager()).generate_api_jwt( + user, expiration_time_in_seconds=expiration_time_in_seconds + ) diff --git a/providers/keycloak/tests/unit/keycloak/auth_manager/services/test_token.py b/providers/keycloak/tests/unit/keycloak/auth_manager/services/test_token.py index a3e365051e46d..b02bcc1c95adc 100644 --- a/providers/keycloak/tests/unit/keycloak/auth_manager/services/test_token.py +++ b/providers/keycloak/tests/unit/keycloak/auth_manager/services/test_token.py @@ -55,9 +55,13 @@ def test_create_token(self, mock_get_keycloak_client, mock_get_auth_manager): mock_get_keycloak_client.return_value = mock_keycloak_client mock_auth_manager = Mock() mock_get_auth_manager.return_value = mock_auth_manager - mock_auth_manager.generate_jwt.return_value = self.token + mock_auth_manager.generate_api_jwt.return_value = self.token assert create_token_for(username=self.test_username, password=self.test_password) == self.token + # API tokens must be minted with the Keycloak JWTs in their claims, since a + # header-authenticated client sends no cookies to supply them. + mock_auth_manager.generate_api_jwt.assert_called_once() + mock_auth_manager.generate_jwt.assert_not_called() mock_keycloak_client.token.assert_called_once_with(self.test_username, self.test_password) mock_keycloak_client.userinfo.assert_called_once_with(self.test_access_token) @@ -104,7 +108,7 @@ def test_create_token_client_credentials(self, mock_get_keycloak_client, mock_ge mock_get_keycloak_client.return_value = mock_keycloak_client mock_auth_manager = Mock() mock_get_auth_manager.return_value = mock_auth_manager - mock_auth_manager.generate_jwt.return_value = self.token + mock_auth_manager.generate_api_jwt.return_value = self.token result = create_client_credentials_token(client_id=test_client_id, client_secret=test_client_secret) diff --git a/providers/keycloak/tests/unit/keycloak/auth_manager/test_keycloak_auth_manager.py b/providers/keycloak/tests/unit/keycloak/auth_manager/test_keycloak_auth_manager.py index 32ddab09514ba..3e05e01304ac5 100644 --- a/providers/keycloak/tests/unit/keycloak/auth_manager/test_keycloak_auth_manager.py +++ b/providers/keycloak/tests/unit/keycloak/auth_manager/test_keycloak_auth_manager.py @@ -255,6 +255,61 @@ async def test_get_user_from_token(self, auth_manager): assert user.access_token == "access_token" assert user.refresh_token == "refresh_token" + @pytest.mark.asyncio + async def test_api_token_authenticates_without_cookies(self, auth_manager): + """A token from ``POST /auth/token`` must authorize an ``Authorization`` header request. + + Such a client sends no cookies, so ``get_user_from_token`` is called with the + token alone and the user has to be reconstructible from its claims. + """ + access_token = keycloak_token("user_id") + user = KeycloakAuthManagerUser( + user_id="user_id", name="name", access_token=access_token, refresh_token="refresh_token" + ) + + minted_claims: dict = {} + + class _CapturingSigner: + def generate(self, claims): + minted_claims.update(claims) + return "token" + + with patch.object(KeycloakAuthManager, "_get_token_signer", Mock(return_value=_CapturingSigner())): + auth_manager.generate_api_jwt(user) + + mock_token_validator = Mock() + mock_token_validator.avalidated_claims = AsyncMock(return_value=minted_claims) + with patch.object( + KeycloakAuthManager, "_get_token_validator", Mock(return_value=mock_token_validator) + ): + resolved = await auth_manager.get_user_from_token("token") + + assert resolved is not None + assert resolved.get_id() == "user_id" + assert resolved.access_token == access_token + assert resolved.refresh_token == "refresh_token" + + @pytest.mark.skipif(not AIRFLOW_V_3_3_PLUS, reason="Browser tokens omit the Keycloak JWTs") + @pytest.mark.asyncio + async def test_browser_token_without_cookies_is_not_authenticated(self, auth_manager): + """A browser token carries no Keycloak JWTs, so it cannot stand in for the cookies.""" + browser_claims = auth_manager.serialize_user( + KeycloakAuthManagerUser( + user_id="user_id", + name="name", + access_token=keycloak_token("user_id"), + refresh_token="refresh_token", + ) + ) + assert "access_token" not in browser_claims + + mock_token_validator = Mock() + mock_token_validator.avalidated_claims = AsyncMock(return_value=browser_claims) + with patch.object( + KeycloakAuthManager, "_get_token_validator", Mock(return_value=mock_token_validator) + ): + assert await auth_manager.get_user_from_token("token") is None + @pytest.mark.skipif(not AIRFLOW_V_3_3_PLUS, reason="Uses KeycloakJWTMiddleware and separate cookies") @pytest.mark.asyncio async def test_get_user_from_token_keycloak_jwts_missing(self, auth_manager): From f845e3340ccd1074d5ece250c71b3210541b156c Mon Sep 17 00:00:00 2001 From: Ed Summers Date: Wed, 2 Sep 2026 10:52:59 -0400 Subject: [PATCH 2/2] Expose generate_api_jwt on BaseAuthManager Review feedback: put the mechanism on the base class so any auth manager that keeps part of its state in cookies can opt into a different token shape for header-authenticated clients. `BaseAuthManager.generate_api_jwt()` defaults to `generate_jwt()`, so auth managers whose tokens are already self-contained are unaffected -- Simple, FAB and AWS all serialize everything they need for authorization into the claims. Keycloak is the outlier: it replays the user's own Keycloak access token to Keycloak at authorization time, so that credential has to reach the request somehow, and cookies do not reach a header-authenticated client. Its method is now an override rather than a Keycloak-only addition. This also removes the `cast("KeycloakAuthManager", ...)` the previous revision needed in `services/token.py`, since the method is now declared on the base type. Also fixes the docs spellcheck failure on this branch: "authorizable" is not in docs/spelling_wordlist.txt, so the docstring now says "authorized". Verified: providers/keycloak suite 296 passed (2 pre-existing environment errors, unchanged); airflow-core base auth manager tests 60 passed; mypy clean across the provider and the modified core file; ruff clean. Co-Authored-By: Claude Opus 5 (1M context) --- .../auth/managers/base_auth_manager.py | 16 ++++++++++++++++ .../auth_manager/keycloak_auth_manager.py | 2 +- .../keycloak/auth_manager/services/token.py | 9 ++------- 3 files changed, 19 insertions(+), 8 deletions(-) diff --git a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py index f8171a9db8345..baa8b0174b092 100644 --- a/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py +++ b/airflow-core/src/airflow/api_fastapi/auth/managers/base_auth_manager.py @@ -191,6 +191,22 @@ def generate_jwt( self.serialize_user(user) ) + def generate_api_jwt( + self, user: T, *, expiration_time_in_seconds: int = conf.getint("api_auth", "jwt_expiration_time") + ) -> str: + """ + Return the JWT token for a client that authenticates with the ``Authorization`` header. + + Such a client sends no cookies, so an auth manager that keeps part of its state in + cookies has to put that state in the token's claims instead for the request to be + authorized. Auth managers whose tokens are already self-contained need not override + this. + + :param user: the user to generate the token for + :param expiration_time_in_seconds: expiration time in seconds of the token + """ + return self.generate_jwt(user, expiration_time_in_seconds=expiration_time_in_seconds) + @abstractmethod def get_url_login(self, **kwargs) -> str: """Return the login page url.""" diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py index 2479c7256fb82..3fe177bc6cc04 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py @@ -174,7 +174,7 @@ def generate_api_jwt( Return a JWT for a client that authenticates with the ``Authorization`` header. Such a client sends no cookies, so the Keycloak tokens have to travel in the - claims for the request to be authorizable. + claims for the request to be authorized. :param user: the user to generate the token for :param expiration_time_in_seconds: expiration time in seconds of the token diff --git a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py index 054416f6fc762..94da35310f732 100644 --- a/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py +++ b/providers/keycloak/src/airflow/providers/keycloak/auth_manager/services/token.py @@ -18,7 +18,6 @@ from __future__ import annotations import json -from typing import cast from fastapi import HTTPException, status from keycloak import KeycloakAuthenticationError @@ -59,9 +58,7 @@ def create_token_for( refresh_token=tokens["refresh_token"], ) - return cast("KeycloakAuthManager", get_auth_manager()).generate_api_jwt( - user, expiration_time_in_seconds=expiration_time_in_seconds - ) + return get_auth_manager().generate_api_jwt(user, expiration_time_in_seconds=expiration_time_in_seconds) def create_client_credentials_token( @@ -121,6 +118,4 @@ def create_client_credentials_token( ), # client_credentials may not return refresh_token (RFC6749 section 4.4.3) ) - return cast("KeycloakAuthManager", get_auth_manager()).generate_api_jwt( - user, expiration_time_in_seconds=expiration_time_in_seconds - ) + return get_auth_manager().generate_api_jwt(user, expiration_time_in_seconds=expiration_time_in_seconds)