Bug: Incorrect Handling of Timestamps in exp, nbf, and iat Claims in fast-jwt

[rajibchy]

Description

There is an issue in fast-jwt where the exp, nbf, and iat claims are incorrectly calculated when expiresIn is provided in seconds. Specifically, the issue arises when payload.iat is in milliseconds, but expiresIn is in seconds, which results in incorrect expiration calculations.

Steps to Reproduce

1. Create a JWT with expiresIn in seconds.
2. Ensure that payload.iat is set (or not set) and potentially in milliseconds.
3. Observe that the exp and nbf values are incorrectly calculated if iat is in milliseconds.

Expected Behavior

• The exp, nbf, and iat claims should all be calculated correctly with consistent units (typically in seconds).
• The calculation should correctly handle cases where iat is in milliseconds.

Actual Behavior

• When payload.iat is in milliseconds, and expiresIn is given in seconds, the exp (expiration) and nbf (not before) values are incorrectly calculated due to a mismatch in units.

Code Snippet

The issue is occurring in the following code:

const finalPayload = {
    ...payload,
    ...fixedPayload,
    iat: noTimestamp ? undefined : Math.floor(iat / 1000),  // this line
    exp: payload.exp ? payload.exp : expiresIn ? Math.floor((iat + expiresIn) / 1000) : undefined,  // this line
    nbf: payload.nbf ? payload.nbf : notBefore ? Math.floor((iat + notBefore) / 1000) : undefined  // this line
}

Suggested Fix

• To solve the issue of inconsistent timestamp units, the iat should be converted to seconds before performing calculations for exp and nbf. Here’s the corrected solution:

const iat = payload.iat * 1000 || clockTimestamp || Date.now(); // Handle `iat` in milliseconds
const iatSec = (iat / 1000); // Convert `iat` to seconds

const finalPayload = {
    ...payload,
    ...fixedPayload,
    iat: noTimestamp ? undefined : Math.floor(iatSec),
    exp: payload.exp ? payload.exp : expiresIn ? Math.floor(iatSec + expiresIn) : undefined,
    nbf: payload.nbf ? payload.nbf : notBefore ? Math.floor(iatSec + notBefore) : undefined
}

Explanation:

• iat is originally in milliseconds (as it's calculated by Date.now()), so it is first converted to seconds (iatSec).
• The exp (expiration) and nbf (not before) are then calculated using iatSec, ensuring all timestamp calculations are in seconds.
• This fix resolves the inconsistency in units between expiresIn (in seconds) and iat (in milliseconds).

Environment

• fast-jwt version: 5.0.5
• Node.js version: v20.9.0
• OS: Windows X64

Additional Information

• The issue may not be immediately apparent unless the iat is not manually set and the expiresIn is in seconds.
• This issue leads to incorrect expiration times if the iat is in milliseconds.

Link to Source Code

You can find the relevant code in the fast-jwt repository here: fast-jwt Signer.js - Line 93.

[dosubot]

Hey, @rajibchy! I'm here to help you with any bugs, questions, or contributions. Let's work together to resolve the issue you're facing.

The issue you've described involves a mismatch in units between the iat claim, which is in milliseconds, and the expiresIn parameter, which is in seconds. This can lead to incorrect calculations for the exp and nbf claims.

Your suggested fix is on the right track. By converting iat to seconds before using it in calculations for exp and nbf, you ensure consistency in units. This approach should resolve the issue of incorrect timestamp calculations.

Here's a summary of the key changes:

1. Convert iat from milliseconds to seconds using iatSec = iat / 1000.
2. Use iatSec for calculating exp and nbf to maintain consistent units.

This ensures that all time-related calculations are performed in seconds, aligning with the expiresIn parameter. Your code snippet correctly implements this fix, and it should address the problem effectively.

To continue talking to Dosu, mention @dosu.

---

^{Help Dosu learn! Give it feedback: Great Response | Irrelevant Answer | Incorrect Sources | Too Verbose | Hallucination | Bug Report | Other}

[simoneb]

@rajibchy thanks for reporting, but I'm not sure I understand the issue. The documentation says that expiresIn should be provided in milliseconds, so it's somewhat implied that if you provide it in seconds, it won't work correctly.

I'm also not sure how the fix addresses the issue, it looks like you're simply assuming that expiresIn should be populated in seconds, which contradicts the documentation.

[rajibchy]

Oh.. Yes are you right according to documentation.

expiresIn: Time span (in milliseconds or text describing time) after which the token expires, added as the exp claim in the payload as defined by the [section 4.1.4 of RFC 7519](https://datatracker.ietf.org/doc/html/rfc7519#section-4.1.4). This will override any existing value in the claim.