Skip to content

Commit

Permalink
Clear claims cache for apps that switched their cache preferences [LT…
Browse files Browse the repository at this point in the history
…S] (#6356)

Some apps fetch tokens with high frequency and changing claims (new
claim per request). Prior to #6187, these tokens over flowed the local
cache. However, there is still the issue of stale tokens to be cleared
when the `claimsBasedCachingEnabled` feature is set to false, which is
not addressed.

This PR adds a new utility function `clearTokensAndKeysWithClaims()`
which will be triggered when the application is `initialized` to clear
any old tokens cached with claims.

The initial choice is to clear the cache at token acquisition. However
apps facing this issue will not be able to fetch tokens since the
`tokenKeys` cache entry is already large and freezes the app. Hence the
choice of adding this when the app is initialized and the extra
requirement of any application on the msal v2 to specifically call
`msal.initialize()` as a pre-requisite.

Applications are mandated to call `initialize()` for `msal v3` and we
are defaulting to `claimsBasedCachingEnabled: false` in v3. Hoping both
these settings will mitigate this issue from day one in the new version.

So here is the guidance:

`msal-browser v2`: App needs to explicitly set
`claimsBasedCachingEnabled: false` and call `initialize()` to use msal
to mitigate this issue.
`msal-browser v3`: MSAL JS should handle this in the back ground.

P.S: Currently this approach is in test and once approved will push the
changes to v3.

---------

Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
  • Loading branch information
sameerag and tnorling committed Aug 24, 2023
1 parent 55a0972 commit f85761c
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 0 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Clear tokens and token keys for claims based ATs when `claimsBasedCachingEnabled` is set to `false` #6356",
"packageName": "@azure/msal-browser",
"email": "sameera.gajjarapu@microsoft.com",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Clean up claims cached in tokens when claimsclaimsBasedCachingEnabled is set to false #6356",
"packageName": "@azure/msal-common",
"email": "sameera.gajjarapu@microsoft.com",
"dependentChangeType": "patch"
}
8 changes: 8 additions & 0 deletions lib/msal-browser/src/app/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,14 @@ export abstract class ClientApplication {
this.logger.verbose(e);
}
}

if(!this.config.cache.claimsBasedCachingEnabled) {
this.logger.verbose("Claims-based caching is disabled. Clearing the previous cache with claims");
const claimsTokensRemovalMeasurement = this.performanceClient.startMeasurement(PerformanceEvents.ClearTokensAndKeysWithClaims);
await this.browserStorage.clearTokensAndKeysWithClaims();
claimsTokensRemovalMeasurement.endMeasurement({success: true});
}

this.initialized = true;
this.eventHandler.emitEvent(EventType.INITIALIZE_END);

Expand Down
26 changes: 26 additions & 0 deletions lib/msal-browser/src/cache/BrowserCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -953,6 +953,32 @@ export class BrowserCacheManager extends CacheManager {
this.internalStorage.clear();
}

/**
* Clears all access tokes that have claims prior to saving the current one
* @param credential
* @returns
*/
async clearTokensAndKeysWithClaims(): Promise<void> {

this.logger.trace("BrowserCacheManager.clearTokensAndKeysWithClaims called");
const tokenKeys = this.getTokenKeys();

const removedAccessTokens: Array<Promise<void>> = [];
tokenKeys.accessToken.forEach((key: string) => {
// if the access token has claims in its key, remove the token key and the token
const credential = this.getAccessTokenCredential(key);
if(credential?.requestedClaimsHash && key.includes(credential.requestedClaimsHash.toLowerCase())) {
removedAccessTokens.push(this.removeAccessToken(key));
}
});
await Promise.all(removedAccessTokens);

// warn if any access tokens are removed
if(removedAccessTokens.length > 0) {
this.logger.warning(`${removedAccessTokens.length} access tokens with claims in the cache keys have been removed from the cache.`);
}
}

