Keats / jsonwebtoken

JWT lib in rust
MIT License
1.66k stars 266 forks source link

Add support for negative leeway values #365

Closed ploftness-tesla closed 6 months ago

ploftness-tesla commented 8 months ago

I use jsonwebtoken as part of a client application. During the course of development, I have encountered two issues with how token expiration is handled:

  1. The Validation::get_current_timestamp() method truncates the current time to the previous second. This behaves differently from a library in a different language used by a server application that the client interacts with. This library considers a token to be expired if the current time is 0.001 seconds past the expiration in the token, rather than 1.000 seconds past.
  2. The Validation::validate() method does not provide a way to judge if a token is about to expire shortly. This becomes a problem if a token will expire in 0.001 seconds and the code is preparing to send it over a network socket. It may expire in-transit.

In order to allow library users to work around both these problems, I propose the following solution.

  1. Split the leeway option into separate settings for validating token exp and nbf claims. These settings can be named exp_leeway and nbf_leeway.
  2. Allow exp_leeway values to be negative numbers. This allows library users to specify that a token needs to be replaced at some time interval before expiration to account for inconsistent handling of fractional time values by other software or delays imposed by network latency.
  3. Allow nbf_leeway values to be negative numbers for consistency.

Update: Here is a PR implementing the proposed solution.

Keats commented 8 months ago
  1. I don't see why we would go to ms resolution when we only have seconds in the api/sec
  2. Not an issue with a decent leeway, that's what it's for
ploftness-tesla commented 8 months ago

@Keats thanks for the quick response!

  1. I'm not proposing that we go to ms resolution. My proposed fix is in the attached PR.
  2. I have not been able to identify a leeway value that fixes the following issue. Here is a drawing representing a token which is about to expire, but has not expired yet.

    positive-leeway (1)

    I would like to make the current code fail for this case since the token may expire in transit over the network.

    if matches!(claims.exp, TryParse::Parsed(exp) if options.validate_exp && exp < now - options.leeway)
    {
       return Err(new_error(ErrorKind::ExpiredSignature));
    }

    It looks like it will succeed for all unsigned integers.

    positive-leeway-series

    Can you suggest a leeway value that will allow me to return ErrorKind::ExpiredSignature when the token is about to expire? As a specific example, maybe you could suggest a value that would work when time now=1 and time exp=2.

Keats commented 8 months ago

It seems that what you want, rather than negative leeway which is imo a bit confusing is another option to reject tokens that are x seconds or less from expiration?

ploftness-tesla commented 8 months ago

It seems that what you want, rather than negative leeway which is imo a bit confusing is another option to reject tokens that are x seconds or less from expiration?

@Keats yes, rejecting tokens that are x seconds or less prior to expiration is our goal.

Configuring this as a separate setting rather than as a negative leeway value should work fine.

Keats commented 8 months ago

Something like Validation::reject_tokens_expiring_in_less_than(seconds: int)?

ploftness-tesla commented 8 months ago

Something like Validation::reject_tokens_expiring_in_less_than(seconds: int)?

Yes, that is a clear name in my opinion.

Would you like me to update the existing PR to reflect?

ploftness-tesla commented 7 months ago

@Keats Here is a new PR implementing Validation::reject_tokens_expiring_in_less_than. Can you review and approve?

ploftness-tesla commented 6 months ago

Fix released as part of version 9.3.0; closing this issue