diff --git a/lib/msal-browser/src/app/Configuration.ts b/lib/msal-browser/src/app/Configuration.ts index 47f8822538..1e5c0e3d2e 100644 --- a/lib/msal-browser/src/app/Configuration.ts +++ b/lib/msal-browser/src/app/Configuration.ts @@ -60,8 +60,8 @@ export type Configuration = { const DEFAULT_AUTH_OPTIONS: BrowserAuthOptions = { clientId: "", authority: null, - redirectUri: () => BrowserUtils.getDefaultRedirectUri(), - postLogoutRedirectUri: () => BrowserUtils.getDefaultRedirectUri(), + redirectUri: () => BrowserUtils.getCurrentUri(), + postLogoutRedirectUri: () => BrowserUtils.getCurrentUri(), navigateToLoginRequestUrl: true }; diff --git a/lib/msal-browser/src/app/PublicClientApplication.ts b/lib/msal-browser/src/app/PublicClientApplication.ts index c565d443fd..5ca3fbb752 100644 --- a/lib/msal-browser/src/app/PublicClientApplication.ts +++ b/lib/msal-browser/src/app/PublicClientApplication.ts @@ -27,9 +27,6 @@ export class PublicClientApplication { // auth functions imported from @azure/msal-common module private authModule: AuthorizationCodeModule; - // callback for error/token response - private authCallback: AuthCallback = null; - // Crypto interface implementation private browserCrypto: CryptoOps; @@ -39,6 +36,9 @@ export class PublicClientApplication { // Network interface implementation private networkClient: INetworkModule; + // Response promise + private tokenExchangePromise: Promise; + /** * @constructor * Constructor for the PublicClientApplication used to instantiate the PublicClientApplication object @@ -88,13 +88,15 @@ export class PublicClientApplication { networkInterface: this.networkClient, storageInterface: this.browserStorage }); + + // Check for hash and save response promise + this.tokenExchangePromise = this.handleRedirectResponse(); } // #region Redirect Flow /** - * Set the callback functions for the redirect flow to send back the success or error object, and process - * any redirect-related data. + * Process any redirect-related data and send back the success or error object. * IMPORTANT: Please do not use this function when using the popup APIs, as it may break the response handling * in the main window. * @@ -109,14 +111,14 @@ export class PublicClientApplication { throw BrowserConfigurationAuthError.createInvalidCallbackObjectError(authCallback); } - // Set the callback object. - this.authCallback = authCallback; - // Check if we need to navigate, otherwise handle hash try { - await this.handleRedirectResponse(); + const tokenResponse = await this.tokenExchangePromise; + if (tokenResponse) { + authCallback(null, tokenResponse); + } } catch (err) { - this.authCallback(err); + authCallback(err); } } @@ -125,23 +127,38 @@ export class PublicClientApplication { * - if true, performs logic to cache and navigate * - if false, handles hash string and parses response */ - private async handleRedirectResponse(): Promise { + private async handleRedirectResponse(): Promise { // Get current location hash from window or cache. const { location: { hash } } = window; const cachedHash = this.browserStorage.getItem(TemporaryCacheKeys.URL_HASH); const isResponseHash = UrlString.hashContainsKnownProperties(hash); + + const loginRequestUrl = this.browserStorage.getItem(TemporaryCacheKeys.ORIGIN_URI); + const currentUrl = BrowserUtils.getCurrentUri(); + if (loginRequestUrl === currentUrl) { + // We don't need to navigate - check for hash and prepare to process + if (isResponseHash) { + BrowserUtils.clearHash(); + return this.handleHash(hash); + } else { + // Loaded page with no valid hash - pass in the value retrieved from cache, or null/empty string + return this.handleHash(cachedHash); + } + } + if (this.config.auth.navigateToLoginRequestUrl && isResponseHash && !BrowserUtils.isInIframe()) { // Returned from authority using redirect - need to perform navigation before processing response this.browserStorage.setItem(TemporaryCacheKeys.URL_HASH, hash); - const loginRequestUrl = this.browserStorage.getItem(TemporaryCacheKeys.ORIGIN_URI); + if (StringUtils.isEmpty(loginRequestUrl) || loginRequestUrl === "null") { // Redirect to home page if login request url is null (real null or the string null) this.authModule.logger.warning("Unable to get valid login request url from cache, redirecting to home page"); BrowserUtils.navigateWindow("/", true); } else { + // Navigate to target url BrowserUtils.navigateWindow(loginRequestUrl, true); } - return; + return null; } if (!isResponseHash) { @@ -154,6 +171,8 @@ export class PublicClientApplication { BrowserUtils.clearHash(); return this.handleHash(hash); } + + return null; } /** @@ -161,16 +180,16 @@ export class PublicClientApplication { * @param responseHash * @param interactionHandler */ - private async handleHash(responseHash: string): Promise { + private async handleHash(responseHash: string): Promise { const interactionHandler = new RedirectHandler(this.authModule, this.browserStorage); if (!StringUtils.isEmpty(responseHash)) { // Hash contains known properties - handle and return in callback - const tokenResponse = await interactionHandler.handleCodeResponse(responseHash); - this.authCallback(null, tokenResponse); - } else { - // There is no hash - assume we are in clean state and clear any current request data. - this.cleanRequest(); + return interactionHandler.handleCodeResponse(responseHash); } + + // There is no hash - assume we are in clean state and clear any current request data. + this.cleanRequest(); + return null; } /** @@ -179,19 +198,8 @@ export class PublicClientApplication { * @param {@link (AuthenticationParameters:type)} */ loginRedirect(request: AuthenticationParameters): void { - // block the reload if it occurred inside a hidden iframe - BrowserUtils.blockReloadInHiddenIframes(); - - // Check if callback has been set. If not, handleRedirectCallbacks wasn't called correctly. - if (!this.authCallback) { - throw BrowserConfigurationAuthError.createRedirectCallbacksNotSetError(); - } - - // Check if interaction is in progress. Throw error in callback and return if true. - if (this.interactionInProgress()) { - this.authCallback(BrowserAuthError.createInteractionInProgressError()); - return; - } + // Preflight request + this.preflightRequest(); try { // Create redirect interaction handler. @@ -216,19 +224,8 @@ export class PublicClientApplication { * To acquire only idToken, please pass clientId as the only scope in the Authentication Parameters */ acquireTokenRedirect(request: AuthenticationParameters): void { - // block the reload if it occurred inside a hidden iframe - BrowserUtils.blockReloadInHiddenIframes(); - - // Check if callback has been set. If not, handleRedirectCallbacks wasn't called correctly. - if (!this.authCallback) { - throw BrowserConfigurationAuthError.createRedirectCallbacksNotSetError(); - } - - // Check if interaction is in progress. Throw error in callback and return if true. - if (this.interactionInProgress()) { - this.authCallback(BrowserAuthError.createInteractionInProgressError()); - return; - } + // Preflight request + this.preflightRequest(); try { // Create redirect interaction handler. @@ -257,13 +254,8 @@ export class PublicClientApplication { * @returns {Promise.} - 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: AuthenticationParameters): Promise { - // block the reload if it occurred inside a hidden iframe - BrowserUtils.blockReloadInHiddenIframes(); - - // Check if interaction is in progress. Throw error if true. - if (this.interactionInProgress()) { - throw BrowserAuthError.createInteractionInProgressError(); - } + // Preflight request + this.preflightRequest(); // Create login url, which will by default append the client id scope to the call. const navigateUrl = await this.authModule.createLoginUrl(request); @@ -280,13 +272,8 @@ export class PublicClientApplication { * @returns {Promise.} - 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: AuthenticationParameters): Promise { - // block the reload if it occurred inside a hidden iframe - BrowserUtils.blockReloadInHiddenIframes(); - - // Check if interaction is in progress. Throw error if true. - if (this.interactionInProgress()) { - throw BrowserAuthError.createInteractionInProgressError(); - } + // Preflight request + this.preflightRequest(); // Create acquire token url. const navigateUrl = await this.authModule.createAcquireTokenUrl(request); @@ -481,13 +468,26 @@ export class PublicClientApplication { // #region Helpers /** - * Helper to check whether interaction is in progress + * Helper to check whether interaction is in progress. */ private interactionInProgress(): boolean { // Check whether value in cache is present and equal to expected value return this.browserStorage.getItem(BrowserConstants.INTERACTION_STATUS_KEY) === BrowserConstants.INTERACTION_IN_PROGRESS_VALUE; } + /** + * Helper to validate app environment before making a request. + */ + private preflightRequest(): void { + // block the reload if it occurred inside a hidden iframe + BrowserUtils.blockReloadInHiddenIframes(); + + // Check if interaction is in progress. Throw error if true. + if (this.interactionInProgress()) { + throw BrowserAuthError.createInteractionInProgressError(); + } + } + /** * Helper to remove interaction status and remove tempoarary request data. */ diff --git a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts index a06df03b25..31e79a52b8 100644 --- a/lib/msal-browser/src/interaction_handler/RedirectHandler.ts +++ b/lib/msal-browser/src/interaction_handler/RedirectHandler.ts @@ -18,7 +18,7 @@ export class RedirectHandler extends InteractionHandler { // Navigate if valid URL if (!StringUtils.isEmpty(requestUrl)) { // Set interaction status in the library. - this.browserStorage.setItem(TemporaryCacheKeys.ORIGIN_URI, window.location.href); + this.browserStorage.setItem(TemporaryCacheKeys.ORIGIN_URI, BrowserUtils.getCurrentUri()); this.browserStorage.setItem(BrowserConstants.INTERACTION_STATUS_KEY, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE); this.authModule.logger.infoPii("Navigate to:" + requestUrl); const isIframedApp = BrowserUtils.isInIframe(); diff --git a/lib/msal-browser/src/utils/BrowserUtils.ts b/lib/msal-browser/src/utils/BrowserUtils.ts index 6d47682c7a..3b25351288 100644 --- a/lib/msal-browser/src/utils/BrowserUtils.ts +++ b/lib/msal-browser/src/utils/BrowserUtils.ts @@ -46,7 +46,7 @@ export class BrowserUtils { /** * Returns current window URL as redirect uri */ - static getDefaultRedirectUri(): string { + static getCurrentUri(): string { return window.location.href.split("?")[0].split("#")[0]; } diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index e991879fb9..a357422b70 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -58,6 +58,22 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { expect(pca).to.be.not.null; expect(pca instanceof PublicClientApplication).to.be.true; }); + + it("navigates and caches hash if navigateToLoginRequestUri is true", () => { + 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"); + sinon.stub(BrowserUtils, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => { + expect(noHistory).to.be.true; + expect(urlNavigate).to.be.eq(TEST_URIS.TEST_REDIR_URI); + }); + pca = new PublicClientApplication({ + auth: { + clientId: TEST_CONFIG.MSAL_CLIENT_ID + } + }); + expect(window.sessionStorage.getItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.URL_HASH}`)).to.be.eq(TEST_HASHES.TEST_SUCCESS_CODE_HASH); + }); }); describe("Redirect Flow Unit tests", () => { @@ -75,22 +91,6 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { expect(window.sessionStorage.length).to.be.eq(0); }); - it("navigates and caches hash if navigateToLoginRequestUri is true", () => { - 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, "navigateWindow").callsFake((urlNavigate: string, noHistory?: boolean) => { - expect(noHistory).to.be.true; - expect(urlNavigate).to.be.eq(TEST_URIS.TEST_REDIR_URI); - }); - pca = new PublicClientApplication({ - auth: { - clientId: TEST_CONFIG.MSAL_CLIENT_ID - } - }); - pca.handleRedirectCallback(authCallback); - 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("gets hash from cache and processes response", async () => { const b64Encode = new Base64Encode(); window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${TemporaryCacheKeys.ORIGIN_URI}`, TEST_URIS.TEST_REDIR_URI); @@ -277,18 +277,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { describe("loginRedirect", () => { - it("loginRedirect throws an error if authCallback is not set", () => { - expect(() => pca.loginRedirect({})).to.throw(BrowserConfigurationAuthErrorMessage.noRedirectCallbacksSet.desc); - expect(() => pca.loginRedirect({})).to.throw(BrowserConfigurationAuthError); - }); - it("loginRedirect throws an error if interaction is currently in progress", async () => { - await pca.handleRedirectCallback((authErr: AuthError, response: AuthResponse) => { - expect(authErr instanceof BrowserAuthError).to.be.true; - expect(authErr.errorMessage).to.be.eq(BrowserAuthErrorMessage.interactionInProgress.desc); - }); window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${BrowserConstants.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE); - pca.loginRedirect({}); + expect(() => pca.loginRedirect({})).to.throw(BrowserAuthErrorMessage.interactionInProgress.desc); + expect(() => pca.loginRedirect({})).to.throw(BrowserAuthError); }); it("loginRedirect navigates to created login url", async () => { @@ -325,18 +317,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { describe("acquireTokenRedirect", () => { - it("acquireTokenRedirect throws an error if authCallback is not set", () => { - expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserConfigurationAuthErrorMessage.noRedirectCallbacksSet.desc); - expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserConfigurationAuthError); - }); - it("acquireTokenRedirect throws an error if interaction is currently in progress", async () => { - await pca.handleRedirectCallback((authErr: AuthError, response: AuthResponse) => { - expect(authErr instanceof BrowserAuthError).to.be.true; - expect(authErr.errorMessage).to.be.eq(BrowserAuthErrorMessage.interactionInProgress.desc); - }); window.sessionStorage.setItem(`${Constants.CACHE_PREFIX}.${TEST_CONFIG.MSAL_CLIENT_ID}.${BrowserConstants.INTERACTION_STATUS_KEY}`, BrowserConstants.INTERACTION_IN_PROGRESS_VALUE); - pca.acquireTokenRedirect({}); + expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserAuthErrorMessage.interactionInProgress.desc); + expect(() => pca.acquireTokenRedirect({})).to.throw(BrowserAuthError); }); it("acquireTokenRedirect navigates to created login url", async () => { diff --git a/lib/msal-browser/test/utils/BrowserUtils.spec.ts b/lib/msal-browser/test/utils/BrowserUtils.spec.ts index 2b13851767..854bb8c0f9 100644 --- a/lib/msal-browser/test/utils/BrowserUtils.spec.ts +++ b/lib/msal-browser/test/utils/BrowserUtils.spec.ts @@ -68,8 +68,8 @@ describe("BrowserUtils.ts Function Unit Tests", () => { expect(BrowserUtils.isInIframe()).to.be.true; }); - it("getDefaultRedirectUri returns current location uri of browser", () => { - expect(BrowserUtils.getDefaultRedirectUri()).to.be.eq(TEST_URIS.TEST_REDIR_URI); + it("getCurrentUri() returns current location uri of browser", () => { + expect(BrowserUtils.getCurrentUri()).to.be.eq(TEST_URIS.TEST_REDIR_URI); }); it("getBrowserNetworkClient() returns fetch client if available", () => {