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

Issue:
PWA loginWithPopup blocked

### Description
Android pwa can't popup login. I can login in chrome browser on Android. But this app add to home screen can't login with popup. My `manifest.json` is `"display": "standalone"`.

Popup is `about:blank#blocked`.
### Reproduction
On Android version 9
1. App add to home screen.
2. Login with Popup.
3. `about:blank#blocked`

### Environment
Android version 9 
`"@auth0/auth0-spa-js": "^1.2.3"`

Pull request:
[SDK-1395] Refactor loginWithPopup to optionally accept an existing popup window
### Description

This PR adds a new option to `loginWithPopup` that would allow the developer to pass an existing popup window to be used instead of the SDK creating its own. This allows the popup to work better in scenarios like on iOS where the popup should be opened inside a button click handler without any async wrappers.

It also allows us to fix an issue where the popup sometimes doesn't work properly inside a PWA. The existing logic creates the pop window and then sets the URL, whereas now in this PR those two steps are unified.

### References

Closes #300 

### Testing

Manually tested by adding a new button to the playground that allows a popup window to be specified when calling `loginWithPopup`. Unit tests have also been added.

- [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/Auth0Client.ts b/src/Auth0Client.ts
index a2edd2e..9804745 100644
--- a/src/Auth0Client.ts
+++ b/src/Auth0Client.ts
@@ -10,8 +10,7 @@ import {
   runIframe,
   sha256,
   bufferToBase64UrlEncoded,
-  oauthToken,
-  openPopup
+  oauthToken
 } from './utils';
 
 import Cache from './cache';
@@ -160,7 +159,6 @@ export default class Auth0Client {
     options: PopupLoginOptions = {},
     config: PopupConfigOptions = DEFAULT_POPUP_CONFIG_OPTIONS
   ) {
-    const popup = await openPopup();
     const { ...authorizeOptions } = options;
     const stateIn = encodeState(createRandomString());
     const nonceIn = createRandomString();
@@ -178,7 +176,7 @@ export default class Auth0Client {
       ...params,
       response_mode: 'web_message'
     });
-    const codeResult = await runPopup(popup, url, {
+    const codeResult = await runPopup(url, {
       ...config,
       timeoutInSeconds:
         config.timeoutInSeconds || this.options.authorizeTimeoutInSeconds
diff --git a/src/global.ts b/src/global.ts
index d96146b..189be79 100644
--- a/src/global.ts
+++ b/src/global.ts
@@ -144,6 +144,13 @@ interface PopupConfigOptions {
    * throwing a timeout error. Defaults to 60s
    */
   timeoutInSeconds?: number;
+
+  /**
+   * Accepts an already-created popup window to use. If not specified, the SDK
+   * will create its own. This may be useful for platforms like iOS that have
+   * security restrictions around when popups can be invoked (e.g. from a user click event)
+   */
+  popup?: any;
 }
 
 interface GetUserOptions {
diff --git a/src/utils.ts b/src/utils.ts
index 03d3ce3..d07179a 100644
--- a/src/utils.ts
+++ b/src/utils.ts
@@ -62,29 +62,32 @@ export const runIframe = (
   });
 };
 
-export const openPopup = () => {
+const openPopup = url => {
   const width = 400;
   const height = 600;
   const left = window.screenX + (window.innerWidth - width) / 2;
   const top = window.screenY + (window.innerHeight - height) / 2;
 
-  const popup = window.open(
-    '',
+  return window.open(
+    url,
     'auth0:authorize:popup',
     `left=${left},top=${top},width=${width},height=${height},resizable,scrollbars=yes,status=1`
   );
+};
+
+export const runPopup = (authorizeUrl: string, config: PopupConfigOptions) => {
+  let popup = config.popup;
+
+  if (popup) {
+    popup.location.href = authorizeUrl;
+  } else {
+    popup = openPopup(authorizeUrl);
+  }
+
   if (!popup) {
     throw new Error('Could not open popup');
   }
-  return popup;
-};
 
-export const runPopup = (
-  popup: any,
-  authorizeUrl: string,
-  config: PopupConfigOptions
-) => {
-  popup.location.href = authorizeUrl;
   return new Promise<AuthenticationResult>((resolve, reject) => {
     const timeoutId = setTimeout(() => {
       reject({ ...TIMEOUT_ERROR, popup });
diff --git a/static/index.html b/static/index.html
index 52c4b4f..3bf0154 100644
--- a/static/index.html
+++ b/static/index.html
@@ -12,6 +12,7 @@
       <span id="loaded">loaded</span>
     </div>
     <button id="login_popup">Login popup</button>
+    <button id="login_popup_custom">Login popup (custom)</button>
     <button id="login_redirect">Login redirect</button>
     <button id="login_redirect_callback">Login redirect callback</button>
     <button id="getUser">Get user</button>
@@ -30,6 +31,7 @@
         })
           .then(function(auth0) {
             window.auth0 = auth0;
+
             $('#login_popup').click(function() {
               auth0
                 .loginWithPopup({
@@ -44,6 +46,33 @@
                   });
                 });
             });
+
+            $('#login_popup_custom').click(function() {
+              var popup = window.open(
+                '',
+                'auth0:authorize:popup',
+                'left=100,top=100,width=400,height=600,resizable,scrollbars=yes,status=1'
+              );
+
+              auth0
+                .loginWithPopup(
+                  {
+                    redirect_uri: 'http://localhost:3000/callback.html'
+                  },
+                  {
+                    popup: popup
+                  }
+                )
+                .then(function() {
+                  auth0.getTokenSilently().then(function(token) {
+                    console.log(token);
+                  });
+                  auth0.getUser().then(function(user) {
+                    console.log(user);
+                  });
+                });
+            });
+
             $('#login_redirect').click(function() {
               auth0.loginWithRedirect({
                 redirect_uri: 'http://localhost:3000/'


Test patch:
diff --git a/__tests__/index.test.ts b/__tests__/index.test.ts
index 324bd7b..5eec106 100644
--- a/__tests__/index.test.ts
+++ b/__tests__/index.test.ts
@@ -42,39 +42,48 @@ const setup = async (options = {}) => {
     client_id: TEST_CLIENT_ID,
     ...options
   });
+
   const getInstance = m => require(m).default.mock.instances[0];
+
   const storage = {
     get: require('../src/storage').get,
     save: require('../src/storage').save,
     remove: require('../src/storage').remove
   };
+
   const lock = require('browser-tabs-lock');
   const cache = getInstance('../src/cache');
   const tokenVerifier = require('../src/jwt').verify;
   const transactionManager = getInstance('../src/transaction-manager');
   const utils = require('../src/utils');
+
   utils.createQueryParams.mockReturnValue(TEST_QUERY_PARAMS);
   utils.getUniqueScopes.mockReturnValue(TEST_SCOPES);
   utils.encodeState.mockReturnValue(TEST_ENCODED_STATE);
   utils.createRandomString.mockReturnValue(TEST_RANDOM_STRING);
   utils.sha256.mockReturnValue(Promise.resolve(TEST_ARRAY_BUFFER));
   utils.bufferToBase64UrlEncoded.mockReturnValue(TEST_BASE64_ENCODED_STRING);
+
   utils.parseQueryResult.mockReturnValue({
     state: TEST_ENCODED_STATE,
     code: TEST_CODE
   });
+
   utils.runPopup.mockReturnValue(
     Promise.resolve({ state: TEST_ENCODED_STATE, code: TEST_CODE })
   );
+
   utils.runIframe.mockReturnValue(
     Promise.resolve({ state: TEST_ENCODED_STATE, code: TEST_CODE })
   );
+
   utils.oauthToken.mockReturnValue(
     Promise.resolve({
       id_token: TEST_ID_TOKEN,
       access_token: TEST_ACCESS_TOKEN
     })
   );
+
   tokenVerifier.mockReturnValue({
     user: {
       sub: TEST_USER_ID
@@ -84,6 +93,12 @@ const setup = async (options = {}) => {
       aud: TEST_CLIENT_ID
     }
   });
+
+  const popup = {
+    location: { href: '' },
+    close: jest.fn()
+  };
+
   return {
     auth0,
     storage,
@@ -91,7 +106,8 @@ const setup = async (options = {}) => {
     tokenVerifier,
     transactionManager,
     utils,
-    lock
+    lock,
+    popup
   };
 };
 
@@ -171,17 +187,31 @@ describe('Auth0', () => {
 
   describe('loginWithPopup()', () => {
     it('opens popup', async () => {
-      const { auth0, utils } = await setup();
+      const { auth0 } = await setup();
 
       await auth0.loginWithPopup({});
-      expect(utils.openPopup).toHaveBeenCalled();
     });
+
+    it('uses a custom popup specified in the configuration', async () => {
+      const { auth0, popup, utils } = await setup();
+
+      await auth0.loginWithPopup({}, { popup });
+
+      expect(utils.runPopup).toHaveBeenCalledWith(
+        `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
+        {
+          popup
+        }
+      );
+    });
+
     it('encodes state with random string', async () => {
       const { auth0, utils } = await setup();
 
       await auth0.loginWithPopup({});
       expect(utils.encodeState).toHaveBeenCalledWith(TEST_RANDOM_STRING);
     });
+
     it('creates `code_challenge` by using `utils.sha256` with the result of `utils.createRandomString`', async () => {
       const { auth0, utils } = await setup();
 
@@ -191,6 +221,7 @@ describe('Auth0', () => {
         TEST_ARRAY_BUFFER
       );
     });
+
     it('creates correct query params', async () => {
       const { auth0, utils } = await setup();
 
@@ -210,12 +241,14 @@ describe('Auth0', () => {
         connection: 'test-connection'
       });
     });
+
     it('creates correct query params without leeway', async () => {
       const { auth0, utils } = await setup({ leeway: 10 });
 
       await auth0.loginWithPopup({
         connection: 'test-connection'
       });
+
       expect(utils.createQueryParams).toHaveBeenCalledWith({
         client_id: TEST_CLIENT_ID,
         scope: TEST_SCOPES,
@@ -229,6 +262,7 @@ describe('Auth0', () => {
         connection: 'test-connection'
       });
     });
+
     it('creates correct query params when providing a default redirect_uri', async () => {
       const redirect_uri = 'https://custom-redirect-uri/callback';
       const { auth0, utils } = await setup({
@@ -236,6 +270,7 @@ describe('Auth0', () => {
       });
 
       await auth0.loginWithPopup({});
+
       expect(utils.createQueryParams).toHaveBeenCalledWith({
         client_id: TEST_CLIENT_ID,
         scope: TEST_SCOPES,
@@ -248,10 +283,12 @@ describe('Auth0', () => {
         code_challenge_method: 'S256'
       });
     });
+
     it('creates correct query params with custom params', async () => {
       const { auth0, utils } = await setup();
 
       await auth0.loginWithPopup({ audience: 'test' });
+
       expect(utils.createQueryParams).toHaveBeenCalledWith({
         audience: 'test',
         client_id: TEST_CLIENT_ID,
@@ -265,24 +302,20 @@ describe('Auth0', () => {
         code_challenge_method: 'S256'
       });
     });
+
     it('opens popup with correct popup, url and default config', async () => {
       const { auth0, utils } = await setup();
-      const popup = {};
-      utils.openPopup.mockReturnValue(popup);
       await auth0.loginWithPopup();
+
       expect(utils.runPopup).toHaveBeenCalledWith(
-        popup,
         `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
         DEFAULT_POPUP_CONFIG_OPTIONS
       );
     });
     it('opens popup with correct popup, url and custom config', async () => {
       const { auth0, utils } = await setup();
-      const popup = {};
-      utils.openPopup.mockReturnValue(popup);
       await auth0.loginWithPopup({}, { timeoutInSeconds: 1 });
       expect(utils.runPopup).toHaveBeenCalledWith(
-        popup,
         `https://test.auth0.com/authorize?${TEST_QUERY_PARAMS}${TEST_TELEMETRY_QUERY_STRING}`,
         { timeoutInSeconds: 1 }
       );
@@ -290,11 +323,8 @@ describe('Auth0', () => {
 
     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 }
       );
@@ -316,6 +346,7 @@ describe('Auth0', () => {
       const { auth0, utils } = await setup();
 
       await auth0.loginWithPopup({});
+
       expect(utils.oauthToken).toHaveBeenCalledWith({
         audience: undefined,
         baseUrl: 'https://test.auth0.com',
@@ -479,7 +510,6 @@ describe('Auth0', () => {
       const { auth0, utils } = await setup();
 
       await auth0.loginWithPopup();
-      expect(utils.openPopup).toHaveBeenCalled();
     });
   });
   describe('buildAuthorizeUrl()', () => {
diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts
index 41ce0a9..9b3796b 100644
--- a/__tests__/utils.test.ts
+++ b/__tests__/utils.test.ts
@@ -8,7 +8,6 @@ import {
   encodeState,
   decodeState,
   sha256,
-  openPopup,
   runPopup,
   runIframe,
   urlDecodeB64,
@@ -239,21 +238,6 @@ describe('utils', () => {
       expect(result).toBe('dGVzdA');
     });
   });
-  describe('openPopup', () => {
-    it('opens the popup', () => {
-      window.open = <any>jest.fn(() => true);
-      expect(openPopup()).toBe(true);
-      expect(window.open).toHaveBeenCalledWith(
-        '',
-        'auth0:authorize:popup',
-        'left=312,top=84,width=400,height=600,resizable,scrollbars=yes,status=1'
-      );
-    });
-    it('throws error when the popup is blocked', () => {
-      window.open = jest.fn(() => undefined);
-      expect(openPopup).toThrowError('Could not open popup');
-    });
-  });
   describe('oauthToken', () => {
     let oauthToken;
     let mockUnfetch;
@@ -337,18 +321,23 @@ describe('utils', () => {
   });
   describe('runPopup', () => {
     const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' };
+
+    const url = 'https://authorize.com';
+
     const setup = customMessage => {
       const popup = {
-        location: { href: '' },
+        location: { href: url },
         close: jest.fn()
       };
-      const url = 'https://authorize.com';
+
       window.addEventListener = <any>jest.fn((message, callback) => {
         expect(message).toBe('message');
         callback(customMessage);
       });
+
       return { popup, url };
     };
+
     describe('with invalid messages', () => {
       ['', {}, { data: 'test' }, { data: { type: 'other-type' } }].forEach(
         m => {
@@ -364,7 +353,7 @@ describe('utils', () => {
               jest.runAllTimers();
             }, 10);
             jest.useFakeTimers();
-            await expect(runPopup(popup, url, {})).rejects.toMatchObject(
+            await expect(runPopup(url, { popup })).rejects.toMatchObject(
               TIMEOUT_ERROR
             );
             jest.useRealTimers();
@@ -372,6 +361,7 @@ describe('utils', () => {
         }
       );
     });
+
     it('returns authorization response message', async () => {
       const message = {
         data: {
@@ -379,13 +369,17 @@ describe('utils', () => {
           response: { id_token: 'id_token' }
         }
       };
+
       const { popup, url } = setup(message);
-      await expect(runPopup(popup, url, {})).resolves.toMatchObject(
+
+      await expect(runPopup(url, { popup })).resolves.toMatchObject(
         message.data.response
       );
+
       expect(popup.location.href).toBe(url);
       expect(popup.close).toHaveBeenCalled();
     });
+
     it('returns authorization error message', async () => {
       const message = {
         data: {
@@ -393,16 +387,21 @@ describe('utils', () => {
           response: { error: 'error' }
         }
       };
+
       const { popup, url } = setup(message);
-      await expect(runPopup(popup, url, {})).rejects.toMatchObject(
+
+      await expect(runPopup(url, { popup })).rejects.toMatchObject(
         message.data.response
       );
+
       expect(popup.location.href).toBe(url);
       expect(popup.close).toHaveBeenCalled();
     });
+
     it('times out after config.timeoutInSeconds', async () => {
       const { popup, url } = setup('');
       const seconds = 10;
+
       /**
        * We need to run the timers after we start `runPopup`, but we also
        * need to use `jest.useFakeTimers` to trigger the timeout.
@@ -412,16 +411,21 @@ describe('utils', () => {
       setTimeout(() => {
         jest.runTimersToTime(seconds * 1000);
       }, 10);
+
       jest.useFakeTimers();
+
       await expect(
-        runPopup(popup, url, {
-          timeoutInSeconds: seconds
+        runPopup(url, {
+          timeoutInSeconds: seconds,
+          popup
         })
       ).rejects.toMatchObject({ ...TIMEOUT_ERROR, popup });
+
       jest.useRealTimers();
     });
     it('times out after DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS if config is not defined', async () => {
       const { popup, url } = setup('');
+
       /**
        * We need to run the timers after we start `runPopup`, but we also
        * need to use `jest.useFakeTimers` to trigger the timeout.
@@ -431,12 +435,38 @@ describe('utils', () => {
       setTimeout(() => {
         jest.runTimersToTime(DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS * 1000);
       }, 10);
+
       jest.useFakeTimers();
-      await expect(runPopup(popup, url, {})).rejects.toMatchObject(
+
+      await expect(runPopup(url, { popup })).rejects.toMatchObject(
         TIMEOUT_ERROR
       );
+
       jest.useRealTimers();
     });
+
+    it('creates and uses a popup window if none was given', async () => {
+      const message = {
+        data: {
+          type: 'authorization_response',
+          response: { id_token: 'id_token' }
+        }
+      };
+
+      const { popup, url } = setup(message);
+      const oldOpenFn = window.open;
+
+      window.open = <any>jest.fn(() => popup);
+
+      await expect(runPopup(url, {})).resolves.toMatchObject(
+        message.data.response
+      );
+
+      expect(popup.location.href).toBe(url);
+      expect(popup.close).toHaveBeenCalled();
+
+      window.open = oldOpenFn;
+    });
   });
   describe('runIframe', () => {
     const TIMEOUT_ERROR = { error: 'timeout', error_description: 'Timeout' };