Skip to content

Commit

Permalink
Move build request functions (#6951)
Browse files Browse the repository at this point in the history
- Move initializeBaseRequest and initializeSilentRequest out of the
InteractionClient classes
- Move implementation for hashString from CryptoOps to BrowserCrypto

These changes pave the way for better handling of concurrent iframe
requests
  • Loading branch information
tnorling committed Mar 15, 2024
1 parent b3f0e72 commit 69df8f6
Show file tree
Hide file tree
Showing 11 changed files with 191 additions and 161 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "move build request functions",
"packageName": "@azure/msal-browser",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch"
}
27 changes: 14 additions & 13 deletions lib/msal-browser/src/controllers/StandardController.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,7 @@ import { IController } from "./IController";
import { AuthenticationResult } from "../response/AuthenticationResult";
import { ClearCacheRequest } from "../request/ClearCacheRequest";
import { createNewGuid } from "../crypto/BrowserCrypto";
import { initializeSilentRequest } from "../request/RequestHelpers";

export class StandardController implements IController {
// OperatingContext
Expand Down Expand Up @@ -1072,7 +1073,6 @@ export class StandardController implements IController {
* @returns A promise that, when resolved, returns the access token
*/
protected async acquireTokenFromCache(
silentCacheClient: SilentCacheClient,
commonRequest: CommonSilentFlowRequest,
cacheLookupPolicy: CacheLookupPolicy
): Promise<AuthenticationResult> {
Expand All @@ -1084,6 +1084,9 @@ export class StandardController implements IController {
case CacheLookupPolicy.Default:
case CacheLookupPolicy.AccessToken:
case CacheLookupPolicy.AccessTokenAndRefreshToken:
const silentCacheClient = this.createSilentCacheClient(
commonRequest.correlationId
);
return invokeAsync(
silentCacheClient.acquireToken.bind(silentCacheClient),
PerformanceEvents.SilentCacheClientAcquireToken,
Expand Down Expand Up @@ -1939,7 +1942,7 @@ export class StandardController implements IController {
* @returns {Promise.<AuthenticationResult>} - a promise that is fulfilled when this function has completed, or rejected if an error was raised. Returns the {@link AuthResponse}
*/
protected async acquireTokenSilentAsync(
request: SilentRequest,
request: SilentRequest & { correlationId: string },
account: AccountInfo
): Promise<AuthenticationResult> {
this.performanceClient.addQueueMeasurement(
Expand Down Expand Up @@ -2004,19 +2007,19 @@ export class StandardController implements IController {
"acquireTokenSilent - attempting to acquire token from web flow"
);

const silentCacheClient = this.createSilentCacheClient(
request.correlationId
);

const silentRequest = await invokeAsync(
silentCacheClient.initializeSilentRequest.bind(
silentCacheClient
),
initializeSilentRequest,
PerformanceEvents.InitializeSilentRequest,
this.logger,
this.performanceClient,
request.correlationId
)(request, account);
)(
request,
account,
this.config,
this.performanceClient,
this.logger
);

const cacheLookupPolicy =
request.cacheLookupPolicy || CacheLookupPolicy.Default;
Expand All @@ -2027,7 +2030,7 @@ export class StandardController implements IController {
this.logger,
this.performanceClient,
silentRequest.correlationId
)(silentCacheClient, silentRequest, cacheLookupPolicy).catch(
)(silentRequest, cacheLookupPolicy).catch(
(cacheError: AuthError) => {
if (
request.cacheLookupPolicy ===
Expand All @@ -2036,8 +2039,6 @@ export class StandardController implements IController {
throw cacheError;
}

// block the reload if it occurred inside a hidden iframe
BrowserUtils.blockReloadInHiddenIframes();
this.eventHandler.emitEvent(
EventType.ACQUIRE_TOKEN_NETWORK_START,
InteractionType.Silent,
Expand Down
11 changes: 11 additions & 0 deletions lib/msal-browser/src/crypto/BrowserCrypto.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ import {
PerformanceEvents,
} from "@azure/msal-common";
import { KEY_FORMAT_JWK } from "../utils/BrowserConstants";
import { urlEncodeArr } from "../encode/Base64Encode";

/**
* This file defines functions used by the browser library to perform cryptography operations such as
Expand Down Expand Up @@ -202,3 +203,13 @@ export async function sign(
data
) as Promise<ArrayBuffer>;
}

/**
* Returns the SHA-256 hash of an input string
* @param plainText
*/
export async function hashString(plainText: string): Promise<string> {
const hashBuffer: ArrayBuffer = await sha256Digest(plainText);
const hashBytes = new Uint8Array(hashBuffer);
return urlEncodeArr(hashBytes);
}
6 changes: 1 addition & 5 deletions lib/msal-browser/src/crypto/CryptoOps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -249,11 +249,7 @@ export class CryptoOps implements ICrypto {
* @param plainText
*/
async hashString(plainText: string): Promise<string> {
const hashBuffer: ArrayBuffer = await BrowserCrypto.sha256Digest(
plainText
);
const hashBytes = new Uint8Array(hashBuffer);
return urlEncodeArr(hashBytes);
return BrowserCrypto.hashString(plainText);
}
}

Expand Down
66 changes: 0 additions & 66 deletions lib/msal-browser/src/interaction_client/BaseInteractionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@ import {
Logger,
AccountInfo,
AccountEntity,
BaseAuthRequest,
AuthenticationScheme,
UrlString,
ServerTelemetryManager,
ServerTelemetryRequest,
Expand All @@ -21,7 +19,6 @@ import {
AuthorityFactory,
IPerformanceClient,
PerformanceEvents,
StringUtils,
AzureCloudOptions,
invokeAsync,
} from "@azure/msal-common";
Expand Down Expand Up @@ -133,69 +130,6 @@ export abstract class BaseInteractionClient {
}
}

/**
* Initializer function for all request APIs
* @param request
*/
protected async initializeBaseRequest(
request: Partial<BaseAuthRequest>
): Promise<BaseAuthRequest> {
this.performanceClient.addQueueMeasurement(
PerformanceEvents.InitializeBaseRequest,
this.correlationId
);
const authority = request.authority || this.config.auth.authority;

const scopes = [...((request && request.scopes) || [])];

const validatedRequest: BaseAuthRequest = {
...request,
correlationId: this.correlationId,
authority,
scopes,
};

// Set authenticationScheme to BEARER if not explicitly set in the request
if (!validatedRequest.authenticationScheme) {
validatedRequest.authenticationScheme = AuthenticationScheme.BEARER;
this.logger.verbose(
'Authentication Scheme wasn\'t explicitly set in request, defaulting to "Bearer" request'
);
} else {
if (
validatedRequest.authenticationScheme ===
AuthenticationScheme.SSH
) {
if (!request.sshJwk) {
throw createClientConfigurationError(
ClientConfigurationErrorCodes.missingSshJwk
);
}
if (!request.sshKid) {
throw createClientConfigurationError(
ClientConfigurationErrorCodes.missingSshKid
);
}
}
this.logger.verbose(
`Authentication Scheme set to "${validatedRequest.authenticationScheme}" as configured in Auth request`
);
}

// Set requested claims hash if claims-based caching is enabled and claims were requested
if (
this.config.cache.claimsBasedCachingEnabled &&
request.claims &&
// Checks for empty stringified object "{}" which doesn't qualify as requested claims
!StringUtils.isEmptyObj(request.claims)
) {
validatedRequest.requestedClaimsHash =
await this.browserCrypto.hashString(request.claims);
}

return validatedRequest;
}

/**
*
* Use to get the redirect uri configured in MSAL or null.
Expand Down
62 changes: 11 additions & 51 deletions lib/msal-browser/src/interaction_client/SilentCacheClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,9 @@ import { StandardInteractionClient } from "./StandardInteractionClient";
import {
CommonSilentFlowRequest,
SilentFlowClient,
ServerTelemetryManager,
AccountInfo,
AzureCloudOptions,
PerformanceEvents,
invokeAsync,
} from "@azure/msal-common";
import { SilentRequest } from "../request/SilentRequest";
import { ApiId } from "../utils/BrowserConstants";
import {
BrowserAuthError,
Expand All @@ -39,12 +35,22 @@ export class SilentCacheClient extends StandardInteractionClient {
ApiId.acquireTokenSilent_silentFlow
);

const silentAuthClient = await this.createSilentFlowClient(
const clientConfig = await invokeAsync(
this.getClientConfiguration.bind(this),
PerformanceEvents.StandardInteractionClientGetClientConfiguration,
this.logger,
this.performanceClient,
this.correlationId
)(
serverTelemetryManager,
silentRequest.authority,
silentRequest.azureCloudOptions,
silentRequest.account
);
const silentAuthClient = new SilentFlowClient(
clientConfig,
this.performanceClient
);
this.logger.verbose("Silent auth client created");

try {
Expand Down Expand Up @@ -86,50 +92,4 @@ export class SilentCacheClient extends StandardInteractionClient {
const validLogoutRequest = this.initializeLogoutRequest(logoutRequest);
return this.clearCacheOnLogout(validLogoutRequest?.account);
}

/**
* Creates an Silent Flow Client with the given authority, or the default authority.
* @param serverTelemetryManager
* @param authorityUrl
*/
protected async createSilentFlowClient(
serverTelemetryManager: ServerTelemetryManager,
authorityUrl?: string,
azureCloudOptions?: AzureCloudOptions,
account?: AccountInfo
): Promise<SilentFlowClient> {
// Create auth module.
const clientConfig = await invokeAsync(
this.getClientConfiguration.bind(this),
PerformanceEvents.StandardInteractionClientGetClientConfiguration,
this.logger,
this.performanceClient,
this.correlationId
)(serverTelemetryManager, authorityUrl, azureCloudOptions, account);
return new SilentFlowClient(clientConfig, this.performanceClient);
}

async initializeSilentRequest(
request: SilentRequest,
account: AccountInfo
): Promise<CommonSilentFlowRequest> {
this.performanceClient.addQueueMeasurement(
PerformanceEvents.InitializeSilentRequest,
this.correlationId
);

const baseRequest = await invokeAsync(
this.initializeBaseRequest.bind(this),
PerformanceEvents.InitializeBaseRequest,
this.logger,
this.performanceClient,
this.correlationId
)(request);
return {
...request,
...baseRequest,
account: account,
forceRefresh: request.forceRefresh || false,
};
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import {
BrowserAuthErrorCodes,
} from "../error/BrowserAuthError";
import { AuthenticationResult } from "../response/AuthenticationResult";
import { initializeBaseRequest } from "../request/RequestHelpers";

export class SilentRefreshClient extends StandardInteractionClient {
/**
Expand All @@ -35,12 +36,12 @@ export class SilentRefreshClient extends StandardInteractionClient {
);

const baseRequest = await invokeAsync(
this.initializeBaseRequest.bind(this),
initializeBaseRequest,
PerformanceEvents.InitializeBaseRequest,
this.logger,
this.performanceClient,
request.correlationId
)(request);
)(request, this.config, this.performanceClient, this.logger);
const silentRequest: CommonSilentFlowRequest = {
...request,
...baseRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ import { PopupRequest } from "../request/PopupRequest";
import { SsoSilentRequest } from "../request/SsoSilentRequest";
import { generatePkceCodes } from "../crypto/PkceGenerator";
import { createNewGuid } from "../crypto/BrowserCrypto";
import { initializeBaseRequest } from "../request/RequestHelpers";

/**
* Defines the class structure and helper functions used by the "standard", non-brokered auth flows (popup, redirect, silent (RT), silent (iframe))
Expand Down Expand Up @@ -315,12 +316,17 @@ export abstract class StandardInteractionClient extends BaseInteractionClient {
);

const baseRequest: BaseAuthRequest = await invokeAsync(
this.initializeBaseRequest.bind(this),
initializeBaseRequest,
PerformanceEvents.InitializeBaseRequest,
this.logger,
this.performanceClient,
this.correlationId
)(request);
)(
{ ...request, correlationId: this.correlationId },
this.config,
this.performanceClient,
this.logger
);

const validatedRequest: AuthorizationUrlRequest = {
...baseRequest,
Expand Down
Loading

0 comments on commit 69df8f6

Please sign in to comment.