From 243b68bcb1d9fb90adbfcfe4c17773a32881ed3c Mon Sep 17 00:00:00 2001 From: shylasummers Date: Mon, 7 Oct 2024 13:45:26 -0700 Subject: [PATCH] Fixing window unavailable bugs for PCA and PBA (#7355) Currently, the PCA constructor in 1P and some functions for PCA (both 1P and 3P) and PBA throw when the window is unavailable. This creates problems with server-side rendering. Additionally, some functions that should throw a non_browser_environment error when the window is unavailable (such as the acquireToken functions) throw a different error. This PR makes changes to prevent the PCA and PBA constructor and certain functions from throwing solely due to the window being unavailable and to ensure that the non_browser_environment error is thrown when appropriate. It also adds corresponding unit tests. --------- Co-authored-by: Hector Morales --- ...-1aaf2555-690a-4057-bfad-d93a65f8e0ca.json | 7 + .../src/controllers/StandardController.ts | 13 +- .../test/app/PCANonBrowser.spec.ts | 349 ++++++++++++++++-- 3 files changed, 342 insertions(+), 27 deletions(-) create mode 100644 change/@azure-msal-browser-1aaf2555-690a-4057-bfad-d93a65f8e0ca.json diff --git a/change/@azure-msal-browser-1aaf2555-690a-4057-bfad-d93a65f8e0ca.json b/change/@azure-msal-browser-1aaf2555-690a-4057-bfad-d93a65f8e0ca.json new file mode 100644 index 0000000000..9b34e6ff05 --- /dev/null +++ b/change/@azure-msal-browser-1aaf2555-690a-4057-bfad-d93a65f8e0ca.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Bug fixes for windowless PCA", + "packageName": "@azure/msal-browser", + "email": "shylasummers@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts index bb49ab17bf..75923f8d54 100644 --- a/lib/msal-browser/src/controllers/StandardController.ts +++ b/lib/msal-browser/src/controllers/StandardController.ts @@ -314,6 +314,13 @@ export class StandardController implements IController { return; } + if (!this.isBrowserEnvironment) { + this.logger.info("in non-browser environment, exiting early."); + this.initialized = true; + this.eventHandler.emitEvent(EventType.INITIALIZE_END); + return; + } + const initCorrelationId = request?.correlationId || this.getRequestCorrelationId(); const allowNativeBroker = this.config.system.allowNativeBroker; @@ -354,7 +361,6 @@ export class StandardController implements IController { this.initialized = true; this.eventHandler.emitEvent(EventType.INITIALIZE_END); - initMeasurement.end({ allowNativeBroker, success: true }); } @@ -1336,6 +1342,10 @@ export class StandardController implements IController { * @param logoutRequest */ async clearCache(logoutRequest?: ClearCacheRequest): Promise { + if (!this.isBrowserEnvironment) { + this.logger.info("in non-browser environment, returning early."); + return; + } const correlationId = this.getRequestCorrelationId(logoutRequest); const cacheClient = this.createSilentCacheClient(correlationId); return cacheClient.logout(logoutRequest); @@ -1716,6 +1726,7 @@ export class StandardController implements IController { * @returns {string} */ addPerformanceCallback(callback: PerformanceCallbackFunction): string { + BrowserUtils.blockNonBrowserEnvironment(); return this.performanceClient.addPerformanceCallback(callback); } diff --git a/lib/msal-browser/test/app/PCANonBrowser.spec.ts b/lib/msal-browser/test/app/PCANonBrowser.spec.ts index f09a6bdc93..c511faf6e7 100644 --- a/lib/msal-browser/test/app/PCANonBrowser.spec.ts +++ b/lib/msal-browser/test/app/PCANonBrowser.spec.ts @@ -4,6 +4,20 @@ import { TEST_CONFIG } from "../utils/StringConstants"; import { PublicClientApplication } from "../../src/app/PublicClientApplication"; import { BrowserAuthErrorMessage } from "../../src/error/BrowserAuthError"; +import { AccountInfo, AuthenticationScheme, Logger } from "@azure/msal-common"; +import { + ID_TOKEN_CLAIMS, + RANDOM_TEST_GUID, + TEST_DATA_CLIENT_INFO, + TEST_TOKENS, +} from "../utils/StringConstants.js"; +import { + CacheLookupPolicy, + WrapperSKU, +} from "../../src/utils/BrowserConstants.js"; +import { NavigationClient } from "../../src/navigation/NavigationClient.js"; +import { SilentRequest } from "../../src/request/SilentRequest.js"; +import { AuthenticationResult } from "../../src/response/AuthenticationResult.js"; /** * Tests for PublicClientApplication.ts when run in a non-browser environment @@ -25,6 +39,49 @@ describe("Non-browser environment", () => { }); }); + it("initialize doesn't throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + await instance.initialize(); + }); + + it("acquireTokenPopup throws", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + try { + await instance.acquireTokenPopup({ scopes: ["openid"] }); + } catch (error: any) { + expect(error.errorCode).toEqual( + BrowserAuthErrorMessage.notInBrowserEnvironment.code + ); + } + }); + + it("acquireTokenRedirect throws", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + try { + await instance.acquireTokenRedirect({ scopes: ["openid"] }); + } catch (error: any) { + expect(error.errorCode).toEqual( + BrowserAuthErrorMessage.notInBrowserEnvironment.code + ); + } + }); + it("acquireTokenSilent throws", async () => { const instance = new PublicClientApplication({ auth: { @@ -45,7 +102,7 @@ describe("Non-browser environment", () => { } }); - it("ssoSilent throws", async () => { + it("acquireTokenByCode throws", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, @@ -54,7 +111,10 @@ describe("Non-browser environment", () => { await instance.initialize(); try { - await instance.ssoSilent({}); + await instance.acquireTokenByCode({ + scopes: ["openid"], + code: "auth-code", + }); } catch (error: any) { expect(error.errorCode).toEqual( BrowserAuthErrorMessage.notInBrowserEnvironment.code @@ -62,7 +122,29 @@ describe("Non-browser environment", () => { } }); - it("acquireTokenPopup throws", async () => { + it("addEventCallback does not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + expect(() => instance.addEventCallback(() => {})).not.toThrow(); + }); + + it("removeEventCallback does not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.removeEventCallback(""); + }); + + it("addPerformanceCallback throws", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, @@ -71,7 +153,7 @@ describe("Non-browser environment", () => { await instance.initialize(); try { - await instance.acquireTokenPopup({ scopes: ["openid"] }); + instance.addPerformanceCallback(() => {}); } catch (error: any) { expect(error.errorCode).toEqual( BrowserAuthErrorMessage.notInBrowserEnvironment.code @@ -79,7 +161,124 @@ describe("Non-browser environment", () => { } }); - it("acquireTokenRedirect throws", async () => { + it("removePerformanceCallback does not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.removePerformanceCallback(""); + }); + + it("enableAccountStorageEvents does not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.enableAccountStorageEvents(); + }); + + it("disableAccountStorageEvents does not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.disableAccountStorageEvents(); + }); + + it("getAccount returns null", async () => { + const testAccount: AccountInfo = { + homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID, + localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID, + environment: "login.windows.net", + tenantId: "3338040d-6c67-4c5b-b112-36a304b66dad", + username: "AbeLi@microsoft.com", + }; + + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + await instance.initialize(); + const account = instance.getAccount(testAccount); + expect(account).toEqual(null); + }); + + it("getAccountByHomeId returns null", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + await instance.initialize(); + const account = instance.getAccountByHomeId( + TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID + ); + expect(account).toEqual(null); + }); + + it("getAccountByLocalId returns null", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + await instance.initialize(); + const account = instance.getAccountByLocalId( + TEST_DATA_CLIENT_INFO.TEST_UID + ); + expect(account).toEqual(null); + }); + + it("getAccountByUsername returns null", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + await instance.initialize(); + const account = instance.getAccountByUsername("example@test.com"); + expect(account).toEqual(null); + }); + + it("getAllAccounts returns empty array", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + await instance.initialize(); + const accounts = instance.getAllAccounts(); + expect(accounts).toEqual([]); + }); + + it("handleRedirectPromise returns null", (done) => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + system: { + allowNativeBroker: false, + }, + }); + instance.initialize().then(() => { + instance.handleRedirectPromise().then((result: any) => { + expect(result).toBeNull(); + done(); + }); + }); + }); + + it("loginPopup throws", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, @@ -88,7 +287,7 @@ describe("Non-browser environment", () => { await instance.initialize(); try { - await instance.acquireTokenRedirect({ scopes: ["openid"] }); + await instance.loginPopup({ scopes: ["openid"] }); } catch (error: any) { expect(error.errorCode).toEqual( BrowserAuthErrorMessage.notInBrowserEnvironment.code @@ -96,7 +295,7 @@ describe("Non-browser environment", () => { } }); - it("loginPopup throws", async () => { + it("loginRedirect throws", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, @@ -105,7 +304,7 @@ describe("Non-browser environment", () => { await instance.initialize(); try { - await instance.loginPopup({ scopes: ["openid"] }); + await instance.loginRedirect({ scopes: ["openid"] }); } catch (error: any) { expect(error.errorCode).toEqual( BrowserAuthErrorMessage.notInBrowserEnvironment.code @@ -113,7 +312,7 @@ describe("Non-browser environment", () => { } }); - it("loginRedirect throws", async () => { + it("logout throws", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, @@ -122,7 +321,7 @@ describe("Non-browser environment", () => { await instance.initialize(); try { - await instance.loginRedirect({ scopes: ["openid"] }); + await instance.logout(); } catch (error: any) { expect(error.errorCode).toEqual( BrowserAuthErrorMessage.notInBrowserEnvironment.code @@ -163,46 +362,71 @@ describe("Non-browser environment", () => { } }); - it("getAllAccounts returns empty array", async () => { + it("ssoSilent throws", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, }, }); + await instance.initialize(); - const accounts = instance.getAllAccounts(); - expect(accounts).toEqual([]); + try { + await instance.ssoSilent({}); + } catch (error: any) { + expect(error.errorCode).toEqual( + BrowserAuthErrorMessage.notInBrowserEnvironment.code + ); + } }); - it("getAccountByUsername returns null", async () => { + it("getTokenCache returns an ITokenCache", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, }, }); + await instance.initialize(); - const account = instance.getAccountByUsername("example@test.com"); - expect(account).toEqual(null); + const tokenCache = instance.getTokenCache(); + expect(typeof tokenCache.loadExternalTokens).toBe("function"); }); - it("handleRedirectPromise returns null", (done) => { + it("getLogger should not throw", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, }, - system: { - allowNativeBroker: false, + }); + + await instance.initialize(); + await instance.getLogger(); + }); + + it("setLogger should not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, }, }); - instance.initialize().then(() => { - instance.handleRedirectPromise().then((result) => { - expect(result).toBeNull(); - done(); - }); + + await instance.initialize(); + + const logger = new Logger({}); + instance.setLogger(logger); + }); + + it("setActiveAccount should not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, }); + + await instance.initialize(); + instance.setActiveAccount(null); }); - it("addEventCallback does not throw", async () => { + it("getActiveAccount should return null", async () => { const instance = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, @@ -210,6 +434,79 @@ describe("Non-browser environment", () => { }); await instance.initialize(); - expect(() => instance.addEventCallback(() => {})).not.toThrow(); + const account = instance.getActiveAccount(); + expect(account).toBeNull(); + }); + + it("initializeWrapperLibrary should not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.initializeWrapperLibrary(WrapperSKU.React, "1.0.0"); + }); + + it("setNavigationClient should not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + const navigationClient = new NavigationClient(); + instance.setNavigationClient(navigationClient); + }); + + it("hydrateCache should not throw", async () => { + const testAccount: AccountInfo = { + homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID, + localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID, + environment: "login.windows.net", + tenantId: "3338040d-6c67-4c5b-b112-36a304b66dad", + username: "AbeLi@microsoft.com", + }; + const testAuthenticationResult: AuthenticationResult = { + authority: TEST_CONFIG.validAuthority, + uniqueId: testAccount.localAccountId, + tenantId: testAccount.tenantId, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + idToken: TEST_TOKENS.IDTOKEN_V2, + idTokenClaims: ID_TOKEN_CLAIMS, + accessToken: TEST_TOKENS.ACCESS_TOKEN, + fromCache: false, + correlationId: RANDOM_TEST_GUID, + expiresOn: new Date(Date.now() + 3600000), + account: testAccount, + tokenType: AuthenticationScheme.BEARER, + }; + const request: SilentRequest = { + scopes: ["openid", "profile"], + account: testAccount, + cacheLookupPolicy: CacheLookupPolicy.AccessToken, + }; + + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.hydrateCache(testAuthenticationResult, request); + }); + + it("clearCache should not throw", async () => { + const instance = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + }, + }); + + await instance.initialize(); + instance.clearCache(); }); });