Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow applications to bypass network request for OpenID configuration #1578

Merged
merged 11 commits into from
May 21, 2020
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
28 changes: 28 additions & 0 deletions lib/msal-core/docs/performance.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
# Performance

This document will outline techniques your application can use to improve the performance of acquire tokens using MSAL.js.

## Bypass authority metadata resolution

By default, during the process of retrieving a token MSAL.js will make two network requests to retrieve metadata from the authority configured for the request. If you would like to skip those network requests, you can provide the required metadata in the configuration of `UserAgentApplication`.

**Important:** It is your application's responsibility to ensure it is using correct, up-to-date authority metadata. Failure to do so may result in your application not working correctly.

Instructions:

1. Determine the authorize endpoint for your authority. For example, if you are using `https://login.microsoftonline.com/common/`, the authorize endpoint is `https://login.microsoftonline.com/common/oauth2/v2.0/authorize`.
2. Determine the instance discovery endpoint for your authority. The instance discovery API is located at `https://login.microsoftonline.com/common/discovery/instance?api-version=1.0&authorization_endpoint={authorizeEndpoint}`. If you are using the `common` endpoint, this url is `https://login.microsoftonline.com/common/discovery/instance?api-version=1.0&authorization_endpoint=https://login.microsoftonline.com/common/oauth2/v2.0/authorize`.
3. Make a request to the instance discovery endpoint.
4. Parse the `tenant_discovery_endpoint` property from the response.
5. Make a request to the url for the `tenant_discovery_endpoint` property.
6. Take the **entire** response and provide the raw JSON string as the `auth.authorityMetadata` property for `UserAgentApplication`. It can also be passed per-request as a part of `AuthenticationParameters`.

Example:

```js
const msalInstance = new msal.UserAgentApplication({
auth: {
authorityMetadata: '{"token_endpoint":"https://login.microsoftonline.com/common/oauth2/v2.0/token","token_endpoint_auth_methods_supported":["client_secret_post","private_key_jwt","client_secret_basic"],"jwks_uri":"https://login.microsoftonline.com/common/discovery/v2.0/keys","response_modes_supported":["query","fragment","form_post"],"subject_types_supported":["pairwise"],"id_token_signing_alg_values_supported":["RS256"],"response_types_supported":["code","id_token","code id_token","id_token token"],"scopes_supported":["openid","profile","email","offline_access"],"issuer":"https://login.microsoftonline.com/{tenantid}/v2.0","request_uri_parameter_supported":false,"userinfo_endpoint":"https://graph.microsoft.com/oidc/userinfo","authorization_endpoint":"https://login.microsoftonline.com/common/oauth2/v2.0/authorize","http_logout_supported":true,"frontchannel_logout_supported":true,"end_session_endpoint":"https://login.microsoftonline.com/common/oauth2/v2.0/logout","claims_supported":["sub","iss","cloud_instance_name","cloud_instance_host_name","cloud_graph_host_name","msgraph_host","aud","exp","iat","auth_time","acr","nonce","preferred_username","name","tid","ver","at_hash","c_hash","email"],"tenant_region_scope":null,"cloud_instance_name":"microsoftonline.com","cloud_graph_host_name":"graph.windows.net","msgraph_host":"graph.microsoft.com","rbac_url":"https://pas.windows.net"}'
}
});
```
1 change: 1 addition & 0 deletions lib/msal-core/src/AuthenticationParameters.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ export type AuthenticationParameters = {
forceRefresh?: boolean;
redirectUri?: string;
redirectStartPage?: string;
authorityMetadata?: string
};

