From 002681f711fe6f2f99ba141b46f96e758083c938 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Wed, 26 Jun 2019 16:00:59 -0700 Subject: [PATCH 01/20] Refactoring interactive APIs --- lib/msal-core/src/Constants.ts | 5 + lib/msal-core/src/UserAgentApplication.ts | 539 +++++++--------------- 2 files changed, 184 insertions(+), 360 deletions(-) diff --git a/lib/msal-core/src/Constants.ts b/lib/msal-core/src/Constants.ts index 8218ebb859..5b39fa88da 100644 --- a/lib/msal-core/src/Constants.ts +++ b/lib/msal-core/src/Constants.ts @@ -80,6 +80,9 @@ export class Constants { static get cacheLocationLocal(): CacheLocation { return "localStorage"; } static get cacheLocationSession(): CacheLocation { return "sessionStorage"; } + + static get interactionTypeRedirect(): InteractionType { return "redirectInteraction"; } + static get interactionTypePopup(): InteractionType { return "popupInteraction"; } } /** @@ -120,6 +123,8 @@ export const BlacklistedEQParams = [ SSOTypes.LOGIN_HINT ]; +export type InteractionType = "redirectInteraction" | "popupInteraction"; + /** * we considered making this "enum" in the request instead of string, however it looks like the allowed list of * prompt values kept changing over past couple of years. There are some undocumented prompt values for some diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 893622bb88..e48707e6ff 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -7,7 +7,7 @@ import { AccessTokenValue } from "./AccessTokenValue"; import { ServerRequestParameters } from "./ServerRequestParameters"; import { Authority } from "./Authority"; import { ClientInfo } from "./ClientInfo"; -import { Constants, SSOTypes, PromptState, BlacklistedEQParams } from "./Constants"; +import { Constants, SSOTypes, PromptState, BlacklistedEQParams, InteractionType } from "./Constants"; import { IdToken } from "./IdToken"; import { Logger } from "./Logger"; import { Storage } from "./Storage"; @@ -303,122 +303,40 @@ export class UserAgentApplication { } } - //#endregion + private successHandler(interactionType: InteractionType, response: AuthResponse, resolve?: any) : void { + if (interactionType === Constants.interactionTypeRedirect) { + if (this.errorReceivedCallback) { + this.tokenReceivedCallback(response); + } else if (this.authResponseCallback) { + this.authResponseCallback(null, response); + } + } else { + resolve(response); + } + } - //#region Redirect Flow + private errorHandler(interactionType: InteractionType, authErr: AuthError, response: AuthResponse, reject?: any) : void { + if (interactionType === Constants.interactionTypeRedirect) { + if (this.errorReceivedCallback) { + this.errorReceivedCallback(authErr, response.accountState); + } else { + this.authResponseCallback(authErr, response); + } + } else { + reject(authErr); + } + } + //#endregion /** * Use when initiating the login process by redirecting the user's browser to the authorization endpoint. * @param {@link (AuthenticationParameters:type)} */ loginRedirect(request?: AuthenticationParameters): void { - - // Throw error if callbacks are not set before redirect if (!this.redirectCallbacksSet) { throw ClientConfigurationError.createRedirectCallbacksNotSetError(); } - - // Creates navigate url; saves value in cache; redirect user to AAD - if (this.loginInProgress) { - this.redirectErrorHandler(ClientAuthError.createLoginInProgressError(), buildResponseStateOnly(request && request.state)); - return; - } - - // if extraScopesToConsent is passed, append them to the login request - let scopes: Array = this.appendScopes(request); - - // Validate and filter scopes (the validate function will throw if validation fails) - this.validateInputScope(scopes, false); - - const account: Account = this.getAccount(); - - // defer queryParameters generation to Helper if developer passes account/sid/login_hint - if (Utils.isSSOParam(request)) { - // if account is not provided, we pass null - this.loginRedirectHelper(account, request, scopes); - } - // else handle the library data - else { - // extract ADAL id_token if exists - let adalIdToken = this.extractADALIdToken(); - - // silent login if ADAL id_token is retrieved successfully - SSO - if (adalIdToken && !scopes) { - this.logger.info("ADAL's idToken exists. Extracting login information from ADAL's idToken "); - let tokenRequest: AuthenticationParameters = this.buildIDTokenRequest(request); - - this.silentLogin = true; - this.acquireTokenSilent(tokenRequest).then(response => { - this.silentLogin = false; - this.logger.info("Unified cache call is successful"); - - if (this.redirectCallbacksSet) { - this.redirectSuccessHandler(response); - } - return; - }, (error) => { - this.silentLogin = false; - this.logger.error("Error occurred during unified cache ATS"); - - // call the loginRedirectHelper later with no user account context - this.loginRedirectHelper(null, request, scopes); - }); - } - // else proceed to login - else { - // call the loginRedirectHelper later with no user account context - this.loginRedirectHelper(null, request, scopes); - } - } - - } - - /** - * @hidden - * @ignore - * Helper function to loginRedirect - * - * @param account - * @param AuthenticationParameters - * @param scopes - */ - private loginRedirectHelper(account: Account, request?: AuthenticationParameters, scopes?: Array) { - // Track login in progress - this.loginInProgress = true; - - this.authorityInstance.resolveEndpointsAsync().then(() => { - - // create the Request to be sent to the Server - let serverAuthenticationRequest = new ServerRequestParameters( - this.authorityInstance, - this.clientId, scopes, - ResponseTypes.id_token, - this.getRedirectUri(), - request && request.state - ); - - // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer - serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); - - // if the user sets the login start page - angular only?? - let loginStartPage = this.cacheStorage.getItem(Constants.angularLoginRequest); - if (!loginStartPage || loginStartPage === "") { - loginStartPage = window.location.href; - } else { - this.cacheStorage.setItem(Constants.angularLoginRequest, ""); - } - - this.updateCacheEntries(serverAuthenticationRequest, account, loginStartPage); - - // build URL to navigate to proceed with the login - let urlNavigate = serverAuthenticationRequest.createNavigateUrl(scopes) + Constants.response_mode_fragment; - - // Redirect user to login URL - this.promptUser(urlNavigate); - }).catch((err) => { - this.logger.warning("could not resolve endpoints"); - this.redirectErrorHandler(ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request && request.state)); - }); + this.acquireTokenInteractive(Constants.interactionTypeRedirect, true, request); } /** @@ -428,120 +346,73 @@ export class UserAgentApplication { * To renew idToken, please pass clientId as the only scope in the Authentication Parameters */ acquireTokenRedirect(request: AuthenticationParameters): void { - // Throw error if callbacks are not set before redirect if (!this.redirectCallbacksSet) { throw ClientConfigurationError.createRedirectCallbacksNotSetError(); } + this.acquireTokenInteractive(Constants.interactionTypeRedirect, false, request); + } - // Validate and filter scopes (the validate function will throw if validation fails) - this.validateInputScope(request.scopes, true); - - // Get the account object if a session exists - const account: Account = request.account || this.getAccount(); - - // If already in progress, do not proceed - if (this.acquireTokenInProgress) { - this.redirectErrorHandler(ClientAuthError.createAcquireTokenInProgressError(), buildResponseStateOnly(this.getAccountState(request.state))); - return; - } - - // If no session exists, prompt the user to login. - if (!account && !(request.sid || request.loginHint)) { - this.logger.info("User login is required"); - throw ClientAuthError.createUserLoginRequiredError(); - } - - let serverAuthenticationRequest: ServerRequestParameters; - const acquireTokenAuthority = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; - - // Track the acquireToken progress - this.acquireTokenInProgress = true; - - acquireTokenAuthority.resolveEndpointsAsync().then(() => { - // On Fulfillment - const responseType = this.getTokenType(account, request.scopes, false); - serverAuthenticationRequest = new ServerRequestParameters( - acquireTokenAuthority, - this.clientId, - request.scopes, - responseType, - this.getRedirectUri(), - request.state - ); - - this.updateCacheEntries(serverAuthenticationRequest, account); - - // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer - serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); - - // Construct urlNavigate - let urlNavigate = serverAuthenticationRequest.createNavigateUrl(request.scopes) + Constants.response_mode_fragment; - - // set state in cache and redirect to urlNavigate - if (urlNavigate) { - this.cacheStorage.setItem(Constants.stateAcquireToken, serverAuthenticationRequest.state, this.inCookie); - window.location.replace(urlNavigate); - } - }).catch((err) => { - this.logger.warning("could not resolve endpoints"); - this.redirectErrorHandler(ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request.state)); + /** + * Use when initiating the login process via opening a popup window in the user's browser + * + * @param {@link (AuthenticationParameters:type)} + * + * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object + */ + loginPopup(request?: AuthenticationParameters): Promise { + return new Promise((resolve, reject) => { + this.acquireTokenInteractive(Constants.interactionTypePopup, true, request, resolve, reject); }); } /** - * @hidden - * @ignore - * Checks if the redirect response is received from the STS. In case of redirect, the url fragment has either id_token, access_token or error. - * @param {string} hash - Hash passed from redirect page. - * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. + * Use when you want to obtain an access_token for your API via opening a popup window in the user's browser + * @param {@link AuthenticationParameters} + * + * To renew idToken, please pass clientId as the only scope in the Authentication Parameters + * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object */ - // TODO - rename this, the name is confusing - isCallback(hash: string): boolean { - hash = this.getHash(hash); - const parameters = Utils.deserialize(hash); - return ( - parameters.hasOwnProperty(Constants.errorDescription) || - parameters.hasOwnProperty(Constants.error) || - parameters.hasOwnProperty(Constants.accessToken) || - parameters.hasOwnProperty(Constants.idToken) - ); + acquireTokenPopup(request: AuthenticationParameters): Promise { + return new Promise((resolve, reject) => { + this.acquireTokenInteractive(Constants.interactionTypePopup, false, request, resolve, reject); + }); } - //#endregion - - //#region Popup Flow + //#region Acquire Token /** - * Use when initiating the login process via opening a popup window in the user's browser - * + * Use when initiating the login process or when you want to obtain an access_token for your API, + * either by redirecting the user's browser window to the authorization endpoint or via opening a popup window in the user's browser. * @param {@link (AuthenticationParameters:type)} * - * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object + * To renew idToken, please pass clientId as the only scope in the Authentication Parameters */ - loginPopup(request?: AuthenticationParameters): Promise { - // Creates navigate url; saves value in cache; redirect user to AAD - return new Promise((resolve, reject) => { - // Fail if login is already in progress - if (this.loginInProgress) { - return reject(ClientAuthError.createLoginInProgressError()); - } + private acquireTokenInteractive(interactionType: InteractionType, isLoginCall: boolean, request?: AuthenticationParameters, resolve?: any, reject?: any): void { + // If already in progress, do not proceed + if (this.acquireTokenInProgress) { + this.errorHandler(interactionType, ClientAuthError.createAcquireTokenInProgressError(), buildResponseStateOnly(this.getAccountState(request && request.state)), reject); + return; + } + let scopes: Array; + // Throw error if callbacks are not set before redirect + if (isLoginCall) { // if extraScopesToConsent is passed, append them to the login request - let scopes: Array = this.appendScopes(request); + scopes = this.appendScopes(request); + } else { + scopes = request.scopes; + } - // Validate and filter scopes (the validate function will throw if validation fails) - this.validateInputScope(scopes, false); + // Validate and filter scopes (the validate function will throw if validation fails) + this.validateInputScope(scopes, !isLoginCall); - let account = this.getAccount(); + // Get the account object if a session exists + const account: Account = (request && request.account && !isLoginCall) ? request.account : this.getAccount(); - // add the prompt parameter to the 'extraQueryParameters' if passed - if (Utils.isSSOParam(request)) { - // if account is not provided, we pass null - this.loginPopupHelper(account, resolve, reject, request, scopes); - } - // else handle the library data - else { - // Extract ADAL id_token if it exists + // If no session exists, prompt the user to login. + if (!account && !Utils.isSSOParam(request)) { + if (isLoginCall) { + // extract ADAL id_token if exists let adalIdToken = this.extractADALIdToken(); // silent login if ADAL id_token is retrieved successfully - SSO @@ -550,199 +421,147 @@ export class UserAgentApplication { let tokenRequest: AuthenticationParameters = this.buildIDTokenRequest(request); this.silentLogin = true; - this.acquireTokenSilent(tokenRequest) - .then(response => { + this.acquireTokenSilent(tokenRequest).then(response => { this.silentLogin = false; this.logger.info("Unified cache call is successful"); - resolve(response); + this.successHandler(interactionType, response, resolve); + return; }, (error) => { this.silentLogin = false; - this.logger.error("Error occurred during unified cache ATS"); - this.loginPopupHelper(null, resolve, reject, request, scopes); + this.logger.error("Error occurred during unified cache ATS: " + error); + + // call the loginRedirectHelper later with no user account context + this.acquireTokenHelper(null, interactionType, isLoginCall, request, scopes, resolve, reject); }); + } else { + this.acquireTokenHelper(null, interactionType, isLoginCall, request, scopes, resolve, reject); } - // else proceed with login - else { - this.loginPopupHelper(null, resolve, reject, request, scopes); - } + } else { + this.logger.info("User login is required"); + throw ClientAuthError.createUserLoginRequiredError(); } - }); + } else { + this.acquireTokenHelper(account, interactionType, isLoginCall, request, scopes, resolve, reject); + } } /** * @hidden - * Helper function to loginPopup + * @ignore + * Helper function to acquireToken * - * @param account - * @param request - * @param resolve - * @param reject - * @param scopes */ - private loginPopupHelper(account: Account, resolve: any, reject: any, request?: AuthenticationParameters, scopes?: Array) { - if (!scopes) { - scopes = [this.clientId]; - } - const scope = scopes.join(" ").toLowerCase(); - - // Generate a popup window - const popUpWindow = this.openWindow("about:blank", "_blank", 1, this, resolve, reject); - if (!popUpWindow) { - // We pass reject in openWindow, we reject there during an error - return; - } - - // Track login progress - this.loginInProgress = true; - - // Resolve endpoint - this.authorityInstance.resolveEndpointsAsync().then(() => { - let serverAuthenticationRequest = new ServerRequestParameters(this.authorityInstance, this.clientId, scopes, ResponseTypes.id_token, this.getRedirectUri(), request && request.state); + private acquireTokenHelper(account: Account, interactionType: InteractionType, isLoginCall: boolean, request?: AuthenticationParameters, scopes?: Array, resolve?: any, reject?: any): void { + // Track the acquireToken progress + this.acquireTokenInProgress = true; - // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer; - serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); + const scope = scopes.join(" ").toLowerCase(); - this.updateCacheEntries(serverAuthenticationRequest, account, window.location.href); + let serverAuthenticationRequest: ServerRequestParameters; + const acquireTokenAuthority = (!isLoginCall && request.authority) ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; - // Cache the state, nonce, and login request data - this.cacheStorage.setItem(Constants.loginRequest, window.location.href, this.inCookie); - this.cacheStorage.setItem(Constants.loginError, ""); + let popUpWindow: Window; + if (interactionType === Constants.interactionTypePopup) { + // Generate a popup window + popUpWindow = this.openWindow("about:blank", "_blank", 1, this, resolve, reject); + if (!popUpWindow) { + // We pass reject in openWindow, we reject there during an error + return; + } + } - this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie); + acquireTokenAuthority.resolveEndpointsAsync().then(() => { + // On Fulfillment + let responseType: string; + let loginStartPage: string; + + if (isLoginCall) { + // if the user sets the login start page - angular only?? + loginStartPage = this.cacheStorage.getItem(Constants.angularLoginRequest); + if (!loginStartPage || loginStartPage === "") { + loginStartPage = window.location.href; + } else { + this.cacheStorage.setItem(Constants.angularLoginRequest, ""); + } - this.cacheStorage.setItem(Constants.msalError, ""); - this.cacheStorage.setItem(Constants.msalErrorDescription, ""); + // Set response type + responseType = ResponseTypes.id_token; + } else { + responseType = this.getTokenType(account, request.scopes, false); + } - // cache authorityKey - this.setAuthorityCache(serverAuthenticationRequest.state, this.authority); + serverAuthenticationRequest = new ServerRequestParameters( + acquireTokenAuthority, + this.clientId, + scopes, + responseType, + this.getRedirectUri(), + request && request.state + ); - // Build the URL to navigate to in the popup window - let urlNavigate = serverAuthenticationRequest.createNavigateUrl(scopes) + Constants.response_mode_fragment; + this.updateCacheEntries(serverAuthenticationRequest, account, loginStartPage); - window.renewStates.push(serverAuthenticationRequest.state); - window.requestType = Constants.login; + // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer + serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); - // Register callback to capture results from server - this.registerCallback(serverAuthenticationRequest.state, scope, resolve, reject); + // Construct urlNavigate + let urlNavigate = serverAuthenticationRequest.createNavigateUrl(request.scopes) + Constants.response_mode_fragment; - // Navigate url in popupWindow - if (popUpWindow) { - this.logger.infoPii("Navigated Popup window to:" + urlNavigate); - popUpWindow.location.href = urlNavigate; - } - }, () => { - // Endpoint resolution failure error - this.logger.info(ClientAuthErrorMessage.endpointResolutionError.code + ":" + ClientAuthErrorMessage.endpointResolutionError.desc); - this.cacheStorage.setItem(Constants.msalError, ClientAuthErrorMessage.endpointResolutionError.code); - this.cacheStorage.setItem(Constants.msalErrorDescription, ClientAuthErrorMessage.endpointResolutionError.desc); + // set state in cache + if (interactionType === Constants.interactionTypeRedirect) { + if (!isLoginCall) { + this.cacheStorage.setItem(Constants.stateAcquireToken, serverAuthenticationRequest.state, this.inCookie); + } + } else { + window.renewStates.push(serverAuthenticationRequest.state); + if (isLoginCall) { + window.requestType = Constants.login; + } else { + window.requestType = Constants.renewToken; + } - // reject that is passed in - REDO this in the subsequent refactor, passing reject is confusing - if (reject) { - reject(ClientAuthError.createEndpointResolutionError()); + // Register callback to capture results from server + this.registerCallback(serverAuthenticationRequest.state, scope, resolve, reject); } - // Close the popup window + // prompt user for interaction + this.promptUser(urlNavigate, popUpWindow); + }).catch((err) => { + this.logger.warning("could not resolve endpoints"); + this.errorHandler(interactionType, ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request.state), reject); if (popUpWindow) { popUpWindow.close(); } - // this is an all catch for any failure for the above code except the specific 'reject' call - }).catch((err) => { - this.logger.warning("could not resolve endpoints"); - reject(ClientAuthError.createEndpointResolutionError(err.toString)); }); } - /** - * Use when you want to obtain an access_token for your API via opening a popup window in the user's browser - * @param {@link AuthenticationParameters} - * - * To renew idToken, please pass clientId as the only scope in the Authentication Parameters - * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object - */ - acquireTokenPopup(request: AuthenticationParameters): Promise { - return new Promise((resolve, reject) => { - // Validate and filter scopes (the validate function will throw if validation fails) - this.validateInputScope(request.scopes, true); - - const scope = request.scopes.join(" ").toLowerCase(); - - // Get the account object if a session exists - const account: Account = request.account || this.getAccount(); - - // If already in progress, throw an error and reject the request - if (this.acquireTokenInProgress) { - return reject(ClientAuthError.createAcquireTokenInProgressError()); - } - - // If no session exists, prompt the user to login. - if (!account && !(request.sid || request.loginHint)) { - this.logger.info("User login is required"); - return reject(ClientAuthError.createUserLoginRequiredError()); - } - - // track the acquireToken progress - this.acquireTokenInProgress = true; - - let serverAuthenticationRequest: ServerRequestParameters; - const acquireTokenAuthority = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; - - // Open the popup window - const popUpWindow = this.openWindow("about:blank", "_blank", 1, this, resolve, reject); - if (!popUpWindow) { - // We pass reject to openWindow, so we are rejecting there. - return; - } - - acquireTokenAuthority.resolveEndpointsAsync().then(() => { - // On fullfillment - const responseType = this.getTokenType(account, request.scopes, false); - serverAuthenticationRequest = new ServerRequestParameters( - acquireTokenAuthority, - this.clientId, - request.scopes, - responseType, - this.getRedirectUri(), - request.state - ); - - // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer - serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); - - this.updateCacheEntries(serverAuthenticationRequest, account); - - // Construct the urlNavigate - let urlNavigate = serverAuthenticationRequest.createNavigateUrl(request.scopes) + Constants.response_mode_fragment; + //#endregion - window.renewStates.push(serverAuthenticationRequest.state); - window.requestType = Constants.renewToken; - this.registerCallback(serverAuthenticationRequest.state, scope, resolve, reject); + //#region Redirect Flow - // open popup window to urlNavigate - if (popUpWindow) { - popUpWindow.location.href = urlNavigate; - } + /** + * @hidden + * @ignore + * Checks if the redirect response is received from the STS. In case of redirect, the url fragment has either id_token, access_token or error. + * @param {string} hash - Hash passed from redirect page. + * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. + */ + // TODO - rename this, the name is confusing + isCallback(hash: string): boolean { + hash = this.getHash(hash); + const parameters = Utils.deserialize(hash); + return ( + parameters.hasOwnProperty(Constants.errorDescription) || + parameters.hasOwnProperty(Constants.error) || + parameters.hasOwnProperty(Constants.accessToken) || + parameters.hasOwnProperty(Constants.idToken) + ); + } - }, () => { - // Endpoint resolution failure error - this.logger.info(ClientAuthErrorMessage.endpointResolutionError.code + ":" + ClientAuthErrorMessage.endpointResolutionError.desc); - this.cacheStorage.setItem(Constants.msalError, ClientAuthErrorMessage.endpointResolutionError.code); - this.cacheStorage.setItem(Constants.msalErrorDescription, ClientAuthErrorMessage.endpointResolutionError.desc); + //#endregion - // reject that is passed in - REDO this in the subsequent refactor, passing reject is confusing - if (reject) { - reject(ClientAuthError.createEndpointResolutionError()); - } - if (popUpWindow) { - popUpWindow.close(); - } - // this is an all catch for any failure for the above code except the specific 'reject' call - }).catch((err) => { - this.logger.warning("could not resolve endpoints"); - reject(ClientAuthError.createEndpointResolutionError(err.toString())); - }); - }); - } + //#region Popup Window Creation /** * @hidden @@ -1157,11 +976,12 @@ export class UserAgentApplication { * Used to redirect the browser to the STS authorization endpoint * @param {string} urlNavigate - URL of the authorization endpoint */ - private promptUser(urlNavigate: string) { + private promptUser(urlNavigate: string, popupWindow?: Window) { // Navigate if valid URL if (urlNavigate && !Utils.isEmpty(urlNavigate)) { - this.logger.infoPii("Navigate to:" + urlNavigate); - window.location.replace(urlNavigate); + let navigateWindow: Window = popupWindow ? popupWindow : window; + popupWindow ? this.logger.infoPii("Navigated Popup window to:" + urlNavigate) : this.logger.infoPii("Navigate to:" + urlNavigate); + navigateWindow.location.replace(urlNavigate); } else { this.logger.info("Navigate url is empty"); @@ -2451,7 +2271,6 @@ export class UserAgentApplication { this.cacheStorage.setItem(Constants.loginError, ""); this.cacheStorage.setItem(Constants.stateLogin, serverAuthenticationRequest.state, this.inCookie); - this.cacheStorage.setItem(Constants.nonceIdToken, serverAuthenticationRequest.nonce, this.inCookie); this.cacheStorage.setItem(Constants.msalError, ""); this.cacheStorage.setItem(Constants.msalErrorDescription, ""); From d6a43fe49caeddb90570e582a222392bfee6edca Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Wed, 26 Jun 2019 16:04:35 -0700 Subject: [PATCH 02/20] Moving around code, renaming isCallback --- lib/msal-core/src/UserAgentApplication.ts | 303 +++++++++++----------- 1 file changed, 149 insertions(+), 154 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index e48707e6ff..bb787fa6a3 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -240,7 +240,7 @@ export class UserAgentApplication { window.msal = this; const urlHash = window.location.hash; - const isCallback = this.isCallback(urlHash); + const isCallback = this.urlContainsHash(urlHash); // On the server 302 - Redirect, handle this if (!this.config.framework.isAngular) { @@ -536,27 +536,125 @@ export class UserAgentApplication { }); } - //#endregion - - //#region Redirect Flow - /** - * @hidden - * @ignore - * Checks if the redirect response is received from the STS. In case of redirect, the url fragment has either id_token, access_token or error. - * @param {string} hash - Hash passed from redirect page. - * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. + * Use this function to obtain a token before every call to the API / resource provider + * + * MSAL return's a cached token when available + * Or it send's a request to the STS to obtain a new token using a hidden iframe. + * + * @param {@link AuthenticationParameters} + * + * To renew idToken, please pass clientId as the only scope in the Authentication Parameters + * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object + * */ - // TODO - rename this, the name is confusing - isCallback(hash: string): boolean { - hash = this.getHash(hash); - const parameters = Utils.deserialize(hash); - return ( - parameters.hasOwnProperty(Constants.errorDescription) || - parameters.hasOwnProperty(Constants.error) || - parameters.hasOwnProperty(Constants.accessToken) || - parameters.hasOwnProperty(Constants.idToken) - ); + @resolveTokenOnlyIfOutOfIframe + acquireTokenSilent(request: AuthenticationParameters): Promise { + return new Promise((resolve, reject) => { + + // Validate and filter scopes (the validate function will throw if validation fails) + this.validateInputScope(request.scopes, true); + + const scope = request.scopes.join(" ").toLowerCase(); + + // if the developer passes an account give him the priority + const account: Account = request.account || this.getAccount(); + + // extract if there is an adalIdToken stashed in the cache + const adalIdToken = this.cacheStorage.getItem(Constants.adalIdToken); + + //if there is no account logged in and no login_hint/sid is passed in the request + if (!account && !(request.sid || request.loginHint) && Utils.isEmpty(adalIdToken) ) { + this.logger.info("User login is required"); + return reject(ClientAuthError.createUserLoginRequiredError()); + } + + const responseType = this.getTokenType(account, request.scopes, true); + + let serverAuthenticationRequest = new ServerRequestParameters( + AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority), + this.clientId, + request.scopes, + responseType, + this.getRedirectUri(), + request && request.state + ); + + // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer + if (Utils.isSSOParam(request) || account) { + serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); + } + //if user didn't pass login_hint/sid and adal's idtoken is present, extract the login_hint from the adalIdToken + else if (!account && !Utils.isEmpty(adalIdToken)) { + // if adalIdToken exists, extract the SSO info from the same + const adalIdTokenObject = Utils.extractIdToken(adalIdToken); + this.logger.verbose("ADAL's idToken exists. Extracting login information from ADAL's idToken "); + serverAuthenticationRequest = this.populateQueryParams(account, null, serverAuthenticationRequest, adalIdTokenObject); + } + let userContainedClaims = request.claimsRequest || serverAuthenticationRequest.claimsValue; + + let authErr: AuthError; + let cacheResultResponse; + + if (!userContainedClaims) { + try { + cacheResultResponse = this.getCachedToken(serverAuthenticationRequest, account); + } catch (e) { + authErr = e; + } + } + + // resolve/reject based on cacheResult + if (cacheResultResponse) { + this.logger.info("Token is already in cache for scope:" + scope); + resolve(cacheResultResponse); + return null; + } + else if (authErr) { + this.logger.infoPii(authErr.errorCode + ":" + authErr.errorMessage); + reject(authErr); + return null; + } + // else proceed with login + else { + if (userContainedClaims) { + this.logger.verbose("Skipped cache lookup since claims were given."); + } else { + this.logger.verbose("Token is not in cache for scope:" + scope); + } + // Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the api. + if (!serverAuthenticationRequest.authorityInstance) { + serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; + } + // cache miss + return serverAuthenticationRequest.authorityInstance.resolveEndpointsAsync() + .then(() => { + // refresh attempt with iframe + // Already renewing for this scope, callback when we get the token. + if (window.activeRenewals[scope]) { + this.logger.verbose("Renew token for scope: " + scope + " is in progress. Registering callback"); + // Active renewals contains the state for each renewal. + this.registerCallback(window.activeRenewals[scope], scope, resolve, reject); + } + else { + if (request.scopes && request.scopes.indexOf(this.clientId) > -1 && request.scopes.length === 1) { + // App uses idToken to send to api endpoints + // Default scope is tracked as clientId to store this token + this.logger.verbose("renewing idToken"); + this.renewIdToken(request.scopes, resolve, reject, account, serverAuthenticationRequest); + } else { + // renew access token + this.logger.verbose("renewing accesstoken"); + this.renewToken(request.scopes, resolve, reject, account, serverAuthenticationRequest); + } + } + }).catch((err) => { + this.logger.warning("could not resolve endpoints"); + reject(ClientAuthError.createEndpointResolutionError(err.toString())); + return null; + }); + } + }); } //#endregion @@ -690,128 +788,7 @@ export class UserAgentApplication { //#endregion - //#region Silent Flow - - /** - * Use this function to obtain a token before every call to the API / resource provider - * - * MSAL return's a cached token when available - * Or it send's a request to the STS to obtain a new token using a hidden iframe. - * - * @param {@link AuthenticationParameters} - * - * To renew idToken, please pass clientId as the only scope in the Authentication Parameters - * @returns {Promise.} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse} object - * - */ - @resolveTokenOnlyIfOutOfIframe - acquireTokenSilent(request: AuthenticationParameters): Promise { - return new Promise((resolve, reject) => { - - // Validate and filter scopes (the validate function will throw if validation fails) - this.validateInputScope(request.scopes, true); - - const scope = request.scopes.join(" ").toLowerCase(); - - // if the developer passes an account give him the priority - const account: Account = request.account || this.getAccount(); - - // extract if there is an adalIdToken stashed in the cache - const adalIdToken = this.cacheStorage.getItem(Constants.adalIdToken); - - //if there is no account logged in and no login_hint/sid is passed in the request - if (!account && !(request.sid || request.loginHint) && Utils.isEmpty(adalIdToken) ) { - this.logger.info("User login is required"); - return reject(ClientAuthError.createUserLoginRequiredError()); - } - - const responseType = this.getTokenType(account, request.scopes, true); - - let serverAuthenticationRequest = new ServerRequestParameters( - AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority), - this.clientId, - request.scopes, - responseType, - this.getRedirectUri(), - request && request.state - ); - - // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer - if (Utils.isSSOParam(request) || account) { - serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); - } - //if user didn't pass login_hint/sid and adal's idtoken is present, extract the login_hint from the adalIdToken - else if (!account && !Utils.isEmpty(adalIdToken)) { - // if adalIdToken exists, extract the SSO info from the same - const adalIdTokenObject = Utils.extractIdToken(adalIdToken); - this.logger.verbose("ADAL's idToken exists. Extracting login information from ADAL's idToken "); - serverAuthenticationRequest = this.populateQueryParams(account, null, serverAuthenticationRequest, adalIdTokenObject); - } - let userContainedClaims = request.claimsRequest || serverAuthenticationRequest.claimsValue; - - let authErr: AuthError; - let cacheResultResponse; - - if (!userContainedClaims) { - try { - cacheResultResponse = this.getCachedToken(serverAuthenticationRequest, account); - } catch (e) { - authErr = e; - } - } - - // resolve/reject based on cacheResult - if (cacheResultResponse) { - this.logger.info("Token is already in cache for scope:" + scope); - resolve(cacheResultResponse); - return null; - } - else if (authErr) { - this.logger.infoPii(authErr.errorCode + ":" + authErr.errorMessage); - reject(authErr); - return null; - } - // else proceed with login - else { - if (userContainedClaims) { - this.logger.verbose("Skipped cache lookup since claims were given."); - } else { - this.logger.verbose("Token is not in cache for scope:" + scope); - } - // Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the api. - if (!serverAuthenticationRequest.authorityInstance) { - serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; - } - // cache miss - return serverAuthenticationRequest.authorityInstance.resolveEndpointsAsync() - .then(() => { - // refresh attempt with iframe - // Already renewing for this scope, callback when we get the token. - if (window.activeRenewals[scope]) { - this.logger.verbose("Renew token for scope: " + scope + " is in progress. Registering callback"); - // Active renewals contains the state for each renewal. - this.registerCallback(window.activeRenewals[scope], scope, resolve, reject); - } - else { - if (request.scopes && request.scopes.indexOf(this.clientId) > -1 && request.scopes.length === 1) { - // App uses idToken to send to api endpoints - // Default scope is tracked as clientId to store this token - this.logger.verbose("renewing idToken"); - this.renewIdToken(request.scopes, resolve, reject, account, serverAuthenticationRequest); - } else { - // renew access token - this.logger.verbose("renewing accesstoken"); - this.renewToken(request.scopes, resolve, reject, account, serverAuthenticationRequest); - } - } - }).catch((err) => { - this.logger.warning("could not resolve endpoints"); - reject(ClientAuthError.createEndpointResolutionError(err.toString())); - return null; - }); - } - }); - } + //#region Iframe Management /** * @hidden @@ -830,18 +807,6 @@ export class UserAgentApplication { return window.parent !== window && window.parent.msal; } - /** - * @hidden - */ - private isInteractionRequired(errorString: string) : boolean { - if (errorString.indexOf("interaction_required") !== -1 || - errorString.indexOf("consent_required") !== -1 || - errorString.indexOf("login_required") !== -1) { - return true; - } - return false; - } - /** * @hidden * Calling _loadFrame but with a timeout to signal failure in loadframeStatus. Callbacks are left. @@ -928,6 +893,18 @@ export class UserAgentApplication { //#region General Helpers + /** + * @hidden + */ + private isInteractionRequired(errorString: string) : boolean { + if (errorString.indexOf("interaction_required") !== -1 || + errorString.indexOf("consent_required") !== -1 || + errorString.indexOf("login_required") !== -1) { + return true; + } + return false; + } + /** * @hidden * @@ -1096,6 +1073,24 @@ export class UserAgentApplication { //#region Response + /** + * @hidden + * @ignore + * Checks if the redirect response is received from the STS. In case of redirect, the url fragment has either id_token, access_token or error. + * @param {string} hash - Hash passed from redirect page. + * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. + */ + urlContainsHash(hash: string): boolean { + hash = this.getHash(hash); + const parameters = Utils.deserialize(hash); + return ( + parameters.hasOwnProperty(Constants.errorDescription) || + parameters.hasOwnProperty(Constants.error) || + parameters.hasOwnProperty(Constants.accessToken) || + parameters.hasOwnProperty(Constants.idToken) + ); + } + /** * @hidden * Used to call the constructor callback with the token/error From 979ec39cadfcfa2c906eb1ca6e62b3cf64111073 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 27 Jun 2019 11:44:42 -0700 Subject: [PATCH 03/20] Fixing failed tests, adding final changes for renaming --- lib/msal-core/src/UserAgentApplication.ts | 20 +++++++++++-------- .../test/UserAgentApplication.spec.ts | 10 +++++----- 2 files changed, 17 insertions(+), 13 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index bb787fa6a3..ef3cd78e2a 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -388,9 +388,13 @@ export class UserAgentApplication { * To renew idToken, please pass clientId as the only scope in the Authentication Parameters */ private acquireTokenInteractive(interactionType: InteractionType, isLoginCall: boolean, request?: AuthenticationParameters, resolve?: any, reject?: any): void { - // If already in progress, do not proceed - if (this.acquireTokenInProgress) { - this.errorHandler(interactionType, ClientAuthError.createAcquireTokenInProgressError(), buildResponseStateOnly(this.getAccountState(request && request.state)), reject); + + // If already in progress, do not proceed + if (this.loginInProgress || this.acquireTokenInProgress) { + this.errorHandler(interactionType, + this.loginInProgress ? ClientAuthError.createLoginInProgressError() : ClientAuthError.createAcquireTokenInProgressError(), + buildResponseStateOnly(this.getAccountState(request && request.state)), + reject); return; } @@ -454,9 +458,9 @@ export class UserAgentApplication { */ private acquireTokenHelper(account: Account, interactionType: InteractionType, isLoginCall: boolean, request?: AuthenticationParameters, scopes?: Array, resolve?: any, reject?: any): void { // Track the acquireToken progress - this.acquireTokenInProgress = true; + isLoginCall ? this.loginInProgress = true : this.acquireTokenInProgress = true; - const scope = scopes.join(" ").toLowerCase(); + const scope = scopes ? scopes.join(" ").toLowerCase() : this.clientId.toLowerCase(); let serverAuthenticationRequest: ServerRequestParameters; const acquireTokenAuthority = (!isLoginCall && request.authority) ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; @@ -526,7 +530,7 @@ export class UserAgentApplication { } // prompt user for interaction - this.promptUser(urlNavigate, popUpWindow); + this.navigateWindow(urlNavigate, popUpWindow); }).catch((err) => { this.logger.warning("could not resolve endpoints"); this.errorHandler(interactionType, ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request.state), reject); @@ -953,7 +957,7 @@ export class UserAgentApplication { * Used to redirect the browser to the STS authorization endpoint * @param {string} urlNavigate - URL of the authorization endpoint */ - private promptUser(urlNavigate: string, popupWindow?: Window) { + private navigateWindow(urlNavigate: string, popupWindow?: Window) { // Navigate if valid URL if (urlNavigate && !Utils.isEmpty(urlNavigate)) { let navigateWindow: Window = popupWindow ? popupWindow : window; @@ -1034,7 +1038,7 @@ export class UserAgentApplication { const urlNavigate = authority.EndSessionEndpoint ? `${authority.EndSessionEndpoint}?${logout}` : `${this.authority}oauth2/v2.0/logout?${logout}`; - this.promptUser(urlNavigate); + this.navigateWindow(urlNavigate); }); } diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts index 03b4989661..e21b8dc2fd 100644 --- a/lib/msal-core/test/UserAgentApplication.spec.ts +++ b/lib/msal-core/test/UserAgentApplication.spec.ts @@ -925,11 +925,11 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests that isCallback correctly identifies url hash", function (done) { - expect(msal.isCallback("not a callback")).to.be.false; - expect(msal.isCallback("#error_description=someting_wrong")).to.be.true; - expect(msal.isCallback("#/error_description=someting_wrong")).to.be.true; - expect(msal.isCallback("#access_token=token123")).to.be.true; - expect(msal.isCallback("#id_token=idtoken234")).to.be.true; + expect(msal.urlContainsHash("not a callback")).to.be.false; + expect(msal.urlContainsHash("#error_description=someting_wrong")).to.be.true; + expect(msal.urlContainsHash("#/error_description=someting_wrong")).to.be.true; + expect(msal.urlContainsHash("#access_token=token123")).to.be.true; + expect(msal.urlContainsHash("#id_token=idtoken234")).to.be.true; done(); }); }); From 9a21af4feb35043bfb2077820661283d8c11166c Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Tue, 2 Jul 2019 14:15:05 -0700 Subject: [PATCH 04/20] Made changes based on feedback, moved functions to separate classes for better separation of duty --- lib/msal-core/src/UserAgentApplication.ts | 43 ++++--------------- lib/msal-core/src/Utils.ts | 18 ++++++++ .../src/error/InteractionRequiredAuthError.ts | 8 ++++ .../test/UserAgentApplication.spec.ts | 10 ++--- 4 files changed, 39 insertions(+), 40 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index ef3cd78e2a..d005ae020d 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -240,7 +240,7 @@ export class UserAgentApplication { window.msal = this; const urlHash = window.location.hash; - const isCallback = this.urlContainsHash(urlHash); + const isCallback = this.isStsResponse(urlHash); // On the server 302 - Redirect, handle this if (!this.config.framework.isAngular) { @@ -897,18 +897,6 @@ export class UserAgentApplication { //#region General Helpers - /** - * @hidden - */ - private isInteractionRequired(errorString: string) : boolean { - if (errorString.indexOf("interaction_required") !== -1 || - errorString.indexOf("consent_required") !== -1 || - errorString.indexOf("login_required") !== -1) { - return true; - } - return false; - } - /** * @hidden * @@ -961,7 +949,8 @@ export class UserAgentApplication { // Navigate if valid URL if (urlNavigate && !Utils.isEmpty(urlNavigate)) { let navigateWindow: Window = popupWindow ? popupWindow : window; - popupWindow ? this.logger.infoPii("Navigated Popup window to:" + urlNavigate) : this.logger.infoPii("Navigate to:" + urlNavigate); + let logMessage: string = popupWindow ? "Navigated Popup window to:" + urlNavigate : "Navigate to:" + urlNavigate; + this.logger.infoPii(logMessage); navigateWindow.location.replace(urlNavigate); } else { @@ -1084,9 +1073,8 @@ export class UserAgentApplication { * @param {string} hash - Hash passed from redirect page. * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. */ - urlContainsHash(hash: string): boolean { - hash = this.getHash(hash); - const parameters = Utils.deserialize(hash); + isStsResponse(hash: string): boolean { + const parameters = this.deserializeHash(hash); return ( parameters.hasOwnProperty(Constants.errorDescription) || parameters.hasOwnProperty(Constants.error) || @@ -1234,7 +1222,7 @@ export class UserAgentApplication { * @param hash */ private deserializeHash(hash: string) { - hash = this.getHash(hash); + hash = Utils.getHashFromUrl(hash); return Utils.deserialize(hash); } @@ -1624,7 +1612,8 @@ export class UserAgentApplication { acquireTokenAccountKey = Storage.generateAcquireTokenAccountKey(accountId, stateInfo.state); } - if (this.isInteractionRequired(hashParams[Constants.error]) || this.isInteractionRequired(hashParams[Constants.errorDescription])) { + if (InteractionRequiredAuthError.isInteractionRequiredError(hashParams[Constants.error]) || + InteractionRequiredAuthError.isInteractionRequiredError(hashParams[Constants.errorDescription])) { error = new InteractionRequiredAuthError(hashParams[Constants.error], hashParams[Constants.errorDescription]); } else { error = new ServerError(hashParams[Constants.error], hashParams[Constants.errorDescription]); @@ -2147,22 +2136,6 @@ export class UserAgentApplication { //#region String Util (Should be extracted to Utils.ts) - /** - * @hidden - * @ignore - * - * Returns the anchor part(#) of the URL - */ - private getHash(hash: string): string { - if (hash.indexOf("#/") > -1) { - hash = hash.substring(hash.indexOf("#/") + 2); - } else if (hash.indexOf("#") > -1) { - hash = hash.substring(1); - } - - return hash; - } - /** * @hidden * @ignore diff --git a/lib/msal-core/src/Utils.ts b/lib/msal-core/src/Utils.ts index b44ebd08fc..39785e37b7 100644 --- a/lib/msal-core/src/Utils.ts +++ b/lib/msal-core/src/Utils.ts @@ -538,6 +538,24 @@ export class Utils { return url; } + /** + * @hidden + * @ignore + * + * Returns the anchor part(#) of the URL + */ + static getHashFromUrl(urlStringOrFragment: string): string { + let hash = urlStringOrFragment; + const hashIndex1 = urlStringOrFragment.indexOf("#"); + const hashIndex2 = urlStringOrFragment.indexOf("#/"); + if (hashIndex2 > -1) { + hash = urlStringOrFragment.substring(hashIndex2 + 2); + } else if (hashIndex1 > -1) { + hash = urlStringOrFragment.substring(hashIndex1 + 1); + } + return hash; + } + //#endregion //#region ExtraQueryParameters Processing (Extract?) diff --git a/lib/msal-core/src/error/InteractionRequiredAuthError.ts b/lib/msal-core/src/error/InteractionRequiredAuthError.ts index df62b730ba..2ea3297702 100644 --- a/lib/msal-core/src/error/InteractionRequiredAuthError.ts +++ b/lib/msal-core/src/error/InteractionRequiredAuthError.ts @@ -27,6 +27,14 @@ export class InteractionRequiredAuthError extends ServerError { Object.setPrototypeOf(this, InteractionRequiredAuthError.prototype); } + static isInteractionRequiredError(errorString: string) : boolean { + return ( + errorString.indexOf("interaction_required") !== -1 || + errorString.indexOf("consent_required") !== -1 || + errorString.indexOf("login_required") !== -1 + ); + } + static createLoginRequiredAuthError(errorDesc: string): InteractionRequiredAuthError { return new InteractionRequiredAuthError(InteractionRequiredAuthErrorMessage.loginRequired.code, errorDesc); } diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts index e21b8dc2fd..70f658cf29 100644 --- a/lib/msal-core/test/UserAgentApplication.spec.ts +++ b/lib/msal-core/test/UserAgentApplication.spec.ts @@ -925,11 +925,11 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests that isCallback correctly identifies url hash", function (done) { - expect(msal.urlContainsHash("not a callback")).to.be.false; - expect(msal.urlContainsHash("#error_description=someting_wrong")).to.be.true; - expect(msal.urlContainsHash("#/error_description=someting_wrong")).to.be.true; - expect(msal.urlContainsHash("#access_token=token123")).to.be.true; - expect(msal.urlContainsHash("#id_token=idtoken234")).to.be.true; + expect(msal.isStsResponse("not a callback")).to.be.false; + expect(msal.isStsResponse("#error_description=someting_wrong")).to.be.true; + expect(msal.isStsResponse("#/error_description=someting_wrong")).to.be.true; + expect(msal.isStsResponse("#access_token=token123")).to.be.true; + expect(msal.isStsResponse("#id_token=idtoken234")).to.be.true; done(); }); }); From 5cb5a8a2d4203c32950d1180cac7482eb7cc86cd Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Tue, 2 Jul 2019 14:17:55 -0700 Subject: [PATCH 05/20] Changed strings to variables --- .../src/error/InteractionRequiredAuthError.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/msal-core/src/error/InteractionRequiredAuthError.ts b/lib/msal-core/src/error/InteractionRequiredAuthError.ts index 2ea3297702..f3d76df076 100644 --- a/lib/msal-core/src/error/InteractionRequiredAuthError.ts +++ b/lib/msal-core/src/error/InteractionRequiredAuthError.ts @@ -4,15 +4,15 @@ import { ServerError } from "./ServerError"; export const InteractionRequiredAuthErrorMessage = { - loginRequired: { - code: "login_required" - }, interactionRequired: { code: "interaction_required" }, consentRequired: { code: "consent_required" }, + loginRequired: { + code: "login_required" + }, }; /** @@ -29,9 +29,9 @@ export class InteractionRequiredAuthError extends ServerError { static isInteractionRequiredError(errorString: string) : boolean { return ( - errorString.indexOf("interaction_required") !== -1 || - errorString.indexOf("consent_required") !== -1 || - errorString.indexOf("login_required") !== -1 + errorString.indexOf(InteractionRequiredAuthErrorMessage.interactionRequired.code) !== -1 || + errorString.indexOf(InteractionRequiredAuthErrorMessage.consentRequired.code) !== -1 || + errorString.indexOf(InteractionRequiredAuthErrorMessage.loginRequired.code) !== -1 ); } From b7c063681f66f0c9e763ceb6d92058b878e42592 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Tue, 2 Jul 2019 20:35:25 -0700 Subject: [PATCH 06/20] Added changes from feedback during review meeting --- lib/msal-core/src/UserAgentApplication.ts | 56 +++++++++---------- lib/msal-core/src/Utils.ts | 6 +- lib/msal-core/src/error/ClientAuthError.ts | 9 +++ .../test/UserAgentApplication.spec.ts | 10 ++-- 4 files changed, 44 insertions(+), 37 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index d005ae020d..1b0a674612 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -240,7 +240,7 @@ export class UserAgentApplication { window.msal = this; const urlHash = window.location.hash; - const isCallback = this.isStsResponse(urlHash); + const isCallback = this.isCallback(urlHash); // On the server 302 - Redirect, handle this if (!this.config.framework.isAngular) { @@ -310,8 +310,10 @@ export class UserAgentApplication { } else if (this.authResponseCallback) { this.authResponseCallback(null, response); } - } else { + } else if (interactionType === Constants.interactionTypePopup) { resolve(response); + } else { + throw ClientAuthError.createInvalidInteractionTypeError(); } } @@ -322,8 +324,10 @@ export class UserAgentApplication { } else { this.authResponseCallback(authErr, response); } - } else { + } else if (interactionType === Constants.interactionTypePopup) { reject(authErr); + } else { + throw ClientAuthError.createInvalidInteractionTypeError(); } } @@ -333,6 +337,7 @@ export class UserAgentApplication { * @param {@link (AuthenticationParameters:type)} */ loginRedirect(request?: AuthenticationParameters): void { + // Throw error if callbacks are not set before redirect if (!this.redirectCallbacksSet) { throw ClientConfigurationError.createRedirectCallbacksNotSetError(); } @@ -346,6 +351,7 @@ export class UserAgentApplication { * To renew idToken, please pass clientId as the only scope in the Authentication Parameters */ acquireTokenRedirect(request: AuthenticationParameters): void { + // Throw error if callbacks are not set before redirect if (!this.redirectCallbacksSet) { throw ClientConfigurationError.createRedirectCallbacksNotSetError(); } @@ -391,21 +397,17 @@ export class UserAgentApplication { // If already in progress, do not proceed if (this.loginInProgress || this.acquireTokenInProgress) { + let thrownError = this.loginInProgress ? ClientAuthError.createLoginInProgressError() : ClientAuthError.createAcquireTokenInProgressError(); + let stateOnlyResponse = buildResponseStateOnly(this.getAccountState(request && request.state)); this.errorHandler(interactionType, - this.loginInProgress ? ClientAuthError.createLoginInProgressError() : ClientAuthError.createAcquireTokenInProgressError(), - buildResponseStateOnly(this.getAccountState(request && request.state)), + thrownError, + stateOnlyResponse, reject); return; } - let scopes: Array; - // Throw error if callbacks are not set before redirect - if (isLoginCall) { - // if extraScopesToConsent is passed, append them to the login request - scopes = this.appendScopes(request); - } else { - scopes = request.scopes; - } + // if extraScopesToConsent is passed in loginCall, append them to the login request + let scopes: Array = isLoginCall ? this.appendScopes(request) : request.scopes; // Validate and filter scopes (the validate function will throw if validation fails) this.validateInputScope(scopes, !isLoginCall); @@ -458,7 +460,11 @@ export class UserAgentApplication { */ private acquireTokenHelper(account: Account, interactionType: InteractionType, isLoginCall: boolean, request?: AuthenticationParameters, scopes?: Array, resolve?: any, reject?: any): void { // Track the acquireToken progress - isLoginCall ? this.loginInProgress = true : this.acquireTokenInProgress = true; + if (isLoginCall) { + this.loginInProgress = true; + } else { + this.acquireTokenInProgress = true; + } const scope = scopes ? scopes.join(" ").toLowerCase() : this.clientId.toLowerCase(); @@ -519,11 +525,7 @@ export class UserAgentApplication { } } else { window.renewStates.push(serverAuthenticationRequest.state); - if (isLoginCall) { - window.requestType = Constants.login; - } else { - window.requestType = Constants.renewToken; - } + window.requestType = isLoginCall ? Constants.login : Constants.renewToken; // Register callback to capture results from server this.registerCallback(serverAuthenticationRequest.state, scope, resolve, reject); @@ -561,7 +563,7 @@ export class UserAgentApplication { const scope = request.scopes.join(" ").toLowerCase(); - // if the developer passes an account give him the priority + // if the developer passes an account, give that account the priority const account: Account = request.account || this.getAccount(); // extract if there is an adalIdToken stashed in the cache @@ -621,11 +623,9 @@ export class UserAgentApplication { } // else proceed with login else { - if (userContainedClaims) { - this.logger.verbose("Skipped cache lookup since claims were given."); - } else { - this.logger.verbose("Token is not in cache for scope:" + scope); - } + let logMessage = userContainedClaims ? "Skipped cache lookup since claims were given." : "Token is not in cache for scope:" + scope; + this.logger.verbose(logMessage); + // Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the api. if (!serverAuthenticationRequest.authorityInstance) { serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; @@ -1073,7 +1073,7 @@ export class UserAgentApplication { * @param {string} hash - Hash passed from redirect page. * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. */ - isStsResponse(hash: string): boolean { + isCallback(hash: string): boolean { const parameters = this.deserializeHash(hash); return ( parameters.hasOwnProperty(Constants.errorDescription) || @@ -1221,8 +1221,8 @@ export class UserAgentApplication { * Returns deserialized portion of URL hash * @param hash */ - private deserializeHash(hash: string) { - hash = Utils.getHashFromUrl(hash); + private deserializeHash(urlFragment: string) { + let hash = Utils.getHashFromUrl(urlFragment); return Utils.deserialize(hash); } diff --git a/lib/msal-core/src/Utils.ts b/lib/msal-core/src/Utils.ts index 39785e37b7..094709796b 100644 --- a/lib/msal-core/src/Utils.ts +++ b/lib/msal-core/src/Utils.ts @@ -545,15 +545,13 @@ export class Utils { * Returns the anchor part(#) of the URL */ static getHashFromUrl(urlStringOrFragment: string): string { - let hash = urlStringOrFragment; const hashIndex1 = urlStringOrFragment.indexOf("#"); const hashIndex2 = urlStringOrFragment.indexOf("#/"); if (hashIndex2 > -1) { - hash = urlStringOrFragment.substring(hashIndex2 + 2); + return urlStringOrFragment.substring(hashIndex2 + 2); } else if (hashIndex1 > -1) { - hash = urlStringOrFragment.substring(hashIndex1 + 1); + return urlStringOrFragment.substring(hashIndex1 + 1); } - return hash; } //#endregion diff --git a/lib/msal-core/src/error/ClientAuthError.ts b/lib/msal-core/src/error/ClientAuthError.ts index 5a0365e32b..1fe8e82197 100644 --- a/lib/msal-core/src/error/ClientAuthError.ts +++ b/lib/msal-core/src/error/ClientAuthError.ts @@ -82,6 +82,10 @@ export const ClientAuthErrorMessage = { tokenEncodingError: { code: "token_encoding_error", desc: "The token to be decoded is not encoded correctly." + }, + invalidInteractionType: { + code: "invalid_interaction_type", + desc: "The interaction type passed to the handler was incorrect or unknown" } }; @@ -199,4 +203,9 @@ export class ClientAuthError extends AuthError { return new ClientAuthError(ClientAuthErrorMessage.tokenEncodingError.code, `${ClientAuthErrorMessage.tokenEncodingError.desc} Attempted to decode: ${incorrectlyEncodedToken}`); } + + static createInvalidInteractionTypeError() : ClientAuthError { + return new ClientAuthError(ClientAuthErrorMessage.invalidInteractionType.code, + ClientAuthErrorMessage.invalidInteractionType.desc); + } } diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts index 70f658cf29..03b4989661 100644 --- a/lib/msal-core/test/UserAgentApplication.spec.ts +++ b/lib/msal-core/test/UserAgentApplication.spec.ts @@ -925,11 +925,11 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests that isCallback correctly identifies url hash", function (done) { - expect(msal.isStsResponse("not a callback")).to.be.false; - expect(msal.isStsResponse("#error_description=someting_wrong")).to.be.true; - expect(msal.isStsResponse("#/error_description=someting_wrong")).to.be.true; - expect(msal.isStsResponse("#access_token=token123")).to.be.true; - expect(msal.isStsResponse("#id_token=idtoken234")).to.be.true; + expect(msal.isCallback("not a callback")).to.be.false; + expect(msal.isCallback("#error_description=someting_wrong")).to.be.true; + expect(msal.isCallback("#/error_description=someting_wrong")).to.be.true; + expect(msal.isCallback("#access_token=token123")).to.be.true; + expect(msal.isCallback("#id_token=idtoken234")).to.be.true; done(); }); }); From 5144589e286b544098f65477b95361ee0992894e Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Tue, 2 Jul 2019 20:36:47 -0700 Subject: [PATCH 07/20] Update Utils.ts --- lib/msal-core/src/Utils.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/msal-core/src/Utils.ts b/lib/msal-core/src/Utils.ts index 094709796b..7b5167feee 100644 --- a/lib/msal-core/src/Utils.ts +++ b/lib/msal-core/src/Utils.ts @@ -552,6 +552,7 @@ export class Utils { } else if (hashIndex1 > -1) { return urlStringOrFragment.substring(hashIndex1 + 1); } + return urlStringOrFragment; } //#endregion From 8f2badcd6e54d2e613766ac766bcf96b559b08eb Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Tue, 2 Jul 2019 21:16:52 -0700 Subject: [PATCH 08/20] Changing scopes used during helper call --- lib/msal-core/src/UserAgentApplication.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 1b0a674612..008e48625d 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -498,7 +498,7 @@ export class UserAgentApplication { // Set response type responseType = ResponseTypes.id_token; } else { - responseType = this.getTokenType(account, request.scopes, false); + responseType = this.getTokenType(account, scopes, false); } serverAuthenticationRequest = new ServerRequestParameters( @@ -516,7 +516,7 @@ export class UserAgentApplication { serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); // Construct urlNavigate - let urlNavigate = serverAuthenticationRequest.createNavigateUrl(request.scopes) + Constants.response_mode_fragment; + let urlNavigate = serverAuthenticationRequest.createNavigateUrl(scopes) + Constants.response_mode_fragment; // set state in cache if (interactionType === Constants.interactionTypeRedirect) { @@ -2184,7 +2184,7 @@ export class UserAgentApplication { // all other cases else { if (!Utils.compareAccounts(accountObject, this.getAccount())) { - tokenType = ResponseTypes.id_token_token; + tokenType = ResponseTypes.id_token_token; } else { tokenType = (scopes.indexOf(this.clientId) > -1) ? ResponseTypes.id_token : ResponseTypes.token; From a94d996c895265dad3be151d2ad8d4171c5c6850 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Wed, 17 Jul 2019 15:59:46 -0700 Subject: [PATCH 09/20] Adding updates from feedback --- lib/msal-core/src/UserAgentApplication.ts | 47 +++++-------------- lib/msal-core/test/Utils.spec.ts | 33 +++++++++++++ .../InteractionRequiredAuthError.spec.ts | 12 +++++ 3 files changed, 58 insertions(+), 34 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 008e48625d..37e20883b3 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -287,23 +287,7 @@ export class UserAgentApplication { } } - private redirectSuccessHandler(response: AuthResponse) : void { - if (this.errorReceivedCallback) { - this.tokenReceivedCallback(response); - } else if (this.authResponseCallback) { - this.authResponseCallback(null, response); - } - } - - private redirectErrorHandler(authErr: AuthError, response: AuthResponse) : void { - if (this.errorReceivedCallback) { - this.errorReceivedCallback(authErr, response.accountState); - } else { - this.authResponseCallback(authErr, response); - } - } - - private successHandler(interactionType: InteractionType, response: AuthResponse, resolve?: any) : void { + private responseHandler(interactionType: InteractionType, response: AuthResponse, resolve?: any) : void { if (interactionType === Constants.interactionTypeRedirect) { if (this.errorReceivedCallback) { this.tokenReceivedCallback(response); @@ -317,7 +301,7 @@ export class UserAgentApplication { } } - private errorHandler(interactionType: InteractionType, authErr: AuthError, response: AuthResponse, reject?: any) : void { + private authErrorHandler(interactionType: InteractionType, authErr: AuthError, response: AuthResponse, reject?: any) : void { if (interactionType === Constants.interactionTypeRedirect) { if (this.errorReceivedCallback) { this.errorReceivedCallback(authErr, response.accountState); @@ -397,9 +381,9 @@ export class UserAgentApplication { // If already in progress, do not proceed if (this.loginInProgress || this.acquireTokenInProgress) { - let thrownError = this.loginInProgress ? ClientAuthError.createLoginInProgressError() : ClientAuthError.createAcquireTokenInProgressError(); - let stateOnlyResponse = buildResponseStateOnly(this.getAccountState(request && request.state)); - this.errorHandler(interactionType, + const thrownError = this.loginInProgress ? ClientAuthError.createLoginInProgressError() : ClientAuthError.createAcquireTokenInProgressError(); + const stateOnlyResponse = buildResponseStateOnly(this.getAccountState(request && request.state)); + this.authErrorHandler(interactionType, thrownError, stateOnlyResponse, reject); @@ -407,7 +391,7 @@ export class UserAgentApplication { } // if extraScopesToConsent is passed in loginCall, append them to the login request - let scopes: Array = isLoginCall ? this.appendScopes(request) : request.scopes; + const scopes: Array = isLoginCall ? this.appendScopes(request) : request.scopes; // Validate and filter scopes (the validate function will throw if validation fails) this.validateInputScope(scopes, !isLoginCall); @@ -431,7 +415,7 @@ export class UserAgentApplication { this.silentLogin = false; this.logger.info("Unified cache call is successful"); - this.successHandler(interactionType, response, resolve); + this.responseHandler(interactionType, response, resolve); return; }, (error) => { this.silentLogin = false; @@ -483,7 +467,7 @@ export class UserAgentApplication { acquireTokenAuthority.resolveEndpointsAsync().then(() => { // On Fulfillment - let responseType: string; + const responseType: string = isLoginCall ? ResponseTypes.id_token : this.getTokenType(account, scopes, false); let loginStartPage: string; if (isLoginCall) { @@ -494,11 +478,6 @@ export class UserAgentApplication { } else { this.cacheStorage.setItem(Constants.angularLoginRequest, ""); } - - // Set response type - responseType = ResponseTypes.id_token; - } else { - responseType = this.getTokenType(account, scopes, false); } serverAuthenticationRequest = new ServerRequestParameters( @@ -535,7 +514,7 @@ export class UserAgentApplication { this.navigateWindow(urlNavigate, popUpWindow); }).catch((err) => { this.logger.warning("could not resolve endpoints"); - this.errorHandler(interactionType, ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request.state), reject); + this.authErrorHandler(interactionType, ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request.state), reject); if (popUpWindow) { popUpWindow.close(); } @@ -597,7 +576,7 @@ export class UserAgentApplication { this.logger.verbose("ADAL's idToken exists. Extracting login information from ADAL's idToken "); serverAuthenticationRequest = this.populateQueryParams(account, null, serverAuthenticationRequest, adalIdTokenObject); } - let userContainedClaims = request.claimsRequest || serverAuthenticationRequest.claimsValue; + const userContainedClaims = request.claimsRequest || serverAuthenticationRequest.claimsValue; let authErr: AuthError; let cacheResultResponse; @@ -1124,11 +1103,11 @@ export class UserAgentApplication { response.tokenType = Constants.idToken; } if (!parentCallback) { - this.redirectSuccessHandler(response); + this.responseHandler(Constants.interactionTypeRedirect, response); return; } } else if (!parentCallback) { - this.redirectErrorHandler(authErr, buildResponseStateOnly(accountState)); + this.authErrorHandler(Constants.interactionTypeRedirect, authErr, buildResponseStateOnly(accountState)); return; } @@ -1222,7 +1201,7 @@ export class UserAgentApplication { * @param hash */ private deserializeHash(urlFragment: string) { - let hash = Utils.getHashFromUrl(urlFragment); + const hash = Utils.getHashFromUrl(urlFragment); return Utils.deserialize(hash); } diff --git a/lib/msal-core/test/Utils.spec.ts b/lib/msal-core/test/Utils.spec.ts index ff052f1d98..1bdb25c8f6 100644 --- a/lib/msal-core/test/Utils.spec.ts +++ b/lib/msal-core/test/Utils.spec.ts @@ -3,6 +3,21 @@ import { Utils } from "../src/Utils"; import { IdToken } from "../src/IdToken"; describe("Utils.ts class", () => { + + const TEST_ID_TOKEN = "eyJraWQiOiIxZTlnZGs3IiwiYWxnIjoiUlMyNTYifQ" + + ".ewogImlzcyI6ICJodHRwOi8vc2VydmVyLmV4YW1wbGUuY29tIiwKICJzdWIiOiAiMjQ4Mjg5NzYxMDAxIiwKICJhdWQiOiAiczZCaGRSa3F0MyIsCiAibm9uY2UiOiAidGVzdF9ub25jZSIsCiAiZXhwIjogMTMxMTI4MTk3MCwKICJpYXQiOiAxMzExMjgwOTcwLAogIm5hbWUiOiAiSmFuZSBEb2UiLAogImdpdmVuX25hbWUiOiAiSmFuZSIsCiAiZmFtaWx5X25hbWUiOiAiRG9lIiwKICJnZW5kZXIiOiAiZmVtYWxlIiwKICJ0aWQiOiAiMTI0ZHMzMjQtNDNkZS1uODltLTc0NzctNDY2ZmVmczQ1YTg1IiwKICJiaXJ0aGRhdGUiOiAiMDAwMC0xMC0zMSIsCiAiZW1haWwiOiAiamFuZWRvZUBleGFtcGxlLmNvbSIsCiAicGljdHVyZSI6ICJodHRwOi8vZXhhbXBsZS5jb20vamFuZWRvZS9tZS5qcGciCn0=" + + ".rHQjEmBqn9Jre0OLykYNnspA10Qql2rvx4FsD00jwlB0Sym4NzpgvPKsDjn_wMkHxcp6CilPcoKrWHcipR2iAjzLvDNAReF97zoJqq880ZD1bwY82JDauCXELVR9O6_B0w3K-E7yM2macAAgNCUwtik6SjoSUZRcf-O5lygIyLENx882p6MtmwaL1hd6qn5RZOQ0TLrOYu0532g9Exxcm-ChymrB4xLykpDj3lUivJt63eEGGN6DH5K6o33TcxkIjNrCD4XB1CKKumZvCedgHHF3IAK4dVEDSUoGlH9z4pP_eWYNXvqQOjGs-rDaQzUHl6cQQWNiDpWOl_lxXjQEvQ"; + + const TEST_RAW_CLIENT_INFO = "eyJ1aWQiOiIxMjMtdGVzdC11aWQiLCJ1dGlkIjoiNDU2LXRlc3QtdXRpZCJ9"; + + // Test Hashes + const TEST_SUCCESS_PARAMS = `id_token=${TEST_ID_TOKEN}&client_info=${TEST_RAW_CLIENT_INFO}&state=RANDOM-GUID-HERE|`; + const TEST_SUCCESS_HASH_1 = `#${TEST_SUCCESS_PARAMS}`; + const TEST_SUCCESS_HASH_2 = `#/${TEST_SUCCESS_PARAMS}`; + const TEST_URL_NO_HASH = `http://localhost:3000/`; + const TEST_URL_HASH_SINGLE_CHAR = `${TEST_URL_NO_HASH}${TEST_SUCCESS_HASH_1}`; + const TEST_URL_HASH_TWO_CHAR = `${TEST_URL_NO_HASH}${TEST_SUCCESS_HASH_2}`; + it("get getLibraryVersion()", () => { const version: string = Utils.getLibraryVersion(); @@ -33,4 +48,22 @@ describe("Utils.ts class", () => { expect(Utils.base64EncodeStringUrlSafe("Avrán")).to.be.equal("QXZyw6Fu"); expect(Utils.base64DecodeStringUrlSafe("QXZyw6Fu")).to.be.equal("Avrán"); }); + + it("test getHashFromUrl returns hash from url if hash is single character", () => { + const hash = Utils.getHashFromUrl(TEST_URL_HASH_SINGLE_CHAR); + + expect(hash).to.be.equal(TEST_SUCCESS_PARAMS); + }); + + it("test getHashFromUrl returns hash from url if hash is two character", () => { + const hash = Utils.getHashFromUrl(TEST_URL_HASH_TWO_CHAR); + + expect(hash).to.be.equal(TEST_SUCCESS_PARAMS); + }); + + it("test getHashFromUrl returns original url from url if no hash is present", () => { + const hash = Utils.getHashFromUrl(TEST_URL_NO_HASH); + + expect(hash).to.be.equal(TEST_URL_NO_HASH); + }); }); diff --git a/lib/msal-core/test/error/InteractionRequiredAuthError.spec.ts b/lib/msal-core/test/error/InteractionRequiredAuthError.spec.ts index 112439dd03..b91a1237d7 100644 --- a/lib/msal-core/test/error/InteractionRequiredAuthError.spec.ts +++ b/lib/msal-core/test/error/InteractionRequiredAuthError.spec.ts @@ -7,6 +7,11 @@ describe("InteractionRequiredAuthError.ts Class", () => { const ERROR_DESC = "Error from the server"; + const INTERACTION_REQ_STRING = "interaction_required"; + const LOGIN_REQ_STRING = "login_required"; + const CONSENT_REQ_STRING = "consent_required"; + const INCORRECT_REQ_STRING = "something_else_required"; + it("InteractionRequiredAuthError object can be created", () => { const TEST_ERROR_CODE: string = "test"; @@ -84,5 +89,12 @@ describe("InteractionRequiredAuthError.ts Class", () => { expect(err.stack).to.include("InteractionRequiredAuthError.spec.js"); }); + it("isInteractionRequiredError function correctly detects _required strings", () => { + expect(InteractionRequiredAuthError.isInteractionRequiredError(INTERACTION_REQ_STRING)).to.be.true; + expect(InteractionRequiredAuthError.isInteractionRequiredError(LOGIN_REQ_STRING)).to.be.true; + expect(InteractionRequiredAuthError.isInteractionRequiredError(CONSENT_REQ_STRING)).to.be.true; + expect(InteractionRequiredAuthError.isInteractionRequiredError(INCORRECT_REQ_STRING)).to.be.false; + }); + }); From 77e35e0c7acf104a309ef6cf765521c81e9a6907 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Wed, 17 Jul 2019 16:02:31 -0700 Subject: [PATCH 10/20] Update Utils.ts --- lib/msal-core/src/Utils.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/lib/msal-core/src/Utils.ts b/lib/msal-core/src/Utils.ts index 7b5167feee..83dee0d968 100644 --- a/lib/msal-core/src/Utils.ts +++ b/lib/msal-core/src/Utils.ts @@ -549,7 +549,8 @@ export class Utils { const hashIndex2 = urlStringOrFragment.indexOf("#/"); if (hashIndex2 > -1) { return urlStringOrFragment.substring(hashIndex2 + 2); - } else if (hashIndex1 > -1) { + } + if (hashIndex1 > -1) { return urlStringOrFragment.substring(hashIndex1 + 1); } return urlStringOrFragment; From 271c5d7c028cde96c7a56075321518798dbaae1f Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Wed, 17 Jul 2019 16:36:20 -0700 Subject: [PATCH 11/20] Fixing forceRefresh --- lib/msal-core/src/UserAgentApplication.ts | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index c4dc62f156..661a37f500 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -593,7 +593,7 @@ export class UserAgentApplication { let authErr: AuthError; let cacheResultResponse; - if (!userContainedClaims) { + if (!userContainedClaims && !request.forceRefresh) { try { cacheResultResponse = this.getCachedToken(serverAuthenticationRequest, account); } catch (e) { @@ -614,7 +614,14 @@ export class UserAgentApplication { } // else proceed with login else { - let logMessage = userContainedClaims ? "Skipped cache lookup since claims were given." : "Token is not in cache for scope:" + scope; + let logMessage; + if (userContainedClaims) { + logMessage = "Skipped cache lookup since claims were given."; + } else if (request.forceRefresh) { + logMessage = "Skipped cache lookup since request.forceRefresh option was set to true"; + } else { + logMessage = "Token is not in cache for scope:" + scope; + } this.logger.verbose(logMessage); // Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the api. From a440e03b0fa59a6626eccedbd6304f990235fca9 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 17:38:09 -0700 Subject: [PATCH 12/20] Update Utils.ts --- lib/msal-core/src/Utils.ts | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/lib/msal-core/src/Utils.ts b/lib/msal-core/src/Utils.ts index c466124111..815598cb0a 100644 --- a/lib/msal-core/src/Utils.ts +++ b/lib/msal-core/src/Utils.ts @@ -546,13 +546,13 @@ export class Utils { * Returns the anchor part(#) of the URL */ static getHashFromUrl(urlStringOrFragment: string): string { - const hashIndex1 = urlStringOrFragment.indexOf("#"); - const hashIndex2 = urlStringOrFragment.indexOf("#/"); - if (hashIndex2 > -1) { - return urlStringOrFragment.substring(hashIndex2 + 2); + const index = urlStringOrFragment.indexOf("#"); + const indexWithSlash = urlStringOrFragment.indexOf("#/"); + if (indexWithSlash > -1) { + return urlStringOrFragment.substring(indexWithSlash + 2); } - if (hashIndex1 > -1) { - return urlStringOrFragment.substring(hashIndex1 + 1); + if (index > -1) { + return urlStringOrFragment.substring(index + 1); } return urlStringOrFragment; } From d2b30c6767a92bb64e3f9c0b9dbd28edd0744cb3 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 17:41:15 -0700 Subject: [PATCH 13/20] Update UserAgentApplication.ts --- lib/msal-core/src/UserAgentApplication.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 661a37f500..e2a8054204 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -288,7 +288,7 @@ export class UserAgentApplication { } } - private responseHandler(interactionType: InteractionType, response: AuthResponse, resolve?: any) : void { + private authResponseHandler(interactionType: InteractionType, response: AuthResponse, resolve?: any) : void { if (interactionType === Constants.interactionTypeRedirect) { if (this.errorReceivedCallback) { this.tokenReceivedCallback(response); @@ -424,7 +424,7 @@ export class UserAgentApplication { this.silentLogin = false; this.logger.info("Unified cache call is successful"); - this.responseHandler(interactionType, response, resolve); + this.authResponseHandler(interactionType, response, resolve); return; }, (error) => { this.silentLogin = false; @@ -1122,7 +1122,7 @@ export class UserAgentApplication { response.tokenType = Constants.idToken; } if (!parentCallback) { - this.responseHandler(Constants.interactionTypeRedirect, response); + this.authResponseHandler(Constants.interactionTypeRedirect, response); return; } } else if (!parentCallback) { From 0481dc8b40e23ab08462a1d8e49f55bfb8c60213 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 18:14:22 -0700 Subject: [PATCH 14/20] Adding comments --- lib/msal-core/src/UserAgentApplication.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index e2a8054204..1e7e59b72c 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -433,14 +433,20 @@ export class UserAgentApplication { // call the loginRedirectHelper later with no user account context this.acquireTokenHelper(null, interactionType, isLoginCall, request, scopes, resolve, reject); }); - } else { + } + // No ADAL token found, proceed to login + else { this.acquireTokenHelper(null, interactionType, isLoginCall, request, scopes, resolve, reject); } - } else { + } + // AcquireToken call, but no account or context given, so throw error + else { this.logger.info("User login is required"); throw ClientAuthError.createUserLoginRequiredError(); } - } else { + } + // User session exists + else { this.acquireTokenHelper(account, interactionType, isLoginCall, request, scopes, resolve, reject); } } From dad8e70dac2267430da2ae775cb146d46887974b Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 18:16:24 -0700 Subject: [PATCH 15/20] Update UserAgentApplication.ts --- lib/msal-core/src/UserAgentApplication.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 1e7e59b72c..fd5fa2041d 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -430,7 +430,7 @@ export class UserAgentApplication { this.silentLogin = false; this.logger.error("Error occurred during unified cache ATS: " + error); - // call the loginRedirectHelper later with no user account context + // proceed to login since ATS failed this.acquireTokenHelper(null, interactionType, isLoginCall, request, scopes, resolve, reject); }); } From 33ccfcc0ef50468f828f13a06d22df43cd95ec0a Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 18:22:34 -0700 Subject: [PATCH 16/20] Update UserAgentApplication.ts --- lib/msal-core/src/UserAgentApplication.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index fd5fa2041d..80474d5bc2 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -468,7 +468,7 @@ export class UserAgentApplication { const scope = scopes ? scopes.join(" ").toLowerCase() : this.clientId.toLowerCase(); let serverAuthenticationRequest: ServerRequestParameters; - const acquireTokenAuthority = (!isLoginCall && request.authority) ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; + const acquireTokenAuthority = (!isLoginCall && request && request.authority) ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance; let popUpWindow: Window; if (interactionType === Constants.interactionTypePopup) { From 6a93c4d7d3f4cbfbeb8bb9a2a80587ff768af3ec Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 18:26:34 -0700 Subject: [PATCH 17/20] Update UserAgentApplication.ts --- lib/msal-core/src/UserAgentApplication.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 80474d5bc2..0e74d14ede 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -509,7 +509,7 @@ export class UserAgentApplication { // populate QueryParameters (sid/login_hint/domain_hint) and any other extraQueryParameters set by the developer serverAuthenticationRequest = this.populateQueryParams(account, request, serverAuthenticationRequest); - // Construct urlNavigate + // Construct url to navigate to let urlNavigate = serverAuthenticationRequest.createNavigateUrl(scopes) + Constants.response_mode_fragment; // set state in cache From 1d22b531b074d9b675e0eaff3cf55124f82ab9d0 Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Thu, 18 Jul 2019 18:54:09 -0700 Subject: [PATCH 18/20] adding error checks for interactionType --- lib/msal-core/src/UserAgentApplication.ts | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 0e74d14ede..5c357017fd 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -517,12 +517,14 @@ export class UserAgentApplication { if (!isLoginCall) { this.cacheStorage.setItem(Constants.stateAcquireToken, serverAuthenticationRequest.state, this.inCookie); } - } else { + } else if (interactionType === Constants.interactionTypePopup) { window.renewStates.push(serverAuthenticationRequest.state); window.requestType = isLoginCall ? Constants.login : Constants.renewToken; // Register callback to capture results from server this.registerCallback(serverAuthenticationRequest.state, scope, resolve, reject); + } else { + throw ClientAuthError.createInvalidInteractionTypeError(); } // prompt user for interaction From 0aa92004c9dd37efa46d6323ec0cb47bc4e4236b Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Mon, 22 Jul 2019 14:13:33 -0700 Subject: [PATCH 19/20] Update UserAgentApplication.ts --- lib/msal-core/src/UserAgentApplication.ts | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 5c357017fd..0f4785394f 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -241,11 +241,11 @@ export class UserAgentApplication { window.msal = this; const urlHash = window.location.hash; - const isCallback = this.isCallback(urlHash); + const urlContainsHash = this.urlContainsHash(urlHash); // On the server 302 - Redirect, handle this if (!this.config.framework.isAngular) { - if (isCallback) { + if (urlContainsHash) { this.handleAuthenticationResponse(urlHash); } } @@ -1080,7 +1080,11 @@ export class UserAgentApplication { * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. */ isCallback(hash: string): boolean { - const parameters = this.deserializeHash(hash); + return this.urlContainsHash(hash); + } + + private urlContainsHash(urlString: string): boolean { + const parameters = this.deserializeHash(urlString); return ( parameters.hasOwnProperty(Constants.errorDescription) || parameters.hasOwnProperty(Constants.error) || From 9e9d7ade44e87394dc7c70dd3c26b580ca83528f Mon Sep 17 00:00:00 2001 From: Prithvi Kanherkar Date: Mon, 22 Jul 2019 14:17:02 -0700 Subject: [PATCH 20/20] Update UserAgentApplication.ts --- lib/msal-core/src/UserAgentApplication.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 0f4785394f..9cd48847d0 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -1080,6 +1080,7 @@ export class UserAgentApplication { * @returns {Boolean} - true if response contains id_token, access_token or error, false otherwise. */ isCallback(hash: string): boolean { + this.logger.info("isCallback will be deprecated in favor of urlContainsHash in MSAL.js v2.0."); return this.urlContainsHash(hash); }