diff --git a/lib/msal-browser/src/app/PublicClientApplication.ts b/lib/msal-browser/src/app/PublicClientApplication.ts index f976ec360b5..43dd5ea3090 100644 --- a/lib/msal-browser/src/app/PublicClientApplication.ts +++ b/lib/msal-browser/src/app/PublicClientApplication.ts @@ -321,8 +321,17 @@ 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(); @@ -330,21 +339,6 @@ export class PublicClientApplication implements IPublicClientApplication { } } - /** - * 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 { - // 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 index ce6ed727143..890dfe5ca6b 100644 --- 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, "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, "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, "executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_RESPONSE); - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); sinon.stub(BaseClient.prototype, "executePostToTokenEndpoint").resolves(AUTHENTICATION_RESULT); const queryStringSpy = sinon.spy(DeviceCodeClient.prototype, "createQueryString"); const createTokenRequestBodySpy = sinon.spy(DeviceCodeClient.prototype, "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, "executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_RESPONSE); - sinon.stub(Authority.prototype, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); const tokenRequestStub = sinon.stub(BaseClient.prototype, "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, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); sinon.stub(DeviceCodeClient.prototype, "executePostRequestToDeviceCodeEndpoint").resolves(DEVICE_CODE_RESPONSE); sinon.stub(BaseClient.prototype, "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, "discoverEndpoints").resolves(DEFAULT_OPENID_CONFIG_RESPONSE); sinon.stub(DeviceCodeClient.prototype, "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 index aa626b630c5..d0466776d3f 100644 --- 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 index cfdf6db5aba..49fca4b38e9 100644 --- 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 index abcde4496ea..6c2d69a4184 100644 --- 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 index 76b6d665022..9489b57c954 100644 --- 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 index aa626b630c5..d0466776d3f 100644 --- 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 index f7dff9c803e..9452638ca78 100644 --- 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 index a08d801568c..019938f2009 100644 --- 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) {