Skip to content

Commit bd1d3fb

Browse files
Copilotkonstantin-msftCopilot
authored
Port PR #8378: Add support for client data telemetry with CLI_DATA parameter (#8427)
## Summary Ports #8378 to `dev`. ## Changes **Client Data Parsing and Instrumentation (msal-browser):** - Added `parseClientData` function to extract structured information (account type, error, sub-error, cloud instance, caller data boundary) from the `clientdata` response parameter returned by the `/authorize` endpoint - Added `instrumentClientData` function that processes parsed clientdata and instruments telemetry fields (`accountType`, `serverErrorNo`, `serverSubErrorNo`) via `performanceClient` - Called `instrumentClientData` in both `handleResponseCode` and `handleResponseEAR` flows for early telemetry capture **API and Telemetry Updates (msal-common):** - Added `CLI_DATA = "clidata"` constant to `AADServerParamKeys` - Added `addCliData` function to `RequestParameterBuilder` to include `clidata=1` in authorize requests - Called `addCliData` in `getStandardAuthorizeRequestParameters` - Added `clientdata?: string` property to `AuthorizeResponse` - Added `serverSubErrorNo?: string` field to `PerformanceEvent` - Updated `addError` in `PerformanceClient` to not overwrite `serverErrorNo` when it was already set via `addFields` (from clientdata instrumentation) - Updated `msal-common.api.md` API review file **Testing:** - Added test for `CLI_DATA=1` in msal-common `Authorize.spec.ts` - Added tests for `serverErrorNo` non-overwrite behavior in `PerformanceClient.spec.ts` - Added tests for `accountType` addFields preservation/overwrite behavior - Updated browser `Authorize.spec.ts` with clidata=1 checks and `instrumentClientData` integration tests - Created new `ClientDataParser.spec.ts` with comprehensive unit tests for `parseClientData` **Change Files:** - Added beachball change files for `@azure/msal-browser` and `@azure/msal-common` (type: minor) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: konstantin-msft <119362382+konstantin-msft@users.noreply.github.com> Co-authored-by: Konstantin <kshabelko@microsoft.com> Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
1 parent f16b843 commit bd1d3fb

15 files changed

Lines changed: 714 additions & 632 deletions

‎change/@azure-msal-browser-7b41d23d-898b-4f41-b815-fbdc56503858.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "minor",
3+
"comment": "Add support for client data telemetry with CLI_DATA parameter [#8378](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/8378)",
4+
"packageName": "@azure/msal-browser",
5+
"email": "kshabelko@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎change/@azure-msal-common-efa161ad-2d6d-49ae-ab7c-2d12057eafb0.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "minor",
3+
"comment": "Add support for client data telemetry with CLI_DATA parameter [#8378](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/8378)",
4+
"packageName": "@azure/msal-common",
5+
"email": "kshabelko@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎lib/msal-browser/src/protocol/Authorize.ts‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,99 @@ import { EventHandler } from "../event/EventHandler.js";
4444
import { decryptEarResponse } from "../crypto/BrowserCrypto.js";
4545
import { IPlatformAuthHandler } from "../broker/nativeBroker/IPlatformAuthHandler.js";
4646

