Skip to content

Commit

Permalink
Merge pull request #675 from AzureAD/cache_cleanup
Browse files Browse the repository at this point in the history 
Fix MSAL cache cleaning method
  • Loading branch information
@sameerag
sameerag committed May 3, 2019
2 parents 325abf1 + b4afc0c commit 77f0470
Show file tree
Hide file tree
Showing 5 changed files with 68 additions and 56 deletions.
2 changes: 1 addition & 1 deletion lib/msal-core/src/Constants.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ export class Constants {
* @hidden
*/
export const CacheKeys = {
AUTHORITY: "msal_authority",
AUTHORITY: "msal.authority",
ACQUIRE_TOKEN_USER: "msal.acquireTokenUser"
};

Expand Down
29 changes: 23 additions & 6 deletions lib/msal-core/src/Storage.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,18 +90,37 @@ export class Storage {// Singleton
return results;
}

removeAcquireTokenEntries(authorityKey: string, acquireTokenAccountKey: string): void {
removeAcquireTokenEntries(): void {
const storage = window[this.cacheLocation];
if (storage) {
let key: string;
for (key in storage) {
if (storage.hasOwnProperty(key)) {
if ((authorityKey !== "" && key.indexOf(authorityKey) > -1) || (acquireTokenAccountKey !== "" && key.indexOf(acquireTokenAccountKey) > -1)) {
this.removeItem(key);
if (key.indexOf(CacheKeys.AUTHORITY) !== -1 || key.indexOf(CacheKeys.ACQUIRE_TOKEN_USER) !== 1) {
const splitKey = key.split(Constants.resourceDelimiter);
let state;
if (splitKey.length > 1) {
state = splitKey[1];
}
if (state && !this.tokenRenewalInProgress(state)) {
this.removeItem(key);
this.removeItem(Constants.renewStatus + state);
this.removeItem(Constants.stateLogin);
this.removeItem(Constants.stateAcquireToken);
this.setItemCookie(key, "", -1);
}
}
}
}
}

this.clearCookie();
}

private tokenRenewalInProgress(stateValue: string): boolean {
const storage = window[this.cacheLocation];
const renewStatus = storage[Constants.renewStatus + stateValue];
return !(!renewStatus || renewStatus !== Constants.tokenRenewStatusInProgress);
}

resetCacheItems(): void {
Expand All @@ -113,11 +132,9 @@ export class Storage {// Singleton
if (key.indexOf(Constants.msal) !== -1) {
this.setItem(key, "");
}
if (key.indexOf(Constants.renewStatus) !== -1) {
this.removeItem(key);
}
}
}
this.removeAcquireTokenEntries();
}
}

Expand Down
75 changes: 35 additions & 40 deletions lib/msal-core/src/UserAgentApplication.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -352,18 +352,7 @@ export class UserAgentApplication {
this.cacheStorage.setItem(Constants.angularLoginRequest, "");
}

// Cache the state, nonce, and login request data
this.cacheStorage.setItem(Constants.loginRequest, loginStartPage, this.inCookie);
this.cacheStorage.setItem(Constants.loginError, "");

this.cacheStorage.setItem(Constants.stateLogin, serverAuthenticationRequest.state, this.inCookie);
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);

this.cacheStorage.setItem(Constants.msalError, "");
this.cacheStorage.setItem(Constants.msalErrorDescription, "");

// Cache authorityKey
this.setAuthorityCache(serverAuthenticationRequest.state, this.authority);
this.updateCacheEntries(serverAuthenticationRequest, account, loginStartPage);

// build URL to navigate to proceed with the login
let urlNavigate = serverAuthenticationRequest.createNavigateUrl(scopes) + Constants.response_mode_fragment;
Expand Down Expand Up @@ -430,12 +419,7 @@ export class UserAgentApplication {
request.state
);

// Cache nonce
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);

// Cache account and authority
this.setAccountCache(account, serverAuthenticationRequest.state);
this.setAuthorityCache(serverAuthenticationRequest.state, acquireTokenAuthority.CanonicalAuthority);
this.updateCacheEntries(serverAuthenticationRequest, account);

// populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer
serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest);
Expand Down Expand Up @@ -466,7 +450,6 @@ export class UserAgentApplication {
parameters.hasOwnProperty(Constants.error) ||
parameters.hasOwnProperty(Constants.accessToken) ||
parameters.hasOwnProperty(Constants.idToken)

);
}

