Check the cache before acquiring lock to avoid latency (#558)

Co-authored-by: Steve Hobbs <[email protected]>

__mocks__/browser-tabs-lock/index.ts

@@ -1,7 +1,15 @@
-export const acquireLockMock = jest.fn();
-export const releaseLockMock = jest.fn();
+const Lock = jest.requireActual('browser-tabs-lock').default;
 
-export default class FakeLock {
-  acquireLock = acquireLockMock;
-  releaseLock = releaseLockMock;
+export const acquireLockSpy = jest.fn();
+export const releaseLockSpy = jest.fn();
+
+export default class extends Lock {
+  acquireLock(...args) {
+    acquireLockSpy(...args);
+    return super.acquireLock(...args);
+  }
+  releaseLock(...args) {
+    releaseLockSpy(...args);
+    return super.releaseLock(...args);
+  }
 }

__tests__/Auth0Client.test.ts

@@ -8,14 +8,14 @@ import * as utils from '../src/utils';
 import { Auth0ClientOptions, IdToken } from '../src';
 import * as scope from '../src/scope';
 import { expectToHaveBeenCalledWithAuth0ClientParam } from './helpers';
+// @ts-ignore
+import { acquireLockSpy } from 'browser-tabs-lock';
 
 jest.mock('unfetch');
 jest.mock('es-cookie');
 jest.mock('../src/jwt');
 jest.mock('../src/token.worker');
 
-jest.unmock('browser-tabs-lock');
-
 const mockWindow = <any>global;
 const mockFetch = (mockWindow.fetch = <jest.Mock>unfetch);
 const mockVerify = <jest.Mock>verify;
@@ -684,6 +684,31 @@ describe('Auth0Client', () => {
     expect(utils.runIframe).not.toHaveBeenCalled();
   });
 
+  it('should not acquire a browser lock when cache is populated', async () => {
+    const auth0 = setup();
+    await login(auth0);
+    jest.spyOn(<any>utils, 'runIframe').mockResolvedValue({
+      access_token: 'my_access_token',
+      state: 'MTIz'
+    });
+    mockFetch.mockResolvedValue(
+      fetchResponse(true, {
+        id_token: 'my_id_token',
+        refresh_token: 'my_refresh_token',
+        access_token: 'my_access_token',
+        expires_in: 86400
+      })
+    );
+    let access_token = await auth0.getTokenSilently({ audience: 'foo' });
+    expect(access_token).toEqual('my_access_token');
+    expect(acquireLockSpy).toHaveBeenCalled();
+    acquireLockSpy.mockClear();
+    // This request will hit the cache, so should not acquire the lock
+    access_token = await auth0.getTokenSilently({ audience: 'foo' });
+    expect(access_token).toEqual('my_access_token');
+    expect(acquireLockSpy).not.toHaveBeenCalled();
+  });
+
   it('sends custom options through to the token endpoint when using an iframe', async () => {
     const auth0 = setup({
       custom_param: 'foo',

__tests__/index.test.ts

@@ -1,13 +1,14 @@
-jest.mock('browser-tabs-lock');
+import { expectToHaveBeenCalledWithAuth0ClientParam } from './helpers';
+import { CacheLocation, Auth0ClientOptions } from '../src/global';
+import * as scope from '../src/scope';
+// @ts-ignore
+import { acquireLockSpy, releaseLockSpy } from 'browser-tabs-lock';
+
 jest.mock('../src/jwt');
 jest.mock('../src/storage');
 jest.mock('../src/transaction-manager');
 jest.mock('../src/utils');
 
-import { expectToHaveBeenCalledWithAuth0ClientParam } from './helpers';
-import { CacheLocation, Auth0ClientOptions } from '../src/global';
-import * as scope from '../src/scope';
-
 import createAuth0Client, {
   Auth0Client,
   GetTokenSilentlyOptions
@@ -64,12 +65,6 @@ const webWorkerMatcher = expect.objectContaining({
 });
 
 const setup = async (clientOptions: Partial<Auth0ClientOptions> = {}) => {
-  const auth0 = await createAuth0Client({
-    domain: TEST_DOMAIN,
-    client_id: TEST_CLIENT_ID,
-    ...clientOptions
-  });
-
   const getDefaultInstance = m => require(m).default.mock.instances[0];
 
   const storage = {
@@ -78,11 +73,9 @@ const setup = async (clientOptions: Partial<Auth0ClientOptions> = {}) => {
     remove: require('../src/storage').remove
   };
 
-  const lock = require('browser-tabs-lock');
   const cache = mockEnclosedCache;
 
   const tokenVerifier = require('../src/jwt').verify;
-  const transactionManager = getDefaultInstance('../src/transaction-manager');
   const utils = require('../src/utils');
 
   utils.createQueryParams.mockReturnValue(TEST_QUERY_PARAMS);
@@ -126,14 +119,20 @@ const setup = async (clientOptions: Partial<Auth0ClientOptions> = {}) => {
     close: jest.fn()
   };
 
+  const auth0 = await createAuth0Client({
+    domain: TEST_DOMAIN,
+    client_id: TEST_CLIENT_ID,
+    ...clientOptions
+  });
+  const transactionManager = getDefaultInstance('../src/transaction-manager');
+
   return {
     auth0,
     storage,
     cache,
     tokenVerifier,
     transactionManager,
     utils,
-    lock,
     popup
   };
 };
@@ -155,7 +154,7 @@ describe('Auth0', () => {
   });
 
   afterEach(() => {
-    jest.resetAllMocks();
+    jest.clearAllMocks();
     getUniqueScopesSpy.mockRestore();
   });
 
@@ -216,14 +215,14 @@ describe('Auth0', () => {
         'access_denied'
       ];
       for (let error of recoverableErrors) {
+        utils.runIframe.mockClear();
         utils.runIframe.mockRejectedValue({ error });
         const auth0 = await createAuth0Client({
           domain: TEST_DOMAIN,
           client_id: TEST_CLIENT_ID
         });
         expect(auth0).toBeInstanceOf(Auth0Client);
         expect(utils.runIframe).toHaveBeenCalledTimes(1);
-        utils.runIframe.mockClear();
       }
     });
 
@@ -1527,7 +1526,8 @@ describe('Auth0', () => {
         });
 
         it('continues method execution when there is no cache available', async () => {
-          const { auth0, utils } = await setup();
+          const { auth0, utils, cache } = await setup();
+          cache.get.mockReturnValue(undefined);
 
           await auth0.getTokenSilently();
 
@@ -1718,18 +1718,20 @@ describe('Auth0', () => {
       };
 
       it('releases the lock when there is an error', async () => {
-        const { auth0, lock, utils } = await setup();
+        const { auth0, utils } = await setup();
 
-        utils.runIframe.mockReturnValue(Promise.reject(new Error('Failed')));
+        utils.runIframe.mockImplementation(() =>
+          Promise.reject(new Error('Failed'))
+        );
 
         await expect(auth0.getTokenSilently()).rejects.toThrowError('Failed');
 
-        expect(lock.acquireLockMock).toHaveBeenCalledWith(
+        expect(acquireLockSpy).toHaveBeenCalledWith(
           GET_TOKEN_SILENTLY_LOCK_KEY,
           5000
         );
 
-        expect(lock.releaseLockMock).toHaveBeenCalledWith(
+        expect(releaseLockSpy).toHaveBeenCalledWith(
           GET_TOKEN_SILENTLY_LOCK_KEY
         );
       });
@@ -1892,7 +1894,7 @@ describe('Auth0', () => {
       });
 
       it('throws error if state from popup response is different from the provided state', async () => {
-        const { auth0, utils, lock } = await setup();
+        const { auth0, utils } = await setup();
 
         utils.runIframe.mockReturnValue(
           Promise.resolve({
@@ -1904,7 +1906,7 @@ describe('Auth0', () => {
           auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue)
         ).rejects.toThrowError('Invalid state');
 
-        expect(lock.releaseLockMock).toHaveBeenCalledWith(
+        expect(releaseLockSpy).toHaveBeenCalledWith(
           GET_TOKEN_SILENTLY_LOCK_KEY
         );
       });
@@ -1969,15 +1971,15 @@ describe('Auth0', () => {
         );
       });
       it('acquires and releases lock', async () => {
-        const { auth0, lock } = await setup();
+        const { auth0 } = await setup();
 
         await auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue);
 
-        expect(lock.acquireLockMock).toHaveBeenCalledWith(
+        expect(acquireLockSpy).toHaveBeenCalledWith(
           GET_TOKEN_SILENTLY_LOCK_KEY,
           5000
         );
-        expect(lock.releaseLockMock).toHaveBeenCalledWith(
+        expect(releaseLockSpy).toHaveBeenCalledWith(
           GET_TOKEN_SILENTLY_LOCK_KEY
         );
       });
@@ -2179,17 +2181,18 @@ describe('Auth0', () => {
 });
 
 describe('default creation function', () => {
-  it('does nothing if there is nothing storage', async () => {
-    Auth0Client.prototype.getTokenSilently = jest.fn();
+  it('does nothing if there is nothing in storage', async () => {
+    jest.spyOn(Auth0Client.prototype, 'getTokenSilently');
+    const getSpy = jest
+      .spyOn(require('../src/storage'), 'get')
+      .mockReturnValueOnce(false);
 
     const auth0 = await createAuth0Client({
       domain: TEST_DOMAIN,
       client_id: TEST_CLIENT_ID
     });
 
-    expect(require('../src/storage').get).toHaveBeenCalledWith(
-      'auth0.is.authenticated'
-    );
+    expect(getSpy).toHaveBeenCalledWith('auth0.is.authenticated');
 
     expect(auth0.getTokenSilently).not.toHaveBeenCalled();
   });

src/Auth0Client.ts

@@ -582,22 +582,36 @@ export default class Auth0Client {
       scope: getUniqueScopes(this.defaultScope, this.scope, options.scope)
     };
 
+    const getAccessTokenFromCache = () => {
+      const cache = this.cache.get(
+        {
+          scope: getTokenOptions.scope,
+          audience: getTokenOptions.audience || 'default',
+          client_id: this.options.client_id
+        },
+        60 // get a new token if within 60 seconds of expiring
+      );
+
+      return cache && cache.access_token;
+    };
+
+    // Check the cache before acquiring the lock to avoid the latency of
+    // `lock.acquireLock` when the cache is populated.
+    if (!ignoreCache) {
+      let accessToken = getAccessTokenFromCache();
+      if (accessToken) {
+        return accessToken;
+      }
+    }
+
     try {
       await lock.acquireLock(GET_TOKEN_SILENTLY_LOCK_KEY, 5000);
-
+      // Check the cache a second time, because it may have been populated
+      // by a previous call while this call was waiting to acquire the lock.
       if (!ignoreCache) {
-        const cache = this.cache.get(
-          {
-            scope: getTokenOptions.scope,
-            audience: getTokenOptions.audience || 'default',
-            client_id: this.options.client_id
-          },
-          60 // get a new token if within 60 seconds of expiring
-        );
-
-        if (cache && cache.access_token) {
-          await lock.releaseLock(GET_TOKEN_SILENTLY_LOCK_KEY);
-          return cache.access_token;
+        let accessToken = getAccessTokenFromCache();
+        if (accessToken) {
+          return accessToken;
         }
       }