Skip to content

Commit

Permalink
Refactor token cache entities into types (#6580)
Browse files Browse the repository at this point in the history
Refactors `CredentialEntity`, `IdTokenEntity`, `AccessTokenEntity`, and
`RefreshTokenEntity` to be Types rather than a Class and moves static
class methods into functions exported to the `CacheHelpers` namespace.

Making these types and separating the class methods from the type
definition allows us to read from the cache and directly use the value
without needing to first copy each key/value pair into an instance of
the class (the `toObject` helper function). Doing it this way also
results in a small bundle size improvement.

Reviews can be focused on the `msal-common/src/cache` folder. The rest
of the changes are repetitively changing references to the affected
functions
  • Loading branch information
tnorling authored Oct 24, 2023
1 parent f0092e2 commit 5f49a5e
Show file tree
Hide file tree
Showing 41 changed files with 991 additions and 1,451 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Refactor token cache entities to be defined as Types rather than Classes #6580",
"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": "Refactor token cache entities to be defined as Types rather than Classes #6580",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Refactor token cache entities to be defined as Types rather than Classes #6580",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
62 changes: 22 additions & 40 deletions lib/msal-browser/src/cache/BrowserCacheManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
PerformanceEvents,
IPerformanceClient,
StaticAuthorityOptions,
CacheHelpers,
} from "@azure/msal-common";
import { CacheOptions } from "../config/Configuration";
import {
Expand Down Expand Up @@ -234,17 +235,15 @@ export class BrowserCacheManager extends CacheManager {
if (credObj && credObj.hasOwnProperty("credentialType")) {
switch (credObj["credentialType"]) {
case CredentialType.ID_TOKEN:
if (IdTokenEntity.isIdTokenEntity(credObj)) {
if (CacheHelpers.isIdTokenEntity(credObj)) {
this.logger.trace(
"BrowserCacheManager:createKeyMaps - idToken found, saving key to token key map"
);
this.logger.tracePii(
`BrowserCacheManager:createKeyMaps - idToken with key: ${key} found, saving key to token key map`
);
const idTokenEntity = CacheManager.toObject(
new IdTokenEntity(),
credObj
);
const idTokenEntity =
credObj as IdTokenEntity;
const newKey =
this.updateCredentialCacheKey(
key,
Expand All @@ -266,22 +265,15 @@ export class BrowserCacheManager extends CacheManager {
break;
case CredentialType.ACCESS_TOKEN:
case CredentialType.ACCESS_TOKEN_WITH_AUTH_SCHEME:
if (
AccessTokenEntity.isAccessTokenEntity(
credObj
)
) {
if (CacheHelpers.isAccessTokenEntity(credObj)) {
this.logger.trace(
"BrowserCacheManager:createKeyMaps - accessToken found, saving key to token key map"
);
this.logger.tracePii(
`BrowserCacheManager:createKeyMaps - accessToken with key: ${key} found, saving key to token key map`
);
const accessTokenEntity =
CacheManager.toObject(
new AccessTokenEntity(),
credObj
);
credObj as AccessTokenEntity;
const newKey =
this.updateCredentialCacheKey(
key,
Expand All @@ -303,9 +295,7 @@ export class BrowserCacheManager extends CacheManager {
break;
case CredentialType.REFRESH_TOKEN:
if (
RefreshTokenEntity.isRefreshTokenEntity(
credObj
)
CacheHelpers.isRefreshTokenEntity(credObj)
) {
this.logger.trace(
"BrowserCacheManager:createKeyMaps - refreshToken found, saving key to token key map"
Expand All @@ -314,10 +304,7 @@ export class BrowserCacheManager extends CacheManager {
`BrowserCacheManager:createKeyMaps - refreshToken with key: ${key} found, saving key to token key map`
);
const refreshTokenEntity =
CacheManager.toObject(
new RefreshTokenEntity(),
credObj
);
credObj as RefreshTokenEntity;
const newKey =
this.updateCredentialCacheKey(
key,
Expand Down Expand Up @@ -718,7 +705,7 @@ export class BrowserCacheManager extends CacheManager {
}

const parsedIdToken = this.validateAndParseJson(value);
if (!parsedIdToken || !IdTokenEntity.isIdTokenEntity(parsedIdToken)) {
if (!parsedIdToken || !CacheHelpers.isIdTokenEntity(parsedIdToken)) {
this.logger.trace(
"BrowserCacheManager.getIdTokenCredential: called, no cache hit"
);
Expand All @@ -729,7 +716,7 @@ export class BrowserCacheManager extends CacheManager {
this.logger.trace(
"BrowserCacheManager.getIdTokenCredential: cache hit"
);
return CacheManager.toObject(new IdTokenEntity(), parsedIdToken);
return parsedIdToken as IdTokenEntity;
}

/**
Expand All @@ -738,7 +725,7 @@ export class BrowserCacheManager extends CacheManager {
*/
setIdTokenCredential(idToken: IdTokenEntity): void {
this.logger.trace("BrowserCacheManager.setIdTokenCredential called");
const idTokenKey = idToken.generateCredentialKey();
const idTokenKey = CacheHelpers.generateCredentialKey(idToken);

this.setItem(idTokenKey, JSON.stringify(idToken));

Expand All @@ -761,7 +748,7 @@ export class BrowserCacheManager extends CacheManager {
const parsedAccessToken = this.validateAndParseJson(value);
if (
!parsedAccessToken ||
!AccessTokenEntity.isAccessTokenEntity(parsedAccessToken)
!CacheHelpers.isAccessTokenEntity(parsedAccessToken)
) {
this.logger.trace(
"BrowserCacheManager.getAccessTokenCredential: called, no cache hit"
Expand All @@ -773,10 +760,7 @@ export class BrowserCacheManager extends CacheManager {
this.logger.trace(
"BrowserCacheManager.getAccessTokenCredential: cache hit"
);
return CacheManager.toObject(
new AccessTokenEntity(),
parsedAccessToken
);
return parsedAccessToken as AccessTokenEntity;
}

/**
Expand All @@ -787,7 +771,7 @@ export class BrowserCacheManager extends CacheManager {
this.logger.trace(
"BrowserCacheManager.setAccessTokenCredential called"
);
const accessTokenKey = accessToken.generateCredentialKey();
const accessTokenKey = CacheHelpers.generateCredentialKey(accessToken);
this.setItem(accessTokenKey, JSON.stringify(accessToken));

this.addTokenKey(accessTokenKey, CredentialType.ACCESS_TOKEN);
Expand All @@ -811,7 +795,7 @@ export class BrowserCacheManager extends CacheManager {
const parsedRefreshToken = this.validateAndParseJson(value);
if (
!parsedRefreshToken ||
!RefreshTokenEntity.isRefreshTokenEntity(parsedRefreshToken)
!CacheHelpers.isRefreshTokenEntity(parsedRefreshToken)
) {
this.logger.trace(
"BrowserCacheManager.getRefreshTokenCredential: called, no cache hit"
Expand All @@ -823,10 +807,7 @@ export class BrowserCacheManager extends CacheManager {
this.logger.trace(
"BrowserCacheManager.getRefreshTokenCredential: cache hit"
);
return CacheManager.toObject(
new RefreshTokenEntity(),
parsedRefreshToken
);
return parsedRefreshToken as RefreshTokenEntity;
}

/**
Expand All @@ -837,7 +818,8 @@ export class BrowserCacheManager extends CacheManager {
this.logger.trace(
"BrowserCacheManager.setRefreshTokenCredential called"
);
const refreshTokenKey = refreshToken.generateCredentialKey();
const refreshTokenKey =
CacheHelpers.generateCredentialKey(refreshToken);
this.setItem(refreshTokenKey, JSON.stringify(refreshToken));

this.addTokenKey(refreshTokenKey, CredentialType.REFRESH_TOKEN);
Expand Down Expand Up @@ -1804,7 +1786,7 @@ export class BrowserCacheManager extends CacheManager {
currentCacheKey: string,
credential: ValidCredentialType
): string {
const updatedCacheKey = credential.generateCredentialKey();
const updatedCacheKey = CacheHelpers.generateCredentialKey(credential);

if (currentCacheKey !== updatedCacheKey) {
const cacheItem = this.getItem(currentCacheKey);
Expand Down Expand Up @@ -1860,7 +1842,7 @@ export class BrowserCacheManager extends CacheManager {
| RedirectRequest
| PopupRequest
): Promise<void> {
const idTokenEntity = IdTokenEntity.createIdTokenEntity(
const idTokenEntity = CacheHelpers.createIdTokenEntity(
result.account?.homeAccountId,
result.account?.environment,
result.idToken,
Expand All @@ -1872,7 +1854,7 @@ export class BrowserCacheManager extends CacheManager {
if (request.claims) {
claimsHash = await this.cryptoImpl.hashString(request.claims);
}
const accessTokenEntity = AccessTokenEntity.createAccessTokenEntity(
const accessTokenEntity = CacheHelpers.createAccessTokenEntity(
result.account?.homeAccountId,
result.account.environment,
result.accessToken,
Expand All @@ -1881,7 +1863,7 @@ export class BrowserCacheManager extends CacheManager {
result.scopes.join(" "),
result.expiresOn?.getTime() || 0,
result.extExpiresOn?.getTime() || 0,
this.cryptoImpl,
base64Decode,
undefined, // refreshOn
result.tokenType as AuthenticationScheme,
undefined, // userAssertionHash
Expand Down
9 changes: 5 additions & 4 deletions lib/msal-browser/src/cache/TokenCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
Constants,
CacheRecord,
TokenClaims,
CacheHelpers,
} from "@azure/msal-common";
import { BrowserConfiguration } from "../config/Configuration";
import { SilentRequest } from "../request/SilentRequest";
Expand Down Expand Up @@ -294,7 +295,7 @@ export class TokenCache implements ITokenCache {
environment: string,
tenantId: string
): IdTokenEntity {
const idTokenEntity = IdTokenEntity.createIdTokenEntity(
const idTokenEntity = CacheHelpers.createIdTokenEntity(
homeAccountId,
environment,
idToken,
Expand Down Expand Up @@ -355,7 +356,7 @@ export class TokenCache implements ITokenCache {
response.expires_in + new Date().getTime() / 1000;
const extendedExpiresOn = options.extendedExpiresOn;

const accessTokenEntity = AccessTokenEntity.createAccessTokenEntity(
const accessTokenEntity = CacheHelpers.createAccessTokenEntity(
homeAccountId,
environment,
response.access_token,
Expand All @@ -364,7 +365,7 @@ export class TokenCache implements ITokenCache {
scopes,
expiresOn,
extendedExpiresOn,
this.cryptoObj
base64Decode
);

if (this.isBrowserEnvironment) {
Expand Down Expand Up @@ -399,7 +400,7 @@ export class TokenCache implements ITokenCache {
return null;
}

const refreshTokenEntity = RefreshTokenEntity.createRefreshTokenEntity(
const refreshTokenEntity = CacheHelpers.createRefreshTokenEntity(
homeAccountId,
environment,
response.refresh_token,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import {
invokeAsync,
createAuthError,
AuthErrorCodes,
CacheHelpers,
} from "@azure/msal-common";
import { BaseInteractionClient } from "./BaseInteractionClient";
import { BrowserConfiguration } from "../config/Configuration";
Expand Down Expand Up @@ -656,7 +657,7 @@ export class NativeInteractionClient extends BaseInteractionClient {
reqTimestamp: number
): void {
const cachedIdToken: IdTokenEntity | null =
IdTokenEntity.createIdTokenEntity(
CacheHelpers.createIdTokenEntity(
homeAccountIdentifier,
request.authority,
response.id_token || "",
Expand All @@ -675,7 +676,7 @@ export class NativeInteractionClient extends BaseInteractionClient {
const responseScopes = this.generateScopes(response, request);

const cachedAccessToken: AccessTokenEntity | null =
AccessTokenEntity.createAccessTokenEntity(
CacheHelpers.createAccessTokenEntity(
homeAccountIdentifier,
request.authority,
responseAccessToken,
Expand All @@ -684,7 +685,7 @@ export class NativeInteractionClient extends BaseInteractionClient {
responseScopes.printScopes(),
tokenExpirationSeconds,
0,
this.browserCrypto
base64Decode
);

const nativeCacheRecord = new CacheRecord(
Expand Down
Loading

0 comments on commit 5f49a5e

Please sign in to comment.