Refresh tokens don't refresh when device is asleep longer than expiration time.

[mike1o1]

I'm using refresh tokens with a 15 minute expiration with everything working well, however I'm having an issue where if the users device goes to sleep for longer than the refresh token is active, when their device wakes back up, the next ajax request will log them out.

The problem is that while the device is asleep, the refresh timer doesn't kick off. Then, once the user wakes up their device and the application attempts to hit an endpoint, the access token is expired and the session is invalidated and user logs out.

Normally if a user reloads the application after the refresh token is expired, the session is restored and during that process a new access token is obtained. However, that session restore never happens as the timer expires but the device is asleep.

It would be nice if before making an ajax request, we can check to see if the token is expired and request a new one. This is the approach I take with Angular using an http interceptor to check for a 401, but Ember Data/ESA seems to have a different flow.

My approach to fix this is outlined below. The code isn't the best, but it gets it done.

1) Modify the ajax method in my adapter. I can access the session service and check if the token has expired. If it is, and user is authenticated, restore the session.

2) After session restore promise resolves, perform the original request.

Any thoughts or suggested ways around this? Happy to help put a pull request together as well if this option should be added to the adapter mixin.

My code is below, but any other workarounds are appreciated. Below is what I added to my application adapter.

  ajax() {
    const session = this.get('session.session');

    // We'll need to manually do the _super apply, so
    // it's bound correctly from within our promise return
    var args = arguments,
      self = this,
      _super = this._super;

    if (new Date().getTime() > session.get('content.authenticated.expires_at')) {
      return session.restore()
        .then(function() {
          return _super.apply(self, args);
        });
    } else {
      return this._super(...arguments);
    }
  }

[mschinis]

+1 for this. I have the same issue.

[marcoow]

Hm, interesting case.

The problem with your solution is that it uses private API though. The authenticators expose the sessionDataUpdated event which is automatically handled by the session and it would be good to leverage that to solve this case doing sth. like this:

ajax() {
  const session = this.get('session');

  if (new Date().getTime() > session.get('data.authenticated.expires_at')) {
    const authenticator = this.container.lookup(session.get('data.authenticated.authenticator'));
    return authenticator._refreshAccessToken().then(() => {
      return this._super(...arguments);
    });
  } else {
    return this._super(...arguments);
  }
}

In this case the authenticator would fire the sessionDataUpdated event after the token update which will automatically be handled by the session, thus making sure the updated token is sent with the request.

Of course the _refreshAccessToken is private API as well currently but I think it could be made public.

[marcoow]

Any update on this wrt my last comment?

[mike1o1]

I haven't tested using your code, but I haven't had any issues with my example above. I'll test in the next few days. Are there plans to make the refreshAccessToken public?

[marcoow]

No plans really but I guess a PR making it public could be merged - I don't see any hard problems with that.

[rhyek]

If anyone else tries @marcoow's suggestion, it works great, but I had to make a few adjustments using Ember 2.5:

  ajax() {
    const _super = this._super.bind(this);
    const session = this.get('session');
    const sessionData = session.get('data.authenticated');
    const authenticator = Ember.getOwner(this).lookup(sessionData['authenticator']);
    if (new Date().getTime() > sessionData['expires_at']) {
      return authenticator._refreshAccessToken(sessionData['expires_in'], sessionData['refresh_token']).then(() => {
        return _super(...arguments); // this._super doesn't work here for some reason.
      });
    } else {
      return _super(...arguments);
    }
  }

[stevenwu]

Several token bugs were fixed in the latest release. Please reopen if this is still an issue.

[rmachielse]

