potiuk commented on code in PR #24399:
URL: https://github.com/apache/airflow/pull/24399#discussion_r903431656
##########
airflow/utils/jwt_signer.py:
##########
@@ -73,9 +73,7 @@ def verify_token(self, token: str) -> Dict[str, Any]:
algorithms=[self._algorithm],
options={
"verify_signature": True,
- "require_exp": True,
- "require_iat": True,
- "require_nbf": True,
+ "require": ["exp", "iat", "nbf"],
Review Comment:
It's a change in PyJWT 1-> 2. I mentioned it in the commit message:
```
switch to PyJWT 2.* by using non-deprecated "required" claim as
list rather than separate fields
```
But the detailed link is here:
https://pyjwt.readthedocs.io/en/stable/changelog.html#dropped-deprecated-require-options-in-jwt-decode
The problem was that PyJWT used in FAB 3 did not have it deprecated (and
dictionary of claims did not work - you hadd to use "require_iat" etc. - I
tried dict before but it was not implemented yet in the older PyJWT we had).
On the other hand, the PyJWT upgrade used in FAB 4 "jumps" the deprecation -
so PyJWT used by FAB not only deprecated the "require_*" parameters but also
managed to remove them altogether so it stopped working :). So I had to
implement it in old way in #24519 and switch to new way here.
Side comment: I am actually super glad I added tests for it when I
implemented it in #24519 , because the change was TERRIBLY breaking. Suddenly
your "requirements" stopped being respected and were suddenly dropped silently
(!) - no warning (opening the way to requests that had no validity
requirements). I was thinking about raising an issue to PyJWT. They should -
IMHO fail signers that still use require_nnn, rather than the dictionary - that
would have been much safer for their users. But it was a change impemented in
December, so that ship has long sailed.
--
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]
