Fix native brokering use case for cache lookups (#7601)

sameerag · web-flow · commit 29bf8d4cdef8 · 2025-03-04T14:26:42.000-08:00
This PR fixes a bug in double brokering use cases, native brokering
should not be triggered when cache look up is set to `AccessToken` only.
diff --git a/change/@azure-msal-browser-a7e892b4-c765-4859-b48a-d360f4da38d1.json b/change/@azure-msal-browser-a7e892b4-c765-4859-b48a-d360f4da38d1.json
@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Fix double brokering bug: Native flow should not be triggered for cache only look up",
+  "packageName": "@azure/msal-browser",
+  "email": "sameera.gajjarapu@microsoft.com",
+  "dependentChangeType": "patch"
+}
diff --git a/lib/msal-browser/src/controllers/StandardController.ts b/lib/msal-browser/src/controllers/StandardController.ts
@@ -1519,7 +1519,8 @@ export class StandardController implements IController {
     public async acquireTokenNative(
         request: PopupRequest | SilentRequest | SsoSilentRequest,
         apiId: ApiId,
-        accountId?: string
+        accountId?: string,
+        cacheLookupPolicy?: CacheLookupPolicy
     ): Promise<AuthenticationResult> {
         this.logger.trace("acquireTokenNative called");
         if (!this.nativeExtensionProvider) {
@@ -1543,7 +1544,7 @@ export class StandardController implements IController {
             request.correlationId
         );
 
-        return nativeClient.acquireToken(request);
+        return nativeClient.acquireToken(request, cacheLookupPolicy);
     }
 
     /**
@@ -2223,6 +2224,7 @@ export class StandardController implements IController {
         silentRequest: CommonSilentFlowRequest,
         cacheLookupPolicy: CacheLookupPolicy
     ): Promise<AuthenticationResult> {
+        // if the cache policy is set to access_token only, we should not be hitting the native layer yet
         if (
             NativeMessageHandler.isPlatformBrokerAvailable(
                 this.config,
@@ -2237,7 +2239,9 @@ export class StandardController implements IController {
             );
             return this.acquireTokenNative(
                 silentRequest,
-                ApiId.acquireTokenSilent_silentFlow
+                ApiId.acquireTokenSilent_silentFlow,
+                silentRequest.account.nativeAccountId,
+                cacheLookupPolicy
             ).catch(async (e: AuthError) => {
                 // If native token acquisition fails for availability reasons fallback to web flow
                 if (e instanceof NativeAuthError && isFatalNativeAuthError(e)) {
@@ -2257,6 +2261,12 @@ export class StandardController implements IController {
             this.logger.verbose(
                 "acquireTokenSilent - attempting to acquire token from web flow"
             );
+            // add logs to identify embedded cache retrieval
+            if (cacheLookupPolicy === CacheLookupPolicy.AccessToken) {
+                this.logger.verbose(
+                    "acquireTokenSilent - cache lookup policy set to AccessToken, attempting to acquire token from local cache"
+                );
+            }
             return invokeAsync(
                 this.acquireTokenFromCache.bind(this),
                 PerformanceEvents.AcquireTokenFromCache,
diff --git a/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts b/lib/msal-browser/src/interaction_client/NativeInteractionClient.ts
@@ -52,6 +52,7 @@ import {
     TemporaryCacheKeys,
     NativeConstants,
     BrowserConstants,
+    CacheLookupPolicy,
 } from "../utils/BrowserConstants.js";
 import {
     NativeExtensionRequestBody,
@@ -157,7 +158,8 @@ export class NativeInteractionClient extends BaseInteractionClient {
      * @param request
      */
     async acquireToken(
-        request: PopupRequest | SilentRequest | SsoSilentRequest
+        request: PopupRequest | SilentRequest | SsoSilentRequest,
+        cacheLookupPolicy?: CacheLookupPolicy
     ): Promise<AuthenticationResult> {
         this.performanceClient.addQueueMeasurement(
             PerformanceEvents.NativeInteractionClientAcquireToken,
@@ -192,6 +194,12 @@ export class NativeInteractionClient extends BaseInteractionClient {
                 });
                 return result;
             } catch (e) {
+                if (cacheLookupPolicy === CacheLookupPolicy.AccessToken) {
+                    this.logger.info(
+                        "MSAL internal Cache does not contain tokens, return error as per cache policy"
+                    );
+                    throw e;
+                }
                 // continue with a native call for any and all errors
                 this.logger.info(
                     "MSAL internal Cache does not contain tokens, proceed to make a native call"
diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts
@@ -117,6 +117,7 @@ import {
     Configuration,
 } from "../../src/config/Configuration.js";
 import { buildAccountFromIdTokenClaims, buildIdToken } from "msal-test-utils";
+import { nativeConnectionNotEstablished } from "../../src/error/BrowserAuthErrorCodes.js";
 
 const cacheConfig = {
     temporaryCacheLocation: BrowserCacheLocation.SessionStorage,
@@ -5773,10 +5774,52 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                 expect(silentIframeSpy).toHaveBeenCalledTimes(0);
             });
 
-            it("Calls SilentCacheClient.acquireToken and SilentRefreshClient.acquireToken, and does not call SilentIframeClient.acquireToken if cache lookup throws and refresh token is expired when CacheLookupPolicy is set to AccessTokenAndRefreshToken", async () => {
+            it("Calls SilentCacheClient.acquireToken, and calls NativeInteractionClient.acquireToken when CacheLookupPolicy is set to AccessToken", async () => {
                 const silentCacheSpy: jest.SpyInstance = jest
                     .spyOn(SilentCacheClient.prototype, "acquireToken")
                     .mockRejectedValue(refreshRequiredCacheError);
+                const silentRefreshSpy = jest
+                    .spyOn(SilentRefreshClient.prototype, "acquireToken")
+                    .mockImplementation();
+                const silentIframeSpy = jest
+                    .spyOn(SilentIframeClient.prototype, "acquireToken")
+                    .mockImplementation();
+
+                const isPlatformBrokerAvailableSpy = jest
+                    .spyOn(NativeMessageHandler, "isPlatformBrokerAvailable")
+                    .mockReturnValue(true);
+                const nativeAcquireTokenSpy: jest.SpyInstance = jest
+                    .spyOn(NativeInteractionClient.prototype, "acquireToken")
+                    .mockImplementation();
+                const cacheAccount = testAccount;
+                cacheAccount.nativeAccountId = "nativeAccountId";
+
+                await expect(
+                    pca.acquireTokenSilent({
+                        scopes: ["openid"],
+                        account: cacheAccount,
+                        cacheLookupPolicy: CacheLookupPolicy.AccessToken,
+                    })
+                )
+                    .rejects.toThrow(BrowserAuthError)
+                    .catch((error) => {
+                        expect(error.errorCode).toBe(
+                            BrowserAuthErrorCodes.nativeConnectionNotEstablished
+                        );
+                    });
+                expect(silentCacheSpy).toHaveBeenCalledTimes(0);
+                expect(silentRefreshSpy).toHaveBeenCalledTimes(0);
+                expect(silentIframeSpy).toHaveBeenCalledTimes(0);
+                expect(nativeAcquireTokenSpy).toHaveBeenCalledTimes(0);
+
+                nativeAcquireTokenSpy.mockRestore();
+                isPlatformBrokerAvailableSpy.mockRestore();
+            });
+
+            it("Calls SilentRefreshClient.acquireToken, and does not call SilentCacheClient.acquireToken or SilentIframeClient.acquireToken if refresh token is expired when CacheLookupPolicy is set to RefreshToken", async () => {
+                const silentCacheSpy = jest
+                    .spyOn(SilentCacheClient.prototype, "acquireToken")
+                    .mockImplementation();
                 const silentRefreshSpy: jest.SpyInstance = jest
                     .spyOn(SilentRefreshClient.prototype, "acquireToken")
                     .mockRejectedValue(refreshRequiredServerError);
@@ -5788,15 +5831,57 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
                     pca.acquireTokenSilent({
                         scopes: ["openid"],
                         account: testAccount,
-                        cacheLookupPolicy:
-                            CacheLookupPolicy.AccessTokenAndRefreshToken,
+                        cacheLookupPolicy: CacheLookupPolicy.RefreshToken,
                     })
                 ).rejects.toThrow(refreshRequiredServerError);
-                expect(silentCacheSpy).toHaveBeenCalledTimes(1);
+                expect(silentCacheSpy).toHaveBeenCalledTimes(0);
                 expect(silentRefreshSpy).toHaveBeenCalledTimes(1);
                 expect(silentIframeSpy).toHaveBeenCalledTimes(0);
             });
 
+            it("Calls NativeInteractionClient.acquireToken when CacheLookupPolicy is set to AccessTokenAndRefreshToken", async () => {
+                const silentCacheSpy: jest.SpyInstance = jest
+                    .spyOn(SilentCacheClient.prototype, "acquireToken")
+                    .mockRejectedValue(refreshRequiredCacheError);
+                const silentRefreshSpy: jest.SpyInstance = jest
+                    .spyOn(SilentRefreshClient.prototype, "acquireToken")
+                    .mockRejectedValue(refreshRequiredServerError);
+                const silentIframeSpy = jest
+                    .spyOn(SilentIframeClient.prototype, "acquireToken")
+                    .mockImplementation();
+                const nativeAcquireTokenSpy: jest.SpyInstance = jest
+                    .spyOn(NativeInteractionClient.prototype, "acquireToken")
+                    .mockImplementation();
+
+                const cacheAccount = testAccount;
+                cacheAccount.nativeAccountId = "nativeAccountId";
+                const isPlatformBrokerAvailableSpy = jest
+                    .spyOn(NativeMessageHandler, "isPlatformBrokerAvailable")
+                    .mockReturnValue(true);
+                testAccount.nativeAccountId = "nativeAccountId";
+
+                await expect(
+                    pca.acquireTokenSilent({
+                        scopes: ["openid"],
+                        account: cacheAccount,
+                        cacheLookupPolicy:
+                            CacheLookupPolicy.AccessTokenAndRefreshToken,
+                    })
+                )
+                    .rejects.toThrow(BrowserAuthError)
+                    .catch((error) => {
+                        expect(error.errorCode).toBe(
+                            BrowserAuthErrorCodes.nativeConnectionNotEstablished
+                        );
+                    });
+                expect(silentCacheSpy).toHaveBeenCalledTimes(0);
+                expect(silentRefreshSpy).toHaveBeenCalledTimes(0);
+                expect(silentIframeSpy).toHaveBeenCalledTimes(0);
+                expect(nativeAcquireTokenSpy).toHaveBeenCalledTimes(0);
+                nativeAcquireTokenSpy.mockRestore();
+                isPlatformBrokerAvailableSpy.mockRestore();
+            });
+
             it("Calls SilentRefreshClient.acquireToken, and does not call SilentCacheClient.acquireToken or SilentIframeClient.acquireToken if refresh token is expired when CacheLookupPolicy is set to RefreshToken", async () => {
                 const silentCacheSpy = jest
                     .spyOn(SilentCacheClient.prototype, "acquireToken")
