bugraoz93 commented on code in PR #51657: URL: https://github.com/apache/airflow/pull/51657#discussion_r2167425659
########## airflow-core/src/airflow/api_fastapi/auth/managers/middleware/refresh_token.py: ########## @@ -0,0 +1,54 @@ +# Licensed to the Apache Software Foundation (ASF) under one +# or more contributor license agreements. See the NOTICE file +# distributed with this work for additional information +# regarding copyright ownership. The ASF licenses this file +# to you under the Apache License, Version 2.0 (the +# "License"); you may not use this file except in compliance +# with the License. You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, +# software distributed under the License is distributed on an +# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +# KIND, either express or implied. See the License for the +# specific language governing permissions and limitations +# under the License. + +from __future__ import annotations + +import logging + +from fastapi import Request +from starlette.middleware.base import BaseHTTPMiddleware + +from airflow.api_fastapi.auth.managers.base_auth_manager import COOKIE_NAME_JWT_TOKEN, BaseAuthManager +from airflow.configuration import conf + +log = logging.getLogger(__name__) + + +class RefreshTokenMiddleware(BaseHTTPMiddleware): Review Comment: Normally, they should be called with the new token because in every refresh, both the access and refresh tokens are renewed. We may need to wait until a new token is refreshed from middleware until the UI hits one of the ui endpoints. Although I am testing while writing and seeing that nearly all the pages somehow hit one of the UI endpoints but if a specific button is clicked, it could end up only calling the /api endpoint. I have also tested with the 5 minutes and refresh steps, and it refreshed after hitting the endpoints with the current implementation. When it gets an error, indeed, the `refresh_token` expires after a certain time according to Keycloak configurations, the refresh token fails with an expired error. I have tested with multiple Keycloak settings from 10 seconds to 5 minutes, including tweaking idle and max settings. I see that it depends on Keycloak settings of client expirations, either in realm settings or in Advanced settings in the client that is used in Airflow. If the user only tweaks the access token expiration, for example, 10 seconds, it will refresh and use a new token but if Client Session Idle and Client Session Max are adjusted, it fails to refresh the token and full logout and login are needed in that case. I think if we implement logout, similar to other auth managers, this should also be supported via the logout flow. All good! I am happy with what it has become, better with your help, thank you very much, Vincent! :) Sure, I am okay with that. It shouldn't make any changes in the sense of acquiring the token. The only thing is that we will increase the number of token exchanges in total. I am also not limiting what can ease the probability of failures. Let me delete the cors check too :) -- 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: commits-unsubscr...@airflow.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
