Fix leaking windows message event listener (#422)

* Fix leaking event listener on loginWithPopup * iframe symbol can just be a const * Fix tests * Move event listener removal and add test assertions * Reinstate removeEventListener inside iframeEventHandler Co-authored-by: Steve Hobbs <[email protected]> Co-authored-by: Steve Hobbs <[email protected]>
auth0 · Oct 29, 2020 · ed9b388 · ed9b388
1 parent 185ffba
commit ed9b388
Show file tree

Hide file tree

Showing 2 changed files with 35 additions and 9 deletions.
diff --git a/__tests__/utils.test.ts b/__tests__/utils.test.ts
@@ -793,6 +793,7 @@ describe('utils', () => {
       jest.runAllTimers();
       expect(message.source.close).toHaveBeenCalled();
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
+      expect(window.removeEventListener).toBeCalled();
     });
     it('times out after timeoutInSeconds', async () => {
       const { iframe, url, origin } = setup('');
@@ -803,6 +804,15 @@ describe('utils', () => {
       await expect(promise).rejects.toMatchObject(TIMEOUT_ERROR);
       expect(window.document.body.removeChild).toHaveBeenCalledWith(iframe);
     });
+    it('removes the message event listener in the event of a timeout', async () => {
+      const { url, origin } = setup('');
+      const seconds = 10;
+      jest.useFakeTimers();
+      const promise = runIframe(url, origin, seconds);
+      jest.runTimersToTime(seconds * 1000);
+      await expect(promise).rejects.toMatchObject(TIMEOUT_ERROR);
+      expect(window.removeEventListener).toBeCalled();
+    });
   });
   describe('getCrypto', () => {
     it('should use msCrypto when window.crypto is unavailable', () => {
@@ -854,9 +864,7 @@ describe('utils', () => {
       (<any>global).crypto = {};
 
       expect(validateCrypto).toThrowError(`
-      auth0-spa-js must run on a secure origin.
-      See https://github.com/auth0/auth0-spa-js/blob/master/FAQ.md#why-do-i-get-auth0-spa-js-must-run-on-a-secure-origin 
-      for more information.
+      auth0-spa-js must run on a secure origin. See https://github.com/auth0/auth0-spa-js/blob/master/FAQ.md#why-do-i-get-auth0-spa-js-must-run-on-a-secure-origin for more information.
     `);
     });
   });

diff --git a/src/utils.ts b/src/utils.ts
@@ -43,37 +43,47 @@ export const runIframe = (
 ) => {
   return new Promise<AuthenticationResult>((res, rej) => {
     const iframe = window.document.createElement('iframe');
+
     iframe.setAttribute('width', '0');
     iframe.setAttribute('height', '0');
     iframe.style.display = 'none';
 
     const removeIframe = () => {
       if (window.document.body.contains(iframe)) {
         window.document.body.removeChild(iframe);
+        window.removeEventListener('message', iframeEventHandler, false);
       }
     };
 
+    let iframeEventHandler: (e: MessageEvent) => void;
+
     const timeoutSetTimeoutId = setTimeout(() => {
       rej(new TimeoutError());
       removeIframe();
     }, timeoutInSeconds * 1000);
 
-    const iframeEventHandler = function (e: MessageEvent) {
+    iframeEventHandler = function (e: MessageEvent) {
       if (e.origin != eventOrigin) return;
       if (!e.data || e.data.type !== 'authorization_response') return;
+
       const eventSource = e.source;
+
       if (eventSource) {
         (eventSource as any).close();
       }
+
       e.data.response.error
         ? rej(GenericError.fromPayload(e.data.response))
         : res(e.data.response);
+
       clearTimeout(timeoutSetTimeoutId);
       window.removeEventListener('message', iframeEventHandler, false);
+
       // Delay the removal of the iframe to prevent hanging loading status
       // in Chrome: https://github.com/auth0/auth0-spa-js/issues/240
       setTimeout(removeIframe, CLEANUP_IFRAME_TIMEOUT_IN_SECONDS * 1000);
     };
+
     window.addEventListener('message', iframeEventHandler, false);
     window.document.body.appendChild(iframe);
     iframe.setAttribute('src', authorizeUrl);
@@ -107,20 +117,30 @@ export const runPopup = (authorizeUrl: string, config: PopupConfigOptions) => {
   }
 
   return new Promise<AuthenticationResult>((resolve, reject) => {
+    let popupEventListener;
+
     const timeoutId = setTimeout(() => {
       reject(new PopupTimeoutError(popup));
+      window.removeEventListener('message', popupEventListener, false);
     }, (config.timeoutInSeconds || DEFAULT_AUTHORIZE_TIMEOUT_IN_SECONDS) * 1000);
-    window.addEventListener('message', e => {
+
+    popupEventListener = function (e: MessageEvent) {
       if (!e.data || e.data.type !== 'authorization_response') {
         return;
       }
+
       clearTimeout(timeoutId);
+      window.removeEventListener('message', popupEventListener, false);
       popup.close();
+
       if (e.data.response.error) {
         return reject(GenericError.fromPayload(e.data.response));
       }
+
       resolve(e.data.response);
-    });
+    };
+
+    window.addEventListener('message', e => popupEventListener(e));
   });
 };
 
@@ -367,9 +387,7 @@ export const validateCrypto = () => {
   }
   if (typeof getCryptoSubtle() === 'undefined') {
     throw new Error(`
-      auth0-spa-js must run on a secure origin.
-      See https://github.com/auth0/auth0-spa-js/blob/master/FAQ.md#why-do-i-get-auth0-spa-js-must-run-on-a-secure-origin 
-      for more information.
+      auth0-spa-js must run on a secure origin. See https://github.com/auth0/auth0-spa-js/blob/master/FAQ.md#why-do-i-get-auth0-spa-js-must-run-on-a-secure-origin for more information.
     `);
   }
 };