From e34974f451542d2970984d03306a7393ea60a214 Mon Sep 17 00:00:00 2001 From: Stephen Rosen Date: Thu, 20 Jul 2023 14:43:14 -0500 Subject: [PATCH] Add support for pyjwt `leeway` (#790) On the surface, this may look like a potentially incompatible change because we remove `leeway` from the passed `jwt_params`. However, those are passed to `options` and `leeway` isn't a supported value there for pyjwt, so the change is in effect strictly additive. pyjwt source has a comment indicating that `leeway` might be added to `options` in the future (it would make sense), along with values we control like `audience`. For the time being, however, this makes sense as a mechanism for passing `leeway` for JWT handling in the SDK. Because the same `leeway` is used for the `iat`, `nbf`, and `exp` claims, we can check that `leeway` is passed correctly by using it to make a very old `exp` claim pass validation in our tests. A new default is set for `leeway` of 0.5s internally. This is not part of the `decode_id_token` docs -- kept as an implementation detail -- but it makes the default behavior slightly more tolerant of clock drift. As such, this part of the change is documented as a fix in the changelog, whereas the rest is an addition. --- ...0719_120125_sirosen_support_jwt_leeway.rst | 12 ++++++++++ .../services/auth/response/oauth.py | 10 ++++++++- .../functional/services/auth/test_id_token.py | 22 +++++++++++++++++++ 3 files changed, 43 insertions(+), 1 deletion(-) create mode 100644 changelog.d/20230719_120125_sirosen_support_jwt_leeway.rst diff --git a/changelog.d/20230719_120125_sirosen_support_jwt_leeway.rst b/changelog.d/20230719_120125_sirosen_support_jwt_leeway.rst new file mode 100644 index 000000000..3ce57fb97 --- /dev/null +++ b/changelog.d/20230719_120125_sirosen_support_jwt_leeway.rst @@ -0,0 +1,12 @@ +Added +~~~~~ + +- The ``jwt_params`` argument to ``decode_id_token()`` now allows ``"leeway"`` + to be included to pass a ``leeway`` parameter to pyjwt. (:pr:`NUMBER`) + +Fixed +~~~~~ + +- ``decode_id_token()`` defaulted to having no tolerance for clock drift. Slight + clock drift could lead to JWT claim validation errors. The new default is + 0.5s which should be sufficient for most cases. (:pr:`NUMBER`) diff --git a/src/globus_sdk/services/auth/response/oauth.py b/src/globus_sdk/services/auth/response/oauth.py index 2d79f32b0..d5657e02d 100644 --- a/src/globus_sdk/services/auth/response/oauth.py +++ b/src/globus_sdk/services/auth/response/oauth.py @@ -1,5 +1,6 @@ from __future__ import annotations +import datetime import json import logging import textwrap @@ -190,7 +191,8 @@ def decode_id_token( will be fetched and parsed automatically. :type jwk: RSAPublicKey :param jwt_params: An optional dict of parameters to pass to the jwt decode - step. These are passed verbatim to the jwt library. + step. If ``"leeway"`` is included, it will be passed as the ``leeway`` + parameter, and all other values are passed as ``options``. :type jwt_params: dict """ logger.info('Decoding ID Token "%s"', self["id_token"]) @@ -198,6 +200,11 @@ def decode_id_token( jwt_params = jwt_params or {} + jwt_leeway: float | datetime.timedelta = 0.5 + if "leeway" in jwt_params: + jwt_params = jwt_params.copy() + jwt_leeway = jwt_params.pop("leeway") + if not openid_configuration: if jwk: raise exc.GlobusSDKUsageError( @@ -222,6 +229,7 @@ def decode_id_token( algorithms=signing_algos, audience=auth_client.client_id, options=jwt_params, + leeway=jwt_leeway, ) logger.debug("decode ID token finished successfully") return decoded diff --git a/tests/functional/services/auth/test_id_token.py b/tests/functional/services/auth/test_id_token.py index 5bef26047..0684b7532 100644 --- a/tests/functional/services/auth/test_id_token.py +++ b/tests/functional/services/auth/test_id_token.py @@ -1,4 +1,5 @@ import json +import time import jwt import pytest @@ -69,6 +70,9 @@ "id_token": "eyJ0eXAiOiJKV1QiLCJhbGciOiJSUzUxMiJ9.eyJzdWIiOiJjOGFhZDQzZS1kMjc0LTExZTUtYmY5OC04YjAyODk2Y2Y3ODIiLCJvcmdhbml6YXRpb24iOiJHbG9idXMiLCJuYW1lIjoiU3RlcGhlbiBSb3NlbiIsInByZWZlcnJlZF91c2VybmFtZSI6InNpcm9zZW4yQGdsb2J1c2lkLm9yZyIsImlkZW50aXR5X3Byb3ZpZGVyIjoiNDExNDM3NDMtZjNjOC00ZDYwLWJiZGItZWVlY2FiYTg1YmQ5IiwiaWRlbnRpdHlfcHJvdmlkZXJfZGlzcGxheV9uYW1lIjoiR2xvYnVzIElEIiwiZW1haWwiOiJzaXJvc2VuQHVjaGljYWdvLmVkdSIsImxhc3RfYXV0aGVudGljYXRpb24iOjE2MjE0ODEwMDYsImlkZW50aXR5X3NldCI6W3sic3ViIjoiYzhhYWQ0M2UtZDI3NC0xMWU1LWJmOTgtOGIwMjg5NmNmNzgyIiwib3JnYW5pemF0aW9uIjoiR2xvYnVzIiwibmFtZSI6IlN0ZXBoZW4gUm9zZW4iLCJ1c2VybmFtZSI6InNpcm9zZW4yQGdsb2J1c2lkLm9yZyIsImlkZW50aXR5X3Byb3ZpZGVyIjoiNDExNDM3NDMtZjNjOC00ZDYwLWJiZGItZWVlY2FiYTg1YmQ5IiwiaWRlbnRpdHlfcHJvdmlkZXJfZGlzcGxheV9uYW1lIjoiR2xvYnVzIElEIiwiZW1haWwiOiJzaXJvc2VuQHVjaGljYWdvLmVkdSIsImxhc3RfYXV0aGVudGljYXRpb24iOjE2MjE0ODEwMDZ9LHsic3ViIjoiYjZlMjI3ZTgtZGI1Mi0xMWU1LWI2ZmYtYzNiMWNjMjU5ZTBkIiwibmFtZSI6bnVsbCwidXNlcm5hbWUiOiJzaXJvc2VuK2JhZGVtYWlsQGdsb2J1cy5vcmciLCJpZGVudGl0eV9wcm92aWRlciI6IjkyN2Q3MjM4LWY5MTctNGViMi05YWNlLWM1MjNmYTliYTM0ZSIsImlkZW50aXR5X3Byb3ZpZGVyX2Rpc3BsYXlfbmFtZSI6Ikdsb2J1cyBTdGFmZiIsImVtYWlsIjoic2lyb3NlbitiYWRlbWFpbEBnbG9idXMub3JnIiwibGFzdF9hdXRoZW50aWNhdGlvbiI6bnVsbH0seyJzdWIiOiJmN2Y4OWQwYS1kYzllLTExZTUtYWRkMC1hM2NiZDFhNTU5YjMiLCJuYW1lIjpudWxsLCJ1c2VybmFtZSI6InNpcm9zZW4rYmFkZW1haWwyQGdsb2J1cy5vcmciLCJpZGVudGl0eV9wcm92aWRlciI6IjkyN2Q3MjM4LWY5MTctNGViMi05YWNlLWM1MjNmYTliYTM0ZSIsImlkZW50aXR5X3Byb3ZpZGVyX2Rpc3BsYXlfbmFtZSI6Ikdsb2J1cyBTdGFmZiIsImVtYWlsIjoic2lyb3NlbitiYWRlbWFpbDJAZ2xvYnVzLm9yZyIsImxhc3RfYXV0aGVudGljYXRpb24iOm51bGx9XSwiaXNzIjoiaHR0cHM6Ly9hdXRoLmdsb2J1cy5vcmciLCJhdWQiOiI3ZmI1OGUwMC04MzlkLTQ0ZTMtODA0Ny0xMGE1MDI2MTJkY2EiLCJleHAiOjE2MjE2NTM4MTEsImlhdCI6MTYyMTQ4MTAxMSwiYXRfaGFzaCI6IjFQdlVhbmNFdUxfc2cxV1BsNWx1TUVGR2tjTDZQaDh1cWdpVUZzejhkZUEifQ.CtfnFtfM32ICo0euHv9GnpVHFL1jWz0NriPTXAv6w08Ylk9JBJtmB3oMKNSO-1TGoWUPFDp9TFFk6N32VyF0hsVDtT5DT3t5oq0qfqbPrZA3R04HARW0xtcK_ejNDHBmj6wysey3EzjT764XTvcGOe63CKQ_RJm97ulVaseIT0Aet7AYo5tQuOiSOQ70xzL7Oax3W6TrWi3FIAA-PIMSrAJKbsG7imGOVkaIObG9a-X5yTOcrB4IG4Wat-pN_QiCiiOw_LDCF-r455PwalmnSGUugMYfsdL2k3UxqwOMLIppHnx5-UVAzj3mygj8eZTp6imjqxNMdakS3vhG8dtxbw", # noqa: E501 } +# this is the 'exp' value encoded above +FIXED_JWT_EXPIRATION_TIME = 1621653811 + @pytest.fixture def client(): @@ -130,3 +134,21 @@ def test_decode_id_token_with_saved_oidc_config_and_jwk(token_response): def test_invalid_decode_id_token_usage(token_response): with pytest.raises(globus_sdk.exc.GlobusSDKUsageError): token_response.decode_id_token(jwk=JWK_PEM, jwt_params={"verify_exp": False}) + + +def test_decode_id_token_with_leeway(token_response): + register_api_route( + "auth", + "/.well-known/openid-configuration", + method="GET", + body=json.dumps(OIDC_CONFIG), + ) + register_api_route("auth", "/jwk.json", method="GET", body=json.dumps(JWK)) + + # do a decode with a leeway parameter set high enough that the ancient + # expiration time will be tolerated + expiration_delta = time.time() - FIXED_JWT_EXPIRATION_TIME + decoded = token_response.decode_id_token( + jwt_params={"leeway": expiration_delta + 1} + ) + assert decoded["preferred_username"] == "sirosen2@globusid.org"