Skip to content

feat: allow to configure JWT leeway in open_id_connect and increase default to 1 second #1031

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

jirutka
Copy link

@jirutka jirutka commented Mar 5, 2025

Proposed changes

The default value defined in the jwt module is 0 seconds, so when the time is off just by a few milliseconds, the token validation fails with error "The token is not yet valid (iat)".

Checklist

@nijel
Copy link
Member

nijel commented Mar 13, 2025

Isn't ID_TOKEN_MAX_AGE supposed to cover this? It might be a caused by 013d27d where jose and pyjwt might behave differently in this.

@jirutka
Copy link
Author

jirutka commented Mar 13, 2025

No, ID_TOKEN_MAX_AGE has a different purpose. As you can see in the code base, default value for ID_TOKEN_MAX_AGE is 600 and 3600 seconds, that would be too high for the leeway parameter. JWT_LEEWAY is used in JWT validation to allow a small amount of clock drift when verifying token expiration (exp), not-before (nbf), and issued-at (iat) claims.

jirutka added 2 commits March 13, 2025 21:37
The default value defined in the jwt module is 0.
The default value defined in the jwt module is 0 seconds, so when the
time is off just by a few milliseconds, the token validation fails with
error "The token is not yet valid (iat)".
@nijel
Copy link
Member

nijel commented Mar 14, 2025

that would be too high for the leeway parameter.

Having a wrong value doesn't mean it was not built with the same purpose. Apparently, ID_TOKEN_MAX_AGE is used to verify the iat claim (and is not applied to the nbf claim):

if "nbf" in id_token and utc_timestamp < id_token["nbf"]:
raise AuthTokenError(self, "Incorrect id_token: nbf")
# Verify the token was issued in the last 10 minutes
iat_leeway = self.setting("ID_TOKEN_MAX_AGE", self.ID_TOKEN_MAX_AGE)
if utc_timestamp > id_token["iat"] + iat_leeway:
raise AuthTokenError(self, "Incorrect id_token: iat")

If this validation is now done inside the JWT library, the additional logic probably can be removed and consolidated across the backends:

  • OpenIdConnectAuth.validate_claims seems to duplicate the
  • LinkedinOpenIdConnect seems to have a copy of this as well
  • MediaWiki is passing leeway parameter to jwt.decode and doing manual validation later (but this one is not a OpenIdConnect subclass)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants