Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Refresh token after computer sleep #300

Closed
ghusse opened this issue Mar 25, 2021 · 10 comments
Closed

Refresh token after computer sleep #300

ghusse opened this issue Mar 25, 2021 · 10 comments

Comments

@ghusse
Copy link

ghusse commented Mar 25, 2021

I experienced an issue with this library, when using refresh tokens.

If the computer goes into sleep mode, and if the authentication token expires during this period, after awakening the computer the user is logged out whereas the refresh token should be refreshed.

I figured out that:

  • When the computer exits the sleep mode, all timeouts are executed in order.
  • Both callbacks for refreshing the token AND expiring the authentication token are executed
  • But as refreshing the token needs to make an HTTP call, the token cannot be refreshed before the auth token being expired
  • As a result, the user is logged out
@fenichelar
Copy link
Owner

fenichelar commented Mar 25, 2021

@ghusse That is the intended behavior. This library can't assume it is safe to wait to invalidate the session while the refresh request is processing because the session could be used by the user during this time.

You can extend JWT authenticator and handle this scenario for you specific application. Specifically, you would probably need to extend refreshAccessToken and handleAccessTokenExpiration. Extending these methods should be pretty easy, but you will probably need to do implement a whole page loading overlay or something the prevents the user from using the invalid session while the token is being refreshed. And it gets a little more complicated if you have any functionality like auto refresh where API calls can be made without user interaction. These would need to be held while the access token is being refreshed.

Let me know if you have any questions about how to extend the authenticator.

@masda70
Copy link

masda70 commented Mar 28, 2021

In my application, I have noticed that on a mobile browser, sessions tend to last less that on a desktop browser. In fact, much less than the refresh access token expiration time. I believe that the cause is similar to @ghusse has pointed out here.

My impression is that a solution to this problem would involving making the Ember adapter be aware of the possibility that the access token may have expired and cause requests to be held until the refresh query promise finishes.

@masda70
Copy link

masda70 commented Apr 9, 2021

I came up with the following solution, which is definitely better than the base implementation if most of the time that the access token expires the refresh token is still valid and if session invalidation already forced a page refresh. If not, this solution may cause most of the time an unwanted extra page refresh.

 async handleAccessTokenExpiration() {
    window.location.reload();
  }

@fenichelar
Copy link
Owner

If you set the tokenExpirationInvalidateSession property to false then the session won't be invalidated when the access token expires. The access token will still be refreshed. This works so long an API request isn't made before the token refresh is completed.

@masda70
Copy link

masda70 commented Apr 28, 2021

Refresh token request error

I have identified another potential cause for the session being invalidated under circumstances where the client could reasonably recover on its own: The refresh token request resolves in an error other than a HTTP 401 or 403 error (e.g., transient network error). I have come up with a solution for this:

// authenticators/application.js
import JWTAuthenticator from 'ember-simple-auth-token/authenticators/jwt';

export default class ApplicationAuthenticator extends JWTAuthenticator {
  refreshTokenTimeoutIfError = 5; // seconds

  async restore(data) {
    try {
      return await super.restore(data);
    } catch (error) {
      if (error._maintainSession) {
        // the session might still be recoverable, do not invalidate the session yet
        return data;
      } else {
        throw error;
      }
    }
  }

  async refreshAccessToken(token) {
    try {
      return await super.refreshAccessToken(token);
    } catch (error) {
      const errorStatus = error.status
      if (errorStatus !== 401 && errorStatus !== 403) {
        // e.g., a network error, possibly transient, try again in a few
        const now = this.getCurrentTime();
        const expiresAt = now + this.refreshLeeway + this.refreshTokenTimeoutIfError;
        this.scheduleAccessTokenRefresh(expiresAt, token);
        error._maintainSession = true;
      }
      throw error;
    }
  }
}

API requests when session is authenticated but access token has expired

Thanks @fenichelar for mentioning tokenExpirationInvalidateSession = false as a solution to preventing session invalidation on access token expiration, but I think that the API request exception you mentioned is the real problem: How should we handle an API request resolving in failure and subsequently causing the session to be invalidated while the access token is being refreshed? In my application, some components make periodic requests to the server and such a failure case is likely after recovering from computer sleep (or mobile phone sleep for that matter). One possibility to avoid session invalidation in such case is to disable session invalidation behavior in the handleResponse method of the TokenAdapter mixin. But then the responsibility of handling unauthorized errors is given to the parts of the application from which an API request might originate, which is typically a lot of places where we do not want to specifically consider such kind error. However, assuming that the refresh token is still valid and that the access token is in the brink of being refreshed, I could argue that the desirable ways to resolve the error could be one of the following:

  1. A friendly error message is shown to the user, which might prompt the user to retry the process that was interrupted because of this error. This assumes that this process provides a "retry" button. Worst case, the user has to reload the page and loses DOM-only data (e.g., form data).
  2. The error might not cause much disruption in case the request comes from a component that is triggering periodic model refreshes that are fault tolerant (i.e., an API error does not prevent a subsequent refresh)
  3. The adapter transparently retries the request, at least as soon as the token is refreshed, and holds the promise from resolving until the new request resolves.