export function validateClaimsRequest(request: AuthenticationParameters) {
Expand Down
3 changes: 3 additions & 0 deletions lib/msal-core/src/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ const NAVIGATE_FRAME_WAIT = 500;
* - clientId - Client ID of your app registered with our Application registration portal : https://portal.azure.com/#blade/Microsoft_AAD_IAM/ActiveDirectoryMenuBlade/RegisteredAppsPreview in Microsoft Identity Platform
* - authority - You can configure a specific authority, defaults to " " or "https://login.microsoftonline.com/common"
* - validateAuthority - Used to turn authority validation on/off. When set to true (default), MSAL will compare the application's authority against well-known URLs templates representing well-formed authorities. It is useful when the authority is obtained at run time to prevent MSAL from displaying authentication prompts from malicious pages.
* - authorityMetadata - OpenID configuration metadata for the configured authority. Must be passed as a JSON string.
* - knownAuthorities - If validateAuthority is set to True, this will be used to set the Trusted Host list. Defaults to empty array
* - redirectUri - The redirect URI of the application, this should be same as the value in the application registration portal.Defaults to `window.location.href`.
* - postLogoutRedirectUri - Used to redirect the user to this location after logout. Defaults to `window.location.href`.
Expand All @@ -37,6 +38,7 @@ export type AuthOptions = {
clientId: string;
authority?: string;
validateAuthority?: boolean;
authorityMetadata?: string;
knownAuthorities?: Array<string>;
redirectUri?: string | (() => string);
postLogoutRedirectUri?: string | (() => string);
Expand Down Expand Up @@ -117,6 +119,7 @@ const DEFAULT_AUTH_OPTIONS: AuthOptions = {
clientId: "",
authority: null,
validateAuthority: true,
authorityMetadata: "",
jasonnutter marked this conversation as resolved.
Show resolved Hide resolved
knownAuthorities: [],
redirectUri: () => UrlUtils.getCurrentUrl(),
postLogoutRedirectUri: () => UrlUtils.getCurrentUrl(),
Expand Down
143 changes: 85 additions & 58 deletions lib/msal-core/src/UserAgentApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,6 +220,7 @@ export class UserAgentApplication {
this.telemetryManager = this.getTelemetryManagerFromConfig(this.config.system.telemetry, this.clientId);

AuthorityFactory.setKnownAuthorities(this.config.auth.validateAuthority, this.config.auth.knownAuthorities);
AuthorityFactory.saveMetadataFromConfig(this.config.auth.authority, this.config.auth.authorityMetadata);

// if no authority is passed, set the default: "https://login.microsoftonline.com/common"
this.authority = this.config.auth.authority || DEFAULT_AUTHORITY;
Expand Down Expand Up @@ -483,13 +484,13 @@ export class UserAgentApplication {
* Helper function to acquireToken
*
*/
private acquireTokenHelper(account: Account, interactionType: InteractionType, isLoginCall: boolean, request: AuthenticationParameters, resolve?: any, reject?: any): void {
private async acquireTokenHelper(account: Account, interactionType: InteractionType, isLoginCall: boolean, request: AuthenticationParameters, resolve?: any, reject?: any): Promise<void> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this effect performance for calls that don't need this async?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potentially, but should be offset when not making the openid-config call at all. Will verify.

// Track the acquireToken progress
this.cacheStorage.setItem(TemporaryCacheKeys.INTERACTION_STATUS, Constants.inProgress);
const scope = request.scopes ? request.scopes.join(" ").toLowerCase() : this.clientId.toLowerCase();

let serverAuthenticationRequest: ServerRequestParameters;
const acquireTokenAuthority = (request && request.authority) ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance;
const acquireTokenAuthority = (request && request.authority) ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority, request.authorityMetadata) : this.authorityInstance;

let popUpWindow: Window;

Expand All @@ -514,7 +515,14 @@ export class UserAgentApplication {
}
}

acquireTokenAuthority.resolveEndpointsAsync(this.telemetryManager, request.correlationId).then(async () => {
try {
if (!acquireTokenAuthority.hasCachedMetadata()) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of checking for this here, why don't we just do this inside resolveAuthorityAsync()? If the cached metadata is available, just return it, otherwise make a network request (similar to acquireTokenSilent)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this too. Though this would prevent intentional refreshes without an additional forceRefresh parameter. Not sure if there's a valid argument for supporting that use case but something to consider.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can't do intentional refreshes anyway with the current code.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One of the goals is to minimize the number of async code paths, which is why the if is done here, so that way if the check fails, we don't enter an async function. If this was factored out, it would enter the async every time.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I did not consider if forceRefresh should force a network call for openid configuration, in addition to forcing a network call for a new token. Initially, I would think not. @DarylThayil any opinion here?

this.logger.verbose("No cached metadata for authority");
await AuthorityFactory.saveMetadataFromNetwork(acquireTokenAuthority, this.telemetryManager, request.correlationId);
} else {
this.logger.verbose("Cached metadata found for authority");
}
jasonnutter marked this conversation as resolved.
Show resolved Hide resolved

// On Fulfillment
const responseType: string = isLoginCall ? ResponseTypes.id_token : this.getTokenType(account, request.scopes, false);

Expand Down Expand Up @@ -586,14 +594,14 @@ export class UserAgentApplication {
}
}
}
}).catch((err) => {
} catch (err) {
this.logger.error(err);
this.cacheStorage.resetTempCacheItems(request.state);
this.authErrorHandler(interactionType, ClientAuthError.createEndpointResolutionError(err.toString), buildResponseStateOnly(request.state), reject);
if (popUpWindow) {
popUpWindow.close();
}
});
}
}

