Update msal-browser SilentHandler.monitorIframeForHash to be purely timing-based

lib/msal-browser/src/app/PublicClientApplication.ts

@@ -283,7 +283,7 @@ export class PublicClientApplication implements IPublicClientApplication {
         // Show the UI once the url has been created. Get the window handle for the popup.
         const popupWindow: Window = interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest);
         // Monitor the window for the hash. Return the string value and close the popup when the hash is received. Default timeout is 60 seconds.
-        const hash = await interactionHandler.monitorWindowForHash(popupWindow, this.config.system.windowHashTimeout, navigateUrl);
+        const hash = await interactionHandler.monitorPopupForHash(popupWindow, this.config.system.windowHashTimeout);
         // Handle response from hash string.
         return await interactionHandler.handleCodeResponse(hash);
     }
@@ -405,7 +405,7 @@ export class PublicClientApplication implements IPublicClientApplication {
             // Get the frame handle for the silent request
             const msalFrame = await silentHandler.initiateAuthRequest(navigateUrl, authCodeRequest, userRequestScopes);
             // Monitor the window for the hash. Return the string value and close the popup when the hash is received. Default timeout is 60 seconds.
-            const hash = await silentHandler.monitorFrameForHash(msalFrame, this.config.system.iframeHashTimeout, navigateUrl);
+            const hash = await silentHandler.monitorIframeForHash(msalFrame, this.config.system.iframeHashTimeout);
             // Handle response from hash string.
             return await silentHandler.handleCodeResponse(hash);
         } catch (e) {

lib/msal-browser/src/error/BrowserAuthError.ts

@@ -180,10 +180,9 @@ export class BrowserAuthError extends AuthError {
      * Creates an error thrown when monitorWindowFromHash times out for a given popup.
      * @param urlNavigate 
      */
-    static createMonitorWindowTimeoutError(urlNavigate: string): BrowserAuthError {
-        const errorMessage = `URL navigated to is ${urlNavigate}, ${BrowserAuthErrorMessage.monitorWindowTimeoutError.desc}`;
+    static createMonitorWindowTimeoutError(): BrowserAuthError {
         return new BrowserAuthError(BrowserAuthErrorMessage.monitorWindowTimeoutError.code,
-            errorMessage);
+            BrowserAuthErrorMessage.monitorWindowTimeoutError.desc);
     }
 
     /**

lib/msal-browser/src/interaction_handler/PopupHandler.ts

@@ -46,17 +46,17 @@ export class PopupHandler extends InteractionHandler {
 
     /**
      * Monitors a window until it loads a url with a known hash, or hits a specified timeout.
-     * @param contentWindow - window that is being monitored
+     * @param popupWindow - window that is being monitored
      * @param timeout - milliseconds until timeout
      * @param urlNavigate - url that was navigated to
      */
-    monitorWindowForHash(contentWindow: Window, timeout: number, urlNavigate: string): Promise<string> {
+    monitorPopupForHash(popupWindow: Window, timeout: number): Promise<string> {
         return new Promise((resolve, reject) => {
             const maxTicks = timeout / BrowserConstants.POLL_INTERVAL_MS;
             let ticks = 0;
 
             const intervalId = setInterval(() => {
-                if (contentWindow.closed) {
+                if (popupWindow.closed) {
                     // Window is closed
                     this.cleanPopup();
                     clearInterval(intervalId);
@@ -71,7 +71,7 @@ export class PopupHandler extends InteractionHandler {
                      * which should be caught and ignored
                      * since we need the interval to keep running while on STS UI.
                      */
-                    href = contentWindow.location.href;
+                    href = popupWindow.location.href;
                 } catch (e) {}
 
                 // Don't process blank pages or cross domain
@@ -84,16 +84,16 @@ export class PopupHandler extends InteractionHandler {
 
                 if (UrlString.hashContainsKnownProperties(href)) {
                     // Success case
-                    const contentHash = contentWindow.location.hash;
-                    this.cleanPopup(contentWindow);
+                    const contentHash = popupWindow.location.hash;
+                    this.cleanPopup(popupWindow);
                     clearInterval(intervalId);
                     resolve(contentHash);
                     return;
                 } else if (ticks > maxTicks) {
                     // Timeout error
-                    this.cleanPopup(contentWindow);
+                    this.cleanPopup(popupWindow);
                     clearInterval(intervalId);
-                    reject(BrowserAuthError.createMonitorWindowTimeoutError(urlNavigate));
+                    reject(BrowserAuthError.createMonitorWindowTimeoutError());
                     return;
                 }
             }, BrowserConstants.POLL_INTERVAL_MS);

lib/msal-browser/src/interaction_handler/SilentHandler.ts

@@ -35,22 +35,23 @@ export class SilentHandler extends InteractionHandler {
 
     /**
      * Monitors an iframe content window until it loads a url with a known hash, or hits a specified timeout.
-     * @param iframeContentWindow 
+     * @param iframe 
      * @param timeout 
-     * @param urlNavigate 
      */
-    monitorFrameForHash(iframe: HTMLIFrameElement, timeout: number, urlNavigate: string): Promise<string> {
-        const contentWindow = iframe.contentWindow;
+    monitorIframeForHash(iframe: HTMLIFrameElement, timeout: number): Promise<string> {
         return new Promise((resolve, reject) => {
-            const maxTicks = timeout / BrowserConstants.POLL_INTERVAL_MS;
-            let ticks = 0;
+            /*
+             * Polling for iframes can be purely timing based,
+             * since we don't need to account for interaction.
+             */
+            const nowMark = window.performance.now();
+            const timeoutMark = nowMark + timeout;
 
             const intervalId = setInterval(() => {
-                if (contentWindow.closed) {
-                    // Window is closed
+                if (window.performance.now() > timeoutMark) {
                     this.removeHiddenIframe(iframe);
                     clearInterval(intervalId);
-                    reject(BrowserAuthError.createIframeClosedPrematurelyError());
+                    reject(BrowserAuthError.createMonitorWindowTimeoutError());
                     return;
                 }
 
@@ -61,29 +62,16 @@ export class SilentHandler extends InteractionHandler {
                      * which should be caught and ignored
                      * since we need the interval to keep running while on STS UI.
                      */
-                    href = contentWindow.location.href;
+                    href = iframe.contentWindow.location.href;
                 } catch (e) {}
 
-                /*
-                 * Always run clock for silent calls
-                 * as silent operations should be short,
-                 * and to ensure they always at worst timeout.
-                 */
-                ticks++;
-
                 if (UrlString.hashContainsKnownProperties(href)) {
                     // Success case
-                    const contentHash = contentWindow.location.hash;
+                    const contentHash = iframe.contentWindow.location.hash;
                     this.removeHiddenIframe(iframe);
                     clearInterval(intervalId);
                     resolve(contentHash);
                     return;
-                } else if (ticks > maxTicks) {
-                    // Timeout error
-                    this.removeHiddenIframe(iframe);
-                    clearInterval(intervalId);
-                    reject(BrowserAuthError.createMonitorWindowTimeoutError(urlNavigate));
-                    return;
                 }
             }, BrowserConstants.POLL_INTERVAL_MS);
         });

lib/msal-browser/test/app/PublicClientApplication.spec.ts

@@ -706,7 +706,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                     expect(requestUrl).to.be.eq(testNavUrl);
                     return window;
                 });
-                sinon.stub(PopupHandler.prototype, "monitorWindowForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+                sinon.stub(PopupHandler.prototype, "monitorPopupForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
                 sinon.stub(PopupHandler.prototype, "handleCodeResponse").resolves(testTokenResponse);
                 sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
                     challenge: TEST_CONFIG.TEST_CHALLENGE,
@@ -791,7 +791,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                     expect(requestUrl).to.be.eq(testNavUrl);
                     return window;
                 });
-                sinon.stub(PopupHandler.prototype, "monitorWindowForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+                sinon.stub(PopupHandler.prototype, "monitorPopupForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
                 sinon.stub(PopupHandler.prototype, "handleCodeResponse").resolves(testTokenResponse);
                 sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
                     challenge: TEST_CONFIG.TEST_CHALLENGE,
@@ -891,7 +891,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
             };
             sinon.stub(AuthorizationCodeClient.prototype, "getAuthCodeUrl").resolves(testNavUrl);
             const loadFrameSyncSpy = sinon.spy(SilentHandler.prototype, <any>"loadFrameSync");
-            sinon.stub(SilentHandler.prototype, "monitorFrameForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+            sinon.stub(SilentHandler.prototype, "monitorIframeForHash").resolves(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
             sinon.stub(SilentHandler.prototype, "handleCodeResponse").resolves(testTokenResponse);
             sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
                 challenge: TEST_CONFIG.TEST_CHALLENGE,

lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts

@@ -179,4 +179,68 @@ describe("PopupHandler.ts Unit Tests", () => {
             expect(browserStorage.getItem(browserStorage.generateCacheKey(BrowserConstants.INTERACTION_STATUS_KEY), CacheSchemaType.TEMPORARY)).to.be.eq(BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
         });
     });
+
+    describe("monitorPopupForHash", () => {
+        it("times out", done => {
+            const popup = {
+                location: {
+                    href: "http://localhost",
+                    hash: ""
+                },
+                close: () => {}
+            };
+
+            // @ts-ignore
+            popupHandler.monitorPopupForHash(popup, 500)
+                .catch(() => {
+                    done();
+                });
+        });
+
+        it("returns hash", done => {
+            const popup = {
+                location: {
+                    href: "http://localhost",
+                    hash: ""
+                },
+                close: () => {}
+            };
+
+            // @ts-ignore
+            popupHandler.monitorPopupForHash(popup, 1000)
+                .then((hash: string) => {
+                    expect(hash).to.equal("#code=hello");
+                    done();
+                });
+
+            setTimeout(() => {
+                popup.location = {
+                    href: "http://localhost/#/code=hello",
+                    hash: "#code=hello"
+                };
+            }, 500);
+        });
+
+        it("closed", done => {
+            const popup = {
+                location: {
+                    href: "http://localhost",
+                    hash: ""
+                },
+                close: () => {},
+                closed: false
+            };
+
+            // @ts-ignore
+            popupHandler.monitorPopupForHash(popup, 1000)
+                .catch((error) => {
+                    expect(error.errorCode).to.equal('user_cancelled');
+                    done();
+                });
+
+            setTimeout(() => {
+                popup.closed = true;
+            }, 500);
+        });
+    });
 });

lib/msal-browser/test/interaction_handler/SilentHandler.spec.ts

@@ -195,4 +195,83 @@ describe("SilentHandler.ts Unit Tests", () => {
             expect(authFrame.id).to.be.eq("msalTokenFrame");
         });
     });
+
+    describe("monitorIframeForHash", () => {
+        it("times out", done => {
+            const iframe = {
+                contentWindow: {
+                    // @ts-ignore
+                    location: null // example of scenario that would never otherwise resolve
+                }
+            };
+
+            // @ts-ignore
+            silentHandler.monitorIframeForHash(iframe, 500)
+                .catch(() => {
+                    done();
+                });
+        });
+
+        it("times out when event loop is suspended", function(done) {
+            this.timeout(5000);
+
+            const iframe = {
+                contentWindow: {
+                    location: {
+                        href: "http://localhost",
+                        hash: ""
+                    }
+                }
+            };
+
+            // @ts-ignore
+            silentHandler.monitorIframeForHash(iframe, 2000)
+                .catch(() => {
+                    done();
+                });
+
+            setTimeout(() => {
+                iframe.contentWindow.location = {
+                    href: "http://localhost/#/code=hello",
+                    hash: "#code=hello"
+                };
+            }, 1600);
+
+            /**
+             * This code mimics the JS event loop being synchonously paused (e.g. tab suspension) midway through polling the iframe.
+             * If the event loop is suspended for longer than the configured timeout,
+             * the polling operation should throw an error for a timeout.
+             */
+            const startPauseDelay = 200;
+            const pauseDuration = 3000;
+            setTimeout(() => {
+                Atomics.wait(new Int32Array(new SharedArrayBuffer(4)), 0, 0, pauseDuration);
+            }, startPauseDelay);
+        });
+
+        it("returns hash", done => {
+            const iframe = {
+                contentWindow: {
+                    location: {
+                        href: "http://localhost",
+                        hash: ""
+                    }
+                }
+            };
+
+            // @ts-ignore
+            silentHandler.monitorIframeForHash(iframe, 1000)
+                .then((hash: string) => {
+                    expect(hash).to.equal("#code=hello");
+                    done();
+                });
+
+            setTimeout(() => {
+                iframe.contentWindow.location = {
+                    href: "http://localhost/#code=hello",
+                    hash: "#code=hello"
+                };
+            }, 500);
+        });
+    });
 });

lib/msal-core/src/utils/WindowUtils.ts

@@ -50,7 +50,7 @@ export class WindowUtils {
         return new Promise((resolve, reject) => {
             /*
              * Polling for iframes can be purely timing based,
-             * since we don't need to accout for interaction.
+             * since we don't need to account for interaction.
              */
             const nowMark = TimeUtils.relativeNowMs();
             const timeoutMark = nowMark + timeout;

lib/msal-core/test/utils/WindowUtils.spec.ts

@@ -73,7 +73,7 @@ describe("WindowUtils", () => {
             };
 
             // @ts-ignore
-            WindowUtils.monitorPopupForHash(iframe.contentWindow, 1000, "url", logger)
+            WindowUtils.monitorIframeForHash(iframe.contentWindow, 1000, "url", logger)
                 .then((hash: string) => {
                     expect(hash).to.equal("#access_token=hello");
                     done();