Skip to content

Commit

Permalink
Merge pull request #1921 from AzureAD/msal-node-MSER-Telemetry
Browse files Browse the repository at this point in the history
Telemetry #3: Implement Telemetry in msal-node
  • Loading branch information
tnorling committed Sep 10, 2020
2 parents 126b83b + 0475b8d commit aa74343
Show file tree
Hide file tree
Showing 8 changed files with 137 additions and 46 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "patch",
"comment": "Track Suberrors in Telemetry (#1921)",
"packageName": "@azure/msal-common",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-09-02T20:01:33.353Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "Implement Telemetry in msal-node (#1921)",
"packageName": "@azure/msal-node",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-09-02T20:01:47.198Z"
}
4 changes: 3 additions & 1 deletion lib/msal-common/src/error/AuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ export class AuthError extends Error {
errorCode: string;
// Detailed description of error
errorMessage: string;
suberror: string;

constructor(errorCode: string, errorMessage?: string) {
constructor(errorCode: string, errorMessage?: string, suberror?: string) {
const errorString = errorMessage ? `${errorCode}: ${errorMessage}` : errorCode;
super(errorString);
Object.setPrototypeOf(this, AuthError.prototype);

this.errorCode = errorCode;
this.errorMessage = errorMessage;
this.suberror = suberror;
this.name = "AuthError";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { CacheManager } from "../../cache/CacheManager";
import { ServerTelemetryCacheValue } from "./ServerTelemetryCacheValue";
import { AuthError } from "../../error/AuthError";
import { ServerTelemetryRequest } from "./ServerTelemetryRequest";
import { StringUtils } from "../../utils/StringUtils";

export class ServerTelemetryManager {
private cacheManager: CacheManager;
Expand Down Expand Up @@ -49,7 +50,7 @@ export class ServerTelemetryManager {
cacheFailedRequest(error: AuthError): void {
const lastRequests = this.getLastRequests();
lastRequests.failedRequests.push(this.apiId, this.correlationId);
lastRequests.errors.push(error.errorCode);
lastRequests.errors.push(StringUtils.isEmpty(error.suberror)? error.errorCode: error.suberror);
lastRequests.errorCount += 1;

if (lastRequests.errors.length > SERVER_TELEM_CONSTANTS.FAILURE_LIMIT) {
Expand Down
101 changes: 70 additions & 31 deletions lib/msal-node/src/client/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,22 @@ import {
BaseAuthRequest,
SilentFlowRequest,
SilentFlowClient,
Logger
Logger,
ServerTelemetryManager,
ServerTelemetryRequest
} from "@azure/msal-common";
import { Configuration, buildAppConfiguration } from "../config/Configuration";
import { CryptoProvider } from "../crypto/CryptoProvider";
import { Storage } from "../cache/Storage";
import { Constants as NodeConstants } from "./../utils/Constants";
import { Constants as NodeConstants, ApiId } from "./../utils/Constants";
import { TokenCache } from "../cache/TokenCache";
import { ClientAssertion } from "../client/ClientAssertion";
import { version } from "../../package.json";

export abstract class ClientApplication {
private _authority: Authority;
private readonly cryptoProvider: CryptoProvider;
private storage: Storage;
protected storage: Storage;
private tokenCache: TokenCache;
protected logger: Logger;
protected config: Configuration;
Expand Down Expand Up @@ -74,7 +76,7 @@ export abstract class ClientApplication {
const authorizationCodeClient = new AuthorizationCodeClient(
authClientConfig
);
return authorizationCodeClient.getAuthCodeUrl(this.initializeRequestScopes(request) as AuthorizationUrlRequest);
return authorizationCodeClient.getAuthCodeUrl(this.initializeRequest(request) as AuthorizationUrlRequest);
}

/**
Expand All @@ -87,14 +89,22 @@ export abstract class ClientApplication {
*/
async acquireTokenByCode(request: AuthorizationCodeRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByCode called");
const authClientConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const authorizationCodeClient = new AuthorizationCodeClient(
authClientConfig
);
return authorizationCodeClient.acquireToken(this.initializeRequestScopes(request) as AuthorizationCodeRequest);
const validRequest = this.initializeRequest(request) as AuthorizationCodeRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByCode, validRequest.correlationId!);
try {
const authClientConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const authorizationCodeClient = new AuthorizationCodeClient(
authClientConfig
);
return authorizationCodeClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -106,14 +116,22 @@ export abstract class ClientApplication {
*/
async acquireTokenByRefreshToken(request: RefreshTokenRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByRefreshToken called");
const refreshTokenClientConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const refreshTokenClient = new RefreshTokenClient(
refreshTokenClientConfig
);
return refreshTokenClient.acquireToken(this.initializeRequestScopes(request) as RefreshTokenRequest);
const validRequest = this.initializeRequest(request) as RefreshTokenRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByRefreshToken, validRequest.correlationId!);
try {
const refreshTokenClientConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const refreshTokenClient = new RefreshTokenClient(
refreshTokenClientConfig
);
return refreshTokenClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -125,13 +143,21 @@ export abstract class ClientApplication {
* and the guidance is for the user to call any interactive token acquisition API (eg: `acquireTokenByCode()`).
*/
async acquireTokenSilent(request: SilentFlowRequest): Promise<AuthenticationResult> {
const silentFlowClientConfig = await this.buildOauthClientConfiguration(
request.authority
);
const silentFlowClient = new SilentFlowClient(
silentFlowClientConfig
);
return silentFlowClient.acquireToken(this.initializeRequestScopes(request) as SilentFlowRequest);
const validRequest = this.initializeRequest(request) as SilentFlowRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenSilent, validRequest.correlationId!, validRequest.forceRefresh);
try {
const silentFlowClientConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
const silentFlowClient = new SilentFlowClient(
silentFlowClientConfig
);
return silentFlowClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -142,7 +168,7 @@ export abstract class ClientApplication {
return this.tokenCache;
}

protected async buildOauthClientConfiguration(authority?: string): Promise<ClientConfiguration> {
protected async buildOauthClientConfiguration(authority?: string, serverTelemetryManager?: ServerTelemetryManager): Promise<ClientConfiguration> {
this.logger.verbose("buildOauthClientConfiguration called");
// using null assertion operator as we ensure that all config values have default values in buildConfiguration()

Expand All @@ -163,6 +189,7 @@ export abstract class ClientApplication {
cryptoInterface: this.cryptoProvider,
networkInterface: this.config.system!.networkClient,
storageInterface: this.storage,
serverTelemetryManager: serverTelemetryManager,
clientCredentials: {
clientSecret: this.clientSecret,
clientAssertion: this.clientAssertion ? this.getClientAssertion() : undefined,
Expand All @@ -184,16 +211,28 @@ export abstract class ClientApplication {
}

/**
* Generates a request with the default scopes.
* Generates a request with the default scopes & generates a correlationId.
* @param authRequest
*/
protected initializeRequestScopes(authRequest: BaseAuthRequest): BaseAuthRequest {
protected initializeRequest(authRequest: BaseAuthRequest): BaseAuthRequest {
this.logger.verbose("initializeRequestScopes called");

return {
...authRequest,
scopes: [...((authRequest && authRequest.scopes) || []), Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, Constants.OFFLINE_ACCESS_SCOPE]
scopes: [...((authRequest && authRequest.scopes) || []), Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, Constants.OFFLINE_ACCESS_SCOPE],
correlationId: authRequest && authRequest.correlationId || this.cryptoProvider.createNewGuid()
};
}

protected initializeServerTelemetryManager(apiId: number, correlationId: string, forceRefresh?: boolean): ServerTelemetryManager {
const telemetryPayload: ServerTelemetryRequest = {
clientId: this.config.auth.clientId,
correlationId: correlationId,
apiId: apiId,
forceRefresh: forceRefresh || false
};

return new ServerTelemetryManager(telemetryPayload, this.storage);
}

/**
Expand Down
23 changes: 16 additions & 7 deletions lib/msal-node/src/client/ConfidentialClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { ClientApplication } from "./ClientApplication";
import { Configuration } from "../config/Configuration";
import { ClientAssertion } from "../client/ClientAssertion";
import { ApiId } from "../utils/Constants";
import {
ClientCredentialRequest,
ClientCredentialClient,
Expand Down Expand Up @@ -47,12 +48,20 @@ export class ConfidentialClientApplication extends ClientApplication {
*/
public async acquireTokenByClientCredential(request: ClientCredentialRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByClientCredential called");
const clientCredentialConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const clientCredentialClient = new ClientCredentialClient(clientCredentialConfig);
return clientCredentialClient.acquireToken(request);
const validRequest = this.initializeRequest(request) as ClientCredentialRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByClientCredential, validRequest.correlationId!, validRequest.skipCache);
try {
const clientCredentialConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const clientCredentialClient = new ClientCredentialClient(clientCredentialConfig);
return clientCredentialClient.acquireToken(request);
} catch(e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -73,7 +82,7 @@ export class ConfidentialClientApplication extends ClientApplication {
);
this.logger.verbose("Auth client config generated");
const oboClient = new OnBehalfOfClient(clientCredentialConfig);
return oboClient.acquireToken(this.initializeRequestScopes(request) as OnBehalfOfRequest);
return oboClient.acquireToken(this.initializeRequest(request) as OnBehalfOfRequest);
}

private setClientCredential(configuration: Configuration): void {
Expand Down
21 changes: 15 additions & 6 deletions lib/msal-node/src/client/PublicClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
* Licensed under the MIT License.
*/

import { ApiId } from "../utils/Constants";
import { DeviceCodeClient, DeviceCodeRequest, AuthenticationResult } from "@azure/msal-common";
import { Configuration } from "../config/Configuration";
import { ClientApplication } from "./ClientApplication";
Expand Down Expand Up @@ -44,11 +45,19 @@ export class PublicClientApplication extends ClientApplication {
*/
public async acquireTokenByDeviceCode(request: DeviceCodeRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByDeviceCode called");
const deviceCodeConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const deviceCodeClient = new DeviceCodeClient(deviceCodeConfig);
return deviceCodeClient.acquireToken(this.initializeRequestScopes(request) as DeviceCodeRequest);
const validRequest = this.initializeRequest(request) as DeviceCodeRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByDeviceCode, validRequest.correlationId!);
try {
const deviceCodeConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const deviceCodeClient = new DeviceCodeClient(deviceCodeConfig);
return deviceCodeClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}
}
15 changes: 15 additions & 0 deletions lib/msal-node/src/utils/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,21 @@ export const Constants = {
JWT_BEARER_ASSERTION_TYPE: "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"
};

/**
* API Codes for Telemetry purposes.
* Before adding a new code you must claim it in the MSAL Telemetry tracker as these number spaces are shared across all MSALs
* 0-99 Silent Flow
* 600-699 Device Code Flow
* 800-899 Auth Code Flow
*/
export enum ApiId {
acquireTokenSilent = 62,
acquireTokenByCode = 871,
acquireTokenByRefreshToken = 872,
acquireTokenByDeviceCode = 671,
acquireTokenByClientCredential = 771
}

/**
* JWT constants
*/
Expand Down

0 comments on commit aa74343

Please sign in to comment.