onRedirectNavigate deprecation fix

change/@azure-msal-browser-92feeccb-6366-4bec-9fdd-4467a5cf21e9.json

@@ -0,0 +1,7 @@
+{
+  "type": "patch",
+  "comment": "onRedirectNavigate deprecation fix #7251",
+  "packageName": "@azure/msal-browser",
+  "email": "[email protected]",
+  "dependentChangeType": "patch"
+}

lib/msal-browser/src/cache/BrowserCacheManager.ts

@@ -1552,6 +1552,9 @@ export class BrowserCacheManager extends CacheManager {
         this.removeTemporaryItem(
             this.generateCacheKey(TemporaryCacheKeys.NATIVE_REQUEST)
         );
+        this.removeTemporaryItem(
+            this.generateCacheKey(TemporaryCacheKeys.REDIRECT_REQUEST)
+        );
         this.setInteractionInProgress(false);
     }
 

lib/msal-browser/src/controllers/StandardController.ts

@@ -584,19 +584,37 @@ export class StandardController implements IController {
             scenarioId: request.scenarioId,
         });
 
+        // Override on request only if set, as onRedirectNavigate field is deprecated
         const onRedirectNavigateCb = request.onRedirectNavigate;
-        request.onRedirectNavigate = (url: string) => {
-            const navigate =
-                typeof onRedirectNavigateCb === "function"
-                    ? onRedirectNavigateCb(url)
-                    : undefined;
-            if (navigate !== false) {
-                atrMeasurement.end({ success: true });
-            } else {
-                atrMeasurement.discard();
-            }
-            return navigate;
-        };
+        if (onRedirectNavigateCb) {
+            request.onRedirectNavigate = (url: string) => {
+                const navigate =
+                    typeof onRedirectNavigateCb === "function"
+                        ? onRedirectNavigateCb(url)
+                        : undefined;
+                if (navigate !== false) {
+                    atrMeasurement.end({ success: true });
+                } else {
+                    atrMeasurement.discard();
+                }
+                return navigate;
+            };
+        } else {
+            const configOnRedirectNavigateCb =
+                this.config.auth.onRedirectNavigate;
+            this.config.auth.onRedirectNavigate = (url: string) => {
+                const navigate =
+                    typeof configOnRedirectNavigateCb === "function"
+                        ? configOnRedirectNavigateCb(url)
+                        : undefined;
+                if (navigate !== false) {
+                    atrMeasurement.end({ success: true });
+                } else {
+                    atrMeasurement.discard();
+                }
+                return navigate;
+            };
+        }
 
         // If logged in, emit acquire token events
         const isLoggedIn = this.getAllAccounts().length > 0;

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

@@ -384,11 +384,6 @@ export class RedirectClient extends StandardInteractionClient {
                 return null;
             }
 
-            this.browserStorage.removeTemporaryItem(
-                this.browserStorage.generateCacheKey(
-                    TemporaryCacheKeys.REDIRECT_REQUEST
-                )
-            );
             this.browserStorage.removeRequestRetried();
             throw e;
         } finally {

lib/msal-browser/src/interaction_handler/RedirectHandler.ts

@@ -223,11 +223,6 @@ export class RedirectHandler {
 
         this.browserStorage.cleanRequestByState(state);
         this.browserStorage.removeRequestRetried();
-        this.browserStorage.removeTemporaryItem(
-            this.browserStorage.generateCacheKey(
-                TemporaryCacheKeys.REDIRECT_REQUEST
-            )
-        );
         return tokenResponse;
     }
 

lib/msal-browser/test/app/PublicClientApplication.spec.ts

@@ -1900,6 +1900,52 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
             pca.acquireTokenRedirect(loginRequest);
         });
 
