Skip to content

Commit

Permalink
Merge pull request #1358 from AzureAD/Fix-redirect-callback-return
Browse files Browse the repository at this point in the history 
handleRedirectCallback updates
  • Loading branch information
@tnorling
tnorling authored Mar 25, 2020
2 parents 1935d00 + 4c3b869 commit 37913c9
Show file tree
Hide file tree
Showing 6 changed files with 150 additions and 148 deletions.
2 changes: 1 addition & 1 deletion lib/msal-core/README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ Before using MSAL.js you will need to [register an application in Azure AD](http

`UserAgentApplication` can be configured with a variety of different options, detailed in our [Wiki](https://github.com/AzureAD/microsoft-authentication-library-for-js/wiki/MSAL.js-1.0.0-api-release#configuration-options), but the only required parameter is `auth.clientId`.

After instantiating your instance, if you plan on using a redirect flow (`loginRedirect` and `acquireTokenRedirect`), you must register a callback handlers using `handleRedirectCallback(authCallback)` where `authCallback = function(AuthError, AuthResponse)`. The callback function is called after the authentication request is completed either successfully or with a failure. This is not required for the popup flows since they return promises.
After instantiating your instance, if you plan on using a redirect flow in MSAL 1.2.x or earlier (`loginRedirect` and `acquireTokenRedirect`), you must register a callback handler using `handleRedirectCallback(authCallback)` where `authCallback = function(AuthError, AuthResponse)`. As of MSAL 1.3.0 this is optional. The callback function is called after the authentication request is completed either successfully or with a failure. This is not required for the popup flows since they return promises.

```JavaScript
import * as Msal from "msal";
Expand Down
4 changes: 2 additions & 2 deletions lib/msal-core/src/Configuration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,8 +115,8 @@ const DEFAULT_AUTH_OPTIONS: AuthOptions = {
clientId: "",
authority: null,
validateAuthority: true,
redirectUri: () => UrlUtils.getDefaultRedirectUri(),
postLogoutRedirectUri: () => UrlUtils.getDefaultRedirectUri(),
redirectUri: () => UrlUtils.getCurrentUrl(),
postLogoutRedirectUri: () => UrlUtils.getCurrentUrl(),
navigateToLoginRequestUrl: true
};

Expand Down
115 changes: 48 additions & 67 deletions lib/msal-core/src/UserAgentApplication.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,8 @@ export class UserAgentApplication {
// state variables
private silentAuthenticationState: string;
private silentLogin: boolean;
private redirectCallbacksSet: boolean;
private redirectResponse: AuthResponse;
private redirectError: AuthError;

// Authority Functionality
protected authorityInstance: Authority;
Expand Down Expand Up @@ -208,9 +209,6 @@ export class UserAgentApplication {
// Set the Configuration
this.config = buildConfiguration(configuration);

// Set the callback boolean
this.redirectCallbacksSet = false;

this.logger = this.config.system.logger;
this.clientId = this.config.auth.clientId;
this.inCookie = this.config.cache.storeAuthStateInCookie;
Expand Down Expand Up @@ -238,7 +236,7 @@ export class UserAgentApplication {

// On the server 302 - Redirect, handle this
if (urlContainsHash && !WindowUtils.isInIframe() && !WindowUtils.isInPopup()) {
this.handleAuthenticationResponse(urlHash);
this.handleRedirectAuthenticationResponse(urlHash);
}
}

Expand All @@ -255,7 +253,6 @@ export class UserAgentApplication {
handleRedirectCallback(authCallback: authResponseCallback): void;
handleRedirectCallback(authOrTokenCallback: authResponseCallback | tokenReceivedCallback, errorReceivedCallback?: errorReceivedCallback): void {
if (!authOrTokenCallback) {
this.redirectCallbacksSet = false;
throw ClientConfigurationError.createInvalidCallbackObjectError(authOrTokenCallback);
}

Expand All @@ -268,12 +265,10 @@ export class UserAgentApplication {
this.authResponseCallback = authOrTokenCallback as authResponseCallback;
}

this.redirectCallbacksSet = true;

// On the server 302 - Redirect, handle this
const cachedHash = this.cacheStorage.getItem(TemporaryCacheKeys.URL_HASH);
if (cachedHash) {
this.processCallBack(cachedHash, null);
if (this.redirectError) {
this.authErrorHandler(Constants.interactionTypeRedirect, this.redirectError, this.redirectResponse);
} else if (this.redirectResponse) {
this.authResponseHandler(Constants.interactionTypeRedirect, this.redirectResponse);
}
}

Expand Down Expand Up @@ -322,7 +317,7 @@ export class UserAgentApplication {
*/
loginRedirect(userRequest?: AuthenticationParameters): void {
// validate request
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, true, this.clientId, Constants.interactionTypeRedirect, this.redirectCallbacksSet);
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, true, this.clientId);
this.acquireTokenInteractive(Constants.interactionTypeRedirect, true, request, null, null);
}

Expand All @@ -334,7 +329,7 @@ export class UserAgentApplication {
*/
acquireTokenRedirect(userRequest: AuthenticationParameters): void {
// validate request
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypeRedirect, this.redirectCallbacksSet);
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, false, this.clientId);
this.acquireTokenInteractive(Constants.interactionTypeRedirect, false, request, null, null);
}

Expand All @@ -347,7 +342,7 @@ export class UserAgentApplication {
*/
loginPopup(userRequest?: AuthenticationParameters): Promise<AuthResponse> {
// validate request
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, true, this.clientId, Constants.interactionTypePopup);
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, true, this.clientId);

return new Promise<AuthResponse>((resolve, reject) => {
this.acquireTokenInteractive(Constants.interactionTypePopup, true, request, resolve, reject);
Expand All @@ -366,7 +361,7 @@ export class UserAgentApplication {
*/
acquireTokenPopup(userRequest: AuthenticationParameters): Promise<AuthResponse> {
// validate request
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypePopup);
const request: AuthenticationParameters = RequestUtils.validateRequest(userRequest, false, this.clientId);

return new Promise<AuthResponse>((resolve, reject) => {
this.acquireTokenInteractive(Constants.interactionTypePopup, false, request, resolve, reject);
Expand Down Expand Up @@ -991,9 +986,6 @@ export class UserAgentApplication {
authErr = err;
}

// remove hash from the cache
this.cacheStorage.removeItem(TemporaryCacheKeys.URL_HASH);

try {
// Clear the cookie in the hash
this.cacheStorage.clearMsalCookie(stateInfo.state);
Expand All @@ -1011,12 +1003,13 @@ export class UserAgentApplication {
response.tokenType = ServerHashParamKeys.ID_TOKEN;
}
if (!parentCallback) {
this.authResponseHandler(Constants.interactionTypeRedirect, response);
this.redirectResponse = response;
return;
}
} else if (!parentCallback) {
this.redirectResponse = buildResponseStateOnly(accountState);
this.redirectError = authErr;
this.cacheStorage.resetTempCacheItems(stateInfo.state);
this.authErrorHandler(Constants.interactionTypeRedirect, authErr, buildResponseStateOnly(accountState));
return;
}

Expand All @@ -1029,64 +1022,56 @@ export class UserAgentApplication {

/**
* @hidden
* This method must be called for processing the response received from the STS. It extracts the hash, processes the token or error information and saves it in the cache. It then
* calls the registered callbacks in case of redirect or resolves the promises with the result.
* This method must be called for processing the response received from the STS if using popups or iframes. It extracts the hash, processes the token or error
* information and saves it in the cache. It then resolves the promises with the result.
* @param {string} [hash=window.location.hash] - Hash fragment of Url.
*/
private handleAuthenticationResponse(hash: string): void {
// retrieve the hash
const locationHash = hash || window.location.hash;

// Check if the current flow is popup or hidden iframe
const iframeWithHash = WindowUtils.getIframeWithHash(locationHash);
const popUpWithHash = WindowUtils.getPopUpWithHash(locationHash);
const isPopupOrIframe = !!(iframeWithHash || popUpWithHash);

// if (window.parent !== window), by using self, window.parent becomes equal to window in getResponseState method specifically
const stateInfo = this.getResponseState(locationHash);

let tokenResponseCallback: (response: AuthResponse, error: AuthError) => void = null;
const tokenResponseCallback = window.callbackMappedToRenewStates[stateInfo.state];
this.processCallBack(locationHash, stateInfo, tokenResponseCallback);

// If current window is opener, close all windows
WindowUtils.closePopups();
}

/**
* @hidden
* This method must be called for processing the response received from the STS when using redirect flows. It extracts the hash, processes the token or error
* information and saves it in the cache. The result can then be accessed by user registered callbacks.
* @param {string} [hash=window.location.hash] - Hash fragment of Url.
*/
private handleRedirectAuthenticationResponse(hash: string): void {
this.logger.info("Returned from redirect url");
// If parent window is the msal instance which opened the current window (iframe)
if (isPopupOrIframe) {
tokenResponseCallback = window.callbackMappedToRenewStates[stateInfo.state];
} else {
// Redirect cases
tokenResponseCallback = null;
// if set to navigate to loginRequest page post login
if (this.config.auth.navigateToLoginRequestUrl) {
this.cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, locationHash);
if (window.parent === window) {
const loginRequestUrl = this.cacheStorage.getItem(`${TemporaryCacheKeys.LOGIN_REQUEST}${Constants.resourceDelimiter}${stateInfo.state}`, this.inCookie);

// Redirect to home page if login request url is null (real null or the string null)
if (!loginRequestUrl || loginRequestUrl === "null") {
this.logger.error("Unable to get valid login request url from cache, redirecting to home page");
window.location.href = "/";
} else {
window.location.href = loginRequestUrl;
}
}
return;
}
else {
window.location.hash = "";
}

// clear hash from window
window.location.hash = "";

// if (window.parent !== window), by using self, window.parent becomes equal to window in getResponseState method specifically
const stateInfo = this.getResponseState(hash);

if (!this.redirectCallbacksSet) {
// We reached this point too early - cache hash, return and process in handleRedirectCallbacks
this.cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, locationHash);
// if set to navigate to loginRequest page post login
if (this.config.auth.navigateToLoginRequestUrl && window.parent === window) {
const loginRequestUrl = this.cacheStorage.getItem(`${TemporaryCacheKeys.LOGIN_REQUEST}${Constants.resourceDelimiter}${stateInfo.state}`, this.inCookie);
const currentUrl = UrlUtils.getCurrentUrl();

// Redirect to home page if login request url is null (real null or the string null)
if (!loginRequestUrl || loginRequestUrl === "null") {
this.logger.error("Unable to get valid login request url from cache, redirecting to home page");
window.location.href = "/";
return;
} else if (currentUrl !== loginRequestUrl) {
window.location.href = `${loginRequestUrl}${hash}`;
return;
}
}

this.processCallBack(locationHash, stateInfo, tokenResponseCallback);

// If current window is opener, close all windows
if (isPopupOrIframe) {
WindowUtils.closePopups();
}
this.processCallBack(hash, stateInfo, null);
}

/**
Expand Down Expand Up @@ -1857,10 +1842,6 @@ export class UserAgentApplication {
* @returns {boolean} true/false
*/
public getLoginInProgress(): boolean {
const pendingCallback = this.cacheStorage.getItem(TemporaryCacheKeys.URL_HASH);
if (pendingCallback) {
return true;
}
return this.cacheStorage.getItem(TemporaryCacheKeys.INTERACTION_STATUS) === Constants.inProgress;
}

Expand Down
9 changes: 1 addition & 8 deletions lib/msal-core/src/utils/RequestUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,25 +21,18 @@ export class RequestUtils {
*
* @param request
* @param isLoginCall
* @param requestType
* @param redirectCallbacksSet
* @param cacheStorage
* @param clientId
*
* validates all request parameters and generates a consumable request object
*/
static validateRequest(request: AuthenticationParameters, isLoginCall: boolean, clientId: string, requestType?: string, redirectCallbacksSet?: boolean): AuthenticationParameters {
static validateRequest(request: AuthenticationParameters, isLoginCall: boolean, clientId: string): AuthenticationParameters {

// Throw error if request is empty for acquire * calls
if(!isLoginCall && !request) {
throw ClientConfigurationError.createEmptyRequestError();
}

// Throw error if callbacks are not set before redirect
if(requestType == Constants.interactionTypeRedirect && !redirectCallbacksSet) {
throw ClientConfigurationError.createRedirectCallbacksNotSetError();
}

let scopes: Array<string>;
let extraQueryParameters: StringDict;

Expand Down
2 changes: 1 addition & 1 deletion lib/msal-core/src/utils/UrlUtils.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ export class UrlUtils {
/**
* Returns current window URL as redirect uri
*/
static getDefaultRedirectUri(): string {
static getCurrentUrl(): string {
return window.location.href.split("?")[0].split("#")[0];
}

Expand Down
Loading

0 comments on commit 37913c9

Please sign in to comment.