Merge pull request #2132 from AzureAD/browser-popup-window-change

[msal-browser] Open popup change with configuration
AzureAD · Aug 20, 2020 · 98c3f78 · 98c3f78
2 parents 1ef8c3d + abda11c
commit 98c3f78
Show file tree

Hide file tree

Showing 8 changed files with 229 additions and 29 deletions.
diff --git a/change/@azure-msal-browser-2020-08-12-16-02-55-browser-popup-window-change.json b/change/@azure-msal-browser-2020-08-12-16-02-55-browser-popup-window-change.json
@@ -0,0 +1,8 @@
+{
+  "type": "patch",
+  "comment": "Change msal-browser loginPopup and openPopup, add ability to configure popup delay (#2132)",
+  "packageName": "@azure/msal-browser",
+  "email": "joarroyo@microsoft.com",
+  "dependentChangeType": "patch",
+  "date": "2020-08-12T23:02:55.249Z"
+}
diff --git a/lib/msal-browser/docs/configuration.md b/lib/msal-browser/docs/configuration.md
@@ -48,7 +48,8 @@ const msalConfig = {
         },
         windowHashTimeout: 60000,
         iframeHashTimeout: 6000,
-        loadFrameTimeout: 0
+        loadFrameTimeout: 0,
+        asyncPopups: false
     };
 }
 
@@ -81,6 +82,7 @@ const msalInstance = new PublicClientApplication(msalConfig);
 | `windowHashTimeout` | Timeout in milliseconds to wait for popup authentication to resolve. | integer (milliseconds) | `60000` |
 | `iframeHashTimeout` | Timeout in milliseconds to wait for iframe authentication to resolve | integer (milliseconds) | `6000` |
 | `loadFrameTimeout` | Timeout in milliseconds to wait for the iframe to load in the window. | integer (milliseconds) | In IE or Edge: `500`, in all other browsers: `0` |
+| `asyncPopups` | Sets whether popups are opened asynchronously. When set to false, blank popups are opened before anything else happens. When set to true, popups are opened when making the network request. Can be set to true for scenarios where `about:blank` is not supported, e.g. desktop apps or progressive web apps | boolean | `false` |
 
 ### Logger Config Options
 | Option | Description | Format | Default Value |