+        it("emits pre-redirect telemetry event when onRedirectNavigate callback is set in configuration", async () => {
+            const onRedirectNavigate = (url: string) => {
+                expect(url).toBeDefined();
+            };
+
+            pca = new PublicClientApplication({
+                auth: {
+                    clientId: TEST_CONFIG.MSAL_CLIENT_ID,
+                    onRedirectNavigate,
+                },
+                telemetry: {
+                    client: new BrowserPerformanceClient(testAppConfig),
+                    application: {
+                        appName: TEST_CONFIG.applicationName,
+                        appVersion: TEST_CONFIG.applicationVersion,
+                    },
+                },
+            });
+            pca = (pca as any).controller;
+            await pca.initialize();
+
+            const callbackId = pca.addPerformanceCallback((events) => {
+                expect(events[0].success).toBe(true);
+                expect(events[0].name).toBe(
+                    PerformanceEvents.AcquireTokenPreRedirect
+                );
+                pca.removePerformanceCallback(callbackId);
+            });
+
+            jest.spyOn(
+                NavigationClient.prototype,
+                "navigateExternal"
+            ).mockImplementation(() => Promise.resolve(true));
+
+            jest.spyOn(PkceGenerator, "generatePkceCodes").mockResolvedValue({
+                challenge: TEST_CONFIG.TEST_CHALLENGE,
+                verifier: TEST_CONFIG.TEST_VERIFIER,
+            });
+            const loginRequest: RedirectRequest = {
+                redirectUri: TEST_URIS.TEST_REDIR_URI,
+                scopes: ["user.read", "openid", "profile"],
+                state: TEST_STATE_VALUES.USER_STATE,
+            };
+            await pca.acquireTokenRedirect(loginRequest);
+        });
+
         it("discards pre-redirect telemetry event when onRedirectNavigate callback returns false", async () => {
             const onRedirectNavigate = (url: string) => {
                 return false;

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

@@ -486,7 +486,7 @@ describe("RedirectClient", () => {
             };
             window.sessionStorage.setItem(
                 `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REDIRECT_REQUEST}`,
-                base64Encode(JSON.stringify(testRedirectRequest))
+                JSON.stringify(testRedirectRequest)
             );
             const testTokenReq: CommonAuthorizationCodeRequest = {
                 redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`,
@@ -2034,7 +2034,7 @@ describe("RedirectClient", () => {
             };
             window.sessionStorage.setItem(
                 `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REDIRECT_REQUEST}`,
-                base64Encode(JSON.stringify(testRedirectRequest))
+                JSON.stringify(testRedirectRequest)
             );
             const testTokenReq: CommonAuthorizationCodeRequest = {
                 redirectUri: `${TEST_URIS.DEFAULT_INSTANCE}/`,
@@ -2420,6 +2420,13 @@ describe("RedirectClient", () => {
                                 )
                             )
                         ).toEqual(null);
+                        expect(
+                            browserStorage.getTemporaryCache(
+                                browserStorage.generateCacheKey(
+                                    TemporaryCacheKeys.REDIRECT_REQUEST
+                                )
+                            )
+                        ).toEqual(null);
                         done();
                         return Promise.resolve(true);
                     }
@@ -2740,7 +2747,7 @@ describe("RedirectClient", () => {
                 await redirectClient.acquireToken(emptyRequest);
             } catch (e) {
                 // Test that error was cached for telemetry purposes and then thrown
-                expect(window.sessionStorage).toHaveLength(2);
+                expect(window.sessionStorage).toHaveLength(1);
                 const failures = window.sessionStorage.getItem(
                     `server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
                 );
@@ -3325,7 +3332,7 @@ describe("RedirectClient", () => {
                 await redirectClient.acquireToken(emptyRequest);
             } catch (e) {
                 // Test that error was cached for telemetry purposes and then thrown
-                expect(window.sessionStorage).toHaveLength(2);
+                expect(window.sessionStorage).toHaveLength(1);
                 const failures = window.sessionStorage.getItem(
                     `server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
                 );

lib/msal-browser/test/interaction_handler/RedirectHandler.spec.ts

@@ -54,6 +54,7 @@ import { DatabaseStorage } from "../../src/cache/DatabaseStorage";
 import { BrowserCacheManager } from "../../src/cache/BrowserCacheManager";
 import { NavigationClient } from "../../src/navigation/NavigationClient";
 import { NavigationOptions } from "../../src/navigation/NavigationOptions";
+import { RedirectRequest } from "../../src/request/RedirectRequest";
 
 const testPkceCodes = {
     challenge: "TestChallenge",
@@ -444,6 +445,24 @@ describe("RedirectHandler.ts Unit Tests", () => {
                 browserStorage.generateCacheKey(TemporaryCacheKeys.URL_HASH),
                 TEST_HASHES.TEST_SUCCESS_CODE_HASH_REDIRECT
             );
+            browserStorage.setItem(
+                `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.REQUEST_RETRY}.${TEST_CONFIG.MSAL_CLIENT_ID}`,
+                JSON.stringify(1)
+            );
+            const testRedirectRequest: RedirectRequest = {
+                redirectUri: TEST_URIS.TEST_REDIR_URI,
+                scopes: TEST_CONFIG.DEFAULT_SCOPES,
+                correlationId: TEST_CONFIG.CORRELATION_ID,
+                state: TEST_STATE_VALUES.USER_STATE,
+                authority: TEST_CONFIG.validAuthority,
+                nonce: "",
+                authenticationScheme:
+                    TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme,
+            };
+            browserStorage.setItem(
+                `${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.REDIRECT_REQUEST}`,
+                JSON.stringify(testRedirectRequest)
+            );
             sinon
                 .stub(
                     AuthorizationCodeClient.prototype,
@@ -481,6 +500,14 @@ describe("RedirectHandler.ts Unit Tests", () => {
                     browserStorage.generateCacheKey(TemporaryCacheKeys.URL_HASH)
                 )
             ).toBe(null);
+            expect(
+                browserStorage.getTemporaryCache(
+                    browserStorage.generateCacheKey(TemporaryCacheKeys.REDIRECT_REQUEST)
+                )
+            ).toBe(null);
+            expect(
+                browserStorage.getRequestRetried()
+            ).toBe(null);
         });
 
         it("successfully handles response adds CCS credential to auth code request", async () => {