Repository navigation
Conversation
…turning None Since Airflow 3.3 the browser JWT no longer carries the Keycloak tokens, and get_user_from_token returned None when they were not supplied from the cookies either. The core get_user dependency passed that None on to the is_authorized_* checks, which failed with "'NoneType' object has no attribute 'access_token'" and answered every request with a 500. That happens as soon as the Keycloak refresh token of a browser session has expired or its access token cookie is missing, and the user stays stuck because the middleware never gets to clear the session cookie. Raise InvalidTokenError instead, as BaseAuthManager.get_user_from_token promises. The API then answers 403 "Invalid JWT token", which sends the UI to the login page. closes: apache#76352
| # there is nothing to authorize against. | ||
| return user if user.access_token else None | ||
| # there is nothing to authorize against, so the token is rejected. | ||
| if not user.access_token: |
There was a problem hiding this comment.
The purpose of returning a None user is to bypass the refresh loop when KeycloakAuthManager.get_user_from_token is called from airflow.api_fastapi.auth.middlewares.refresh_token.JWTRefreshMiddleware, as this middleware is not Keycloak JWT aware. Without this, the refresh middleware will create a login loop as it gets called before the KeycloakJWTMiddleware. I think this change will break that behaviour.
There was a problem hiding this comment.
You're right, thanks. The core JWTRefreshMiddleware resolves the bare _token on every request without the Keycloak cookies, so raising there turned every request into an invalidated session and cleared the cookie after the first one.
So the None stays (with a comment saying why), and the fix moved to KeycloakJWTMiddleware: once _refresh_user fails it already clears _token on the response, and now it also removes _token from the request it passes on. get_user then answers 401 instead of handing None to the authorization checks, and the core middleware passes the request through. The new tests run both middlewares as init_middlewares installs them: a session with its Keycloak tokens keeps its cookie (that one fails on the previous commit), and an expired refresh token or a missing access token cookie answers 401 and clears it.
…oken Raising InvalidTokenError from get_user_from_token broke every browser session: the core JWTRefreshMiddleware resolves the Airflow JWT on every request without the Keycloak cookies, and treated the exception as an invalidated session, clearing the cookie after the first request. The None it returned before is what lets that middleware pass the request on, so it stays, now with a comment saying why. The 500 comes from the other caller. When KeycloakJWTMiddleware cannot attach the user (the Keycloak refresh token expired, or the access token cookie is missing), it already ends the session by clearing the Airflow JWT on the response, but the route still received the JWT and get_user resolved it to None for the authorization checks. The middleware now also removes the JWT from the request it passes on, so get_user answers 401 and the UI goes to the login page, while routes that need no user keep working. The new tests run the middlewares as the API server installs them in front of a route that needs the user: a session with its Keycloak tokens keeps its cookie, and an expired or missing Keycloak session answers 401 and clears it.
stephen-bracken
left a comment
stephen-bracken
left a comment
There was a problem hiding this comment.
LGTM. I will include this with #70707
Since Airflow 3.3 the browser JWT no longer carries the Keycloak tokens:
KeycloakJWTMiddlewarereads them from their own cookies. When it cannot complete the session (the Keycloak refresh token expired, or the access token cookie is missing), it clears the Airflow JWT on the response, but the request still reaches the route with that JWT.get_userthen falls back toresolve_user_from_token,KeycloakAuthManager.get_user_from_tokenreturnsNonefor a browser token without Keycloak tokens, and theis_authorized_*checks fail onuser.access_token: every UI request answers 500 until the user deletes the cookies.KeycloakJWTMiddlewarenow also removes the Airflow JWT from the request it passes on once it has ended the session.get_userthen answers 401 "Not authenticated", which sends the UI to the login page, and routes that need no user (the login route among them) keep working.get_user_from_tokenkeeps returningNonein that case: the coreJWTRefreshMiddlewareresolves the Airflow JWT on every request without the Keycloak cookies, and an exception there would end every browser session. A comment now says so.Tests: a new class runs the middlewares as
init_middlewaresinstalls them, in front of a route that needs the user. A session with its Keycloak tokens keeps its cookie (fails with the first version of this PR, which raised instead of returningNone); an expired Keycloak refresh token and a missing access token cookie answer 401 and clear the cookie (both fail onmain, where the route receivesNoneas user).The
Nonepath came with #70550, which moved the Keycloak tokens into their own cookies in Airflow 3.3. #70707 reworks the middleware for Airflow 3.4.1+; this change is independent of it and covers 3.3 to 3.4.0.closes: #76352
related: #70550, #70707
Was generative AI tooling used to co-author this PR?
Generated-by: Claude Code (Opus 5.5) following the guidelines
🤖 Generated with Claude Code