Skip to content

Commit 52465a8

Browse files
author
Prithvi Kanherkar
committed
Fixing issues with cache, ready to merge with overall branch
1 parent 37d94e5 commit 52465a8

File tree

6 files changed

+96
-101
lines changed

6 files changed

+96
-101
lines changed

lib/msal-core/src/UserAgentApplication.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2168,7 +2168,6 @@ export class UserAgentApplication {
21682168
*
21692169
*/
21702170
private getTokenType(accountObject: Account, scopes: string[], silentCall: boolean): string {
2171-
21722171
/*
21732172
* if account is passed and matches the account object/or set to getAccount() from cache
21742173
* if client-id is passed as scope, get id_token else token/id_token_token (in case no session exists)

lib/msal-core/src/cache/AuthCache.ts

Lines changed: 37 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -14,10 +14,13 @@ import { BrowserStorage } from "./BrowserStorage";
1414
export class AuthCache extends BrowserStorage {// Singleton
1515

1616
private clientId: string;
17+
private rollbackEnabled: boolean;
1718

1819
constructor(clientId: string, cacheLocation: CacheLocation, storeAuthStateInCookie: boolean) {
1920
super(cacheLocation);
2021
this.clientId = clientId;
22+
// This is hardcoded to true for now. We may make this configurable in the future
23+
this.rollbackEnabled = true;
2124
this.migrateCacheEntries(storeAuthStateInCookie);
2225
}
2326

@@ -34,60 +37,76 @@ export class AuthCache extends BrowserStorage {// Singleton
3437
const values = [idTokenValue, clientInfoValue, errorValue, errorDescValue];
3538
const keysToMigrate = [CacheKeys.IDTOKEN, CacheKeys.CLIENT_INFO, CacheKeys.ERROR, CacheKeys.ERROR_DESC];
3639

37-
keysToMigrate.forEach((cacheKey, index) => this.replaceCacheEntry(`${CacheKeys.PREFIX}.${cacheKey}`, cacheKey, values[index], storeAuthStateInCookie));
40+
keysToMigrate.forEach((cacheKey, index) => this.duplicateCacheEntry(cacheKey, values[index], storeAuthStateInCookie));
3841
}
3942

40-
private replaceCacheEntry(oldKey: string, newKey: string, value: string, storeAuthStateInCookie?: boolean) {
43+
private duplicateCacheEntry(newKey: string, value: string, storeAuthStateInCookie?: boolean) {
4144
if (value) {
42-
super.removeItem(oldKey);
4345
this.setItem(newKey, value, storeAuthStateInCookie);
4446
}
4547
}
4648

4749
// Prepend msal.<client-id> to each key
48-
private generateCacheKey(key: string): string {
50+
private generateCacheKey(key: string, addInstanceId: boolean): string {
4951
try {
5052
JSON.parse(key);
5153
return key;
5254
} catch (e) {
5355
if (key.startsWith(`${CacheKeys.PREFIX}.${this.clientId}`)) {
5456
return key;
5557
} else {
56-
return `${CacheKeys.PREFIX}.${this.clientId}.${key}`;
58+
return addInstanceId ? `${CacheKeys.PREFIX}.${this.clientId}.${key}` : `${CacheKeys.PREFIX}.${key}`;
5759
}
58-
5960
}
6061
}
6162

6263
// add value to storage
6364
setItem(key: string, value: string, enableCookieStorage?: boolean): void {
64-
super.setItem(this.generateCacheKey(key), value, enableCookieStorage);
65+
super.setItem(this.generateCacheKey(key, true), value, enableCookieStorage);
66+
67+
if (this.rollbackEnabled) {
68+
super.setItem(this.generateCacheKey(key, false), value, enableCookieStorage);
69+
}
6570
}
6671

6772
// get one item by key from storage
6873
getItem(key: string, enableCookieStorage?: boolean): string {
69-
return super.getItem(this.generateCacheKey(key), enableCookieStorage);
74+
return super.getItem(this.generateCacheKey(key, true), enableCookieStorage);
7075
}
7176

7277
// remove value from storage
7378
removeItem(key: string): void {
74-
super.removeItem(this.generateCacheKey(key));
79+
super.removeItem(this.generateCacheKey(key, true));
80+
if (this.rollbackEnabled) {
81+
super.removeItem(this.generateCacheKey(key, false));
82+
}
7583
}
7684

7785
resetCacheItems(): void {
7886
const storage = window[this.cacheLocation];
79-
if (storage) {
80-
super.resetCacheItems();
81-
this.removeAcquireTokenEntries();
87+
let key: string;
88+
this.removeAcquireTokenEntries();
89+
for (key in storage) {
90+
if (storage.hasOwnProperty(key)) {
91+
// Check if key contains msal prefix
92+
if (key.indexOf(CacheKeys.PREFIX) !== -1) {
93+
// For now, we are clearing all cache items created by MSAL.js
94+
super.removeItem(key);
95+
// TODO: Clear cache based on client id (clarify use cases where this is needed)
96+
}
97+
}
8298
}
8399
}
84100

85101
setItemCookie(cName: string, cValue: string, expires?: number): void {
86-
super.setItemCookie(this.generateCacheKey(cName), cValue, expires);
102+
super.setItemCookie(this.generateCacheKey(cName, true), cValue, expires);
103+
if (this.rollbackEnabled) {
104+
super.setItemCookie(this.generateCacheKey(cName, false), cValue, expires);
105+
}
87106
}
88107

89108
getItemCookie(cName: string): string {
90-
return super.getItemCookie(this.generateCacheKey(cName));
109+
return super.getItemCookie(this.generateCacheKey(cName, true));
91110
}
92111

93112
getAllAccessTokens(clientId: string, homeAccountIdentifier: string): Array<AccessTokenCacheItem> {
@@ -124,10 +143,10 @@ export class AuthCache extends BrowserStorage {// Singleton
124143
keyState = resourceDelimSplitKey[resourceDelimSplitKey.length-1];
125144
}
126145
if (keyState && !this.tokenRenewalInProgress(keyState)) {
127-
this.removeItem(key);
128-
this.removeItem(CacheKeys.RENEW_STATUS + state);
129-
this.removeItem(CacheKeys.STATE_LOGIN);
130-
this.removeItem(CacheKeys.STATE_ACQ_TOKEN);
146+
super.removeItem(key);
147+
super.removeItem(CacheKeys.RENEW_STATUS + state);
148+
super.removeItem(CacheKeys.STATE_LOGIN);
149+
super.removeItem(CacheKeys.STATE_ACQ_TOKEN);
131150
this.setItemCookie(key, "", -1);
132151
}
133152
}

lib/msal-core/src/cache/BrowserStorage.ts

Lines changed: 0 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -53,18 +53,6 @@ export class BrowserStorage {// Singleton
5353
return window[this.cacheLocation].clear();
5454
}
5555

56-
resetCacheItems(): void {
57-
const storage = window[this.cacheLocation];
58-
let key: string;
59-
for (key in storage) {
60-
if (storage.hasOwnProperty(key)) {
61-
if (key.indexOf(CacheKeys.PREFIX) !== -1) {
62-
this.removeItem(key);
63-
}
64-
}
65-
}
66-
}
67-
6856
setItemCookie(cName: string, cValue: string, expires?: number): void {
6957
let cookieStr = cName + "=" + cValue + ";";
7058
if (expires) {

lib/msal-core/src/utils/Constants.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ export class Constants {
2525

2626
static get response_mode_fragment(): string { return "&response_mode=fragment"; }
2727
static get resourceDelimiter(): string { return "|"; }
28+
static get cacheDelimiter(): string { return "."; }
2829

2930
static get tokenRenewStatusCancelled(): string { return "Canceled"; }
3031
static get tokenRenewStatusCompleted(): string { return "Completed"; }

lib/msal-core/test/Storage.localStorage.spec.ts

Lines changed: 29 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -171,35 +171,6 @@ describe("CacheStorage.ts Class - Local Storage", function () {
171171
expect(actualNextDayUTC).to.be.eq(nextDayUTC.toUTCString());
172172
expect(actualDayAfterUTC).to.be.eq(dayAfterUTC.toUTCString());
173173
});
174-
175-
176-
it("resetCacheItems deletes all msal related cache items", function () {
177-
let clientInfoKey = `${CacheKeys.PREFIX}.${CacheKeys.CLIENT_INFO}`;
178-
let stateLoginKey = `${CacheKeys.PREFIX}.${CacheKeys.STATE_LOGIN}`;
179-
let idTokenKey = `${CacheKeys.PREFIX}.${CacheKeys.IDTOKEN}`;
180-
let nonceIdTokenKey = `${CacheKeys.PREFIX}.${CacheKeys.NONCE_IDTOKEN}`;
181-
let renewStatusKey = `${CacheKeys.PREFIX}.${CacheKeys.RENEW_STATUS}` + "|RANDOM_GUID";
182-
183-
window.localStorage.setItem(clientInfoKey, "clientInfo");
184-
window.localStorage.setItem(stateLoginKey, "stateLogin");
185-
window.localStorage.setItem(idTokenKey, "idToken1");
186-
window.localStorage.setItem(nonceIdTokenKey, "idTokenNonce");
187-
window.localStorage.setItem(renewStatusKey, "Completed");
188-
189-
expect(cacheStorage.getItem(clientInfoKey)).to.be.eq("clientInfo");
190-
expect(cacheStorage.getItem(stateLoginKey)).to.be.eq("stateLogin");
191-
expect(cacheStorage.getItem(idTokenKey)).to.be.eq("idToken1");
192-
expect(cacheStorage.getItem(nonceIdTokenKey)).to.be.eq("idTokenNonce");
193-
expect(cacheStorage.getItem(renewStatusKey)).to.be.eq("Completed");
194-
195-
cacheStorage.resetCacheItems();
196-
197-
expect(cacheStorage.getItem(clientInfoKey)).to.be.null;
198-
expect(cacheStorage.getItem(stateLoginKey)).to.be.null;
199-
expect(cacheStorage.getItem(idTokenKey)).to.be.null;
200-
expect(cacheStorage.getItem(nonceIdTokenKey)).to.be.null;
201-
expect(cacheStorage.getItem(renewStatusKey)).to.be.null;
202-
});
203174
});
204175

205176
describe("MSAL Cache Item Management", function () {
@@ -299,20 +270,43 @@ describe("CacheStorage.ts Class - Local Storage", function () {
299270
let stateLoginString = "stateLogin";
300271
let loginRequestString = "loginRequest";
301272
let stateAcquireTokenString = "stateAcquireToken";
302-
console.log(document.cookie);
303273
msalCacheStorage.setItemCookie(CacheKeys.NONCE_IDTOKEN, idTokenNonceString);
304-
console.log(document.cookie);
305274
msalCacheStorage.setItemCookie(CacheKeys.STATE_LOGIN, stateLoginString);
306-
console.log(document.cookie);
307275
msalCacheStorage.setItemCookie(CacheKeys.LOGIN_REQUEST, loginRequestString);
308-
console.log(document.cookie);
309276
msalCacheStorage.setItemCookie(CacheKeys.STATE_ACQ_TOKEN, stateAcquireTokenString);
310-
console.log(document.cookie);
311277
msalCacheStorage.clearMsalCookie();
312-
console.log(document.cookie);
313278
expect(document.cookie).to.be.empty;
314279
});
315280

281+
it("resetCacheItems deletes msal related cache items", function () {
282+
let clientInfoKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.CLIENT_INFO}`;
283+
let stateLoginKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.STATE_LOGIN}`;
284+
let idTokenKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.IDTOKEN}`;
285+
let nonceIdTokenKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.NONCE_IDTOKEN}`;
286+
let renewStatusKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.RENEW_STATUS}` + "|RANDOM_GUID";
287+
288+
window.localStorage.setItem(clientInfoKey, "clientInfo");
289+
window.localStorage.setItem(stateLoginKey, "stateLogin");
290+
window.localStorage.setItem(idTokenKey, "idToken1");
291+
window.localStorage.setItem(nonceIdTokenKey, "idTokenNonce");
292+
window.localStorage.setItem(renewStatusKey, "Completed");
293+
294+
expect(msalCacheStorage.getItem(CacheKeys.CLIENT_INFO)).to.be.eq("clientInfo");
295+
expect(msalCacheStorage.getItem(CacheKeys.STATE_LOGIN)).to.be.eq("stateLogin");
296+
expect(msalCacheStorage.getItem(CacheKeys.IDTOKEN)).to.be.eq("idToken1");
297+
expect(msalCacheStorage.getItem(CacheKeys.NONCE_IDTOKEN)).to.be.eq("idTokenNonce");
298+
expect(msalCacheStorage.getItem(CacheKeys.RENEW_STATUS + "|RANDOM_GUID")).to.be.eq("Completed");
299+
300+
msalCacheStorage.resetCacheItems();
301+
302+
expect(msalCacheStorage.getItem(CacheKeys.CLIENT_INFO)).to.be.null;
303+
expect(msalCacheStorage.getItem(CacheKeys.STATE_LOGIN)).to.be.null;
304+
expect(msalCacheStorage.getItem(CacheKeys.IDTOKEN)).to.be.null;
305+
expect(msalCacheStorage.getItem(CacheKeys.NONCE_IDTOKEN)).to.be.null;
306+
expect(msalCacheStorage.getItem(CacheKeys.RENEW_STATUS + "|RANDOM_GUID")).to.be.null;
307+
});
308+
309+
it.skip("tests that resetCacheItems only deletes instance-specific cache items");
316310
});
317311

318312
describe("static key generators", function () {

lib/msal-core/test/Storage.sessionStorage.spec.ts

Lines changed: 29 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -170,35 +170,6 @@ describe("CacheStorage.ts Class - Session Storage", function () {
170170
expect(actualNextDayUTC).to.be.eq(nextDayUTC.toUTCString());
171171
expect(actualDayAfterUTC).to.be.eq(dayAfterUTC.toUTCString());
172172
});
173-
174-
175-
it("resetCacheItems deletes all msal related cache items", function () {
176-
let clientInfoKey = `${CacheKeys.PREFIX}.${CacheKeys.CLIENT_INFO}`;
177-
let stateLoginKey = `${CacheKeys.PREFIX}.${CacheKeys.STATE_LOGIN}`;
178-
let idTokenKey = `${CacheKeys.PREFIX}.${CacheKeys.IDTOKEN}`;
179-
let nonceIdTokenKey = `${CacheKeys.PREFIX}.${CacheKeys.NONCE_IDTOKEN}`;
180-
let renewStatusKey = `${CacheKeys.PREFIX}.${CacheKeys.RENEW_STATUS}` + "|RANDOM_GUID";
181-
182-
window.sessionStorage.setItem(clientInfoKey, "clientInfo");
183-
window.sessionStorage.setItem(stateLoginKey, "stateLogin");
184-
window.sessionStorage.setItem(idTokenKey, "idToken1");
185-
window.sessionStorage.setItem(nonceIdTokenKey, "idTokenNonce");
186-
window.sessionStorage.setItem(renewStatusKey, "Completed");
187-
188-
expect(cacheStorage.getItem(clientInfoKey)).to.be.eq("clientInfo");
189-
expect(cacheStorage.getItem(stateLoginKey)).to.be.eq("stateLogin");
190-
expect(cacheStorage.getItem(idTokenKey)).to.be.eq("idToken1");
191-
expect(cacheStorage.getItem(nonceIdTokenKey)).to.be.eq("idTokenNonce");
192-
expect(cacheStorage.getItem(renewStatusKey)).to.be.eq("Completed");
193-
194-
cacheStorage.resetCacheItems();
195-
196-
expect(cacheStorage.getItem(clientInfoKey)).to.be.null;
197-
expect(cacheStorage.getItem(stateLoginKey)).to.be.null;
198-
expect(cacheStorage.getItem(idTokenKey)).to.be.null;
199-
expect(cacheStorage.getItem(nonceIdTokenKey)).to.be.null;
200-
expect(cacheStorage.getItem(renewStatusKey)).to.be.null;
201-
});
202173
});
203174

204175
describe("MSAL Cache Item Management", function () {
@@ -298,20 +269,43 @@ describe("CacheStorage.ts Class - Session Storage", function () {
298269
let stateLoginString = "stateLogin";
299270
let loginRequestString = "loginRequest";
300271
let stateAcquireTokenString = "stateAcquireToken";
301-
console.log(document.cookie);
302272
msalCacheStorage.setItemCookie(CacheKeys.NONCE_IDTOKEN, idTokenNonceString);
303-
console.log(document.cookie);
304273
msalCacheStorage.setItemCookie(CacheKeys.STATE_LOGIN, stateLoginString);
305-
console.log(document.cookie);
306274
msalCacheStorage.setItemCookie(CacheKeys.LOGIN_REQUEST, loginRequestString);
307-
console.log(document.cookie);
308275
msalCacheStorage.setItemCookie(CacheKeys.STATE_ACQ_TOKEN, stateAcquireTokenString);
309-
console.log(document.cookie);
310276
msalCacheStorage.clearMsalCookie();
311-
console.log(document.cookie);
312277
expect(document.cookie).to.be.empty;
313278
});
314279

280+
it("resetCacheItems deletes msal related cache items", function () {
281+
let clientInfoKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.CLIENT_INFO}`;
282+
let stateLoginKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.STATE_LOGIN}`;
283+
let idTokenKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.IDTOKEN}`;
284+
let nonceIdTokenKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.NONCE_IDTOKEN}`;
285+
let renewStatusKey = `${CacheKeys.PREFIX}.${MSAL_CLIENT_ID}.${CacheKeys.RENEW_STATUS}` + "|RANDOM_GUID";
286+
287+
window.sessionStorage.setItem(clientInfoKey, "clientInfo");
288+
window.sessionStorage.setItem(stateLoginKey, "stateLogin");
289+
window.sessionStorage.setItem(idTokenKey, "idToken1");
290+
window.sessionStorage.setItem(nonceIdTokenKey, "idTokenNonce");
291+
window.sessionStorage.setItem(renewStatusKey, "Completed");
292+
293+
expect(msalCacheStorage.getItem(CacheKeys.CLIENT_INFO)).to.be.eq("clientInfo");
294+
expect(msalCacheStorage.getItem(CacheKeys.STATE_LOGIN)).to.be.eq("stateLogin");
295+
expect(msalCacheStorage.getItem(CacheKeys.IDTOKEN)).to.be.eq("idToken1");
296+
expect(msalCacheStorage.getItem(CacheKeys.NONCE_IDTOKEN)).to.be.eq("idTokenNonce");
297+
expect(msalCacheStorage.getItem(CacheKeys.RENEW_STATUS + "|RANDOM_GUID")).to.be.eq("Completed");
298+
299+
msalCacheStorage.resetCacheItems();
300+
301+
expect(msalCacheStorage.getItem(CacheKeys.CLIENT_INFO)).to.be.null;
302+
expect(msalCacheStorage.getItem(CacheKeys.STATE_LOGIN)).to.be.null;
303+
expect(msalCacheStorage.getItem(CacheKeys.IDTOKEN)).to.be.null;
304+
expect(msalCacheStorage.getItem(CacheKeys.NONCE_IDTOKEN)).to.be.null;
305+
expect(msalCacheStorage.getItem(CacheKeys.RENEW_STATUS + "|RANDOM_GUID")).to.be.null;
306+
});
307+
308+
it.skip("tests that resetCacheItems only deletes instance-specific cache items");
315309
});
316310

317311
describe("static key generators", function () {

0 commit comments

Comments
 (0)