@marcoow @stevenwu we used the above solution for some time, but found this was not a very robust solution (it's causing issues when having multiple tabs open for example, and we also have cases of token usage outside of the adapter).

I think it should be possible to fix this from the oauth password grant authenticator. So, the main problem is that setTimeout is paused when the tab is asleep. I can think of two potential solutions:

• Reschedule the token refresh when the tab comes out of sleep. I have made a proof of concept for this based on the window's focus event, but this is not triggered when you have a tab open in the background, when waking up your computer. See here: https://github.com/simplabs/ember-simple-auth/compare/master...feedbackfruits:fix/access-token-refresh-on-focus

• Using setInterval (or maybe even requestAnimationFrame?) to reverify every "alive" second if the token needs to be refreshed. I've also made a proof of concept for this one, see here: https://github.com/simplabs/ember-simple-auth/compare/master...feedbackfruits:fix/access-token-refresh-interval

I'm totally willing to make a pr for this. What do you think?

[marcoow]

@rmachielse: I'm not really happy with either solution to be honest. Having to check something every second seems wasteful and I guess wouldn't solve the problem in all cases (e.g. if the app for some reason made a request right after the system woke back up and before a second passed or when the app made a request before the refresh request returned)?

Scheduling a (potential) refresh when the window receives focus seems pretty brittle and introduces another level of complexity (and likely lots and lots of edge cases) to the authenticator.

I guess ideally this would be fixed by adding something to the DataAdapterMixin that would allow to (potentially) make a "preflight" request to any real request in case the token has expired. That would of course require Ember Data to support that which I don't think is the case (yet).

[rmachielse]

@marcoow I completely understand your hesitance.

The problem with an adapter solution is that it doesn’t work with other uses of the token, while the information is there to refresh it. It also makes timeout based refreshment a bit useless.

Although I understand your fear of edge cases, as far as I can see this only broadens the covered scenarios at the cost of useless calculations. The check in the interval should be pretty simple though.

Maybe we can make this an optional configuration on the authenticator?

[marcoow]

I'm not sure this necessarily needs to be handled by ESA itself though. A solution adding this to the DataAdapterMixin (and Ember Data before) would likely work for most people most of the time and could serve as a blueprint for custom solution (e.g. if you had a network service or so that all requests go through that could use a similar mechanism to potentially allow a "preflight" request). I think instead adding this to ESA and having to deal with intervals, window focus state (both approaches wouldn't fix all potential cases anyway) this seems to be fixed much better by adding the concept of "preflight requests" to Ember Data (and potentially lower level networking libraries like ember-fetch).

[rmachielse]

@marcoow maybe we should expose a promise on the session or something that will always resolve with a valid access token, that can function as a base for such mechanisms. Such as this.get('session').authenticated().then(..., ...). Although it does raise the question if normal timeout based refreshes are still needed then. And it needs to prevent duplicates refreshes through multiple tabs somehow. Sounds like quite some work 😄

[marcoow]

I don't really think any of this (besides a public API for refreshing the token) needs to be in ESA at all. I think something like

// app/services/network.js
import Service, { inject } from '@ember/service';
import RSVP from 'rsvp';

export default Service.extend({
  session: inject(),

  _ensureToken() {
    let session = this.get('session');

    if (new Date().getTime() > session.get('data.authenticated.expires_at')) {
      let authenticator = this.container.lookup(session.get('data.authenticated.authenticator'));
      return authenticator._refreshAccessToken();
    } else {
      return RSVP.resolve();
    }
  },

  fetch(url, …) {
    return this._ensureToken().then(() => {
      return // actually make request here
    });
  }
});

should work just fine but is out of scope for ESA (but potentially within scope for ember-fetch, ember-data etc.).

[bgentry]

I think I'm encountering this same issue. I'm using plain OAuth2PasswordGrant with no changes aside from setting the token and revocation endpoints.

I'm on an iMac and left my app open on it overnight. At some point while I was out, it appears to have woken up briefly enough that ESA attempted to refresh my auth token and logged this error while hitting my token refresh endpoint:

There are no other console logs from this event. Presumably the network connection was interrupted during the request. However, now my app is left in a state where it is no longer making attempts to refresh the token, and the session is left with a token that expired 12 hours ago.

I don't really understand why you'd want to have a token auto-refresh feature but not want to make it robust enough to handle these situations. Why include the feature if the best practice is to disable it and handle refreshes yourself? I think the auto-refresh should either be robust out-of-the-box, or not be included at all.

In either case I think there needs to be some sort of public API change to handle this. I like the idea of a method on the authenticator that returns a promise that resolves only once it has a valid token. Then the _refreshAccessToken method can stay private. Thinking about this though, does it open up the possibility for multiple refreshes to be started at the same time if multiple data requests are being made (even within the same tab)?

Separately, maybe the token refresh should also be re-scheduled for some time in the future if it fails instead of just giving up forever as it currently does?

For now I'll see if I can resolve this for myself using something similar to your last comment.

[bgentry]

Ran into one issue w/ the suggested solution. The private API for refreshing requires some arguments:

function _refreshAccessToken(expiresIn, refreshToken) {
  // ...
}

It's not impossible to provide those but I think it illustrates why the best solution would be for the auth library to handle more of this.

[bgentry]

