okta-auth-js icon indicating copy to clipboard operation
okta-auth-js copied to clipboard

TokenManager can return expired token in auto-renewal mode

Open bordecal opened this issue 3 years ago • 6 comments

Prior to the introduction of timer-based auto-renewal, the token manager's getAsync method contained a mechanism for preventing return of an expired token: https://github.com/okta/okta-auth-js/blob/d717437dd08000b3f194f6214ba1dd3ddb4f42a0/lib/TokenManager.js#L118-L131

This mechanism was removed when timer-based auto-renewal was introduced. Unfortunately, the timer-based approach does not always work reliably, which results in expired tokens being returned from TokenManager.getAsync. This also seems to have an impact on the default implementation of AuthStateManager.isAuthenticated, which does not depend on expiry of the tokens and will contain stale data if the tokens are not removed or refreshed.

The particular scenario in which we have noticed the timer-based mechanism fail is the following:

  1. On a mobile device (Android or iOS), open an "installed" PWA that uses okta-auth-js 4.1.2 in auto-renew mode.
  2. Log into the app.
  3. Leaving the app in the foreground, turn off (sleep, not shut down) the device.
  4. Wait until the access token has expired.
  5. Activate the device and attempt to interact with the app. The expired token will still be present, presumably because the expected timer-based renewal did not occur while the device was sleeping.

(The above behaviour is also observed if the app is opened in Safari on iOS before sleeping, but we haven't checked whether this is also the case with other browsers or on Android.)

Given that there may also be other scenarios in which the timer-based mechanism might not act as expected, would it perhaps make sense to conserve the previous behaviour in addition to the timer-based mechanism?

bordecal avatar Dec 08 '20 21:12 bordecal

@bordecal - Thanks for the detailed report.

Interesting. We moved away from the previous model because retrieving data shouldn't CHANGE that data - those side effects were causing any number of race conditions, loops, and hard-to-diagnose situations.

However, you highlight a real situation, so we will have to make sure this is covered. Perhaps that will be done by allowing a check for expiration on retrieval (not changing the data, but just not returning the expired token and giving the expiration timers a chance to fire).

I assume the timer will still fire in the example you describe (I'm not an Android dev), but you're seeing the problems before it has a chance to fire. We'll investigate, thanks again.

Internal ref: OKTA-351817

swiftone avatar Dec 08 '20 22:12 swiftone

@bordecal As of version 4.x tokenManager.get will return the value from storage and it is not guaranteed to be a valid token. In some cases it may be in the process of being renewed at the time of the call. The utility method hasExpired is provided to check for these race conditions:

https://github.com/okta/okta-auth-js/blob/master/README.md#tokenmanagergetkey

Version 4 also introduced the AuthStateManager which will give you validated tokens on a subscription interface. Calling updateAuthState will evaluate all tokens in storage and renew/remove as appropriate and then fire an auth change event.

aarongranick-okta avatar Jan 14 '21 22:01 aarongranick-okta

Am I correct in assuming this is still current behaviour? We're in the process of evaluating different OAuth js sdks...

If I read the setExpireEventTimeout code correctly, it doesn't take into account that the device could have been asleep, during which time the setTimeout clock is just paused AFAIK.

We moved away from the previous model because retrieving data shouldn't CHANGE that data - those side effects were causing any number of race conditions, loops, and hard-to-diagnose situations.

Interesting... I would have argued that an async function, if the docs mark it clearly as side-effectful, would be perfectly fine: devs can think of it as a network call, which sometimes is cached so it doesn't actually always do a network call.

Could you mention or link to some example cases that were causing trouble? I couldn't find much of a rationale in the 4.0.0 release notes for the introduction of the "active" autoRenew..?

Thanks!

mb21 avatar Feb 28 '22 14:02 mb21

since this is being used in okta react SDK as well, we are seeing similar issues in chrome browser where if the user puts their laptop to sleep for more than the expiry time of the access token in that case the SDK continues to return the expired token till the timer kicks in refresh it in the background If we go by trying to fix this on our own the only solution that we can think of is what was the previous implementation, but then that would leave us with all the same cons of the older solution in terms of race condition etc.

akashvyom avatar Mar 02 '22 09:03 akashvyom

@mb21 @akashvyom If you're experiencing issues please create your own bug report with as much detail as possible. This thread is referencing okta-auth-js v4 which is already retired (https://github.com/okta/okta-auth-js#release-status). My advice before reporting is to try the most recent versions of any Okta sdks in use

jaredperreault-okta avatar Mar 02 '22 14:03 jaredperreault-okta

@jaredperreault-okta we were directed to this ticket by okta internal support team, didn't want to file a duplicate ticket for the same functionality as this was broken in the latest versions as well. Additionally it looks like the release 6.2.0 from last week has more granular support for autoRenew with option for active and passive renewal, making a call to isAuthenticated now ensures if the token has expired in the local due to sleep/inactive issue then it automatically renews it https://github.com/okta/okta-auth-js/blob/f1c9ead677ccdfff5bb8a2ae42107413c8e74af7/lib/OktaAuth.ts#L580-L599 Upgrading to 6.2.0 and calling isAuthenticated has solved the issue for us

akashvyom avatar Mar 08 '22 10:03 akashvyom