diff --git a/lib/msal-browser/src/app/PublicClientApplication.ts b/lib/msal-browser/src/app/PublicClientApplication.ts
@@ -291,7 +291,7 @@ export class PublicClientApplication implements IPublicClientApplication {
      *
      * @returns {Promise.<AuthenticationResult>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
      */
-    async loginPopup(request?: PopupRequest): Promise<AuthenticationResult> {
+    loginPopup(request?: PopupRequest): Promise<AuthenticationResult> {
         return this.acquireTokenPopup(request || DEFAULT_REQUEST);
     }
 
@@ -301,7 +301,24 @@ export class PublicClientApplication implements IPublicClientApplication {
      *
      * @returns {Promise.<AuthenticationResult>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
      */
-    async acquireTokenPopup(request: PopupRequest): Promise<AuthenticationResult> {
+    acquireTokenPopup(request: PopupRequest): Promise<AuthenticationResult> {
+        // asyncPopups flag is true. Acquires token without first opening popup. Popup will be opened later asynchronously.
+        if (this.config.system.asyncPopups) {
+            return this.acquireTokenPopupAsync(request);
+        } else {
+            // asyncPopups flag is set to false. Opens popup before acquiring token.
+            const popup = PopupHandler.openSizedPopup();
+            return this.acquireTokenPopupAsync(request, popup);
+        }
+    }
+
+    /**
+     * Helper which obtains an access_token for your API via opening a popup window in the user's browser
+     * @param {@link (PopupRequest:type)}
+     *
+     * @returns {Promise.<AuthenticationResult>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
+     */
+    private async acquireTokenPopupAsync(request: PopupRequest, popup?: Window|null): Promise<AuthenticationResult> {
         // Preflight request
         const validRequest: AuthorizationUrlRequest = this.preflightInteractiveRequest(request, InteractionType.POPUP);
         const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenPopup, validRequest.correlationId);
@@ -320,7 +337,7 @@ export class PublicClientApplication implements IPublicClientApplication {
             const interactionHandler = new PopupHandler(authClient, this.browserStorage);
 
             // Show the UI once the url has been created. Get the window handle for the popup.
-            const popupWindow: Window = interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest);
+            const popupWindow: Window = interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest, popup);
 
             // 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.monitorPopupForHash(popupWindow, this.config.system.windowHashTimeout);

diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts
@@ -48,16 +48,18 @@ export type CacheOptions = {
  * - tokenRenewalOffsetSeconds    - Sets the window of offset needed to renew the token before expiry
  * - loggerOptions                - Used to initialize the Logger object (See ClientConfiguration.ts)
  * - networkClient                - Network interface implementation
- * - windowHashTimeout            - sets the timeout for waiting for a response hash in a popup
- * - iframeHashTimeout            - sets the timeout for waiting for a response hash in an iframe
- * - loadFrameTimeout             - maximum time the library should wait for a frame to load
+ * - windowHashTimeout            - Sets the timeout for waiting for a response hash in a popup
+ * - iframeHashTimeout            - Sets the timeout for waiting for a response hash in an iframe
+ * - loadFrameTimeout             - Maximum time the library should wait for a frame to load
+ * - asyncPopups                  - Sets whether popups are opened asynchronously. By default, this flag is set to false. When set to false, blank popups are opened before anything else happens. When set to true, popups are opened when making the network request.
  */
 export type BrowserSystemOptions = SystemOptions & {
     loggerOptions?: LoggerOptions;
     networkClient?: INetworkModule;
     windowHashTimeout?: number;
     iframeHashTimeout?: number;
     loadFrameTimeout?: number;
+    asyncPopups?: boolean;
 };
 
 /**
@@ -104,7 +106,8 @@ const DEFAULT_BROWSER_SYSTEM_OPTIONS: BrowserSystemOptions = {
     networkClient: BrowserUtils.getBrowserNetworkClient(),
     windowHashTimeout: DEFAULT_POPUP_TIMEOUT_MS,
     iframeHashTimeout: DEFAULT_IFRAME_TIMEOUT_MS,
-    loadFrameTimeout: BrowserUtils.detectIEOrEdge() ? 500 : 0
+    loadFrameTimeout: BrowserUtils.detectIEOrEdge() ? 500 : 0,
+    asyncPopups: false
 };
 
 /**

diff --git a/lib/msal-browser/src/interaction_handler/PopupHandler.ts b/lib/msal-browser/src/interaction_handler/PopupHandler.ts
@@ -27,7 +27,7 @@ export class PopupHandler extends InteractionHandler {
      * Opens a popup window with given request Url.
      * @param requestUrl
      */
-    initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest): Window {
+    initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest, popup?: Window|null): Window {
         // Check that request url is not empty.
         if (!StringUtils.isEmpty(requestUrl)) {
             // Save auth code request
@@ -36,7 +36,7 @@ export class PopupHandler extends InteractionHandler {
             this.browserStorage.setItem(this.browserStorage.generateCacheKey(BrowserConstants.INTERACTION_STATUS_KEY), BrowserConstants.INTERACTION_IN_PROGRESS_VALUE, CacheSchemaType.TEMPORARY);
             this.authModule.logger.infoPii("Navigate to:" + requestUrl);
             // Open the popup window to requestUrl.
-            return this.openPopup(requestUrl, Constants.LIBRARY_NAME, BrowserConstants.POPUP_WIDTH, BrowserConstants.POPUP_HEIGHT);
+            return this.openPopup(requestUrl, popup);
         } else {
             // Throw error if request URL is empty.
             this.authModule.logger.error("Navigate url is empty");
@@ -111,25 +111,19 @@ export class PopupHandler extends InteractionHandler {
      * @ignore
      * @hidden
      */
-    private openPopup(urlNavigate: string, title: string, popUpWidth: number, popUpHeight: number): Window {
+    private openPopup(urlNavigate: string, popup?: Window|null): Window {
         try {
-            /**
-             * adding winLeft and winTop to account for dual monitor
-             * using screenLeft and screenTop for IE8 and earlier
-             */
-            const winLeft = window.screenLeft ? window.screenLeft : window.screenX;
-            const winTop = window.screenTop ? window.screenTop : window.screenY;
-            /**
-             * window.innerWidth displays browser window"s height and width excluding toolbars
-             * using document.documentElement.clientWidth for IE8 and earlier
-             */
-            const width = window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
-            const height = window.innerHeight || document.documentElement.clientHeight || document.body.clientHeight;
-            const left = Math.max(0, ((width / 2) - (popUpWidth / 2)) + winLeft);
-            const top = Math.max(0, ((height / 2) - (popUpHeight / 2)) + winTop);
-
-            // open the window
-            const popupWindow = window.open(urlNavigate, title, "width=" + popUpWidth + ", height=" + popUpHeight + ", top=" + top + ", left=" + left);
+            let popupWindow;
+            // Popup window passed in, setting url to navigate to
+            if (popup) {
+                popupWindow = popup;
+                popupWindow.location.assign(urlNavigate);
+            } else if (typeof popup === "undefined") { 
+                // Popup will be undefined if it was not passed in
+                popupWindow = PopupHandler.openSizedPopup(urlNavigate);
+            }
+
+            // Popup will be null if popups are blocked
             if (!popupWindow) {
                 throw BrowserAuthError.createEmptyWindowCreatedError();
             }
@@ -147,6 +141,25 @@ export class PopupHandler extends InteractionHandler {
         }
     }
 
+    static openSizedPopup(urlNavigate: string = "about:blank"): Window|null {
+        /**
+         * adding winLeft and winTop to account for dual monitor
+         * using screenLeft and screenTop for IE8 and earlier
+         */
+        const winLeft = window.screenLeft ? window.screenLeft : window.screenX;
+        const winTop = window.screenTop ? window.screenTop : window.screenY;
+        /**
+         * window.innerWidth displays browser window"s height and width excluding toolbars
+         * using document.documentElement.clientWidth for IE8 and earlier
+         */
+        const width = window.innerWidth || document.documentElement.clientWidth || document.body.clientWidth;
+        const height = window.innerHeight || document.documentElement.clientHeight || document.body.clientHeight;
+        const left = Math.max(0, ((width / 2) - (BrowserConstants.POPUP_WIDTH / 2)) + winLeft);
+        const top = Math.max(0, ((height / 2) - (BrowserConstants.POPUP_HEIGHT / 2)) + winTop);
+
+        return window.open(urlNavigate, Constants.LIBRARY_NAME, "width=" + BrowserConstants.POPUP_WIDTH + ", height=" + BrowserConstants.POPUP_HEIGHT + ", top=" + top + ", left=" + left);
+    }
+
     /**
      * Event callback to unload main window.
      */

diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts
@@ -893,6 +893,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
     describe("Popup Flow Unit tests", () => {
 
         describe("loginPopup", () => {
+            beforeEach(() => {
+                sinon.stub(window, "open").returns(window);
+            });
+
+            afterEach(() => {
+                sinon.restore();
+            });
 
             it("throws error if interaction is in progress", async () => {
                 window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${BrowserConstants.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
@@ -992,6 +999,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
         });
 
         describe("acquireTokenPopup", () => {
+            beforeEach(() => {
+                sinon.stub(window, "open").returns(window);
+            });
+
+            afterEach(() => {
+                sinon.restore();
+            });
 
             it("throws error if interaction is in progress", async () => {
                 window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${BrowserConstants.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
@@ -1005,6 +1019,50 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                 })).rejectedWith(BrowserAuthError);
             });
 
+            it("opens popup window before network request by default", () => {
+                const request: AuthorizationUrlRequest = {
+					redirectUri: TEST_URIS.TEST_REDIR_URI,
+					scopes: ["scope"],
+					loginHint: "AbeLi@microsoft.com",
+                    state: TEST_STATE_VALUES.USER_STATE
+                };
+
+                sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
+                    challenge: TEST_CONFIG.TEST_CHALLENGE,
+                    verifier: TEST_CONFIG.TEST_VERIFIER
+                });
+
+                const popupSpy = sinon.stub(PopupHandler, "openSizedPopup");
+
+                pca.acquireTokenPopup(request);
+                expect(popupSpy.calledWith()).to.be.true;
+            });
+
+            it("opens popups asynchronously if configured", () => {
+                pca = new PublicClientApplication({
+                    system: {
+                        asyncPopups: true
+                    }
+                });
+
+                sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
+                    challenge: TEST_CONFIG.TEST_CHALLENGE,
+                    verifier: TEST_CONFIG.TEST_VERIFIER
+                });
+
+                const request: AuthorizationUrlRequest = {
+					redirectUri: TEST_URIS.TEST_REDIR_URI,
+					scopes: ["scope"],
+					loginHint: "AbeLi@microsoft.com",
+                    state: TEST_STATE_VALUES.USER_STATE
+                };
+
+                const popupSpy = sinon.stub(PopupHandler, "openSizedPopup");
+
+                pca.acquireTokenPopup(request);
+                expect(popupSpy.calledWith()).to.be.false;
+            });
+
             it("resolves the response successfully", async () => {
                 const testServerTokenResponse = {
                     token_type: TEST_CONFIG.TOKEN_TYPE_BEARER,

diff --git a/lib/msal-browser/test/config/Configuration.spec.ts b/lib/msal-browser/test/config/Configuration.spec.ts
@@ -47,6 +47,7 @@ describe("Configuration.ts Class Unit Tests", () => {
         expect(emptyConfig.system.windowHashTimeout).to.be.not.null.and.not.undefined;
         expect(emptyConfig.system.windowHashTimeout).to.be.eq(DEFAULT_POPUP_TIMEOUT_MS);
         expect(emptyConfig.system.tokenRenewalOffsetSeconds).to.be.eq(300);
+        expect(emptyConfig.system.asyncPopups).to.be.false;
     });
 
     it("Tests logger", () => {
@@ -114,7 +115,8 @@ describe("Configuration.ts Class Unit Tests", () => {
                 loggerOptions: {
                     loggerCallback: testLoggerCallback,
                     piiLoggingEnabled: true
-                }
+                },
+                asyncPopups: true
             }
         });
         // Auth config checks
@@ -139,5 +141,6 @@ describe("Configuration.ts Class Unit Tests", () => {
         expect(newConfig.system.loggerOptions).to.be.not.null;
         expect(newConfig.system.loggerOptions.loggerCallback).to.be.not.null;
         expect(newConfig.system.loggerOptions.piiLoggingEnabled).to.be.true;
+        expect(newConfig.system.asyncPopups).to.be.true;
     });
 });
diff --git a/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts b/lib/msal-browser/test/interaction_handler/PopupHandler.spec.ts
@@ -242,4 +242,100 @@ describe("PopupHandler.ts Unit Tests", () => {
             }, 500);
         });
     });
+
+    describe("openPopup", () => {
+        afterEach(() => {
+            sinon.restore();
+        });
+
+        it("assigns urlNavigate if popup passed in", () => {
+            const assignSpy = sinon.spy();
+            const focusSpy = sinon.spy();
+
+            let windowObject = {
+                location: {
+                    assign: assignSpy
+                },
+                focus: focusSpy
+            };
+
+            const testRequest: AuthorizationCodeRequest = {
+				redirectUri: "",
+				code: "thisIsATestCode",
+				scopes: TEST_CONFIG.DEFAULT_SCOPES,
+				codeVerifier: TEST_CONFIG.TEST_VERIFIER,
+				authority: `${Constants.DEFAULT_AUTHORITY}/`,
+				correlationId: RANDOM_TEST_GUID
+            };
+
+            // @ts-ignore
+            const popupWindow = popupHandler.initiateAuthRequest("http://localhost/#/code=hello", testRequest, windowObject);
+
+            expect(assignSpy.calledWith("http://localhost/#/code=hello")).to.be.true;
+            expect(popupWindow).to.equal(windowObject);
+        });
+
+        it("opens popup if no popup window is passed in", () => {
+            sinon.stub(window, "open").returns(window);
+            sinon.stub(window, "focus");
+
+            const testRequest: AuthorizationCodeRequest = {
+				redirectUri: "",
+				code: "thisIsATestCode",
+				scopes: TEST_CONFIG.DEFAULT_SCOPES,
+				codeVerifier: TEST_CONFIG.TEST_VERIFIER,
+				authority: `${Constants.DEFAULT_AUTHORITY}/`,
+				correlationId: RANDOM_TEST_GUID
+            };
+
+            const popupWindow = popupHandler.initiateAuthRequest("http://localhost/#/code=hello", testRequest);
+
+            expect(popupWindow).to.equal(window);
+        });
+
+        it("throws error if no popup passed in but window.open returns null", () => {
+            sinon.stub(window, "open").returns(null);
+
+            const testRequest: AuthorizationCodeRequest = {
+				redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`,
+				code: "thisIsATestCode",
+				scopes: TEST_CONFIG.DEFAULT_SCOPES,
+				codeVerifier: TEST_CONFIG.TEST_VERIFIER,
+				authority: `${Constants.DEFAULT_AUTHORITY}/`,
+				correlationId: RANDOM_TEST_GUID
+            };
+
+            expect(() => popupHandler.initiateAuthRequest("http://localhost/#/code=hello", testRequest)).to.throw(BrowserAuthErrorMessage.emptyWindowError.desc);
+        });
+
+        it("throws error if popup passed in is null", () => {
+            const testRequest: AuthorizationCodeRequest = {
+				redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`,
+				code: "thisIsATestCode",
+				scopes: TEST_CONFIG.DEFAULT_SCOPES,
+				codeVerifier: TEST_CONFIG.TEST_VERIFIER,
+				authority: `${Constants.DEFAULT_AUTHORITY}/`,
+				correlationId: RANDOM_TEST_GUID
+            };
+
+            expect(() => popupHandler.initiateAuthRequest("http://localhost/#/code=hello", testRequest, null)).to.throw(BrowserAuthErrorMessage.emptyWindowError.desc);
+            expect(() => popupHandler.initiateAuthRequest("http://localhost/#/code=hello", testRequest, null)).to.throw(BrowserAuthError);
+        });
+    });
+
+    describe("openSizedPopup", () => {
+        it("opens a popup with urlNavigate", () => {
+            const windowOpenSpy = sinon.stub(window, "open");
+            PopupHandler.openSizedPopup("http://localhost/");
+
+            expect(windowOpenSpy.calledWith("http://localhost/")).to.be.true;
+        });
+
+        it("opens a popup with about:blank if no urlNavigate passed in", () => {
+            const windowOpenSpy = sinon.stub(window, "open");
+            PopupHandler.openSizedPopup();
+
+            expect(windowOpenSpy.calledWith("about:blank")).to.be.true;
+        });
+    });
 });