Skip to content

Commit

Permalink
Add support for pyjwt leeway
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sirosen committed Jul 19, 2023
1 parent 9557bd8 commit 951020f
Show file tree
Hide file tree
Showing 3 changed files with 43 additions and 1 deletion.
12 changes: 12 additions & 0 deletions changelog.d/20230719_120125_sirosen_support_jwt_leeway.rst
Original file line number Diff line number Diff line change
@@ -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`)
10 changes: 9 additions & 1 deletion src/globus_sdk/services/auth/response/oauth.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
from __future__ import annotations

import datetime
import json
import logging
import textwrap
Expand Down Expand Up @@ -190,14 +191,20 @@ 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"])
auth_client = t.cast("AuthClient", self.client)

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(
Expand All @@ -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
Expand Down
22 changes: 22 additions & 0 deletions tests/functional/services/auth/test_id_token.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import json
import time

import jwt
import pytest
Expand Down Expand Up @@ -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():
Expand Down Expand Up @@ -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"

0 comments on commit 951020f

Please sign in to comment.