diff --git a/change/@azure-msal-browser-95758454-20db-46fa-bff7-0fa737d064dc.json b/change/@azure-msal-browser-95758454-20db-46fa-bff7-0fa737d064dc.json new file mode 100644 index 0000000000..737697b38f --- /dev/null +++ b/change/@azure-msal-browser-95758454-20db-46fa-bff7-0fa737d064dc.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Throw timeout error in acquireTokenRedirect if the redirect doesn't happen", + "packageName": "@azure/msal-browser", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/apiReview/msal-browser.api.md b/lib/msal-browser/apiReview/msal-browser.api.md index 02363a4de3..4f44cf9b13 100644 --- a/lib/msal-browser/apiReview/msal-browser.api.md +++ b/lib/msal-browser/apiReview/msal-browser.api.md @@ -242,7 +242,8 @@ declare namespace BrowserAuthErrorCodes { invalidPopTokenRequest, failedToBuildHeaders, failedToParseHeaders, - failedToDecryptEarResponse + failedToDecryptEarResponse, + timedOut } } export { BrowserAuthErrorCodes } @@ -1738,6 +1739,11 @@ export { StubPerformanceClient } export { TenantProfile } +// Warning: (ae-missing-release-tag) "timedOut" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +const timedOut = "timed_out"; + // Warning: (ae-missing-release-tag) "unableToAcquireTokenFromNativePlatform" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public (undocumented) @@ -1808,7 +1814,7 @@ export type WrapperSKU = (typeof WrapperSKU)[keyof typeof WrapperSKU]; // src/event/EventHandler.ts:139:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/index.ts:8:12 - (tsdoc-characters-after-block-tag) The token "@azure" looks like a TSDoc tag but contains an invalid character "/"; if it is not a tag, use a backslash to escape the "@" // src/index.ts:8:4 - (tsdoc-undefined-tag) The TSDoc tag "@module" is not defined in this configuration -// src/navigation/NavigationClient.ts:36:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/navigation/NavigationClient.ts:37:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen +// src/navigation/NavigationClient.ts:40:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen +// src/navigation/NavigationClient.ts:41:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen ``` diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index 2d8d180818..3c5849dc9b 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -100,6 +100,7 @@ export const BrowserAuthErrorMessages = { "Failed to parse response headers", [BrowserAuthErrorCodes.failedToDecryptEarResponse]: "Failed to decrypt ear response", + [BrowserAuthErrorCodes.timedOut]: "The request timed out.", }; /** diff --git a/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts b/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts index 92ee27d1d9..31ee9fc9e5 100644 --- a/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts +++ b/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts @@ -60,3 +60,4 @@ export const invalidPopTokenRequest = "invalid_pop_token_request"; export const failedToBuildHeaders = "failed_to_build_headers"; export const failedToParseHeaders = "failed_to_parse_headers"; export const failedToDecryptEarResponse = "failed_to_decrypt_ear_response"; +export const timedOut = "timed_out"; diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 23240187ba..5c728ff3d6 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -282,6 +282,16 @@ export class RedirectClient extends StandardInteractionClient { this.performanceClient ); form.submit(); + return new Promise((resolve, reject) => { + setTimeout(() => { + reject( + createBrowserAuthError( + BrowserAuthErrorCodes.timedOut, + "failed_to_redirect" + ) + ); + }, this.config.system.redirectNavigationTimeout); + }); } /** diff --git a/lib/msal-browser/src/navigation/NavigationClient.ts b/lib/msal-browser/src/navigation/NavigationClient.ts index da78e700f5..a9ff74766d 100644 --- a/lib/msal-browser/src/navigation/NavigationClient.ts +++ b/lib/msal-browser/src/navigation/NavigationClient.ts @@ -3,6 +3,10 @@ * Licensed under the MIT License. */ +import { + BrowserAuthErrorCodes, + createBrowserAuthError, +} from "../error/BrowserAuthError.js"; import { INavigationClient } from "./INavigationClient.js"; import { NavigationOptions } from "./NavigationOptions.js"; @@ -46,9 +50,14 @@ export class NavigationClient implements INavigationClient { window.location.assign(url); // CodeQL [SM03712] Application owner controls the URL. User can't change it. } - return new Promise((resolve) => { + return new Promise((resolve, reject) => { setTimeout(() => { - resolve(true); + reject( + createBrowserAuthError( + BrowserAuthErrorCodes.timedOut, + "failed_to_redirect" + ) + ); }, options.timeout); }); } diff --git a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts index 62811be193..963ebc965b 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -2907,20 +2907,15 @@ describe("RedirectClient", () => { }); describe("EAR Flow Tests", () => { - beforeAll(() => { - jest.useFakeTimers(); - }); - - afterAll(() => { - jest.useRealTimers(); - }); - beforeEach(async () => { pca = new PublicClientApplication({ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID, protocolMode: ProtocolMode.EAR, }, + system: { + redirectNavigationTimeout: 1000, + }, }); await pca.initialize(); @@ -2929,7 +2924,7 @@ describe("RedirectClient", () => { ); }); - it("Invokes EAR flow when protocolMode is set to EAR", async () => { + it("Invokes EAR flow when protocolMode is set to EAR", (done) => { const validRequest: RedirectRequest = { authority: TEST_CONFIG.validAuthority, scopes: ["openid", "profile", "offline_access"], @@ -2941,18 +2936,40 @@ describe("RedirectClient", () => { jest.spyOn(ProtocolUtils, "setRequestState").mockReturnValue( TEST_STATE_VALUES.TEST_STATE_REDIRECT ); + jest.spyOn(HTMLFormElement.prototype, "submit").mockImplementation( + () => { + // Supress navigation + pca.handleRedirectPromise( + `#ear_jwe=${validEarJWE}&state=${TEST_STATE_VALUES.TEST_STATE_REDIRECT}` + ).then((result) => { + expect(result).toEqual(getTestAuthenticationResult()); + done(); + }); + } + ); + + pca.acquireTokenRedirect(validRequest).catch(() => {}); + }); + + it("Throws a timeout error if the form post failed to redirect within the alloted time", async () => { + const validRequest: RedirectRequest = { + scopes: ["openid", "profile", "offline_access"], + }; const earFormSpy = jest .spyOn(HTMLFormElement.prototype, "submit") .mockImplementation(() => { // Supress navigation }); - await pca.acquireTokenRedirect(validRequest); - expect(earFormSpy).toHaveBeenCalled(); - const result = await pca.handleRedirectPromise( - `#ear_jwe=${validEarJWE}&state=${TEST_STATE_VALUES.TEST_STATE_REDIRECT}` + await expect(() => + pca.acquireTokenRedirect(validRequest) + ).rejects.toEqual( + createBrowserAuthError( + BrowserAuthErrorCodes.timedOut, + "failed_to_redirect" + ) ); - expect(result).toEqual(getTestAuthenticationResult()); + expect(earFormSpy).toHaveBeenCalled(); }); }); }); diff --git a/lib/msal-browser/test/navigation/NavigationClient.spec.ts b/lib/msal-browser/test/navigation/NavigationClient.spec.ts index 4c862302bc..e57ed28bef 100644 --- a/lib/msal-browser/test/navigation/NavigationClient.spec.ts +++ b/lib/msal-browser/test/navigation/NavigationClient.spec.ts @@ -3,8 +3,11 @@ * Licensed under the MIT License. */ +import { create } from "domain"; import { NavigationClient } from "../../src/navigation/NavigationClient.js"; import { TEST_URIS } from "../utils/StringConstants.js"; +import { BrowserAuthError, BrowserAuthErrorCodes } from "../../src/index.js"; +import { createBrowserAuthError } from "../../src/error/BrowserAuthError.js"; describe("NavigationClient.ts Unit Tests", () => { const navigationClient = new NavigationClient(); @@ -22,6 +25,7 @@ describe("NavigationClient.ts Unit Tests", () => { afterEach(() => { window = oldWindow; jest.restoreAllMocks(); + jest.useRealTimers(); }); describe("navigateInternal tests", () => { @@ -118,22 +122,32 @@ describe("NavigationClient.ts Unit Tests", () => { expect(windowReplaceSpy).toHaveBeenCalledTimes(1); }); - it("navigateExternal() logs if navigation does not take place within 30 seconds", (done) => { + it("navigateExternal() throws if navigation does not take place within specified timeout", (done) => { + jest.useRealTimers(); window.location = { ...oldWindowLocation, replace: function (url: string) { - expect(url).toBe(TEST_URIS.TEST_LOGOUT_URI); - done(); + // Do nothing }, }; const windowReplaceSpy = jest.spyOn(window.location, "replace"); - navigationClient.navigateExternal(TEST_URIS.TEST_LOGOUT_URI, { - timeout: 30000, - noHistory: true, - //@ts-ignore - apiId: 0, - }); + navigationClient + .navigateExternal(TEST_URIS.TEST_LOGOUT_URI, { + timeout: 100, + noHistory: true, + //@ts-ignore + apiId: 0, + }) + .catch((error) => { + expect(error).toEqual( + createBrowserAuthError( + BrowserAuthErrorCodes.timedOut, + "failed_to_redirect" + ) + ); + done(); + }); expect(windowReplaceSpy).toHaveBeenCalledTimes(1); }); });