Repository: auth0/auth0-spa-js (ts)

Issue:
Expired token was received from getTokenSilently


### Description

Expired token was received from getTokenSilently. 



### Reproduction

1. User logins or already logged in
2. The user hasn't done any actions for more than 24 hours
3. After more than 24 hours, the user interacts with the app, and an API call is made
4. The token is fetched using getTokenSilently and is set on the backend request header
5. the backend fails the request with a token expired exception

Analysing auth0 logs, I see that the user hasn't made any calls to auth0 server with "Success Silent Auth" before calling the backend.
This means that an expired token was returned from the cache.

Looking on the implementation of the cache, I see that it uses setTimeout to invalidate the cache entries. This is not reliable in case that the user computer was in sleep mode:
https://stackoverflow.com/questions/6346849/what-happens-to-settimeout-when-the-computer-goes-to-sleep

I think that a better cache mechanism would be to check the cache entry validity on get, and if the entry has expired, then remove it from the cache at this point.


### Environment

- **Version of this library used:** 1.6.0
- **Other relevant versions (language, server software, OS, browser):** User was on macOS with Chrome 79


Pull request:
[SDK-1352] Removed setTimeout cache removal in favour of removal-on-read
### Description

The crux of this PR is to change the cache removal strategy so that cache data is removed on read rather than using a `setTimeout`.

* `setTimeout` is unreliable when a machine goes into standby
* It's difficult to test
* Makes the cache more difficult to implement

With these changes, the cache data is removed when the cache is read and the `expiresAt` has passed. Additionally, the format of the in-memory cache has changed and now uses a wrapped object with the expiry on it, similar to how the local storage cache works.

### References
Fixes #343 

### Testing

- [X] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [X] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [X] All active GitHub checks for tests, formatting, and security are passing
- [X] The correct base branch is being used, if not `master`


Patch:
diff --git a/src/cache.ts b/src/cache.ts
index 5c7741e..bbe5183 100644
--- a/src/cache.ts
+++ b/src/cache.ts
@@ -22,10 +22,6 @@ interface CacheEntry {
   refresh_token?: string;
 }
 
