From cf1a5251929f5142e949ca438f5b307d417db3de Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Wed, 18 Mar 2020 17:23:20 -0700 Subject: [PATCH 01/10] Turn library state into encoded string that contains guid and timestamp, fixes #1259 --- lib/msal-core/src/ServerRequestParameters.ts | 9 ++++-- lib/msal-core/src/UserAgentApplication.ts | 9 ++++-- lib/msal-core/src/utils/Constants.ts | 1 + lib/msal-core/src/utils/RequestUtils.ts | 34 +++++++++++++++++++- lib/msal-core/src/utils/TimeUtils.ts | 2 +- lib/msal-core/src/utils/UrlUtils.ts | 2 +- samples/react-sample-app/package-lock.json | 6 ++-- samples/react-sample-app/package.json | 2 +- 8 files changed, 53 insertions(+), 12 deletions(-) diff --git a/lib/msal-core/src/ServerRequestParameters.ts b/lib/msal-core/src/ServerRequestParameters.ts index a4f1dec2e5..294da74447 100644 --- a/lib/msal-core/src/ServerRequestParameters.ts +++ b/lib/msal-core/src/ServerRequestParameters.ts @@ -10,6 +10,7 @@ import { StringDict } from "./MsalTypes"; import { Account } from "./Account"; import { SSOTypes, Constants, PromptState, libraryVersion } from "./utils/Constants"; import { StringUtils } from "./utils/StringUtils"; +import { RequestUtils } from './utils/RequestUtils'; /** * Nonce: OIDC Nonce definition: https://openid.net/specs/openid-connect-core-1_0.html#IDToken @@ -24,6 +25,7 @@ export class ServerRequestParameters { nonce: string; state: string; + encodedState: string; // telemetry information xClientVer: string; @@ -50,9 +52,9 @@ export class ServerRequestParameters { * @param scope * @param responseType * @param redirectUri - * @param state + * @param encodedState */ - constructor (authority: Authority, clientId: string, responseType: string, redirectUri: string, scopes: Array, state: string, correlationId: string) { + constructor (authority: Authority, clientId: string, responseType: string, redirectUri: string, scopes: Array, encodedState: string, correlationId: string) { this.authorityInstance = authority; this.clientId = clientId; this.nonce = CryptoUtils.createNewGuid(); @@ -61,7 +63,8 @@ export class ServerRequestParameters { this.scopes = scopes? [ ...scopes] : [clientId]; // set state (already set at top level) - this.state = state; + this.encodedState = encodedState; + this.state = RequestUtils.parseLibraryState(encodedState).state; // set correlationId this.correlationId = correlationId; diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 3c2d2b72f9..9df0bf3b75 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -94,6 +94,7 @@ export interface CacheResult { */ export type ResponseStateInfo = { state: string; + iat: number, stateMatch: boolean; requestType: string; }; @@ -1103,9 +1104,12 @@ export class UserAgentApplication { throw AuthError.createUnexpectedError("Hash was not parsed correctly."); } if (parameters.hasOwnProperty("state")) { + const parsedState = RequestUtils.parseLibraryState(parameters.state); + stateResponse = { requestType: Constants.unknown, - state: parameters.state, + state: parsedState.state, + iat: parsedState.iat, stateMatch: false }; } else { @@ -1389,7 +1393,8 @@ export class UserAgentApplication { // Generate and cache accessTokenKey and accessTokenValue const expiresIn = TimeUtils.parseExpiresIn(parameters[ServerHashParamKeys.EXPIRES_IN]); - expiration = TimeUtils.now() + expiresIn; + const parsedState = RequestUtils.parseLibraryState(parameters[ServerHashParamKeys.STATE]); + expiration = parsedState.iat + expiresIn; const accessTokenKey = new AccessTokenKey(authority, this.clientId, scope, clientObj.uid, clientObj.utid); const accessTokenValue = new AccessTokenValue(parameters[ServerHashParamKeys.ACCESS_TOKEN], idTokenObj.rawIdToken, expiration.toString(), clientInfo); diff --git a/lib/msal-core/src/utils/Constants.ts b/lib/msal-core/src/utils/Constants.ts index 0f39b56ca9..2d40024f9e 100644 --- a/lib/msal-core/src/utils/Constants.ts +++ b/lib/msal-core/src/utils/Constants.ts @@ -58,6 +58,7 @@ export class Constants { */ export enum ServerHashParamKeys { SCOPE = "scope", + STATE = "state", ERROR = "error", ERROR_DESCRIPTION = "error_description", ACCESS_TOKEN = "access_token", diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 0b8e29ba1e..9acf9a5d47 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -10,6 +10,12 @@ import { ScopeSet } from "../ScopeSet"; import { StringDict } from "../MsalTypes"; import { StringUtils } from "../utils/StringUtils"; import { CryptoUtils } from "../utils/CryptoUtils"; +import { TimeUtils } from './TimeUtils'; + +export type StateObject = { + state: string, + iat: number +} /** * @hidden @@ -140,7 +146,33 @@ export class RequestUtils { */ static validateAndGenerateState(state: string): string { // append GUID to user set state or set one for the user if null - return !StringUtils.isEmpty(state) ? CryptoUtils.createNewGuid() + "|" + state : CryptoUtils.createNewGuid(); + return !StringUtils.isEmpty(state) ? RequestUtils.generateLibraryState() + "|" + state : RequestUtils.generateLibraryState(); + } + + static generateLibraryState(): string { + const stateObject: StateObject = { + state: CryptoUtils.createNewGuid(), + iat: TimeUtils.now() + }; + + const stateString = JSON.stringify(stateObject); + + return CryptoUtils.base64Encode(stateString); + } + + static parseLibraryState(state: string): StateObject { + if (CryptoUtils.isGuid(state)) { + return { + state, + iat: TimeUtils.now() + } + } + + const stateString = CryptoUtils.base64Decode(state); + + const stateObject = JSON.parse(stateString); + + return stateObject; } /** diff --git a/lib/msal-core/src/utils/TimeUtils.ts b/lib/msal-core/src/utils/TimeUtils.ts index 3960a5ccff..92c8d499ba 100644 --- a/lib/msal-core/src/utils/TimeUtils.ts +++ b/lib/msal-core/src/utils/TimeUtils.ts @@ -21,7 +21,7 @@ export class TimeUtils { } /** - * return the current time in Unix time. Date.getTime() returns in milliseconds. + * Return the current time in Unix time (seconds). Date.getTime() returns in milliseconds. */ static now(): number { return Math.round(new Date().getTime() / 1000.0); diff --git a/lib/msal-core/src/utils/UrlUtils.ts b/lib/msal-core/src/utils/UrlUtils.ts index 8daa4bf829..b1a2c22f28 100644 --- a/lib/msal-core/src/utils/UrlUtils.ts +++ b/lib/msal-core/src/utils/UrlUtils.ts @@ -52,7 +52,7 @@ export class UrlUtils { str.push("client_id=" + encodeURIComponent(serverRequestParams.clientId)); str.push("redirect_uri=" + encodeURIComponent(serverRequestParams.redirectUri)); - str.push("state=" + encodeURIComponent(serverRequestParams.state)); + str.push("state=" + encodeURIComponent(serverRequestParams.encodedState)); str.push("nonce=" + encodeURIComponent(serverRequestParams.nonce)); str.push("client_info=1"); diff --git a/samples/react-sample-app/package-lock.json b/samples/react-sample-app/package-lock.json index 22856c45ff..d2763a48bd 100644 --- a/samples/react-sample-app/package-lock.json +++ b/samples/react-sample-app/package-lock.json @@ -8308,9 +8308,9 @@ "integrity": "sha512-sGkPx+VjMtmA6MX27oA4FBFELFCZZ4S4XqeGOXCv68tT+jb3vk/RyaKWP0PTKyWtmLSM0b+adUTEvbs1PEaH2w==" }, "msal": { - "version": "1.2.1", - "resolved": "https://registry.npmjs.org/msal/-/msal-1.2.1.tgz", - "integrity": "sha512-Zo28eyRtT/Un+zcpMfPtTPD+eo/OqzsRER0k5dyk8Mje/K1oLlaEOAgZHlJs59Y2xyuVg8OrcKqSn/1MeNjZYw==", + "version": "1.2.2-beta.3", + "resolved": "https://registry.npmjs.org/msal/-/msal-1.2.2-beta.3.tgz", + "integrity": "sha512-UifKAEPPCbXWaUx2UCyuXBJVdDt3DKx7qbo8HzU2p7JQ9qa1Ab9ycNGluOXc6Ik73aA9sqdPuyV5coQAuX2z9A==", "requires": { "tslib": "^1.9.3" } diff --git a/samples/react-sample-app/package.json b/samples/react-sample-app/package.json index 56b05f289c..b5597d25f3 100644 --- a/samples/react-sample-app/package.json +++ b/samples/react-sample-app/package.json @@ -3,7 +3,7 @@ "version": "0.1.0", "private": true, "dependencies": { - "msal": "^1.0.0", + "msal": "^1.2.2-beta.3", "prop-types": "^15.7.2", "react": "^16.8.6", "react-app-polyfill": "^1.0.1", From acbe15690649b215e154331b0e36b03de051bd64 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Tue, 24 Mar 2020 13:32:46 -0700 Subject: [PATCH 02/10] Use entire encoded state value as-is, dont decode for validation --- lib/msal-core/src/ServerRequestParameters.ts | 8 +++----- lib/msal-core/src/UserAgentApplication.ts | 8 ++++---- lib/msal-core/src/utils/RequestUtils.ts | 17 ++++++++++++++--- lib/msal-core/src/utils/UrlUtils.ts | 2 +- 4 files changed, 22 insertions(+), 13 deletions(-) diff --git a/lib/msal-core/src/ServerRequestParameters.ts b/lib/msal-core/src/ServerRequestParameters.ts index 294da74447..304b59a405 100644 --- a/lib/msal-core/src/ServerRequestParameters.ts +++ b/lib/msal-core/src/ServerRequestParameters.ts @@ -25,7 +25,6 @@ export class ServerRequestParameters { nonce: string; state: string; - encodedState: string; // telemetry information xClientVer: string; @@ -52,9 +51,9 @@ export class ServerRequestParameters { * @param scope * @param responseType * @param redirectUri - * @param encodedState + * @param state */ - constructor (authority: Authority, clientId: string, responseType: string, redirectUri: string, scopes: Array, encodedState: string, correlationId: string) { + constructor (authority: Authority, clientId: string, responseType: string, redirectUri: string, scopes: Array, state: string, correlationId: string) { this.authorityInstance = authority; this.clientId = clientId; this.nonce = CryptoUtils.createNewGuid(); @@ -63,8 +62,7 @@ export class ServerRequestParameters { this.scopes = scopes? [ ...scopes] : [clientId]; // set state (already set at top level) - this.encodedState = encodedState; - this.state = RequestUtils.parseLibraryState(encodedState).state; + this.state = state; // set correlationId this.correlationId = correlationId; diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 9df0bf3b75..707bd9f9fb 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -94,7 +94,7 @@ export interface CacheResult { */ export type ResponseStateInfo = { state: string; - iat: number, + ts: number, stateMatch: boolean; requestType: string; }; @@ -1108,8 +1108,8 @@ export class UserAgentApplication { stateResponse = { requestType: Constants.unknown, - state: parsedState.state, - iat: parsedState.iat, + state: parameters.state, + ts: parsedState.ts, stateMatch: false }; } else { @@ -1394,7 +1394,7 @@ export class UserAgentApplication { // Generate and cache accessTokenKey and accessTokenValue const expiresIn = TimeUtils.parseExpiresIn(parameters[ServerHashParamKeys.EXPIRES_IN]); const parsedState = RequestUtils.parseLibraryState(parameters[ServerHashParamKeys.STATE]); - expiration = parsedState.iat + expiresIn; + expiration = parsedState.ts + expiresIn; const accessTokenKey = new AccessTokenKey(authority, this.clientId, scope, clientObj.uid, clientObj.utid); const accessTokenValue = new AccessTokenValue(parameters[ServerHashParamKeys.ACCESS_TOKEN], idTokenObj.rawIdToken, expiration.toString(), clientInfo); diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 9acf9a5d47..9cb76a2cf4 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -14,7 +14,7 @@ import { TimeUtils } from './TimeUtils'; export type StateObject = { state: string, - iat: number + ts: number } /** @@ -149,10 +149,15 @@ export class RequestUtils { return !StringUtils.isEmpty(state) ? RequestUtils.generateLibraryState() + "|" + state : RequestUtils.generateLibraryState(); } + /** + * Generates the state value used by the library. + * + * @returns Base64 encoded string representing the state + */ static generateLibraryState(): string { const stateObject: StateObject = { state: CryptoUtils.createNewGuid(), - iat: TimeUtils.now() + ts: TimeUtils.now() }; const stateString = JSON.stringify(stateObject); @@ -160,11 +165,17 @@ export class RequestUtils { return CryptoUtils.base64Encode(stateString); } + /** + * Decodes the state value into a StateObject + * + * @param state State value returned in the request + * @returns Parsed values from the encoded state value + */ static parseLibraryState(state: string): StateObject { if (CryptoUtils.isGuid(state)) { return { state, - iat: TimeUtils.now() + ts: TimeUtils.now() } } diff --git a/lib/msal-core/src/utils/UrlUtils.ts b/lib/msal-core/src/utils/UrlUtils.ts index b1a2c22f28..8daa4bf829 100644 --- a/lib/msal-core/src/utils/UrlUtils.ts +++ b/lib/msal-core/src/utils/UrlUtils.ts @@ -52,7 +52,7 @@ export class UrlUtils { str.push("client_id=" + encodeURIComponent(serverRequestParams.clientId)); str.push("redirect_uri=" + encodeURIComponent(serverRequestParams.redirectUri)); - str.push("state=" + encodeURIComponent(serverRequestParams.encodedState)); + str.push("state=" + encodeURIComponent(serverRequestParams.state)); str.push("nonce=" + encodeURIComponent(serverRequestParams.nonce)); str.push("client_info=1"); From 02b6f4570d47a628a6c0116cf94917622ef06ff2 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Tue, 24 Mar 2020 13:48:18 -0700 Subject: [PATCH 03/10] Ensure we separate library state from user state --- lib/msal-core/src/utils/RequestUtils.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 9cb76a2cf4..98c97a436d 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -172,14 +172,16 @@ export class RequestUtils { * @returns Parsed values from the encoded state value */ static parseLibraryState(state: string): StateObject { - if (CryptoUtils.isGuid(state)) { + const libraryState = state.split("|")[0]; + + if (CryptoUtils.isGuid(libraryState)) { return { state, ts: TimeUtils.now() } } - const stateString = CryptoUtils.base64Decode(state); + const stateString = CryptoUtils.base64Decode(libraryState); const stateObject = JSON.parse(stateString); From 594de76f464493272486d48818f76d9651f20a97 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Tue, 24 Mar 2020 17:49:40 -0700 Subject: [PATCH 04/10] Fix tests and lint for encoded state value --- lib/msal-core/src/ServerRequestParameters.ts | 2 +- lib/msal-core/src/utils/RequestUtils.ts | 17 ++-- lib/msal-core/test/TestConstants.ts | 24 +++--- .../test/UserAgentApplication.spec.ts | 82 ++++++++++--------- lib/msal-core/test/utils/RequestUtils.spec.ts | 15 ++-- 5 files changed, 77 insertions(+), 63 deletions(-) diff --git a/lib/msal-core/src/ServerRequestParameters.ts b/lib/msal-core/src/ServerRequestParameters.ts index 304b59a405..4d724f9310 100644 --- a/lib/msal-core/src/ServerRequestParameters.ts +++ b/lib/msal-core/src/ServerRequestParameters.ts @@ -10,7 +10,7 @@ import { StringDict } from "./MsalTypes"; import { Account } from "./Account"; import { SSOTypes, Constants, PromptState, libraryVersion } from "./utils/Constants"; import { StringUtils } from "./utils/StringUtils"; -import { RequestUtils } from './utils/RequestUtils'; +import { RequestUtils } from "./utils/RequestUtils"; /** * Nonce: OIDC Nonce definition: https://openid.net/specs/openid-connect-core-1_0.html#IDToken diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 98c97a436d..9284b39bb5 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -10,12 +10,13 @@ import { ScopeSet } from "../ScopeSet"; import { StringDict } from "../MsalTypes"; import { StringUtils } from "../utils/StringUtils"; import { CryptoUtils } from "../utils/CryptoUtils"; -import { TimeUtils } from './TimeUtils'; +import { TimeUtils } from "./TimeUtils"; +import { ClientAuthError } from "../error/ClientAuthError"; export type StateObject = { state: string, ts: number -} +}; /** * @hidden @@ -178,14 +179,18 @@ export class RequestUtils { return { state, ts: TimeUtils.now() - } + }; } - const stateString = CryptoUtils.base64Decode(libraryState); + try { + const stateString = CryptoUtils.base64Decode(libraryState); - const stateObject = JSON.parse(stateString); + const stateObject = JSON.parse(stateString); - return stateObject; + return stateObject; + } catch (e) { + throw ClientAuthError.createInvalidStateError(state, null); + } } /** diff --git a/lib/msal-core/test/TestConstants.ts b/lib/msal-core/test/TestConstants.ts index a1e61a6d20..59066d9199 100644 --- a/lib/msal-core/test/TestConstants.ts +++ b/lib/msal-core/test/TestConstants.ts @@ -38,18 +38,20 @@ export const TEST_TOKEN_LIFETIMES = { TEST_ACCESS_TOKEN_EXP: 1537234948 }; + + // Test Hashes -export const TEST_HASHES = { - TEST_SUCCESS_ID_TOKEN_HASH: `#id_token=${TEST_TOKENS.IDTOKEN_V2}&client_info=${TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO}&state=RANDOM-GUID-HERE|`, - TEST_SUCCESS_ACCESS_TOKEN_HASH: `#access_token=${TEST_TOKENS.ACCESSTOKEN}&id_token=${TEST_TOKENS.IDTOKEN_V2}&scope=test&expiresIn=${TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN}&client_info=${TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO}&state=RANDOM-GUID-HERE|`, - TEST_ERROR_HASH: "#error=error_code&error_description=msal+error+description&state=RANDOM-GUID-HERE|", - TEST_INTERACTION_REQ_ERROR_HASH1: "#error=interaction_required&error_description=msal+error+description&state=RANDOM-GUID-HERE|", - TEST_INTERACTION_REQ_ERROR_HASH2: "#error=interaction_required&error_description=msal+error+description+interaction_required&state=RANDOM-GUID-HERE|", - TEST_LOGIN_REQ_ERROR_HASH1: "#error=login_required&error_description=msal+error+description&state=RANDOM-GUID-HERE|", - TEST_LOGIN_REQ_ERROR_HASH2: "#error=login_required&error_description=msal+error+description+login_required&state=RANDOM-GUID-HERE|", - TEST_CONSENT_REQ_ERROR_HASH1: "#error=consent_required&error_description=msal+error+description&state=RANDOM-GUID-HERE|", - TEST_CONSENT_REQ_ERROR_HASH2: "#error=consent_required&error_description=msal+error+description+consent_required&state=RANDOM-GUID-HERE|" -}; +export const testHashesForState = state => ({ + TEST_SUCCESS_ID_TOKEN_HASH: `#id_token=${TEST_TOKENS.IDTOKEN_V2}&client_info=${TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO}&state=${state}|`, + TEST_SUCCESS_ACCESS_TOKEN_HASH: `#access_token=${TEST_TOKENS.ACCESSTOKEN}&id_token=${TEST_TOKENS.IDTOKEN_V2}&scope=test&expiresIn=${TEST_TOKEN_LIFETIMES.DEFAULT_EXPIRES_IN}&client_info=${TEST_DATA_CLIENT_INFO.TEST_RAW_CLIENT_INFO}&state=${state}|`, + TEST_ERROR_HASH: `#error=error_code&error_description=msal+error+description&state=${state}|`, + TEST_INTERACTION_REQ_ERROR_HASH1: `#error=interaction_required&error_description=msal+error+description&state=${state}|`, + TEST_INTERACTION_REQ_ERROR_HASH2: `#error=interaction_required&error_description=msal+error+description+interaction_required&state=${state}|`, + TEST_LOGIN_REQ_ERROR_HASH1: `#error=login_required&error_description=msal+error+description&state=${state}|`, + TEST_LOGIN_REQ_ERROR_HASH2: `#error=login_required&error_description=msal+error+description+login_required&state=${state}|`, + TEST_CONSENT_REQ_ERROR_HASH1: `#error=consent_required&error_description=msal+error+description&state=${state}|`, + TEST_CONSENT_REQ_ERROR_HASH2: `#error=consent_required&error_description=msal+error+description+consent_required&state=${state}|` +}); // Test MSAL config params export const TEST_CONFIG = { diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts index aa41c38696..88c37650f1 100644 --- a/lib/msal-core/test/UserAgentApplication.spec.ts +++ b/lib/msal-core/test/UserAgentApplication.spec.ts @@ -28,9 +28,10 @@ import { ClientAuthErrorMessage } from "../src/error/ClientAuthError"; import { ClientConfigurationErrorMessage } from "../src/error/ClientConfigurationError"; import { InteractionRequiredAuthErrorMessage } from "../src/error/InteractionRequiredAuthError"; import { ServerRequestParameters } from "../src/ServerRequestParameters"; -import { TEST_URIS, TEST_DATA_CLIENT_INFO, TEST_HASHES, TEST_TOKENS, TEST_CONFIG, TEST_TOKEN_LIFETIMES } from "./TestConstants"; +import { TEST_URIS, TEST_DATA_CLIENT_INFO, testHashesForState, TEST_TOKENS, TEST_CONFIG, TEST_TOKEN_LIFETIMES } from "./TestConstants"; import { IdToken } from "../src/IdToken"; import { TimeUtils } from "../src/utils/TimeUtils"; +import { RequestUtils } from "../src/utils/RequestUtils"; type kv = { [key: string]: string; @@ -39,6 +40,7 @@ type kv = { describe("UserAgentApplication.ts Class", function () { // Test state params + const TEST_LIBRARY_STATE = RequestUtils.generateLibraryState(); const TEST_USER_STATE_NUM = "1234"; const TEST_USER_STATE_URL = "https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-implicit-grant-flow/scope1"; @@ -52,7 +54,7 @@ describe("UserAgentApplication.ts Class", function () { const TEST_ERROR_DESC = "msal error description"; const TEST_ACCESS_DENIED = "access_denied"; - const TEST_SERVER_ERROR_SUBCODE_CANCEL = "#error=access_denied&error_subcode=cancel&state=RANDOM-GUID-HERE|"; + const TEST_SERVER_ERROR_SUBCODE_CANCEL = `#error=access_denied&error_subcode=cancel&state=${TEST_LIBRARY_STATE}|`; // Test SSO params const TEST_LOGIN_HINT = "test@test.com"; @@ -706,8 +708,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("Calls the error callback if two callbacks are sent", function (done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_ERROR_HASH + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_ERROR_HASH + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, accountState: string) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof ServerError).to.be.true; @@ -722,9 +724,9 @@ describe("UserAgentApplication.ts Class", function () { }); it("Calls the token callback if two callbacks are sent", function (done) { - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, TEST_NONCE); - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); + cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, TEST_NONCE); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM); const checkResponseFromServer = function(response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; @@ -738,9 +740,9 @@ describe("UserAgentApplication.ts Class", function () { }); it("Calls the response callback if single callback is sent", function (done) { - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, TEST_NONCE); - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); + cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, TEST_NONCE); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM); const checkResponseFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; @@ -1085,9 +1087,9 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of response", function(done) { - const successHash = TEST_HASHES.TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM; - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, TEST_NONCE); + const successHash = testHashesForState(TEST_LIBRARY_STATE).TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM; + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); + cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, TEST_NONCE); cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, successHash); const checkRespFromServer = function(response: AuthResponse) { expect(response.uniqueId).to.be.eq(TEST_UNIQUE_ID); @@ -1101,8 +1103,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of error", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_ERROR_HASH + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_ERROR_HASH + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof ServerError).to.be.true; @@ -1119,7 +1121,7 @@ describe("UserAgentApplication.ts Class", function () { // TEST_SERVER_ERROR_SUBCODE_CANCEL it("tests saveTokenForHash in case of non-consentable scopes / return to the application without consenting", function(done) { cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_SERVER_ERROR_SUBCODE_CANCEL + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof ServerError).to.be.true; @@ -1132,8 +1134,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests if you get the state back in errorReceived callback, if state is a number", function (done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_ERROR_HASH + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_ERROR_HASH + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorHasState = function(error: AuthError, response: AuthResponse) { expect(response.accountState).to.include(TEST_USER_STATE_NUM); done(); @@ -1142,8 +1144,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests if you get the state back in errorReceived callback, if state is a url", function (done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_ERROR_HASH + TEST_USER_STATE_URL); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_ERROR_HASH + TEST_USER_STATE_URL); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorHasState = function(error: AuthError, response: AuthResponse) { expect(response.accountState).to.include(TEST_USER_STATE_URL); done(); @@ -1162,11 +1164,11 @@ describe("UserAgentApplication.ts Class", function () { it("tests that expiresIn returns the correct date for access tokens", function (done) { sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); - const acquireTokenAccountKey = AuthCache.generateAcquireTokenAccountKey(account.homeAccountIdentifier, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + const acquireTokenAccountKey = AuthCache.generateAcquireTokenAccountKey(account.homeAccountIdentifier, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); cacheStorage.setItem(acquireTokenAccountKey, JSON.stringify(account)); - const successHash = TEST_HASHES.TEST_SUCCESS_ACCESS_TOKEN_HASH + TEST_USER_STATE_NUM; - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_ACQ_TOKEN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, TEST_NONCE); + const successHash = testHashesForState(TEST_LIBRARY_STATE).TEST_SUCCESS_ACCESS_TOKEN_HASH + TEST_USER_STATE_NUM; + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_ACQ_TOKEN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); + cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, TEST_NONCE); cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, successHash); const checkRespFromServer = function(response: AuthResponse) { @@ -1182,11 +1184,11 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests that expiresIn returns the correct date for id tokens", function (done) { - const acquireTokenAccountKey = AuthCache.generateAcquireTokenAccountKey(account.homeAccountIdentifier, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + const acquireTokenAccountKey = AuthCache.generateAcquireTokenAccountKey(account.homeAccountIdentifier, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); cacheStorage.setItem(acquireTokenAccountKey, JSON.stringify(account)); - const successHash = TEST_HASHES.TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM; - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, TEST_NONCE); + const successHash = testHashesForState(TEST_LIBRARY_STATE).TEST_SUCCESS_ID_TOKEN_HASH + TEST_USER_STATE_NUM; + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); + cacheStorage.setItem(`${TemporaryCacheKeys.NONCE_IDTOKEN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, TEST_NONCE); cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, successHash); const checkRespFromServer = function(response: AuthResponse) { expect(response.uniqueId).to.be.eq(TEST_UNIQUE_ID); @@ -1222,8 +1224,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of interaction_required error code", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_INTERACTION_REQ_ERROR_HASH1 + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_INTERACTION_REQ_ERROR_HASH1 + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof InteractionRequiredAuthError).to.be.true; @@ -1238,8 +1240,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of interaction_required error code and description", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_INTERACTION_REQ_ERROR_HASH2 + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_INTERACTION_REQ_ERROR_HASH2 + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof InteractionRequiredAuthError).to.be.true; @@ -1256,8 +1258,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of login_required error code", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_LOGIN_REQ_ERROR_HASH1 + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_LOGIN_REQ_ERROR_HASH1 + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof InteractionRequiredAuthError).to.be.true; @@ -1272,8 +1274,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of login_required error code and description", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_LOGIN_REQ_ERROR_HASH2 + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_LOGIN_REQ_ERROR_HASH2 + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof InteractionRequiredAuthError).to.be.true; @@ -1290,8 +1292,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of consent_required error code", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_CONSENT_REQ_ERROR_HASH1 + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_CONSENT_REQ_ERROR_HASH1 + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof InteractionRequiredAuthError).to.be.true; @@ -1306,8 +1308,8 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests saveTokenForHash in case of consent_required error code and description", function(done) { - cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, TEST_HASHES.TEST_CONSENT_REQ_ERROR_HASH2 + TEST_USER_STATE_NUM); - cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|RANDOM-GUID-HERE|${TEST_USER_STATE_NUM}`, "RANDOM-GUID-HERE|" + TEST_USER_STATE_NUM); + cacheStorage.setItem(TemporaryCacheKeys.URL_HASH, testHashesForState(TEST_LIBRARY_STATE).TEST_CONSENT_REQ_ERROR_HASH2 + TEST_USER_STATE_NUM); + cacheStorage.setItem(`${TemporaryCacheKeys.STATE_LOGIN}|${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); const checkErrorFromServer = function(error: AuthError, response: AuthResponse) { expect(cacheStorage.getItem(TemporaryCacheKeys.URL_HASH)).to.be.null; expect(error instanceof InteractionRequiredAuthError).to.be.true; diff --git a/lib/msal-core/test/utils/RequestUtils.spec.ts b/lib/msal-core/test/utils/RequestUtils.spec.ts index 91beb5c61e..af8d0666ec 100644 --- a/lib/msal-core/test/utils/RequestUtils.spec.ts +++ b/lib/msal-core/test/utils/RequestUtils.spec.ts @@ -3,8 +3,10 @@ import { RequestUtils } from "../../src/utils/RequestUtils"; import { CryptoUtils } from "../../src/utils/CryptoUtils"; import { AuthenticationParameters } from "../../src/AuthenticationParameters"; import { ClientConfigurationError, ClientConfigurationErrorMessage } from "../../src/error/ClientConfigurationError"; -import { TEST_CONFIG } from "../TestConstants"; +import { TEST_CONFIG, TEST_TOKEN_LIFETIMES } from "../TestConstants"; import { StringDict } from "../../src/MsalTypes"; +import { TimeUtils } from "../../src/utils/TimeUtils"; +import sinon from "sinon"; describe("RequestUtils.ts class", () => { @@ -89,13 +91,16 @@ describe("RequestUtils.ts class", () => { }); it("validate and generate state", () => { + sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); const userState: string = "abcd"; const state: string = RequestUtils.validateAndGenerateState(userState); + const now = TimeUtils.now(); const splitKey: Array = state.split("|"); + expect(splitKey[1]).to.contain("abcd"); - expect(state).to.contain("|"); - expect(state).to.contain("abcd"); - expect(CryptoUtils.isGuid(splitKey[0])).to.be.equal(true); + const parsedState = RequestUtils.parseLibraryState(state); + expect(CryptoUtils.isGuid(parsedState.state)).to.be.equal(true); + expect(parsedState.ts === now).to.be.equal(true); }); it("validate and generate correlationId", () => { @@ -112,7 +117,7 @@ describe("RequestUtils.ts class", () => { expect(request.scopes).to.be.equal(undefined); expect(request.prompt).to.be.equal(undefined); expect(request.extraQueryParameters).to.be.equal(undefined); - expect(CryptoUtils.isGuid(request.state)).to.be.equal(true); + expect(typeof request.state).to.be.equal("string"); expect(CryptoUtils.isGuid(request.correlationId)).to.be.equal(true); }); From 3344225b401551f065014c31be1f1ca1ffcf656d Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Wed, 25 Mar 2020 10:48:06 -0700 Subject: [PATCH 05/10] Fix time-based tests by ensure TimeUtils is properly stubbed --- lib/msal-core/test/UserAgentApplication.spec.ts | 4 +++- lib/msal-core/test/utils/RequestUtils.spec.ts | 3 ++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/msal-core/test/UserAgentApplication.spec.ts b/lib/msal-core/test/UserAgentApplication.spec.ts index 88c37650f1..3c8ca27c56 100644 --- a/lib/msal-core/test/UserAgentApplication.spec.ts +++ b/lib/msal-core/test/UserAgentApplication.spec.ts @@ -40,7 +40,9 @@ type kv = { describe("UserAgentApplication.ts Class", function () { // Test state params + sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); const TEST_LIBRARY_STATE = RequestUtils.generateLibraryState(); + const TEST_USER_STATE_NUM = "1234"; const TEST_USER_STATE_URL = "https://docs.microsoft.com/en-us/azure/active-directory/develop/v2-oauth2-implicit-grant-flow/scope1"; @@ -103,6 +105,7 @@ describe("UserAgentApplication.ts Class", function () { sinon.stub(msal.getAuthorityInstance(), "EndSessionEndpoint").value(validOpenIdConfigurationResponse.EndSessionEndpoint); sinon.stub(msal.getAuthorityInstance(), "SelfSignedJwtAudience").value(validOpenIdConfigurationResponse.Issuer); sinon.stub(WindowUtils, "isInIframe").returns(false); + sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); }; const setUtilUnifiedCacheQPStubs = function (params: kv) { @@ -1163,7 +1166,6 @@ describe("UserAgentApplication.ts Class", function () { }); it("tests that expiresIn returns the correct date for access tokens", function (done) { - sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); const acquireTokenAccountKey = AuthCache.generateAcquireTokenAccountKey(account.homeAccountIdentifier, `${TEST_LIBRARY_STATE}|${TEST_USER_STATE_NUM}`); cacheStorage.setItem(acquireTokenAccountKey, JSON.stringify(account)); const successHash = testHashesForState(TEST_LIBRARY_STATE).TEST_SUCCESS_ACCESS_TOKEN_HASH + TEST_USER_STATE_NUM; diff --git a/lib/msal-core/test/utils/RequestUtils.spec.ts b/lib/msal-core/test/utils/RequestUtils.spec.ts index af8d0666ec..b736702ed6 100644 --- a/lib/msal-core/test/utils/RequestUtils.spec.ts +++ b/lib/msal-core/test/utils/RequestUtils.spec.ts @@ -91,7 +91,7 @@ describe("RequestUtils.ts class", () => { }); it("validate and generate state", () => { - sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); + const nowStub = sinon.stub(TimeUtils, "now").returns(TEST_TOKEN_LIFETIMES.BASELINE_DATE_CHECK); const userState: string = "abcd"; const state: string = RequestUtils.validateAndGenerateState(userState); const now = TimeUtils.now(); @@ -101,6 +101,7 @@ describe("RequestUtils.ts class", () => { const parsedState = RequestUtils.parseLibraryState(state); expect(CryptoUtils.isGuid(parsedState.state)).to.be.equal(true); expect(parsedState.ts === now).to.be.equal(true); + nowStub.restore(); }); it("validate and generate correlationId", () => { From 2273f8e25d2b25cac60b27877ab5b02d6a76dbd3 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Wed, 25 Mar 2020 11:11:17 -0700 Subject: [PATCH 06/10] Rename librarystateobject type, timestamp property, and use constant for vertical bar --- lib/msal-core/src/ScopeSet.ts | 4 ++-- lib/msal-core/src/UserAgentApplication.ts | 6 +++--- lib/msal-core/src/utils/RequestUtils.ts | 17 +++++++++-------- 3 files changed, 14 insertions(+), 13 deletions(-) diff --git a/lib/msal-core/src/ScopeSet.ts b/lib/msal-core/src/ScopeSet.ts index 1043eb702c..0e81587a65 100644 --- a/lib/msal-core/src/ScopeSet.ts +++ b/lib/msal-core/src/ScopeSet.ts @@ -4,7 +4,7 @@ */ import { ClientConfigurationError } from "./error/ClientConfigurationError"; -import { AuthenticationParameters } from "./AuthenticationParameters"; +import { Constants } from './utils/Constants'; export class ScopeSet { @@ -117,7 +117,7 @@ export class ScopeSet { */ static getScopeFromState(state: string): string { if (state) { - const splitIndex = state.indexOf("|"); + const splitIndex = state.indexOf(Constants.resourceDelimiter); if (splitIndex > -1 && splitIndex + 1 < state.length) { return state.substring(splitIndex + 1); } diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 707bd9f9fb..63c73a9b2d 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -94,7 +94,7 @@ export interface CacheResult { */ export type ResponseStateInfo = { state: string; - ts: number, + timestamp: number, stateMatch: boolean; requestType: string; }; @@ -1109,7 +1109,7 @@ export class UserAgentApplication { stateResponse = { requestType: Constants.unknown, state: parameters.state, - ts: parsedState.ts, + timestamp: parsedState.ts, stateMatch: false }; } else { @@ -1706,7 +1706,7 @@ export class UserAgentApplication { */ getAccountState (state: string) { if (state) { - const splitIndex = state.indexOf("|"); + const splitIndex = state.indexOf(Constants.resourceDelimiter); if (splitIndex > -1 && splitIndex + 1 < state.length) { return state.substring(splitIndex + 1); } diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 9284b39bb5..2078c7927b 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -13,7 +13,7 @@ import { CryptoUtils } from "../utils/CryptoUtils"; import { TimeUtils } from "./TimeUtils"; import { ClientAuthError } from "../error/ClientAuthError"; -export type StateObject = { +export type LibraryStateObject = { state: string, ts: number }; @@ -143,11 +143,12 @@ export class RequestUtils { * @ignore * * generate unique state per request - * @param request + * @param userState User-provided state value + * @returns State string include library state and user state */ - static validateAndGenerateState(state: string): string { - // append GUID to user set state or set one for the user if null - return !StringUtils.isEmpty(state) ? RequestUtils.generateLibraryState() + "|" + state : RequestUtils.generateLibraryState(); + static validateAndGenerateState(userState: string): string { + // append GUID to user set state or set one for the user if null + return !StringUtils.isEmpty(userState) ? `${RequestUtils.generateLibraryState()}${Constants.resourceDelimiter}${userState}` : RequestUtils.generateLibraryState(); } /** @@ -156,7 +157,7 @@ export class RequestUtils { * @returns Base64 encoded string representing the state */ static generateLibraryState(): string { - const stateObject: StateObject = { + const stateObject: LibraryStateObject = { state: CryptoUtils.createNewGuid(), ts: TimeUtils.now() }; @@ -172,8 +173,8 @@ export class RequestUtils { * @param state State value returned in the request * @returns Parsed values from the encoded state value */ - static parseLibraryState(state: string): StateObject { - const libraryState = state.split("|")[0]; + static parseLibraryState(state: string): LibraryStateObject { + const libraryState = state.split(Constants.resourceDelimiter)[0]; if (CryptoUtils.isGuid(libraryState)) { return { From 875708e62766719ff8f550b4995f64fb7597e6ad Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Wed, 25 Mar 2020 11:12:15 -0700 Subject: [PATCH 07/10] Fix lint --- lib/msal-core/src/ScopeSet.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msal-core/src/ScopeSet.ts b/lib/msal-core/src/ScopeSet.ts index 0e81587a65..76c2cc6462 100644 --- a/lib/msal-core/src/ScopeSet.ts +++ b/lib/msal-core/src/ScopeSet.ts @@ -4,7 +4,7 @@ */ import { ClientConfigurationError } from "./error/ClientConfigurationError"; -import { Constants } from './utils/Constants'; +import { Constants } from "./utils/Constants"; export class ScopeSet { From 0bd64903574fb7e32fe19059f549fe1593a80929 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Wed, 25 Mar 2020 18:25:11 -0700 Subject: [PATCH 08/10] Rename LibraryStateObject.state to id --- lib/msal-core/src/UserAgentApplication.ts | 12 ++++++------ lib/msal-core/src/utils/RequestUtils.ts | 6 +++--- lib/msal-core/test/utils/RequestUtils.spec.ts | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 0bf5940b47..62b3fe35ce 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -1023,7 +1023,7 @@ export class UserAgentApplication { /** * @hidden - * 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 + * 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. */ @@ -1043,13 +1043,13 @@ export class UserAgentApplication { /** * @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 + * 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"); - + // clear hash from window window.location.hash = ""; @@ -1088,7 +1088,7 @@ export class UserAgentApplication { if (!parameters) { throw AuthError.createUnexpectedError("Hash was not parsed correctly."); } - if (parameters.hasOwnProperty("state")) { + if (parameters.hasOwnProperty(ServerHashParamKeys.STATE)) { const parsedState = RequestUtils.parseLibraryState(parameters.state); stateResponse = { @@ -1106,13 +1106,13 @@ export class UserAgentApplication { */ // loginRedirect - if (stateResponse.state === this.cacheStorage.getItem(`${TemporaryCacheKeys.STATE_LOGIN}${Constants.resourceDelimiter}${stateResponse.state}`, this.inCookie) || stateResponse.state === this.silentAuthenticationState) { // loginRedirect + if (stateResponse.state === this.cacheStorage.getItem(`${TemporaryCacheKeys.STATE_LOGIN}${Constants.resourceDelimiter}${stateResponse.state}`, this.inCookie) || stateResponse.state === this.silentAuthenticationState) { stateResponse.requestType = Constants.login; stateResponse.stateMatch = true; return stateResponse; } // acquireTokenRedirect - else if (stateResponse.state === this.cacheStorage.getItem(`${TemporaryCacheKeys.STATE_ACQ_TOKEN}${Constants.resourceDelimiter}${stateResponse.state}`, this.inCookie)) { // acquireTokenRedirect + else if (stateResponse.state === this.cacheStorage.getItem(`${TemporaryCacheKeys.STATE_ACQ_TOKEN}${Constants.resourceDelimiter}${stateResponse.state}`, this.inCookie)) { stateResponse.requestType = Constants.renewToken; stateResponse.stateMatch = true; return stateResponse; diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 20a480a6d6..158f3194ab 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -14,7 +14,7 @@ import { TimeUtils } from "./TimeUtils"; import { ClientAuthError } from "../error/ClientAuthError"; export type LibraryStateObject = { - state: string, + id: string, ts: number }; @@ -151,7 +151,7 @@ export class RequestUtils { */ static generateLibraryState(): string { const stateObject: LibraryStateObject = { - state: CryptoUtils.createNewGuid(), + id: CryptoUtils.createNewGuid(), ts: TimeUtils.now() }; @@ -171,7 +171,7 @@ export class RequestUtils { if (CryptoUtils.isGuid(libraryState)) { return { - state, + id: libraryState, ts: TimeUtils.now() }; } diff --git a/lib/msal-core/test/utils/RequestUtils.spec.ts b/lib/msal-core/test/utils/RequestUtils.spec.ts index b736702ed6..3cc454b3cb 100644 --- a/lib/msal-core/test/utils/RequestUtils.spec.ts +++ b/lib/msal-core/test/utils/RequestUtils.spec.ts @@ -99,7 +99,7 @@ describe("RequestUtils.ts class", () => { expect(splitKey[1]).to.contain("abcd"); const parsedState = RequestUtils.parseLibraryState(state); - expect(CryptoUtils.isGuid(parsedState.state)).to.be.equal(true); + expect(CryptoUtils.isGuid(parsedState.id)).to.be.equal(true); expect(parsedState.ts === now).to.be.equal(true); nowStub.restore(); }); From 3687cb4cd3c16959d789e0ffaa10df125e98c079 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Thu, 26 Mar 2020 10:08:20 -0700 Subject: [PATCH 09/10] Cleanup unneeded import and comment --- lib/msal-core/src/ServerRequestParameters.ts | 1 - lib/msal-core/src/utils/RequestUtils.ts | 1 - 2 files changed, 2 deletions(-) diff --git a/lib/msal-core/src/ServerRequestParameters.ts b/lib/msal-core/src/ServerRequestParameters.ts index 4d724f9310..a4f1dec2e5 100644 --- a/lib/msal-core/src/ServerRequestParameters.ts +++ b/lib/msal-core/src/ServerRequestParameters.ts @@ -10,7 +10,6 @@ import { StringDict } from "./MsalTypes"; import { Account } from "./Account"; import { SSOTypes, Constants, PromptState, libraryVersion } from "./utils/Constants"; import { StringUtils } from "./utils/StringUtils"; -import { RequestUtils } from "./utils/RequestUtils"; /** * Nonce: OIDC Nonce definition: https://openid.net/specs/openid-connect-core-1_0.html#IDToken diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index 158f3194ab..e7f6e1136e 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -140,7 +140,6 @@ export class RequestUtils { * @returns State string include library state and user state */ static validateAndGenerateState(userState: string): string { - // append GUID to user set state or set one for the user if null return !StringUtils.isEmpty(userState) ? `${RequestUtils.generateLibraryState()}${Constants.resourceDelimiter}${userState}` : RequestUtils.generateLibraryState(); } From 1f8f9dd9f7605406f56d7c25cc488044cd26e249 Mon Sep 17 00:00:00 2001 From: Jason Nutter Date: Mon, 30 Mar 2020 11:11:46 -0700 Subject: [PATCH 10/10] Add unit tests to mimick tab suspending for generate state with timestamp --- lib/msal-core/test/utils/RequestUtils.spec.ts | 21 ++++++++++++++++++- 1 file changed, 20 insertions(+), 1 deletion(-) diff --git a/lib/msal-core/test/utils/RequestUtils.spec.ts b/lib/msal-core/test/utils/RequestUtils.spec.ts index 3cc454b3cb..97b8f321fd 100644 --- a/lib/msal-core/test/utils/RequestUtils.spec.ts +++ b/lib/msal-core/test/utils/RequestUtils.spec.ts @@ -100,10 +100,29 @@ describe("RequestUtils.ts class", () => { const parsedState = RequestUtils.parseLibraryState(state); expect(CryptoUtils.isGuid(parsedState.id)).to.be.equal(true); - expect(parsedState.ts === now).to.be.equal(true); + expect(parsedState.ts).to.be.equal(now); nowStub.restore(); }); + it("generates expected state if there is a delay between generating and parsing", function(done) { + this.timeout(5000); + + sinon.restore(); + const now = TimeUtils.now(); + const nowStub = sinon.stub(TimeUtils, "now").returns(now); + + const userState: string = "abcd"; + const state: string = RequestUtils.validateAndGenerateState(userState); + nowStub.restore(); + + // Mimicks tab suspending + setTimeout(() => { + const parsedState = RequestUtils.parseLibraryState(state); + expect(parsedState.ts).to.be.equal(now); + done(); + }, 4000); + }); + it("validate and generate correlationId", () => { const userCorrelationId: string = null; const correlationId: string = RequestUtils.validateAndGenerateCorrelationId(userCorrelationId);