Expand Down Expand Up @@ -519,7 +502,6 @@ export class UserAgentApplication {

resolve(response);
}, (error) => {

this.silentLogin = false;
this.logger.error("Error occurred during unified cache ATS");
this.loginPopupHelper(null, request, resolve, reject, scopes);
Expand Down Expand Up @@ -565,6 +547,8 @@ export class UserAgentApplication {
// populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer;
serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest);

this.updateCacheEntries(serverAuthenticationRequest, account, window.location.href);

// Cache the state, nonce, and login request data
this.cacheStorage.setItem(Constants.loginRequest, window.location.href, this.inCookie);
this.cacheStorage.setItem(Constants.loginError, "");
Expand Down Expand Up @@ -674,13 +658,7 @@ export class UserAgentApplication {
// populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer
serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest);

// Cache nonce
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);
serverAuthenticationRequest.state = serverAuthenticationRequest.state;

// Cache account and authority
this.setAccountCache(account, serverAuthenticationRequest.state);
this.setAuthorityCache(serverAuthenticationRequest.state, acquireTokenAuthority.CanonicalAuthority);
this.updateCacheEntries(serverAuthenticationRequest, account);

// Construct the urlNavigate
let urlNavigate = serverAuthenticationRequest.createNavigateUrl(request.scopes) + Constants.response_mode_fragment;
Expand Down Expand Up @@ -1573,12 +1551,7 @@ export class UserAgentApplication {
this.logger.verbose("renewToken is called for scope:" + scope);
const frameHandle = this.addHiddenIFrame("msalRenewFrame" + scope);

// Cache account and authority
this.setAccountCache(account, serverAuthenticationRequest.state);
this.setAuthorityCache(serverAuthenticationRequest.state, serverAuthenticationRequest.authority);

// renew happens in iframe, so it keeps javascript context
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);
this.updateCacheEntries(serverAuthenticationRequest, account);
this.logger.verbose("Renew token Expected state: " + serverAuthenticationRequest.state);

// Build urlNavigate with "prompt=none" and navigate to URL in hidden iFrame
Expand All @@ -1602,12 +1575,7 @@ export class UserAgentApplication {
this.logger.info("renewidToken is called");
const frameHandle = this.addHiddenIFrame("msalIdTokenFrame");

// Cache account and authority
this.setAccountCache(account, serverAuthenticationRequest.state);
this.setAuthorityCache(serverAuthenticationRequest.state, serverAuthenticationRequest.authority);

// Cache nonce
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);
this.updateCacheEntries(serverAuthenticationRequest, account);

this.logger.verbose("Renew Idtoken Expected state: " + serverAuthenticationRequest.state);

Expand Down Expand Up @@ -1889,7 +1857,7 @@ export class UserAgentApplication {
}

this.cacheStorage.setItem(Constants.renewStatus + stateInfo.state, Constants.tokenRenewStatusCompleted);
this.cacheStorage.removeAcquireTokenEntries(authorityKey, acquireTokenAccountKey);
this.cacheStorage.removeAcquireTokenEntries();
// this is required if navigateToLoginRequestUrl=false
if (this.inCookie) {
this.cacheStorage.setItemCookie(authorityKey, "", -1);
Expand Down Expand Up @@ -2318,6 +2286,33 @@ export class UserAgentApplication {
this.cacheStorage.setItem(authorityKey, Utils.CanonicalizeUri(authority), this.inCookie);
}

/**
* Updates account, authority, and nonce in cache
* @param serverAuthenticationRequest
* @param account
*/
private updateCacheEntries(serverAuthenticationRequest: ServerRequestParameters, account: Account, loginStartPage?: any) {
// Cache account and authority
if (loginStartPage) {
// Cache the state, nonce, and login request data
this.cacheStorage.setItem(Constants.loginRequest, loginStartPage, this.inCookie);
this.cacheStorage.setItem(Constants.loginError, "");

this.cacheStorage.setItem(Constants.stateLogin, serverAuthenticationRequest.state, this.inCookie);
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);

this.cacheStorage.setItem(Constants.msalError, "");
this.cacheStorage.setItem(Constants.msalErrorDescription, "");
} else {
this.setAccountCache(account, serverAuthenticationRequest.state);
}
// Cache authorityKey
this.setAuthorityCache(serverAuthenticationRequest.state, serverAuthenticationRequest.authority);

// Cache nonce
this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie);
}

