Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Fix iframe fallback when RT is not found in cache",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
6 changes: 0 additions & 6 deletions lib/msal-browser/src/controllers/IController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import {
Logger,
PerformanceCallbackFunction,
IPerformanceClient,
CommonSilentFlowRequest,
AccountFilter,
} from "@azure/msal-common";
import { RedirectRequest } from "../request/RedirectRequest";
Expand Down Expand Up @@ -48,11 +47,6 @@ export interface IController {
accountId?: string
): Promise<AuthenticationResult>;

acquireTokenByRefreshToken(
commonRequest: CommonSilentFlowRequest,
silentRequest: SilentRequest
): Promise<AuthenticationResult>;

addEventCallback(callback: EventCallbackFunction): string | null;

removeEventCallback(callbackId: string): void;
Expand Down
88 changes: 42 additions & 46 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
PromptValue,
InProgressPerformanceEvent,
RequestThumbprint,
ServerError,
AccountEntity,
ServerResponseType,
UrlString,
Expand All @@ -30,6 +29,7 @@ import {
ClientAuthErrorCodes,
AccountFilter,
buildStaticAuthorityOptions,
InteractionRequiredAuthErrorCodes,
} from "@azure/msal-common";
import {
BrowserCacheManager,
Expand Down Expand Up @@ -1047,13 +1047,13 @@ export class StandardController implements IController {
protected async acquireTokenFromCache(
silentCacheClient: SilentCacheClient,
commonRequest: CommonSilentFlowRequest,
silentRequest: SilentRequest
cacheLookupPolicy: CacheLookupPolicy
): Promise<AuthenticationResult> {
this.performanceClient.addQueueMeasurement(
PerformanceEvents.AcquireTokenFromCache,
commonRequest.correlationId
);
switch (silentRequest.cacheLookupPolicy) {
switch (cacheLookupPolicy) {
case CacheLookupPolicy.Default:
case CacheLookupPolicy.AccessToken:
case CacheLookupPolicy.AccessTokenAndRefreshToken:
Expand All @@ -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<AuthenticationResult> {
this.performanceClient.addQueueMeasurement(
PerformanceEvents.AcquireTokenByRefreshToken,
commonRequest.correlationId
);
switch (silentRequest.cacheLookupPolicy) {
switch (cacheLookupPolicy) {
case CacheLookupPolicy.Default:
case CacheLookupPolicy.AccessTokenAndRefreshToken:
case CacheLookupPolicy.RefreshToken:
Expand Down Expand Up @@ -2055,23 +2055,19 @@ 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),
PerformanceEvents.AcquireTokenFromCache,
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;
Expand All @@ -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);
}
);
}
Expand Down
24 changes: 5 additions & 19 deletions lib/msal-browser/src/interaction_client/SilentRefreshClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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(
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These things are added at the top level API

{
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<AuthenticationResult>;
}

/**
Expand Down
46 changes: 46 additions & 0 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: "[email protected]",
};
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,
Expand Down