Repository: auth0/auth0-spa-js
Pull request #347 (2020-02-04 09:45:10)

Configurable timeout for getTokenSilently()
By submitting a PR to this repository, you agree to the terms within the [Auth0 Code of Conduct](https://github.com/auth0/open-source-template/blob/master/CODE-OF-CONDUCT.md). Please see the [contributing guidelines](https://github.com/auth0/.github/blob/master/CONTRIBUTING.md) for how to create and submit a high-quality PR for this repo.

### Description
`getTokenSilently()` is currently hardcoded to have a 60 seconds timeout. While this might be fine for an operation which is mostly called in background, it can be an issue when you want to wait for its completion and the operation fails for some reason (ie: random network issues,  missing `Web Origin`,  annoying browser extensions), as the application would hang for an entire minute.

As a proposed solution, this PR adds a configurable `timeoutInSeconds` parameter to `runIframe()`, and `getTokenSilently()` defaulting to the already-defined `DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS`.

For consistency with other options, the timeout can also be set with `authorizeTimeoutInSeconds` and will be considered in calls to `getTokenSilently()`, `getTokenWithPopup()` and `loginWithPopup()` unless overriden in their options.

### Testing

Unit tests were updated for handling the new parameters and new variable timeout in `runIframe()`

Manual tests can be done by calling `getTokenSilently({timeoutInSeconds: 5})` on a non-whitelisted Web Origin, observing the timeout after 5 seconds

#### Environment
**npm** 6.9.0
**node** 10.16.0
**Browsers**: Chromium 76 / Firefox 70
**OS**: Pop Os! 19.10 (Ubuntu derivative)

- [x] This change adds test coverage for new/changed/fixed functionality

### Checklist

- [ ] I have added documentation for new/changed functionality in this PR or in auth0.com/docs
- [ ] All active GitHub checks for tests, formatting, and security are passing
- [ ] The correct base branch is being used, if not `master`


stevehobbsdev: Thanks for this @Serjlee. The integration tests are failing, let me look into that and get back to you.
stevehobbsdev: @Serjlee A fix for this is provided in #348, will need to you to merge or rebase this once that is merged.
Serjlee: @stevehobbsdev thanks! I rebased the PR.

I also just noticed that `getTokenSilently()` is called in the createAuth0Client function when the `auth0.is.authenticated` cookie is set, so I'd like to expose a `timeoutSeconds` param in its config object, too. What do you think?
stevehobbsdev: @Serjlee In that case I would instead promote the property you have now for `getUser` to the client options instead, and have everything use that. Rather than have two separate timeout values. Make sense?
Serjlee: @stevehobbsdev sounds fine to me! I updated the PR description and committed a few changes:
- The timeout can now be set in the client options with the `authorizeTimeoutInSeconds` parameter
- The timeout set in the client options can be overriden while calling the affected method directly, for consistency with other client options
- `authorizeTimeoutInSeconds` is also considered  in `getTokenWithPopup/loginWithPopup`

Let me know if something doesn't look right or if I should rename something (I'm not that happy with `authorizeTimeoutInSeconds`)

Patch:
diff --git a/.circleci/config.yml b/.circleci/config.yml
index 3fca67a..168e5f1 100644
--- a/.circleci/config.yml
+++ b/.circleci/config.yml
@@ -12,3 +12,17 @@ jobs:
       - run: npm run test:integration
       - run: npm run test:es-check
       - run: npm run print-bundle-size
+      - run:
+          name: Copy screenshots
+          command: |
+            mkdir /tmp/screenshots
+            cp -R ~/repo/cypress/screenshots/*/*.png /tmp/screenshots 2>/dev/null || :
+      - run:
+          name: Copy videos
+          command: |
+            mkdir /tmp/videos
+            cp -R ~/repo/cypress/videos/*.mp4 /tmp/videos 2>/dev/null || :
+      - store_artifacts:
+          path: /tmp/screenshots
+      - store_artifacts:
+          path: /tmp/videos
diff --git a/src/Auth0Client.ts b/src/Auth0Client.ts
index 7af4e17..e27148c 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -178,7 +178,11 @@ export default class Auth0Client {
       ...params,
       response_mode: 'web_message'
     });
-    const codeResult = await runPopup(popup, url, config);
+    const codeResult = await runPopup(popup, url, {
+      ...config,
+      timeoutInSeconds:
+        config.timeoutInSeconds || this.options.authorizeTimeoutInSeconds
+    });
     if (stateIn !== codeResult.state) {
       throw new Error('Invalid state');
     }
@@ -347,6 +351,7 @@ export default class Auth0Client {
         audience,
         scope,
         ignoreCache,
+        timeoutInSeconds,
         ...additionalQueryParams
       } = options;
 
@@ -389,7 +394,11 @@ export default class Auth0Client {
         response_mode: 'web_message'
       });
 
-      const codeResult = await runIframe(url, this.domainUrl);
+      const codeResult = await runIframe(
+        url,
+        this.domainUrl,
+        timeoutInSeconds || this.options.authorizeTimeoutInSeconds
+      );
 
       if (stateIn !== codeResult.state) {
         throw new Error('Invalid state');
diff --git a/src/constants.ts b/src/constants.ts
index 710c5aa..6891215 100644
--- a/src/constants.ts
+++ b/src/constants.ts
@@ -6,6 +6,4 @@ export const DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS = 60;
 /**
  * @ignore
  */
