Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
vincbeck commented on code in PR #54196:
URL: https://github.com/apache/airflow/pull/54196#discussion_r2611858757
##
airflow-core/docs/core-concepts/auth-manager/index.rst:
##
@@ -170,8 +170,76 @@ cookie named ``_token`` before redirecting to the Airflow
UI. The Airflow UI wil
return response
.. note::
-Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
+ Ensure that the cookie parameter ``httponly`` is set to ``True``. UI no
longer manages the token.
+Refreshing JWT Token
+
+Refreshing token is optional feature and its availability depends on the
specific implementation of the auth manager.
+The auth manager is responsible for refreshing the JWT token when it expires.
+The Airflow API uses middleware that intercepts every request and checks the
validity of the JWT token.
+Token communication is handled through ``httponly`` cookies to improve
security.
+When the token expires, the middleware calls the auth manager's
``refresh_token`` method to obtain a new token.
+
+To support token refresh operations, the auth manager must implement the
``refresh_token`` method.
+This method receives an expired token and must return a new valid token.
+User information is extracted from the expired token and used to generate a
fresh token.
+
+An example implementation of ``refresh_user`` could be:
+
+.. code-block:: python
+
+def refresh_user(self, *, user: KeycloakAuthManagerUser) ->
KeycloakAuthManagerUser | None:
+if self._token_expired(user.access_token):
+log.debug("Refreshing the token")
+client = self.get_keycloak_client()
+tokens = client.refresh_token(user.refresh_token)
+user.refresh_token = tokens["refresh_token"]
+user.access_token = tokens["access_token"]
+return user
+
+return None
+
+User information is derived from the ``BaseUser`` instance. It is important
that the user object contains all the fields required to refresh the token. An
example user class that includes all necessary fields is shown below:
+
+.. code-block:: python
+
+class KeycloakAuthManagerUser(BaseUser):
Review Comment:
Same here
##
airflow-core/docs/core-concepts/auth-manager/index.rst:
##
@@ -170,8 +170,76 @@ cookie named ``_token`` before redirecting to the Airflow
UI. The Airflow UI wil
return response
.. note::
-Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
+ Ensure that the cookie parameter ``httponly`` is set to ``True``. UI no
longer manages the token.
+Refreshing JWT Token
+
+Refreshing token is optional feature and its availability depends on the
specific implementation of the auth manager.
+The auth manager is responsible for refreshing the JWT token when it expires.
+The Airflow API uses middleware that intercepts every request and checks the
validity of the JWT token.
+Token communication is handled through ``httponly`` cookies to improve
security.
+When the token expires, the middleware calls the auth manager's
``refresh_token`` method to obtain a new token.
+
+To support token refresh operations, the auth manager must implement the
``refresh_token`` method.
+This method receives an expired token and must return a new valid token.
+User information is extracted from the expired token and used to generate a
fresh token.
+
+An example implementation of ``refresh_user`` could be:
Review Comment:
To avoid having out of date code, maybe link to the keycloak auth manager
implementation of `refresh_user`?
##
airflow-core/docs/core-concepts/auth-manager/index.rst:
##
@@ -170,8 +170,76 @@ cookie named ``_token`` before redirecting to the Airflow
UI. The Airflow UI wil
return response
.. note::
-Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
+ Ensure that the cookie parameter ``httponly`` is set to ``True``. UI no
longer manages the token.
Review Comment:
I am not sure we should have "no longer" in the doc. We always try to avoid
comparing version in docs, just mention how the state is
##
airflow-core/docs/core-concepts/auth-manager/index.rst:
##
@@ -170,8 +170,76 @@ cookie named ``_token`` before redirecting to the Airflow
UI. The Airflow UI wil
return response
.. note::
-Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
+ Ensure that the cookie parameter ``httponly`` is set to ``True``. UI no
longer manages the token.
+Refreshing JWT Token
+
+Refreshing token is optional feature and its availability depends on the
specific implementation of the auth manager.
+The auth manager is responsible for refreshing the JWT token when it expires.
+The
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2611839251 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -170,8 +170,76 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil return response .. note:: -Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. + Ensure that the cookie parameter ``httponly`` is set to ``True``. UI no longer manages the token. +Refreshing JWT Token + +Refreshing token is optional feature and its availability depends on the specific implementation of the auth manager. +The auth manager is responsible for refreshing the JWT token when it expires. +The Airflow API uses middleware that intercepts every request and checks the validity of the JWT token. +Token communication is handled through ``httponly`` cookies to improve security. +When the token expires, the middleware calls the auth manager's ``refresh_token`` method to obtain a new token. + +To support token refresh operations, the auth manager must implement the ``refresh_token`` method. Review Comment: ```suggestion To support token refresh operations, the auth manager must implement the ``refresh_user`` method. ``` -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2611820468 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -170,8 +170,76 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil return response .. note:: -Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. + Ensure that the cookie parameter ``httponly`` is set to ``True``. UI no longer manages the token. Review Comment: I think this also needs to change. I initially considered a separate PR without backporting, but since this is after 3.1.1 and v-3-1-test is already past 3.1.4, it should be fine. -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on code in PR #54196:
URL: https://github.com/apache/airflow/pull/54196#discussion_r2611814601
##
airflow-core/docs/core-concepts/auth-manager/index.rst:
##
@@ -172,6 +174,60 @@ cookie named ``_token`` before redirecting to the Airflow
UI. The Airflow UI wil
.. note::
Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
+Refreshing JWT Token
+
+The refresh token logic is to automatically refresh the JWT token when it is
about to expire.
+The auth manager should implement ``get_url_refresh`` method to return the URL
of the refresh token endpoint.
+
+It requires the user to be authenticated, and it is usually called by the
Airflow UI/API when the JWT token is about to expire.
+This endpoint is used to refresh the JWT token when it is about to expire.
+The auth manager should implement this endpoint to allow the Airflow UI/API to
refresh the JWT token.
+If the auth manager does not implement this endpoint, the Airflow UI/API will
not be able to refresh the JWT token.
+The user will be logged out when the JWT token expires in that case, and they
will have to log in again.
+
+This procedure is following the same pattern as the initial token generation
endpoints and login/logout logic.
+
+If the auth manager have a token which expires and need to be refreshed, it
should override the endpoint.
+
+Example token structure below shows that we need to refresh the token via
using the ``refresh_token`` key in the token dict.
+This is example and the names can be different in your auth manager
implementation.
+If this is not the case, auth manager don't need to implement the refresh
token endpoint.
+
+.. code-block:: python
+
+ token = {
+ "access_token": "ACCESS_TOKEN",
+ "refresh_token": "REFRESH_TOKEN",
+ "param1": "value1",
+ "param2": "value2",
+ "...": "...",
+ }
+
+
+A typical implementation of the refresh token endpoint would look like this:
+
+
+.. code-block:: python
+
[email protected]("/auth/token/refresh")
+def refresh_token(
+request: Request,
+user: T = Depends(get_current_user),
+) -> TokenResponse:
+"""
+Refresh the JWT token for the current user.
+"""
+# Generate a new token for the user
+new_token = auth_manager.generate_token(user) # Or similar with
calling the client from auth manager
+
+# Set the new token in the cookie
+secure = request.base_url.scheme == "https" or bool(conf.get("api",
"ssl_cert", fallback=""))
+response = RedirectResponse(url="/")
+response.set_cookie(COOKIE_NAME_JWT_TOKEN, new_token, secure=secure)
+
+return response
+.. note::
+Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
Review Comment:
Updated the doc :)
--
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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
pierrejeambrun commented on code in PR #54196:
URL: https://github.com/apache/airflow/pull/54196#discussion_r2610970544
##
airflow-core/docs/core-concepts/auth-manager/index.rst:
##
@@ -172,6 +174,60 @@ cookie named ``_token`` before redirecting to the Airflow
UI. The Airflow UI wil
.. note::
Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
+Refreshing JWT Token
+
+The refresh token logic is to automatically refresh the JWT token when it is
about to expire.
+The auth manager should implement ``get_url_refresh`` method to return the URL
of the refresh token endpoint.
+
+It requires the user to be authenticated, and it is usually called by the
Airflow UI/API when the JWT token is about to expire.
+This endpoint is used to refresh the JWT token when it is about to expire.
+The auth manager should implement this endpoint to allow the Airflow UI/API to
refresh the JWT token.
+If the auth manager does not implement this endpoint, the Airflow UI/API will
not be able to refresh the JWT token.
+The user will be logged out when the JWT token expires in that case, and they
will have to log in again.
+
+This procedure is following the same pattern as the initial token generation
endpoints and login/logout logic.
+
+If the auth manager have a token which expires and need to be refreshed, it
should override the endpoint.
+
+Example token structure below shows that we need to refresh the token via
using the ``refresh_token`` key in the token dict.
+This is example and the names can be different in your auth manager
implementation.
+If this is not the case, auth manager don't need to implement the refresh
token endpoint.
+
+.. code-block:: python
+
+ token = {
+ "access_token": "ACCESS_TOKEN",
+ "refresh_token": "REFRESH_TOKEN",
+ "param1": "value1",
+ "param2": "value2",
+ "...": "...",
+ }
+
+
+A typical implementation of the refresh token endpoint would look like this:
+
+
+.. code-block:: python
+
[email protected]("/auth/token/refresh")
+def refresh_token(
+request: Request,
+user: T = Depends(get_current_user),
+) -> TokenResponse:
+"""
+Refresh the JWT token for the current user.
+"""
+# Generate a new token for the user
+new_token = auth_manager.generate_token(user) # Or similar with
calling the client from auth manager
+
+# Set the new token in the cookie
+secure = request.base_url.scheme == "https" or bool(conf.get("api",
"ssl_cert", fallback=""))
+response = RedirectResponse(url="/")
+response.set_cookie(COOKIE_NAME_JWT_TOKEN, new_token, secure=secure)
+
+return response
+.. note::
+Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs
to access the JWT token from the cookie.
Review Comment:
we need to update that part, it's not accurate anymore
--
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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
pierrejeambrun commented on PR #54196: URL: https://github.com/apache/airflow/pull/54196#issuecomment-3613101848 I removed my request for change. Things have been discussed, implemented and stabilized, we're ready for a doc update :) -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on PR #54196: URL: https://github.com/apache/airflow/pull/54196#issuecomment-3607638543 Okay it is already time. I will update the doc in a day or two. Finishing some CI work to lose dependency on v3 test branches on integration tests -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
github-actions[bot] commented on PR #54196: URL: https://github.com/apache/airflow/pull/54196#issuecomment-3604482752 This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on PR #54196: URL: https://github.com/apache/airflow/pull/54196#issuecomment-3324895060 Not stale, will update after the solution merged -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
github-actions[bot] commented on PR #54196: URL: https://github.com/apache/airflow/pull/54196#issuecomment-3321960261 This pull request has been automatically marked as stale because it has not had recent activity. It will be closed in 5 days if no further activity occurs. Thank you for your contributions. -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2263615409 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,40 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token `` `` is ``POST /auth/token/refresh``. It returns a new JWT token in the cookie where updated token is stored. +It requires the user to be authenticated, and it is usually called by the Airflow UI/API when the JWT token is about to expire. +This endpoint is used to refresh the JWT token when it is about to expire. +The auth manager should implement this endpoint to allow the Airflow UI/API to refresh the JWT token. Review Comment: Make sense! added example and mentioned that this is not needed in case the logic is not there and gave the example for the keys similar to Keycloak, but mentioned that they are tentative and can be different -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2263614098 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,40 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token `` `` is ``POST /auth/token/refresh``. It returns a new JWT token in the cookie where updated token is stored. Review Comment: Removed the path and included the method. Also included the method in `Authentication related methods` title. -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
vincbeck commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2262946655 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,40 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token `` `` is ``POST /auth/token/refresh``. It returns a new JWT token in the cookie where updated token is stored. +It requires the user to be authenticated, and it is usually called by the Airflow UI/API when the JWT token is about to expire. +This endpoint is used to refresh the JWT token when it is about to expire. +The auth manager should implement this endpoint to allow the Airflow UI/API to refresh the JWT token. Review Comment: We should also mention why an auth manager should override this method. If the auth manager does not use any underlying token underneath, there is no value implementing this method. An auth manager should override this method if it uses a token which expires and need to be refreshed (like Keycloak) ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,40 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token `` `` is ``POST /auth/token/refresh``. It returns a new JWT token in the cookie where updated token is stored. Review Comment: I do not think it is needed to mention the path, each auth manager can come up with their own path. It does not really matter because Airflow can get this path with `get_url_refresh`. By the way, it would be great to mention this method in the doc, saying that if the auth manager provide an endpoint to refresh the token, it should be returned with `get_url_refresh` -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2261662432 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,39 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token endpoint is ``POST /auth/token/refresh`` with the current JWT token in the cookie or acquired from the API. Review Comment: I split the sentence into multiple pieces. I beleive this is more understanable ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,39 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token endpoint is ``POST /auth/token/refresh`` with the current JWT token in the cookie or acquired from the API. Review Comment: I split the sentence into multiple pieces. I believe this is more understandable -- 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]
Re: [PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
vincbeck commented on code in PR #54196: URL: https://github.com/apache/airflow/pull/54196#discussion_r2258279911 ## airflow-core/docs/core-concepts/auth-manager/index.rst: ## @@ -172,6 +172,39 @@ cookie named ``_token`` before redirecting to the Airflow UI. The Airflow UI wil .. note:: Do not set the cookie parameter ``httponly`` to ``True``. Airflow UI needs to access the JWT token from the cookie. +Refreshing JWT Token + +The refresh token endpoint is ``POST /auth/token/refresh`` with the current JWT token in the cookie or acquired from the API. Review Comment: Not sure I understand this sentence -- 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]
[PR] feat(doc): Add Refresh Token logic to auth manager docs [airflow]
bugraoz93 opened a new pull request, #54196:
URL: https://github.com/apache/airflow/pull/54196
follow up docs
relates: #51657
---
**^ Add meaningful description above**
Read the **[Pull Request
Guidelines](https://github.com/apache/airflow/blob/main/contributing-docs/05_pull_requests.rst#pull-request-guidelines)**
for more information.
In case of fundamental code changes, an Airflow Improvement Proposal
([AIP](https://cwiki.apache.org/confluence/display/AIRFLOW/Airflow+Improvement+Proposals))
is needed.
In case of a new dependency, check compliance with the [ASF 3rd Party
License Policy](https://www.apache.org/legal/resolved.html#category-x).
In case of backwards incompatible changes please leave a note in a
newsfragment file, named `{pr_number}.significant.rst` or
`{issue_number}.significant.rst`, in
[airflow-core/newsfragments](https://github.com/apache/airflow/tree/main/airflow-core/newsfragments).
--
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]
