Skip to content

Commit

Permalink
Merge pull request #1413 from AzureAD/redirect-library-state
Browse files Browse the repository at this point in the history
Ensure responses from redirect requests are always processed
  • Loading branch information
jasonnutter authored Apr 3, 2020
2 parents b5b82d3 + abcf8ca commit 3e32907
Show file tree
Hide file tree
Showing 5 changed files with 165 additions and 43 deletions.
19 changes: 12 additions & 7 deletions lib/msal-core/src/UserAgentApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@ export interface CacheResult {
export type ResponseStateInfo = {
state: string;
timestamp: number,
method: string;
stateMatch: boolean;
requestType: string;
};
Expand Down Expand Up @@ -236,8 +237,11 @@ export class UserAgentApplication {
WindowUtils.checkIfBackButtonIsPressed(this.cacheStorage);

// On the server 302 - Redirect, handle this
if (urlContainsHash && !WindowUtils.isInIframe() && !WindowUtils.isInPopup()) {
this.handleRedirectAuthenticationResponse(urlHash);
if (urlContainsHash) {
const stateInfo = this.getResponseState(urlHash);
if (stateInfo.method === Constants.interactionTypeRedirect) {
this.handleRedirectAuthenticationResponse(urlHash);
}
}
}

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

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

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

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

return new Promise<AuthResponse>((resolve, reject) => {
this.acquireTokenInteractive(Constants.interactionTypePopup, false, request, resolve, reject);
Expand Down Expand Up @@ -593,7 +597,7 @@ export class UserAgentApplication {
apiEvent.apiCode = API_CODE.AcquireTokenSilent;
this.telemetryManager.startEvent(apiEvent);
// validate the request
const request = RequestUtils.validateRequest(userRequest, false, this.clientId);
const request = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypeSilent);

return new Promise<AuthResponse>((resolve, reject) => {

Expand Down Expand Up @@ -1095,6 +1099,7 @@ export class UserAgentApplication {
requestType: Constants.unknown,
state: parameters.state,
timestamp: parsedState.ts,
method: parsedState.method,
stateMatch: false
};
} else {
Expand Down
3 changes: 2 additions & 1 deletion lib/msal-core/src/utils/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ export class Constants {

static get interactionTypeRedirect(): InteractionType { return "redirectInteraction"; }
static get interactionTypePopup(): InteractionType { return "popupInteraction"; }
static get interactionTypeSilent(): InteractionType { return "silentInteraction"; }
static get inProgress(): string { return "inProgress"; }
}

Expand Down Expand Up @@ -129,7 +130,7 @@ export const BlacklistedEQParams = [
SSOTypes.LOGIN_HINT
];

export type InteractionType = "redirectInteraction" | "popupInteraction";
export type InteractionType = "redirectInteraction" | "popupInteraction" | "silentInteraction";

/**
* we considered making this "enum" in the request instead of string, however it looks like the allowed list of
Expand Down
20 changes: 12 additions & 8 deletions lib/msal-core/src/utils/RequestUtils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
*/

import { AuthenticationParameters } from "../AuthenticationParameters";
import { Constants, PromptState, BlacklistedEQParams } from "../utils/Constants";
import { Constants, PromptState, BlacklistedEQParams, InteractionType } from "../utils/Constants";
import { ClientConfigurationError } from "../error/ClientConfigurationError";
import { ScopeSet } from "../ScopeSet";
import { StringDict } from "../MsalTypes";
Expand All @@ -16,6 +16,7 @@ import { ClientAuthError } from "../error/ClientAuthError";
export type LibraryStateObject = {
id: string,
ts: number
method: string
};

/**
Expand All @@ -33,7 +34,7 @@ export class RequestUtils {
*
* validates all request parameters and generates a consumable request object
*/
static validateRequest(request: AuthenticationParameters, isLoginCall: boolean, clientId: string): AuthenticationParameters {
static validateRequest(request: AuthenticationParameters, isLoginCall: boolean, clientId: string, interactionType: InteractionType): AuthenticationParameters {

// Throw error if request is empty for acquire * calls
if(!isLoginCall && !request) {
Expand All @@ -60,7 +61,7 @@ export class RequestUtils {
}

// validate and generate state and correlationId
const state = this.validateAndGenerateState(request && request.state);
const state = this.validateAndGenerateState(request && request.state, interactionType);
const correlationId = this.validateAndGenerateCorrelationId(request && request.correlationId);

const validatedRequest: AuthenticationParameters = {
Expand Down Expand Up @@ -139,19 +140,20 @@ export class RequestUtils {
* @param userState User-provided state value
* @returns State string include library state and user state
*/
static validateAndGenerateState(userState: string): string {
return !StringUtils.isEmpty(userState) ? `${RequestUtils.generateLibraryState()}${Constants.resourceDelimiter}${userState}` : RequestUtils.generateLibraryState();
static validateAndGenerateState(userState: string, interactionType: InteractionType): string {
return !StringUtils.isEmpty(userState) ? `${RequestUtils.generateLibraryState(interactionType)}${Constants.resourceDelimiter}${userState}` : RequestUtils.generateLibraryState(interactionType);
}

/**
* Generates the state value used by the library.
*
* @returns Base64 encoded string representing the state
*/
static generateLibraryState(): string {
static generateLibraryState(interactionType: InteractionType): string {
const stateObject: LibraryStateObject = {
id: CryptoUtils.createNewGuid(),
ts: TimeUtils.now()
ts: TimeUtils.now(),
method: interactionType
};

const stateString = JSON.stringify(stateObject);
Expand All @@ -169,9 +171,11 @@ export class RequestUtils {
const libraryState = state.split(Constants.resourceDelimiter)[0];

if (CryptoUtils.isGuid(libraryState)) {
// If state is guid, assume timestamp is now and is redirect, as redirect should be only method where this can happen.
return {
id: libraryState,
ts: TimeUtils.now()
ts: TimeUtils.now(),
method: Constants.interactionTypeRedirect
};
}

Expand Down
Loading

0 comments on commit 3e32907

Please sign in to comment.