Skip to content

Commit

Permalink
Reduce calls to resolveEndpointsAsync (#6838)
Browse files Browse the repository at this point in the history
We are building an `Authority` object and parsing/validating endpoint
metadata at least twice in a single request - once to validate it
matches the account object passed in and again to build the final
request object. This PR refactors to do both these steps at the same
time using a single `Authority` object. Additionally, telemetry is added
to track the number of times an API is invoked over the course of a
request to identify redundancies easier.
  • Loading branch information
tnorling authored Jan 25, 2024
1 parent 77c4f3d commit 813bda7
Show file tree
Hide file tree
Showing 13 changed files with 133 additions and 106 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "reduce number of calls to resolveEndpoints",
"packageName": "@azure/msal-browser",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Track number of times an API is invoked in a single request",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
71 changes: 40 additions & 31 deletions lib/msal-browser/src/interaction_client/BaseInteractionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
IPerformanceClient,
PerformanceEvents,
StringUtils,
AzureCloudOptions,
invokeAsync,
} from "@azure/msal-common";
import { BrowserConfiguration } from "../config/Configuration";
import { BrowserCacheManager } from "../cache/BrowserCacheManager";
Expand Down Expand Up @@ -136,19 +138,14 @@ export abstract class BaseInteractionClient {
* @param request
*/
protected async initializeBaseRequest(
request: Partial<BaseAuthRequest>,
account?: AccountInfo
request: Partial<BaseAuthRequest>
): Promise<BaseAuthRequest> {
this.performanceClient.addQueueMeasurement(
PerformanceEvents.InitializeBaseRequest,
this.correlationId
);
const authority = request.authority || this.config.auth.authority;

if (account) {
await this.validateRequestAuthority(authority, account);
}

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

const validatedRequest: BaseAuthRequest = {
Expand Down Expand Up @@ -218,25 +215,6 @@ export abstract class BaseInteractionClient {
);
}

/*
* If authority provided in the request does not match environment/authority specified
* in the account or MSAL config, we throw an error.
*/
async validateRequestAuthority(
authority: string,
account: AccountInfo
): Promise<void> {
const discoveredAuthority = await this.getDiscoveredAuthority(
authority
);

if (!discoveredAuthority.isAlias(account.environment)) {
throw createClientConfigurationError(
ClientConfigurationErrorCodes.authorityMismatch
);
}
}

/**
*
* @param apiId
Expand Down Expand Up @@ -266,29 +244,60 @@ export abstract class BaseInteractionClient {
/**
* Used to get a discovered version of the default authority.
* @param requestAuthority
* @param requestAzureCloudOptions
* @param account
*/
protected async getDiscoveredAuthority(
requestAuthority?: string
requestAuthority?: string,
requestAzureCloudOptions?: AzureCloudOptions,
account?: AccountInfo
): Promise<Authority> {
this.logger.verbose("getDiscoveredAuthority called");
this.performanceClient.addQueueMeasurement(
PerformanceEvents.StandardInteractionClientGetDiscoveredAuthority,
this.correlationId
);
const authorityOptions: AuthorityOptions = {
protocolMode: this.config.auth.protocolMode,
OIDCOptions: this.config.auth.OIDCOptions,
knownAuthorities: this.config.auth.knownAuthorities,
cloudDiscoveryMetadata: this.config.auth.cloudDiscoveryMetadata,
authorityMetadata: this.config.auth.authorityMetadata,
skipAuthorityMetadataCache:
this.config.auth.skipAuthorityMetadataCache,
};

const authority = requestAuthority || this.config.auth.authority;
this.logger.verbose(`Creating discovered authority with ${authority}`);
return AuthorityFactory.createDiscoveredInstance(
authority,
// build authority string based on auth params, precedence - azureCloudInstance + tenant >> authority
const userAuthority = requestAuthority
? requestAuthority
: this.config.auth.authority;

// fall back to the authority from config
const builtAuthority = Authority.generateAuthority(
userAuthority,
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;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -809,7 +809,12 @@ export class NativeInteractionClient extends BaseInteractionClient {
const authority = request.authority || this.config.auth.authority;

if (request.account) {
await this.validateRequestAuthority(authority, request.account);
// validate authority
await this.getDiscoveredAuthority(
authority,
request.azureCloudOptions,
request.account
);
}

const canonicalAuthority = new UrlString(authority);
Expand Down
10 changes: 8 additions & 2 deletions lib/msal-browser/src/interaction_client/PopupClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,8 @@ export class PopupClient extends StandardInteractionClient {
)(
serverTelemetryManager,
validRequest.authority,
validRequest.azureCloudOptions
validRequest.azureCloudOptions,
validRequest.account
);

const isNativeBroker = NativeMessageHandler.isNativeAvailable(
Expand Down Expand Up @@ -406,7 +407,12 @@ export class PopupClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
this.correlationId
)(serverTelemetryManager, requestAuthority);
)(
serverTelemetryManager,
requestAuthority,
undefined, // AzureCloudOptions
validRequest.account || undefined
);

try {
authClient.authority.endSessionEndpoint;
Expand Down
10 changes: 8 additions & 2 deletions lib/msal-browser/src/interaction_client/RedirectClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,8 @@ export class RedirectClient extends StandardInteractionClient {
)(
serverTelemetryManager,
validRequest.authority,
validRequest.azureCloudOptions
validRequest.azureCloudOptions,
validRequest.account
);

// Create redirect interaction handler.
Expand Down Expand Up @@ -520,7 +521,12 @@ export class RedirectClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
this.correlationId
)(serverTelemetryManager, logoutRequest && logoutRequest.authority);
)(
serverTelemetryManager,
logoutRequest && logoutRequest.authority,
undefined, // AzureCloudOptions
(logoutRequest && logoutRequest.account) || undefined
);

if (authClient.authority.protocolMode === ProtocolMode.OIDC) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,12 @@ export class SilentAuthCodeClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
request.correlationId
)(serverTelemetryManager, silentRequest.authority);
)(
serverTelemetryManager,
silentRequest.authority,
silentRequest.azureCloudOptions,
silentRequest.account
);
const authClient: HybridSpaAuthorizationCodeClient =
new HybridSpaAuthorizationCodeClient(clientConfig);
this.logger.verbose("Auth code client created");
Expand Down
10 changes: 6 additions & 4 deletions lib/msal-browser/src/interaction_client/SilentCacheClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,8 @@ export class SilentCacheClient extends StandardInteractionClient {
const silentAuthClient = await this.createSilentFlowClient(
serverTelemetryManager,
silentRequest.authority,
silentRequest.azureCloudOptions
silentRequest.azureCloudOptions,
silentRequest.account
);
this.logger.verbose("Silent auth client created");

Expand Down Expand Up @@ -94,7 +95,8 @@ export class SilentCacheClient extends StandardInteractionClient {
protected async createSilentFlowClient(
serverTelemetryManager: ServerTelemetryManager,
authorityUrl?: string,
azureCloudOptions?: AzureCloudOptions
azureCloudOptions?: AzureCloudOptions,
account?: AccountInfo
): Promise<SilentFlowClient> {
// Create auth module.
const clientConfig = await invokeAsync(
Expand All @@ -103,7 +105,7 @@ export class SilentCacheClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
this.correlationId
)(serverTelemetryManager, authorityUrl, azureCloudOptions);
)(serverTelemetryManager, authorityUrl, azureCloudOptions, account);
return new SilentFlowClient(clientConfig, this.performanceClient);
}

Expand All @@ -122,7 +124,7 @@ export class SilentCacheClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
this.correlationId
)(request, account);
)(request);
return {
...request,
...baseRequest,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,8 @@ export class SilentIframeClient extends StandardInteractionClient {
)(
serverTelemetryManager,
silentRequest.authority,
silentRequest.azureCloudOptions
silentRequest.azureCloudOptions,
silentRequest.account
);

return await invokeAsync(
Expand Down
11 changes: 7 additions & 4 deletions lib/msal-browser/src/interaction_client/SilentRefreshClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
AzureCloudOptions,
PerformanceEvents,
invokeAsync,
AccountInfo,
} from "@azure/msal-common";
import { ApiId } from "../utils/BrowserConstants";
import {
Expand Down Expand Up @@ -39,7 +40,7 @@ export class SilentRefreshClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
request.correlationId
)(request, request.account);
)(request);
const silentRequest: CommonSilentFlowRequest = {
...request,
...baseRequest,
Expand All @@ -59,7 +60,8 @@ export class SilentRefreshClient extends StandardInteractionClient {
const refreshTokenClient = await this.createRefreshTokenClient(
serverTelemetryManager,
silentRequest.authority,
silentRequest.azureCloudOptions
silentRequest.azureCloudOptions,
silentRequest.account
);
// Send request to renew token. Auth module will throw errors if token cannot be renewed.
return invokeAsync(
Expand Down Expand Up @@ -97,7 +99,8 @@ export class SilentRefreshClient extends StandardInteractionClient {
protected async createRefreshTokenClient(
serverTelemetryManager: ServerTelemetryManager,
authorityUrl?: string,
azureCloudOptions?: AzureCloudOptions
azureCloudOptions?: AzureCloudOptions,
account?: AccountInfo
): Promise<RefreshTokenClient> {
// Create auth module.
const clientConfig = await invokeAsync(
Expand All @@ -106,7 +109,7 @@ export class SilentRefreshClient extends StandardInteractionClient {
this.logger,
this.performanceClient,
this.correlationId
)(serverTelemetryManager, authorityUrl, azureCloudOptions);
)(serverTelemetryManager, authorityUrl, azureCloudOptions, account);
return new RefreshTokenClient(clientConfig, this.performanceClient);
}
}
Loading

0 comments on commit 813bda7

Please sign in to comment.