Here's what I ended up with inside my app/services/apollo.js which gets executed prior to every API request (I use ember-apollo-client):

  _runAuthorize() {
    let { session } = this;

    if (!session.isAuthenticated) {
      return {};
    }

    let { authenticated } = session.data;

    if (new Date().getTime() > authenticated.expires_at) {
      return this._refreshToken.perform(authenticated);
    }
    let accessToken = authenticated.access_token;
    return { headers: { Authorization: `Bearer ${accessToken}` } };
  },

  _refreshToken: task(function*(authenticated) {
    let authenticator = getOwner(this).lookup(authenticated.authenticator);
    yield authenticator._refreshAccessToken(
      authenticated.expires_in,
      authenticated.refresh_token
    );

    let accessToken = this.session.data.authenticated.access_token;
    return { headers: { Authorization: `Bearer ${accessToken}` } };
  }).drop(),

Importantly, I'm using ember-concurrency to make sure that only one refresh happens at a time. I still left refreshAccessTokens: true in my authenticator because that nicely covers the much more common scenario of cleanly refreshing the token before it expires. But no matter what, my data service needs to verify that the session is in fact unexpired before trying to make an API request.

I really think this sort of thing should be the default, out-of-the-box behavior for ember-simple-auth, or at least made as easy as possible for its users. It's common plumbing that everybody using refreshable tokens needs, even if they don't know it or their users encounter it rarely.

[fabianeichinger]

This problem is hitting our app pretty hard, especially in Safari, and we've a hard time fixing it on the application layer. We're using the OAuth2PasswordGrant authenticator.

There are a few, kinda intertwined, things that just seem missing to make this work reliable without complicating the library API or application code too much:

1. The authenticator doesn't keep track of the authentication / token refresh requests it started.
2. There is no way for other parts of the app to ask for authentication data asynchronously, e.g. as a promise which resolves when auth data is available that is probably valid. This is also related to 1. in that the authenticator is unable to reliably decide whether it can attach to an existing refresh request or needs to start a new one.
3. Other parts also can't notify the authenticator when they notice that an access token is invalid (expired early or whatever) to trigger a refresh in the authenticator.

The application layer code that I'd wish would work looks something like this:

