From 06f7350dba8aa1ac854641ecc3c994be09cb9045 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 2 Jun 2025 12:51:38 -0700 Subject: [PATCH 01/12] Factor getRedirectUri out into a standalone function --- .../BaseInteractionClient.ts | 32 +++++++++---------- .../PlatformAuthInteractionClient.ts | 6 ++-- .../interaction_client/SilentRefreshClient.ts | 7 ++-- .../StandardInteractionClient.ts | 4 +-- 4 files changed, 26 insertions(+), 23 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index 8fb12442f7..582a6dd7c3 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -131,22 +131,6 @@ export abstract class BaseInteractionClient { } } - /** - * - * Use to get the redirect uri configured in MSAL or null. - * @param requestRedirectUri - * @returns Redirect URL - * - */ - getRedirectUri(requestRedirectUri?: string): string { - this.logger.verbose("getRedirectUri called"); - const redirectUri = requestRedirectUri || this.config.auth.redirectUri; - return UrlString.getAbsoluteUrl( - redirectUri, - BrowserUtils.getCurrentUri() - ); - } - /** * * @param apiId @@ -249,3 +233,19 @@ export abstract class BaseInteractionClient { return discoveredAuthority; } } + +/** + * + * Use to get the redirect uri configured in MSAL or null. + * @param requestRedirectUri + * @returns Redirect URL + * + */ +export function getRedirectUri(requestRedirectUri?: string, clientConfig?: BrowserConfiguration, logger?: Logger): string { + logger?.verbose("getRedirectUri called"); + const redirectUri = requestRedirectUri || clientConfig?.auth.redirectUri || ""; + return UrlString.getAbsoluteUrl( + redirectUri, + BrowserUtils.getCurrentUri() + ); +} diff --git a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts index 31a12ccc07..9352486af1 100644 --- a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts @@ -34,7 +34,7 @@ import { AccountEntityUtils, Constants, } from "@azure/msal-common/browser"; -import { BaseInteractionClient } from "./BaseInteractionClient.js"; +import { BaseInteractionClient, getRedirectUri } from "./BaseInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -337,7 +337,7 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { }; const redirectUri = this.config.auth.navigateToLoginRequestUrl ? window.location.href - : this.getRedirectUri(request.redirectUri); + : getRedirectUri(request.redirectUri, this.config, this.logger); rootMeasurement.end({ success: true }); await this.navigationClient.navigateExternal( redirectUri, @@ -881,7 +881,7 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { clientId: this.config.auth.clientId, authority: canonicalAuthority.urlString, scope: scopeSet.printScopes(), - redirectUri: this.getRedirectUri(request.redirectUri), + redirectUri: getRedirectUri(request.redirectUri, this.config, this.logger), prompt: this.getPrompt(request.prompt), correlationId: this.correlationId, tokenType: request.authenticationScheme, diff --git a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts index ee5f00dbc2..c7ba15db21 100644 --- a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts @@ -22,6 +22,7 @@ import { } from "../error/BrowserAuthError.js"; import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { initializeBaseRequest } from "../request/RequestHelpers.js"; +import { getRedirectUri } from "./BaseInteractionClient.js"; export class SilentRefreshClient extends StandardInteractionClient { /** @@ -45,8 +46,10 @@ export class SilentRefreshClient extends StandardInteractionClient { if (request.redirectUri) { // Make sure any passed redirectUri is converted to an absolute URL - redirectUri is not a required parameter for refresh token redemption so only include if explicitly provided - silentRequest.redirectUri = this.getRedirectUri( - request.redirectUri + silentRequest.redirectUri = getRedirectUri( + request.redirectUri, + this.config, + this.logger ); } diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 0f8493dced..023ea878cd 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -19,7 +19,7 @@ import { StringDict, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { BaseInteractionClient } from "./BaseInteractionClient.js"; +import { BaseInteractionClient, getRedirectUri } from "./BaseInteractionClient.js"; import { BrowserConstants, InteractionType, @@ -277,7 +277,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { request: RedirectRequest | PopupRequest | SsoSilentRequest, interactionType: InteractionType ): Promise { - const redirectUri = this.getRedirectUri(request.redirectUri); + const redirectUri = getRedirectUri(request.redirectUri, this.config, this.logger); const browserState: BrowserStateObject = { interactionType: interactionType, }; From a2f4b3a7ebb8b0a1548e21c43aa29f5628769996 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 2 Jun 2025 12:58:36 -0700 Subject: [PATCH 02/12] Factor out initializeServerTelemetryManager --- .../BaseInteractionClient.ts | 56 ++++++++++--------- .../PlatformAuthInteractionClient.ts | 14 +++-- .../src/interaction_client/PopupClient.ts | 17 ++++-- .../src/interaction_client/RedirectClient.ts | 25 +++++++-- .../SilentAuthCodeClient.ts | 9 ++- .../interaction_client/SilentCacheClient.ts | 9 ++- .../interaction_client/SilentIframeClient.ts | 9 ++- .../interaction_client/SilentRefreshClient.ts | 10 +++- 8 files changed, 99 insertions(+), 50 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index 582a6dd7c3..a42ca1ce01 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -131,32 +131,6 @@ export abstract class BaseInteractionClient { } } - /** - * - * @param apiId - * @param correlationId - * @param forceRefresh - */ - protected initializeServerTelemetryManager( - apiId: number, - forceRefresh?: boolean - ): ServerTelemetryManager { - this.logger.verbose("initializeServerTelemetryManager called"); - const telemetryPayload: ServerTelemetryRequest = { - clientId: this.config.auth.clientId, - correlationId: this.correlationId, - apiId: apiId, - forceRefresh: forceRefresh || false, - wrapperSKU: this.browserStorage.getWrapperMetadata()[0], - wrapperVer: this.browserStorage.getWrapperMetadata()[1], - }; - - return new ServerTelemetryManager( - telemetryPayload, - this.browserStorage - ); - } - /** * Used to get a discovered version of the default authority. * @param params { @@ -249,3 +223,33 @@ export function getRedirectUri(requestRedirectUri?: string, clientConfig?: Brows BrowserUtils.getCurrentUri() ); } + + /** + * + * @param apiId + * @param correlationId + * @param forceRefresh + */ + export function initializeServerTelemetryManager( + apiId: number, + config: BrowserConfiguration, + correlationId: string, + browserStorage: BrowserCacheManager, + logger?: Logger, + forceRefresh?: boolean + ): ServerTelemetryManager { + logger?.verbose("initializeServerTelemetryManager called"); + const telemetryPayload: ServerTelemetryRequest = { + clientId: config.auth.clientId, + correlationId: correlationId, + apiId: apiId, + forceRefresh: forceRefresh || false, + wrapperSKU: browserStorage.getWrapperMetadata()[0], + wrapperVer: browserStorage.getWrapperMetadata()[1], + }; + + return new ServerTelemetryManager( + telemetryPayload, + browserStorage + ); + } diff --git a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts index 9352486af1..635c50bbd5 100644 --- a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts @@ -34,7 +34,7 @@ import { AccountEntityUtils, Constants, } from "@azure/msal-common/browser"; -import { BaseInteractionClient, getRedirectUri } from "./BaseInteractionClient.js"; +import { BaseInteractionClient, getRedirectUri, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -160,8 +160,12 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { ); const reqTimestamp = TimeUtils.nowSeconds(); - const serverTelemetryManager = this.initializeServerTelemetryManager( - this.apiId + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId , + this.browserStorage, + this.logger ); try { @@ -317,7 +321,7 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { // Only throw fatal errors here to allow application to fallback to regular redirect. Otherwise proceed and the error will be thrown in handleRedirectPromise if (e instanceof NativeAuthError) { const serverTelemetryManager = - this.initializeServerTelemetryManager(this.apiId); + initializeServerTelemetryManager(this.apiId, this.config, this.correlationId, this.browserStorage, this.logger); serverTelemetryManager.setNativeBrokerErrorCode(e.errorCode); if (isFatalNativeAuthError(e)) { throw e; @@ -407,7 +411,7 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { ); const serverTelemetryManager = - this.initializeServerTelemetryManager(this.apiId); + initializeServerTelemetryManager(this.apiId, this.config, this.correlationId, this.browserStorage, this.logger); serverTelemetryManager.clearNativeBrokerErrorCode(); return authResult; } catch (e) { diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index a15b7da4ad..b904765a8b 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -47,6 +47,7 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; +import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export type PopupParams = { popup?: Window | null; @@ -250,8 +251,12 @@ export class PopupClient extends StandardInteractionClient { pkceCodes?: PkceCodes ): Promise { const correlationId = request.correlationId; - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.acquireTokenPopup + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.acquireTokenPopup, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); const pkce = @@ -473,8 +478,12 @@ export class PopupClient extends StandardInteractionClient { validRequest ); - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.logoutPopup + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.logoutPopup, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); try { diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 50774c2255..ba48a0d1e5 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -48,6 +48,7 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; +import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; function getNavigationType(): NavigationTimingType | undefined { if ( @@ -166,8 +167,12 @@ export class RedirectClient extends StandardInteractionClient { request: CommonAuthorizationUrlRequest ): Promise { const correlationId = request.correlationId; - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.acquireTokenRedirect + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.acquireTokenRedirect, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); const pkceCodes = await invokeAsync( @@ -289,8 +294,12 @@ export class RedirectClient extends StandardInteractionClient { pkceVerifier: string, parentMeasurement: InProgressPerformanceEvent ): Promise { - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.handleRedirectPromise + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.handleRedirectPromise, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); try { @@ -652,8 +661,12 @@ export class RedirectClient extends StandardInteractionClient { async logout(logoutRequest?: EndSessionRequest): Promise { this.logger.verbose("logoutRedirect called"); const validLogoutRequest = this.initializeLogoutRequest(logoutRequest); - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.logout + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.logout, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); try { diff --git a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts index 53da018ba4..4dded7efbd 100644 --- a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts @@ -28,6 +28,7 @@ import { HybridSpaAuthorizationCodeClient } from "./HybridSpaAuthorizationCodeCl import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { InteractionHandler } from "../interaction_handler/InteractionHandler.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; +import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export class SilentAuthCodeClient extends StandardInteractionClient { private apiId: ApiId; @@ -81,8 +82,12 @@ export class SilentAuthCodeClient extends StandardInteractionClient { request.correlationId )(request, InteractionType.Silent); - const serverTelemetryManager = this.initializeServerTelemetryManager( - this.apiId + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); try { diff --git a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts index dee5809724..20e0493fab 100644 --- a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts @@ -17,6 +17,7 @@ import { } from "../error/BrowserAuthError.js"; import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { ClearCacheRequest } from "../request/ClearCacheRequest.js"; +import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export class SilentCacheClient extends StandardInteractionClient { /** @@ -27,8 +28,12 @@ export class SilentCacheClient extends StandardInteractionClient { silentRequest: CommonSilentFlowRequest ): Promise { // Telemetry manager only used to increment cacheHits here - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.acquireTokenSilent_silentFlow + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.acquireTokenSilent_silentFlow, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); const clientConfig = await invokeAsync( diff --git a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts index 17d2ca166f..b1371cd42b 100644 --- a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts @@ -44,6 +44,7 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; +import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export class SilentIframeClient extends StandardInteractionClient { protected apiId: ApiId; @@ -143,8 +144,12 @@ export class SilentIframeClient extends StandardInteractionClient { request: CommonAuthorizationUrlRequest ): Promise { let authClient: AuthorizationCodeClient | undefined; - const serverTelemetryManager = this.initializeServerTelemetryManager( - this.apiId + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); try { diff --git a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts index c7ba15db21..29ba0cc217 100644 --- a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts @@ -22,7 +22,7 @@ import { } from "../error/BrowserAuthError.js"; import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { initializeBaseRequest } from "../request/RequestHelpers.js"; -import { getRedirectUri } from "./BaseInteractionClient.js"; +import { getRedirectUri, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export class SilentRefreshClient extends StandardInteractionClient { /** @@ -53,8 +53,12 @@ export class SilentRefreshClient extends StandardInteractionClient { ); } - const serverTelemetryManager = this.initializeServerTelemetryManager( - ApiId.acquireTokenSilent_silentFlow + const serverTelemetryManager = initializeServerTelemetryManager( + ApiId.acquireTokenSilent_silentFlow, + this.config, + this.correlationId, + this.browserStorage, + this.logger ); const refreshTokenClient = await this.createRefreshTokenClient({ From 9dc113098a3574ed7e80b0d1c2aa9b714bcefe47 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 2 Jun 2025 13:21:01 -0700 Subject: [PATCH 03/12] Factor out getDiscoveredAuthority from BaseInteractionClient --- .../BaseInteractionClient.ts | 211 +++++++++--------- .../PlatformAuthInteractionClient.ts | 10 +- .../src/interaction_client/PopupClient.ts | 6 +- .../src/interaction_client/RedirectClient.ts | 10 +- .../interaction_client/SilentIframeClient.ts | 6 +- .../StandardInteractionClient.ts | 6 +- .../BaseInteractionClient.spec.ts | 32 ++- .../StandardInteractionClient.spec.ts | 3 +- 8 files changed, 144 insertions(+), 140 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index a42ca1ce01..330db1585a 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -130,82 +130,6 @@ export abstract class BaseInteractionClient { } } } - - /** - * Used to get a discovered version of the default authority. - * @param params { - * requestAuthority?: string; - * requestAzureCloudOptions?: AzureCloudOptions; - * requestExtraQueryParameters?: StringDict; - * account?: AccountInfo; - * } - */ - protected async getDiscoveredAuthority(params: { - requestAuthority?: string; - requestAzureCloudOptions?: AzureCloudOptions; - requestExtraQueryParameters?: StringDict; - account?: AccountInfo; - }): Promise { - const { account } = params; - const instanceAwareEQ = - params.requestExtraQueryParameters && - params.requestExtraQueryParameters.hasOwnProperty("instance_aware") - ? params.requestExtraQueryParameters["instance_aware"] - : undefined; - - const authorityOptions: AuthorityOptions = { - protocolMode: this.config.system.protocolMode, - OIDCOptions: this.config.auth.OIDCOptions, - knownAuthorities: this.config.auth.knownAuthorities, - cloudDiscoveryMetadata: this.config.auth.cloudDiscoveryMetadata, - authorityMetadata: this.config.auth.authorityMetadata, - }; - - // build authority string based on auth params, precedence - azureCloudInstance + tenant >> authority - const resolvedAuthority = - params.requestAuthority || this.config.auth.authority; - const resolvedInstanceAware = instanceAwareEQ?.length - ? instanceAwareEQ === "true" - : this.config.auth.instanceAware; - - const userAuthority = - account && resolvedInstanceAware - ? this.config.auth.authority.replace( - UrlString.getDomainFromUrl(resolvedAuthority), - account.environment - ) - : resolvedAuthority; - - // fall back to the authority from config - const builtAuthority = Authority.generateAuthority( - userAuthority, - params.requestAzureCloudOptions || - this.config.auth.azureCloudOptions - ); - const discoveredAuthority = await invokeAsync( - AuthorityFactory.createDiscoveredInstance, - PerformanceEvents.AuthorityFactoryCreateDiscoveredInstance, - this.logger, - this.performanceClient, - this.correlationId - )( - builtAuthority, - this.config.system.networkClient, - this.browserStorage, - authorityOptions, - this.logger, - this.correlationId, - this.performanceClient - ); - - if (account && !discoveredAuthority.isAlias(account.environment)) { - throw createClientConfigurationError( - ClientConfigurationErrorCodes.authorityMismatch - ); - } - - return discoveredAuthority; - } } /** @@ -224,32 +148,113 @@ export function getRedirectUri(requestRedirectUri?: string, clientConfig?: Brows ); } - /** - * - * @param apiId - * @param correlationId - * @param forceRefresh - */ - export function initializeServerTelemetryManager( - apiId: number, - config: BrowserConfiguration, - correlationId: string, - browserStorage: BrowserCacheManager, - logger?: Logger, - forceRefresh?: boolean - ): ServerTelemetryManager { - logger?.verbose("initializeServerTelemetryManager called"); - const telemetryPayload: ServerTelemetryRequest = { - clientId: config.auth.clientId, - correlationId: correlationId, - apiId: apiId, - forceRefresh: forceRefresh || false, - wrapperSKU: browserStorage.getWrapperMetadata()[0], - wrapperVer: browserStorage.getWrapperMetadata()[1], - }; - - return new ServerTelemetryManager( - telemetryPayload, - browserStorage +/** + * + * @param apiId + * @param correlationId + * @param forceRefresh + */ +export function initializeServerTelemetryManager( + apiId: number, + config: BrowserConfiguration, + correlationId: string, + browserStorage: BrowserCacheManager, + logger?: Logger, + forceRefresh?: boolean +): ServerTelemetryManager { + logger?.verbose("initializeServerTelemetryManager called"); + const telemetryPayload: ServerTelemetryRequest = { + clientId: config.auth.clientId, + correlationId: correlationId, + apiId: apiId, + forceRefresh: forceRefresh || false, + wrapperSKU: browserStorage.getWrapperMetadata()[0], + wrapperVer: browserStorage.getWrapperMetadata()[1], + }; + + return new ServerTelemetryManager( + telemetryPayload, + browserStorage + ); +} + +/** + * Used to get a discovered version of the default authority. + * @param params { + * requestAuthority?: string; + * requestAzureCloudOptions?: AzureCloudOptions; + * requestExtraQueryParameters?: StringDict; + * account?: AccountInfo; + * } + */ +export async function getDiscoveredAuthority(params: { + requestAuthority?: string; + requestAzureCloudOptions?: AzureCloudOptions; + requestExtraQueryParameters?: StringDict; + account?: AccountInfo; + }, + config: BrowserConfiguration, + correlationId: string, + performanceClient: IPerformanceClient, + browserStorage: BrowserCacheManager, + logger: Logger): Promise { + const { account } = params; + const instanceAwareEQ = + params.requestExtraQueryParameters && + params.requestExtraQueryParameters.hasOwnProperty("instance_aware") + ? params.requestExtraQueryParameters["instance_aware"] + : undefined; + + const authorityOptions: AuthorityOptions = { + protocolMode: config.system.protocolMode, + OIDCOptions: config.auth.OIDCOptions, + knownAuthorities: config.auth.knownAuthorities, + cloudDiscoveryMetadata: config.auth.cloudDiscoveryMetadata, + authorityMetadata: config.auth.authorityMetadata, + }; + + // build authority string based on auth params, precedence - azureCloudInstance + tenant >> authority + const resolvedAuthority = + params.requestAuthority || config.auth.authority; + const resolvedInstanceAware = instanceAwareEQ?.length + ? instanceAwareEQ === "true" + : config.auth.instanceAware; + + const userAuthority = + account && resolvedInstanceAware + ? config.auth.authority.replace( + UrlString.getDomainFromUrl(resolvedAuthority), + account.environment + ) + : resolvedAuthority; + + // fall back to the authority from config + const builtAuthority = Authority.generateAuthority( + userAuthority, + params.requestAzureCloudOptions || + config.auth.azureCloudOptions + ); + const discoveredAuthority = await invokeAsync( + AuthorityFactory.createDiscoveredInstance, + PerformanceEvents.AuthorityFactoryCreateDiscoveredInstance, + logger, + performanceClient, + correlationId + )( + builtAuthority, + config.system.networkClient, + browserStorage, + authorityOptions, + logger, + correlationId, + performanceClient + ); + + if (account && !discoveredAuthority.isAlias(account.environment)) { + throw createClientConfigurationError( + ClientConfigurationErrorCodes.authorityMismatch ); } + + return discoveredAuthority; +} diff --git a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts index 635c50bbd5..f3b325d538 100644 --- a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts @@ -34,7 +34,7 @@ import { AccountEntityUtils, Constants, } from "@azure/msal-common/browser"; -import { BaseInteractionClient, getRedirectUri, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { BaseInteractionClient, getDiscoveredAuthority, getRedirectUri, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -476,9 +476,9 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { } // Get the preferred_cache domain for the given authority - const authority = await this.getDiscoveredAuthority({ + const authority = await getDiscoveredAuthority({ requestAuthority: request.authority, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); const baseAccount = buildAccountToCache( this.browserStorage, @@ -960,11 +960,11 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { if (request.account) { // validate authority - await this.getDiscoveredAuthority({ + await getDiscoveredAuthority({ requestAuthority, requestAzureCloudOptions: request.azureCloudOptions, account: request.account, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); } const canonicalAuthority = new UrlString(requestAuthority); diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index b904765a8b..31520c21ed 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -47,7 +47,7 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; -import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { getDiscoveredAuthority, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export type PopupParams = { popup?: Window | null; @@ -378,7 +378,7 @@ export class PopupClient extends StandardInteractionClient { const correlationId = request.correlationId; // Get the frame handle for the silent request const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, PerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, @@ -388,7 +388,7 @@ export class PopupClient extends StandardInteractionClient { requestAzureCloudOptions: request.azureCloudOptions, requestExtraQueryParameters: request.extraQueryParameters, account: request.account, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); const earJwk = await invokeAsync( generateEarKey, diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index ba48a0d1e5..6ce203e70d 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -48,7 +48,7 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; -import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { getDiscoveredAuthority, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; function getNavigationType(): NavigationTimingType | undefined { if ( @@ -245,7 +245,7 @@ export class RedirectClient extends StandardInteractionClient { const correlationId = request.correlationId; // Get the frame handle for the silent request const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, PerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, @@ -255,7 +255,7 @@ export class RedirectClient extends StandardInteractionClient { requestAzureCloudOptions: request.azureCloudOptions, requestExtraQueryParameters: request.extraQueryParameters, account: request.account, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); const earJwk = await invokeAsync( generateEarKey, @@ -527,7 +527,7 @@ export class RedirectClient extends StandardInteractionClient { if (serverParams.ear_jwe) { const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, PerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, @@ -537,7 +537,7 @@ export class RedirectClient extends StandardInteractionClient { requestAzureCloudOptions: request.azureCloudOptions, requestExtraQueryParameters: request.extraQueryParameters, account: request.account, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); return invokeAsync( Authorize.handleResponseEAR, PerformanceEvents.HandleResponseEar, diff --git a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts index b1371cd42b..9d739b5373 100644 --- a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts @@ -44,7 +44,7 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; -import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { getDiscoveredAuthority, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export class SilentIframeClient extends StandardInteractionClient { protected apiId: ApiId; @@ -215,7 +215,7 @@ export class SilentIframeClient extends StandardInteractionClient { ): Promise { const correlationId = request.correlationId; const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, PerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, @@ -225,7 +225,7 @@ export class SilentIframeClient extends StandardInteractionClient { requestAzureCloudOptions: request.azureCloudOptions, requestExtraQueryParameters: request.extraQueryParameters, account: request.account, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); const earJwk = await invokeAsync( generateEarKey, diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 023ea878cd..3ea3ea68b9 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -19,7 +19,7 @@ import { StringDict, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { BaseInteractionClient, getRedirectUri } from "./BaseInteractionClient.js"; +import { BaseInteractionClient, getDiscoveredAuthority, getRedirectUri } from "./BaseInteractionClient.js"; import { BrowserConstants, InteractionType, @@ -223,7 +223,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { } = params; const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, PerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, @@ -233,7 +233,7 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { requestAzureCloudOptions, requestExtraQueryParameters, account, - }); + }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); const logger = this.config.system.loggerOptions; return { diff --git a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts index 7fb86d53e0..7b193d1710 100644 --- a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts @@ -22,7 +22,7 @@ import { ID_TOKEN_CLAIMS, ID_TOKEN_ALT_CLAIMS, } from "../utils/StringConstants.js"; -import { BaseInteractionClient } from "../../src/interaction_client/BaseInteractionClient.js"; +import { BaseInteractionClient, getDiscoveredAuthority } from "../../src/interaction_client/BaseInteractionClient.js"; import { EndSessionRequest, PublicClientApplication, @@ -244,13 +244,13 @@ describe("BaseInteractionClient", () => { username: "AbeLi@microsoft.com", }; - await testClient - // @ts-ignore - .getDiscoveredAuthority({ + let clientInst = testClient as any; + + await getDiscoveredAuthority({ requestAuthority: "https://login.microsoftonline.com/common", account: testAccount, - }) + }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) .then(() => { throw "This is unexpected. This call should have failed."; }) @@ -272,13 +272,13 @@ describe("BaseInteractionClient", () => { username: "AbeLi@microsoft.com", }; - testClient - // @ts-ignore - .getDiscoveredAuthority({ + const clientInst = testClient as any; + + getDiscoveredAuthority({ requestAuthority: "https://login.microsoftonline.com/common", account: testAccount, - }) + }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) .then(() => { done(); }) @@ -316,11 +316,10 @@ describe("BaseInteractionClient", () => { pca.performanceClient ); - interactionClient - // @ts-ignore - .getDiscoveredAuthority({ + const clientInst = interactionClient as any; + getDiscoveredAuthority({ account: testAccount, - }) + }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) .then(() => { done(); }) @@ -358,13 +357,12 @@ describe("BaseInteractionClient", () => { pca.performanceClient ); - interactionClient - // @ts-ignore - .getDiscoveredAuthority({ + const clientInst = interactionClient as any; + getDiscoveredAuthority({ account: testAccount, requestAuthority: "https://login.microsoftonline.com/common", - }) + }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) .then(() => { done(); }) diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index 60c086ff51..2738db523a 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -32,6 +32,7 @@ import * as PkceGenerator from "../../src/crypto/PkceGenerator.js"; import { FetchClient } from "../../src/network/FetchClient.js"; import { InteractionType } from "../../src/utils/BrowserConstants.js"; import { buildAccountFromIdTokenClaims } from "msal-test-utils"; +import { getDiscoveredAuthority } from "../../src/interaction_client/BaseInteractionClient.js"; class testStandardInteractionClient extends StandardInteractionClient { acquireToken(): Promise { @@ -49,7 +50,7 @@ class testStandardInteractionClient extends StandardInteractionClient { requestAuthority?: string; requestAzureCloudOptions?: AzureCloudOptions; }) { - return super.getDiscoveredAuthority(params); + return await getDiscoveredAuthority(params, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); } logout(request: EndSessionRequest): Promise { From 03fcc51dd24f183dfa39ab9d92e3da70cc9cbfae Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Thu, 5 Jun 2025 12:41:37 -0700 Subject: [PATCH 04/12] Temp commit --- .../src/interaction_client/PopupClient.ts | 6 +- .../src/interaction_client/RedirectClient.ts | 6 +- .../SilentAuthCodeClient.ts | 6 +- .../interaction_client/SilentIframeClient.ts | 6 +- .../StandardInteractionClient.ts | 123 ++++++++++-------- 5 files changed, 79 insertions(+), 68 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index 31520c21ed..93c922763b 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -19,7 +19,7 @@ import { PkceCodes, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; +import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; import { EventType } from "../event/EventType.js"; import { InteractionType, @@ -208,12 +208,12 @@ export class PopupClient extends StandardInteractionClient { this.logger.verbose("acquireTokenPopupAsync called"); const validRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, PerformanceEvents.StandardInteractionClientInitializeAuthorizationRequest, this.logger, this.performanceClient, this.correlationId - )(request, InteractionType.Popup); + )(request, InteractionType.Popup, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); /* * Skip pre-connect for async popups to reduce time between user interaction and popup window creation to avoid diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 6ce203e70d..b6b3f10c3a 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -20,7 +20,7 @@ import { InProgressPerformanceEvent, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; +import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; import { ApiId, INTERACTION_TYPE, @@ -101,12 +101,12 @@ export class RedirectClient extends StandardInteractionClient { */ async acquireToken(request: RedirectRequest): Promise { const validRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, PerformanceEvents.StandardInteractionClientInitializeAuthorizationRequest, this.logger, this.performanceClient, this.correlationId - )(request, InteractionType.Redirect); + )(request, InteractionType.Redirect, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); validRequest.platformBroker = isPlatformAuthAllowed( this.config, diff --git a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts index 4dded7efbd..9324575a3a 100644 --- a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts @@ -13,7 +13,7 @@ import { invokeAsync, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; +import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -75,12 +75,12 @@ export class SilentAuthCodeClient extends StandardInteractionClient { // Create silent request const silentRequest: CommonAuthorizationUrlRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, PerformanceEvents.StandardInteractionClientInitializeAuthorizationRequest, this.logger, this.performanceClient, request.correlationId - )(request, InteractionType.Silent); + )(request, InteractionType.Silent, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); const serverTelemetryManager = initializeServerTelemetryManager( this.apiId, diff --git a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts index 9d739b5373..9d58caf98e 100644 --- a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts @@ -16,7 +16,7 @@ import { ProtocolMode, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; +import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -114,12 +114,12 @@ export class SilentIframeClient extends StandardInteractionClient { // Create silent request const silentRequest: CommonAuthorizationUrlRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, PerformanceEvents.StandardInteractionClientInitializeAuthorizationRequest, this.logger, this.performanceClient, request.correlationId - )(inputRequest, InteractionType.Silent); + )(inputRequest, InteractionType.Silent, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); silentRequest.platformBroker = isPlatformAuthAllowed( this.config, this.logger, diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 3ea3ea68b9..5e77648c17 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -18,6 +18,9 @@ import { BaseAuthRequest, StringDict, CommonAuthorizationUrlRequest, + ICrypto, + Logger, + IPerformanceClient, } from "@azure/msal-common/browser"; import { BaseInteractionClient, getDiscoveredAuthority, getRedirectUri } from "./BaseInteractionClient.js"; import { @@ -33,6 +36,8 @@ import { PopupRequest } from "../request/PopupRequest.js"; import { SsoSilentRequest } from "../request/SsoSilentRequest.js"; import { createNewGuid } from "../crypto/BrowserCrypto.js"; import { initializeBaseRequest } from "../request/RequestHelpers.js"; +import { BrowserConfiguration } from "../config/Configuration.js"; +import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; /** * Defines the class structure and helper functions used by the "standard", non-brokered auth flows (popup, redirect, silent (RT), silent (iframe)) @@ -267,66 +272,72 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { telemetry: this.config.telemetry, }; } +} - /** - * Helper to initialize required request parameters for interactive APIs and ssoSilent() - * @param request - * @param interactionType - */ - protected async initializeAuthorizationRequest( - request: RedirectRequest | PopupRequest | SsoSilentRequest, - interactionType: InteractionType - ): Promise { - const redirectUri = getRedirectUri(request.redirectUri, this.config, this.logger); - const browserState: BrowserStateObject = { - interactionType: interactionType, - }; - const state = ProtocolUtils.setRequestState( - this.browserCrypto, - (request && request.state) || "", - browserState - ); - - const baseRequest: BaseAuthRequest = await invokeAsync( - initializeBaseRequest, - PerformanceEvents.InitializeBaseRequest, - this.logger, - this.performanceClient, - this.correlationId - )( - { ...request, correlationId: this.correlationId }, - this.config, - this.performanceClient, - this.logger - ); - - const validatedRequest: CommonAuthorizationUrlRequest = { - ...baseRequest, - redirectUri: redirectUri, - state: state, - nonce: request.nonce || createNewGuid(), - responseMode: this.config.auth.OIDCOptions.responseMode, - }; +/** + * Helper to initialize required request parameters for interactive APIs and ssoSilent() + * @param request + * @param interactionType + */ +export async function initializeAuthorizationRequest( + request: RedirectRequest | PopupRequest | SsoSilentRequest, + interactionType: InteractionType, + config: BrowserConfiguration, + browserCrypto: ICrypto, + browserStorage: BrowserCacheManager, + logger: Logger, + performanceClient: IPerformanceClient, + correlationId: string +): Promise { + const redirectUri = getRedirectUri(request.redirectUri, config, logger); + const browserState: BrowserStateObject = { + interactionType: interactionType, + }; + const state = ProtocolUtils.setRequestState( + browserCrypto, + (request && request.state) || "", + browserState + ); - // Skip active account lookup if either login hint or session id is set - if (request.loginHint || request.sid) { - return validatedRequest; - } + const baseRequest: BaseAuthRequest = await invokeAsync( + initializeBaseRequest, + PerformanceEvents.InitializeBaseRequest, + logger, + performanceClient, + correlationId + )( + { ...request, correlationId: correlationId }, + config, + performanceClient, + logger + ); - const account = - request.account || this.browserStorage.getActiveAccount(); - if (account) { - this.logger.verbose( - "Setting validated request account", - this.correlationId - ); - this.logger.verbosePii( - `Setting validated request account: ${account.homeAccountId}`, - this.correlationId - ); - validatedRequest.account = account; - } + const validatedRequest: CommonAuthorizationUrlRequest = { + ...baseRequest, + redirectUri: redirectUri, + state: state, + nonce: request.nonce || createNewGuid(), + responseMode: config.auth.OIDCOptions.responseMode, + }; + // Skip active account lookup if either login hint or session id is set + if (request.loginHint || request.sid) { return validatedRequest; } + + const account = + request.account || browserStorage.getActiveAccount(); + if (account) { + logger.verbose( + "Setting validated request account", + correlationId + ); + logger.verbosePii( + `Setting validated request account: ${account.homeAccountId}`, + correlationId + ); + validatedRequest.account = account; + } + + return validatedRequest; } From 0c9b1ed453f6e0297efcbef8347d9adb9356cb17 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Tue, 3 Jun 2025 12:14:14 -0700 Subject: [PATCH 05/12] Factor initializeAuthorizationRequest out of StandardInteractionClient --- .../SilentIframeClient.spec.ts | 43 +++++++++---------- .../StandardInteractionClient.spec.ts | 4 +- 2 files changed, 22 insertions(+), 25 deletions(-) diff --git a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts index ce32f250aa..b9b599a913 100644 --- a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts @@ -54,11 +54,13 @@ import { FetchClient } from "../../src/network/FetchClient.js"; import { TestTimeUtils } from "msal-test-utils"; import { AuthenticationResult } from "../../src/response/AuthenticationResult.js"; import { SsoSilentRequest } from "../../src/index.js"; +import * as StandardInteractionClientExports from "../../src/interaction_client/StandardInteractionClient.js"; describe("SilentIframeClient", () => { let silentIframeClient: SilentIframeClient; let pca: PublicClientApplication; let browserCacheManager: BrowserCacheManager; + let clientProperties: SilentIframeClient; beforeEach(() => { pca = new PublicClientApplication({ @@ -95,6 +97,8 @@ describe("SilentIframeClient", () => { undefined, TEST_CONFIG.CORRELATION_ID ); + + clientProperties = silentIframeClient as any; }); afterEach(() => { @@ -167,24 +171,32 @@ describe("SilentIframeClient", () => { RANDOM_TEST_GUID ); - const initializeAuthorizationRequestSpy = jest.spyOn( - SilentIframeClient.prototype, - // @ts-ignore - "initializeAuthorizationRequest" - ); + const initializeAuthorizationRequestSpy = jest.spyOn(StandardInteractionClientExports, "initializeAuthorizationRequest"); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", prompt: Constants.PromptValue.SELECT_ACCOUNT, }); expect(tokenResp).toEqual(testTokenResponse); - expect(initializeAuthorizationRequestSpy).toBeCalledWith( + expect(initializeAuthorizationRequestSpy).toHaveBeenCalledWith( { redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", prompt: Constants.PromptValue.NONE, }, - InteractionType.Silent + InteractionType.Silent, + // @ts-ignore + clientProperties.config, + //@ts-ignore + clientProperties.browserCrypto, + //@ts-ignore + clientProperties.browserStorage, + //@ts-ignore + clientProperties.logger, + //@ts-ignore + clientProperties.performanceClient, + //@ts-ignore + clientProperties.correlationId ); }); @@ -906,11 +918,6 @@ describe("SilentIframeClient", () => { "generateAuthority" ); - const initializeAuthorizationRequestSpy = jest.spyOn( - SilentIframeClient.prototype, - // @ts-ignore - "initializeAuthorizationRequest" - ); const tokenResp = await testClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", @@ -1013,11 +1020,6 @@ describe("SilentIframeClient", () => { "generateAuthority" ); - const initializeAuthorizationRequestSpy = jest.spyOn( - SilentIframeClient.prototype, - // @ts-ignore - "initializeAuthorizationRequest" - ); const tokenResp = await testClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", @@ -1123,12 +1125,7 @@ describe("SilentIframeClient", () => { Authority, "generateAuthority" ); - - const initializeAuthorizationRequestSpy = jest.spyOn( - SilentIframeClient.prototype, - // @ts-ignore - "initializeAuthorizationRequest" - ); + const tokenResp = await testClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index 2738db523a..fd05838e5a 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -15,7 +15,7 @@ import { AccountEntityUtils, } from "@azure/msal-common"; import { PublicClientApplication } from "../../src/app/PublicClientApplication.js"; -import { StandardInteractionClient } from "../../src/interaction_client/StandardInteractionClient.js"; +import { initializeAuthorizationRequest, StandardInteractionClient } from "../../src/interaction_client/StandardInteractionClient.js"; import { EndSessionRequest } from "../../src/request/EndSessionRequest.js"; import { TEST_CONFIG, @@ -43,7 +43,7 @@ class testStandardInteractionClient extends StandardInteractionClient { request: RedirectRequest, interactionType: InteractionType ) { - return super.initializeAuthorizationRequest(request, interactionType); + return initializeAuthorizationRequest(request, interactionType, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); } async getDiscoveredAuthority(params: { From 3f09bab49a0ba547708a356eebff3886950e0f0e Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Thu, 5 Jun 2025 12:42:21 -0700 Subject: [PATCH 06/12] Fix format --- .../BaseInteractionClient.ts | 37 ++++++----- .../PlatformAuthInteractionClient.ts | 63 ++++++++++++++----- .../src/interaction_client/PopupClient.ts | 40 +++++++++--- .../src/interaction_client/RedirectClient.ts | 61 +++++++++++++----- .../SilentAuthCodeClient.ts | 16 ++++- .../interaction_client/SilentIframeClient.ts | 40 +++++++++--- .../interaction_client/SilentRefreshClient.ts | 5 +- .../StandardInteractionClient.ts | 35 ++++++----- .../BaseInteractionClient.spec.ts | 49 ++++++++++++--- .../SilentIframeClient.spec.ts | 7 ++- .../StandardInteractionClient.spec.ts | 25 +++++++- 11 files changed, 279 insertions(+), 99 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index 330db1585a..fcaf473a6f 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -139,13 +139,15 @@ export abstract class BaseInteractionClient { * @returns Redirect URL * */ -export function getRedirectUri(requestRedirectUri?: string, clientConfig?: BrowserConfiguration, logger?: Logger): string { +export function getRedirectUri( + requestRedirectUri?: string, + clientConfig?: BrowserConfiguration, + logger?: Logger +): string { logger?.verbose("getRedirectUri called"); - const redirectUri = requestRedirectUri || clientConfig?.auth.redirectUri || ""; - return UrlString.getAbsoluteUrl( - redirectUri, - BrowserUtils.getCurrentUri() - ); + const redirectUri = + requestRedirectUri || clientConfig?.auth.redirectUri || ""; + return UrlString.getAbsoluteUrl(redirectUri, BrowserUtils.getCurrentUri()); } /** @@ -172,10 +174,7 @@ export function initializeServerTelemetryManager( wrapperVer: browserStorage.getWrapperMetadata()[1], }; - return new ServerTelemetryManager( - telemetryPayload, - browserStorage - ); + return new ServerTelemetryManager(telemetryPayload, browserStorage); } /** @@ -187,7 +186,8 @@ export function initializeServerTelemetryManager( * account?: AccountInfo; * } */ -export async function getDiscoveredAuthority(params: { +export async function getDiscoveredAuthority( + params: { requestAuthority?: string; requestAzureCloudOptions?: AzureCloudOptions; requestExtraQueryParameters?: StringDict; @@ -197,7 +197,8 @@ export async function getDiscoveredAuthority(params: { correlationId: string, performanceClient: IPerformanceClient, browserStorage: BrowserCacheManager, - logger: Logger): Promise { + logger: Logger +): Promise { const { account } = params; const instanceAwareEQ = params.requestExtraQueryParameters && @@ -214,8 +215,7 @@ export async function getDiscoveredAuthority(params: { }; // build authority string based on auth params, precedence - azureCloudInstance + tenant >> authority - const resolvedAuthority = - params.requestAuthority || config.auth.authority; + const resolvedAuthority = params.requestAuthority || config.auth.authority; const resolvedInstanceAware = instanceAwareEQ?.length ? instanceAwareEQ === "true" : config.auth.instanceAware; @@ -223,16 +223,15 @@ export async function getDiscoveredAuthority(params: { const userAuthority = account && resolvedInstanceAware ? config.auth.authority.replace( - UrlString.getDomainFromUrl(resolvedAuthority), - account.environment - ) + UrlString.getDomainFromUrl(resolvedAuthority), + account.environment + ) : resolvedAuthority; // fall back to the authority from config const builtAuthority = Authority.generateAuthority( userAuthority, - params.requestAzureCloudOptions || - config.auth.azureCloudOptions + params.requestAzureCloudOptions || config.auth.azureCloudOptions ); const discoveredAuthority = await invokeAsync( AuthorityFactory.createDiscoveredInstance, diff --git a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts index f3b325d538..756796808f 100644 --- a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts @@ -34,7 +34,12 @@ import { AccountEntityUtils, Constants, } from "@azure/msal-common/browser"; -import { BaseInteractionClient, getDiscoveredAuthority, getRedirectUri, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { + BaseInteractionClient, + getDiscoveredAuthority, + getRedirectUri, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -163,7 +168,7 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { const serverTelemetryManager = initializeServerTelemetryManager( this.apiId, this.config, - this.correlationId , + this.correlationId, this.browserStorage, this.logger ); @@ -320,8 +325,13 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { } catch (e) { // Only throw fatal errors here to allow application to fallback to regular redirect. Otherwise proceed and the error will be thrown in handleRedirectPromise if (e instanceof NativeAuthError) { - const serverTelemetryManager = - initializeServerTelemetryManager(this.apiId, this.config, this.correlationId, this.browserStorage, this.logger); + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId, + this.browserStorage, + this.logger + ); serverTelemetryManager.setNativeBrokerErrorCode(e.errorCode); if (isFatalNativeAuthError(e)) { throw e; @@ -410,8 +420,13 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { reqTimestamp ); - const serverTelemetryManager = - initializeServerTelemetryManager(this.apiId, this.config, this.correlationId, this.browserStorage, this.logger); + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId, + this.browserStorage, + this.logger + ); serverTelemetryManager.clearNativeBrokerErrorCode(); return authResult; } catch (e) { @@ -476,9 +491,16 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { } // Get the preferred_cache domain for the given authority - const authority = await getDiscoveredAuthority({ - requestAuthority: request.authority, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + const authority = await getDiscoveredAuthority( + { + requestAuthority: request.authority, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const baseAccount = buildAccountToCache( this.browserStorage, @@ -885,7 +907,11 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { clientId: this.config.auth.clientId, authority: canonicalAuthority.urlString, scope: scopeSet.printScopes(), - redirectUri: getRedirectUri(request.redirectUri, this.config, this.logger), + redirectUri: getRedirectUri( + request.redirectUri, + this.config, + this.logger + ), prompt: this.getPrompt(request.prompt), correlationId: this.correlationId, tokenType: request.authenticationScheme, @@ -960,11 +986,18 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { if (request.account) { // validate authority - await getDiscoveredAuthority({ - requestAuthority, - requestAzureCloudOptions: request.azureCloudOptions, - account: request.account, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + await getDiscoveredAuthority( + { + requestAuthority, + requestAzureCloudOptions: request.azureCloudOptions, + account: request.account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); } const canonicalAuthority = new UrlString(requestAuthority); diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index 93c922763b..fc5eb75008 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -19,7 +19,10 @@ import { PkceCodes, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import { EventType } from "../event/EventType.js"; import { InteractionType, @@ -47,7 +50,10 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; -import { getDiscoveredAuthority, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { + getDiscoveredAuthority, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; export type PopupParams = { popup?: Window | null; @@ -213,7 +219,16 @@ export class PopupClient extends StandardInteractionClient { this.logger, this.performanceClient, this.correlationId - )(request, InteractionType.Popup, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); + )( + request, + InteractionType.Popup, + this.config, + this.browserCrypto, + this.browserStorage, + this.logger, + this.performanceClient, + this.correlationId + ); /* * Skip pre-connect for async popups to reduce time between user interaction and popup window creation to avoid @@ -383,12 +398,19 @@ export class PopupClient extends StandardInteractionClient { this.logger, this.performanceClient, correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + )( + { + requestAuthority: request.authority, + requestAzureCloudOptions: request.azureCloudOptions, + requestExtraQueryParameters: request.extraQueryParameters, + account: request.account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const earJwk = await invokeAsync( generateEarKey, diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index b6b3f10c3a..308f215eb4 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -20,7 +20,10 @@ import { InProgressPerformanceEvent, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import { ApiId, INTERACTION_TYPE, @@ -48,7 +51,10 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; -import { getDiscoveredAuthority, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { + getDiscoveredAuthority, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; function getNavigationType(): NavigationTimingType | undefined { if ( @@ -106,7 +112,16 @@ export class RedirectClient extends StandardInteractionClient { this.logger, this.performanceClient, this.correlationId - )(request, InteractionType.Redirect, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); + )( + request, + InteractionType.Redirect, + this.config, + this.browserCrypto, + this.browserStorage, + this.logger, + this.performanceClient, + this.correlationId + ); validRequest.platformBroker = isPlatformAuthAllowed( this.config, @@ -169,7 +184,7 @@ export class RedirectClient extends StandardInteractionClient { const correlationId = request.correlationId; const serverTelemetryManager = initializeServerTelemetryManager( ApiId.acquireTokenRedirect, - this.config, + this.config, this.correlationId, this.browserStorage, this.logger @@ -250,12 +265,19 @@ export class RedirectClient extends StandardInteractionClient { this.logger, this.performanceClient, correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + )( + { + requestAuthority: request.authority, + requestAzureCloudOptions: request.azureCloudOptions, + requestExtraQueryParameters: request.extraQueryParameters, + account: request.account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const earJwk = await invokeAsync( generateEarKey, @@ -532,12 +554,19 @@ export class RedirectClient extends StandardInteractionClient { this.logger, this.performanceClient, request.correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + )( + { + requestAuthority: request.authority, + requestAzureCloudOptions: request.azureCloudOptions, + requestExtraQueryParameters: request.extraQueryParameters, + account: request.account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); return invokeAsync( Authorize.handleResponseEAR, PerformanceEvents.HandleResponseEar, diff --git a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts index 9324575a3a..aa7b3d2c56 100644 --- a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts @@ -13,7 +13,10 @@ import { invokeAsync, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -80,7 +83,16 @@ export class SilentAuthCodeClient extends StandardInteractionClient { this.logger, this.performanceClient, request.correlationId - )(request, InteractionType.Silent, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); + )( + request, + InteractionType.Silent, + this.config, + this.browserCrypto, + this.browserStorage, + this.logger, + this.performanceClient, + this.correlationId + ); const serverTelemetryManager = initializeServerTelemetryManager( this.apiId, diff --git a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts index 9d58caf98e..f5a525d6dd 100644 --- a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts @@ -16,7 +16,10 @@ import { ProtocolMode, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; -import { initializeAuthorizationRequest, StandardInteractionClient } from "./StandardInteractionClient.js"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -44,7 +47,10 @@ import { generatePkceCodes } from "../crypto/PkceGenerator.js"; import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvider.js"; import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; -import { getDiscoveredAuthority, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { + getDiscoveredAuthority, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; export class SilentIframeClient extends StandardInteractionClient { protected apiId: ApiId; @@ -119,7 +125,16 @@ export class SilentIframeClient extends StandardInteractionClient { this.logger, this.performanceClient, request.correlationId - )(inputRequest, InteractionType.Silent, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); + )( + inputRequest, + InteractionType.Silent, + this.config, + this.browserCrypto, + this.browserStorage, + this.logger, + this.performanceClient, + this.correlationId + ); silentRequest.platformBroker = isPlatformAuthAllowed( this.config, this.logger, @@ -220,12 +235,19 @@ export class SilentIframeClient extends StandardInteractionClient { this.logger, this.performanceClient, correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + )( + { + requestAuthority: request.authority, + requestAzureCloudOptions: request.azureCloudOptions, + requestExtraQueryParameters: request.extraQueryParameters, + account: request.account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const earJwk = await invokeAsync( generateEarKey, diff --git a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts index 29ba0cc217..721e0a4247 100644 --- a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts @@ -22,7 +22,10 @@ import { } from "../error/BrowserAuthError.js"; import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { initializeBaseRequest } from "../request/RequestHelpers.js"; -import { getRedirectUri, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { + getRedirectUri, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; export class SilentRefreshClient extends StandardInteractionClient { /** diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 5e77648c17..04e9a3186b 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -22,7 +22,11 @@ import { Logger, IPerformanceClient, } from "@azure/msal-common/browser"; -import { BaseInteractionClient, getDiscoveredAuthority, getRedirectUri } from "./BaseInteractionClient.js"; +import { + BaseInteractionClient, + getDiscoveredAuthority, + getRedirectUri, +} from "./BaseInteractionClient.js"; import { BrowserConstants, InteractionType, @@ -233,12 +237,19 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { this.logger, this.performanceClient, this.correlationId - )({ - requestAuthority, - requestAzureCloudOptions, - requestExtraQueryParameters, - account, - }, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + )( + { + requestAuthority, + requestAzureCloudOptions, + requestExtraQueryParameters, + account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const logger = this.config.system.loggerOptions; return { @@ -283,7 +294,7 @@ export async function initializeAuthorizationRequest( request: RedirectRequest | PopupRequest | SsoSilentRequest, interactionType: InteractionType, config: BrowserConfiguration, - browserCrypto: ICrypto, + browserCrypto: ICrypto, browserStorage: BrowserCacheManager, logger: Logger, performanceClient: IPerformanceClient, @@ -325,13 +336,9 @@ export async function initializeAuthorizationRequest( return validatedRequest; } - const account = - request.account || browserStorage.getActiveAccount(); + const account = request.account || browserStorage.getActiveAccount(); if (account) { - logger.verbose( - "Setting validated request account", - correlationId - ); + logger.verbose("Setting validated request account", correlationId); logger.verbosePii( `Setting validated request account: ${account.homeAccountId}`, correlationId diff --git a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts index 7b193d1710..7695e56b71 100644 --- a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts @@ -22,7 +22,10 @@ import { ID_TOKEN_CLAIMS, ID_TOKEN_ALT_CLAIMS, } from "../utils/StringConstants.js"; -import { BaseInteractionClient, getDiscoveredAuthority } from "../../src/interaction_client/BaseInteractionClient.js"; +import { + BaseInteractionClient, + getDiscoveredAuthority, +} from "../../src/interaction_client/BaseInteractionClient.js"; import { EndSessionRequest, PublicClientApplication, @@ -246,11 +249,18 @@ describe("BaseInteractionClient", () => { let clientInst = testClient as any; - await getDiscoveredAuthority({ + await getDiscoveredAuthority( + { requestAuthority: "https://login.microsoftonline.com/common", account: testAccount, - }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) + }, + clientInst.config, + clientInst.correlationId, + clientInst.performanceClient, + clientInst.browserStorage, + clientInst.logger + ) .then(() => { throw "This is unexpected. This call should have failed."; }) @@ -274,11 +284,18 @@ describe("BaseInteractionClient", () => { const clientInst = testClient as any; - getDiscoveredAuthority({ + getDiscoveredAuthority( + { requestAuthority: "https://login.microsoftonline.com/common", account: testAccount, - }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) + }, + clientInst.config, + clientInst.correlationId, + clientInst.performanceClient, + clientInst.browserStorage, + clientInst.logger + ) .then(() => { done(); }) @@ -317,9 +334,16 @@ describe("BaseInteractionClient", () => { ); const clientInst = interactionClient as any; - getDiscoveredAuthority({ + getDiscoveredAuthority( + { account: testAccount, - }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) + }, + clientInst.config, + clientInst.correlationId, + clientInst.performanceClient, + clientInst.browserStorage, + clientInst.logger + ) .then(() => { done(); }) @@ -358,11 +382,18 @@ describe("BaseInteractionClient", () => { ); const clientInst = interactionClient as any; - getDiscoveredAuthority({ + getDiscoveredAuthority( + { account: testAccount, requestAuthority: "https://login.microsoftonline.com/common", - }, clientInst.config, clientInst.correlationId, clientInst.performanceClient, clientInst.browserStorage, clientInst.logger) + }, + clientInst.config, + clientInst.correlationId, + clientInst.performanceClient, + clientInst.browserStorage, + clientInst.logger + ) .then(() => { done(); }) diff --git a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts index b9b599a913..c98c2ab8d2 100644 --- a/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts @@ -171,7 +171,10 @@ describe("SilentIframeClient", () => { RANDOM_TEST_GUID ); - const initializeAuthorizationRequestSpy = jest.spyOn(StandardInteractionClientExports, "initializeAuthorizationRequest"); + const initializeAuthorizationRequestSpy = jest.spyOn( + StandardInteractionClientExports, + "initializeAuthorizationRequest" + ); const tokenResp = await silentIframeClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", @@ -1125,7 +1128,7 @@ describe("SilentIframeClient", () => { Authority, "generateAuthority" ); - + const tokenResp = await testClient.acquireToken({ redirectUri: TEST_URIS.TEST_REDIR_URI, loginHint: "testLoginHint", diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index fd05838e5a..c2bc8fa6b6 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -15,7 +15,10 @@ import { AccountEntityUtils, } from "@azure/msal-common"; import { PublicClientApplication } from "../../src/app/PublicClientApplication.js"; -import { initializeAuthorizationRequest, StandardInteractionClient } from "../../src/interaction_client/StandardInteractionClient.js"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "../../src/interaction_client/StandardInteractionClient.js"; import { EndSessionRequest } from "../../src/request/EndSessionRequest.js"; import { TEST_CONFIG, @@ -43,14 +46,30 @@ class testStandardInteractionClient extends StandardInteractionClient { request: RedirectRequest, interactionType: InteractionType ) { - return initializeAuthorizationRequest(request, interactionType, this.config, this.browserCrypto, this.browserStorage, this.logger, this.performanceClient, this.correlationId); + return initializeAuthorizationRequest( + request, + interactionType, + this.config, + this.browserCrypto, + this.browserStorage, + this.logger, + this.performanceClient, + this.correlationId + ); } async getDiscoveredAuthority(params: { requestAuthority?: string; requestAzureCloudOptions?: AzureCloudOptions; }) { - return await getDiscoveredAuthority(params, this.config, this.correlationId, this.performanceClient, this.browserStorage, this.logger); + return await getDiscoveredAuthority( + params, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); } logout(request: EndSessionRequest): Promise { From c1111f6d1b5407304a84f486bc07683db3315daa Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Thu, 5 Jun 2025 15:34:19 -0700 Subject: [PATCH 07/12] Trigger CI From fa45a8710ea34bada4cb3e4d89be0f2aabeb9f98 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 9 Jun 2025 14:31:18 -0700 Subject: [PATCH 08/12] Factor out clearCacheOnLogout from BaseInteractionClient --- .../BaseInteractionClient.ts | 94 ++++++++++--------- .../src/interaction_client/PopupClient.ts | 9 +- .../src/interaction_client/RedirectClient.ts | 9 +- .../interaction_client/SilentCacheClient.ts | 10 +- .../BaseInteractionClient.spec.ts | 9 +- .../StandardInteractionClient.spec.ts | 10 +- 6 files changed, 89 insertions(+), 52 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index 9817f37c52..4c17b37631 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -85,51 +85,6 @@ export abstract class BaseInteractionClient { abstract logout( request: EndSessionRequest | ClearCacheRequest | undefined ): Promise; - - protected async clearCacheOnLogout( - account?: AccountInfo | null - ): Promise { - if (account) { - if ( - AccountEntityUtils.accountInfoIsEqual( - account, - this.browserStorage.getActiveAccount(), - false - ) - ) { - this.logger.verbose("Setting active account to null"); - this.browserStorage.setActiveAccount(null); - } - // Clear given account. - try { - await this.browserStorage.removeAccount( - AccountEntityUtils.generateAccountCacheKey(account) - ); - this.logger.verbose( - "Cleared cache items belonging to the account provided in the logout request." - ); - } catch (error) { - this.logger.error( - "Account provided in logout request was not found. Local cache unchanged." - ); - } - } else { - try { - this.logger.verbose( - "No account provided in logout request, clearing all cache items.", - this.correlationId - ); - // Clear all accounts and tokens - await this.browserStorage.clear(); - // Clear any stray keys from IndexedDB - await this.browserCrypto.clearKeystore(); - } catch (e) { - this.logger.error( - "Attempted to clear all MSAL cache items and failed. Local cache unchanged." - ); - } - } - } } /** @@ -257,3 +212,52 @@ export async function getDiscoveredAuthority( return discoveredAuthority; } + +export async function clearCacheOnLogout( + browserStorage: BrowserCacheManager, + browserCrypto: ICrypto, + logger: Logger, + correlationId: string, + account?: AccountInfo | null, + ): Promise { + if (account) { + if ( + AccountEntityUtils.accountInfoIsEqual( + account, + browserStorage.getActiveAccount(), + false + ) + ) { + logger.verbose("Setting active account to null"); + browserStorage.setActiveAccount(null); + } + // Clear given account. + try { + await browserStorage.removeAccount( + AccountEntityUtils.generateAccountCacheKey(account) + ); + logger.verbose( + "Cleared cache items belonging to the account provided in the logout request." + ); + } catch (error) { + logger.error( + "Account provided in logout request was not found. Local cache unchanged." + ); + } + } else { + try { + logger.verbose( + "No account provided in logout request, clearing all cache items.", + correlationId + ); + // Clear all accounts and tokens + await browserStorage.clear(); + // Clear any stray keys from IndexedDB + await browserCrypto.clearKeystore(); + } catch (e) { + logger.error( + "Attempted to clear all MSAL cache items and failed. Local cache unchanged." + ); + } + } + } diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index eee06d933e..f1d57a07c1 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -52,6 +52,7 @@ import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvid import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; import { + clearCacheOnLogout, getDiscoveredAuthority, initializeServerTelemetryManager, } from "./BaseInteractionClient.js"; @@ -511,7 +512,13 @@ export class PopupClient extends StandardInteractionClient { try { // Clear cache on logout - await this.clearCacheOnLogout(validRequest.account); + await clearCacheOnLogout( + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + validRequest.account + ); // Initialize the client const authClient = await invokeAsync( diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 2ebb9c3703..546d09b8a7 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -53,6 +53,7 @@ import { isPlatformAuthAllowed } from "../broker/nativeBroker/PlatformAuthProvid import { generateEarKey } from "../crypto/BrowserCrypto.js"; import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js"; import { + clearCacheOnLogout, getDiscoveredAuthority, initializeServerTelemetryManager, } from "./BaseInteractionClient.js"; @@ -707,7 +708,13 @@ export class RedirectClient extends StandardInteractionClient { ); // Clear cache on logout - await this.clearCacheOnLogout(validLogoutRequest.account); + await clearCacheOnLogout( + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + validLogoutRequest.account + ); const navigationOptions: NavigationOptions = { apiId: ApiId.logout, diff --git a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts index 61433cf30a..de0e9d084d 100644 --- a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts @@ -17,7 +17,7 @@ import { } from "../error/BrowserAuthError.js"; import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { ClearCacheRequest } from "../request/ClearCacheRequest.js"; -import { initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { clearCacheOnLogout, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; export class SilentCacheClient extends StandardInteractionClient { /** @@ -91,6 +91,12 @@ export class SilentCacheClient extends StandardInteractionClient { logout(logoutRequest?: ClearCacheRequest): Promise { this.logger.verbose("logoutRedirect called"); const validLogoutRequest = this.initializeLogoutRequest(logoutRequest); - return this.clearCacheOnLogout(validLogoutRequest?.account); + return clearCacheOnLogout( + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + validLogoutRequest.account + ); } } diff --git a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts index 7695e56b71..b2ba769096 100644 --- a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts @@ -24,6 +24,7 @@ import { } from "../utils/StringConstants.js"; import { BaseInteractionClient, + clearCacheOnLogout, getDiscoveredAuthority, } from "../../src/interaction_client/BaseInteractionClient.js"; import { @@ -39,7 +40,13 @@ class testInteractionClient extends BaseInteractionClient { } logout(request: EndSessionRequest): Promise { - return this.clearCacheOnLogout(request.account); + return clearCacheOnLogout( + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + request.account + ); } } diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index c2bc8fa6b6..b62aee8de3 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -35,7 +35,7 @@ import * as PkceGenerator from "../../src/crypto/PkceGenerator.js"; import { FetchClient } from "../../src/network/FetchClient.js"; import { InteractionType } from "../../src/utils/BrowserConstants.js"; import { buildAccountFromIdTokenClaims } from "msal-test-utils"; -import { getDiscoveredAuthority } from "../../src/interaction_client/BaseInteractionClient.js"; +import { clearCacheOnLogout, getDiscoveredAuthority } from "../../src/interaction_client/BaseInteractionClient.js"; class testStandardInteractionClient extends StandardInteractionClient { acquireToken(): Promise { @@ -73,7 +73,13 @@ class testStandardInteractionClient extends StandardInteractionClient { } logout(request: EndSessionRequest): Promise { - return this.clearCacheOnLogout(request.account); + return clearCacheOnLogout( + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + request.account + ); } } From 5af45781ff7f45ce97567cd7d6c097a91d78f2b6 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 16 Jun 2025 13:50:05 -0700 Subject: [PATCH 09/12] Factor out PopupClient utils --- .../src/interaction_client/PopupClient.ts | 100 +++--------------- lib/msal-browser/src/utils/PopupUtils.ts | 92 ++++++++++++++++ .../test/app/PublicClientApplication.spec.ts | 7 +- .../interaction_client/PopupClient.spec.ts | 54 ++++++---- .../cache/cache-test-files/default-cache.json | 88 +-------------- 5 files changed, 143 insertions(+), 198 deletions(-) create mode 100644 lib/msal-browser/src/utils/PopupUtils.ts diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index f68c32ba05..02556a4c86 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -56,6 +56,7 @@ import { getDiscoveredAuthority, initializeServerTelemetryManager, } from "./BaseInteractionClient.js"; +import { monitorPopupForHash } from "../utils/PopupUtils.js"; export type PopupParams = { popup?: Window | null; @@ -339,9 +340,12 @@ export class PopupClient extends StandardInteractionClient { ); // Monitor the window for the hash. Return the string value and close the popup when the hash is received. Default timeout is 60 seconds. - const responseString = await this.monitorPopupForHash( + const responseString = await monitorPopupForHash( popupWindow, - popupParams.popupWindowParent + popupParams.popupWindowParent, + this.config, + this.logger, + this.unloadWindow ); const serverParams = invoke( @@ -444,12 +448,12 @@ export class PopupClient extends StandardInteractionClient { // Monitor the popup for the hash. Return the string value and close the popup when the hash is received. Default timeout is 60 seconds. const responseString = await invokeAsync( - this.monitorPopupForHash.bind(this), + monitorPopupForHash.bind(this), BrowserPerformanceEvents.SilentHandlerMonitorIframeForHash, this.logger, this.performanceClient, correlationId - )(popupWindow, popupParams.popupWindowParent); + )(popupWindow, popupParams.popupWindowParent, this.config, this.logger, this.unloadWindow); const serverParams = invoke( ResponseHandler.deserializeResponse, @@ -597,9 +601,12 @@ export class PopupClient extends StandardInteractionClient { null ); - await this.monitorPopupForHash( + await monitorPopupForHash( popupWindow, - popupParams.popupWindowParent + popupParams.popupWindowParent, + this.config, + this.logger, + this.unloadWindow ).catch(() => { // Swallow any errors related to monitoring the window. Server logout is best effort }); @@ -674,72 +681,6 @@ export class PopupClient extends StandardInteractionClient { } } - /** - * Monitors a window until it loads a url with the same origin. - * @param popupWindow - window that is being monitored - * @param timeout - timeout for processing hash once popup is redirected back to application - */ - monitorPopupForHash( - popupWindow: Window, - popupWindowParent: Window - ): Promise { - return new Promise((resolve, reject) => { - this.logger.verbose( - "PopupHandler.monitorPopupForHash - polling started" - ); - - const intervalId = setInterval(() => { - // Window is closed - if (popupWindow.closed) { - this.logger.error( - "PopupHandler.monitorPopupForHash - window closed" - ); - clearInterval(intervalId); - reject( - createBrowserAuthError( - BrowserAuthErrorCodes.userCancelled - ) - ); - return; - } - - let href = ""; - try { - /* - * Will throw if cross origin, - * which should be caught and ignored - * since we need the interval to keep running while on STS UI. - */ - href = popupWindow.location.href; - } catch (e) {} - - // Don't process blank pages or cross domain - if (!href || href === "about:blank") { - return; - } - clearInterval(intervalId); - - let responseString = ""; - const responseType = this.config.auth.OIDCOptions.responseMode; - if (popupWindow) { - if (responseType === Constants.ResponseMode.QUERY) { - responseString = popupWindow.location.search; - } else { - responseString = popupWindow.location.hash; - } - } - - this.logger.verbose( - "PopupHandler.monitorPopupForHash - popup window is on same origin as caller" - ); - - resolve(responseString); - }, this.config.system.pollIntervalMilliseconds); - }).finally(() => { - this.cleanPopup(popupWindow, popupWindowParent); - }); - } - /** * @hidden * @@ -888,21 +829,6 @@ export class PopupClient extends StandardInteractionClient { e.preventDefault(); } - /** - * Closes popup, removes any state vars created during popup calls. - * @param popupWindow - */ - cleanPopup(popupWindow: Window, popupWindowParent: Window): void { - // Close window. - popupWindow.close(); - - // Remove window unload function - popupWindowParent.removeEventListener( - "beforeunload", - this.unloadWindow - ); - } - /** * Generates the name for the popup based on the client id and request * @param clientId diff --git a/lib/msal-browser/src/utils/PopupUtils.ts b/lib/msal-browser/src/utils/PopupUtils.ts new file mode 100644 index 0000000000..6eaaa08f4d --- /dev/null +++ b/lib/msal-browser/src/utils/PopupUtils.ts @@ -0,0 +1,92 @@ +/* + * Copyright (c) Microsoft Corporation. All rights reserved. + * Licensed under the MIT License. + */ + +import { Constants, Logger } from "@azure/msal-common"; +import { BrowserConfiguration } from "../config/Configuration.js"; +import { BrowserAuthErrorCodes, createBrowserAuthError } from "../error/BrowserAuthError.js"; + +/** + * Monitors a window until it loads a url with the same origin. + * @param popupWindow - window that is being monitored + * @param timeout - timeout for processing hash once popup is redirected back to application + */ +export async function monitorPopupForHash( + popupWindow: Window, + popupWindowParent: Window, + config: BrowserConfiguration, + logger: Logger, + unloadWindow: (e: Event) => void +): Promise { + return new Promise((resolve, reject) => { + logger.verbose( + "PopupHandler.monitorPopupForHash - polling started" + ); + + const intervalId = setInterval(() => { + // Window is closed + if (popupWindow.closed) { + logger.error( + "PopupHandler.monitorPopupForHash - window closed" + ); + clearInterval(intervalId); + reject( + createBrowserAuthError( + BrowserAuthErrorCodes.userCancelled + ) + ); + return; + } + + let href = ""; + try { + /* + * Will throw if cross origin, + * which should be caught and ignored + * since we need the interval to keep running while on STS UI. + */ + href = popupWindow.location.href; + } catch (e) {} + + // Don't process blank pages or cross domain + if (!href || href === "about:blank") { + return; + } + clearInterval(intervalId); + + let responseString = ""; + const responseType = config.auth.OIDCOptions.responseMode; + if (popupWindow) { + if (responseType === Constants.ResponseMode.QUERY) { + responseString = popupWindow.location.search; + } else { + responseString = popupWindow.location.hash; + } + } + + logger.verbose( + "PopupHandler.monitorPopupForHash - popup window is on same origin as caller" + ); + + resolve(responseString); + }, config.system.pollIntervalMilliseconds); + }).finally(() => { + cleanPopup(popupWindow, popupWindowParent, unloadWindow); + }); +} + +/** + * Closes popup, removes any state vars created during popup calls. + * @param popupWindow + */ +export function cleanPopup(popupWindow: Window, popupWindowParent: Window, unloadWindow: (e: Event) => void): void { + // Close window. + popupWindow.close(); + + // Remove window unload function + popupWindowParent.removeEventListener( + "beforeunload", + unloadWindow + ); +} diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 4325db3949..9ac26bbbf1 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -84,6 +84,7 @@ import { import * as BrowserUtils from "../../src/utils/BrowserUtils.js"; import { RedirectClient } from "../../src/interaction_client/RedirectClient.js"; import { PopupClient } from "../../src/interaction_client/PopupClient.js"; +import * as PopupUtils from "../../src/utils/PopupUtils.js"; import { SilentCacheClient } from "../../src/interaction_client/SilentCacheClient.js"; import { SilentRefreshClient } from "../../src/interaction_client/SilentRefreshClient.js"; import { SilentAuthCodeClient } from "../../src/interaction_client/SilentAuthCodeClient.js"; @@ -3225,7 +3226,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }; jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockRejectedValue("Not important for this test"); @@ -3283,7 +3284,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { }; jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockRejectedValue("Not important for this test"); try { @@ -6613,7 +6614,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { "openSizedPopup" ).mockReturnValue(popupWindow); jest.spyOn( - PopupClient.prototype, + PopupUtils, "cleanPopup" ).mockImplementation(); }); diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index d19264aa40..3695cd95a2 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -48,6 +48,7 @@ import * as PkceGenerator from "../../src/crypto/PkceGenerator.js"; import * as AuthorizeProtocol from "../../src/protocol/Authorize.js"; import { NavigationClient } from "../../src/navigation/NavigationClient.js"; import { EndSessionPopupRequest } from "../../src/request/EndSessionPopupRequest.js"; +import * as PopupUtils from "../../src/utils/PopupUtils.js"; import { PopupClient } from "../../src/interaction_client/PopupClient.js"; import { PlatformAuthInteractionClient } from "../../src/interaction_client/PlatformAuthInteractionClient.js"; import { PlatformAuthExtensionHandler } from "../../src/broker/nativeBroker/PlatformAuthExtensionHandler.js"; @@ -389,7 +390,7 @@ describe("PopupClient", () => { return window; }); jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue( TEST_HASHES.TEST_SUCCESS_NATIVE_ACCOUNT_ID_POPUP @@ -516,7 +517,7 @@ describe("PopupClient", () => { return window; }); jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue( TEST_HASHES.TEST_SUCCESS_NATIVE_ACCOUNT_ID_POPUP @@ -623,7 +624,7 @@ describe("PopupClient", () => { return window; }); jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue(TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP); jest.spyOn( @@ -646,7 +647,7 @@ describe("PopupClient", () => { it("throws hash_empty_error if popup returns to redirectUri without a hash", (done) => { jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue(""); @@ -667,7 +668,7 @@ describe("PopupClient", () => { it("throws hash_does_not_contain_known_properties error if popup returns to redirectUri with unrecognized params in the hash", (done) => { jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue( "#fakeKey=fakeValue&anotherFakeKey=anotherFakeValue" @@ -697,7 +698,7 @@ describe("PopupClient", () => { window ); jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue(TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP); jest.spyOn( @@ -894,7 +895,7 @@ describe("PopupClient", () => { // Suppress navigation }); jest.spyOn( - PopupClient.prototype, + PopupUtils, "monitorPopupForHash" ).mockResolvedValue( `#ear_jwe=${validEarJWE}&state=${TEST_STATE_VALUES.TEST_STATE_POPUP}` @@ -1421,7 +1422,7 @@ describe("PopupClient", () => { popupWindow ); jest.spyOn( - PopupClient.prototype, + PopupUtils, "cleanPopup" ).mockImplementation(); jest.spyOn( @@ -1451,7 +1452,7 @@ describe("PopupClient", () => { popupWindow ); jest.spyOn( - PopupClient.prototype, + PopupUtils, "cleanPopup" ).mockImplementation(); @@ -1504,7 +1505,7 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn(PopupClient.prototype, "cleanPopup").mockImplementation( + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation( (popup) => { window.sessionStorage.removeItem( `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}` @@ -1806,7 +1807,8 @@ describe("PopupClient", () => { close: () => {}, closed: false, }; - popupClient.monitorPopupForHash(popup, window).catch((error) => { + const clientImpl = popupClient as any; + PopupUtils.monitorPopupForHash(popup, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow).catch((error) => { expect(error.errorCode).toEqual("user_cancelled"); done(); }); @@ -1827,7 +1829,8 @@ describe("PopupClient", () => { close: () => {}, closed: false, }; - popupClient.monitorPopupForHash(popup, window).then((hash) => { + const clientImpl = popupClient as any; + PopupUtils.monitorPopupForHash(popup, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow).then((hash) => { expect(hash).toEqual("code=testCode"); done(); }); @@ -1882,9 +1885,8 @@ describe("PopupClient", () => { undefined, TEST_CONFIG.CORRELATION_ID ); - - const result = await popupClient - .monitorPopupForHash(popup as Window, window) + const clientImpl = popupClient as any; + const result = await PopupUtils.monitorPopupForHash(popup as Window, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow) .catch((e) => { expect(e.errorCode).toEqual( BrowserAuthErrorCodes.monitorPopupTimeout @@ -1906,8 +1908,8 @@ describe("PopupClient", () => { close: () => {}, }; - popupClient - .monitorPopupForHash(popup as unknown as Window, window) + const clientImpl = popupClient as any; + PopupUtils.monitorPopupForHash(popup as unknown as Window, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow) .then((hash: string) => { expect(hash).toEqual("#code=hello"); done(); @@ -1964,9 +1966,13 @@ describe("PopupClient", () => { close: () => {}, }; - const result = await popupClient.monitorPopupForHash( + const clientImpl = popupClient as any; + const result = await PopupUtils.monitorPopupForHash( popup as unknown as Window, - window + window, + clientImpl.config, + clientImpl.logger, + clientImpl.unloadWindow ); expect(result).toEqual("?code=authCode"); }); @@ -1981,8 +1987,14 @@ describe("PopupClient", () => { closed: true, }; - popupClient - .monitorPopupForHash(popup as unknown as Window, window) + const clientImpl = popupClient as any; + PopupUtils.monitorPopupForHash( + popup as unknown as Window, + window, + clientImpl.config, + clientImpl.logger, + clientImpl.unloadWindow + ) .catch((error: AuthError) => { expect(error.errorCode).toEqual("user_cancelled"); done(); diff --git a/lib/msal-node/test/cache/cache-test-files/default-cache.json b/lib/msal-node/test/cache/cache-test-files/default-cache.json index 40747cdef3..f0e0e1d07f 100644 --- a/lib/msal-node/test/cache/cache-test-files/default-cache.json +++ b/lib/msal-node/test/cache/cache-test-files/default-cache.json @@ -1,87 +1 @@ -{ - "Account": { - "uid.utid-login.microsoftonline.com-microsoft": { - "username": "John Doe", - "local_account_id": "object1234", - "realm": "microsoft", - "environment": "login.microsoftonline.com", - "home_account_id": "uid.utid", - "authority_type": "MSSTS", - "client_info": "eyJ1aWQiOiJ1aWQiLCAidXRpZCI6InV0aWQifQ==" - } - }, - "RefreshToken": { - "uid.utid-login.microsoftonline.com-refreshtoken-mock_client_id----": { - "environment": "login.microsoftonline.com", - "credential_type": "RefreshToken", - "secret": "a refresh token", - "client_id": "mock_client_id", - "home_account_id": "uid.utid" - }, - "uid.utid-login.microsoftonline.com-refreshtoken-1----": { - "environment": "login.microsoftonline.com", - "credential_type": "RefreshToken", - "secret": "a refresh token", - "client_id": "mock_client_id", - "home_account_id": "uid.utid", - "family_id": "1" - } - }, - "AccessToken": { - "uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope1 scope2 scope3--": { - "environment": "login.microsoftonline.com", - "credential_type": "AccessToken", - "secret": "an access token", - "realm": "microsoft", - "target": "scope1 scope2 scope3", - "client_id": "mock_client_id", - "cached_at": "1000", - "home_account_id": "uid.utid", - "extended_expires_on": "4600", - "expires_on": "4600" - }, - "uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope4 scope5--": { - "environment": "login.microsoftonline.com", - "credential_type": "AccessToken", - "secret": "an access token", - "realm": "microsoft", - "target": "scope4 scope5", - "client_id": "mock_client_id", - "cached_at": "1000", - "home_account_id": "uid.utid", - "extended_expires_on": "4600", - "expires_on": "4600" - }, - "uid.utid-login.microsoftonline.com-accesstoken_with_authscheme-mock_client_id-microsoft-scope4 scope5--pop": { - "environment": "login.microsoftonline.com", - "credential_type": "AccessToken_With_AuthScheme", - "secret": "a POP-protected access token", - "realm": "microsoft", - "target": "scope4 scope5", - "client_id": "mock_client_id", - "cached_at": "1000", - "home_account_id": "uid.utid", - "extended_expires_on": "4600", - "expires_on": "4600", - "keyId": "NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs", - "tokenType": "pop" - } - }, - "IdToken": { - "uid.utid-login.microsoftonline.com-idtoken-mock_client_id-microsoft---": { - "realm": "microsoft", - "environment": "login.microsoftonline.com", - "credential_type": "IdToken", - "secret": "header.eyJvaWQiOiAib2JqZWN0MTIzNCIsICJwcmVmZXJyZWRfdXNlcm5hbWUiOiAiSm9obiBEb2UiLCAic3ViIjogInN1YiJ9.signature", - "client_id": "mock_client_id", - "home_account_id": "uid.utid" - } - }, - "AppMetadata": { - "appmetadata-login.microsoftonline.com-mock_client_id": { - "environment": "login.microsoftonline.com", - "family_id": "1", - "client_id": "mock_client_id" - } - } -} +{"Account":{"uid.utid-login.microsoftonline.com-microsoft":{"username":"John Doe","local_account_id":"object1234","realm":"microsoft","environment":"login.microsoftonline.com","home_account_id":"uid.utid","authority_type":"MSSTS","client_info":"eyJ1aWQiOiJ1aWQiLCAidXRpZCI6InV0aWQifQ=="}},"RefreshToken":{"uid.utid-login.microsoftonline.com-refreshtoken-mock_client_id----":{"environment":"login.microsoftonline.com","credential_type":"RefreshToken","secret":"a refresh token","client_id":"mock_client_id","home_account_id":"uid.utid"},"uid.utid-login.microsoftonline.com-refreshtoken-1----":{"environment":"login.microsoftonline.com","credential_type":"RefreshToken","secret":"a refresh token","client_id":"mock_client_id","home_account_id":"uid.utid","family_id":"1"}},"AccessToken":{"uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope1 scope2 scope3--":{"environment":"login.microsoftonline.com","credential_type":"AccessToken","secret":"an access token","realm":"microsoft","target":"scope1 scope2 scope3","client_id":"mock_client_id","cached_at":"1000","home_account_id":"uid.utid","extended_expires_on":"4600","expires_on":"4600"},"uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope4 scope5--":{"environment":"login.microsoftonline.com","credential_type":"AccessToken","secret":"an access token","realm":"microsoft","target":"scope4 scope5","client_id":"mock_client_id","cached_at":"1000","home_account_id":"uid.utid","extended_expires_on":"4600","expires_on":"4600"},"uid.utid-login.microsoftonline.com-accesstoken_with_authscheme-mock_client_id-microsoft-scope4 scope5--pop":{"environment":"login.microsoftonline.com","credential_type":"AccessToken_With_AuthScheme","secret":"a POP-protected access token","realm":"microsoft","target":"scope4 scope5","client_id":"mock_client_id","cached_at":"1000","home_account_id":"uid.utid","extended_expires_on":"4600","expires_on":"4600","keyId":"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs","tokenType":"pop"}},"IdToken":{"uid.utid-login.microsoftonline.com-idtoken-mock_client_id-microsoft---":{"realm":"microsoft","environment":"login.microsoftonline.com","credential_type":"IdToken","secret":"header.eyJvaWQiOiAib2JqZWN0MTIzNCIsICJwcmVmZXJyZWRfdXNlcm5hbWUiOiAiSm9obiBEb2UiLCAic3ViIjogInN1YiJ9.signature","client_id":"mock_client_id","home_account_id":"uid.utid"}},"AppMetadata":{"appmetadata-login.microsoftonline.com-mock_client_id":{"environment":"login.microsoftonline.com","family_id":"1","client_id":"mock_client_id"}}} \ No newline at end of file From be45e1f955fd6892ed29d88d18103212e513ad5c Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 16 Jun 2025 13:52:10 -0700 Subject: [PATCH 10/12] Revert unintentional changes --- .../cache/cache-test-files/default-cache.json | 88 ++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/lib/msal-node/test/cache/cache-test-files/default-cache.json b/lib/msal-node/test/cache/cache-test-files/default-cache.json index f0e0e1d07f..40747cdef3 100644 --- a/lib/msal-node/test/cache/cache-test-files/default-cache.json +++ b/lib/msal-node/test/cache/cache-test-files/default-cache.json @@ -1 +1,87 @@ -{"Account":{"uid.utid-login.microsoftonline.com-microsoft":{"username":"John Doe","local_account_id":"object1234","realm":"microsoft","environment":"login.microsoftonline.com","home_account_id":"uid.utid","authority_type":"MSSTS","client_info":"eyJ1aWQiOiJ1aWQiLCAidXRpZCI6InV0aWQifQ=="}},"RefreshToken":{"uid.utid-login.microsoftonline.com-refreshtoken-mock_client_id----":{"environment":"login.microsoftonline.com","credential_type":"RefreshToken","secret":"a refresh token","client_id":"mock_client_id","home_account_id":"uid.utid"},"uid.utid-login.microsoftonline.com-refreshtoken-1----":{"environment":"login.microsoftonline.com","credential_type":"RefreshToken","secret":"a refresh token","client_id":"mock_client_id","home_account_id":"uid.utid","family_id":"1"}},"AccessToken":{"uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope1 scope2 scope3--":{"environment":"login.microsoftonline.com","credential_type":"AccessToken","secret":"an access token","realm":"microsoft","target":"scope1 scope2 scope3","client_id":"mock_client_id","cached_at":"1000","home_account_id":"uid.utid","extended_expires_on":"4600","expires_on":"4600"},"uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope4 scope5--":{"environment":"login.microsoftonline.com","credential_type":"AccessToken","secret":"an access token","realm":"microsoft","target":"scope4 scope5","client_id":"mock_client_id","cached_at":"1000","home_account_id":"uid.utid","extended_expires_on":"4600","expires_on":"4600"},"uid.utid-login.microsoftonline.com-accesstoken_with_authscheme-mock_client_id-microsoft-scope4 scope5--pop":{"environment":"login.microsoftonline.com","credential_type":"AccessToken_With_AuthScheme","secret":"a POP-protected access token","realm":"microsoft","target":"scope4 scope5","client_id":"mock_client_id","cached_at":"1000","home_account_id":"uid.utid","extended_expires_on":"4600","expires_on":"4600","keyId":"NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs","tokenType":"pop"}},"IdToken":{"uid.utid-login.microsoftonline.com-idtoken-mock_client_id-microsoft---":{"realm":"microsoft","environment":"login.microsoftonline.com","credential_type":"IdToken","secret":"header.eyJvaWQiOiAib2JqZWN0MTIzNCIsICJwcmVmZXJyZWRfdXNlcm5hbWUiOiAiSm9obiBEb2UiLCAic3ViIjogInN1YiJ9.signature","client_id":"mock_client_id","home_account_id":"uid.utid"}},"AppMetadata":{"appmetadata-login.microsoftonline.com-mock_client_id":{"environment":"login.microsoftonline.com","family_id":"1","client_id":"mock_client_id"}}} \ No newline at end of file +{ + "Account": { + "uid.utid-login.microsoftonline.com-microsoft": { + "username": "John Doe", + "local_account_id": "object1234", + "realm": "microsoft", + "environment": "login.microsoftonline.com", + "home_account_id": "uid.utid", + "authority_type": "MSSTS", + "client_info": "eyJ1aWQiOiJ1aWQiLCAidXRpZCI6InV0aWQifQ==" + } + }, + "RefreshToken": { + "uid.utid-login.microsoftonline.com-refreshtoken-mock_client_id----": { + "environment": "login.microsoftonline.com", + "credential_type": "RefreshToken", + "secret": "a refresh token", + "client_id": "mock_client_id", + "home_account_id": "uid.utid" + }, + "uid.utid-login.microsoftonline.com-refreshtoken-1----": { + "environment": "login.microsoftonline.com", + "credential_type": "RefreshToken", + "secret": "a refresh token", + "client_id": "mock_client_id", + "home_account_id": "uid.utid", + "family_id": "1" + } + }, + "AccessToken": { + "uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope1 scope2 scope3--": { + "environment": "login.microsoftonline.com", + "credential_type": "AccessToken", + "secret": "an access token", + "realm": "microsoft", + "target": "scope1 scope2 scope3", + "client_id": "mock_client_id", + "cached_at": "1000", + "home_account_id": "uid.utid", + "extended_expires_on": "4600", + "expires_on": "4600" + }, + "uid.utid-login.microsoftonline.com-accesstoken-mock_client_id-microsoft-scope4 scope5--": { + "environment": "login.microsoftonline.com", + "credential_type": "AccessToken", + "secret": "an access token", + "realm": "microsoft", + "target": "scope4 scope5", + "client_id": "mock_client_id", + "cached_at": "1000", + "home_account_id": "uid.utid", + "extended_expires_on": "4600", + "expires_on": "4600" + }, + "uid.utid-login.microsoftonline.com-accesstoken_with_authscheme-mock_client_id-microsoft-scope4 scope5--pop": { + "environment": "login.microsoftonline.com", + "credential_type": "AccessToken_With_AuthScheme", + "secret": "a POP-protected access token", + "realm": "microsoft", + "target": "scope4 scope5", + "client_id": "mock_client_id", + "cached_at": "1000", + "home_account_id": "uid.utid", + "extended_expires_on": "4600", + "expires_on": "4600", + "keyId": "NzbLsXh8uDCcd-6MNwXF4W_7noWXFZAfHkxZsRGC9Xs", + "tokenType": "pop" + } + }, + "IdToken": { + "uid.utid-login.microsoftonline.com-idtoken-mock_client_id-microsoft---": { + "realm": "microsoft", + "environment": "login.microsoftonline.com", + "credential_type": "IdToken", + "secret": "header.eyJvaWQiOiAib2JqZWN0MTIzNCIsICJwcmVmZXJyZWRfdXNlcm5hbWUiOiAiSm9obiBEb2UiLCAic3ViIjogInN1YiJ9.signature", + "client_id": "mock_client_id", + "home_account_id": "uid.utid" + } + }, + "AppMetadata": { + "appmetadata-login.microsoftonline.com-mock_client_id": { + "environment": "login.microsoftonline.com", + "family_id": "1", + "client_id": "mock_client_id" + } + } +} From 369052ce4a96ed03f100d1ebe3a5c0f3c4e76fb9 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 16 Jun 2025 13:58:18 -0700 Subject: [PATCH 11/12] Change files --- ...-msal-browser-1eb95f8f-3382-4c0d-880d-a81245933a08.json | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 change/@azure-msal-browser-1eb95f8f-3382-4c0d-880d-a81245933a08.json diff --git a/change/@azure-msal-browser-1eb95f8f-3382-4c0d-880d-a81245933a08.json b/change/@azure-msal-browser-1eb95f8f-3382-4c0d-880d-a81245933a08.json new file mode 100644 index 0000000000..8dc63934b2 --- /dev/null +++ b/change/@azure-msal-browser-1eb95f8f-3382-4c0d-880d-a81245933a08.json @@ -0,0 +1,7 @@ +{ + "type": "patch", + "comment": "Standalone functions in PCA #7810", + "packageName": "@azure/msal-browser", + "email": "hemoral@microsoft.com", + "dependentChangeType": "patch" +} From 9c43546481f0f3d65365212108d795de45995580 Mon Sep 17 00:00:00 2001 From: Hector Morales Date: Mon, 16 Jun 2025 14:07:11 -0700 Subject: [PATCH 12/12] Format fix --- .../BaseInteractionClient.ts | 92 +++++++------- .../src/interaction_client/PopupClient.ts | 8 +- .../src/interaction_client/RedirectClient.ts | 12 +- .../interaction_client/SilentCacheClient.ts | 17 +-- lib/msal-browser/src/utils/PopupUtils.ts | 24 ++-- .../test/app/PublicClientApplication.spec.ts | 19 ++- .../BaseInteractionClient.spec.ts | 12 +- .../interaction_client/PopupClient.spec.ts | 118 +++++++++--------- .../StandardInteractionClient.spec.ts | 17 +-- 9 files changed, 161 insertions(+), 158 deletions(-) diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index c7302e224d..050fa4dcc3 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -214,51 +214,51 @@ export async function getDiscoveredAuthority( } export async function clearCacheOnLogout( - browserStorage: BrowserCacheManager, - browserCrypto: ICrypto, - logger: Logger, - correlationId: string, - account?: AccountInfo | null, - ): Promise { - if (account) { - if ( - AccountEntityUtils.accountInfoIsEqual( - account, - browserStorage.getActiveAccount(), - false - ) - ) { - logger.verbose("Setting active account to null"); - browserStorage.setActiveAccount(null); - } - // Clear given account. - try { - browserStorage.removeAccount( - AccountEntityUtils.generateAccountCacheKey(account), - correlationId - ); - logger.verbose( - "Cleared cache items belonging to the account provided in the logout request." - ); - } catch (error) { - logger.error( - "Account provided in logout request was not found. Local cache unchanged." - ); - } - } else { - try { - logger.verbose( - "No account provided in logout request, clearing all cache items.", - correlationId - ); - // Clear all accounts and tokens - await browserStorage.clear(correlationId); - // Clear any stray keys from IndexedDB - await browserCrypto.clearKeystore(); - } catch (e) { - logger.error( - "Attempted to clear all MSAL cache items and failed. Local cache unchanged." - ); - } + browserStorage: BrowserCacheManager, + browserCrypto: ICrypto, + logger: Logger, + correlationId: string, + account?: AccountInfo | null +): Promise { + if (account) { + if ( + AccountEntityUtils.accountInfoIsEqual( + account, + browserStorage.getActiveAccount(), + false + ) + ) { + logger.verbose("Setting active account to null"); + browserStorage.setActiveAccount(null); + } + // Clear given account. + try { + browserStorage.removeAccount( + AccountEntityUtils.generateAccountCacheKey(account), + correlationId + ); + logger.verbose( + "Cleared cache items belonging to the account provided in the logout request." + ); + } catch (error) { + logger.error( + "Account provided in logout request was not found. Local cache unchanged." + ); + } + } else { + try { + logger.verbose( + "No account provided in logout request, clearing all cache items.", + correlationId + ); + // Clear all accounts and tokens + await browserStorage.clear(correlationId); + // Clear any stray keys from IndexedDB + await browserCrypto.clearKeystore(); + } catch (e) { + logger.error( + "Attempted to clear all MSAL cache items and failed. Local cache unchanged." + ); } } +} diff --git a/lib/msal-browser/src/interaction_client/PopupClient.ts b/lib/msal-browser/src/interaction_client/PopupClient.ts index 02556a4c86..8f6122a716 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -453,7 +453,13 @@ export class PopupClient extends StandardInteractionClient { this.logger, this.performanceClient, correlationId - )(popupWindow, popupParams.popupWindowParent, this.config, this.logger, this.unloadWindow); + )( + popupWindow, + popupParams.popupWindowParent, + this.config, + this.logger, + this.unloadWindow + ); const serverParams = invoke( ResponseHandler.deserializeResponse, diff --git a/lib/msal-browser/src/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 0edbcd7cb0..ce8ec0e84b 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -709,12 +709,12 @@ export class RedirectClient extends StandardInteractionClient { // Clear cache on logout await clearCacheOnLogout( - this.browserStorage, - this.browserCrypto, - this.logger, - this.correlationId, - validLogoutRequest.account - ); + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + validLogoutRequest.account + ); const navigationOptions: NavigationOptions = { apiId: ApiId.logout, diff --git a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts index de0e9d084d..f1fc63ae76 100644 --- a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts @@ -17,7 +17,10 @@ import { } from "../error/BrowserAuthError.js"; import { AuthenticationResult } from "../response/AuthenticationResult.js"; import { ClearCacheRequest } from "../request/ClearCacheRequest.js"; -import { clearCacheOnLogout, initializeServerTelemetryManager } from "./BaseInteractionClient.js"; +import { + clearCacheOnLogout, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; export class SilentCacheClient extends StandardInteractionClient { /** @@ -92,11 +95,11 @@ export class SilentCacheClient extends StandardInteractionClient { this.logger.verbose("logoutRedirect called"); const validLogoutRequest = this.initializeLogoutRequest(logoutRequest); return clearCacheOnLogout( - this.browserStorage, - this.browserCrypto, - this.logger, - this.correlationId, - validLogoutRequest.account - ); + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + validLogoutRequest.account + ); } } diff --git a/lib/msal-browser/src/utils/PopupUtils.ts b/lib/msal-browser/src/utils/PopupUtils.ts index 6eaaa08f4d..3cfc8b8f1e 100644 --- a/lib/msal-browser/src/utils/PopupUtils.ts +++ b/lib/msal-browser/src/utils/PopupUtils.ts @@ -5,7 +5,10 @@ import { Constants, Logger } from "@azure/msal-common"; import { BrowserConfiguration } from "../config/Configuration.js"; -import { BrowserAuthErrorCodes, createBrowserAuthError } from "../error/BrowserAuthError.js"; +import { + BrowserAuthErrorCodes, + createBrowserAuthError, +} from "../error/BrowserAuthError.js"; /** * Monitors a window until it loads a url with the same origin. @@ -20,9 +23,7 @@ export async function monitorPopupForHash( unloadWindow: (e: Event) => void ): Promise { return new Promise((resolve, reject) => { - logger.verbose( - "PopupHandler.monitorPopupForHash - polling started" - ); + logger.verbose("PopupHandler.monitorPopupForHash - polling started"); const intervalId = setInterval(() => { // Window is closed @@ -32,9 +33,7 @@ export async function monitorPopupForHash( ); clearInterval(intervalId); reject( - createBrowserAuthError( - BrowserAuthErrorCodes.userCancelled - ) + createBrowserAuthError(BrowserAuthErrorCodes.userCancelled) ); return; } @@ -80,13 +79,14 @@ export async function monitorPopupForHash( * Closes popup, removes any state vars created during popup calls. * @param popupWindow */ -export function cleanPopup(popupWindow: Window, popupWindowParent: Window, unloadWindow: (e: Event) => void): void { +export function cleanPopup( + popupWindow: Window, + popupWindowParent: Window, + unloadWindow: (e: Event) => void +): void { // Close window. popupWindow.close(); // Remove window unload function - popupWindowParent.removeEventListener( - "beforeunload", - unloadWindow - ); + popupWindowParent.removeEventListener("beforeunload", unloadWindow); } diff --git a/lib/msal-browser/test/app/PublicClientApplication.spec.ts b/lib/msal-browser/test/app/PublicClientApplication.spec.ts index 9ac26bbbf1..b1eee40892 100644 --- a/lib/msal-browser/test/app/PublicClientApplication.spec.ts +++ b/lib/msal-browser/test/app/PublicClientApplication.spec.ts @@ -3225,10 +3225,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as Constants.AuthenticationScheme, }; - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockRejectedValue("Not important for this test"); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockRejectedValue( + "Not important for this test" + ); try { await testPca.acquireTokenPopup(request); @@ -3283,10 +3282,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as Constants.AuthenticationScheme, }; - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockRejectedValue("Not important for this test"); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockRejectedValue( + "Not important for this test" + ); try { await testPca.acquireTokenPopup(request); } catch (e) {} @@ -6613,10 +6611,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { PopupClient.prototype, "openSizedPopup" ).mockReturnValue(popupWindow); - jest.spyOn( - PopupUtils, - "cleanPopup" - ).mockImplementation(); + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation(); }); it("Clears active account on logoutRedirect with no account", async () => { diff --git a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts index b2ba769096..5c65332d42 100644 --- a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts @@ -41,12 +41,12 @@ class testInteractionClient extends BaseInteractionClient { logout(request: EndSessionRequest): Promise { return clearCacheOnLogout( - this.browserStorage, - this.browserCrypto, - this.logger, - this.correlationId, - request.account - ); + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + request.account + ); } } diff --git a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index 3695cd95a2..201e27cb58 100644 --- a/lib/msal-browser/test/interaction_client/PopupClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts @@ -389,10 +389,7 @@ describe("PopupClient", () => { expect(requestUrl).toEqual(testNavUrl); return window; }); - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( TEST_HASHES.TEST_SUCCESS_NATIVE_ACCOUNT_ID_POPUP ); jest.spyOn( @@ -516,10 +513,7 @@ describe("PopupClient", () => { expect(requestUrl).toEqual(testNavUrl); return window; }); - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( TEST_HASHES.TEST_SUCCESS_NATIVE_ACCOUNT_ID_POPUP ); jest.spyOn( @@ -623,10 +617,9 @@ describe("PopupClient", () => { expect(requestUrl).toEqual(testNavUrl); return window; }); - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue(TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( + TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP + ); jest.spyOn( InteractionHandler.prototype, "handleCodeResponse" @@ -646,10 +639,7 @@ describe("PopupClient", () => { }); it("throws hash_empty_error if popup returns to redirectUri without a hash", (done) => { - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue(""); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue(""); popupClient .acquireToken({ @@ -667,10 +657,7 @@ describe("PopupClient", () => { }); it("throws hash_does_not_contain_known_properties error if popup returns to redirectUri with unrecognized params in the hash", (done) => { - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( "#fakeKey=fakeValue&anotherFakeKey=anotherFakeValue" ); @@ -697,10 +684,9 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( window ); - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue(TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( + TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP + ); jest.spyOn( FetchClient.prototype, "sendPostRequestAsync" @@ -894,10 +880,7 @@ describe("PopupClient", () => { .mockImplementation(() => { // Suppress navigation }); - jest.spyOn( - PopupUtils, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( `#ear_jwe=${validEarJWE}&state=${TEST_STATE_VALUES.TEST_STATE_POPUP}` ); @@ -1421,10 +1404,7 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn( - PopupUtils, - "cleanPopup" - ).mockImplementation(); + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation(); jest.spyOn( NavigationClient.prototype, "navigateInternal" @@ -1451,10 +1431,7 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn( - PopupUtils, - "cleanPopup" - ).mockImplementation(); + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation(); popupClient.logout().then(() => { done(); @@ -1505,13 +1482,11 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn(PopupUtils, "cleanPopup").mockImplementation( - (popup) => { - window.sessionStorage.removeItem( - `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}` - ); - } - ); + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation((popup) => { + window.sessionStorage.removeItem( + `${Constants.CACHE_PREFIX}.${TemporaryCacheKeys.INTERACTION_STATUS_KEY}` + ); + }); jest.spyOn( NavigationClient.prototype, "navigateInternal" @@ -1808,7 +1783,13 @@ describe("PopupClient", () => { closed: false, }; const clientImpl = popupClient as any; - PopupUtils.monitorPopupForHash(popup, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow).catch((error) => { + PopupUtils.monitorPopupForHash( + popup, + window, + clientImpl.config, + clientImpl.logger, + clientImpl.unloadWindow + ).catch((error) => { expect(error.errorCode).toEqual("user_cancelled"); done(); }); @@ -1830,7 +1811,13 @@ describe("PopupClient", () => { closed: false, }; const clientImpl = popupClient as any; - PopupUtils.monitorPopupForHash(popup, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow).then((hash) => { + PopupUtils.monitorPopupForHash( + popup, + window, + clientImpl.config, + clientImpl.logger, + clientImpl.unloadWindow + ).then((hash) => { expect(hash).toEqual("code=testCode"); done(); }); @@ -1886,12 +1873,17 @@ describe("PopupClient", () => { TEST_CONFIG.CORRELATION_ID ); const clientImpl = popupClient as any; - const result = await PopupUtils.monitorPopupForHash(popup as Window, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow) - .catch((e) => { - expect(e.errorCode).toEqual( - BrowserAuthErrorCodes.monitorPopupTimeout - ); - }); + const result = await PopupUtils.monitorPopupForHash( + popup as Window, + window, + clientImpl.config, + clientImpl.logger, + clientImpl.unloadWindow + ).catch((e) => { + expect(e.errorCode).toEqual( + BrowserAuthErrorCodes.monitorPopupTimeout + ); + }); }); it("returns hash", (done) => { @@ -1909,11 +1901,16 @@ describe("PopupClient", () => { }; const clientImpl = popupClient as any; - PopupUtils.monitorPopupForHash(popup as unknown as Window, window, clientImpl.config, clientImpl.logger, clientImpl.unloadWindow) - .then((hash: string) => { - expect(hash).toEqual("#code=hello"); - done(); - }); + PopupUtils.monitorPopupForHash( + popup as unknown as Window, + window, + clientImpl.config, + clientImpl.logger, + clientImpl.unloadWindow + ).then((hash: string) => { + expect(hash).toEqual("#code=hello"); + done(); + }); }); it("returns server code response in query form when responseMode in OIDCOptions is query", async () => { @@ -1994,11 +1991,10 @@ describe("PopupClient", () => { clientImpl.config, clientImpl.logger, clientImpl.unloadWindow - ) - .catch((error: AuthError) => { - expect(error.errorCode).toEqual("user_cancelled"); - done(); - }); + ).catch((error: AuthError) => { + expect(error.errorCode).toEqual("user_cancelled"); + done(); + }); }); }); diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index b62aee8de3..1b14207bc2 100644 --- a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts @@ -35,7 +35,10 @@ import * as PkceGenerator from "../../src/crypto/PkceGenerator.js"; import { FetchClient } from "../../src/network/FetchClient.js"; import { InteractionType } from "../../src/utils/BrowserConstants.js"; import { buildAccountFromIdTokenClaims } from "msal-test-utils"; -import { clearCacheOnLogout, getDiscoveredAuthority } from "../../src/interaction_client/BaseInteractionClient.js"; +import { + clearCacheOnLogout, + getDiscoveredAuthority, +} from "../../src/interaction_client/BaseInteractionClient.js"; class testStandardInteractionClient extends StandardInteractionClient { acquireToken(): Promise { @@ -74,12 +77,12 @@ class testStandardInteractionClient extends StandardInteractionClient { logout(request: EndSessionRequest): Promise { return clearCacheOnLogout( - this.browserStorage, - this.browserCrypto, - this.logger, - this.correlationId, - request.account - ); + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + request.account + ); } }