Skip to content

Commit

Permalink
Track MSAL node SKU for broker flows (#7213)
Browse files Browse the repository at this point in the history
- Track MSAL node SKU for broker flows.
- Move SKU instrumentation to common
  • Loading branch information
konstantin-msft authored Jul 19, 2024
1 parent 8130884 commit 97fb781
Show file tree
Hide file tree
Showing 12 changed files with 227 additions and 31 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Track MSAL node SKU for broker flows #7213",
"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 MSAL node SKU for broker flows #7213",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Track MSAL node SKU for broker flows #7213",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Track MSAL node SKU for broker flows #7213",
"packageName": "@azure/msal-node-extensions",
"email": "[email protected]",
"dependentChangeType": "patch"
}
19 changes: 19 additions & 0 deletions extensions/msal-node-extensions/src/broker/NativeBrokerPlugin.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
*/

import {
AADServerParamKeys,
AccountInfo,
AuthenticationResult,
AuthenticationScheme,
Expand All @@ -21,6 +22,7 @@ import {
NativeSignOutRequest,
PromptValue,
ServerError,
ServerTelemetryManager,
} from "@azure/msal-common";
import {
msalNodeRuntime,
Expand Down Expand Up @@ -487,6 +489,23 @@ export class NativeBrokerPlugin implements INativeBrokerPlugin {
}
);
}

