client.expired() seems to not detect expiration anymore

[RobinJayaswal]

Just updated to latest version of SDK, and am running into the following issue.

Here is my code

// I retrieve an access obj up here
console.log(Date.now());
console.log(access);
if (client.expired(access)) {
      console.log('Expired');
      return client.exchangeToken(access.refresh_token)
      .then(function(newAccess) {
        ....
} else {
    console.log('Not Expired');
    return access
}

This prints:

1482199722437
{ access_token: '60826ece-faf7-4f25-abde-b0c9f9fc1222',
  token_type: 'Bearer',
  expires_in: 7200,
  refresh_token: '40751cf8-d2ac-4fb3-8f24-9f5c2c31e692',
  created_at: 1481916486864 }
Not Expired

The Unix timestamps are 3 days apart, so it should print Expired. Instead the code continues and I get the following error as soon as I try to use the access token:

{ authentication_error: Invalid or expired token provided: 60826ece-faf7-4f25-abde-b0c9f9fc1222
  at /mnt/c/Users/robinjayaswal/Documents/GitHub/iotUIClient/serverless_site/node_modules/smartcar-sdk/lib/util.js:53:15
  at tryCatcher ...

I see in line 103 of index.js that the check for expiration is this:

/**
 * check if an access object's access token is expired
 * @param  {Access} access access object to be checked
 * @return {Boolean} true if expired, false if not expired
 */
Client.prototype.expired = function(access) {
  return (Date.now() / 1000) > (access.created_at + access.expires_in);
};

Is there a reason Date.now() is divided by 1000?

[sankethkatta]

Okay this explains the odd behavior I was having with the messenger bot. It seems to make me re-login every time I use it. I just get back "cannot complete action right now". It must be sending requests with an expired token.

@gurpreetatwal refactored this to make it more readable. I thought Date.now() was divided because it returned milliseconds instead of seconds.

I think the problem is that when created_at is set: https://github.com/smartcar/node-sdk/blob/5cc1e72067292a539a5fa43d294869b01593b6f8/lib/util.js#L19 it is in milliseconds, then when when expires_in is added it is in seconds.

I think if we set it in properly in seconds in util.js. Or convert everything to milliseconds that should also work:

return Date.now() > access.created_at + (access.expires_in * 1000);

[gurpreetatwal]

Whoops, that was my bad. I had assumed that created_at was in seconds being that it is underscore cased like expires_in.

Should we standardize on ms or seconds?

I'd say if we go with ms we should camelcase the property so it appears different from the standard oatuh properties

[sankethkatta]

I say we can go seconds, as milliseconds don't really matter here if the expires_in is in seconds anyways.

Or another option is to set an expiration property instead of created_at and also remove the expires_in. Then all we need to store is access_token, refresh_token, and expiration. And there's no additions that need to be done at compare time.

[sankethkatta]

Use ISO string for expiration: https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString