Merge f8e9ee2 into cb1bcbe

AzureAD · Aug 4, 2020 · 9d29cde · 9d29cde
2 parents cb1bcbe + f8e9ee2
commit 9d29cde
Show file tree

Hide file tree

Showing 9 changed files with 52 additions and 70 deletions.
diff --git a/lib/msal-browser/src/app/PublicClientApplication.ts b/lib/msal-browser/src/app/PublicClientApplication.ts
@@ -321,30 +321,24 @@ export class PublicClientApplication implements IPublicClientApplication {
             // Create acquire token url.
             const navigateUrl = await authClient.getAuthCodeUrl(validRequest);
 
-            // Acquire token with popup
-            return await this.popupTokenHelper(navigateUrl, authCodeRequest, authClient);
+            // Create popup interaction handler.
+            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);
+
+            // 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);
+
+            // Handle response from hash string.
+            return await interactionHandler.handleCodeResponse(hash);
         } catch (e) {
             serverTelemetryManager.cacheFailedRequest(e);
             this.browserStorage.cleanRequest();
             throw e;
         }
     }
 
-    /**
-     * Helper which acquires an authorization code with a popup from given url, and exchanges the code for a set of OAuth tokens.
-     * @param navigateUrl
-     */
-    private async popupTokenHelper(navigateUrl: string, authCodeRequest: AuthorizationCodeRequest, authClient: AuthorizationCodeClient): Promise<AuthenticationResult> {
-        // Create popup interaction handler.
-        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);
-        // 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);
-        // Handle response from hash string.
-        return await interactionHandler.handleCodeResponse(hash);
-    }
-
     // #endregion
 
     // #region Silent Flow
@@ -399,7 +393,7 @@ export class PublicClientApplication implements IPublicClientApplication {
             // Create authorize request url
             const navigateUrl = await authClient.getAuthCodeUrl(silentRequest);
 
-            return this.silentTokenHelper(navigateUrl, authCodeRequest, authClient, scopeString);
+            return await this.silentTokenHelper(navigateUrl, authCodeRequest, authClient, scopeString);
         } catch (e) {
             serverTelemetryManager.cacheFailedRequest(e);
             this.browserStorage.cleanRequest();
@@ -457,7 +451,7 @@ export class PublicClientApplication implements IPublicClientApplication {
                     // Get scopeString for iframe ID
                     const scopeString = silentAuthUrlRequest.scopes ? silentAuthUrlRequest.scopes.join(" ") : "";
 
-                    return this.silentTokenHelper(navigateUrl, authCodeRequest, authClient, scopeString);
+                    return await this.silentTokenHelper(navigateUrl, authCodeRequest, authClient, scopeString);
                 } catch (e) {
                     serverTelemetryManager.cacheFailedRequest(e);
                     this.browserStorage.cleanRequest();
@@ -483,7 +477,7 @@ export class PublicClientApplication implements IPublicClientApplication {
         // 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.monitorIframeForHash(msalFrame, this.config.system.iframeHashTimeout);
         // Handle response from hash string.
-        return await silentHandler.handleCodeResponse(hash);
+        return silentHandler.handleCodeResponse(hash);
     }
 
     // #endregion

diff --git a/lib/msal-common/test/client/DeviceCodeClient.spec.ts b/lib/msal-common/test/client/DeviceCodeClient.spec.ts
@@ -7,6 +7,7 @@ import {
     DeviceCodeClient,
     DeviceCodeRequest,
     IdToken,
+    ClientConfiguration,
 } from "../../src";
 import {
     AUTHENTICATION_RESULT, AUTHORIZATION_PENDING_RESPONSE,
@@ -22,34 +23,16 @@ import { AADServerParamKeys, GrantType } from "../../src/utils/Constants";
 import { ClientTestUtils } from "./ClientTestUtils";
 
 describe("DeviceCodeClient unit tests", async () => {
+    let config: ClientConfiguration;
 
     before(() => {
         sinon.restore();
     });
 
-    beforeEach(() => {
+    beforeEach(async () => {
         ClientTestUtils.setCloudDiscoveryMetadataStubs();
-    });
-
-    afterEach(() => {
-        sinon.restore();
-    });
-
-    describe("Constructor", () => {
-
-        it("creates a DeviceCodeClient", async () => {
-            sinon.stub(Authority.prototype, <any>"discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE);
-            const config = await ClientTestUtils.createTestClientConfiguration();
-            const client = new DeviceCodeClient(config);
-            expect(client).to.be.not.null;
-            expect(client instanceof DeviceCodeClient).to.be.true;
-            expect(client instanceof BaseClient).to.be.true;
-        });
-    });
-
-    describe("Acquire a token", async () => {
-
-        const config = await ClientTestUtils.createTestClientConfiguration();
+        sinon.stub(Authority.prototype, <any>"discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE);
+        config = await ClientTestUtils.createTestClientConfiguration();
         // Set up required objects and mocked return values
         const testState = `eyAiaWQiOiAidGVzdGlkIiwgInRzIjogMTU5Mjg0NjQ4MiB9${Constants.RESOURCE_DELIM}userState`;
         const decodedLibState = `{ "id": "testid", "ts": 1592846482 }`;
@@ -90,24 +73,37 @@ describe("DeviceCodeClient unit tests", async () => {
             nonce: "123523",
         };
         sinon.stub(IdToken, "extractIdToken").returns(idTokenClaims);
+    });
 
-        it("Acquires a token successfully", async () => {
+    afterEach(() => {
+        sinon.restore();
+    });
+
+    describe("Constructor", () => {
+
+        it("creates a DeviceCodeClient", async () => {
+            const client = new DeviceCodeClient(config);
+            expect(client).to.be.not.null;
+            expect(client instanceof DeviceCodeClient).to.be.true;
+            expect(client instanceof BaseClient).to.be.true;
+        });
+    });
+
+    describe("Acquire a token", async () => {
 
+        it("Acquires a token successfully", async () => {
             sinon.stub(DeviceCodeClient.prototype, <any>"executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_RESPONSE);
-            sinon.stub(Authority.prototype, <any>"discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE);
             sinon.stub(BaseClient.prototype, <any>"executePostToTokenEndpoint").resolves(AUTHENTICATION_RESULT);
 
             const queryStringSpy = sinon.spy(DeviceCodeClient.prototype, <any>"createQueryString");
             const createTokenRequestBodySpy = sinon.spy(DeviceCodeClient.prototype, <any>"createTokenRequestBody");
 
-
             let deviceCodeResponse = null;
             const request: DeviceCodeRequest = {
                 scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE, ...TEST_CONFIG.DEFAULT_SCOPES],
                 deviceCodeCallback: (response) => deviceCodeResponse = response
             };
 
-
             const client = new DeviceCodeClient(config);
             const authenticationResult = await client.acquireToken(request);
 
@@ -127,9 +123,7 @@ describe("DeviceCodeClient unit tests", async () => {
         }).timeout(6000);
 
         it("Acquires a token successfully after authorization_pending error", async () => {
-
             sinon.stub(DeviceCodeClient.prototype, <any>"executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_RESPONSE);
-            sinon.stub(Authority.prototype, <any>"discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE);
             const tokenRequestStub = sinon.stub(BaseClient.prototype, <any>"executePostToTokenEndpoint");
 
             tokenRequestStub.onFirstCall().resolves(AUTHORIZATION_PENDING_RESPONSE);
@@ -141,7 +135,6 @@ describe("DeviceCodeClient unit tests", async () => {
                 deviceCodeCallback: (response) => deviceCodeResponse = response
             };
 
-            const config = await ClientTestUtils.createTestClientConfiguration();
             const client = new DeviceCodeClient(config);
             const authenticationResult = await client.acquireToken(request);
         }).timeout(12000);
@@ -150,8 +143,6 @@ describe("DeviceCodeClient unit tests", async () => {
     describe("Device code exceptions", () => {
 
         it("Throw device code flow cancelled exception if DeviceCodeRequest.cancel=true", async () => {
-
-            sinon.stub(Authority.prototype, <any>"discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE);
             sinon.stub(DeviceCodeClient.prototype, <any>"executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_RESPONSE);
             sinon.stub(BaseClient.prototype, <any>"executePostToTokenEndpoint").resolves(AUTHENTICATION_RESULT);
 
@@ -161,14 +152,12 @@ describe("DeviceCodeClient unit tests", async () => {
                 deviceCodeCallback: (response) => deviceCodeResponse = response,
             };
 
-            const config = await ClientTestUtils.createTestClientConfiguration();
             const client = new DeviceCodeClient(config);
             request.cancel = true;
             await expect(client.acquireToken(request)).to.be.rejectedWith(`${ClientAuthErrorMessage.DeviceCodePollingCancelled.desc}`);
         }).timeout(6000);
 
         it("Throw device code expired exception if device code is expired", async () => {
-            sinon.stub(Authority.prototype, <any>"discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE);
             sinon.stub(DeviceCodeClient.prototype, <any>"executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_EXPIRED_RESPONSE);
 
             let deviceCodeResponse = null;
@@ -177,7 +166,6 @@ describe("DeviceCodeClient unit tests", async () => {
                 deviceCodeCallback: (response) => deviceCodeResponse = response,
             };
 
-            const config = await ClientTestUtils.createTestClientConfiguration();
             const client = new DeviceCodeClient(config);
             await expect(client.acquireToken(request)).to.be.rejectedWith(`${ClientAuthErrorMessage.DeviceCodeExpired.desc}`);
         }).timeout(6000);

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/default/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/default/auth.js
@@ -27,7 +27,7 @@ function handleResponse(resp) {
     } else {
         // need to call getAccount here?
         const currentAccounts = myMSALObj.getAllAccounts();
-        if (currentAccounts === null) {
+        if (!currentAccounts || currentAccounts.length < 1) {
             return;
         } else if (currentAccounts.length > 1) {
             // Add choose account code here

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/ie11-sample/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/ie11-sample/auth.js
@@ -1,13 +1,3 @@
-// Browser check variables
-// If you support IE, our recommendation is that you sign-in using Redirect APIs
-// If you as a developer are testing using Edge InPrivate mode, please add "isEdge" to the if check
-const ua = window.navigator.userAgent;
-const msie = ua.indexOf("MSIE ");
-const msie11 = ua.indexOf("Trident/");
-const msedge = ua.indexOf("Edge/");
-const isIE = msie > 0 || msie11 > 0;
-const isEdge = msedge > 0;
-
 let username = "";
 
 // Create the main myMSALObj instance
@@ -31,7 +21,7 @@ function handleResponse(resp) {
     } else {
         // need to call getAccount here?
         const currentAccounts = myMSALObj.getAllAccounts();
-        if (currentAccounts === null) {
+        if (!currentAccounts || currentAccounts.length < 1) {
             return;
         } else if (currentAccounts.length > 1) {
             // Add choose account code here

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/ie11-sample/authConfig.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/ie11-sample/authConfig.js
@@ -1,3 +1,13 @@
+// Browser check variables
+// If you support IE, our recommendation is that you sign-in using Redirect APIs
+// If you as a developer are testing using Edge InPrivate mode, please add "isEdge" to the if check
+const ua = window.navigator.userAgent;
+const msie = ua.indexOf("MSIE ");
+const msie11 = ua.indexOf("Trident/");
+const msedge = ua.indexOf("Edge/");
+const isIE = msie > 0 || msie11 > 0;
+const isEdge = msedge > 0;
+
 // Config object to be passed to Msal on creation
 const msalConfig = {
     auth: {

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/multipleResources/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/multipleResources/auth.js
@@ -28,7 +28,7 @@ function handleResponse(resp) {
     } else {
         // need to call getAccount here?
         const currentAccounts = myMSALObj.getAllAccounts();
-        if (currentAccounts === null) {
+        if (!currentAccounts || currentAccounts.length < 1) {
             return;
         } else if (currentAccounts.length > 1) {
             // Add choose account code here

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/navigateToLoginPage/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/navigateToLoginPage/auth.js
@@ -27,7 +27,7 @@ function handleResponse(resp) {
     } else {
         // need to call getAccount here?
         const currentAccounts = myMSALObj.getAllAccounts();
-        if (currentAccounts === null) {
+        if (!currentAccounts || currentAccounts.length < 1) {
             return;
         } else if (currentAccounts.length > 1) {
             // Add choose account code here

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/onPageLoad/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/onPageLoad/auth.js
@@ -28,7 +28,7 @@ function handleResponse(resp) {
     } else {
         // need to call getAccount here?
         const currentAccounts = myMSALObj.getAllAccounts();
-        if (currentAccounts === null) {
+        if (!currentAccounts || currentAccounts.length < 1) {
             signIn("loginRedirect");
         } else if (currentAccounts.length > 1) {
             // Add choose account code here

diff --git a/samples/msal-browser-samples/VanillaJSTestApp2.0/app/ssoSilent/auth.js b/samples/msal-browser-samples/VanillaJSTestApp2.0/app/ssoSilent/auth.js
@@ -28,7 +28,7 @@ function handleResponse(resp) {
     } else {
         // need to call getAccount here?
         const currentAccounts = myMSALObj.getAllAccounts();
-        if (currentAccounts === null) {
+        if (!currentAccounts || currentAccounts.length < 1) {
             myMSALObj.ssoSilent(silentRequest).then(handleResponse).catch(error => {
                 console.error("Silent Error: " + error);
                 if (error instanceof msal.InteractionRequiredAuthError) {