Fixed issue with cache not retaining refresh token (#333)

* Fixed issue with cache not retaining refresh token * Fix integration tests * Removed unused core-js import * Extracted 1 day in seconds value to a constant * Applied comment to be consistant with related test * Applied brace styling for consistancy * Reworked expiry tests to mock date instead of using negative exp * Added some comments to the cache tests to explain the test scenario * Cleaned up JS return statement styling
auth0 · Jan 26, 2020 · e06e3c3 · e06e3c3
1 parent 6f95174
commit e06e3c3
Show file tree

Hide file tree

Showing 6 changed files with 320 additions and 90 deletions.
diff --git a/__tests__/cache.test.ts b/__tests__/cache.test.ts
@@ -1,6 +1,7 @@
 import { InMemoryCache, LocalStorageCache } from '../src/cache';
 
 const nowSeconds = () => Math.floor(Date.now() / 1000);
+const dayInSeconds = 86400;
 
 describe('InMemoryCache', () => {
   let cache: InMemoryCache;
@@ -54,17 +55,56 @@ describe('InMemoryCache', () => {
         user: { name: 'Test' }
       }
     });
+
+    // Test that the cache state is normal up until just before the expiry time..
     jest.advanceTimersByTime(799);
     expect(Object.keys(cache.cache).length).toBe(1);
 
+    // Advance the time to match the expiry time..
     jest.advanceTimersByTime(1);
+
+    // and test that the cache has been emptied.
     expect(Object.keys(cache.cache).length).toBe(0);
   });
 
+  it('strips everything except the refresh token when expiry has been reached', () => {
+    cache.save({
+      client_id: 'test-client',
+      audience: 'the_audience',
+      scope: 'the_scope',
+      id_token: 'idtoken',
+      access_token: 'accesstoken',
+      refresh_token: 'refreshtoken',
+      expires_in: 1,
+      decodedToken: {
+        claims: {
+          __raw: 'idtoken',
+          name: 'Test',
+          exp: new Date().getTime() / 1000 + 2
+        },
+        user: { name: 'Test' }
+      }
+    });
+
+    // Test that the cache state is normal up until just before the expiry time..
+    jest.advanceTimersByTime(799);
+    expect(Object.keys(cache.cache).length).toBe(1);
+
+    // Advance the time to just past the expiry..
+    jest.advanceTimersByTime(1);
+
+    // And test that the cache has been emptied, except for the refresh token
+    expect(cache.cache).toStrictEqual({
+      '@@auth0spajs@@::test-client::the_audience::the_scope': {
+        refresh_token: 'refreshtoken'
+      }
+    });
+  });
+
   it('expires after `user.exp` when `user.exp` < `expires_in`', () => {
     cache.save({
       client_id: 'test-client',
-      audience: 'the_audiene',
+      audience: 'the_audience',
       scope: 'the_scope',
       id_token: 'idtoken',
       access_token: 'accesstoken',
@@ -78,9 +118,15 @@ describe('InMemoryCache', () => {
         user: { name: 'Test' }
       }
     });
+
+    // Test that the cache state is normal up until just before the expiry time..
     jest.advanceTimersByTime(799);
     expect(Object.keys(cache.cache).length).toBe(1);
+
+    // Advance the time to just past the expiry..
     jest.advanceTimersByTime(1);
+
+    // And test that the cache has been emptied
     expect(Object.keys(cache.cache).length).toBe(0);
   });
 });
