From 9b44cb454bd645284362f8ad6b0d98b9bf0a717a Mon Sep 17 00:00:00 2001 From: Jo Arroyo Date: Wed, 7 Aug 2024 17:05:39 -0700 Subject: [PATCH] Retry for invalid_grant errors - Redirect (#7231) PR 3 of 3 addressing need for retry for backup auth service. --------- Co-authored-by: Thomas Norling --- ...-19a76fe3-e6e0-470e-b076-b131405ffef6.json | 7 + .../apiReview/msal-browser.api.md | 11 +- lib/msal-browser/docs/configuration.md | 1 + .../src/cache/BrowserCacheManager.ts | 98 ++++ lib/msal-browser/src/config/Configuration.ts | 10 +- .../src/error/BrowserAuthError.ts | 2 + .../src/error/BrowserAuthErrorCodes.ts | 1 + .../src/interaction_client/RedirectClient.ts | 66 ++- .../interaction_handler/RedirectHandler.ts | 6 + .../src/request/RedirectRequest.ts | 5 + .../src/utils/BrowserConstants.ts | 2 + .../test/cache/BrowserCacheManager.spec.ts | 141 ++++++ .../interaction_client/RedirectClient.spec.ts | 440 +++++++++++++++++- 13 files changed, 782 insertions(+), 8 deletions(-) create mode 100644 change/@azure-msal-browser-19a76fe3-e6e0-470e-b076-b131405ffef6.json diff --git a/change/@azure-msal-browser-19a76fe3-e6e0-470e-b076-b131405ffef6.json b/change/@azure-msal-browser-19a76fe3-e6e0-470e-b076-b131405ffef6.json new file mode 100644 index 0000000000..a2a65795e0 --- /dev/null +++ b/change/@azure-msal-browser-19a76fe3-e6e0-470e-b076-b131405ffef6.json @@ -0,0 +1,7 @@ +{ + "type": "minor", + "comment": "Retry in RedirectClient for invalid_grant errors #7231", + "packageName": "@azure/msal-browser", + "email": "joarroyo@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 f1fc7b82d5..faa073e16e 100644 --- a/lib/msal-browser/apiReview/msal-browser.api.md +++ b/lib/msal-browser/apiReview/msal-browser.api.md @@ -235,7 +235,8 @@ declare namespace BrowserAuthErrorCodes { uninitializedPublicClientApplication, nativePromptNotSupported, invalidBase64String, - invalidPopTokenRequest + invalidPopTokenRequest, + failedToRetry } } export { BrowserAuthErrorCodes } @@ -448,6 +449,7 @@ export type BrowserAuthOptions = { azureCloudOptions?: AzureCloudOptions; skipAuthorityMetadataCache?: boolean; supportsNestedAppAuth?: boolean; + onRedirectNavigate?: (url: string) => boolean | void; }; // Warning: (ae-missing-release-tag) "BrowserCacheLocation" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) @@ -838,6 +840,11 @@ export { ExternalTokenResponse } // @public (undocumented) const failedToParseResponse = "failed_to_parse_response"; +// Warning: (ae-missing-release-tag) "failedToRetry" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) +// +// @public (undocumented) +const failedToRetry = "failed_to_retry"; + // Warning: (ae-missing-release-tag) "getCurrentUri" is part of the package's API, but it is missing a release tag (@alpha, @beta, @public, or @internal) // // @public @@ -1699,7 +1706,7 @@ export type WrapperSKU = (typeof WrapperSKU)[keyof typeof WrapperSKU]; // src/app/PublicClientNext.ts:81:79 - (tsdoc-malformed-inline-tag) Expecting a TSDoc tag starting with "{@" // src/app/PublicClientNext.ts:84:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen // src/app/PublicClientNext.ts:85:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen -// src/config/Configuration.ts:233:5 - (ae-forgotten-export) The symbol "InternalAuthOptions" needs to be exported by the entry point index.d.ts +// src/config/Configuration.ts:241:5 - (ae-forgotten-export) The symbol "InternalAuthOptions" needs to be exported by the entry point index.d.ts // 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 diff --git a/lib/msal-browser/docs/configuration.md b/lib/msal-browser/docs/configuration.md index f6b1a54c2c..31fb3fa8f2 100644 --- a/lib/msal-browser/docs/configuration.md +++ b/lib/msal-browser/docs/configuration.md @@ -89,6 +89,7 @@ const msalInstance = new PublicClientApplication(msalConfig); | `protocolMode` | Enum representing the protocol mode to use. If `"AAD"`, will function on the OIDC-compliant AAD v2 endpoints; if `"OIDC"`, will function on other OIDC-compliant endpoints. | string | `"AAD"` | | `azureCloudOptions` | A defined set of azure cloud options for developers to default to their specific cloud authorities, for specific clouds supported please refer to the [AzureCloudInstance](https://aka.ms/msaljs/azure_cloud_instance) | [AzureCloudOptions](https://azuread.github.io/microsoft-authentication-library-for-js/ref/modules/_azure_msal_common.html#azurecloudoptions) | [AzureCloudInstance.None](msaljs/azure_cloud_instance) | | `skipAuthorityMetadataCache` | A flag to choose whether to use the local metadata cache during authority initialization. Metadata cache would be used if no authority metadata is provided and before a network call for metadata has been made (see [Authority](../../msal-common/docs/authority.md)) | boolean | `false` | +| `onRedirectNavigate` | A callback that will be passed the url that MSAL will navigate to in redirect flows. Returning false in the callback will stop navigation. ### Cache Config Options diff --git a/lib/msal-browser/src/cache/BrowserCacheManager.ts b/lib/msal-browser/src/cache/BrowserCacheManager.ts index f6b5939d48..ad0ef4a648 100644 --- a/lib/msal-browser/src/cache/BrowserCacheManager.ts +++ b/lib/msal-browser/src/cache/BrowserCacheManager.ts @@ -1613,6 +1613,104 @@ export class BrowserCacheManager extends CacheManager { this.setInteractionInProgress(false); } + /** + * Create request retry key to cache retry status + * @param correlationId + */ + generateRequestRetriedKey(correlationId: string): string { + return `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.${correlationId}`; + } + + /** + * Gets the request retry value from the cache + * @param correlationId + */ + getRequestRetried(correlationId: string): number | null { + const requestRetriedKey = this.generateRequestRetriedKey(correlationId); + const cachedRetryNumber = this.getTemporaryCache(requestRetriedKey); + if (!cachedRetryNumber) { + return null; + } + return parseInt(cachedRetryNumber); + } + + /** + * Sets the request retry value to "retried" in the cache + * @param correlationId + */ + setRequestRetried(correlationId: string): void { + this.logger.trace("BrowserCacheManager.setRequestRetried called"); + const requestRetriedKey = this.generateRequestRetriedKey(correlationId); + this.setTemporaryCache(requestRetriedKey, "1", false); + } + + /** + * Removes all request retry values in the cache + */ + removeRequestRetried(): void { + this.temporaryCacheStorage.getKeys().forEach((key) => { + if (key.indexOf(TemporaryCacheKeys.REQUEST_RETRY) !== -1) { + this.removeTemporaryItem(key); + } + }); + } + + /** + * Caches the redirectRequest in the cache + * @param redirectRequest + */ + cacheRedirectRequest(redirectRequest: RedirectRequest): void { + this.logger.trace("BrowserCacheManager.cacheRedirectRequest called"); + const { ...restParams } = redirectRequest; + delete restParams.onRedirectNavigate; + const encodedValue = JSON.stringify(restParams); + + this.setTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST, + encodedValue, + true + ); + } + + /** + * Gets redirect request from the cache. Logs an error and returns undefined if nothing is found. + */ + getCachedRedirectRequest(): RedirectRequest | undefined { + this.logger.trace( + "BrowserCacheManager.getCachedRedirectRequest called" + ); + const cachedRedirectRequest = this.getTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST, + true + ); + if (!cachedRedirectRequest) { + this.logger.error(`No cached redirect request found.`); + } else { + this.removeTemporaryItem( + this.generateCacheKey(TemporaryCacheKeys.REDIRECT_REQUEST) + ); + let parsedRequest: RedirectRequest; + try { + parsedRequest = JSON.parse( + cachedRedirectRequest + ) as RedirectRequest; + } catch (e) { + this.logger.errorPii( + `Attempted to parse: ${cachedRedirectRequest}` + ); + this.logger.error( + `Parsing cached redirect request threw with error: ${e}` + ); + return; + } + + if (parsedRequest) { + return parsedRequest; + } + } + return; + } + cacheCodeRequest(authCodeRequest: CommonAuthorizationCodeRequest): void { this.logger.trace("BrowserCacheManager.cacheCodeRequest called"); diff --git a/lib/msal-browser/src/config/Configuration.ts b/lib/msal-browser/src/config/Configuration.ts index df5370648d..2f66680073 100644 --- a/lib/msal-browser/src/config/Configuration.ts +++ b/lib/msal-browser/src/config/Configuration.ts @@ -99,11 +99,19 @@ export type BrowserAuthOptions = { * @deprecated This flag is deprecated and will be removed in the next major version. createNestablePublicClientApplication should be used instead. */ supportsNestedAppAuth?: boolean; + /** + * Callback that will be passed the url that MSAL will navigate to in redirect flows. Returning false in the callback will stop navigation. + */ + onRedirectNavigate?: (url: string) => boolean | void; }; /** @internal */ -export type InternalAuthOptions = Required & { +export type InternalAuthOptions = Omit< + Required, + "onRedirectNavigate" +> & { OIDCOptions: Required; + onRedirectNavigate?: (url: string) => boolean | void; }; /** diff --git a/lib/msal-browser/src/error/BrowserAuthError.ts b/lib/msal-browser/src/error/BrowserAuthError.ts index 483f03d33b..fcb43d3db1 100644 --- a/lib/msal-browser/src/error/BrowserAuthError.ts +++ b/lib/msal-browser/src/error/BrowserAuthError.ts @@ -92,6 +92,8 @@ export const BrowserAuthErrorMessages = { "Invalid base64 encoded string.", [BrowserAuthErrorCodes.invalidPopTokenRequest]: "Invalid PoP token request. The request should not have both a popKid value and signPopToken set to true.", + [BrowserAuthErrorCodes.failedToRetry]: + "Unable to retry failed auth code redemption due to usage of the onRedirectNavigate request parameter. Please set onRedirectNavigate on the PublicClientApplication configuration instead or call loginRedirect again.", }; /** diff --git a/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts b/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts index ac099a1db8..2c0a1c873e 100644 --- a/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts +++ b/lib/msal-browser/src/error/BrowserAuthErrorCodes.ts @@ -56,3 +56,4 @@ export const uninitializedPublicClientApplication = export const nativePromptNotSupported = "native_prompt_not_supported"; export const invalidBase64String = "invalid_base64_string"; export const invalidPopTokenRequest = "invalid_pop_token_request"; +export const failedToRetry = "failed_to_retry"; diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 6968e7b054..4f5d4d5af3 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -22,10 +22,12 @@ import { ServerResponseType, UrlUtils, InProgressPerformanceEvent, + ServerError, } from "@azure/msal-common"; import { StandardInteractionClient } from "./StandardInteractionClient"; import { ApiId, + BrowserConstants, InteractionType, TemporaryCacheKeys, } from "../utils/BrowserConstants"; @@ -83,6 +85,14 @@ export class RedirectClient extends StandardInteractionClient { * @param request */ async acquireToken(request: RedirectRequest): Promise { + if (request.onRedirectNavigate) { + this.logger.warning( + "Unable to cache redirect request, onRedirectNavigate request option has been deprecated. Please set onRedirectNavigate on PublicClientApplication config instead." + ); + } else { + this.browserStorage.cacheRedirectRequest(request); + } + const validRequest = await invokeAsync( this.initializeAuthorizationRequest.bind(this), PerformanceEvents.StandardInteractionClientInitializeAuthorizationRequest, @@ -174,7 +184,9 @@ export class RedirectClient extends StandardInteractionClient { navigationClient: this.navigationClient, redirectTimeout: this.config.system.redirectNavigationTimeout, redirectStartPage: redirectStartPage, - onRedirectNavigate: request.onRedirectNavigate, + onRedirectNavigate: + request.onRedirectNavigate || + this.config.auth.onRedirectNavigate, }); } catch (e) { if (e instanceof AuthError) { @@ -201,6 +213,7 @@ export class RedirectClient extends StandardInteractionClient { const serverTelemetryManager = this.initializeServerTelemetryManager( ApiId.handleRedirectPromise ); + try { if (!this.browserStorage.isInteractionInProgress(true)) { this.logger.info( @@ -331,10 +344,59 @@ export class RedirectClient extends StandardInteractionClient { (e as AuthError).setCorrelationId(this.correlationId); serverTelemetryManager.cacheFailedRequest(e); } + + if ( + e instanceof ServerError && + e.errorCode === BrowserConstants.INVALID_GRANT_ERROR + ) { + this.performanceClient.addFields( + { + retryError: e.errorCode, + }, + this.correlationId + ); + + const requestRetried = this.browserStorage.getRequestRetried( + this.correlationId + ); + + if (requestRetried) { + this.logger.error( + "Retried request already detected. Throwing error." + ); + this.browserStorage.removeRequestRetried(); + throw e; + } + + const redirectRequest = + this.browserStorage.getCachedRedirectRequest(); + if (!redirectRequest) { + this.logger.error( + `Unable to retry. Please retry with redirect request and correlationId: ${this.correlationId}` + ); + this.browserStorage.setRequestRetried(this.correlationId); + throw createBrowserAuthError( + BrowserAuthErrorCodes.failedToRetry + ); + } + + this.browserStorage.setRequestRetried(this.correlationId); + + await this.acquireToken(redirectRequest); + return null; + } + + this.browserStorage.removeTemporaryItem( + this.browserStorage.generateCacheKey( + TemporaryCacheKeys.REDIRECT_REQUEST + ) + ); + this.browserStorage.removeRequestRetried(); + throw e; + } finally { this.browserStorage.cleanRequestByInteractionType( InteractionType.Redirect ); - throw e; } } diff --git a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts index b16a3d7699..0937d242fc 100644 --- a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts +++ b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts @@ -222,6 +222,12 @@ export class RedirectHandler { )) as AuthenticationResult; this.browserStorage.cleanRequestByState(state); + this.browserStorage.removeRequestRetried(); + this.browserStorage.removeTemporaryItem( + this.browserStorage.generateCacheKey( + TemporaryCacheKeys.REDIRECT_REQUEST + ) + ); return tokenResponse; } diff --git a/lib/msal-browser/src/request/RedirectRequest.ts b/lib/msal-browser/src/request/RedirectRequest.ts index 54ef92dd74..2b51527d47 100644 --- a/lib/msal-browser/src/request/RedirectRequest.ts +++ b/lib/msal-browser/src/request/RedirectRequest.ts @@ -46,6 +46,11 @@ export type RedirectRequest = Partial< > & { scopes: Array; redirectStartPage?: string; + /** + * @deprecated + * onRedirectNavigate is deprecated and will be removed in the next major version. + * Set onRedirectNavigate in Configuration instead. + */ onRedirectNavigate?: (url: string) => boolean | void; tokenBodyParameters?: StringDict; }; diff --git a/lib/msal-browser/src/utils/BrowserConstants.ts b/lib/msal-browser/src/utils/BrowserConstants.ts index 10c2043773..6b797a6483 100644 --- a/lib/msal-browser/src/utils/BrowserConstants.ts +++ b/lib/msal-browser/src/utils/BrowserConstants.ts @@ -93,6 +93,8 @@ export const TemporaryCacheKeys = { CORRELATION_ID: "request.correlationId", NATIVE_REQUEST: "request.native", REDIRECT_CONTEXT: "request.redirect.context", + REDIRECT_REQUEST: "request.redirect", + REQUEST_RETRY: "request.retry", } as const; export type TemporaryCacheKeys = (typeof TemporaryCacheKeys)[keyof typeof TemporaryCacheKeys]; diff --git a/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts b/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts index fbe2e25bc3..32b513fa75 100644 --- a/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts +++ b/lib/msal-browser/test/cache/BrowserCacheManager.spec.ts @@ -52,6 +52,7 @@ import { BrowserStateObject } from "../../src/utils/BrowserProtocolUtils"; import { base64Decode } from "../../src/encode/Base64Decode"; import { getDefaultPerformanceClient } from "../utils/TelemetryUtils"; import { BrowserPerformanceClient } from "../../src/telemetry/BrowserPerformanceClient"; +import { RedirectRequest } from "../../src/request/RedirectRequest"; describe("BrowserCacheManager tests", () => { let cacheConfig: Required; @@ -2972,6 +2973,146 @@ describe("BrowserCacheManager tests", () => { ).toBeUndefined(); }); + it("generateRequestRetriedKey() creates a valid cache key for request retry", () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + const requestRetriedKey = browserStorage.generateRequestRetriedKey( + TEST_CONFIG.CORRELATION_ID + ); + expect(requestRetriedKey).toBe( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.${TEST_CONFIG.CORRELATION_ID}` + ); + }); + + it("getRequestRetried() retrieves the request retry value from cache", () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + browserStorage.setTemporaryCache( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.${TEST_CONFIG.CORRELATION_ID}`, + "1" + ); + + browserStorage.getRequestRetried(TEST_CONFIG.CORRELATION_ID); + + expect( + browserStorage.getRequestRetried(TEST_CONFIG.CORRELATION_ID) + ).toEqual(1); + }); + + it("setRequestRetried() sets a request retry value for correlationId", () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + + browserStorage.setRequestRetried(TEST_CONFIG.CORRELATION_ID); + + expect( + window.sessionStorage[ + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.${TEST_CONFIG.CORRELATION_ID}` + ] + ).toBeTruthy(); + }); + + it("removeRequestRetried() removes all temporary cache items with request retried value", () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + browserStorage.setTemporaryCache( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.111`, + "1" + ); + browserStorage.setTemporaryCache( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.222`, + "1" + ); + + browserStorage.removeRequestRetried(); + + expect(browserStorage.getRequestRetried("111")).toEqual(null); + expect(browserStorage.getRequestRetried("222")).toEqual(null); + }); + + it("Successfully retrieves redirect request from cache", async () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + const testRedirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: TEST_CONFIG.CORRELATION_ID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + + browserStorage.setTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST, + JSON.stringify(testRedirectRequest), + true + ); + + const cachedRequest = browserStorage.getCachedRedirectRequest(); + expect(cachedRequest).toEqual(testRedirectRequest); + }); + + it("Returns undefined if redirect cannot be retrieved from cache", async () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + + const cachedRequest = browserStorage.getCachedRedirectRequest(); + expect(cachedRequest).toBeUndefined(); + }); + + it("Returns undefined if cached redirect request cannot be parsed correctly", async () => { + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + logger + ); + const testRedirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: TEST_CONFIG.CORRELATION_ID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + const stringifiedRequest = JSON.stringify(testRedirectRequest); + browserStorage.setTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST, + stringifiedRequest.substring(0, stringifiedRequest.length / 2), + true + ); + const cachedRequest = browserStorage.getCachedRedirectRequest(); + expect(cachedRequest).toBeUndefined(); + }); + it("Successfully retrieves and decodes response from cache", async () => { const browserStorage = new BrowserCacheManager( TEST_CONFIG.MSAL_CLIENT_ID, diff --git a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts index e6933b8b26..bbb5e87f5f 100644 --- a/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/RedirectClient.spec.ts @@ -51,6 +51,7 @@ import { IdTokenEntity, CredentialType, InProgressPerformanceEvent, + StringUtils, } from "@azure/msal-common"; import * as BrowserUtils from "../../src/utils/BrowserUtils"; import { @@ -163,7 +164,9 @@ describe("RedirectClient", () => { //@ts-ignore pca.performanceClient, //@ts-ignore - pca.nativeInternalStorage + pca.nativeInternalStorage, + undefined, + TEST_CONFIG.CORRELATION_ID ); rootMeasurement = new BrowserPerformanceClient( @@ -471,6 +474,20 @@ describe("RedirectClient", () => { `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.NONCE_IDTOKEN}.${stateId}`, "123523" ); + const testRedirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: TEST_CONFIG.CORRELATION_ID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REDIRECT_REQUEST}`, + base64Encode(JSON.stringify(testRedirectRequest)) + ); const testTokenReq: CommonAuthorizationCodeRequest = { redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`, code: "thisIsATestCode", @@ -555,6 +572,14 @@ describe("RedirectClient", () => { testTokenResponse.expiresOn.getMilliseconds() >= tokenResponse.expiresOn.getMilliseconds() ).toBeTruthy(); + expect( + browserStorage.getTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST + ) + ).toEqual(null); + expect( + browserStorage.getRequestRetried(TEST_CONFIG.CORRELATION_ID) + ).toEqual(null); }); it("gets hash from cache and calls native broker if hash contains accountId", async () => { @@ -1827,6 +1852,352 @@ describe("RedirectClient", () => { }); redirectClient.handleRedirectPromise("", rootMeasurement); }); + + it("retries on invalid_grant error when onRedirectNavigate is set in config", async () => { + let pca = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID, + onRedirectNavigate: (url: string) => {}, + }, + }); + + //PCA implementation moved to controller + pca = (pca as any).controller; + + // @ts-ignore + redirectClient = new RedirectClient( + // @ts-ignore + pca.config, + // @ts-ignore + pca.browserStorage, + // @ts-ignore + pca.browserCrypto, + // @ts-ignore + pca.logger, + // @ts-ignore + pca.eventHandler, + // @ts-ignore + pca.navigationClient, + // @ts-ignore + pca.performanceClient, + // @ts-ignore + pca.nativeInternalStorage, + undefined, + TEST_CONFIG.CORRELATION_ID + ); + + const stateString = TEST_STATE_VALUES.TEST_STATE_REDIRECT; + const browserCrypto = new CryptoOps(new Logger({})); + const stateId = ProtocolUtils.parseRequestState( + browserCrypto, + stateString + ).libraryState.id; + + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, + TEST_URIS.TEST_REDIR_URI + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.AUTHORITY}.${stateId}`, + TEST_CONFIG.validAuthority + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REQUEST_STATE}.${stateId}`, + TEST_STATE_VALUES.TEST_STATE_REDIRECT + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`, + TEST_HASHES.TEST_SUCCESS_CODE_HASH_REDIRECT + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}`, + TEST_CONFIG.MSAL_CLIENT_ID + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.NONCE_IDTOKEN}.${stateId}`, + "123523" + ); + const testRedirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: TEST_CONFIG.CORRELATION_ID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REDIRECT_REQUEST}`, + JSON.stringify(testRedirectRequest) + ); + const testTokenReq: CommonAuthorizationCodeRequest = { + redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`, + code: "thisIsATestCode", + scopes: TEST_CONFIG.DEFAULT_SCOPES, + codeVerifier: TEST_CONFIG.TEST_VERIFIER, + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REQUEST_PARAMS}`, + base64Encode(JSON.stringify(testTokenReq)) + ); + const testServerErrorResponse = { + headers: {}, + body: { + error: "invalid_grant", + error_description: "invalid_grant", + error_codes: ["invalid_grant"], + }, + status: 200, + }; + + jest.spyOn( + FetchClient.prototype, + "sendGetRequestAsync" + ).mockImplementation((url): any => { + if (url.includes("discovery/instance")) { + return DEFAULT_TENANT_DISCOVERY_RESPONSE; + } else if (url.includes(".well-known/openid-configuration")) { + return DEFAULT_OPENID_CONFIG_RESPONSE; + } + }); + + jest.spyOn( + FetchClient.prototype, + "sendPostRequestAsync" + ).mockResolvedValueOnce(testServerErrorResponse); + + const acquireTokenSpy = jest.spyOn(redirectClient, "acquireToken"); + + const tokenResponse = await redirectClient.handleRedirectPromise( + "", + rootMeasurement + ); + + expect(tokenResponse).toBe(null); + expect(acquireTokenSpy).toHaveBeenCalledTimes(1); + expect( + browserStorage.getTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST + ) + ).toEqual(null); + expect( + browserStorage.getRequestRetried(TEST_CONFIG.CORRELATION_ID) + ).toEqual(1); + }); + + it("throws invalid_grant error if already retried", (done) => { + const stateString = TEST_STATE_VALUES.TEST_STATE_REDIRECT; + const browserCrypto = new CryptoOps(new Logger({})); + const stateId = ProtocolUtils.parseRequestState( + browserCrypto, + stateString + ).libraryState.id; + + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, + TEST_URIS.TEST_REDIR_URI + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.AUTHORITY}.${stateId}`, + TEST_CONFIG.validAuthority + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REQUEST_STATE}.${stateId}`, + TEST_STATE_VALUES.TEST_STATE_REDIRECT + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`, + TEST_HASHES.TEST_SUCCESS_CODE_HASH_REDIRECT + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}`, + TEST_CONFIG.MSAL_CLIENT_ID + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.NONCE_IDTOKEN}.${stateId}`, + "123523" + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.${TEST_CONFIG.CORRELATION_ID}`, + JSON.stringify(1) + ); + const testRedirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: TEST_CONFIG.CORRELATION_ID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REDIRECT_REQUEST}`, + base64Encode(JSON.stringify(testRedirectRequest)) + ); + const testTokenReq: CommonAuthorizationCodeRequest = { + redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`, + code: "thisIsATestCode", + scopes: TEST_CONFIG.DEFAULT_SCOPES, + codeVerifier: TEST_CONFIG.TEST_VERIFIER, + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REQUEST_PARAMS}`, + base64Encode(JSON.stringify(testTokenReq)) + ); + const testServerErrorResponse = { + headers: {}, + body: { + error: "invalid_grant", + error_description: "invalid_grant", + error_codes: ["invalid_grant"], + }, + status: 200, + }; + + jest.spyOn( + FetchClient.prototype, + "sendGetRequestAsync" + ).mockImplementation((url): any => { + if (url.includes("discovery/instance")) { + return DEFAULT_TENANT_DISCOVERY_RESPONSE; + } else if (url.includes(".well-known/openid-configuration")) { + return DEFAULT_OPENID_CONFIG_RESPONSE; + } + }); + + jest.spyOn( + FetchClient.prototype, + "sendPostRequestAsync" + ).mockResolvedValueOnce(testServerErrorResponse); + + const acquireTokenSpy = jest.spyOn(redirectClient, "acquireToken"); + + redirectClient + .handleRedirectPromise("", rootMeasurement) + .catch((err) => { + expect(err instanceof ServerError).toBeTruthy(); + expect(err.errorCode).toEqual("invalid_grant"); + expect(acquireTokenSpy).toHaveBeenCalledTimes(0); + + expect( + browserStorage.getTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST + ) + ).toEqual(null); + expect( + browserStorage.getRequestRetried( + TEST_CONFIG.CORRELATION_ID + ) + ).toEqual(null); + + done(); + }); + }); + + it("throws failed_to_retry if invalid_grant is returned from server and redirect request is not cached", (done) => { + const stateString = TEST_STATE_VALUES.TEST_STATE_REDIRECT; + const browserCrypto = new CryptoOps(new Logger({})); + const stateId = ProtocolUtils.parseRequestState( + browserCrypto, + stateString + ).libraryState.id; + + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, + TEST_URIS.TEST_REDIR_URI + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.AUTHORITY}.${stateId}`, + TEST_CONFIG.validAuthority + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REQUEST_STATE}.${stateId}`, + TEST_STATE_VALUES.TEST_STATE_REDIRECT + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`, + TEST_HASHES.TEST_SUCCESS_CODE_HASH_REDIRECT + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}`, + TEST_CONFIG.MSAL_CLIENT_ID + ); + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.NONCE_IDTOKEN}.${stateId}`, + "123523" + ); + const testTokenReq: CommonAuthorizationCodeRequest = { + redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`, + code: "thisIsATestCode", + scopes: TEST_CONFIG.DEFAULT_SCOPES, + codeVerifier: TEST_CONFIG.TEST_VERIFIER, + authority: `${Constants.DEFAULT_AUTHORITY}`, + correlationId: TEST_CONFIG.CORRELATION_ID, + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + window.sessionStorage.setItem( + `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REQUEST_PARAMS}`, + base64Encode(JSON.stringify(testTokenReq)) + ); + const testServerErrorResponse = { + headers: {}, + body: { + error: "invalid_grant", + error_description: "invalid_grant", + error_codes: ["invalid_grant"], + }, + status: 200, + }; + + jest.spyOn( + FetchClient.prototype, + "sendGetRequestAsync" + ).mockImplementation((url): any => { + if (url.includes("discovery/instance")) { + return DEFAULT_TENANT_DISCOVERY_RESPONSE; + } else if (url.includes(".well-known/openid-configuration")) { + return DEFAULT_OPENID_CONFIG_RESPONSE; + } + }); + + jest.spyOn( + FetchClient.prototype, + "sendPostRequestAsync" + ).mockResolvedValueOnce(testServerErrorResponse); + + const acquireTokenSpy = jest.spyOn(redirectClient, "acquireToken"); + + redirectClient + .handleRedirectPromise("", rootMeasurement) + .catch((err) => { + expect(err instanceof AuthError).toBeTruthy(); + expect(err.errorCode).toEqual("failed_to_retry"); + expect(acquireTokenSpy).toHaveBeenCalledTimes(0); + + expect( + browserStorage.getTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST + ) + ).toEqual(null); + expect( + browserStorage.getRequestRetried( + TEST_CONFIG.CORRELATION_ID + ) + ).toEqual(1); + done(); + }); + }); }); describe("acquireToken", () => { @@ -2219,6 +2590,69 @@ describe("RedirectClient", () => { ).toEqual(JSON.stringify(testCcsCred)); }); + it("Caches redirect request correctly", async () => { + const redirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: RANDOM_TEST_GUID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + }; + + const browserCrypto = new CryptoOps(new Logger({})); + const testLogger = new Logger(loggerOptions); + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + testLogger + ); + await redirectClient.acquireToken(redirectRequest); + const cachedRequest: RedirectRequest = JSON.parse( + browserStorage.getTemporaryCache( + TemporaryCacheKeys.REDIRECT_REQUEST, + true + ) || "" + ); + expect(cachedRequest.scopes).toEqual(TEST_CONFIG.DEFAULT_SCOPES); + expect(cachedRequest.authority).toEqual( + `${Constants.DEFAULT_AUTHORITY}` + ); + expect(cachedRequest.correlationId).toEqual(RANDOM_TEST_GUID); + expect(cachedRequest.authenticationScheme).toEqual( + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme + ); + }); + + it("Does not cache redirect request if onRedirectNavigate is set", async () => { + const redirectRequest: RedirectRequest = { + redirectUri: TEST_URIS.TEST_REDIR_URI, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + correlationId: RANDOM_TEST_GUID, + state: TEST_STATE_VALUES.USER_STATE, + authority: TEST_CONFIG.validAuthority, + nonce: "", + authenticationScheme: + TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme, + onRedirectNavigate: (url: string) => {}, + }; + + const browserCrypto = new CryptoOps(new Logger({})); + const testLogger = new Logger(loggerOptions); + const browserStorage = new BrowserCacheManager( + TEST_CONFIG.MSAL_CLIENT_ID, + cacheConfig, + browserCrypto, + testLogger + ); + await redirectClient.acquireToken(redirectRequest); + const cachedRequest = browserStorage.getCachedRedirectRequest(); + expect(cachedRequest).toBeUndefined(); + }); + it("Caches token request correctly", async () => { const tokenRequest: CommonAuthorizationUrlRequest = { redirectUri: TEST_URIS.TEST_REDIR_URI, @@ -2318,7 +2752,7 @@ describe("RedirectClient", () => { await redirectClient.acquireToken(emptyRequest); } catch (e) { // Test that error was cached for telemetry purposes and then thrown - expect(window.sessionStorage).toHaveLength(1); + expect(window.sessionStorage).toHaveLength(2); const failures = window.sessionStorage.getItem( `server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}` ); @@ -2903,7 +3337,7 @@ describe("RedirectClient", () => { await redirectClient.acquireToken(emptyRequest); } catch (e) { // Test that error was cached for telemetry purposes and then thrown - expect(window.sessionStorage).toHaveLength(1); + expect(window.sessionStorage).toHaveLength(2); const failures = window.sessionStorage.getItem( `server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}` );