stephen-bracken commented on code in PR #76353:
URL: https://github.com/apache/airflow/pull/76353#discussion_r4466372111
##########
providers/keycloak/src/airflow/providers/keycloak/auth_manager/keycloak_auth_manager.py:
##########
@@ -223,8 +223,10 @@ async def get_user_from_token(
# 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
+ # there is nothing to authorize against, so the token is rejected.
+ if not user.access_token:
Review 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.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
