Skip to content

Commit

Permalink
Make claims-based caching configurable (#6163)
Browse files Browse the repository at this point in the history
This PR:
- Sets ability to cache based on requested claims as a configurable
option under `CacheOptions.claimsBasedCachinEnabled`
- Changes default behavior so in v3 claims-based caching is off by
default and any request containing claims will go to the network
- Provides a mitigation to the issue where requesting different claims
results in a cache miss and a cached token accumulation with no
practical way to clear unused tokens
- Backport for LTS packages (msal-node v1/msal-browser v2) in progress
  • Loading branch information
hectormmg authored Jul 5, 2023
1 parent faae9de commit 4ccef1b
Show file tree
Hide file tree
Showing 26 changed files with 895 additions and 210 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Make claims-based caching configurable #6163",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Make claims-based caching configurable #6163",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "prerelease",
"comment": "Make claims-based caching configurable #6163",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
113 changes: 60 additions & 53 deletions lib/msal-browser/docs/configuration.md

Large diffs are not rendered by default.

19 changes: 19 additions & 0 deletions lib/msal-browser/docs/v2-migration.md
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,25 @@ const msalConfig = {
const msalInstance = await msal.PublicClientApplication.createPublicClientApplication(msalConfig);
```

### Claims-based caching

In MSAL v2.x, adding claims to a request will result in a hash of the requested claims string being added to the token cache key by default. This implies that MSAL 2.x caches and matches tokens based on claims by default. In MSAL v3.x, this behavior is no longer the default. MSAL v3.x default behavior is to go to the network to refresh a token every time claims are requested, regardless of whether the token has been cached previously and is still valid. Then, after going to the network, the token received overwrites the cached token in case a silent request without claims is executed later. In order to enable claims-based caching in MSAL v3.x to maintain the same behavior as in MSAL v2.x, developers must use the `cacheOptions.claimsBasedCachingEnabled` configuration flag set to true in the Client Application configuration object:

```typescript
const msalConfig = {
auth: {
...
},
...
cache: {
claimsBasedCachingEnabled: true
}
}

const msalInstance = new msal.PublicClientApplication(msalConfig);
await msalInstance.initialize();
```

All other APIs are backward compatible with [MSAL v2.x](../../msal-browser/). It is recommended to take a look at the [default sample](../../../samples/msal-browser-samples/VanillaJSTestApp2.0) to see a working example of MSAL v3.0.

### Crypto
Expand Down
1 change: 1 addition & 0 deletions lib/msal-browser/src/cache/BrowserCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1891,6 +1891,7 @@ export const DEFAULT_BROWSER_CACHE_MANAGER = (
storeAuthStateInCookie: false,
secureCookies: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
};
return new BrowserCacheManager(
clientId,
Expand Down
7 changes: 6 additions & 1 deletion lib/msal-browser/src/config/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,10 @@ export type CacheOptions = {
* If set, MSAL will attempt to migrate cache entries from older versions on initialization. By default this flag is set to true if cacheLocation is localStorage, otherwise false.
*/
cacheMigrationEnabled?: boolean;
/**
* Flag that determines whether access tokens are stored based on requested claims
*/
claimsBasedCachingEnabled?: boolean;
};

export type BrowserSystemOptions = SystemOptions & {
Expand Down Expand Up @@ -255,6 +259,7 @@ export function buildConfiguration(
userInputCache.cacheLocation === BrowserCacheLocation.LocalStorage
? true
: false,
claimsBasedCachingEnabled: false,
};

// Default logger options for browser
Expand Down Expand Up @@ -290,7 +295,7 @@ export function buildConfiguration(
nativeBrokerHandshakeTimeout:
userInputSystem?.nativeBrokerHandshakeTimeout ||
DEFAULT_NATIVE_BROKER_HANDSHAKE_TIMEOUT_MS,
pollIntervalMilliseconds: BrowserConstants.DEFAULT_POLL_INTERVAL_MS
pollIntervalMilliseconds: BrowserConstants.DEFAULT_POLL_INTERVAL_MS,
};

const providedSystemOptions: BrowserSystemOptions = {
Expand Down
6 changes: 2 additions & 4 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,7 @@ export class StandardController implements IController {

// Initialize the crypto class.
this.browserCrypto = this.isBrowserEnvironment
? new CryptoOps(
this.logger,
this.performanceClient
)
? new CryptoOps(this.logger, this.performanceClient)
: DEFAULT_CRYPTO_IMPLEMENTATION;

this.eventHandler = new EventHandler(this.logger, this.browserCrypto);
Expand All @@ -226,6 +223,7 @@ export class StandardController implements IController {
storeAuthStateInCookie: false,
secureCookies: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
};
this.nativeInternalStorage = new BrowserCacheManager(
this.config.auth.clientId,
Expand Down
20 changes: 15 additions & 5 deletions lib/msal-browser/src/interaction_client/BaseInteractionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -178,8 +178,13 @@ export abstract class BaseInteractionClient {
);
}

// Set requested claims hash if claims were requested
if (request.claims && !StringUtils.isEmpty(request.claims)) {
// Set requested claims hash if claims-based caching is enabled and claims were requested
if (
this.config.cache.claimsBasedCachingEnabled &&
request.claims &&
// Checks for empty stringified object "{}" which doesn't qualify as requested claims
!StringUtils.isEmptyObj(request.claims)
) {
validatedRequest.requestedClaimsHash =
await this.browserCrypto.hashString(request.claims);
}
Expand Down Expand Up @@ -210,10 +215,15 @@ export abstract class BaseInteractionClient {
* If authority provided in the request does not match environment/authority specified
* in the account or MSAL config, we throw an error.
*/
async validateRequestAuthority(authority: string, account: AccountInfo): Promise<void> {
const discoveredAuthority = await this.getDiscoveredAuthority(authority);
async validateRequestAuthority(
authority: string,
account: AccountInfo
): Promise<void> {
const discoveredAuthority = await this.getDiscoveredAuthority(
authority
);

if(!discoveredAuthority.isAlias(account.environment)) {
if (!discoveredAuthority.isAlias(account.environment)) {
throw ClientConfigurationError.createAuthorityMismatchError();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -278,6 +278,10 @@ export abstract class StandardInteractionClient extends BaseInteractionClient {
logLevel: logger.logLevel,
correlationId: this.correlationId,
},
cacheOptions: {
claimsBasedCachingEnabled:
this.config.cache.claimsBasedCachingEnabled,
},
cryptoInterface: this.browserCrypto,
networkInterface: this.networkClient,
storageInterface: this.browserStorage,
Expand Down
Loading

0 comments on commit 4ccef1b

Please sign in to comment.