/**
* Returns the unique identifier for the logged in account
* @param account
Expand Down
10 changes: 5 additions & 5 deletions lib/msal-core/test/Storage.localStorage.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ describe("Local Storage", function () {
expect(cacheStorage.getItem(acquireTokenAccountKey)).to.be.eq(JSON.stringify(ACCOUNT));
expect(cacheStorage.getItem(authorityKey)).to.be.eq(validAuthority);

cacheStorage.removeAcquireTokenEntries(authorityKey, acquireTokenAccountKey);
cacheStorage.removeAcquireTokenEntries();

expect(cacheStorage.getItem(acquireTokenAccountKey)).to.be.null;
expect(cacheStorage.getItem(authorityKey)).to.be.null;
Expand All @@ -219,23 +219,23 @@ describe("Local Storage", function () {
window.localStorage.setItem(Constants.stateLogin, "stateLogin");
window.localStorage.setItem(Constants.idTokenKey, "idToken1");
window.localStorage.setItem(Constants.nonceIdToken, "idTokenNonce");
window.localStorage.setItem(Constants.renewStatus, "Completed");
window.localStorage.setItem(Constants.renewStatus + "|RANDOM_GUID", "Completed");

expect(cacheStorage.getItem(Constants.msalClientInfo)).to.be.eq("clientInfo");
expect(cacheStorage.getItem(Constants.tokenKeys)).to.be.eq("tokenKeys");
expect(cacheStorage.getItem(Constants.stateLogin)).to.be.eq("stateLogin");
expect(cacheStorage.getItem(Constants.idTokenKey)).to.be.eq("idToken1");
expect(cacheStorage.getItem(Constants.nonceIdToken)).to.be.eq("idTokenNonce");
expect(cacheStorage.getItem(Constants.renewStatus)).to.be.eq("Completed");
expect(cacheStorage.getItem(Constants.renewStatus + "|RANDOM_GUID")).to.be.eq("Completed");

cacheStorage.resetCacheItems();

expect(cacheStorage.getItem(Constants.msalClientInfo)).to.be.eq("");
expect(cacheStorage.getItem(Constants.tokenKeys)).to.be.eq("");
expect(cacheStorage.getItem(Constants.stateLogin)).to.be.eq("");
expect(cacheStorage.getItem(Constants.idTokenKey)).to.be.eq("");
expect(cacheStorage.getItem(Constants.nonceIdToken)).to.be.eq("");
expect(cacheStorage.getItem(Constants.renewStatus)).to.be.null;
expect(cacheStorage.getItem(Constants.stateLogin)).to.be.null;
});

});
Expand Down
8 changes: 4 additions & 4 deletions lib/msal-core/test/Storage.sessionStorage.spec.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -208,7 +208,7 @@ describe("Session Storage", function () {
expect(cacheStorage.getItem(acquireTokenAccountKey)).to.be.eq(JSON.stringify(ACCOUNT));
expect(cacheStorage.getItem(authorityKey)).to.be.eq(validAuthority);

cacheStorage.removeAcquireTokenEntries(authorityKey, acquireTokenAccountKey);
cacheStorage.removeAcquireTokenEntries();

expect(cacheStorage.getItem(acquireTokenAccountKey)).to.be.null;
expect(cacheStorage.getItem(authorityKey)).to.be.null;
Expand All @@ -220,23 +220,23 @@ describe("Session Storage", function () {
window.sessionStorage.setItem(Constants.stateLogin, "stateLogin");
window.sessionStorage.setItem(Constants.idTokenKey, "idToken1");
window.sessionStorage.setItem(Constants.nonceIdToken, "idTokenNonce");
window.sessionStorage.setItem(Constants.renewStatus, "Completed");
window.sessionStorage.setItem(Constants.renewStatus + "|RANDOM_GUID", "Completed");

expect(cacheStorage.getItem(Constants.msalClientInfo)).to.be.eq("clientInfo");
expect(cacheStorage.getItem(Constants.tokenKeys)).to.be.eq("tokenKeys");
expect(cacheStorage.getItem(Constants.stateLogin)).to.be.eq("stateLogin");
expect(cacheStorage.getItem(Constants.idTokenKey)).to.be.eq("idToken1");
expect(cacheStorage.getItem(Constants.nonceIdToken)).to.be.eq("idTokenNonce");
expect(cacheStorage.getItem(Constants.renewStatus)).to.be.eq("Completed");
expect(cacheStorage.getItem(Constants.renewStatus + "|RANDOM_GUID")).to.be.eq("Completed");

cacheStorage.resetCacheItems();

expect(cacheStorage.getItem(Constants.msalClientInfo)).to.be.eq("");
expect(cacheStorage.getItem(Constants.tokenKeys)).to.be.eq("");
expect(cacheStorage.getItem(Constants.stateLogin)).to.be.eq("");
expect(cacheStorage.getItem(Constants.idTokenKey)).to.be.eq("");
expect(cacheStorage.getItem(Constants.nonceIdToken)).to.be.eq("");
expect(cacheStorage.getItem(Constants.renewStatus)).to.be.null;
expect(cacheStorage.getItem(Constants.stateLogin)).to.be.null;
});

});
Expand Down

0 comments on commit 77f0470

Please sign in to comment.