/**
Expand Down Expand Up @@ -637,7 +645,7 @@ export class UserAgentApplication {
const apiEvent: ApiEvent = this.telemetryManager.createAndStartApiEvent(request.correlationId, API_EVENT_IDENTIFIER.AcquireTokenSilent);
const requestSignature = RequestUtils.createRequestSignature(request);

return new Promise<AuthResponse>((resolve, reject) => {
return new Promise<AuthResponse>(async (resolve, reject) => {

// block the request if made from the hidden iframe
WindowUtils.blockReloadInHiddenIframes();
Expand Down Expand Up @@ -671,7 +679,7 @@ export class UserAgentApplication {

// create a serverAuthenticationRequest populating the `queryParameters` to be sent to the Server
const serverAuthenticationRequest = new ServerRequestParameters(
AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority),
AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority, request.authorityMetadata),
this.clientId,
responseType,
this.getRedirectUri(request.redirectUri),
Expand Down Expand Up @@ -737,46 +745,49 @@ export class UserAgentApplication {

// Cache result can return null if cache is empty. In that case, set authority to default value if no authority is passed to the API.
if (!serverAuthenticationRequest.authorityInstance) {
serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority) : this.authorityInstance;
serverAuthenticationRequest.authorityInstance = request.authority ? AuthorityFactory.CreateInstance(request.authority, this.config.auth.validateAuthority, request.authorityMetadata) : this.authorityInstance;
}
this.logger.verbosePii(`Authority instance: ${serverAuthenticationRequest.authority}`);

// cache miss

// start http event
return serverAuthenticationRequest.authorityInstance.resolveEndpointsAsync(this.telemetryManager, request.correlationId)
.then(() => {
/*
* refresh attempt with iframe
* Already renewing for this scope, callback when we get the token.
*/
this.logger.verbosePii(`Authority instance: ${serverAuthenticationRequest.authority}`);