My opinion is that 3 is best and it would have to be implemented at the adapter level. In this case, we might need a method for the adapter to be sure that in the token is in brink of being refreshed or to verify that the user has not simply suddenly lost their access altogether (e.g., the user account was inactivated). Another issue could be that the clock of the client is temporarily late with respect to the server's and the adapter should also ask the authorizer to trigger a token refresh asap. I have not yet implemented automatic request retry from the adapter, but at least I have somewhat covered the triggering of a token refresh from the adapter by forcing a session.restore() (which makes the authorizer trigger a token refresh request). I know this might cause two token refreshes in quick succession after a computer sleep, but I don't think it is a problem.

// adapters/application.js
import JSONAPIAdapter from '@ember-data/adapter/json-api';
import TokenAdapterMixin from 'ember-simple-auth-token/mixins/token-adapter';
import {task, timeout} from "ember-concurrency";

export default class ApplicationAdapter extends JSONAPIAdapter.extend(TokenAdapterMixin) {
  @(task(function * () {
    let debounceTime = 200;
    yield timeout(debounceTime);
    this.get("session").restoreInternalSession();
  }).drop()) handleUnauthorizedErrorResponseTask;

  handleResponse(status) {
    if (status === 401 && this.get('session.isAuthenticated')) {
      // Force the session to be restored if an HTTP authorized error is received
      // and the session was marked as authenticated
      // the following task implements debouncing of the session restore call
      // in case multiple requests fail at the same time
      this.handleUnauthorizedErrorResponseTask.perform();
    }
    return JSONAPIAdapter.prototype.handleResponse.call(this, ...arguments);
  }
}

// services/session.js
import BaseSessionService from 'ember-simple-auth/services/session';

export default class SessionService extends BaseSessionService {
  // named restoreInternalSession instead of restore in case
  // restore is ever defined by the ember-simple-auth API
  // FIXME: uses private parts of ember-simple-auth, may break without prior notice
  restoreInternalSession() {
    const internalSession = this.get('session');
    if(!internalSession._busy){
      internalSession.restore();
    }
  }
}

@fenichelar
Copy link
Owner

fenichelar commented Apr 29, 2021

@masda70 What do you think of this? 787be61 Note I haven't documented anything yet nor tested it.

tokenRefreshInvalidateSessionResponseCodes (array of numbers) specifies the response codes for the token refresh request that cause an immediate session invalidation. Default = [401, 403] otherwise this is a breaking change.

refreshAccessTokenRetryAttempts (number) specifies how many attempts to make when retrying the token refresh. Default = 0 otherwise this is a breaking change, you would want to set this to a number greater than 0.

refreshAccessTokenRetryTimeout (number) specifies how long to wait in milliseconds between each toke refresh retry. Default = 1000.

tokenRefreshFailInvalidateSession (boolean) specifies whether or not the session should be invalidated if the token could not be refreshed after the specified number of retries assuming none of the requests received one of the response codes that causes immediate session invalidation. Default = false otherwise this is a breaking change, you would want to set this to true.

As you mentioned, you would then overwrite handleResponse in your adapter so that if the request fails, it retries after some period of time. You wouldn't need to worry about whether or not the token is about to be refreshed or not. ember-simple-auth-token would be attempting to refresh the token and if it is unable to, then it would invalidate the session. Just retry in your adapter until the session is invalidated.

Note you would still want to set tokenExpirationInvalidateSession to false.

@fenichelar
Copy link
Owner

I didn't like the use of a class variable to store the number of attempts that had been made. So I changed it to pass the number of attempts around in functions calls. I added some documentation. I also did some testing which caught a bug and I fixed that. Still need to add unit tests though.

@masda70
Copy link

masda70 commented Apr 29, 2021

