expiresIn is not working as expected, token is not expired or always expired when working with seconds

[syberkitten]

Any number of seconds provided to expiresIn -> the token never expires.

either expiresIn: 1 either expiresIn: "1s"

ps. does not matter if it's 1 second or more.

[syberkitten]

looks like the issue happens when not providing "clockTimestamp"

inside verify.js

If "clockTimestamp" is not provided, the default time is interpolated to seconds. Then later on line (175)

clockTimestamp is compared to maxAgeTimestamp but maxAgeTimestamp is always in milli-seconds, while clockTimestamp is in seconds.

So it will never be greater then maxAgeTimestamp, therefore token is never expired!

Workaround: always provide clockTimestamp to verify method.

[syberkitten]

It also looks like when providing both expiresIn and maxAge in seconds, there is some problem with timespan.js where the diff is always translated to mili seconds.

supplying maxAge as 4 or '4s' and expiresIn as 10 or '10s' does not calculate maxAgeTimestamp properly,

[syberkitten]

it is easy to reproduce:

1. sign a token with expiresAt: '10s'
2. do a sleep(3) - (optional)
3. verify it with maxAge: '4s' (lets call it variable x)

token will always be expired, when debugging you can see that payload.iat and payload.exp are always the same.

[syberkitten]

all the issues lead to timespan.js: I've commented below the problem.

var ms = require('ms');

module.exports = function (time, iat) {

   // Is mili seconds or seconds? iat is most likely in ms.
  var timestamp = iat || Math.floor(Date.now() / 1000);

  if (typeof time === 'string') {
    var milliseconds = ms(time);
    if (typeof milliseconds === 'undefined') {
      return;
    }

   // Why convert to seconds, timestamp could be mili ?? 

    return Math.floor(timestamp + milliseconds / 1000);
  } else if (typeof time === 'number') {
    return timestamp + time;
  } else {
    return;
  }

};

[mvilrokx]

I've just discovered this exact same issue. As per @syberkitten's comments, iat is in ms so it should be converted to seconds as well, just like Date.now().

[MitMaro]

iat is in ms so it should be converted to seconds as well

Assuming I'm understanding the issue correctly, iat should never be provided in milliseconds, but should be provided in seconds. The docs state:

Remember that exp, nbf and iat are NumericDate

NumericDate as per RFC 7519 is defined as:

NumericDate A JSON numeric value representing the number of seconds from 1970-01-01T00:00:00Z UTC until the specified UTC date/time, ignoring leap seconds. This is equivalent to the IEEE Std 1003.1, 2013 Edition [POSIX.1] definition "Seconds Since the Epoch", in which each day is accounted for by exactly 86400 seconds, other than that non-integer values can be represented. See RFC 3339 [RFC3339] for details regarding date/times in general and UTC in particular.

In this case, I think this library is working correctly.

[mvilrokx]

Yeah, that is what I ended up doing. I'm happy for this issue to be closed.

[ziluvatar]

Closing, @syberkitten re-open it if you still have doubts.

[mvilrokx]

Is there any way this can be documented somewhere?

[ziluvatar]

It is in the README. Could we modified it anyhow to make it clearer?