Skip to content

Commit

Permalink
MSAL JS should not throw user account switch error when home account …
Browse files Browse the repository at this point in the history
…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 <[email protected]>
  • Loading branch information
lalimasharda and tnorling authored Feb 13, 2024
1 parent d51b97d commit c6b5f3d
Show file tree
Hide file tree
Showing 3 changed files with 83 additions and 9 deletions.
Original file line number Diff line number Diff line change
@@ -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": "[email protected]",
"dependentChangeType": "patch"
}
33 changes: 25 additions & 8 deletions lib/msal-browser/src/interaction_client/NativeInteractionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand All @@ -399,6 +399,56 @@ describe("NativeInteractionClient Tests", () => {
properties: {},
};

jest.spyOn(
CacheManager.prototype,
"getAccountInfoFilteredBy"
).mockReturnValue(TEST_ACCOUNT_INFO);

sinon
.stub(NativeMessageHandler.prototype, "sendMessage")
.callsFake((): Promise<object> => {
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<object> => {
Expand Down

0 comments on commit c6b5f3d

Please sign in to comment.