47+
/**
48+
* Parsed representation of the clientdata response parameter from the /authorize endpoint.
49+
*
50+
* Format: urlencoded(account_type|error|sub_error|cloud_instance|caller_data_boundary)
51+
*/
52+
type ClientData = {
53+
/** Account type: MSA, AAD */
54+
accountType: string;
55+
/** Error code string (e.g. "0x8004345C" for MSA) */
56+
error: string;
57+
/** Sub-error code string (e.g. "0x80043588" for MSA) */
58+
subError: string;
59+
/** Cloud instance hostname (e.g. "login.microsoftonline.com") */
60+
cloudInstance: string;
61+
/** Caller data boundary (e.g. "none" for MSA) */
62+
callerDataBoundary: string;
63+
};
64+
65+
const clientDataAccountTypeMapping = new Map([
66+
["e", "AAD"],
67+
["m", "MSA"],
68+
]);
69+
70+
/**
71+
* Parses the clientdata response parameter from the /authorize endpoint.
72+
*
73+
* Logically, the clientdata value is URL-encoded and pipe-delimited:
74+
* urlencoded(account_type | error | sub_error | cloud_instance | caller_data_boundary)
75+
*
76+
* In normal browser flows, this value may already have been URL-decoded
77+
* (e.g. by URLSearchParams). This function will only apply decodeURIComponent
78+
* when the string appears to contain percent-encoded sequences to avoid
79+
* double-decoding.
80+
*
81+
* @param clientdata - The raw clientdata string from the authorize response
82+
* @returns Parsed ClientData object, or null if the input is empty/invalid
83+
*/
84+
export function parseClientData(clientdata?: string): ClientData | null {
85+
if (!clientdata) {
86+
return null;
87+
}
88+
89+
try {
90+
// Only decode when the string appears to contain percent-encoded sequences
91+
const shouldDecode = /%(?:[0-9A-Fa-f]{2})/.test(clientdata);
92+
const decoded = shouldDecode
93+
? decodeURIComponent(clientdata)
94+
: clientdata;
95+
const parts = decoded.split("|");
96+
97+
if (parts.length < 5) {
98+
return null;
99+
}
100+
101+
return {
102+
accountType:
103+
clientDataAccountTypeMapping.get(parts[0]?.trim() || "") || "",
104+
error: parts[1]?.trim() || "",
105+
subError: parts[2]?.trim() || "",
106+
cloudInstance: parts[3]?.trim() || "",
107+
callerDataBoundary: parts[4]?.trim() || "",
108+
};
109+
} catch {
110+
return null;
111+
}
112+
}
113+
114+
/**
115+
* Instruments account type, error, and suberror from clientdata
116+
*/
117+
function instrumentClientData(
118+
response: AuthorizeResponse,
119+
correlationId: string,
120+
performanceClient: IPerformanceClient
121+
): void {
122+
const parsed = parseClientData(response.clientdata);
123+
parsed?.accountType &&
124+
performanceClient.addFields(
125+
{ accountType: parsed.accountType },
126+
correlationId
127+
);
128+
parsed?.error &&
129+
performanceClient.addFields(
130+
{ serverErrorNo: parsed.error },
131+
correlationId
132+
);
133+
parsed?.subError &&
134+
performanceClient.addFields(
135+
{ serverSubErrorNo: parsed.subError },
136+
correlationId
137+
);
138+
}
139+
47140
/**
48141
* Returns map of parameters that are applicable to all calls to /authorize whether using PKCE or EAR
49142
* @param config
@@ -407,6 +500,10 @@ export async function handleResponseCode(
407500
config.auth.clientId,
408501
request
409502
);
503+
504+
// Instrument clientdata telemetry fields from the authorize response
505+
instrumentClientData(response, request.correlationId, performanceClient);
506+
410507
if (response.accountId) {
411508
return invokeAsync(
412509
handleResponsePlatformBroker,
@@ -487,6 +584,9 @@ export async function handleResponseEAR(
487584
request
488585
);
489586

587+
// Instrument clientdata telemetry fields from the authorize response
588+
instrumentClientData(response, request.correlationId, performanceClient);
589+
490590
// Validate state & check response for errors
491591
AuthorizeProtocol.validateAuthorizationResponse(response, request.state);
492592

‎lib/msal-browser/test/protocol/Authorize.spec.ts‎

Lines changed: 248 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -193,6 +193,7 @@ describe("Authorize Protocol Tests", () => {
193193
BrowserConstants.MSAL_SKU
194194
);
195195
checkInputProperties(AADServerParamKeys.X_CLIENT_VER, version);
196+
checkInputProperties(AADServerParamKeys.CLI_DATA, "1");
196197

197198
// Verify correlationId is present in authorize URL query params
198199
const actionUrl = new URL(form.action);
@@ -466,5 +467,252 @@ describe("Authorize Protocol Tests", () => {
466467
actionUrl.searchParams.get(AADServerParamKeys.CLIENT_REQUEST_ID)
467468
).toEqual(validRequest.correlationId);
468469
});
470+
471+
it("Includes clidata=1 in form post body", async () => {
472+
const form = await Authorize.getCodeForm(
473+
document,
474+
config,
475+
authority,
476+
validRequest,
477+
logger,
478+
performanceClient
479+
);
480+
481+
const cliDataInput = form.elements.namedItem(
482+
AADServerParamKeys.CLI_DATA
483+
) as HTMLInputElement;
484+
expect(cliDataInput).toBeTruthy();
485+
expect(cliDataInput.value).toEqual("1");
486+
});
487+
});
488+
489+
describe("instrumentClientData Tests", () => {
490+
const config = buildConfiguration(
491+
{ auth: { clientId: TEST_CONFIG.MSAL_CLIENT_ID } },
492+
true
493+
);
494+
const logger = new Logger({});
495+
const performanceClient = new StubPerformanceClient();
496+
const authorityOptions: AuthorityOptions = {
497+
protocolMode: ProtocolMode.EAR,
498+
knownAuthorities: [],
499+
cloudDiscoveryMetadata: "",
500+
authorityMetadata: "",
501+
};
502+
const eventHandler = new EventHandler();
503+
const cacheManager = new BrowserCacheManager(
504+
TEST_CONFIG.MSAL_CLIENT_ID,
505+
config.cache,
506+
new CryptoOps(logger, performanceClient),
507+
logger,
508+
performanceClient,
509+
eventHandler
510+
);
511+
let authority: Authority;
512+
const validRequest: CommonAuthorizationUrlRequest = {
513+
authority: TEST_CONFIG.validAuthority,
514+
scopes: ["openid", "profile"],
515+
correlationId: TEST_CONFIG.CORRELATION_ID,
516+
redirectUri: window.location.href,
517+
state: TEST_STATE_VALUES.TEST_STATE_REDIRECT,
518+
nonce: ID_TOKEN_CLAIMS.nonce,
519+
responseMode: Constants.ResponseMode.FRAGMENT,
520+
codeChallenge: "code-challenge",
521+
earJwk: validEarJWK,
522+
};
523+
524+
beforeAll(async () => {
525+
jest.useFakeTimers();
526+
authority = await AuthorityFactory.createDiscoveredInstance(
527+
TEST_CONFIG.validAuthority,
528+
config.system.networkClient,
529+
cacheManager,
530+
authorityOptions,
531+
logger,
532+
TEST_CONFIG.CORRELATION_ID,
533+
performanceClient
534+
);
535+
});
536+
537+
afterAll(() => {
538+
jest.useRealTimers();
539+
});
540+
541+
it("handleResponseEAR instruments clientdata telemetry when clientdata is present", async () => {
542+
const addFieldsSpy = jest.spyOn(performanceClient, "addFields");
543+
// clientdata: m|0x8004345C|0x80047857|none|login.microsoftonline.com
544+
const clientdata =
545+
"m%7C0x8004345C%7C0x80047857%7Cnone%7Clogin.microsoftonline.com";
546+
const response = {
547+
ear_jwe: validEarJWE,
548+
state: validRequest.state,
549+
clientdata,
550+
};
551+
552+
await Authorize.handleResponseEAR(
553+
validRequest,
554+
response,
555+
ApiId.acquireTokenPopup,
556+
config,
557+
authority,
558+
cacheManager,
559+
cacheManager,
560+
eventHandler,
561+
logger,
562+
performanceClient
563+
);
564+
565+
expect(addFieldsSpy).toHaveBeenCalledWith(
566+
expect.objectContaining({
567+
accountType: "MSA",
568+
}),
569+
validRequest.correlationId
570+
);
571+
expect(addFieldsSpy).toHaveBeenCalledWith(
572+
expect.objectContaining({
573+
serverErrorNo: "0x8004345C",
574+
}),
575+
validRequest.correlationId
576+
);
577+
expect(addFieldsSpy).toHaveBeenCalledWith(
578+
expect.objectContaining({
579+
serverSubErrorNo: "0x80047857",
580+
}),
581+
validRequest.correlationId
582+
);
583+
addFieldsSpy.mockRestore();
584+
});
585+
586+
it("handleResponseEAR does not instrument clientdata when clientdata is absent", async () => {
587+
const addFieldsSpy = jest.spyOn(performanceClient, "addFields");
588+
addFieldsSpy.mockClear();
589+
const response = {
590+
ear_jwe: validEarJWE,
591+
state: validRequest.state,
592+
};
593+
594+
await Authorize.handleResponseEAR(
595+
validRequest,
596+
response,
597+
ApiId.acquireTokenPopup,
598+
config,
599+
authority,
600+
cacheManager,
601+
cacheManager,
602+
eventHandler,
603+
logger,
604+
performanceClient
605+
);
606+
607+
// addFields may be called for other reasons, but NOT with clientData fields
608+
const clientDataCalls = addFieldsSpy.mock.calls.filter(
609+
(call: unknown[]) => {
610+
const fields = call[0] as
611+
| Record<string, unknown>
612+
| undefined;
613+
return fields && "serverErrorNo" in fields;
614+
}
615+
);
616+
expect(clientDataCalls).toHaveLength(0);
617+
addFieldsSpy.mockRestore();
618+
});
619+
620+
it("handleResponseEAR instruments Entra (AAD) account type from clientdata", async () => {
621+
const addFieldsSpy = jest.spyOn(performanceClient, "addFields");
622+
// clientdata: e|AADSTS50076|basic_action|login.microsoftonline.com|none
623+
const clientdata =
624+
"e%7CAADSTS50076%7Cbasic_action%7Clogin.microsoftonline.com%7Cnone";
625+
const response = {
626+
ear_jwe: validEarJWE,
627+
state: validRequest.state,
628+
clientdata,
629+
};
630+
631+
await Authorize.handleResponseEAR(
632+
validRequest,
633+
response,
634+
ApiId.acquireTokenPopup,
635+
config,
636+
authority,
637+
cacheManager,
638+
cacheManager,
639+
eventHandler,
640+
logger,
641+
performanceClient
642+
);
643+
644+
expect(addFieldsSpy).toHaveBeenCalledWith(
645+
expect.objectContaining({
646+
accountType: "AAD",
647+
}),
648+
validRequest.correlationId
649+
);
650+
expect(addFieldsSpy).toHaveBeenCalledWith(
651+
expect.objectContaining({
652+
serverErrorNo: "AADSTS50076",
653+
}),
654+
validRequest.correlationId
655+
);
656+
expect(addFieldsSpy).toHaveBeenCalledWith(
657+
expect.objectContaining({
658+
serverSubErrorNo: "basic_action",
659+
}),
660+
validRequest.correlationId
661+
);
662+
addFieldsSpy.mockRestore();
663+
});
664+
665+
it("handleResponseCode instruments clientdata telemetry before processing response", async () => {
666+
const addFieldsSpy = jest.spyOn(performanceClient, "addFields");
667+
// clientdata: e|AADSTS65001|consent_required|login.microsoftonline.com|none
668+
const clientdata =
669+
"e%7CAADSTS65001%7Cconsent_required%7Clogin.microsoftonline.com%7Cnone";
670+
const response = {
671+
// Use accountId so it hits the platformBroker path, which throws without a provider
672+
accountId: "test-account-id",
673+
state: validRequest.state,
674+
clientdata,
675+
};
676+
677+
try {
678+
await Authorize.handleResponseCode(
679+
validRequest,
680+
response,
681+
"code-verifier",
682+
ApiId.acquireTokenPopup,
683+
config,
684+
{} as any, // authClient not needed — accountId path is taken
685+
cacheManager,
686+
cacheManager,
687+
eventHandler,
688+
logger,
689+
performanceClient
690+
// no platformAuthProvider → throws nativeConnectionNotEstablished
691+
);
692+
} catch {
693+
// Expected: nativeConnectionNotEstablished
694+
}
695+
696+
// instrumentClientData ran before the throw
697+
expect(addFieldsSpy).toHaveBeenCalledWith(
698+
expect.objectContaining({
699+
accountType: "AAD",
700+
}),
701+
validRequest.correlationId
702+
);
703+
expect(addFieldsSpy).toHaveBeenCalledWith(
704+
expect.objectContaining({
705+
serverErrorNo: "AADSTS65001",
706+
}),
707+
validRequest.correlationId
708+
);
709+
expect(addFieldsSpy).toHaveBeenCalledWith(
710+
expect.objectContaining({
711+
serverSubErrorNo: "consent_required",
712+
}),
713+
validRequest.correlationId
714+
);
715+
addFieldsSpy.mockRestore();
716+
});
469717
});
470718
});

0 commit comments

Comments
 (0)