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" +} diff --git a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts index 96eb5a591d..050fa4dcc3 100644 --- a/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/BaseInteractionClient.ts @@ -85,168 +85,180 @@ 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 { - this.browserStorage.removeAccount( - AccountEntityUtils.generateAccountCacheKey(account), - this.correlationId - ); - 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 - this.browserStorage.clear(this.correlationId); - // 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." - ); - } - } - } +/** + * + * 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()); +} - /** - * - * 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 + * @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], + }; - /** - * - * @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 - ); - } + 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; - * } - */ - protected async getDiscoveredAuthority(params: { +/** + * 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; - }): 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, - BrowserPerformanceEvents.AuthorityFactoryCreateDiscoveredInstance, - this.logger, - this.performanceClient, - this.correlationId - )( - builtAuthority, - this.config.system.networkClient, - this.browserStorage, - authorityOptions, - this.logger, - this.correlationId, - this.performanceClient + }, + 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, + BrowserPerformanceEvents.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; +} - if (account && !discoveredAuthority.isAlias(account.environment)) { - throw createClientConfigurationError( - ClientConfigurationErrorCodes.authorityMismatch +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." ); } - - return discoveredAuthority; } } diff --git a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts index a401585163..dbb852ca52 100644 --- a/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/PlatformAuthInteractionClient.ts @@ -34,8 +34,13 @@ import { Constants, PerformanceEvents, } from "@azure/msal-common/browser"; +import { + BaseInteractionClient, + getDiscoveredAuthority, + getRedirectUri, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; import * as BrowserPerformanceEvents from "../telemetry/BrowserPerformanceEvents.js"; -import { BaseInteractionClient } from "./BaseInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -161,8 +166,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,8 +326,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 = - this.initializeServerTelemetryManager(this.apiId); + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId, + this.browserStorage, + this.logger + ); serverTelemetryManager.setNativeBrokerErrorCode(e.errorCode); if (isFatalNativeAuthError(e)) { throw e; @@ -338,7 +352,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, @@ -407,8 +421,13 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { reqTimestamp ); - const serverTelemetryManager = - this.initializeServerTelemetryManager(this.apiId); + const serverTelemetryManager = initializeServerTelemetryManager( + this.apiId, + this.config, + this.correlationId, + this.browserStorage, + this.logger + ); serverTelemetryManager.clearNativeBrokerErrorCode(); return authResult; } catch (e) { @@ -473,9 +492,16 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { } // Get the preferred_cache domain for the given authority - const authority = await this.getDiscoveredAuthority({ - requestAuthority: request.authority, - }); + const authority = await getDiscoveredAuthority( + { + requestAuthority: request.authority, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const baseAccount = buildAccountToCache( this.browserStorage, @@ -881,7 +907,11 @@ 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, @@ -956,11 +986,18 @@ export class PlatformAuthInteractionClient extends BaseInteractionClient { if (request.account) { // validate authority - await this.getDiscoveredAuthority({ - requestAuthority, - requestAzureCloudOptions: request.azureCloudOptions, - account: request.account, - }); + 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 96aa834473..8f6122a716 100644 --- a/lib/msal-browser/src/interaction_client/PopupClient.ts +++ b/lib/msal-browser/src/interaction_client/PopupClient.ts @@ -19,8 +19,11 @@ import { PkceCodes, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import * as BrowserPerformanceEvents from "../telemetry/BrowserPerformanceEvents.js"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; import { EventType } from "../event/EventType.js"; import { InteractionType, @@ -48,6 +51,12 @@ 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 { + clearCacheOnLogout, + getDiscoveredAuthority, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; +import { monitorPopupForHash } from "../utils/PopupUtils.js"; export type PopupParams = { popup?: Window | null; @@ -212,12 +221,21 @@ export class PopupClient extends StandardInteractionClient { this.logger.verbose("acquireTokenPopupAsync called"); const validRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, BrowserPerformanceEvents.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 @@ -255,8 +273,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 = @@ -318,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( @@ -378,17 +403,24 @@ 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, BrowserPerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }); + )( + { + 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, @@ -416,12 +448,18 @@ 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, @@ -478,13 +516,23 @@ 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 { // 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( @@ -559,9 +607,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 }); @@ -636,72 +687,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 * @@ -850,21 +835,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/interaction_client/RedirectClient.ts b/lib/msal-browser/src/interaction_client/RedirectClient.ts index 7a88256d56..ce8ec0e84b 100644 --- a/lib/msal-browser/src/interaction_client/RedirectClient.ts +++ b/lib/msal-browser/src/interaction_client/RedirectClient.ts @@ -20,8 +20,11 @@ import { InProgressPerformanceEvent, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import * as BrowserPerformanceEvents from "../telemetry/BrowserPerformanceEvents.js"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; import { ApiId, INTERACTION_TYPE, @@ -49,6 +52,11 @@ 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 { + clearCacheOnLogout, + getDiscoveredAuthority, + initializeServerTelemetryManager, +} from "./BaseInteractionClient.js"; function getNavigationType(): NavigationTimingType | undefined { if ( @@ -101,12 +109,21 @@ export class RedirectClient extends StandardInteractionClient { */ async acquireToken(request: RedirectRequest): Promise { const validRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, BrowserPerformanceEvents.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, @@ -167,8 +184,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( @@ -241,17 +262,24 @@ 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, BrowserPerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }); + )( + { + 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, @@ -290,8 +318,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 { @@ -519,17 +551,24 @@ export class RedirectClient extends StandardInteractionClient { if (serverParams.ear_jwe) { const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, BrowserPerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, request.correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }); + )( + { + 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, BrowserPerformanceEvents.HandleResponseEar, @@ -653,8 +692,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 { @@ -665,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/SilentAuthCodeClient.ts b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts index 49fd4bd9e4..b7ed35ab21 100644 --- a/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentAuthCodeClient.ts @@ -13,8 +13,11 @@ import { invokeAsync, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import * as BrowserPerformanceEvents from "../telemetry/BrowserPerformanceEvents.js"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -29,6 +32,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; @@ -75,15 +79,28 @@ export class SilentAuthCodeClient extends StandardInteractionClient { // Create silent request const silentRequest: CommonAuthorizationUrlRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, BrowserPerformanceEvents.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 = 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 96051044bd..f1fc63ae76 100644 --- a/lib/msal-browser/src/interaction_client/SilentCacheClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentCacheClient.ts @@ -17,6 +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"; export class SilentCacheClient extends StandardInteractionClient { /** @@ -27,8 +31,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( @@ -86,6 +94,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/src/interaction_client/SilentIframeClient.ts b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts index bb90228d56..396bf59c89 100644 --- a/lib/msal-browser/src/interaction_client/SilentIframeClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentIframeClient.ts @@ -16,8 +16,11 @@ import { ProtocolMode, CommonAuthorizationUrlRequest, } from "@azure/msal-common/browser"; +import { + initializeAuthorizationRequest, + StandardInteractionClient, +} from "./StandardInteractionClient.js"; import * as BrowserPerformanceEvents from "../telemetry/BrowserPerformanceEvents.js"; -import { StandardInteractionClient } from "./StandardInteractionClient.js"; import { BrowserConfiguration } from "../config/Configuration.js"; import { BrowserCacheManager } from "../cache/BrowserCacheManager.js"; import { EventHandler } from "../event/EventHandler.js"; @@ -45,6 +48,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"; export class SilentIframeClient extends StandardInteractionClient { protected apiId: ApiId; @@ -114,12 +121,21 @@ export class SilentIframeClient extends StandardInteractionClient { // Create silent request const silentRequest: CommonAuthorizationUrlRequest = await invokeAsync( - this.initializeAuthorizationRequest.bind(this), + initializeAuthorizationRequest, BrowserPerformanceEvents.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, @@ -144,8 +160,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 { @@ -211,17 +231,24 @@ export class SilentIframeClient extends StandardInteractionClient { ): Promise { const correlationId = request.correlationId; const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, BrowserPerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, correlationId - )({ - requestAuthority: request.authority, - requestAzureCloudOptions: request.azureCloudOptions, - requestExtraQueryParameters: request.extraQueryParameters, - account: request.account, - }); + )( + { + 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 571b7a64b4..756a3e3138 100644 --- a/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts +++ b/lib/msal-browser/src/interaction_client/SilentRefreshClient.ts @@ -22,6 +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"; export class SilentRefreshClient extends StandardInteractionClient { /** @@ -45,13 +49,19 @@ 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 ); } - 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({ diff --git a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts index 6d8bb3dfde..b2adce17db 100644 --- a/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts +++ b/lib/msal-browser/src/interaction_client/StandardInteractionClient.ts @@ -17,9 +17,16 @@ import { BaseAuthRequest, StringDict, CommonAuthorizationUrlRequest, + ICrypto, + Logger, + IPerformanceClient, } from "@azure/msal-common/browser"; +import { + BaseInteractionClient, + getDiscoveredAuthority, + getRedirectUri, +} from "./BaseInteractionClient.js"; import * as BrowserPerformanceEvents from "../telemetry/BrowserPerformanceEvents.js"; -import { BaseInteractionClient } from "./BaseInteractionClient.js"; import { BrowserConstants, InteractionType, @@ -33,6 +40,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)) @@ -223,17 +232,24 @@ export abstract class StandardInteractionClient extends BaseInteractionClient { } = params; const discoveredAuthority = await invokeAsync( - this.getDiscoveredAuthority.bind(this), + getDiscoveredAuthority, BrowserPerformanceEvents.StandardInteractionClientGetDiscoveredAuthority, this.logger, this.performanceClient, this.correlationId - )({ - requestAuthority, - requestAzureCloudOptions, - requestExtraQueryParameters, - account, - }); + )( + { + requestAuthority, + requestAzureCloudOptions, + requestExtraQueryParameters, + account, + }, + this.config, + this.correlationId, + this.performanceClient, + this.browserStorage, + this.logger + ); const logger = this.config.system.loggerOptions; return { @@ -267,66 +283,68 @@ 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 = this.getRedirectUri(request.redirectUri); - const browserState: BrowserStateObject = { - interactionType: interactionType, - }; - const state = ProtocolUtils.setRequestState( - this.browserCrypto, - (request && request.state) || "", - browserState - ); - - const baseRequest: BaseAuthRequest = await invokeAsync( - initializeBaseRequest, - BrowserPerformanceEvents.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, + BrowserPerformanceEvents.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; } diff --git a/lib/msal-browser/src/utils/PopupUtils.ts b/lib/msal-browser/src/utils/PopupUtils.ts new file mode 100644 index 0000000000..3cfc8b8f1e --- /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..b1eee40892 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"; @@ -3224,10 +3225,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as Constants.AuthenticationScheme, }; - jest.spyOn( - PopupClient.prototype, - "monitorPopupForHash" - ).mockRejectedValue("Not important for this test"); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockRejectedValue( + "Not important for this test" + ); try { await testPca.acquireTokenPopup(request); @@ -3282,10 +3282,9 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { TEST_CONFIG.TOKEN_TYPE_BEARER as Constants.AuthenticationScheme, }; - jest.spyOn( - PopupClient.prototype, - "monitorPopupForHash" - ).mockRejectedValue("Not important for this test"); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockRejectedValue( + "Not important for this test" + ); try { await testPca.acquireTokenPopup(request); } catch (e) {} @@ -6612,10 +6611,7 @@ describe("PublicClientApplication.ts Class Unit Tests", () => { PopupClient.prototype, "openSizedPopup" ).mockReturnValue(popupWindow); - jest.spyOn( - PopupClient.prototype, - "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 7fb86d53e0..5c65332d42 100644 --- a/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts +++ b/lib/msal-browser/test/interaction_client/BaseInteractionClient.spec.ts @@ -22,7 +22,11 @@ import { ID_TOKEN_CLAIMS, ID_TOKEN_ALT_CLAIMS, } from "../utils/StringConstants.js"; -import { BaseInteractionClient } from "../../src/interaction_client/BaseInteractionClient.js"; +import { + BaseInteractionClient, + clearCacheOnLogout, + getDiscoveredAuthority, +} from "../../src/interaction_client/BaseInteractionClient.js"; import { EndSessionRequest, PublicClientApplication, @@ -36,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 + ); } } @@ -244,13 +254,20 @@ 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 +289,20 @@ 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 +340,17 @@ 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 +388,19 @@ 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/PopupClient.spec.ts b/lib/msal-browser/test/interaction_client/PopupClient.spec.ts index d19264aa40..201e27cb58 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"; @@ -388,10 +389,7 @@ describe("PopupClient", () => { expect(requestUrl).toEqual(testNavUrl); return window; }); - jest.spyOn( - PopupClient.prototype, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( TEST_HASHES.TEST_SUCCESS_NATIVE_ACCOUNT_ID_POPUP ); jest.spyOn( @@ -515,10 +513,7 @@ describe("PopupClient", () => { expect(requestUrl).toEqual(testNavUrl); return window; }); - jest.spyOn( - PopupClient.prototype, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( TEST_HASHES.TEST_SUCCESS_NATIVE_ACCOUNT_ID_POPUP ); jest.spyOn( @@ -622,10 +617,9 @@ describe("PopupClient", () => { expect(requestUrl).toEqual(testNavUrl); return window; }); - jest.spyOn( - PopupClient.prototype, - "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" @@ -645,10 +639,7 @@ describe("PopupClient", () => { }); it("throws hash_empty_error if popup returns to redirectUri without a hash", (done) => { - jest.spyOn( - PopupClient.prototype, - "monitorPopupForHash" - ).mockResolvedValue(""); + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue(""); popupClient .acquireToken({ @@ -666,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( - PopupClient.prototype, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( "#fakeKey=fakeValue&anotherFakeKey=anotherFakeValue" ); @@ -696,10 +684,9 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( window ); - jest.spyOn( - PopupClient.prototype, - "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" @@ -893,10 +880,7 @@ describe("PopupClient", () => { .mockImplementation(() => { // Suppress navigation }); - jest.spyOn( - PopupClient.prototype, - "monitorPopupForHash" - ).mockResolvedValue( + jest.spyOn(PopupUtils, "monitorPopupForHash").mockResolvedValue( `#ear_jwe=${validEarJWE}&state=${TEST_STATE_VALUES.TEST_STATE_POPUP}` ); @@ -1420,10 +1404,7 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn( - PopupClient.prototype, - "cleanPopup" - ).mockImplementation(); + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation(); jest.spyOn( NavigationClient.prototype, "navigateInternal" @@ -1450,10 +1431,7 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn( - PopupClient.prototype, - "cleanPopup" - ).mockImplementation(); + jest.spyOn(PopupUtils, "cleanPopup").mockImplementation(); popupClient.logout().then(() => { done(); @@ -1504,13 +1482,11 @@ describe("PopupClient", () => { jest.spyOn(PopupClient.prototype, "openPopup").mockReturnValue( popupWindow ); - jest.spyOn(PopupClient.prototype, "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" @@ -1806,7 +1782,14 @@ 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 +1810,14 @@ 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,14 +1872,18 @@ describe("PopupClient", () => { undefined, TEST_CONFIG.CORRELATION_ID ); - - const result = await popupClient - .monitorPopupForHash(popup as Window, window) - .catch((e) => { - expect(e.errorCode).toEqual( - BrowserAuthErrorCodes.monitorPopupTimeout - ); - }); + 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 + ); + }); }); it("returns hash", (done) => { @@ -1906,12 +1900,17 @@ describe("PopupClient", () => { close: () => {}, }; - popupClient - .monitorPopupForHash(popup as unknown as Window, window) - .then((hash: string) => { - expect(hash).toEqual("#code=hello"); - done(); - }); + 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(); + }); }); it("returns server code response in query form when responseMode in OIDCOptions is query", async () => { @@ -1964,9 +1963,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,12 +1984,17 @@ describe("PopupClient", () => { closed: true, }; - popupClient - .monitorPopupForHash(popup as unknown as Window, window) - .catch((error: AuthError) => { - expect(error.errorCode).toEqual("user_cancelled"); - done(); - }); + 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-browser/test/interaction_client/SilentIframeClient.spec.ts b/lib/msal-browser/test/interaction_client/SilentIframeClient.spec.ts index ce32f250aa..c98c2ab8d2 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(() => { @@ -168,8 +172,7 @@ describe("SilentIframeClient", () => { ); const initializeAuthorizationRequestSpy = jest.spyOn( - SilentIframeClient.prototype, - // @ts-ignore + StandardInteractionClientExports, "initializeAuthorizationRequest" ); const tokenResp = await silentIframeClient.acquireToken({ @@ -178,13 +181,25 @@ describe("SilentIframeClient", () => { 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 +921,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 +1023,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", @@ -1124,11 +1129,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", diff --git a/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts b/lib/msal-browser/test/interaction_client/StandardInteractionClient.spec.ts index 60c086ff51..1b14207bc2 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 { 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, @@ -32,6 +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"; class testStandardInteractionClient extends StandardInteractionClient { acquireToken(): Promise { @@ -42,18 +49,40 @@ 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: { 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 { - return this.clearCacheOnLogout(request.account); + return clearCacheOnLogout( + this.browserStorage, + this.browserCrypto, + this.logger, + this.correlationId, + request.account + ); } }