async authenticatedFetch(...args) {
  const authenticator = this.get('authenticator');

  // promise which resolves immediately when there is no reason to suspect that the access token is invalid
  // when there is an ongoing authentication / refresh request it will attach to that
  // when a token refresh in necessary it will wait for that
  let accessToken = await authenticator.accessToken();

  let response = await fetch(...this._injectAccessTokenIntoRequest(args, accessToken));
  if (response.status === 401) {
    // lets the authenticator know that this accessToken is probably invalid
    // the authenticator can compare it to the one it currently holds and drop it if its the same
    // in that case the next time someone asks for the access token it would trigger a token refresh
    authenticator.accessTokenInvalid(accessToken);

    // at this point would the authenticator would either have no access token
    // and attempt to get a new one with the the refresh token
    // or it would already have a new one available so this fetch request could be retried
    accessToken = await authenticator.accessToken();
    response = await fetch(...this._injectAccessTokenIntoRequest(args, accessToken));
  }
  return response;

[marcoow]

So it seems clear we want some sort of mechanism for the application to trigger a refresh of the access token in certain situations (i.e. in order to make sure the token is valid before making requests). The simplest way to support that would be by adding to the OAuth 2.0 Password Grant authenticator (essentially by making _refreshAccessToken public). That would leave it up to the application when to call that etc.

If we did anything beyond that, the important point to keep in mind is that ESA works with all kinds of authenticators that might implement all kinds of authentication mechanisms and in theory might potentially not even issue tokens. So if anything was added anywhere to the API that would make sure that the current session state is valid and has not expired, that API would have to work with any possible authenticator as the Session, the DataAdapterMixin etc. don't have any knowledge about the specific mechanism that the authenticator the session is currently authenticated with uses. That would mean that if we added something like

await session.ensureNotExpired();

That method would have to use one of the methods available on the BaseAuthenticator class which are authenticate, invalidate and restore. restore is the one that comes closest to what we have here - it takes the current session state as an argument and returns a resolving or rejecting promise depending on whether that session state makes a valid session or not. The OAuth2PasswordGrant authenticator will already try and refresh the session if it finds it has expired. So one option for solving this could be making the current token restoration mechanism available publicly in a slightly modified form and with a different name (e.g. await session.validate() or something like that).

When it comes to automatically detecting window focus etc. I still think that is out of scope for ESA or at least for the authenticator layer. The authenticator's only job is implementing the actual authentication mechanism. Refreshing the session if the application returns from hibernation is an application-level concern and should be handled there. That said, if we had something as described in the previous paragraphs, I think it could make sense to add something (maybe opt-in) that invokes that session validation mechanism on certain events that commonly require session validation and potential refresh (however, most authenticators do not even support a refresh mechanism and when using those, refresh would be a no-op).

[fabianeichinger]

Hi @marcoow . Thank you for considering this issue again!

While I do think your proposal would make it easier to address some individual issues (including with our OAuth situation), I don't think it fixes the issue at the root. I don't think the application layer should have to deal with token refresh or validation at all.

You're right that this needs consideration for all kinds of authentication methods supported by ESA. I didn't reflect this in my code snippet, but I think that fundamentally it would work for any kind of authentication system and anecdotally this kind of flow is pretty reliably for OAuth2 in native apps we've developed.

The thing is that ESA (or any client code) can't guarantee that the authentication data it gives out for requests is actually valid at the time a request is sent out. To me this means that ESA (or rather its authenticators) should do a sensible effort to ensure that the data it hands out is valid and that it should be obvious what the application layer has to do in case it does encounter a 401, e.g. in an ember data request.

This is why I would propose to introduce a "guarded" way to access authentication data, in which the active authenticator can validate / update it and also has the option to say that it can't be produced at the moment (e.g. when the access token is expired but the oauth server is not reachable for a token refresh). This would basically be a generic version of await authenticator.accessToken() from my snippet, maybe something like

await session.aquireAuthorizedData();

The "401 in application request" could then be handled by

session.notifyAuthorizedDataInvalid(data);

to which the authenticator could react if it makes sense. (e.g. the OAuth2 authenticator might compare the access_token it currently hold to the one passed in the data argument and could delete it, if it is the same). This might also be a Promise though I'm not sure if that is necessary.

The biggest hurdle I see for a system like that in ESA is when multiple tabs are open, so there are no two tabs trying to refresh at the same time. It might need some cross-tab mutex. But in turn it might make any kind of window focus or internet connectivity detection unnecessary.

[marcoow]

@fabulant: I think what you're saying makes sense but I'm not sure how that is different from what I suggest above with

await session.ensureNotExpired();

[fabianeichinger]

@marcoow: Reading over your comment again and I might have just misunderstood how your proposed API additions would work based on the method names.

The difference I had in mind was for the session.{ensureNotExpired|validate|aquireAuthorizedData}() promise to resolve with auth data for a request, rather then just resolving and leaving it up to the consuming code to fetch the auth data from session.data.authenticated. I apologise if you've planned the same.

In case we did meant the same, the difference would be just the method name and maybe to which parts of ESA additions would need to be made. (It might be good to add a new method in BaseAuthenticator that can be overloaded by a subclass, rather then relying on restore? But I'm not really sure if this is necessary to be honest.)

Other than that, it is just the additional method to allow the consumer to notify ESA of bad access tokens.

[marcoow]

The difference I had in mind was for the session.{ensureNotExpired|validate|aquireAuthorizedData}() promise to resolve with auth data for a request, rather then just resolving and leaving it up to the consuming code to fetch the auth data from session.data.authenticated. I apologise if you've planned the same.

I think that's mostly an implementation detail really. The idea is the same - making functionality available publicly that kind of "validates" the session and lets the authenticator do whatever necessary to make sure the session is valid.

I think if restore can be used for that, it'd be good to do so rather than introducing a new concept. restore should be renamed in that case though…

[pankajparkar]

@rhyek to make this._super work in that case, I kept the this._super reference inside variable and later used that reference.

ajax() {
    const ajax = this._super, that = this;
    const session = this.get('session');
    const sessionData = session.get('data.authenticated');
    const authenticator = Ember.getOwner(this).lookup(sessionData['authenticator']);
    if (new Date().getTime() > sessionData['expires_at']) {
      return authenticator._refreshAccessToken(sessionData['expires_in'], sessionData['refresh_token']).then(() => {
        return ajax.call(that, ...arguments);
      });
    } else {
      return ajax.call(that , ...arguments);
    }
}

PS: Though above solution is not working properly for me. It makes multiple refreshToken call if multiple ajax call fails. I'm trying to tackle this situation, If I succeed I'll update this comment with my solution.