-export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {
-  timeoutInSeconds: DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS
-};
+export const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {};
diff --git a/src/global.ts b/src/global.ts
index 0f58c3d..0aa6bb8 100644
--- a/src/global.ts
+++ b/src/global.ts
@@ -90,6 +90,11 @@ interface Auth0ClientOptions extends BaseLoginOptions {
    * Defaults to 60s.
    */
   leeway?: number;
+  /**
+   * A maximum number of seconds to wait before declaring background calls to /authorize as failed for timeout
+   * Defaults to 60s.
+   */
+  authorizeTimeoutInSeconds?: number;
 }
 
 /**
@@ -180,6 +185,12 @@ interface GetTokenSilentlyOptions extends GetUserOptions {
    */
   redirect_uri?: string;
 
+  /**
+   * A maximum number of seconds to wait before declaring the background /authorize call as failed for timeout
+   * Defaults to 60s.
+   */
+  timeoutInSeconds?: number;
+
   /**
    * If you need to send custom parameters to the Authorization Server,
    * make sure to use the original parameter name.
diff --git a/src/utils.ts b/src/utils.ts
index 1bc56d2..f289d80 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -31,7 +31,11 @@ export const parseQueryResult = (queryString: string) => {
   };
 };
 
-export const runIframe = (authorizeUrl: string, eventOrigin: string) => {
+export const runIframe = (
+  authorizeUrl: string,
+  eventOrigin: string,
+  timeoutInSeconds: number = DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS
+) => {
   return new Promise<AuthenticationResult>((res, rej) => {
     var iframe = window.document.createElement('iframe');
     iframe.setAttribute('width', '0');
@@ -41,7 +45,7 @@ export const runIframe = (authorizeUrl: string, eventOrigin: string) => {
     const timeoutSetTimeoutId = setTimeout(() => {
       rej(TIMEOUT_ERROR);
       window.document.body.removeChild(iframe);
-    }, 60 * 1000);
+    }, timeoutInSeconds * 1000);
 
     const iframeEventHandler = function(e: MessageEvent) {
       if (e.origin != eventOrigin) return;


Test patch:
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index e3b2cab..9063ec9 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -9,7 +9,6 @@ import Auth0Client from '../src/Auth0Client';
 import createAuth0Client from '../src/index';
 import { AuthenticationError } from '../src/errors';
 import version from '../src/version';
-import { DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS } from '../src/constants';
 const GET_TOKEN_SILENTLY_LOCK_KEY = 'auth0.lock.getTokenSilently';
 
 const TEST_DOMAIN = 'test.auth0.com';
@@ -35,9 +34,7 @@ const TEST_TELEMETRY_QUERY_STRING = `&auth0Client=${encodeURIComponent(
   )
 )}`;
 
-const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {
-  timeoutInSeconds: DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS
-};
+const DEFAULT_POPUP_CONFIG_OPTIONS: PopupConfigOptions = {};
 
 const setup = async (options = {}) => {
   const auth0 = await createAuth0Client({
@@ -239,6 +236,19 @@ describe('Auth0', () => {
         { timeoutInSeconds: 1 }
       );
     });
+
+    it('opens popup with correct popup, url and timeout from client options', async () => {
+      const { auth0, utils } = await setup({ authorizeTimeoutInSeconds: 1 });
+      const popup = {};
+      utils.openPopup.mockReturnValue(popup);
+      await auth0.loginWithPopup({}, DEFAULT_POPUP_CONFIG_OPTIONS);
+      expect(utils.runPopup).toHaveBeenCalledWith(
+        popup,
+        `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
+        { timeoutInSeconds: 1 }
+      );
+    });
+
     it('throws error if state from popup response is different from the provided state', async () => {
       const { auth0, utils } = await setup();
 
@@ -1301,7 +1311,31 @@ describe('Auth0', () => {
         await auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue);
         expect(utils.runIframe).toHaveBeenCalledWith(
           `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
-          'https://test.auth0.com'
+          'https://test.auth0.com',
+          defaultOptionsIgnoreCacheTrue.timeoutInSeconds
+        );
+      });
+
+      it('opens iframe with correct urls and timeout from client options', async () => {
+        const { auth0, utils } = await setup({ authorizeTimeoutInSeconds: 1 });
+        await auth0.getTokenSilently(defaultOptionsIgnoreCacheTrue);
+        expect(utils.runIframe).toHaveBeenCalledWith(
+          `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
+          'https://test.auth0.com',
+          1
+        );
+      });
+
+      it('opens iframe with correct urls and custom timeout', async () => {
+        const { auth0, utils } = await setup();
+        await auth0.getTokenSilently({
+          ...defaultOptionsIgnoreCacheTrue,
+          timeoutInSeconds: 1
+        });
+        expect(utils.runIframe).toHaveBeenCalledWith(
+          `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
+          'https://test.auth0.com',
+          1
         );
       });
 
diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts
index 584f2d3..a680a29 100644
--- a/__tests__/utils.test.ts
+++ b/__tests__/utils.test.ts
@@ -546,8 +546,9 @@ describe('utils', () => {
       expect(message.source.close).toHaveBeenCalled();
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
     });
-    it('times out after 60s', async () => {
+    it('times out after timeoutInSeconds', async () => {
       const { iframe, url, origin } = setup('');
+      const seconds = 10;
       /**
        * We need to run the timers after we start `runIframe` to simulate
        * the window event listener, but we also need to use `jest.useFakeTimers`
@@ -555,10 +556,12 @@ describe('utils', () => {
        * then using fake timers then rolling back to real timers
        */
       setTimeout(() => {
-        jest.runAllTimers();
+        jest.runTimersToTime(seconds * 1000);
       }, 10);
       jest.useFakeTimers();
-      await expect(runIframe(url, origin)).rejects.toMatchObject(TIMEOUT_ERROR);
+      await expect(runIframe(url, origin, seconds)).rejects.toMatchObject(
+        TIMEOUT_ERROR
+      );
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
       jest.useRealTimers();
     });