-interface CachedTokens {
-  [key: string]: Partial<CacheEntry>;
-}
-
 export interface ICache {
   save(entry: CacheEntry): void;
   get(key: CacheKeyData): Partial<CacheEntry>;
@@ -36,66 +32,45 @@ const keyPrefix = '@@auth0spajs@@';
 const createKey = (e: CacheKeyData) =>
   `${keyPrefix}::${e.client_id}::${e.audience}::${e.scope}`;
 
-const getExpirationTimeoutInMilliseconds = (expiresIn: number, exp: number) => {
-  const expTime =
-    (new Date(exp * 1000).getTime() - new Date().getTime()) / 1000;
-  return Math.min(expiresIn, expTime) * 1000 * 0.8;
-};
-
-type LocalStorageCachePayload = {
+type CachePayload = {
   body: Partial<CacheEntry>;
   expiresAt: number;
 };
 
+/**
+ * Wraps the specified cache entry and returns the payload
+ * @param entry The cache entry to wrap
+ */
+const wrapCacheEntry = (entry: CacheEntry): CachePayload => {
+  const expiresInTime = Math.floor(Date.now() / 1000) + entry.expires_in;
+  const expirySeconds =
+    Math.min(expiresInTime, entry.decodedToken.claims.exp) - 60; // take off a small leeway
+
+  return {
+    body: entry,
+    expiresAt: expirySeconds
+  };
+};
+
 export class LocalStorageCache implements ICache {
   public save(entry: CacheEntry): void {
     const cacheKey = createKey(entry);
-    const expiresInTime = Math.floor(Date.now() / 1000) + entry.expires_in;
-
-    const expirySeconds =
-      Math.min(expiresInTime, entry.decodedToken.claims.exp) - 60; // take off a small leeway
-
-    const payload: LocalStorageCachePayload = {
-      body: entry,
-      expiresAt: expirySeconds
-    };
-
-    const timeout = getExpirationTimeoutInMilliseconds(
-      entry.expires_in,
-      entry.decodedToken.claims.exp
-    );
-
-    setTimeout(() => {
-      const payload = this.getPayload(cacheKey);
-
-      if (!payload || !payload.body) {
-        return;
-      }
-
-      if (payload.body.refresh_token) {
-        const newPayload = this.stripPayload(payload);
-        localStorage.setItem(cacheKey, JSON.stringify(newPayload));
-
-        return;
-      }
-
-      localStorage.removeItem(cacheKey);
-    }, timeout);
+    const payload = wrapCacheEntry(entry);
 
     window.localStorage.setItem(cacheKey, JSON.stringify(payload));
   }
 
   public get(key: CacheKeyData): Partial<CacheEntry> {
     const cacheKey = createKey(key);
-    const payload = this.getPayload(cacheKey);
+    const payload = this.readJson(cacheKey);
     const nowSeconds = Math.floor(Date.now() / 1000);
 
     if (!payload) return;
 
     if (payload.expiresAt < nowSeconds) {
       if (payload.body.refresh_token) {
-        const newPayload = this.stripPayload(payload);
-        localStorage.setItem(cacheKey, JSON.stringify(newPayload));
+        const newPayload = this.stripData(payload);
+        this.writeJson(cacheKey, newPayload);
 
         return newPayload.body;
       }
@@ -119,7 +94,7 @@ export class LocalStorageCache implements ICache {
    * Retrieves data from local storage and parses it into the correct format
    * @param cacheKey The cache key
    */
-  private getPayload(cacheKey: string): LocalStorageCachePayload {
+  private readJson(cacheKey: string): CachePayload {
     const json = window.localStorage.getItem(cacheKey);
     let payload;
 
@@ -136,58 +111,77 @@ export class LocalStorageCache implements ICache {
     return payload;
   }
 
+  /**
+   * Writes the payload as JSON to localstorage
+   * @param cacheKey The cache key
+   * @param payload The payload to write as JSON
+   */
+  private writeJson(cacheKey: string, payload: CachePayload) {
+    localStorage.setItem(cacheKey, JSON.stringify(payload));
+  }
+
   /**
    * Produce a copy of the payload with everything removed except the refresh token
    * @param payload The payload
    */
-  private stripPayload(
-    payload: LocalStorageCachePayload
-  ): LocalStorageCachePayload {
+  private stripData(payload: CachePayload): CachePayload {
     const { refresh_token } = payload.body;
 
-    const newPayload: LocalStorageCachePayload = {
+    const strippedPayload: CachePayload = {
       body: { refresh_token: refresh_token },
       expiresAt: payload.expiresAt
     };
 
-    return newPayload;
+    return strippedPayload;
   }
 }
 
 export class InMemoryCache {
   public enclosedCache: ICache = (function() {
-    let cache: CachedTokens = {};
+    let cache: CachePayload = {
+      body: {},
+      expiresAt: 0
+    };
 
     return {
       save(entry: CacheEntry) {
         const key = createKey(entry);
-        cache[key] = entry;
+        const payload = wrapCacheEntry(entry);
+
+        cache[key] = payload;
+      },
 
-        const timeout = getExpirationTimeoutInMilliseconds(
-          entry.expires_in,
-          entry.decodedToken.claims.exp
-        );
+      get(key: CacheKeyData) {
+        const cacheKey = createKey(key);
+        const wrappedEntry: CachePayload = cache[cacheKey];
+        const nowSeconds = Math.floor(Date.now() / 1000);
 
-        setTimeout(() => {
-          const payload = cache[key];
+        if (!wrappedEntry) {
+          return;
+        }
 
-          if (!payload) return;
+        if (wrappedEntry.expiresAt < nowSeconds) {
+          if (wrappedEntry.body.refresh_token) {
+            wrappedEntry.body = {
+              refresh_token: wrappedEntry.body.refresh_token
+            };
 
-          if (payload.refresh_token) {
-            cache[key] = { refresh_token: payload.refresh_token };
-            return;
+            return wrappedEntry.body;
           }
 
-          delete cache[key];
-        }, timeout);
-      },
+          delete cache[cacheKey];
 
-      get(key: CacheKeyData) {
-        return cache[createKey(key)];
+          return;
+        }
+
+        return wrappedEntry.body;
       },
 
       clear() {
-        cache = {};
+        cache = {
+          body: {},
+          expiresAt: 0
+        };
       }
     };
   })();


Test patch:
diff --git a/__tests__/cache.test.ts b/__tests__/cache.test.ts
index e6ec201..32c0855 100644
--- a/__tests__/cache.test.ts
+++ b/__tests__/cache.test.ts
@@ -30,9 +30,13 @@ describe('InMemoryCache', () => {
       scope: 'the_scope',
       id_token: 'idtoken',
       access_token: 'accesstoken',
-      expires_in: 1,
+      expires_in: dayInSeconds,
       decodedToken: {
-        claims: { __raw: 'idtoken', exp: 1, name: 'Test' },
+        claims: {
+          __raw: 'idtoken',
+          exp: nowSeconds() + dayInSeconds,
+          name: 'Test'
+        },
         user: { name: 'Test' }
       }
     };
@@ -48,57 +52,68 @@ describe('InMemoryCache', () => {
     ).toStrictEqual(data);
   });
 
-  it('expires after `expires_in` when `expires_in` < `user.exp`', () => {
-    const data = {
-      client_id: 'test-client',
-      audience: 'the_audience',
-      scope: 'the_scope',
-      id_token: 'idtoken',
-      access_token: 'accesstoken',
-      expires_in: 1,
-      decodedToken: {
-        claims: {
-          __raw: 'idtoken',
-          name: 'Test',
-          exp: new Date().getTime() / 1000 + 2
-        },
-        user: { name: 'Test' }
-      }
-    };
+  describe('when refresh tokens are used', () => {
+    it('strips everything except the refresh token when expiry has been reached', () => {
+      const now = Date.now();
+      const realDateNow = Date.now.bind(global.Date);
 
-    const cacheEntry = {
-      client_id: 'test-client',
-      audience: 'the_audience',
-      scope: 'the_scope'
-    };
+      const data = {
+        client_id: 'test-client',
+        audience: 'the_audience',
+        scope: 'the_scope',
+        id_token: 'idtoken',
+        access_token: 'accesstoken',
+        refresh_token: 'refreshtoken',
+        expires_in: dayInSeconds,
+        decodedToken: {
+          claims: {
+            __raw: 'idtoken',
+            name: 'Test',
+            exp: nowSeconds() + dayInSeconds * 2
+          },
+          user: { name: 'Test' }
+        }
+      };
 
-    cache.save(data);
+      cache.save(data);
 
-    // Test that the cache state is normal up until just before the expiry time..
-    jest.advanceTimersByTime(799);
-    expect(cache.get(cacheEntry)).toStrictEqual(data);
+      const cacheEntry = {
+        client_id: 'test-client',
+        audience: 'the_audience',
+        scope: 'the_scope'
+      };
 
-    // Advance the time to match the expiry time..
-    jest.advanceTimersByTime(1);
+      // Test that the cache state is normal up until just before the expiry time..
+      expect(cache.get(cacheEntry)).toStrictEqual(data);
 
-    // and test that the cache has been emptied.
-    expect(cache.get(cacheEntry)).toBeUndefined();
+      // Advance the time to just past the expiry..
+      const dateNowStub = jest.fn(() => now + (dayInSeconds + 60) * 1000);
+      global.Date.now = dateNowStub;
+
+      expect(cache.get(cacheEntry)).toStrictEqual({
+        refresh_token: 'refreshtoken'
+      });
+
+      global.Date.now = realDateNow;
+    });
   });
 
-  it('strips everything except the refresh token when expiry has been reached', () => {
+  it('expires the cache on read when the date.now > expires_in', () => {
+    const now = Date.now();
+    const realDateNow = Date.now.bind(global.Date);
+
     const data = {
       client_id: 'test-client',
       audience: 'the_audience',
       scope: 'the_scope',
       id_token: 'idtoken',
       access_token: 'accesstoken',
-      refresh_token: 'refreshtoken',
-      expires_in: 1,
+      expires_in: dayInSeconds,
       decodedToken: {
         claims: {
           __raw: 'idtoken',
           name: 'Test',
-          exp: new Date().getTime() / 1000 + 2
+          exp: nowSeconds() + dayInSeconds * 2
         },
         user: { name: 'Test' }
       }
@@ -112,31 +127,35 @@ describe('InMemoryCache', () => {
       scope: 'the_scope'
     };
 
-    // Test that the cache state is normal up until just before the expiry time..
-    jest.advanceTimersByTime(799);
+    // Test that the cache state is normal before we expire the data
     expect(cache.get(cacheEntry)).toStrictEqual(data);
 
     // Advance the time to just past the expiry..
-    jest.advanceTimersByTime(1);
+    const dateNowStub = jest.fn(() => (now + dayInSeconds + 100) * 1000);
+    global.Date.now = dateNowStub;
 
-    expect(cache.get(cacheEntry)).toStrictEqual({
-      refresh_token: 'refreshtoken'
-    });
+    // And test that the cache has been emptied
+    expect(cache.get(cacheEntry)).toBeUndefined();
+
+    global.Date.now = realDateNow;
   });
 
-  it('expires after `user.exp` when `user.exp` < `expires_in`', () => {
+  it('expires the cache on read when the date.now > token.exp', () => {
+    const now = Date.now();
+    const realDateNow = Date.now.bind(global.Date);
+
     const data = {
       client_id: 'test-client',
       audience: 'the_audience',
       scope: 'the_scope',
       id_token: 'idtoken',
       access_token: 'accesstoken',
-      expires_in: 2,
+      expires_in: dayInSeconds * 2,
       decodedToken: {
         claims: {
           __raw: 'idtoken',
           name: 'Test',
-          exp: new Date().getTime() / 1000 + 1
+          exp: nowSeconds() + dayInSeconds
         },
         user: { name: 'Test' }
       }
@@ -150,15 +169,17 @@ describe('InMemoryCache', () => {
       scope: 'the_scope'
     };
 
-    // Test that the cache state is normal up until just before the expiry time..
-    jest.advanceTimersByTime(799);
+    // Test that the cache state is normal before we expire the data
     expect(cache.get(cacheEntry)).toStrictEqual(data);
 
     // Advance the time to just past the expiry..
-    jest.advanceTimersByTime(1);
+    const dateNowStub = jest.fn(() => (now + dayInSeconds + 100) * 1000);
+    global.Date.now = dateNowStub;
 
     // And test that the cache has been emptied
     expect(cache.get(cacheEntry)).toBeUndefined();
+
+    global.Date.now = realDateNow;
   });
 });
 
@@ -341,52 +362,6 @@ describe('LocalStorageCache', () => {
         })
       );
     });
-
-    it('deletes the cache item once the timeout has been reached', () => {
-      const entry = Object.assign({}, defaultEntry, {
-        expires_in: 120,
-        decodedToken: {
-          claims: {
-            exp: nowSeconds() + 240
-          }
-        }
-      });
-
-      cache.save(entry);
-
-      // 96000, because the timeout time will be calculated at expires_in * 1000 * 0.8
-      jest.advanceTimersByTime(96000);
-
-      expect(localStorage.removeItem).toHaveBeenCalled();
-    });
-
-    it('strips the cache data, leaving the refresh token, once the timeout has been reached', () => {
-      const exp = nowSeconds() + 240;
-      const expiresIn = nowSeconds() + 120;
-
-      const entry = Object.assign({}, defaultEntry, {
-        expires_in: 120,
-        refresh_token: 'refresh-token',
-        decodedToken: {
-          claims: {
-            exp
-          }
-        }
-      });
-
-      cache.save(entry);
-
-      // 96000, because the timeout time will be calculated at expires_in * 1000 * 0.8
-      jest.advanceTimersByTime(96000);
-
-      const payload = JSON.parse(
-        localStorage.getItem(
-          '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__'
-        )
-      );
-
-      expect(payload.body).toStrictEqual({ refresh_token: 'refresh-token' });
-    });
   });
 
   it('removes the correct items when the cache is cleared', () => {