From 8e3282f6ddfe949a08e33424c3b05b9d219ab715 Mon Sep 17 00:00:00 2001 From: Sameera Gajjarapu Date: Thu, 13 Oct 2022 16:53:42 -0400 Subject: [PATCH] Remove prompt from redirect to prevent WAM from double prompting (#5239) * Remove prompt from redirect to prevent WAM from double prompting * add test * Change files Co-authored-by: Thomas Norling Co-authored-by: Thomas Norling --- ...-d04a0bb7-1d77-4edb-a0e0-c9b816964438.json | 7 +++ .../NativeInteractionClient.ts | 10 +++- .../NativeInteractionClient.spec.ts | 59 +++++++++++++++++++ 3 files changed, 74 insertions(+), 2 deletions(-) create mode 100644 change/@azure-msal-browser-d04a0bb7-1d77-4edb-a0e0-c9b816964438.json diff --git a/change/@azure-msal-browser-d04a0bb7-1d77-4edb-a0e0-c9b816964438.json b/change/@azure-msal-browser-d04a0bb7-1d77-4edb-a0e0-c9b816964438.json new file mode 100644 index 0000000000..258114f292 --- /dev/null +++ b/change/@azure-msal-browser-d04a0bb7-1d77-4edb-a0e0-c9b816964438.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fix double prompt in native broker redirect flow #5239", + "packageName": "@azure/msal-browser", + "email": "thomas.norling@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 2d9292ceb5..7e921b5228 100644 --- a/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts @@ -179,17 +179,23 @@ export class NativeInteractionClient extends BaseInteractionClient { return null; } + // remove prompt from the request to prevent WAM from prompting twice const cachedRequest = this.browserStorage.getCachedNativeRequest(); if (!cachedRequest) { this.logger.verbose("NativeInteractionClient - handleRedirectPromise called but there is no cached request, returning null."); return null; } + const { prompt, ...request} = cachedRequest; + if (prompt) { + this.logger.verbose("NativeInteractionClient - handleRedirectPromise called and prompt was included in the original request, removing prompt from cached request to prevent second interaction with native broker window."); + } + this.browserStorage.removeItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.NATIVE_REQUEST)); const messageBody: NativeExtensionRequestBody = { method: NativeExtensionMethod.GetToken, - request: cachedRequest + request: request }; const reqTimestamp = TimeUtils.nowSeconds(); @@ -198,7 +204,7 @@ export class NativeInteractionClient extends BaseInteractionClient { this.logger.verbose("NativeInteractionClient - handleRedirectPromise sending message to native broker."); const response: object = await this.nativeMessageHandler.sendMessage(messageBody); this.validateNativeResponse(response); - const result = this.handleNativeResponse(response as NativeResponse, cachedRequest, reqTimestamp); + const result = this.handleNativeResponse(response as NativeResponse, request, reqTimestamp); this.browserStorage.setInteractionInProgress(false); return result; } catch (e) { diff --git a/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts index 3cc3aff8d9..a963c78f97 100644 --- a/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts @@ -14,6 +14,7 @@ import { NavigationClient } from "../../src/navigation/NavigationClient"; import { BrowserAuthErrorMessage } from "../../src/error/BrowserAuthError"; import { NativeAuthError, NativeAuthErrorMessage } from "../../src/error/NativeAuthError"; import { SilentCacheClient } from "../../src/interaction_client/SilentCacheClient"; +import { NativeExtensionRequestBody } from "../../src/broker/nativeBroker/NativeRequest"; const networkInterface = { sendGetRequestAsync(): T { @@ -514,6 +515,64 @@ describe("NativeInteractionClient Tests", () => { expect(response).toEqual(testTokenResponse); }); + it("If request includes a prompt value it is ignored on the 2nd call to native broker", async () => { + // The user should not be prompted twice, prompt value should only be used on the first call to the native broker (before returning to the redirect uri). Native broker calls from handleRedirectPromise should ignore the prompt. + const mockWamResponse = { + access_token: TEST_TOKENS.ACCESS_TOKEN, + id_token: TEST_TOKENS.IDTOKEN_V2, + scope: "User.Read", + expires_in: 3600, + client_info: TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO, + account: { + id: "nativeAccountId" + }, + properties: {} + }; + + const testAccount: AccountInfo = { + homeAccountId: `${TEST_DATA_CLIENT_INFO.TEST_UID}.${TEST_DATA_CLIENT_INFO.TEST_UTID}`, + localAccountId: ID_TOKEN_CLAIMS.oid, + environment: "login.windows.net", + tenantId: ID_TOKEN_CLAIMS.tid, + username: ID_TOKEN_CLAIMS.preferred_username, + name: ID_TOKEN_CLAIMS.name, + idTokenClaims: ID_TOKEN_CLAIMS, + nativeAccountId: mockWamResponse.account.id + }; + + sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((url: string) => { + expect(url).toBe(window.location.href); + return Promise.resolve(true); + }); + sinon.stub(NativeMessageHandler.prototype, "sendMessage").callsFake((messageBody: NativeExtensionRequestBody): Promise => { + expect(messageBody.request && messageBody.request.prompt).toBe(undefined); + return Promise.resolve(mockWamResponse); + }); + // @ts-ignore + pca.browserStorage.setInteractionInProgress(true); + await nativeInteractionClient.acquireTokenRedirect({scopes: ["User.Read"], prompt: "login"}); + const response = await nativeInteractionClient.handleRedirectPromise(); + expect(response).not.toBe(null); + + const testTokenResponse: AuthenticationResult = { + authority: TEST_CONFIG.validAuthority, + uniqueId: testAccount.localAccountId, + tenantId: testAccount.tenantId, + scopes: mockWamResponse.scope.split(" "), + idToken: mockWamResponse.id_token, + idTokenClaims: ID_TOKEN_CLAIMS, + accessToken: mockWamResponse.access_token, + fromCache: false, + state: undefined, + correlationId: RANDOM_TEST_GUID, + expiresOn: response && response.expiresOn, // Steal the expires on from the response as this is variable + account: testAccount, + tokenType: AuthenticationScheme.BEARER, + fromNativeBroker: true + }; + expect(response).toEqual(testTokenResponse); + }); + it("clears interaction in progress if native broker call fails", (done) => { const mockWamResponse = { access_token: TEST_TOKENS.ACCESS_TOKEN,