Thank you, 787be61 is definitely a welcome change! However I am not certain I would set tokenRefreshFailInvalidateSession to true nor refreshAccessTokenRetryAttempts to anything but a high value for my use case. I would argue that the only exception to such logic would then be for the implementation of a rate limiting logic, such as the handling of HTTP 429, where it could be desirable for the client to slow down querying the authentication backend while dismissing the error. An alternative would be to implement a timeout with a exponential backoff where it ends up capping at a reasonably long but not too long period (20 seconds?).

The remaining two issues that I am particularly interested in solving, and somewhat presented in my previous post are:

  • On initial page load, if the frontend correctly loads but the server API is down, the authenticator.restore() method still resolves in an error which would cause the session to be invalidated. I think that the odds of this happening are less than the odds of getting an error at token refresh timeout simply because token refreshes are expected to happen more often than full page reload or initial page access. Also, there fewer cases where the frontend is available while server is unavailable, but I wonder to what extent an offline mode with frontend caching (service worker?) may cause a successful frontend load without any backing network to query the authentication backend. Still, I think it's not too difficult to implement the logic to prevent this issue by making the restore function capture an error coming from the token refresh function that is expected to be retryable. This is kind of included in the code I posted.
  • Adapter query resolving into a 401 error even though the client refresh timeout is not close to expiring (clock error or user access being revoked). Of course, if token refresh period is short (< 5 minutes) and the user refresh token is still valid, the problem would eventually end up resolving on its own, and the long wait might even prompt to user to reload the page. A problem I see here is the adapter retrying to do same query (potentially large in size) over and over again, causing unnecessary network traffic. An optimization could involve to make adapter be able to ask the authenticator to to refresh the token as soon as possible, while periodically running the query again. Another optimization would involve the adapter being able to identify if the authenticator has refreshed the token since last time the query was performed, prompting to wait a little longer. Also I think I would need to override ajax instead of handleResponse to ensure all parameters are available for retrying the request.

@fenichelar
Copy link
Owner

@masda70 This sound like a fairly specific use case. I'm not use it makes sense to try an natively support this use case in this library.

For the first issue, I agree, handling an inaccessible backend on page load is complex. For many applications, the isAuthenticated state is used on page load to determine if a login prompt or the main page should be displayed.

For the second issue, the intent of the retry logic recently added to this library is more about a transient network issue, not a long term network disconnect. So I don't think I want to add anything to this library. But you could pretty easily do soemthing in your adapter. The access token has an exp claim and possible iat, which I would recommend adding if you don't already have iat. Your adapter could look at the iat as part of the retry logic. This would allow your adapter to know if the token had been refreshed or not.

@lancedikson
Copy link

We've encountered a similar issue with cross-tab token refreshing, and the current issue was one of the issues I've looked up for inspiration. Sharing my solution in case somebody ever stumbles upon a similar problem.
We are using ember-simple-auth-token with JWT authenticator, so here's an example of how I made it work with cross-tab sync:

// app/authenticators/jwt.js
import JWT from 'ember-simple-auth-token/authenticators/jwt';
import EmberObject from '@ember/object';
import { inject as service } from '@ember/service';
import { getOwner } from '@ember/application';

export default JWT.extend({
  session: service('session'),

  get tokenRefreshOffset() {
    const min = 1;
    const max = this.refreshLeewayOffsetMax;

    return Math.floor(Math.random() * (max - min)) + min;
  },

  init() {
    this._super(...arguments);
    const owner = getOwner(this);
    const environment = owner ? owner.resolveRegistration('config:environment') || {} : {};
    const config = environment['ember-simple-auth-token'] || {};
    this.refreshLeewayOffsetMax = config.refreshLeewayOffsetMax || 20;
    this.refreshLeeway = this.refreshLeeway + this.tokenRefreshOffset;
  },

  async refreshAccessToken(refreshToken, attempts) {
    const parent = this._super;
    /* try to restore up-to-date session data from store */
    const restoredContent = await this.session.session.store.restore();
    const dataObject = EmberObject.create(restoredContent.authenticated);
    const now = this.getCurrentTime();
    refreshToken = dataObject.get(this.refreshTokenPropertyName);
    let expiresAt = dataObject.get(this.tokenExpireName);

    if (expiresAt > now) {
      const wait = (expiresAt - now - this.refreshLeeway) * 1000;

      if (wait > 0) {
        this.scheduleAccessTokenRefresh(expiresAt, refreshToken);
      } else {
        return await parent.call(this, refreshToken, 0);
      }
    } else {
      return await parent.call(this, refreshToken, attempts);
    }
  },
});

The solution introduces a new option for ESA-token (refreshLeewayOffsetMax), which sets the maximum possible value for a randomly selected offset for a tab, lowering the possibility for a collision to happen.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants