Ability to save and restore token

[kirill-konshin]

Currently it's not possible to get token data, save it in persistent storage between application runs and then recover the token:

function ClientOAuth2Token (client, data) {
  this.client = client
  this.data = data
  this.tokenType = data.token_type && data.token_type.toLowerCase()
  this.accessToken = data.access_token
  this.refreshToken = data.refresh_token

  this.expiresIn(data.expires_in)
}

Constructor does not allow to pass data.expires.

Plus, this class does not handle expiration of refresh token.

Also, sign method should probably throw an error if user tries to sign with expired token.

[blakeembrey]

Everything is stored in data, what's wrong with reading it to save it? It can refresh, but it doesn't have any opinions on an expired token. That sort of functionality is better handled at an application layer where you might want to implement some more complicated form of token sharing/refreshing.

[blakeembrey]

Also, what's data.expires here?

[kirill-konshin]

The problem is that method ClientOAuth2Token.prototype.expired looks at this.expires. Every time the constructor is called it assumes that it's a fresh token. But constructor may also be used through ClientOAuth2.createToken to re-create previously saved token.

In this case user may want to pass data.expires which should be used as this.expires.

[kirill-konshin]

Also, as I said, there are no methods to handle refresh token expiration.

[blakeembrey]

I don't follow, what's wrong with the refresh method? Why not use expiresIn to set the expiration then? It could also be updated to accept a date if you don't want to do the math manually. The constructor won't accept expires because it's not part of the spec, but it's pretty simple to update it using the exposed methods. You could also set .expires directly I suppose.

[kirill-konshin]

Refresh method is fine. "That sort of functionality is better handled at an application layer where you might want to implement some more complicated form of token sharing/refreshing." -- sounds reasonable.

But refresh tokens also can expire and currently library does not provide anything to check it.

Regarding .expires. Exposed methods allow to set expiration relative to current time, which means more calculation on the client. Yes .expires can be set directly, but I don't think that this is a good solution.

Please advice a good way to store token data on client side. Note that page can be refreshed and token will have to be re-created.

[blakeembrey]

Can you point to the specification where we can handled refresh token expiration? I haven't come across anything and the specification says refresh tokens may expire when issuing a new access token but I haven't seen anything more specific to be able to handle it.

I can improve the expiresIn handling to accept dates, I think that's a good idea. For storing information, I'd store accessToken, refreshToken, tokenType and expires into localstorage. When you re-load, load them out of localstorage and re-create using https://github.com/mulesoft/js-client-oauth2#request-options. You can currently re-set expiresIn and if it's expired, try refreshing the information. The way I've been refreshing is by setting a timeout halfway between expires and now to auto-refresh - however, this may be more complicated when multiple tabs are open accessing the data, etc.

[kirill-konshin]

http://stackoverflow.com/questions/7030694/why-do-access-tokens-expire this answer also has some info regarding refresh tokens too. In general, some providers do expire refresh tokens and some don't (like Google). I'm not sure it's covered by OAuth2 specs, most likely it's not mandatory and spec does not enforce any certain naming...

Having an ability to pass a date object or a timestamp as expires parameter to createToken or expiresIn function would be great.

Regarding localStorage, right now this is a bit awkward:

var data = token.data;
data.expires = token.expires.getTime();
localStorage.setItem('oauth', JSON.stringify(data));

var data = JSON.parse(localStorage.getItem('oauth');
var token = ClientOAuth2.createToken(data.access_token, data.refresh_token, data.token_type, data);
data.expires = new Date(data.expires);

As you see you can't get/set all related info as one function call, so there is a short period of inconsistent token state between calls.

[kirill-konshin]

The way I've been refreshing is by setting a timeout halfway between expires and now to auto-refresh - however, this may be more complicated when multiple tabs are open accessing the data, etc.

In RingCentral JS SDK we treated it a bit differently. SDK does nothing until there is an API call and if needed -- it first refreshes the token and then resumes the API call.

We also used to have a special queue mechanism, that prevented simultaneous refreshes if one tab already had one: https://github.com/ringcentral/ringcentral-js/blob/2.0.6/src/platform/Platform.js#L300

Luckily, this is no longer needed because refresh tokens can be used more than once within very short period of time: they return same new access token when used, so if two tabs simultaneously refresh, they both will write same data to local storage. And next calls will get updated data from local storage because all synchronization is performed through there.

[blakeembrey]

Makes sense, I mostly do back-end where it'd be more expensive to refresh on demand (you'd be blocking dozens of users while you refresh), so I try to optimistically update tokens. It might be useful to be optimistic on the front-end too, if only because it makes that expired query a little faster.

Luckily, this is no longer needed because refresh tokens can be used more than once within very short period of time.

I think this may be subjective as the spec doesn't specify how refresh tokens should expire.

FWIW, you may be able to get a fancy workflow going if you're using local storage using "locks" on who is refreshing data. Two tabs can communicate using localstorage to do an update. Also, the tabs can communicate in real-time because it can trigger events too (I've done that in the past).

As you see you can't get/set all related info as one function call, so there is a short period of inconsistent token state between calls.

JavaScript is synchronous, so this can't be a problem. I agree the create method could be nicer. I'll think about how to make the API there nicer to use.

[kirill-konshin]

Two tabs can communicate using localstorage to do an update.

RC SDK works exactly like this. The queue had a mutex in localStorage.

I try to optimistically update tokens. It might be useful to be optimistic on the front-end too

I assumed that this could be done by SDK clients, it's doable. But even if something goes wrong with optimistic way (let's say, computer was sleeping when timeout should have occurred, JS will not re-run it upon wake up) - regular checking-refreshing procedure will safely do the request and all underlying activities.

[blakeembrey]

Absolutely, I agree that it should always handle manual refreshes because there's a lot of cases where a token might be rejected before expiration. Thanks for the heads up, I didn't have time to read over the code of the SDK.

[kirill-konshin]

LGTM, thank you.