Don't use temporary cache for silent & popup flows (#6586)

Silent and Popup flows don't need to use temporary cache because they
don't lose context the way the Redirect flow does. Applying this across
the board causes unnecessary stringification, storage, lookup and
parsing in those flows, especially since the code isn't shared across
those flows anyway.

This PR refactors the popup and silent flows to pass the request object
through the stack instead of storing and looking up in sessionStorage.
The redirect flow will continue to utilize sessionStorage for temp
cache.

Author: tnorling

change/@azure-msal-browser-7c734503-8069-411f-8bfb-472e0dc65ba2.json

@@ -0,0 +1,7 @@
+{
+  "type": "minor",
+  "comment": "Don't use temporary cache for silent & popup flows #6586",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

change/@azure-msal-common-542684b7-2f3a-4793-87ef-e63aa1516284.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "Don't use temporary cache for silent & popup flows #6586",
+  "packageName": "@azure/msal-common",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

lib/msal-browser/src/interaction_client/PopupClient.ts

@@ -42,15 +42,12 @@ import { INavigationClient } from "../navigation/INavigationClient";
 import { EventHandler } from "../event/EventHandler";
 import { BrowserCacheManager } from "../cache/BrowserCacheManager";
 import { BrowserConfiguration } from "../config/Configuration";
-import {
-    InteractionHandler,
-    InteractionParams,
-} from "../interaction_handler/InteractionHandler";
+import { InteractionHandler } from "../interaction_handler/InteractionHandler";
 import { PopupWindowAttributes } from "../request/PopupWindowAttributes";
 import { EventError } from "../event/EventMessage";
 import { AuthenticationResult } from "../response/AuthenticationResult";
 
-export type PopupParams = InteractionParams & {
+export type PopupParams = {
     popup?: Window | null;
     popupName: string;
     popupWindowAttributes: PopupWindowAttributes;
@@ -212,13 +209,6 @@ export class PopupClient extends StandardInteractionClient {
             InteractionType.Popup
         );
         BrowserUtils.preconnect(validRequest.authority);
-        this.browserStorage.updateCacheEntries(
-            validRequest.state,
-            validRequest.nonce,
-            validRequest.authority,
-            validRequest.loginHint || Constants.EMPTY_STRING,
-            validRequest.account || null
-        );
 
         try {
             // Create auth code request and generate PKCE params
@@ -295,11 +285,6 @@ export class PopupClient extends StandardInteractionClient {
             // Deserialize hash fragment response parameters.
             const serverParams: ServerAuthorizationCodeResponse =
                 UrlString.getDeserializedHash(hash);
-            const state = this.validateAndExtractStateFromHash(
-                serverParams,
-                InteractionType.Popup,
-                validRequest.correlationId
-            );
             // Remove throttle if it exists
             ThrottlingUtils.removeThrottle(
                 this.browserStorage,
@@ -340,25 +325,19 @@ export class PopupClient extends StandardInteractionClient {
                 );
                 const { userRequestState } = ProtocolUtils.parseRequestState(
                     this.browserCrypto,
-                    state
+                    validRequest.state
                 );
-                return nativeInteractionClient
-                    .acquireToken({
-                        ...validRequest,
-                        state: userRequestState,
-                        prompt: undefined, // Server should handle the prompt, ideally native broker can do this part silently
-                    })
-                    .finally(() => {
-                        this.browserStorage.cleanRequestByState(state);
-                    });
+                return nativeInteractionClient.acquireToken({
+                    ...validRequest,
+                    state: userRequestState,
+                    prompt: undefined, // Server should handle the prompt, ideally native broker can do this part silently
+                });
             }
 
             // Handle response from hash string.
             const result = await interactionHandler.handleCodeResponseFromHash(
                 hash,
-                state,
-                authClient.authority,
-                this.networkClient
+                validRequest
             );
 
             return result;
@@ -372,7 +351,6 @@ export class PopupClient extends StandardInteractionClient {
                 (e as AuthError).setCorrelationId(this.correlationId);
                 serverTelemetryManager.cacheFailedRequest(e);
             }
-            this.browserStorage.cleanRequestByState(validRequest.state);
             throw e;
         }
     }

lib/msal-browser/src/interaction_client/RedirectClient.ts

@@ -139,7 +139,6 @@ export class RedirectClient extends StandardInteractionClient {
                 this.browserStorage,
                 authCodeRequest,
                 this.logger,
-                this.browserCrypto,
                 this.performanceClient
             );
 
@@ -463,15 +462,9 @@ export class RedirectClient extends StandardInteractionClient {
             this.browserStorage,
             cachedRequest,
             this.logger,
-            this.browserCrypto,
             this.performanceClient
         );
-        return await interactionHandler.handleCodeResponseFromHash(
-            hash,
-            state,
-            authClient.authority,
-            this.networkClient
-        );
+        return await interactionHandler.handleCodeResponseFromHash(hash, state);
     }
 
     /**

lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts

@@ -8,7 +8,6 @@ import {
     Logger,
     CommonAuthorizationCodeRequest,
     AuthError,
-    Constants,
     IPerformanceClient,
     PerformanceEvents,
     invokeAsync,
@@ -81,13 +80,6 @@ export class SilentAuthCodeClient extends StandardInteractionClient {
             this.performanceClient,
             request.correlationId
         )(request, InteractionType.Silent);
-        this.browserStorage.updateCacheEntries(
-            silentRequest.state,
-            silentRequest.nonce,
-            silentRequest.authority,
-            silentRequest.loginHint || Constants.EMPTY_STRING,
-            silentRequest.account || null
-        );
 
         const serverTelemetryManager = this.initializeServerTelemetryManager(
             this.apiId
@@ -137,17 +129,14 @@ export class SilentAuthCodeClient extends StandardInteractionClient {
                     cloud_graph_host_name: request.cloudGraphHostName,
                     cloud_instance_host_name: request.cloudInstanceHostName,
                 },
-                silentRequest.state,
-                authClient.authority,
-                this.networkClient,
+                silentRequest,
                 false
             );
         } catch (e) {
             if (e instanceof AuthError) {
                 (e as AuthError).setCorrelationId(this.correlationId);
                 serverTelemetryManager.cacheFailedRequest(e);
             }
-            this.browserStorage.cleanRequestByState(silentRequest.state);
             throw e;
         }
     }

lib/msal-browser/src/interaction_client/SilentIframeClient.ts

@@ -10,7 +10,6 @@ import {
     CommonAuthorizationCodeRequest,
     AuthorizationCodeClient,
     AuthError,
-    Constants,
     UrlString,
     ServerAuthorizationCodeResponse,
     ProtocolUtils,
@@ -120,13 +119,6 @@ export class SilentIframeClient extends StandardInteractionClient {
             InteractionType.Silent
         );
         BrowserUtils.preconnect(silentRequest.authority);
-        this.browserStorage.updateCacheEntries(
-            silentRequest.state,
-            silentRequest.nonce,
-            silentRequest.authority,
-            silentRequest.loginHint || Constants.EMPTY_STRING,
-            silentRequest.account || null
-        );
 
         const serverTelemetryManager = this.initializeServerTelemetryManager(
             this.apiId
@@ -158,7 +150,6 @@ export class SilentIframeClient extends StandardInteractionClient {
                 (e as AuthError).setCorrelationId(this.correlationId);
                 serverTelemetryManager.cacheFailedRequest(e);
             }
-            this.browserStorage.cleanRequestByState(silentRequest.state);
             throw e;
         }
     }
@@ -273,11 +264,6 @@ export class SilentIframeClient extends StandardInteractionClient {
         // Deserialize hash fragment response parameters.
         const serverParams: ServerAuthorizationCodeResponse =
             UrlString.getDeserializedHash(hash);
-        const state = this.validateAndExtractStateFromHash(
-            serverParams,
-            InteractionType.Silent,
-            correlationId
-        );
 
         if (serverParams.accountId) {
             this.logger.verbose(
@@ -304,7 +290,7 @@ export class SilentIframeClient extends StandardInteractionClient {
             );
             const { userRequestState } = ProtocolUtils.parseRequestState(
                 this.browserCrypto,
-                state
+                silentRequest.state
             );
             return invokeAsync(
                 nativeInteractionClient.acquireToken.bind(
@@ -318,8 +304,6 @@ export class SilentIframeClient extends StandardInteractionClient {
                 ...silentRequest,
                 state: userRequestState,
                 prompt: silentRequest.prompt || PromptValue.NONE,
-            }).finally(() => {
-                this.browserStorage.cleanRequestByState(state);
             });
         }
 
@@ -332,6 +316,6 @@ export class SilentIframeClient extends StandardInteractionClient {
             this.logger,
             this.performanceClient,
             correlationId
-        )(hash, state, authClient.authority, this.networkClient);
+        )(hash, silentRequest);
     }
 }