const skus =
request.extraParameters &&
request.extraParameters[AADServerParamKeys.X_CLIENT_EXTRA_SKU]
?.length
? request.extraParameters[
AADServerParamKeys.X_CLIENT_EXTRA_SKU
]
: "";
authParams.SetAdditionalParameter(
AADServerParamKeys.X_CLIENT_EXTRA_SKU,
ServerTelemetryManager.makeExtraSkuString({
skus,
extensionName: "msal.node.ext",
extensionVersion: version,
})
);
} catch (e) {
const wrappedError = this.wrapError(e);
if (wrappedError) {
Expand Down
43 changes: 14 additions & 29 deletions lib/msal-browser/src/interaction_client/NativeInteractionClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,34 +81,6 @@ const BrokerServerParamKeys = {
BROKER_REDIRECT_URI: "brk_redirect_uri",
};

/**
* Provides MSAL and browser extension SKUs
* @param messageHandler {NativeMessageHandler}
*/
function getSKUs(messageHandler: NativeMessageHandler): string {
const groupSeparator = ",";
const valueSeparator = "|";
const skus = Array.from({ length: 4 }, () => valueSeparator);
// Report MSAL SKU
skus[0] = [BrowserConstants.MSAL_SKU, version].join(valueSeparator);

// Report extension SKU
const extensionName =
messageHandler.getExtensionId() ===
NativeConstants.PREFERRED_EXTENSION_ID
? "chrome"
: messageHandler.getExtensionId()?.length
? "unknown"
: undefined;

if (extensionName) {
skus[2] = [extensionName, messageHandler.getExtensionVersion()].join(
valueSeparator
);
}
return skus.join(groupSeparator);
}

export class NativeInteractionClient extends BaseInteractionClient {
protected apiId: ApiId;
protected accountId: string;
Expand Down Expand Up @@ -161,7 +133,20 @@ export class NativeInteractionClient extends BaseInteractionClient {
this.serverTelemetryManager = this.initializeServerTelemetryManager(
this.apiId
);
this.skus = getSKUs(this.nativeMessageHandler);

const extensionName =
this.nativeMessageHandler.getExtensionId() ===
NativeConstants.PREFERRED_EXTENSION_ID
? "chrome"
: this.nativeMessageHandler.getExtensionId()?.length
? "unknown"
: undefined;
this.skus = ServerTelemetryManager.makeExtraSkuString({
libraryName: BrowserConstants.MSAL_SKU,
libraryVersion: version,
extensionName: extensionName,
extensionVersion: this.nativeMessageHandler.getExtensionVersion(),
});
}

/**
Expand Down
4 changes: 4 additions & 0 deletions lib/msal-common/apiReview/msal-common.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -3664,6 +3664,10 @@ export class ServerTelemetryManager {
getNativeBrokerErrorCode(): string | undefined;
getRegionDiscoveryFields(): string;
incrementCacheHits(): number;
// Warning: (ae-forgotten-export) The symbol "SkuParams" needs to be exported by the entry point index.d.ts
//
// (undocumented)
static makeExtraSkuString(params: SkuParams): string;
// Warning: (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
static maxErrorsToSend(serverTelemetryEntity: ServerTelemetryEntity): number;
setCacheOutcome(cacheOutcome: CacheOutcome): void;
Expand Down
67 changes: 67 additions & 0 deletions lib/msal-common/src/telemetry/server/ServerTelemetryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,69 @@ import { ServerTelemetryRequest } from "./ServerTelemetryRequest";
import { ServerTelemetryEntity } from "../../cache/entities/ServerTelemetryEntity";
import { RegionDiscoveryMetadata } from "../../authority/RegionDiscoveryMetadata";

const skuGroupSeparator = ",";
const skuValueSeparator = "|";

type SkuParams = {
libraryName?: string;
libraryVersion?: string;
extensionName?: string;
extensionVersion?: string;
skus?: string;
};

function makeExtraSkuString(params: SkuParams): string {
const {
skus,
libraryName,
libraryVersion,
extensionName,
extensionVersion,
} = params;
const skuMap: Map<number, (string | undefined)[]> = new Map([
[0, [libraryName, libraryVersion]],
[2, [extensionName, extensionVersion]],
]);
let skuArr: string[] = [];

if (skus?.length) {
skuArr = skus.split(skuGroupSeparator);

// Ignore invalid input sku param
if (skuArr.length < 4) {
return skus;
}
} else {
skuArr = Array.from({ length: 4 }, () => skuValueSeparator);
}

skuMap.forEach((value, key) => {
if (value.length === 2 && value[0]?.length && value[1]?.length) {
setSku({
skuArr,
index: key,
skuName: value[0],
skuVersion: value[1],
});
}
});

return skuArr.join(skuGroupSeparator);
}

function setSku(params: {
skuArr: string[];
index: number;
skuName: string;
skuVersion: string;
}): void {
const { skuArr, index, skuName, skuVersion } = params;
if (index >= skuArr.length) {
return;
}
skuArr[index] = [skuName, skuVersion].join(skuValueSeparator);
}

/** @internal */
export class ServerTelemetryManager {
private cacheManager: CacheManager;
Expand Down Expand Up @@ -304,4 +367,8 @@ export class ServerTelemetryManager {
lastRequests
);
}

static makeExtraSkuString(params: SkuParams): string {
return makeExtraSkuString(params);
}
}
52 changes: 52 additions & 0 deletions lib/msal-common/test/telemetry/ServerTelemetryManager.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -402,4 +402,56 @@ describe("ServerTelemetryManager.ts", () => {
) as ServerTelemetryEntity;
expect(cacheValue.cacheHits).toBe(2);
});

describe("makeExtraSkuString", () => {
it("Creates empty string from scratch", () => {
const skus = ServerTelemetryManager.makeExtraSkuString({});
expect(skus).toEqual("|,|,|,|");
});

it("Does not modify input string", () => {
const skus = ServerTelemetryManager.makeExtraSkuString({
skus: "test_sku|1.0.0,|,|,|",
});
expect(skus).toEqual("test_sku|1.0.0,|,|,|");
});

it("Returns invalid input", () => {
const skus = ServerTelemetryManager.makeExtraSkuString({
skus: "test_sku|1.0.0,|,|",
libraryName: "test_lib_name",
libraryVersion: "1.2.3",
});
expect(skus).toEqual("test_sku|1.0.0,|,|");
});

it("Adds library and extension info", () => {
const skus = ServerTelemetryManager.makeExtraSkuString({
skus: "test_sku|1.0.0,|,test_ext_sku|2.0.0,|",
libraryName: "test_lib_name",
libraryVersion: "1.2.3",
extensionName: "test_ext_name",
extensionVersion: "5.6.7",
});
expect(skus).toEqual("test_lib_name|1.2.3,|,test_ext_name|5.6.7,|");
});

it("Updates input string with library info", () => {
const skus = ServerTelemetryManager.makeExtraSkuString({
skus: "test_sku|1.0.0,|,test_ext_sku|2.0.0,|",
libraryName: "test_lib_name",
libraryVersion: "1.2.3",
});
expect(skus).toEqual("test_lib_name|1.2.3,|,test_ext_sku|2.0.0,|");
});

it("Updates input string with extension info", () => {
const skus = ServerTelemetryManager.makeExtraSkuString({
skus: "test_sku|1.0.0,|,test_ext_sku|2.0.0,|",
extensionName: "test_ext_name",
extensionVersion: "5.6.7",
});
expect(skus).toEqual("test_sku|1.0.0,|,test_ext_name|5.6.7,|");
});
});
});
2 changes: 1 addition & 1 deletion lib/msal-node/apiReview/msal-node.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -758,7 +758,7 @@ export const version = "2.11.1";
// src/client/OnBehalfOfClient.ts:249:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/OnBehalfOfClient.ts:250:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/OnBehalfOfClient.ts:310:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/PublicClientApplication.ts:298:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/PublicClientApplication.ts:310:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/UsernamePasswordClient.ts:74:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/UsernamePasswordClient.ts:75:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/UsernamePasswordClient.ts:114:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
Expand Down
14 changes: 13 additions & 1 deletion lib/msal-node/src/client/PublicClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ import {
AccountInfo,
INativeBrokerPlugin,
ServerAuthorizationCodeResponse,
AADServerParamKeys,
ServerTelemetryManager,
} from "@azure/msal-common";
import { Configuration } from "../config/Configuration.js";
import { ClientApplication } from "./ClientApplication.js";
Expand All @@ -36,6 +38,7 @@ import { SilentFlowRequest } from "../request/SilentFlowRequest.js";
import { SignOutRequest } from "../request/SignOutRequest.js";
import { ILoopbackClient } from "../network/ILoopbackClient.js";
import { DeviceCodeClient } from "./DeviceCodeClient.js";
import { version } from "../packageMetadata";

