Merge pull request #1862 from AzureAD/loginStartPage

Fix redirection to pages with custom hashes or query params

lib/msal-browser/src/app/PublicClientApplication.ts

@@ -40,6 +40,8 @@ import { BrowserConstants, TemporaryCacheKeys } from "../utils/BrowserConstants"
 import { BrowserUtils } from "../utils/BrowserUtils";
 import { version } from "../../package.json";
 import { IPublicClientApplication } from "./IPublicClientApplication";
+import { RedirectRequest } from "../request/RedirectRequest";
+import { PopupRequest } from "../request/PopupRequest";
 
 /**
  * The PublicClientApplication class is the object exposed by the library to perform authentication and authorization functions in Single Page Applications
@@ -135,16 +137,18 @@ export class PublicClientApplication implements IPublicClientApplication {
         const cachedHash = this.browserStorage.getItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.URL_HASH), CacheSchemaType.TEMPORARY) as string;
         const isResponseHash = UrlString.hashContainsKnownProperties(hash);
         const loginRequestUrl = this.browserStorage.getItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.ORIGIN_URI), CacheSchemaType.TEMPORARY) as string;
-        const currentUrl = BrowserUtils.getCurrentUri();
-        if (loginRequestUrl === currentUrl || !this.config.auth.navigateToLoginRequestUrl) {
-            // We don't need to navigate - check for hash and prepare to process
-            if (isResponseHash) {
-                BrowserUtils.clearHash();
-                return this.handleHash(hash);
+
+        const currentUrlNormalized = UrlString.removeHashFromUrl(window.location.href);
+        const loginRequestUrlNormalized = UrlString.removeHashFromUrl(loginRequestUrl || "");
+        if (loginRequestUrlNormalized === currentUrlNormalized) {
+            if (this.config.auth.navigateToLoginRequestUrl) {
+                // Replace current hash with non-msal hash, if present
+                BrowserUtils.replaceHash(loginRequestUrl);
             } else {
-                // Loaded page with no valid hash - pass in the value retrieved from cache, or null/empty string
-                return this.handleHash(cachedHash);
+                BrowserUtils.clearHash();
             }
+
+            return this.handleHash(isResponseHash ? hash : cachedHash);
         }
 
         if (this.config.auth.navigateToLoginRequestUrl && isResponseHash && !BrowserUtils.isInIframe()) {
@@ -191,7 +195,7 @@ export class PublicClientApplication implements IPublicClientApplication {
 	 *
      * @param {@link (AuthenticationParameters:type)}
      */
