diff --git a/change/@azure-msal-browser-6d9ef336-f49e-47ba-9191-05676f55c887.json b/change/@azure-msal-browser-6d9ef336-f49e-47ba-9191-05676f55c887.json new file mode 100644 index 0000000000..76e887fada --- /dev/null +++ b/change/@azure-msal-browser-6d9ef336-f49e-47ba-9191-05676f55c887.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Fix iframe fallback when RT is not found in cache", + "packageName": "@azure/msal-browser", + "email": "thomas.norling@microsoft.com", + "dependentChangeType": "patch" +} diff --git a/lib/msal-browser/src/controllers/IController.ts b/lib/msal-browser/src/controllers/IController.ts index 9da30265ee..b75a6a5e3a 100644 --- a/lib/msal-browser/src/controllers/IController.ts +++ b/lib/msal-browser/src/controllers/IController.ts @@ -8,7 +8,6 @@ import { Logger, PerformanceCallbackFunction, IPerformanceClient, - CommonSilentFlowRequest, AccountFilter, } from "@azure/msal-common"; import { RedirectRequest } from "../request/RedirectRequest"; @@ -48,11 +47,6 @@ export interface IController { accountId?: string ): Promise; - acquireTokenByRefreshToken( - commonRequest: CommonSilentFlowRequest, - silentRequest: SilentRequest - ): Promise; - addEventCallback(callback: EventCallbackFunction): string | null; removeEventCallback(callbackId: string): void; diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts index 2af6081fa9..a040994665 100644 --- a/lib/msal-browser/src/controllers/StandardController.ts +++ b/lib/msal-browser/src/controllers/StandardController.ts @@ -21,7 +21,6 @@ import { PromptValue, InProgressPerformanceEvent, RequestThumbprint, - ServerError, AccountEntity, ServerResponseType, UrlString, @@ -30,6 +29,7 @@ import { ClientAuthErrorCodes, AccountFilter, buildStaticAuthorityOptions, + InteractionRequiredAuthErrorCodes, } from "@azure/msal-common"; import { BrowserCacheManager, @@ -1047,13 +1047,13 @@ export class StandardController implements IController { protected async acquireTokenFromCache( silentCacheClient: SilentCacheClient, commonRequest: CommonSilentFlowRequest, - silentRequest: SilentRequest + cacheLookupPolicy: CacheLookupPolicy ): Promise { this.performanceClient.addQueueMeasurement( PerformanceEvents.AcquireTokenFromCache, commonRequest.correlationId ); - switch (silentRequest.cacheLookupPolicy) { + switch (cacheLookupPolicy) { case CacheLookupPolicy.Default: case CacheLookupPolicy.AccessToken: case CacheLookupPolicy.AccessTokenAndRefreshToken: @@ -1074,18 +1074,18 @@ export class StandardController implements IController { /** * Attempt to acquire an access token via a refresh token * @param commonRequest CommonSilentFlowRequest - * @param silentRequest SilentRequest + * @param cacheLookupPolicy CacheLookupPolicy * @returns A promise that, when resolved, returns the access token */ public async acquireTokenByRefreshToken( commonRequest: CommonSilentFlowRequest, - silentRequest: SilentRequest + cacheLookupPolicy: CacheLookupPolicy ): Promise { this.performanceClient.addQueueMeasurement( PerformanceEvents.AcquireTokenByRefreshToken, commonRequest.correlationId ); - switch (silentRequest.cacheLookupPolicy) { + switch (cacheLookupPolicy) { case CacheLookupPolicy.Default: case CacheLookupPolicy.AccessTokenAndRefreshToken: case CacheLookupPolicy.RefreshToken: @@ -2055,12 +2055,8 @@ export class StandardController implements IController { request.correlationId )(request, account); - const requestWithCLP = { - ...request, - // set the request's CacheLookupPolicy to Default if it was not optionally passed in - cacheLookupPolicy: - request.cacheLookupPolicy || CacheLookupPolicy.Default, - }; + const cacheLookupPolicy = + request.cacheLookupPolicy || CacheLookupPolicy.Default; result = invokeAsync( this.acquireTokenFromCache.bind(this), @@ -2068,10 +2064,10 @@ export class StandardController implements IController { this.logger, this.performanceClient, silentRequest.correlationId - )(silentCacheClient, silentRequest, requestWithCLP).catch( + )(silentCacheClient, silentRequest, cacheLookupPolicy).catch( (cacheError: AuthError) => { if ( - requestWithCLP.cacheLookupPolicy === + request.cacheLookupPolicy === CacheLookupPolicy.AccessToken ) { throw cacheError; @@ -2091,42 +2087,42 @@ export class StandardController implements IController { this.logger, this.performanceClient, silentRequest.correlationId - )(silentRequest, requestWithCLP).catch( + )(silentRequest, cacheLookupPolicy).catch( (refreshTokenError: AuthError) => { - const isServerError = - refreshTokenError instanceof ServerError; - const isInteractionRequiredError = - refreshTokenError instanceof - InteractionRequiredAuthError; - const isInvalidGrantError = + const isSilentlyResolvable = + (!( + refreshTokenError instanceof + InteractionRequiredAuthError + ) && + (refreshTokenError.errorCode === + BrowserConstants.INVALID_GRANT_ERROR || + refreshTokenError.errorCode === + ClientAuthErrorCodes.tokenRefreshRequired)) || refreshTokenError.errorCode === - BrowserConstants.INVALID_GRANT_ERROR; - - if ( - (!isServerError || - !isInvalidGrantError || - isInteractionRequiredError || - requestWithCLP.cacheLookupPolicy === - CacheLookupPolicy.AccessTokenAndRefreshToken || - requestWithCLP.cacheLookupPolicy === - CacheLookupPolicy.RefreshToken) && - requestWithCLP.cacheLookupPolicy !== - CacheLookupPolicy.Skip - ) { + InteractionRequiredAuthErrorCodes.noTokensFound; + + const tryIframeRenewal = + cacheLookupPolicy === + CacheLookupPolicy.Default || + cacheLookupPolicy === CacheLookupPolicy.Skip || + cacheLookupPolicy === + CacheLookupPolicy.RefreshTokenAndNetwork; + + if (isSilentlyResolvable && tryIframeRenewal) { + this.logger.verbose( + "Refresh token expired/invalid or CacheLookupPolicy is set to Skip, attempting acquire token by iframe.", + request.correlationId + ); + return invokeAsync( + this.acquireTokenBySilentIframe.bind(this), + PerformanceEvents.AcquireTokenBySilentIframe, + this.logger, + this.performanceClient, + silentRequest.correlationId + )(silentRequest); + } else { throw refreshTokenError; } - - this.logger.verbose( - "Refresh token expired/invalid or CacheLookupPolicy is set to Skip, attempting acquire token by iframe.", - request.correlationId - ); - return invokeAsync( - this.acquireTokenBySilentIframe.bind(this), - PerformanceEvents.AcquireTokenBySilentIframe, - this.logger, - this.performanceClient, - silentRequest.correlationId - )(silentRequest); } ); } diff --git a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts index a6646ea316..dba9f11600 100644 --- a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts @@ -53,7 +53,6 @@ export class SilentRefreshClient extends StandardInteractionClient { silentRequest.authority, silentRequest.azureCloudOptions ); - this.logger.verbose("Refresh token client created"); // Send request to renew token. Auth module will throw errors if token cannot be renewed. return invokeAsync( refreshTokenClient.acquireTokenByRefreshToken.bind( @@ -63,24 +62,11 @@ export class SilentRefreshClient extends StandardInteractionClient { this.logger, this.performanceClient, request.correlationId - )(silentRequest) - .then((result) => result as AuthenticationResult) - .then((result: AuthenticationResult) => { - this.performanceClient.addFields( - { - fromCache: result.fromCache, - requestId: result.requestId, - }, - request.correlationId - ); - - return result; - }) - .catch((e: AuthError) => { - (e as AuthError).setCorrelationId(this.correlationId); - serverTelemetryManager.cacheFailedRequest(e); - throw e; - }); + )(silentRequest).catch((e: AuthError) => { + (e as AuthError).setCorrelationId(this.correlationId); + serverTelemetryManager.cacheFailedRequest(e); + throw e; + }) as Promise; } /** diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 391d2012dc..c66e8e7158 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -3461,6 +3461,52 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { expect(silentIframeSpy.calledOnce).toBe(true); }); + it("Calls SilentIframeClient.acquireToken and returns its response if no RT is cached", async () => { + const testAccount: AccountInfo = { + homeAccountId: TEST_DATA_CLIENT_INFO.TEST_HOME_ACCOUNT_ID, + localAccountId: TEST_DATA_CLIENT_INFO.TEST_UID, + environment: "login.windows.net", + tenantId: "3338040d-6c67-4c5b-b112-36a304b66dad", + username: "AbeLi@microsoft.com", + }; + const testTokenResponse: AuthenticationResult = { + authority: TEST_CONFIG.validAuthority, + uniqueId: testAccount.localAccountId, + tenantId: testAccount.tenantId, + scopes: TEST_CONFIG.DEFAULT_SCOPES, + idToken: "test-idToken", + idTokenClaims: {}, + accessToken: "test-accessToken", + fromCache: false, + correlationId: RANDOM_TEST_GUID, + expiresOn: new Date(Date.now() + 3600000), + account: testAccount, + tokenType: AuthenticationScheme.BEARER, + }; + const silentCacheSpy = sinon + .stub(SilentCacheClient.prototype, "acquireToken") + .rejects("Expired"); + const silentRefreshSpy = sinon + .stub(SilentRefreshClient.prototype, "acquireToken") + .rejects( + createInteractionRequiredAuthError( + InteractionRequiredAuthErrorCodes.noTokensFound + ) + ); + const silentIframeSpy = sinon + .stub(SilentIframeClient.prototype, "acquireToken") + .resolves(testTokenResponse); + + const response = await pca.acquireTokenSilent({ + scopes: ["openid"], + account: testAccount, + }); + expect(response).toEqual(testTokenResponse); + expect(silentCacheSpy.calledOnce).toBe(true); + expect(silentRefreshSpy.calledOnce).toBe(true); + expect(silentIframeSpy.calledOnce).toBe(true); + }); + it("makes one network request with multiple parallel silent requests with same request", async () => { const testServerTokenResponse = { token_type: TEST_CONFIG.TOKEN_TYPE_BEARER,