Skip to content

Commit

Permalink
Fix broken back/forward cache behavior (#4513)
Browse files Browse the repository at this point in the history
* Fix broken bfcache behavior

* Change files

* remove event listener on failure
  • Loading branch information
tnorling authored Mar 7, 2022
1 parent 7a148d8 commit 131bde2
Show file tree
Hide file tree
Showing 3 changed files with 63 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Clear temporary cache when back button is clicked during redirect flow #4513",
"packageName": "@azure/msal-browser",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch"
}
12 changes: 12 additions & 0 deletions lib/msal-browser/src/interaction_client/RedirectClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@ export class RedirectClient extends StandardInteractionClient {
this.browserStorage.updateCacheEntries(validRequest.state, validRequest.nonce, validRequest.authority, validRequest.loginHint || "", validRequest.account || null);
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenRedirect);

const handleBackButton = (event: PageTransitionEvent) => {
// Clear temporary cache if the back button is clicked during the redirect flow.
if (event.persisted) {
this.logger.verbose("Page was restored from back/forward cache. Clearing temporary cache.");
this.browserStorage.cleanRequestByState(validRequest.state);
}
};

try {
// Create auth code request and generate PKCE params
const authCodeRequest: CommonAuthorizationCodeRequest = await this.initializeAuthorizationCodeRequest(validRequest);
Expand All @@ -41,6 +49,9 @@ export class RedirectClient extends StandardInteractionClient {
const redirectStartPage = this.getRedirectStartPage(request.redirectStartPage);
this.logger.verbosePii(`Redirect start page: ${redirectStartPage}`);

// Clear temporary cache if the back button is clicked during the redirect flow.
window.addEventListener("pageshow", handleBackButton);

// Show the UI once the url has been created. Response will come back in the hash, which will be handled in the handleRedirectCallback function.
return await interactionHandler.initiateAuthRequest(navigateUrl, {
navigationClient: this.navigationClient,
Expand All @@ -52,6 +63,7 @@ export class RedirectClient extends StandardInteractionClient {
if (e instanceof AuthError) {
e.setCorrelationId(this.correlationId);
}
window.removeEventListener("pageshow", handleBackButton);
serverTelemetryManager.cacheFailedRequest(e);
this.browserStorage.cleanRequestByState(validRequest.state);
throw e;
Expand Down
44 changes: 44 additions & 0 deletions lib/msal-browser/test/interaction_client/RedirectClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -922,6 +922,50 @@ describe("RedirectClient", () => {
expect(browserStorage.getTemporaryCache(browserStorage.generateAuthorityKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(`${Constants.DEFAULT_AUTHORITY}`);
});

it("Temporary cache is cleared when 'pageshow' event is fired", (done) => {
let bfCacheCallback: (event: object) => any;
jest.spyOn(window, "addEventListener").mockImplementation((eventName, callback) => {
expect(eventName).toEqual("pageshow");
// @ts-ignore
bfCacheCallback = callback;
});
const emptyRequest: CommonAuthorizationUrlRequest = {
redirectUri: TEST_URIS.TEST_REDIR_URI,
scopes: [],
state: TEST_STATE_VALUES.USER_STATE,
authority: TEST_CONFIG.validAuthority,
correlationId: TEST_CONFIG.CORRELATION_ID,
responseMode: TEST_CONFIG.RESPONSE_MODE as ResponseMode,
nonce: "",
authenticationScheme: TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme
};

sinon.stub(CryptoOps.prototype, "generatePkceCodes").resolves({
challenge: TEST_CONFIG.TEST_CHALLENGE,
verifier: TEST_CONFIG.TEST_VERIFIER
});

const testLogger = new Logger(loggerOptions);
const browserCrypto = new CryptoOps(new Logger({}));
const browserStorage = new BrowserCacheManager(TEST_CONFIG.MSAL_CLIENT_ID, cacheConfig, browserCrypto, testLogger);

sinon.stub(NavigationClient.prototype, "navigateExternal").callsFake((urlNavigate: string, options: NavigationOptions): Promise<boolean> => {
expect(browserStorage.isInteractionInProgress()).toBe(true);
expect(browserStorage.getTemporaryCache(browserStorage.generateStateKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(TEST_STATE_VALUES.TEST_STATE_REDIRECT);
expect(browserStorage.getTemporaryCache(browserStorage.generateNonceKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(RANDOM_TEST_GUID);
expect(browserStorage.getTemporaryCache(browserStorage.generateAuthorityKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(`${Constants.DEFAULT_AUTHORITY}`);
bfCacheCallback({ persisted: true });
expect(browserStorage.isInteractionInProgress()).toBe(false);
expect(browserStorage.getTemporaryCache(browserStorage.generateStateKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(null);
expect(browserStorage.getTemporaryCache(browserStorage.generateNonceKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(null);
expect(browserStorage.getTemporaryCache(browserStorage.generateAuthorityKey(TEST_STATE_VALUES.TEST_STATE_REDIRECT))).toEqual(null);
done();
return Promise.resolve(true);
});
browserStorage.setInteractionInProgress(true); // This happens in PCA so need to set manually here
redirectClient.acquireToken(emptyRequest);
});

it("Adds login_hint as CCS cache entry to the cache and urlNavigate", async () => {
const testIdTokenClaims: TokenClaims = {
"ver": "2.0",
Expand Down

0 comments on commit 131bde2

Please sign in to comment.