Skip to content

[v5] Changes to Configuration - CacheOptions (Config #2) #7697

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
wants to merge 18 commits into
base: msal-v5
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion .github/ISSUE_TEMPLATE/bug_report.yml
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,6 @@ body:
},
cache: {
cacheLocation: "sessionStorage"
storeAuthStateInCookie: false
}
}
validations:
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "major",
"comment": "Configuration changes to CacheOptions #7697",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Deprecate claimsBasedCachingEnabled as part of Configuration change #7697",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
3 changes: 0 additions & 3 deletions lib/msal-angular/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,6 @@ import { PublicClientApplication, InteractionType, BrowserCacheLocation } from "
},
cache: {
cacheLocation: BrowserCacheLocation.LocalStorage,
storeAuthStateInCookie: true, // set to true for IE 11
},
system: {
loggerOptions: {
Expand Down Expand Up @@ -261,7 +260,6 @@ fetch("/assets/configuration.json")
},
"cache": {
"cacheLocation": "localStorage",
"storeAuthStateInCookie": true
}
},
"guard": {
Expand Down Expand Up @@ -471,7 +469,6 @@ export class AppModule {}
},
"cache": {
"cacheLocation": "localStorage",
"storeAuthStateInCookie": true
}
},
"guard": {
Expand Down
1 change: 0 additions & 1 deletion lib/msal-angular/docs/events.md
Original file line number Diff line number Diff line change
Expand Up @@ -222,7 +222,6 @@ import { PublicClientApplication, InteractionType, BrowserCacheLocation } from "
},
cache: {
cacheLocation : BrowserCacheLocation.LocalStorage,
storeAuthStateInCookie: true, // set to true for IE 11
},
system: {
loggerOptions: {
Expand Down
2 changes: 0 additions & 2 deletions lib/msal-angular/docs/initialization.md
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@ import { PublicClientApplication, InteractionType, BrowserCacheLocation } from "
},
cache: {
cacheLocation : BrowserCacheLocation.LocalStorage,
storeAuthStateInCookie: true, // set to true for IE 11
},
system: {
loggerOptions: {
Expand Down Expand Up @@ -115,7 +114,6 @@ import { PublicClientApplication, InteractionType, BrowserCacheLocation } from "
},
cache: {
cacheLocation : BrowserCacheLocation.LocalStorage,
storeAuthStateInCookie: true, // set to true for IE 11
},
system: {
loggerOptions: {
Expand Down
3 changes: 3 additions & 0 deletions lib/msal-browser/docs/caching.md
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,9 @@ To faciliate efficient token acquisition while maintaining a good UX, MSAL cache
> :bulb: The authorization code is only stored in memory and will be discarded after redeeming it for tokens.

## Warning :warning:

**NOTE: `temporaryCacheLocation` is deprecated as of MSAL v5 and will be removed in a future release.**

Overriding `temporaryCacheLocation` should be done with caution. Specifically when choosing `localStorage`. Interaction in more than one tab/window will not be supported and you may receive `interaction_in_progress` errors unexpectedly. This is an escape hatch, not a fully supported feature.

When using MSAL.js with the default configuration in a scenario where the user is redirected after successful authentication in a new window or tab, the OAuth 2.0 Authorization Code with PKCE flow will be interrupted. In this case, the original window or tab where the authentication state (code verifier and challenge) are stored, will be lost, and the authentication flow will fail.
Expand Down
9 changes: 2 additions & 7 deletions lib/msal-browser/docs/configuration.md
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,6 @@ const msalConfig = {
cache: {
cacheLocation: "sessionStorage",
temporaryCacheLocation: "sessionStorage",
storeAuthStateInCookie: false,
secureCookies: false,
claimsBasedCachingEnabled: true,
},
system: {
Expand Down Expand Up @@ -97,11 +95,8 @@ const msalInstance = new PublicClientApplication(msalConfig);
| Option | Description | Format | Default Value |
| --------------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------------------------------------------------------------------------------------- | --------------------------------------------------- |
| `cacheLocation` | Location of token cache in browser. | String value that must be one of the following: `"sessionStorage"`, `"localStorage"`, `"memoryStorage"` | `sessionStorage` |
| `temporaryCacheLocation` | Location of temporary cache in browser. This option should only be changed for specific edge cases. Please refer to [caching](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/caching.md#cached-artifacts) for more. | String value that must be one of the following: `"sessionStorage"`, `"localStorage"`, `"memoryStorage"` | `sessionStorage` |
| `storeAuthStateInCookie` | If true, stores cache items in cookies as well as browser cache. Should be set to true for use cases using IE. | boolean | `false` |
| `secureCookies` | If true and `storeAuthStateInCookies` is also enabled, MSAL adds the `Secure` flag to the browser cookie so it can only be sent over HTTPS. | boolean | `false` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Off the topic, but related. Can we make sure the one cookie we set for KMSI is the only one we manage and add it to the docs under cookies MSAL JS manages.. etc. Also link to the migration guide on the discontinuation of existing cookies.

| `cacheMigrationEnabled` | If true, cache entries from older versions of MSAL will be updated to conform to the latest cache schema on startup. If your application has not been recently updated to a new version of MSAL.js you can safely turn this off. In the event old cache entries are not migrated it may result in a cache miss when attempting to retrieve accounts or tokens and affected users may need to re-authenticate to get up to date. | boolean | `true` when using `localStorage`, `false` otherwise |
| `claimsBasedCachingEnabled` | If `true`, access tokens will be cached under a key containing the hash of the requested claims string, resulting in a cache miss and new network token request when the same token request is made with different or missing claims. If set to `false`, tokens will be cached without claims, but all requests containing claims will go to the network and overwrite any previously cached token with the same scopes. | boolean | `false` |
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let us ensure we test this, that we never cache claims. I want to make sure we have Loop who had this bug before, and Teams who I think are setting this to false are not broken with v5.

| `temporaryCacheLocation` | Location of temporary cache in browser. This option should only be changed for specific edge cases. Please refer to [caching](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-browser/docs/caching.md#cached-artifacts) for more. **Note: This is deprecated and will be removed in a future release.** | String value that must be one of the following: `"sessionStorage"`, `"localStorage"`, `"memoryStorage"` | `sessionStorage` |
| `claimsBasedCachingEnabled` | If `true`, access tokens will be cached under a key containing the hash of the requested claims string, resulting in a cache miss and new network token request when the same token request is made with different or missing claims. If set to `false`, tokens will be cached without claims, but all requests containing claims will go to the network and overwrite any previously cached token with the same scopes. **Note: This is deprecated and will be removed in a future release.** | boolean | `false` |

See [Caching in MSAL](./caching.md) for more.

Expand Down
31 changes: 0 additions & 31 deletions lib/msal-browser/src/cache/BrowserCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -69,8 +69,6 @@ import { EventHandler } from "../event/EventHandler.js";

/**
* This class implements the cache storage interface for MSAL through browser local or session storage.
* Cookies are only used if storeAuthStateInCookie is true, and are only used for
* parameters such as state and nonce, generally.
*/
export class BrowserCacheManager extends CacheManager {
// Cache configuration, either set by user or default values.
Expand Down Expand Up @@ -889,21 +887,10 @@ export class BrowserCacheManager extends CacheManager {

/**
* Gets cache item with given key.
* Will retrieve from cookies if storeAuthStateInCookie is set to true.
* @param key
*/
getTemporaryCache(cacheKey: string, generateKey?: boolean): string | null {
const key = generateKey ? this.generateCacheKey(cacheKey) : cacheKey;
if (this.cacheConfig.storeAuthStateInCookie) {
const itemCookie = this.cookieStorage.getItem(key);
if (itemCookie) {
this.logger.trace(
"BrowserCacheManager.getTemporaryCache: storeAuthStateInCookies set to true, retrieving from cookies"
);
return itemCookie;
}
}

const value = this.temporaryCacheStorage.getItem(key);
if (!value) {
// If temp cache item not found in session/memory, check local storage for items set by old versions
Expand Down Expand Up @@ -932,8 +919,6 @@ export class BrowserCacheManager extends CacheManager {

/**
* Sets the cache item with the key and value given.
* Stores in cookie if storeAuthStateInCookie is set to true.
* This can cause cookie overflow if used incorrectly.
* @param key
* @param value
*/
Expand All @@ -943,14 +928,7 @@ export class BrowserCacheManager extends CacheManager {
generateKey?: boolean
): void {
const key = generateKey ? this.generateCacheKey(cacheKey) : cacheKey;

this.temporaryCacheStorage.setItem(key, value);
if (this.cacheConfig.storeAuthStateInCookie) {
this.logger.trace(
"BrowserCacheManager.setTemporaryCache: storeAuthStateInCookie set to true, setting item cookie"
);
this.cookieStorage.setItem(key, value, undefined);
}
}

/**
Expand All @@ -963,17 +941,10 @@ export class BrowserCacheManager extends CacheManager {

/**
* Removes the temporary cache item with the given key.
* Will also clear the cookie item if storeAuthStateInCookie is set to true.
* @param key
*/
removeTemporaryItem(key: string): void {
this.temporaryCacheStorage.removeItem(key);
if (this.cacheConfig.storeAuthStateInCookie) {
this.logger.trace(
"BrowserCacheManager.removeItem: storeAuthStateInCookie is true, clearing item cookie"
);
this.cookieStorage.removeItem(key);
}
}

/**
Expand Down Expand Up @@ -1374,8 +1345,6 @@ export const DEFAULT_BROWSER_CACHE_MANAGER = (
const cacheOptions: Required<CacheOptions> = {
cacheLocation: BrowserCacheLocation.MemoryStorage,
temporaryCacheLocation: BrowserCacheLocation.MemoryStorage,
storeAuthStateInCookie: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
};
return new BrowserCacheManager(
Expand Down
19 changes: 4 additions & 15 deletions lib/msal-browser/src/config/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -127,18 +127,14 @@ export type CacheOptions = {
*/
cacheLocation?: BrowserCacheLocation | string;
/**
* @deprecated
* temporaryCacheLocation is deprecated and will be removed in a future release.
* Used to specify the temporaryCacheLocation user wants to set. Valid values are "localStorage", "sessionStorage" and "memoryStorage".
*/
temporaryCacheLocation?: BrowserCacheLocation | string;
/**
* If set, MSAL stores the auth request state required for validation of the auth flows in the browser cookies. By default this flag is set to false.
*/
storeAuthStateInCookie?: boolean;
/**
* 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;
/**
* @deprecated
* claimsBasedCachingEnabled is deprecated and will be removed in a future release.
* Flag that determines whether access tokens are stored based on requested claims
*/
claimsBasedCachingEnabled?: boolean;
Expand Down Expand Up @@ -291,13 +287,6 @@ export function buildConfiguration(
const DEFAULT_CACHE_OPTIONS: Required<CacheOptions> = {
cacheLocation: BrowserCacheLocation.SessionStorage,
temporaryCacheLocation: BrowserCacheLocation.SessionStorage,
storeAuthStateInCookie: false,
// Default cache migration to true if cache location is localStorage since entries are preserved across tabs/windows. Migration has little to no benefit in sessionStorage and memoryStorage
cacheMigrationEnabled:
userInputCache &&
userInputCache.cacheLocation === BrowserCacheLocation.LocalStorage
? true
: false,
claimsBasedCachingEnabled: false,
};

Expand Down
2 changes: 0 additions & 2 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -257,8 +257,6 @@ export class StandardController implements IController {
const nativeCacheOptions: Required<CacheOptions> = {
cacheLocation: BrowserCacheLocation.MemoryStorage,
temporaryCacheLocation: BrowserCacheLocation.MemoryStorage,
storeAuthStateInCookie: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
};
this.nativeInternalStorage = new BrowserCacheManager(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ export const BrowserConfigurationAuthErrorMessages = {
[BrowserConfigurationAuthErrorCodes.stubbedPublicClientApplicationCalled]:
"Stub instance of Public Client Application was called. If using msal-react, please ensure context is not used without a provider. For more visit: aka.ms/msaljs/browser-errors",
[BrowserConfigurationAuthErrorCodes.inMemRedirectUnavailable]:
"Redirect cannot be supported. In-memory storage was selected and storeAuthStateInCookie=false, which would cause the library to be unable to handle the incoming hash. If you would like to use the redirect API, please use session/localStorage or set storeAuthStateInCookie=true.",
"Redirect cannot be supported. In-memory storage was selected, which would cause the library to be unable to handle the incoming hash. If you would like to use the redirect API, please use session/localStorage.",
};

/**
Expand Down
7 changes: 2 additions & 5 deletions lib/msal-browser/src/utils/BrowserUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -175,11 +175,8 @@ export function redirectPreflightCheck(
): void {
preflightCheck(initialized);
blockRedirectInIframe(config.system.allowRedirectInIframe);
// Block redirects if memory storage is enabled but storeAuthStateInCookie is not
if (
config.cache.cacheLocation === BrowserCacheLocation.MemoryStorage &&
!config.cache.storeAuthStateInCookie
) {
// Block redirects if memory storage is enabled
if (config.cache.cacheLocation === BrowserCacheLocation.MemoryStorage) {
throw createBrowserConfigurationAuthError(
BrowserConfigurationAuthErrorCodes.inMemRedirectUnavailable
);
Expand Down
7 changes: 1 addition & 6 deletions lib/msal-browser/test/app/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,6 @@ import { INTERACTION_TYPE } from "../../src/utils/BrowserConstants.js";
const cacheConfig = {
temporaryCacheLocation: BrowserCacheLocation.SessionStorage,
cacheLocation: BrowserCacheLocation.SessionStorage,
storeAuthStateInCookie: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
};

Expand Down Expand Up @@ -1934,14 +1932,13 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
}
});

it("throws error if cacheLocation is Memory Storage and storeAuthStateInCookie is false", async () => {
it("throws error if cacheLocation is Memory Storage", async () => {
pca = new PublicClientApplication({
auth: {
clientId: TEST_CONFIG.MSAL_CLIENT_ID,
},
cache: {
cacheLocation: BrowserCacheLocation.MemoryStorage,
storeAuthStateInCookie: false,
},
system: {
allowPlatformBroker: false,
Expand Down Expand Up @@ -7290,8 +7287,6 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
{
cacheLocation: BrowserCacheLocation.LocalStorage,
temporaryCacheLocation: BrowserCacheLocation.SessionStorage,
storeAuthStateInCookie: false,
cacheMigrationEnabled: false,
claimsBasedCachingEnabled: false,
},
new CryptoOps(new Logger({})),
Expand Down
Loading