/**
* This class is to be used to acquire tokens for public client applications (desktop, mobile). Public client applications
Expand All @@ -47,6 +50,7 @@ export class PublicClientApplication
implements IPublicClientApplication
{
private nativeBrokerPlugin?: INativeBrokerPlugin;
private readonly skus: string;
/**
* Important attributes in the Configuration object for auth are:
* - clientID: the application ID of your application. You can obtain one by registering your application with our Application registration portal.
Expand Down Expand Up @@ -78,6 +82,10 @@ export class PublicClientApplication
);
}
}
this.skus = ServerTelemetryManager.makeExtraSkuString({
libraryName: Constants.MSAL_SKU,
libraryVersion: version,
});
}

/**
Expand Down Expand Up @@ -156,6 +164,7 @@ export class PublicClientApplication
extraParameters: {
...remainingProperties.extraQueryParameters,
...remainingProperties.tokenQueryParameters,
[AADServerParamKeys.X_CLIENT_EXTRA_SKU]: this.skus,
},
accountId: remainingProperties.account?.nativeAccountId,
};
Expand Down Expand Up @@ -247,7 +256,10 @@ export class PublicClientApplication
redirectUri: `${Constants.HTTP_PROTOCOL}${Constants.LOCALHOST}`,
authority: request.authority || this.config.auth.authority,
correlationId: correlationId,
extraParameters: request.tokenQueryParameters,
extraParameters: {
...request.tokenQueryParameters,
[AADServerParamKeys.X_CLIENT_EXTRA_SKU]: this.skus,
},
accountId: request.account.nativeAccountId,
forceRefresh: request.forceRefresh || false,
};
Expand Down
29 changes: 29 additions & 0 deletions lib/msal-node/test/client/PublicClientApplication.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ import {
CacheHelpers,
AuthorityFactory,
ProtocolMode,
AADServerParamKeys,
} from "@azure/msal-common";
import {
Configuration,
Expand Down Expand Up @@ -53,6 +54,7 @@ import { ClientAuthErrorCodes } from "@azure/msal-common";
import { TEST_CONFIG } from "../test_kit/StringConstants";
import { HttpClient } from "../../src/network/HttpClient";
import { MockStorageClass } from "./ClientTestUtils";
import { Constants } from "../../src/utils/Constants";

const msalCommon: MSALCommonModule = jest.requireActual("@azure/msal-common");

Expand Down Expand Up @@ -256,6 +258,33 @@ describe("PublicClientApplication", () => {
);
});

test("acquireTokenSilent sends extra telemetry to NativeBrokerPlugin", async () => {
const authApp = new PublicClientApplication({
...appConfig,
broker: {
nativeBrokerPlugin: new MockNativeBrokerPlugin(),
},
});

const request: SilentFlowRequest = {
account: mockNativeAccountInfo,
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
};
const brokerSpy: jest.SpyInstance<unknown, [...unknown[]]> =
jest.spyOn(
MockNativeBrokerPlugin.prototype,
"acquireTokenSilent"
);
await authApp.acquireTokenSilent(request);
const nativeRequest = brokerSpy.mock.calls[0][0];
expect(nativeRequest).toHaveProperty("extraParameters");
// @ts-ignore
expect(nativeRequest.extraParameters).toHaveProperty(
AADServerParamKeys.X_CLIENT_EXTRA_SKU,
`${Constants.MSAL_SKU}|${version},|,|,|`
);
});

test("acquireTokenSilent - calls into NativeBrokerPlugin and throws", (done) => {
const authApp = new PublicClientApplication({
...appConfig,
Expand Down

0 comments on commit 97fb781

Please sign in to comment.