/**
* Add value to cookies
* @param cookieName
Expand Down
97 changes: 97 additions & 0 deletions lib/msal-browser/test/cache/BrowserCacheManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -944,6 +944,103 @@ describe("BrowserCacheManager tests", () => {
expect(browserLocalStorage.getAccessTokenCredential(testAccessTokenWithAuthScheme.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);
}
)

it("clearTokensWithClaimsInCache clears all access tokens with claims in tokenKeys", () => {
const testAT1 = AccessTokenEntity.createAccessTokenEntity("homeAccountId1", "environment", "secret1", "client-id", "tenantId", "openid", 1000, 1000, browserCrypto, 500, AuthenticationScheme.BEARER, "oboAssertion");
const testAT2 = AccessTokenEntity.createAccessTokenEntity("homeAccountId2", "environment", "secret2", "client-id", "tenantId", "openid", 1000, 1000, browserCrypto, 500, AuthenticationScheme.BEARER, "oboAssertion", undefined, "claims", "claims-hash");
const testAT3 = AccessTokenEntity.createAccessTokenEntity("homeAccountId3", "environment", "secret3", "client-id", "tenantId", "openid", 1000, 1000, browserCrypto, 500, AuthenticationScheme.BEARER, "oboAssertion", undefined, "claims");
const testAT4 = AccessTokenEntity.createAccessTokenEntity("homeAccountId4", "environment", "secret4", "client-id", "tenantId", "openid", 1000, 1000, browserCrypto, 500, AuthenticationScheme.BEARER, "oboAssertion", undefined, "claims", "claims-Hash");

expect(browserLocalStorage.getTokenKeys()).toStrictEqual({
idToken: [],
accessToken: [],
refreshToken: []
});

expect(browserSessionStorage.getTokenKeys()).toStrictEqual({
idToken: [],
accessToken: [],
refreshToken: []
});

browserLocalStorage.setAccessTokenCredential(testAT1);
browserSessionStorage.setAccessTokenCredential(testAT1);
browserLocalStorage.setAccessTokenCredential(testAT2);
browserSessionStorage.setAccessTokenCredential(testAT2);
browserLocalStorage.setAccessTokenCredential(testAT3);
browserSessionStorage.setAccessTokenCredential(testAT3);
browserLocalStorage.setAccessTokenCredential(testAT4);
browserSessionStorage.setAccessTokenCredential(testAT4);

expect(browserLocalStorage.getTokenKeys()).toStrictEqual({
idToken: [],
accessToken: [testAT1.generateCredentialKey(), testAT2.generateCredentialKey(), testAT3.generateCredentialKey(), testAT4.generateCredentialKey()],
refreshToken: []
});

expect(browserSessionStorage.getTokenKeys()).toStrictEqual({
idToken: [],
accessToken: [testAT1.generateCredentialKey(), testAT2.generateCredentialKey(), testAT3.generateCredentialKey(), testAT4.generateCredentialKey()],
refreshToken: []
});

expect(browserSessionStorage.getTokenKeys().accessToken.length).toBe(4);
expect(browserLocalStorage.getTokenKeys().accessToken.length).toBe(4);

expect(browserSessionStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toEqual(testAT1);
expect(browserSessionStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);;
expect(browserLocalStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toEqual(testAT1);
expect(browserLocalStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);

expect(browserSessionStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toEqual(testAT2);
expect(browserSessionStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);
expect(browserLocalStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toEqual(testAT2);
expect(browserLocalStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);

expect(browserSessionStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toEqual(testAT3);
expect(browserSessionStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);
expect(browserLocalStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toEqual(testAT3);
expect(browserLocalStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);

expect(browserSessionStorage.getAccessTokenCredential(testAT4.generateCredentialKey())).toEqual(testAT4);
expect(browserSessionStorage.getAccessTokenCredential(testAT4.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);
expect(browserLocalStorage.getAccessTokenCredential(testAT4.generateCredentialKey())).toEqual(testAT4);
expect(browserLocalStorage.getAccessTokenCredential(testAT4.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);

browserSessionStorage.clearTokensAndKeysWithClaims();
browserLocalStorage.clearTokensAndKeysWithClaims();

expect(browserSessionStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toEqual(testAT1);
expect(browserSessionStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);
expect(browserLocalStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toEqual(testAT1);
expect(browserLocalStorage.getAccessTokenCredential(testAT1.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);

expect(browserSessionStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toBeNull();
expect(browserLocalStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toBeNull();

expect(browserSessionStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toEqual(testAT3);
expect(browserSessionStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);
expect(browserLocalStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toEqual(testAT3);
expect(browserLocalStorage.getAccessTokenCredential(testAT3.generateCredentialKey())).toBeInstanceOf(AccessTokenEntity);

expect(browserSessionStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toBeNull();
expect(browserLocalStorage.getAccessTokenCredential(testAT2.generateCredentialKey())).toBeNull();

expect(browserLocalStorage.getTokenKeys()).toStrictEqual({
idToken: [],
accessToken: [testAT1.generateCredentialKey(), testAT3.generateCredentialKey()],
refreshToken: []
});

expect(browserSessionStorage.getTokenKeys()).toStrictEqual({
idToken: [],
accessToken: [testAT1.generateCredentialKey(), testAT3.generateCredentialKey()],
refreshToken: []
});

expect(browserSessionStorage.getTokenKeys().accessToken.length).toBe(2);
expect(browserLocalStorage.getTokenKeys().accessToken.length).toBe(2);
});
});

describe("RefreshTokenCredential", () => {
Expand Down
5 changes: 5 additions & 0 deletions lib/msal-common/src/telemetry/performance/PerformanceEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,11 @@ export enum PerformanceEvents {
UsernamePasswordClientAcquireToken = "usernamePasswordClientAcquireToken",

NativeMessageHandlerHandshake = "nativeMessageHandlerHandshake",

/**
* Cache operations
*/
ClearTokensAndKeysWithClaims = "clearTokensAndKeysWithClaims",
}

/**
Expand Down

0 comments on commit f85761c

Please sign in to comment.