Merge branch 'dev' into pkce-fixes

lib/msal-browser/README.md

@@ -74,7 +74,8 @@ If you have MSAL v1.x currently running in your application, you can follow the
 2. [Logging in a User](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/login-user.md)
 3. [Acquiring and Using an Access Token](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/acquire-token.md)
 4. [Managing Token Lifetimes](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/token-lifetimes.md)
-5. [Logging Out a User](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/logout.md)
+5. [Managing Accounts](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-common/docs/Accounts.md)
+6. [Logging Out a User](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/logout.md)
 
 ### Advanced Topics
 
@@ -85,7 +86,7 @@ If you have MSAL v1.x currently running in your application, you can follow the
 
 ## Samples
 
-The [`VanillaJSTestApp2.0` folder](https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/samples) contains sample applications for our libraries. You can run any sample by changing the `authConfig.js` file in the respective folder to match your app registration and running the `npm` command `npm start -- -s <sample-name> -p <port>`. 
+The [`VanillaJSTestApp2.0` folder](https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/samples) contains sample applications for our libraries. You can run any sample by changing the `authConfig.js` file in the respective folder to match your app registration and running the `npm` command `npm start -- -s <sample-name> -p <port>`.
 
 Here is a complete list of samples for the MSAL.js 2.x library:
 
@@ -143,7 +144,7 @@ MSAL.js 1.x implemented the [Implicit Grant Flow](https://docs.microsoft.com/azu
 
 Our goal is that the library abstracts enough of the protocol away so that you can get plug and play authentication, but it is important to know and understand the implicit flow from a security perspective. The MSAL 1.x client for single-page applications runs in the context of a web browser which cannot manage client secrets securely. It uses the implicit flow, which optimized for single page apps and has one less hop between client and server so tokens are returned directly to the browser. These aspects make it naturally less secure. These security concerns are mitigated per standard practices such as- use of short lived tokens (and so no refresh tokens are returned), the library requiring a registered redirect URI for the app, library matching the request and response with a unique nonce and state parameter. You can read more about the [disadvantages of the implicit flow here](https://tools.ietf.org/html/draft-ietf-oauth-browser-based-apps-04#section-9.8.6).
 
-The MSAL library will now support the Authorization Code Flow with PKCE for Browser-Based Applications without a backend web server. 
+The MSAL library will now support the Authorization Code Flow with PKCE for Browser-Based Applications without a backend web server.
 We plan to continue support for the implicit flow in the `msal-core` library.
 
 You can learn further details about `@azure/msal-browser` functionality documented in our [docs folder](https://github.com/AzureAD/microsoft-authentication-library-for-js/tree/dev/lib/msal-browser/docs) and find complete [code samples](#samples).

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);
+        });
+    });
 });