From c6b5f3d474cdad9b88b415b4ec552d945d9b6062 Mon Sep 17 00:00:00 2001 From: Lalima Sharda Date: Mon, 12 Feb 2024 20:58:28 -0800 Subject: [PATCH] MSAL JS should not throw user account switch error when home account ids of the request/response match (#6846) This PR fixes 2 issues: - When the parent app is requesting tokens on behalf of an iframed application, it should check request/response based on the native account id as well as the homeAccountId. - When the tokens are brokered, the parent's native account id should not be cached in the iframed application. The iframed application should only store it's own native account id. --------- Co-authored-by: Thomas Norling --- ...-827cdb7e-b87e-4aea-a9a2-139125b1528b.json | 7 +++ .../NativeInteractionClient.ts | 33 +++++++++--- .../NativeInteractionClient.spec.ts | 52 ++++++++++++++++++- 3 files changed, 83 insertions(+), 9 deletions(-) create mode 100644 change/@azure-msal-browser-827cdb7e-b87e-4aea-a9a2-139125b1528b.json diff --git a/change/@azure-msal-browser-827cdb7e-b87e-4aea-a9a2-139125b1528b.json b/change/@azure-msal-browser-827cdb7e-b87e-4aea-a9a2-139125b1528b.json new file mode 100644 index 0000000000..431c2953a8 --- /dev/null +++ b/change/@azure-msal-browser-827cdb7e-b87e-4aea-a9a2-139125b1528b.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Bug fix for user switch error and broker app's native account id being stored in embedded app cache #6846", + "packageName": "@azure/msal-browser", + "email": "lalimasharda@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts b/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts index 7ffd327a17..076c56562a 100644 --- a/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts @@ -416,24 +416,33 @@ export class NativeInteractionClient extends BaseInteractionClient { "NativeInteractionClient - handleNativeResponse called." ); - if (response.account.id !== request.accountId) { - // User switch in native broker prompt is not supported. All users must first sign in through web flow to ensure server state is in sync - throw createNativeAuthError(NativeAuthErrorCodes.userSwitch); - } - - // Get the preferred_cache domain for the given authority - const authority = await this.getDiscoveredAuthority(request.authority); - // generate identifiers const idTokenClaims = AuthToken.extractTokenClaims( response.id_token, base64Decode ); + const homeAccountIdentifier = this.createHomeAccountIdentifier( response, idTokenClaims ); + const cachedhomeAccountId = + this.browserStorage.getAccountInfoFilteredBy({ + nativeAccountId: request.accountId, + })?.homeAccountId; + + if ( + homeAccountIdentifier !== cachedhomeAccountId && + response.account.id !== request.accountId + ) { + // User switch in native broker prompt is not supported. All users must first sign in through web flow to ensure server state is in sync + throw createNativeAuthError(NativeAuthErrorCodes.userSwitch); + } + + // Get the preferred_cache domain for the given authority + const authority = await this.getDiscoveredAuthority(request.authority); + const baseAccount = buildAccountToCache( this.browserStorage, authority, @@ -604,6 +613,14 @@ export class NativeInteractionClient extends BaseInteractionClient { idTokenClaims ); + /** + * In pairwise broker flows, this check prevents the broker's native account id + * from being returned over the embedded app's account id. + */ + if (accountInfo.nativeAccountId !== response.account.id) { + accountInfo.nativeAccountId = response.account.id; + } + // generate PoP token as needed const responseAccessToken = await this.generatePopAccessToken( response, diff --git a/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts index 3bde562342..3459a9dce8 100644 --- a/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts @@ -386,7 +386,7 @@ describe("NativeInteractionClient Tests", () => { expect(response.tokenType).toEqual(AuthenticationScheme.BEARER); }); - it("throws on account switch", (done) => { + it("does not throw account switch error when homeaccountid is same", (done) => { const mockWamResponse = { access_token: TEST_TOKENS.ACCESS_TOKEN, id_token: TEST_TOKENS.IDTOKEN_V2, @@ -399,6 +399,56 @@ describe("NativeInteractionClient Tests", () => { properties: {}, }; + jest.spyOn( + CacheManager.prototype, + "getAccountInfoFilteredBy" + ).mockReturnValue(TEST_ACCOUNT_INFO); + + sinon + .stub(NativeMessageHandler.prototype, "sendMessage") + .callsFake((): Promise => { + return Promise.resolve(mockWamResponse); + }); + nativeInteractionClient + .acquireToken({ + scopes: ["User.Read"], + }) + .catch((e) => { + console.error( + "User switch error should not have been thrown." + ); + expect(e.errorCode).not.toBe( + NativeAuthErrorCodes.userSwitch + ); + expect(e.errorMessage).not.toBe( + NativeAuthErrorMessages[NativeAuthErrorCodes.userSwitch] + ); + done(); + }); + done(); + }); + + it("throws error on user switch", (done) => { + const raw_client_info = + "eyJ1aWQiOiAiMDAwMDAwMDAtMDAwMC0wMDAwLTAwMDAtMDAwMDAwMDAwMDAwIiwgInV0aWQiOiI3MmY5ODhiZi04NmYxLTQxYWYtOTFhYi0yZDdjZDAxMWRiNDcifQ=="; + + const mockWamResponse = { + access_token: TEST_TOKENS.ACCESS_TOKEN, + id_token: TEST_TOKENS.IDTOKEN_V2_ALT, + scope: "User.Read", + expires_in: 3600, + client_info: raw_client_info, + account: { + id: "different-nativeAccountId", + }, + properties: {}, + }; + + jest.spyOn( + CacheManager.prototype, + "getAccountInfoFilteredBy" + ).mockReturnValue(TEST_ACCOUNT_INFO); + sinon .stub(NativeMessageHandler.prototype, "sendMessage") .callsFake((): Promise => {