exolightor commented on PR #61773: URL: https://github.com/apache/airflow/pull/61773#issuecomment-3892731974
> > Hello @vincbeck, thanks for looking at the change. With my changes, the access token is not saved in the cookie even after the refresh token flow. After refresh token flow in `KeycloakAuthManager.refresh_user()` the access token is only stored as a attribute in the `KeycloakAuthManagerUser` class. After login is successful, the `KeycloakAuthManager.is_authorized()` method does not look into the cookie but only at the deserialized attribute `KeycloakAuthManagerUser.access_token`. So the access token is not stored at any time in the cookie. > > Splitting access token and refresh token across two cookies does not solve the problem in the long run, as the access token alone can easily exceed 4KB. With 15+ realm roles it already should be big enough. Other open source applications (e.g. superset) solve this by storing the access token server-side instead of in the cookie. This is why I changed the code to only store the access token server side, .i.e., only in the `KeycloakAuthManagerUser` class and storing only the refresh token in the cookie. Refresh token stays small since roles are not stored in it. > > I have tested it locally and deployed with helm on OpenShift and issue #61771 is only resolved with my change. Otherwise the login does not work. In our Keycloak realm access tokens can include 15+ realm roles. > > But at the end this is saved in a cookie. It has to be saved somewhere and we do not store anything on the backend. If you look at > > https://github.com/apache/airflow/blob/9f0099fd46434ee455140984f557ffcb8dcc2d8d/airflow-core/src/airflow/api_fastapi/auth/middlewares/refresh_token.py#L63 > > , the token generated saved in `KeycloakAuthManagerUser` is serialized in then saved in the cookie. > `KeycloakAuthManagerUser` is an object that is serialized and saved in a cookie when logging in (or when the token is refreshed), then when the backend receives a request, it fetches the serialized object from the token, deserialized it and use it to retrieve access token. Ok, thanks for the pointer. I do find it quite odd then that in my case with my changes the access token fits into the cookie after refresh token flow but not after the initial login callback. I will investigate this further if I get the chance. I forgot to copy an additional `KeycloakAuthManager.refresh_user()` call to the PR, which was needed to solve Issue #61771 on my end, I included it in my most recent commit. There I assumed that the access token would be stored in the python class, but if we would exclude the access token from the cookie entirely this should lead to the refresh token flow being triggered on each request, which would be very unnecessary overhead. -- 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]
