Skip to content

Commit 4486b7e

Browse files
lalimashardasameeragtnorling
authored
Fix JSON object conversion in PlatformDOMRequest v4 (#8350)
Co-authored-by: Sameera Gajjarapu <sameera.gajjarapu@microsoft.com> Co-authored-by: Thomas Norling <thomas.norling@microsoft.com>
1 parent 3e5d58b commit 4486b7e

3 files changed

Lines changed: 109 additions & 15 deletions

File tree

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Fix JSON object conversion in PlatformDOMRequest v4 [#8350](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/8350)",
4+
"packageName": "@azure/msal-browser",
5+
"email": "lalimasharda@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

lib/msal-browser/src/broker/nativeBroker/PlatformAuthDOMHandler.ts

Lines changed: 24 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -231,18 +231,29 @@ export class PlatformAuthDOMHandler implements IPlatformAuthHandler {
231231
private getDOMExtraParams(
232232
extraParameters: Record<string, unknown>
233233
): DOMExtraParameters {
234-
const stringifiedParams = Object.entries(extraParameters).reduce(
235-
(record, [key, value]) => {
236-
record[key] = String(value);
237-
return record;
238-
},
239-
{} as StringDict
240-
);
241-
242-
const validExtraParams: DOMExtraParameters = {
243-
...stringifiedParams,
244-
};
245-
246-
return validExtraParams;
234+
try {
235+
const stringifiedProperties: StringDict = {};
236+
for (const [key, value] of Object.entries(extraParameters)) {
237+
if (!value) {
238+
continue;
239+
}
240+
if (typeof value === "object") {
241+
stringifiedProperties[key] = JSON.stringify(value);
242+
} else {
243+
stringifiedProperties[key] = String(value);
244+
}
245+
}
246+
return stringifiedProperties;
247+
} catch (e) {
248+
this.logger.error(
249+
this.platformAuthType + " - Error stringifying extra parameters"
250+
);
251+
this.logger.errorPii(
252+
this.platformAuthType +
253+
" - Error stringifying extra parameters: " +
254+
e
255+
);
256+
return {};
257+
}
247258
}
248259
}

lib/msal-browser/test/broker/PlatformAuthDOMHandler.spec.ts

Lines changed: 78 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -532,9 +532,16 @@ describe("PlatformAuthDOMHandler tests", () => {
532532
nonce: "test-nonce",
533533
claims: "test-claims",
534534
instanceAware: true,
535-
windowTitleSubstring: "test-window-substring",
535+
windowTitleSubstring: null,
536536
extendedExpiryToken: true,
537537
signPopToken: true,
538+
account: {
539+
nativeAccountId: "native-test-id",
540+
userName: "test-user",
541+
name: "Test User",
542+
username: "testest@test.com",
543+
},
544+
someArrayParam: ["value1", "value2"],
538545
};
539546
const domExtraParams =
540547
//@ts-ignore
@@ -544,10 +551,79 @@ describe("PlatformAuthDOMHandler tests", () => {
544551
nonce: "test-nonce",
545552
claims: "test-claims",
546553
instanceAware: "true",
547-
windowTitleSubstring: "test-window-substring",
548554
extendedExpiryToken: "true",
549555
signPopToken: "true",
556+
account: JSON.stringify(testExtraParameters.account),
557+
someArrayParam: '["value1","value2"]',
558+
});
559+
});
560+
561+
it("should omit undefined values", async () => {
562+
getSupportedContractsMock.mockResolvedValue([
563+
PlatformAuthConstants.PLATFORM_DOM_APIS,
564+
]);
565+
const platformAuthDOMHandler =
566+
await PlatformAuthDOMHandler.createProvider(
567+
logger,
568+
performanceClient,
569+
"test-correlation-id"
570+
);
571+
572+
const testExtraParameters = {
573+
prompt: PromptValue.NONE,
574+
nonce: "test-nonce",
575+
claims: "test-claims",
576+
instanceAware: undefined,
577+
};
578+
579+
const domExtraParams =
580+
//@ts-ignore
581+
platformAuthDOMHandler.getDOMExtraParams(testExtraParameters);
582+
expect(domExtraParams).toEqual({
583+
prompt: "none",
584+
nonce: "test-nonce",
585+
claims: "test-claims",
550586
});
587+
expect(domExtraParams).not.toHaveProperty("instanceAware");
588+
});
589+
590+
it("should catch JSON.stringify error and return empty object", async () => {
591+
getSupportedContractsMock.mockResolvedValue([
592+
PlatformAuthConstants.PLATFORM_DOM_APIS,
593+
]);
594+
const platformAuthDOMHandler =
595+
await PlatformAuthDOMHandler.createProvider(
596+
logger,
597+
performanceClient,
598+
"test-correlation-id"
599+
);
600+
601+
// Create a circular reference that will cause JSON.stringify to throw
602+
const circularObj: any = { name: "test" };
603+
circularObj.self = circularObj;
604+
605+
const testExtraParameters = {
606+
prompt: PromptValue.NONE,
607+
problemParam: circularObj,
608+
};
609+
610+
console.log("Testing circular object:", testExtraParameters);
611+
612+
const loggerErrorSpy = jest.spyOn(logger, "error");
613+
const loggerErrorPiiSpy = jest.spyOn(logger, "errorPii");
614+
615+
const domExtraParams =
616+
//@ts-ignore
617+
platformAuthDOMHandler.getDOMExtraParams(testExtraParameters);
618+
619+
expect(domExtraParams).toEqual({});
620+
expect(loggerErrorSpy).toHaveBeenCalledWith(
621+
"PlatformAuthDOMHandler - Error stringifying extra parameters"
622+
);
623+
expect(loggerErrorPiiSpy).toHaveBeenCalled();
624+
expect(loggerErrorPiiSpy.mock.calls[0][0]).toContain(
625+
"Error stringifying extra parameters"
626+
);
551627
});
552628
});
553629
});

0 commit comments

Comments
 (0)