Reduce unnecessary cache lookups (#6753)

When looking up tokens in the cache we:
1. Get and parse: idToken, accessToken, refreshToken, account and
appMetadata
2. Evaluate the accessToken and if not found or expired throw
3. Catch error and lookup the refreshToken again and attempt to exchange
it over the network

There's 2 ways to improve this pattern, both addressed in this PR:
1. Don't retrieve the refresh token until it's actually needed (if and
when the accessToken needs to be refreshed)
2. Don't retrieve idToken, account and appMetadata until they're needed
(accessToken lookup was successful)

This saves 1 cache lookup on all calls and an additional 3 cache lookups
on calls that fail to return an access token from the cache

change/@azure-msal-common-5d3e5b0c-ebb6-4c61-b410-108529331ca9.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Don't lookup tokens until they are needed",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

lib/msal-browser/test/interaction_client/NativeInteractionClient.spec.ts

@@ -216,28 +216,28 @@ describe("NativeInteractionClient Tests", () => {
     });
 
     describe("acquireTokensFromInternalCache Tests", () => {
-        const response: AuthenticationResult = {
-            authority: TEST_CONFIG.validAuthority,
-            uniqueId: TEST_ACCOUNT_INFO.localAccountId,
-            tenantId: TEST_ACCOUNT_INFO.tenantId,
-            scopes: TEST_CONFIG.DEFAULT_SCOPES,
-            account: TEST_ACCOUNT_INFO,
-            idToken: TEST_TOKENS.IDTOKEN_V2,
-            accessToken: TEST_TOKENS.ACCESS_TOKEN,
-            idTokenClaims: ID_TOKEN_CLAIMS,
-            fromCache: true,
-            correlationId: RANDOM_TEST_GUID,
-            expiresOn: new Date(Number(testAccessTokenEntity.expiresOn) * 1000),
-            tokenType: AuthenticationScheme.BEARER,
-        };
-
-        sinon
-            .stub(CacheManager.prototype, "getBaseAccountInfo")
-            .returns(TEST_ACCOUNT_INFO);
-
-        sinon
-            .stub(CacheManager.prototype, "readCacheRecord")
-            .returns(testCacheRecord);
+        beforeEach(() => {
+            jest.spyOn(
+                CacheManager.prototype,
+                "getBaseAccountInfo"
+            ).mockReturnValue(TEST_ACCOUNT_INFO);
+
+            jest.spyOn(
+                CacheManager.prototype,
+                "getAccessToken"
+            ).mockReturnValue(testCacheRecord.accessToken);
+            jest.spyOn(CacheManager.prototype, "getIdToken").mockReturnValue(
+                testCacheRecord.idToken
+            );
+            jest.spyOn(
+                CacheManager.prototype,
+                "readAppMetadataFromCache"
+            ).mockReturnValue(testCacheRecord.appMetadata);
+            jest.spyOn(
+                CacheManager.prototype,
+                "readAccountFromCache"
+            ).mockReturnValue(testCacheRecord.account);
+        });
 
         it("Tokens found in cache", async () => {
             const response = await nativeInteractionClient.acquireToken({

lib/msal-common/src/cache/CacheManager.ts

@@ -49,7 +49,6 @@ import { BaseAuthRequest } from "../request/BaseAuthRequest";
 import { Logger } from "../logger/Logger";
 import { name, version } from "../packageMetadata";
 import { StoreInCache } from "../request/StoreInCache";
-import { getTenantFromAuthorityString } from "../authority/Authority";
 import { getAliasesFromStaticSources } from "../authority/AuthorityMetadata";
 import { StaticAuthorityOptions } from "../authority/AuthorityOptions";
 import { TokenClaims } from "../account/TokenClaims";
@@ -1128,59 +1127,6 @@ export abstract class CacheManager implements ICacheManager {
         return true;
     }
 
-    /**
-     * Retrieve the cached credentials into a cacherecord
-     * @param account {AccountInfo}
-     * @param request {BaseAuthRequest}
-     * @param environment {string}
-     * @param performanceClient {?IPerformanceClient}
-     * @param correlationId {?string}
-     */
-    readCacheRecord(
-        account: AccountInfo,
-        request: BaseAuthRequest,
-        environment: string,
-        performanceClient?: IPerformanceClient,
-        correlationId?: string
-    ): CacheRecord {
-        // Use authority tenantId for cache lookup filter if it's defined, otherwise use tenantId from account passed in
-        const requestTenantId =
-            account.tenantId || getTenantFromAuthorityString(request.authority);
-        const tokenKeys = this.getTokenKeys();
-        const cachedAccount = this.readAccountFromCache(account);
-        const cachedIdToken = this.getIdToken(
-            account,
-            tokenKeys,
-            requestTenantId,
-            performanceClient,
-            correlationId
-        );
-        const cachedAccessToken = this.getAccessToken(
-            account,
-            request,
-            tokenKeys,
-            requestTenantId,
-            performanceClient,
-            correlationId
-        );
-        const cachedRefreshToken = this.getRefreshToken(
-            account,
-            false,
-            tokenKeys,
-            performanceClient,
-            correlationId
-        );
-        const cachedAppMetadata = this.readAppMetadataFromCache(environment);
-
-        return {
-            account: cachedAccount,
-            idToken: cachedIdToken,
-            accessToken: cachedAccessToken,
-            refreshToken: cachedRefreshToken,
-            appMetadata: cachedAppMetadata,
-        };
-    }
-
     /**
      * Retrieve AccountEntity from cache
      * @param account

lib/msal-common/src/client/SilentFlowClient.ts

@@ -23,6 +23,7 @@ import { checkMaxAge, extractTokenClaims } from "../account/AuthToken";
 import { TokenClaims } from "../account/TokenClaims";
 import { PerformanceEvents } from "../telemetry/performance/PerformanceEvent";
 import { invokeAsync } from "../utils/FunctionWrappers";
+import { getTenantFromAuthorityString } from "../authority/Authority";
 
 /** @internal */
 export class SilentFlowClient extends BaseClient {
@@ -118,18 +119,20 @@ export class SilentFlowClient extends BaseClient {
             );
         }
 
-        const environment =
-            request.authority || this.authority.getPreferredCache();
-
-        const cacheRecord = this.cacheManager.readCacheRecord(
+        const requestTenantId =
+            request.account.tenantId ||
+            getTenantFromAuthorityString(request.authority);
+        const tokenKeys = this.cacheManager.getTokenKeys();
+        const cachedAccessToken = this.cacheManager.getAccessToken(
             request.account,
             request,
-            environment,
+            tokenKeys,
+            requestTenantId,
             this.performanceClient,
             request.correlationId
         );
 
-        if (!cacheRecord.accessToken) {
+        if (!cachedAccessToken) {
             // must refresh due to non-existent access_token
             this.setCacheOutcome(
                 CacheOutcome.NO_CACHED_ACCESS_TOKEN,
@@ -139,9 +142,9 @@ export class SilentFlowClient extends BaseClient {
                 ClientAuthErrorCodes.tokenRefreshRequired
             );
         } else if (
-            TimeUtils.wasClockTurnedBack(cacheRecord.accessToken.cachedAt) ||
+            TimeUtils.wasClockTurnedBack(cachedAccessToken.cachedAt) ||
             TimeUtils.isTokenExpired(
-                cacheRecord.accessToken.expiresOn,
+                cachedAccessToken.expiresOn,
                 this.config.systemOptions.tokenRenewalOffsetSeconds
             )
         ) {
@@ -154,15 +157,32 @@ export class SilentFlowClient extends BaseClient {
                 ClientAuthErrorCodes.tokenRefreshRequired
             );
         } else if (
-            cacheRecord.accessToken.refreshOn &&
-            TimeUtils.isTokenExpired(cacheRecord.accessToken.refreshOn, 0)
+            cachedAccessToken.refreshOn &&
+            TimeUtils.isTokenExpired(cachedAccessToken.refreshOn, 0)
         ) {
             // must refresh (in the background) due to the refresh_in value
             lastCacheOutcome = CacheOutcome.PROACTIVELY_REFRESHED;
 
             // don't throw ClientAuthError.createRefreshRequiredError(), return cached token instead
         }
 
+        const environment =
+            request.authority || this.authority.getPreferredCache();
+        const cacheRecord: CacheRecord = {
+            account: this.cacheManager.readAccountFromCache(request.account),
+            accessToken: cachedAccessToken,
+            idToken: this.cacheManager.getIdToken(
+                request.account,
+                tokenKeys,
+                requestTenantId,
+                this.performanceClient,
+                request.correlationId
+            ),
+            refreshToken: null,
+            appMetadata:
+                this.cacheManager.readAppMetadataFromCache(environment),
+        };
+
         this.setCacheOutcome(lastCacheOutcome, request.correlationId);
 
         if (this.config.serverTelemetryManager) {