@@ -93,6 +139,7 @@ describe('LocalStorageCache', () => {
   beforeEach(() => {
     cache = new LocalStorageCache();
 
+    jest.clearAllMocks();
     jest.useFakeTimers();
     localStorage.clear();
     (<any>localStorage.removeItem).mockClear();
@@ -109,11 +156,11 @@ describe('LocalStorageCache', () => {
       scope: '__TEST_SCOPE__',
       id_token: '__ID_TOKEN__',
       access_token: '__ACCESS_TOKEN__',
-      expires_in: 86400,
+      expires_in: dayInSeconds,
       decodedToken: {
         claims: {
           __raw: 'idtoken',
-          exp: nowSeconds() + 86500,
+          exp: nowSeconds() + dayInSeconds + 100,
           name: 'Test'
         },
         user: { name: 'Test' }
@@ -127,59 +174,67 @@ describe('LocalStorageCache', () => {
     global.Date.now = realDateNow;
   });
 
-  it('can set a value into the cache when expires_in < exp', () => {
-    cache.save(defaultEntry);
-
-    expect(localStorage.setItem).toHaveBeenCalledWith(
-      '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
-      JSON.stringify({
-        body: defaultEntry,
-        expiresAt: nowSeconds() + 86400 - 60
-      })
-    );
-  });
-
-  it('can set a value into the cache when exp < expires_in', () => {
-    const entry = Object.assign({}, defaultEntry, {
-      expires_in: 86500,
-      decodedToken: {
-        claims: {
-          exp: nowSeconds() + 100
-        }
-      }
+  describe('cache.get', () => {
+    it('can retrieve an item from the cache', () => {
+      localStorage.setItem(
+        '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
+        JSON.stringify({
+          body: defaultEntry,
+          expiresAt: nowSeconds() + dayInSeconds
+        })
+      );
+
+      expect(
+        cache.get({
+          client_id: '__TEST_CLIENT_ID__',
+          audience: '__TEST_AUDIENCE__',
+          scope: '__TEST_SCOPE__'
+        })
+      ).toStrictEqual(defaultEntry);
     });
 
-    cache.save(entry);
-
-    expect(localStorage.setItem).toHaveBeenCalledWith(
-      '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
-      JSON.stringify({
-        body: entry,
-        expiresAt: nowSeconds() + 40
-      })
-    );
-  });
-
-  it('can retrieve an item from the cache', () => {
-    localStorage.setItem(
-      '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
-      JSON.stringify({
-        body: defaultEntry,
-        expiresAt: nowSeconds() + 86400
-      })
-    );
-
-    expect(
-      cache.get({
-        client_id: '__TEST_CLIENT_ID__',
-        audience: '__TEST_AUDIENCE__',
-        scope: '__TEST_SCOPE__'
-      })
-    ).toStrictEqual(defaultEntry);
-  });
+    it('returns undefined when there is no data', () => {
+      expect(cache.get({ scope: '', audience: '' })).toBeUndefined();
+    });
 
-  it('returns undefined when there is no data', () => {
-    expect(cache.get({ scope: '', audience: '' })).toBeUndefined();
+    it('strips the data, leaving the refresh token, when the expiry has been reached', () => {
+      localStorage.setItem(
+        '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
+        JSON.stringify({
+          body: {
+            client_id: '__TEST_CLIENT_ID__',
+            audience: '__TEST_AUDIENCE__',
+            scope: '__TEST_SCOPE__',
+            id_token: '__ID_TOKEN__',
+            access_token: '__ACCESS_TOKEN__',
+            refresh_token: '__REFRESH_TOKEN__',
+            expires_in: 10,
+            decodedToken: {
+              claims: {
+                __raw: 'idtoken',
+                exp: nowSeconds() + 15,
+                name: 'Test'
+              },
+              user: { name: 'Test' }
+            }
+          },
+          expiresAt: nowSeconds() + 10
+        })
+      );
+
+      const now = nowSeconds();
+      global.Date.now = jest.fn(() => (now + 30) * 1000);
+
+      expect(
+        cache.get({
+          client_id: '__TEST_CLIENT_ID__',
+          audience: '__TEST_AUDIENCE__',
+          scope: '__TEST_SCOPE__'
+        })
+      ).toStrictEqual({
+        refresh_token: '__REFRESH_TOKEN__'
+      });
+    });
   });
 
   it('expires after cache `expiresAt` when expiresAt < current time', () => {
@@ -192,20 +247,23 @@ describe('LocalStorageCache', () => {
           scope: '__TEST_SCOPE__',
           id_token: '__ID_TOKEN__',
           access_token: '__ACCESS_TOKEN__',
-          expires_in: -10,
+          expires_in: 10,
           decodedToken: {
             claims: {
               __raw: 'idtoken',
-              exp: nowSeconds() - 5,
+              exp: nowSeconds() + 15,
               name: 'Test'
             },
             user: { name: 'Test' }
           }
         },
-        expiresAt: nowSeconds() - 10
+        expiresAt: nowSeconds() + 10
       })
     );
 
+    const now = nowSeconds();
+    global.Date.now = jest.fn(() => (now + 30) * 1000);
+
     expect(
       cache.get({
         client_id: '__TEST_CLIENT_ID__',
@@ -219,22 +277,85 @@ 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
+  describe('cache.save', () => {
+    it('can set a value into the cache when expires_in < exp', () => {
+      cache.save(defaultEntry);
+
+      expect(localStorage.setItem).toHaveBeenCalledWith(
+        '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
+        JSON.stringify({
+          body: defaultEntry,
+          expiresAt: nowSeconds() + dayInSeconds - 60
+        })
+      );
+    });
+
+    it('can set a value into the cache when exp < expires_in', () => {
+      const entry = Object.assign({}, defaultEntry, {
+        expires_in: dayInSeconds + 100,
+        decodedToken: {
+          claims: {
+            exp: nowSeconds() + 100
+          }
         }
-      }
+      });
+
+      cache.save(entry);
+
+      expect(localStorage.setItem).toHaveBeenCalledWith(
+        '@@auth0spajs@@::__TEST_CLIENT_ID__::__TEST_AUDIENCE__::__TEST_SCOPE__',
+        JSON.stringify({
+          body: entry,
+          expiresAt: nowSeconds() + 40
+        })
+      );
     });
 
-    cache.save(entry);
+    it('deletes the cache item once the timeout has been reached', () => {
+      const entry = Object.assign({}, defaultEntry, {
+        expires_in: 120,
+        decodedToken: {
+          claims: {
+            exp: nowSeconds() + 240
+          }
+        }
+      });
 
-    // 96000, because the timeout time will be calculated at expires_in * 1000 * 0.8
-    jest.advanceTimersByTime(96000);
+      cache.save(entry);
 
-    expect(localStorage.removeItem).toHaveBeenCalled();
+      // 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', () => {

diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
@@ -1267,7 +1267,19 @@ describe('Auth0', () => {
 
           await auth0.getTokenSilently();
 
-          //we only evaluate that the code didn't bail out because of the cache
+          // we only evaluate that the code didn't bail out because of the cache
+          expect(utils.encode).toHaveBeenCalledWith(TEST_RANDOM_STRING);
+        });
+
+        it('continues method execution when there is a value from the cache but no access token', async () => {
+          const { auth0, utils, cache } = await setup();
+
+          cache.get.mockReturnValue({});
+
+          await auth0.getTokenSilently();
+
+          // we only evaluate that the code didn't bail out because the cache didn't return
+          // an access token
           expect(utils.encode).toHaveBeenCalledWith(TEST_RANDOM_STRING);
         });
       });

diff --git a/cypress/integration/getTokenSilently.js b/cypress/integration/getTokenSilently.js
@@ -55,7 +55,7 @@ describe('getTokenSilently', function() {
           cy.toggleSwitch('local-storage');
 
           cy.login().then(() => {
-            cy.reload();
+            cy.reload().wait(5000);
 
             cy.get('[data-cy=get-token]')
               .click()
@@ -82,7 +82,8 @@ describe('getTokenSilently', function() {
             cy.toggleSwitch('use-cache');
 
             cy.login().then(() => {
-              cy.toggleSwitch('refresh-tokens').wait(250);
+              cy.toggleSwitch('refresh-tokens').wait(1000);
+              win.localStorage.clear();
 
               cy.get('[data-cy=get-token]')
                 .click()