Skip to content

Commit

Permalink
Remove prompt from redirect to prevent WAM from double prompting (#5239)
Browse files Browse the repository at this point in the history
* Remove prompt from redirect to prevent WAM from double prompting

* add test

* Change files

Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
Co-authored-by: Thomas Norling <thomas.l.norling@gmail.com>
  • Loading branch information
3 people committed Oct 13, 2022
1 parent 124f04e commit 8e3282f
Show file tree
Hide file tree
Showing 3 changed files with 74 additions and 2 deletions.
Original file line number Diff line number Diff line change
@@ -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"
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand All @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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>(): T {
Expand Down Expand Up @@ -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<object> => {
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,
Expand Down

0 comments on commit 8e3282f

Please sign in to comment.