From cac0e8578b20f5267a3d2bbb2bcf34a8aef0f868 Mon Sep 17 00:00:00 2001 From: sameerag Date: Thu, 6 Feb 2020 00:21:18 -0800 Subject: [PATCH 1/6] Update the framename to reflect authority and scopes --- lib/msal-core/src/UserAgentApplication.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index caa22d2d4b..6941b938f8 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -1286,9 +1286,10 @@ export class UserAgentApplication { */ private renewToken(scopes: Array, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { const scope = scopes.join(" ").toLowerCase(); - this.logger.verbose("renewToken is called for scope:" + scope); + this.logger.verbose("renewToken is called for scope: " + scope + " authority: " + serverAuthenticationRequest.authority); + + const frameName = `msalRenewFrame${Constants.resourceDelimiter}${scope}${Constants.resourceDelimiter}${serverAuthenticationRequest.authority}`; - const frameName = `msalRenewFrame${scope}`; const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); @@ -1312,7 +1313,8 @@ export class UserAgentApplication { */ private renewIdToken(scopes: Array, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { this.logger.info("renewidToken is called"); - const frameName = "msalIdTokenFrame"; + + const frameName = `msalIdTokenFrame${Constants.resourceDelimiter}${scopes.join(" ").toLowerCase()}${Constants.resourceDelimiter}${serverAuthenticationRequest.authority}`; const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); From 5cc8e6d8d6a2d915cf07ab71b56cda5487259bb7 Mon Sep 17 00:00:00 2001 From: sameerag Date: Wed, 12 Feb 2020 12:48:56 -0800 Subject: [PATCH 2/6] Addressing feedback --- lib/msal-core/src/UserAgentApplication.ts | 6 +++--- lib/msal-core/src/utils/Constants.ts | 12 ++++++++++-- lib/msal-core/src/utils/WindowUtils.ts | 11 +++++++++++ lib/msal-core/test/utils/WindowUtils.spec.ts | 15 +++++++++++++++ 4 files changed, 39 insertions(+), 5 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 6941b938f8..ca78856454 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -39,6 +39,7 @@ import { Constants, TemporaryCacheKeys, PersistentCacheKeys, ErrorCacheKeys, + FramePrefix } from "./utils/Constants"; // default authority @@ -1288,8 +1289,7 @@ export class UserAgentApplication { const scope = scopes.join(" ").toLowerCase(); this.logger.verbose("renewToken is called for scope: " + scope + " authority: " + serverAuthenticationRequest.authority); - const frameName = `msalRenewFrame${Constants.resourceDelimiter}${scope}${Constants.resourceDelimiter}${serverAuthenticationRequest.authority}`; - + const frameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, scopes, serverAuthenticationRequest.authority); const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); @@ -1314,7 +1314,7 @@ export class UserAgentApplication { private renewIdToken(scopes: Array, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { this.logger.info("renewidToken is called"); - const frameName = `msalIdTokenFrame${Constants.resourceDelimiter}${scopes.join(" ").toLowerCase()}${Constants.resourceDelimiter}${serverAuthenticationRequest.authority}`; + const frameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, scopes, serverAuthenticationRequest.authority); const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); diff --git a/lib/msal-core/src/utils/Constants.ts b/lib/msal-core/src/utils/Constants.ts index eb6e39ca3a..e305331bc2 100644 --- a/lib/msal-core/src/utils/Constants.ts +++ b/lib/msal-core/src/utils/Constants.ts @@ -145,7 +145,15 @@ export const PromptState = { LOGIN: "login", SELECT_ACCOUNT: "select_account", CONSENT: "consent", - NONE: "none", + NONE: "none" +}; + +/** + * Frame name prefixes for the hidden iframe created in silent frames + */ +export const FramePrefix = { + ID_TOKEN_FRAME: "msalIdTokenFrame", + TOKEN_FRAME: "msalRenewFrame" }; /** @@ -153,4 +161,4 @@ export const PromptState = { */ export function libraryVersion(): string { return "1.2.1"; -} +}; diff --git a/lib/msal-core/src/utils/WindowUtils.ts b/lib/msal-core/src/utils/WindowUtils.ts index 9c7d65771a..0a833e4d35 100644 --- a/lib/msal-core/src/utils/WindowUtils.ts +++ b/lib/msal-core/src/utils/WindowUtils.ts @@ -30,6 +30,17 @@ export class WindowUtils { return !!(window.opener && window.opener !== window); } + /** + * @hidden + * @param prefix + * @param scopes + * @param authority + */ + static generateFrameName(prefix: string, scopes: string[], authority: string): string { + const scope = scopes.join(" ").toLowerCase(); + return `${prefix}${Constants.resourceDelimiter}${scopes.join(" ").toLowerCase()}${Constants.resourceDelimiter}${authority}`; + } + /** * @hidden * Monitors a window until it loads a url with a hash diff --git a/lib/msal-core/test/utils/WindowUtils.spec.ts b/lib/msal-core/test/utils/WindowUtils.spec.ts index 43d16eeb98..207da35317 100644 --- a/lib/msal-core/test/utils/WindowUtils.spec.ts +++ b/lib/msal-core/test/utils/WindowUtils.spec.ts @@ -1,6 +1,8 @@ import { expect } from "chai"; import { describe, it } from "mocha"; import { WindowUtils } from "../../src/utils/WindowUtils"; +import { FramePrefix } from "../../src/utils/Constants"; +import { TEST_CONFIG } from "../TestConstants"; import { ClientAuthError } from "../../src/error/ClientAuthError"; describe("WindowUtils", () => { @@ -70,4 +72,17 @@ describe("WindowUtils", () => { }, 500); }); }); + + describe("generateFrameName", () => { + it("test idToken frame name created", () => { + const scopes = ["s1", "s2", "s3"]; + const authority = TEST_CONFIG.validAuthority; + + const idTokenFrameName = WindowUtils.generateFrameName(FramePrefix.ID_TOKEN_FRAME, scopes, authority); + const tokenFrameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, scopes, authority); + + expect(idTokenFrameName).to.equal(`${FramePrefix.ID_TOKEN_FRAME}|s1 s2 s3|${TEST_CONFIG.validAuthority}`); + expect(tokenFrameName).to.equal(`${FramePrefix.TOKEN_FRAME}|s1 s2 s3|${TEST_CONFIG.validAuthority}`); + }); + }); }); From 4598ec564e4955c20e3da574a0bf4efd4d8f7c31 Mon Sep 17 00:00:00 2001 From: sameerag Date: Wed, 12 Feb 2020 12:53:50 -0800 Subject: [PATCH 3/6] Removing a typo --- lib/msal-core/src/utils/WindowUtils.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/msal-core/src/utils/WindowUtils.ts b/lib/msal-core/src/utils/WindowUtils.ts index 0a833e4d35..6f9513f645 100644 --- a/lib/msal-core/src/utils/WindowUtils.ts +++ b/lib/msal-core/src/utils/WindowUtils.ts @@ -37,7 +37,6 @@ export class WindowUtils { * @param authority */ static generateFrameName(prefix: string, scopes: string[], authority: string): string { - const scope = scopes.join(" ").toLowerCase(); return `${prefix}${Constants.resourceDelimiter}${scopes.join(" ").toLowerCase()}${Constants.resourceDelimiter}${authority}`; } From d29c350a7a91bb75a7aba5ba833edb01377900e2 Mon Sep 17 00:00:00 2001 From: sameerag Date: Wed, 12 Feb 2020 12:55:03 -0800 Subject: [PATCH 4/6] idToken update --- 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 ca78856454..fbbf111df1 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -1314,7 +1314,7 @@ export class UserAgentApplication { private renewIdToken(scopes: Array, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { this.logger.info("renewidToken is called"); - const frameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, scopes, serverAuthenticationRequest.authority); + const frameName = WindowUtils.generateFrameName(FramePrefix.ID_TOKEN_FRAME, scopes, serverAuthenticationRequest.authority); const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); From dbc1edaff3d691669030232195bd65b0e3533538 Mon Sep 17 00:00:00 2001 From: sameerag Date: Tue, 21 Apr 2020 00:02:47 -0700 Subject: [PATCH 5/6] Pushing changes for request signature --- lib/msal-core/src/UserAgentApplication.ts | 44 ++++++++++---------- lib/msal-core/src/utils/RequestUtils.ts | 8 ++++ lib/msal-core/src/utils/WindowUtils.ts | 4 +- lib/msal-core/test/utils/WindowUtils.spec.ts | 5 ++- 4 files changed, 35 insertions(+), 26 deletions(-) diff --git a/lib/msal-core/src/UserAgentApplication.ts b/lib/msal-core/src/UserAgentApplication.ts index 421639aa6b..c5edb1843d 100644 --- a/lib/msal-core/src/UserAgentApplication.ts +++ b/lib/msal-core/src/UserAgentApplication.ts @@ -633,6 +633,7 @@ export class UserAgentApplication { // validate the request const request = RequestUtils.validateRequest(userRequest, false, this.clientId, Constants.interactionTypeSilent); const apiEvent: ApiEvent = this.telemetryManager.createAndStartApiEvent(request.correlationId, API_EVENT_IDENTIFIER.AcquireTokenSilent, this.logger); + const requestSignature = RequestUtils.createRequestSignature(request); return new Promise((resolve, reject) => { @@ -728,10 +729,10 @@ export class UserAgentApplication { * 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"); + if (window.activeRenewals[requestSignature]) { + this.logger.verbose("Renew token for scope and authority: " + requestSignature + " is in progress. Registering callback"); // Active renewals contains the state for each renewal. - this.registerCallback(window.activeRenewals[scope], scope, resolve, reject); + this.registerCallback(window.activeRenewals[requestSignature], requestSignature, resolve, reject); } else { if (request.scopes && request.scopes.indexOf(this.clientId) > -1 && request.scopes.length === 1) { @@ -741,11 +742,11 @@ export class UserAgentApplication { */ this.logger.verbose("renewing idToken"); this.silentLogin = true; - this.renewIdToken(request.scopes, resolve, reject, account, serverAuthenticationRequest); + this.renewIdToken(requestSignature, resolve, reject, account, serverAuthenticationRequest); } else { // renew access token this.logger.verbose("renewing accesstoken"); - this.renewToken(request.scopes, resolve, reject, account, serverAuthenticationRequest); + this.renewToken(requestSignature, resolve, reject, account, serverAuthenticationRequest); } } }).catch((err) => { @@ -826,10 +827,10 @@ export class UserAgentApplication { * registered when network errors occur and subsequent token requests for same resource are registered to the pending request. * @ignore */ - private async loadIframeTimeout(urlNavigate: string, frameName: string, scope: string): Promise { + private async loadIframeTimeout(urlNavigate: string, frameName: string, requestSignature: string): Promise { // set iframe session to pending - const expectedState = window.activeRenewals[scope]; - this.logger.verbose("Set loading state to pending for: " + scope + ":" + expectedState); + const expectedState = window.activeRenewals[requestSignature]; + this.logger.verbose("Set loading state to pending for: " + requestSignature + ":" + expectedState); this.cacheStorage.setItem(`${TemporaryCacheKeys.RENEW_STATUS}${Constants.resourceDelimiter}${expectedState}`, Constants.inProgress); // render the iframe synchronously if app chooses no timeout, else wait for the set timer to expire @@ -846,7 +847,7 @@ export class UserAgentApplication { } catch (error) { if (this.cacheStorage.getItem(`${TemporaryCacheKeys.RENEW_STATUS}${Constants.resourceDelimiter}${expectedState}`) === Constants.inProgress) { // fail the iframe session if it's in pending state - this.logger.verbose("Loading frame has timed out after: " + (this.config.system.loadFrameTimeout / 1000) + " seconds for scope " + scope + ":" + expectedState); + this.logger.verbose("Loading frame has timed out after: " + (this.config.system.loadFrameTimeout / 1000) + " seconds for scope/authority " + requestSignature + ":" + expectedState); // Error after timeout if (expectedState && window.callbackMappedToRenewStates[expectedState]) { window.callbackMappedToRenewStates[expectedState](null, error); @@ -892,9 +893,9 @@ export class UserAgentApplication { * @param {Function} reject - The reject function of the promise object. * @ignore */ - private registerCallback(expectedState: string, scope: string, resolve: Function, reject: Function): void { + private registerCallback(expectedState: string, requestSignature: string, resolve: Function, reject: Function): void { // track active renewals - window.activeRenewals[scope] = expectedState; + window.activeRenewals[requestSignature] = expectedState; // initialize callbacks mapped array if (!window.promiseMappedToRenewStates[expectedState]) { @@ -907,7 +908,7 @@ export class UserAgentApplication { if (!window.callbackMappedToRenewStates[expectedState]) { window.callbackMappedToRenewStates[expectedState] = (response: AuthResponse, error: AuthError) => { // reset active renewals - window.activeRenewals[scope] = null; + window.activeRenewals[requestSignature] = null; // for all promiseMappedtoRenewStates for a given 'state' - call the reject/resolve with error/token respectively for (let i = 0; i < window.promiseMappedToRenewStates[expectedState].length; ++i) { @@ -1338,11 +1339,10 @@ export class UserAgentApplication { * Acquires access token using a hidden iframe. * @ignore */ - private renewToken(scopes: Array, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { - const scope = scopes.join(" ").toLowerCase(); - this.logger.verbose("renewToken is called for scope: " + scope + " authority: " + serverAuthenticationRequest.authority); + private renewToken(requestSignature: string, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { + this.logger.verbose("renewToken is called for scope and authority: " + requestSignature); - const frameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, scopes, serverAuthenticationRequest.authority); + const frameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, requestSignature); const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); @@ -1353,10 +1353,10 @@ export class UserAgentApplication { window.renewStates.push(serverAuthenticationRequest.state); window.requestType = Constants.renewToken; - this.registerCallback(serverAuthenticationRequest.state, scope, resolve, reject); + this.registerCallback(serverAuthenticationRequest.state, requestSignature, resolve, reject); this.logger.infoPii("Navigate to:" + urlNavigate); frameHandle.src = "about:blank"; - this.loadIframeTimeout(urlNavigate, frameName, scope).catch(error => reject(error)); + this.loadIframeTimeout(urlNavigate, frameName, requestSignature).catch(error => reject(error)); } /** @@ -1364,10 +1364,10 @@ export class UserAgentApplication { * Renews idtoken for app's own backend when clientId is passed as a single scope in the scopes array. * @ignore */ - private renewIdToken(scopes: Array, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { + private renewIdToken(requestSignature: string, resolve: Function, reject: Function, account: Account, serverAuthenticationRequest: ServerRequestParameters): void { this.logger.info("renewidToken is called"); - const frameName = WindowUtils.generateFrameName(FramePrefix.ID_TOKEN_FRAME, scopes, serverAuthenticationRequest.authority); + const frameName = WindowUtils.generateFrameName(FramePrefix.ID_TOKEN_FRAME, requestSignature); const frameHandle = WindowUtils.addHiddenIFrame(frameName, this.logger); this.updateCacheEntries(serverAuthenticationRequest, account); @@ -1386,10 +1386,10 @@ export class UserAgentApplication { } // note: scope here is clientId - this.registerCallback(serverAuthenticationRequest.state, this.clientId, resolve, reject); + this.registerCallback(serverAuthenticationRequest.state, requestSignature, resolve, reject); this.logger.infoPii("Navigate to:" + urlNavigate); frameHandle.src = "about:blank"; - this.loadIframeTimeout(urlNavigate, frameName, this.clientId).catch(error => reject(error)); + this.loadIframeTimeout(urlNavigate, frameName, requestSignature).catch(error => reject(error)); } /** diff --git a/lib/msal-core/src/utils/RequestUtils.ts b/lib/msal-core/src/utils/RequestUtils.ts index f802d763bd..945f2207cf 100644 --- a/lib/msal-core/src/utils/RequestUtils.ts +++ b/lib/msal-core/src/utils/RequestUtils.ts @@ -203,4 +203,12 @@ export class RequestUtils { } return CryptoUtils.isGuid(correlationId)? correlationId : CryptoUtils.createNewGuid(); } + + /** + * Create a request signature + * @param request + */ + static createRequestSignature(request: AuthenticationParameters): string { + return `${request.scopes.join(" ").toLowerCase()}${Constants.resourceDelimiter}${request.authority}`; + } } diff --git a/lib/msal-core/src/utils/WindowUtils.ts b/lib/msal-core/src/utils/WindowUtils.ts index e40a46738e..0a93dc1092 100644 --- a/lib/msal-core/src/utils/WindowUtils.ts +++ b/lib/msal-core/src/utils/WindowUtils.ts @@ -36,8 +36,8 @@ export class WindowUtils { * @param scopes * @param authority */ - static generateFrameName(prefix: string, scopes: string[], authority: string): string { - return `${prefix}${Constants.resourceDelimiter}${scopes.join(" ").toLowerCase()}${Constants.resourceDelimiter}${authority}`; + static generateFrameName(prefix: string, requestSignature: string): string { + return `${prefix}${Constants.resourceDelimiter}${requestSignature}`; } /** diff --git a/lib/msal-core/test/utils/WindowUtils.spec.ts b/lib/msal-core/test/utils/WindowUtils.spec.ts index f6eed2e7d4..c51371cfc7 100644 --- a/lib/msal-core/test/utils/WindowUtils.spec.ts +++ b/lib/msal-core/test/utils/WindowUtils.spec.ts @@ -92,9 +92,10 @@ describe("WindowUtils", () => { it("test idToken frame name created", () => { const scopes = ["s1", "s2", "s3"]; const authority = TEST_CONFIG.validAuthority; + const requestSignature = `${scopes.join(" ").toLowerCase()}|${authority}`; - const idTokenFrameName = WindowUtils.generateFrameName(FramePrefix.ID_TOKEN_FRAME, scopes, authority); - const tokenFrameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, scopes, authority); + const idTokenFrameName = WindowUtils.generateFrameName(FramePrefix.ID_TOKEN_FRAME, requestSignature); + const tokenFrameName = WindowUtils.generateFrameName(FramePrefix.TOKEN_FRAME, requestSignature); expect(idTokenFrameName).to.equal(`${FramePrefix.ID_TOKEN_FRAME}|s1 s2 s3|${TEST_CONFIG.validAuthority}`); expect(tokenFrameName).to.equal(`${FramePrefix.TOKEN_FRAME}|s1 s2 s3|${TEST_CONFIG.validAuthority}`); From 7b0ab80e965ffa6e2cc12635769b35c3b811e7ee Mon Sep 17 00:00:00 2001 From: sameerag Date: Fri, 24 Apr 2020 01:17:41 -0700 Subject: [PATCH 6/6] Add more tests --- lib/msal-core/test/utils/RequestUtils.spec.ts | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/lib/msal-core/test/utils/RequestUtils.spec.ts b/lib/msal-core/test/utils/RequestUtils.spec.ts index 41aa33a221..be18f4fc1e 100644 --- a/lib/msal-core/test/utils/RequestUtils.spec.ts +++ b/lib/msal-core/test/utils/RequestUtils.spec.ts @@ -164,4 +164,11 @@ describe("RequestUtils.ts class", () => { expect(CryptoUtils.isGuid(request.correlationId)).to.be.equal(true); }); + it("generate request signature", () => { + const userRequest: AuthenticationParameters = { scopes: ["s1", "s2", "s3"], authority: TEST_CONFIG.validAuthority}; + const requestSignature = RequestUtils.createRequestSignature(userRequest); + + expect(requestSignature).to.be.equal("s1 s2 s3|https://login.microsoftonline.com/common"); + }); + });