try {
if (!serverAuthenticationRequest.authorityInstance.hasCachedMetadata()) {
jasonnutter marked this conversation as resolved.
Show resolved Hide resolved
this.logger.verbose("No cached metadata for authority");
await AuthorityFactory.saveMetadataFromNetwork(serverAuthenticationRequest.authorityInstance, this.telemetryManager, request.correlationId);
this.logger.verbose("Authority has been updated with endpoint discovery response");
} else {
this.logger.verbose("Cached metadata found for authority");
}

if (window.activeRenewals[requestSignature]) {
this.logger.verbose("Renew token for scope and authority: " + requestSignature + " is in progress. Registering callback");
// Active renewals contains the state for each renewal.
this.registerCallback(window.activeRenewals[requestSignature], requestSignature, resolve, reject);
}
else {
if (request.scopes && request.scopes.indexOf(this.clientId) > -1 && request.scopes.length === 1) {
/*
* App uses idToken to send to api endpoints
* Default scope is tracked as clientId to store this token
*/
this.logger.verbose("Renewing idToken");
this.silentLogin = true;
this.renewIdToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
} else {
// renew access token
this.logger.verbose("Renewing access token");
this.renewToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
}
/*
* refresh attempt with iframe
* Already renewing for this scope, callback when we get the token.
*/
if (window.activeRenewals[requestSignature]) {
this.logger.verbose("Renew token for scope and authority: " + requestSignature + " is in progress. Registering callback");
// Active renewals contains the state for each renewal.
this.registerCallback(window.activeRenewals[requestSignature], requestSignature, resolve, reject);
}
else {
if (request.scopes && request.scopes.indexOf(this.clientId) > -1 && request.scopes.length === 1) {
/*
* App uses idToken to send to api endpoints
* Default scope is tracked as clientId to store this token
*/
this.logger.verbose("Renewing idToken");
this.silentLogin = true;
this.renewIdToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
} else {
// renew access token
this.logger.verbose("Renewing accesstoken");
this.renewToken(requestSignature, resolve, reject, account, serverAuthenticationRequest);
}
}).catch((err) => {
this.logger.warning("Could not resolve endpoints");
reject(ClientAuthError.createEndpointResolutionError(err.toString()));
return null;
});
}
} catch (err) {
this.logger.error(err);
reject(ClientAuthError.createEndpointResolutionError(err.toString()));
return null;
}
}
})
.then(res => {
Expand Down Expand Up @@ -966,27 +977,43 @@ export class UserAgentApplication {
* Default behaviour is to redirect the user to `window.location.href`.
*/
logout(correlationId?: string): void {
// TODO this new correlation id passed in, is not appended to logout request, should add
this.logoutAsync(correlationId);
}

/**
* Async version of logout(). Use to log out the current user.
* @param correlationId Request correlationId
*/
private async logoutAsync(correlationId?: string): Promise<void> {
const requestCorrelationId = correlationId || CryptoUtils.createNewGuid();
const apiEvent = this.telemetryManager.createAndStartApiEvent(requestCorrelationId, API_EVENT_IDENTIFIER.Logout);

this.clearCache();
this.account = null;
let logout = "";
if (this.getPostLogoutRedirectUri()) {
logout = "post_logout_redirect_uri=" + encodeURIComponent(this.getPostLogoutRedirectUri());
}
this.authorityInstance.resolveEndpointsAsync(this.telemetryManager, requestCorrelationId)
.then(authority => {
const urlNavigate = authority.EndSessionEndpoint
? `${authority.EndSessionEndpoint}?${logout}`
: `${this.authority}oauth2/v2.0/logout?${logout}`;
this.telemetryManager.stopAndFlushApiEvent(requestCorrelationId, apiEvent, true);
this.navigateWindow(urlNavigate);
})
.catch((error: AuthError) => {
this.telemetryManager.stopAndFlushApiEvent(requestCorrelationId, apiEvent, false, error.errorCode);
});

try {
if (!this.authorityInstance.hasCachedMetadata()) {
this.logger.verbose("No cached metadata for authority");
await AuthorityFactory.saveMetadataFromNetwork(this.authorityInstance, this.telemetryManager, correlationId);
} else {
this.logger.verbose("Cached metadata found for authority");
}

const correlationIdParam = `client-request-id=${requestCorrelationId}`;

const postLogoutQueryParam = this.getPostLogoutRedirectUri()
? `&post_logout_redirect_uri=${encodeURIComponent(this.getPostLogoutRedirectUri())}`
: "";

const urlNavigate = this.authorityInstance.EndSessionEndpoint
? `${this.authorityInstance.EndSessionEndpoint}?${correlationIdParam}${postLogoutQueryParam}`
: `${this.authority}oauth2/v2.0/logout?${correlationIdParam}${postLogoutQueryParam}`;

this.telemetryManager.stopAndFlushApiEvent(requestCorrelationId, apiEvent, true);
this.navigateWindow(urlNavigate);
} catch (error) {
this.telemetryManager.stopAndFlushApiEvent(requestCorrelationId, apiEvent, false, error.errorCode);
}
}

/**
Expand Down
5 changes: 3 additions & 2 deletions lib/msal-core/src/authority/AadAuthority.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { XhrClient, XhrResponse } from "../XHRClient";
import { AADTrustedHostList } from "../utils/Constants";
import HttpEvent from "../telemetry/HttpEvent";
import TelemetryManager from "../telemetry/TelemetryManager";
import { ITenantDiscoveryResponse } from "./ITenantDiscoveryResponse";

/**
* @hidden
Expand All @@ -19,8 +20,8 @@ export class AadAuthority extends Authority {
return `${AadAuthority.AadInstanceDiscoveryEndpoint}?api-version=1.0&authorization_endpoint=${this.CanonicalAuthority}oauth2/v2.0/authorize`;
}

public constructor(authority: string, validateAuthority: boolean) {
super(authority, validateAuthority);
public constructor(authority: string, validateAuthority: boolean, authorityMetadata?: ITenantDiscoveryResponse) {
super(authority, validateAuthority, authorityMetadata);
}

public get AuthorityType(): AuthorityType {
Expand Down