-    async loginRedirect(request: AuthorizationUrlRequest): Promise<void> {
+    async loginRedirect(request: RedirectRequest): Promise<void> {
         return this.acquireTokenRedirect(request);
     }
 
@@ -205,7 +209,7 @@ export class PublicClientApplication implements IPublicClientApplication {
      *
      * To acquire only idToken, please pass clientId as the only scope in the Authentication Parameters
      */
-    async acquireTokenRedirect(request: AuthorizationUrlRequest): Promise<void> {
+    async acquireTokenRedirect(request: RedirectRequest): Promise<void> {
         try {
             // Preflight request
             const validRequest: AuthorizationUrlRequest = this.preflightInteractiveRequest(request);
@@ -223,7 +227,7 @@ export class PublicClientApplication implements IPublicClientApplication {
             const navigateUrl = await authClient.getAuthCodeUrl(validRequest);
 
             // Show the UI once the url has been created. Response will come back in the hash, which will be handled in the handleRedirectCallback function.
-            interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest, this.browserCrypto);
+            interactionHandler.initiateAuthRequest(navigateUrl, authCodeRequest, request.redirectStartPage, this.browserCrypto);
         } catch (e) {
             this.browserStorage.cleanRequest();
             throw e;
@@ -241,7 +245,7 @@ export class PublicClientApplication implements IPublicClientApplication {
      *
      * @returns {Promise.<AuthenticationResult>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
      */
-    async loginPopup(request: AuthorizationUrlRequest): Promise<AuthenticationResult> {
+    async loginPopup(request: PopupRequest): Promise<AuthenticationResult> {
         return this.acquireTokenPopup(request);
     }
 
@@ -252,7 +256,7 @@ export class PublicClientApplication implements IPublicClientApplication {
      * To acquire only idToken, please pass clientId as the only scope in the Authentication Parameters
      * @returns {Promise.<AuthenticationResult>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object
      */
-    async acquireTokenPopup(request: AuthorizationUrlRequest): Promise<AuthenticationResult> {
+    async acquireTokenPopup(request: PopupRequest): Promise<AuthenticationResult> {
         try {
             // Preflight request
             const validRequest: AuthorizationUrlRequest = this.preflightInteractiveRequest(request);
@@ -550,7 +554,7 @@ export class PublicClientApplication implements IPublicClientApplication {
     /**
      * Helper to validate app environment before making a request.
      */
-    private preflightInteractiveRequest(request: AuthorizationUrlRequest): AuthorizationUrlRequest {
+    private preflightInteractiveRequest(request: RedirectRequest|PopupRequest): AuthorizationUrlRequest {
         // block the reload if it occurred inside a hidden iframe
         BrowserUtils.blockReloadInHiddenIframes();
 
@@ -598,7 +602,7 @@ export class PublicClientApplication implements IPublicClientApplication {
      * Helper to initialize required request parameters for interactive APIs and ssoSilent()
      * @param request
      */
-    private initializeAuthorizationRequest(request: AuthorizationUrlRequest): AuthorizationUrlRequest {
+    private initializeAuthorizationRequest(request: AuthorizationUrlRequest|RedirectRequest|PopupRequest): AuthorizationUrlRequest {
         let validatedRequest: AuthorizationUrlRequest = {
             ...request
         };

lib/msal-browser/src/index.ts

@@ -8,6 +8,8 @@ export { BrowserConfigurationAuthError, BrowserConfigurationAuthErrorMessage } f
 
 // Interfaces
 export { IPublicClientApplication } from "./app/IPublicClientApplication";
+export { PopupRequest } from "./request/PopupRequest";
+export { RedirectRequest } from "./request/RedirectRequest";
 
 // Common Object Formats
 export {

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

@@ -14,15 +14,14 @@ export class RedirectHandler extends InteractionHandler {
      * Redirects window to given URL.
      * @param urlNavigate
      */
-    initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest, browserCrypto?: ICrypto): Window {
+    initiateAuthRequest(requestUrl: string, authCodeRequest: AuthorizationCodeRequest, redirectStartPage?: string, browserCrypto?: ICrypto): Window {
         // Navigate if valid URL
         if (!StringUtils.isEmpty(requestUrl)) {
+            // Cache start page, returns to this page after redirectUri if navigateToLoginRequestUrl is true
+            const loginStartPage = redirectStartPage || window.location.href;
+            this.browserStorage.setItem(this.browserStorage.generateCacheKey(TemporaryCacheKeys.ORIGIN_URI), loginStartPage, CacheSchemaType.TEMPORARY);
+
             // Set interaction status in the library.
-            this.browserStorage.setItem(
-                this.browserStorage.generateCacheKey(TemporaryCacheKeys.ORIGIN_URI), 
-                BrowserUtils.getCurrentUri(), 
-                CacheSchemaType.TEMPORARY
-            );
             this.browserStorage.setItem(this.browserStorage.generateCacheKey(BrowserConstants.INTERACTION_STATUS_KEY), BrowserConstants.INTERACTION_IN_PROGRESS_VALUE, CacheSchemaType.TEMPORARY);
             this.browserStorage.cacheCodeRequest(authCodeRequest, browserCrypto);
             this.authModule.logger.infoPii("Navigate to:" + requestUrl);

lib/msal-browser/src/request/PopupRequest.ts

@@ -0,0 +1,12 @@
+/*
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { AuthorizationUrlRequest } from "@azure/msal-common";
+
+/**
+ * @type PopupRequest: Request object passed by user to retrieve a Code from the
+ * server (first leg of authorization code grant flow)
+ */
+export type PopupRequest = AuthorizationUrlRequest;

lib/msal-browser/src/request/RedirectRequest.ts

@@ -0,0 +1,19 @@
+/*
+ * Copyright (c) Microsoft Corporation. All rights reserved.
+ * Licensed under the MIT License.
+ */
+
+import { AuthorizationUrlRequest } from "@azure/msal-common";
+
+/**
+ * @type RedirectRequest: Request object passed by user to retrieve a Code from the
+ * server (first leg of authorization code grant flow)
+ */
+export type RedirectRequest = AuthorizationUrlRequest & {
+    /**
+     * The page that should be returned to after loginRedirect or acquireTokenRedirect. This should only be used
+     * if this is different from the redirectUri and will default to the page that initiates the request.
+     * When the navigateToLoginRequestUrl config option is set to false this parameter will be ignored.
+     */
+    redirectStartPage?: string;
+};

lib/msal-browser/src/utils/BrowserUtils.ts

@@ -34,6 +34,16 @@ export class BrowserUtils {
         window.location.hash = "";
     }
 
+    /**
+     * Replaces current hash with hash from provided url
+     */
+    static replaceHash(url: string): void {
+        const urlParts = url.split("#");
+        urlParts.shift(); // Remove part before the hash
+
+        window.location.hash = urlParts.length > 0 ? urlParts.join("#") : "";
+    }
+
     /**
      * Returns boolean of whether the current window is in an iframe or not.
      */

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

@@ -61,15 +61,93 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
 
         it("navigates and caches hash if navigateToLoginRequestUri is true", (done) => {
             window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
-            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_REDIR_URI);
-            sinon.stub(BrowserUtils, "getCurrentUri").returns("notAUri");
+            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_ALTERNATE_REDIR_URI);
             sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
                 expect(noHistory).to.be.true;
-                expect(urlNavigate).to.be.eq(TEST_URIS.TEST_REDIR_URI);
+                expect(urlNavigate).to.be.eq(TEST_URIS.TEST_ALTERNATE_REDIR_URI);
+                done();
             });
             pca.handleRedirectPromise();
             expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
-            done();
+        });
+
+        it("navigates to root and caches hash if navigateToLoginRequestUri is true", (done) => {
+            window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
+            sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
+                expect(noHistory).to.be.true;
+                expect(urlNavigate).to.be.eq("/");
+                done();
+            });
+            pca.handleRedirectPromise();
+            expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+        });
+
+        it("navigates to root and caches hash if navigateToLoginRequestUri is true and loginRequestUrl is 'null'", (done) => {
+            window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
+            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, "null");
+            sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
+                expect(noHistory).to.be.true;
+                expect(urlNavigate).to.be.eq("/");
+                done();
+            });
+            pca.handleRedirectPromise();
+            expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+        });
+
+        it("navigates and caches hash if navigateToLoginRequestUri is true and loginRequestUrl contains query string", (done) => {
+            const loginRequestUrl = window.location.href + "?testQueryString=1";
+            window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
+            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, loginRequestUrl);
+            sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
+                expect(noHistory).to.be.true;
+                expect(urlNavigate).to.be.eq(loginRequestUrl);
+                done();
+            });
+            pca.handleRedirectPromise();
+            expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+        });
+
+        it("navigates and caches hash if navigateToLoginRequestUri is true and loginRequestUrl contains query string and hash", (done) => {
+            const loginRequestUrl = window.location.href + "?testQueryString=1#testHash";
+            window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
+            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, loginRequestUrl);
+            sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => {
+                expect(noHistory).to.be.true;
+                expect(urlNavigate).to.be.eq(loginRequestUrl);
+                done();
+            });
+            pca.handleRedirectPromise();
+            expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+        });
+
+        it("replaces custom hash if navigateToLoginRequestUri is true and loginRequestUrl contains custom hash", (done) => {
+            const loginRequestUrl = window.location.href + "#testHash";
+            window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
+            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, loginRequestUrl);
+            sinon.stub(PublicClientApplication.prototype, <any>"handleHash").callsFake((responseHash) => {
+                expect(window.location.href).to.be.eq(loginRequestUrl);
+                expect(responseHash).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+                done();
+            });
+            pca.handleRedirectPromise();
+        });
+
+        it("clears hash if navigateToLoginRequestUri is false and loginRequestUrl contains custom hash", (done) => {
+            pca = new PublicClientApplication({
+                auth: {
+                    clientId: TEST_CONFIG.MSAL_CLIENT_ID,
+                    navigateToLoginRequestUrl: false
+                }
+            });
+            const loginRequestUrl = window.location.href + "#testHash";
+            window.location.hash = TEST_HASHES.TEST_SUCCESS_CODE_HASH;
+            window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, loginRequestUrl);
+            sinon.stub(PublicClientApplication.prototype, <any>"handleHash").callsFake((responseHash) => {
+                expect(window.location.href).to.not.contain("#testHash");
+                expect(responseHash).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH);
+                done();
+            });
+            pca.handleRedirectPromise();
         });
     });
 

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

@@ -148,8 +148,8 @@ describe("RedirectHandler.ts Unit Tests", () => {
 			};
 			const browserCrypto = new CryptoOps();
             sinon.stub(BrowserUtils, "isInIframe").returns(true);
-            expect(() => redirectHandler.initiateAuthRequest(TEST_URIS.TEST_ALTERNATE_REDIR_URI, testTokenReq, browserCrypto)).to.throw(BrowserAuthErrorMessage.redirectInIframeError.desc);
-            expect(() => redirectHandler.initiateAuthRequest(TEST_URIS.TEST_ALTERNATE_REDIR_URI, testTokenReq, browserCrypto)).to.throw(BrowserAuthError);
+            expect(() => redirectHandler.initiateAuthRequest(TEST_URIS.TEST_ALTERNATE_REDIR_URI, testTokenReq, "", browserCrypto)).to.throw(BrowserAuthErrorMessage.redirectInIframeError.desc);
+            expect(() => redirectHandler.initiateAuthRequest(TEST_URIS.TEST_ALTERNATE_REDIR_URI, testTokenReq, "", browserCrypto)).to.throw(BrowserAuthError);
         });
 
         it("navigates browser window to given window location", () => {
@@ -164,9 +164,8 @@ describe("RedirectHandler.ts Unit Tests", () => {
             sinon.stub(BrowserUtils, "navigateWindow").callsFake((requestUrl) => {
                 expect(requestUrl).to.be.eq(TEST_URIS.TEST_ALTERNATE_REDIR_URI);
             });
-            const windowObj = redirectHandler.initiateAuthRequest(TEST_URIS.TEST_ALTERNATE_REDIR_URI, testTokenReq, new CryptoOps());
+            const windowObj = redirectHandler.initiateAuthRequest(TEST_URIS.TEST_ALTERNATE_REDIR_URI, testTokenReq, "", new CryptoOps());
             expect(window).to.be.eq(windowObj);
-            expect(browserStorage.getItem(browserStorage.generateCacheKey(TemporaryCacheKeys.ORIGIN_URI), CacheSchemaType.TEMPORARY)).to.be.eq(TEST_URIS.TEST_REDIR_URI);
             expect(browserStorage.getItem(browserStorage.generateCacheKey(BrowserConstants.INTERACTION_STATUS_KEY), CacheSchemaType.TEMPORARY)).to.be.eq(BrowserConstants.INTERACTION_IN_PROGRESS_VALUE);
         });
     });

lib/msal-browser/test/utils/BrowserUtils.spec.ts

@@ -61,6 +61,21 @@ describe("BrowserUtils.ts Function Unit Tests", () => {
         BrowserUtils.clearHash()
         expect(window.location.hash).to.be.empty;
     });
+
+    it("replaceHash replaces the current window hash with the hash from the provided url", () => {
+        window.location.hash = "thisIsAHash";
+        const url = "http://localhost/#";
+        const testHash = "replacementHash"
+        BrowserUtils.replaceHash(url + testHash)
+        expect(window.location.hash).to.be.eq(testHash);
+    });
+
+    it("replaceHash clears the current window hash when provided url does not have hash", () => {
+        window.location.hash = "thisIsAHash";
+        const url = "http://localhost/";
+        BrowserUtils.replaceHash(url)
+        expect(window.location.hash).to.be.eq("");
+    });
 
     it("isInIframe() returns false if window parent is not the same as the current window", () => {
         expect(BrowserUtils.isInIframe()).to.be.false;

lib/msal-common/src/url/UrlString.ts

@@ -86,6 +86,10 @@ export class UrlString {
         return this.urlString;
     }
 
+    static removeHashFromUrl(url: string): string {
+        return url.split("#")[0];
+    }
+
     /**
      * Given a url like https://a:b/common/d?e=f#g, and a tenantId, returns https://a:b/tenantId/d
      * @param href The url

lib/msal-common/test/url/UrlString.spec.ts

@@ -63,6 +63,12 @@ describe("UrlString.ts Class Unit Tests", () => {
         expect(urlObj2.urlString).to.not.contain("param2=value2");
     });
 
+    it("removes hash from url provided", () => {
+        const baseUrl = "https://localhost/";
+        const fullUrl = baseUrl + "#thisIsATestHash";
+        expect(UrlString.removeHashFromUrl(fullUrl)).to.eq(baseUrl);
+    });
+
     it("replaceTenantPath correctly replaces common with tenant id", () => {
         let urlObj = new UrlString(TEST_URIS.TEST_AUTH_ENDPT);
         const sampleTenantId = "sample-tenant-id";