Skip to content

Commit bb941e7

Browse files
Avery-DunnCopilot
andauthored
Remove extraQueryParameters and extraParameters fields from Request types in msal-node (#8136)
Removes the extraQueryParameters/extraParameters API according to KR [3310905](https://identitydivision.visualstudio.com/Engineering/_workitems/edit/3310905), similar to what is being done in MSAL.NET and MSAL Java: AzureAD/microsoft-authentication-library-for-dotnet#5536 AzureAD/microsoft-authentication-library-for-java#1001 This API was meant for niche scenarios which the library did not explicitly cover, and extensive usage could lead to bad caching behavior: these extra query parameters could affect the contents and validity of tokens but were not used in our caching scheme, so if they changed between requests we could return tokens that were not valid for the latest set of parameters. Therefore we are deprecating the current API and, if there is a need for it, may add a new API in the future to perform similar behavior in a more robust way (like was done in MSAL .NET due to requests from first-party customers). --------- Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
1 parent 9f522d4 commit bb941e7

File tree

7 files changed

+15
-89
lines changed

7 files changed

+15
-89
lines changed
Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "major",
3+
"comment": "Remove extraQueryParameters and extraParameters fields from Request types [#8136](https://github.com/AzureAD/microsoft-authentication-library-for-js/pull/8136)",
4+
"packageName": "@azure/msal-node",
5+
"email": "avdunn@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

lib/msal-node/src/request/ClientCredentialRequest.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,8 +13,6 @@ import { CommonClientCredentialRequest } from "./CommonClientCredentialRequest.j
1313
* - correlationId - Unique GUID set per request to trace a request end-to-end for telemetry purposes.
1414
* - skipCache - Skip token cache lookup and force request to authority to get a a new token. Defaults to false.
1515
* - clientAssertion - An assertion string or a callback function that returns an assertion string (both are Base64Url-encoded signed JWTs) used in the Client Credential flow
16-
* - extraQueryParameters - String to string map of custom query parameters added to outgoing token service requests
17-
* - extraParameters - String to string map of custom query parameters added to outgoing token service requests
1816
* @public
1917
*/
2018
export type ClientCredentialRequest = Partial<

lib/msal-node/src/request/CommonClientCredentialRequest.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,10 +18,11 @@ import {
1818
* - preferredAzureRegionOptions - Options of the user's preferred azure region
1919
* - clientAssertion - An assertion string or a callback function that returns an assertion string (both are Base64Url-encoded signed JWTs) used in the Client Credential flow
2020
* - azureRegion - Azure region to be used for regional authentication
21-
* - extraQueryParameters - String to string map of custom query parameters added to outgoing token service requests
22-
* - extraParameters - String to string map of custom query parameters added to outgoing token service requests
2321
*/
24-
export type CommonClientCredentialRequest = BaseAuthRequest & {
22+
export type CommonClientCredentialRequest = Omit<
23+
BaseAuthRequest,
24+
"extraQueryParameters" | "extraParameters"
25+
> & {
2526
skipCache?: boolean;
2627
azureRegion?: AzureRegion;
2728
clientAssertion?: ClientAssertion;

lib/msal-node/src/request/CommonOnBehalfOfRequest.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,10 +11,11 @@ import { BaseAuthRequest } from "@azure/msal-common/node";
1111
* - correlationId - Unique GUID set per request to trace a request end-to-end for telemetry purposes.
1212
* - oboAssertion - The access token that was sent to the middle-tier API. This token must have an audience of the app making this OBO request.
1313
* - skipCache - Skip token cache lookup and force request to authority to get a a new token. Defaults to false.
14-
* - extraQueryParameters - String to string map of custom query parameters added to outgoing token service requests
15-
* - extraParameters - String to string map of custom query parameters added to outgoing token service requests
1614
*/
17-
export type CommonOnBehalfOfRequest = BaseAuthRequest & {
15+
export type CommonOnBehalfOfRequest = Omit<
16+
BaseAuthRequest,
17+
"extraQueryParameters" | "extraParameters"
18+
> & {
1819
oboAssertion: string;
1920
skipCache?: boolean;
2021
};

lib/msal-node/src/request/OnBehalfOfRequest.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,6 @@ import { CommonOnBehalfOfRequest } from "./CommonOnBehalfOfRequest.js";
1111
* - correlationId - Unique GUID set per request to trace a request end-to-end for telemetry purposes.
1212
* - oboAssertion - The access token that was sent to the middle-tier API. This token must have an audience of the app making this OBO request.
1313
* - skipCache - Skip token cache lookup and force request to authority to get a a new token. Defaults to false.
14-
* - extraQueryParameters - String to string map of custom query parameters added to outgoing token service requests
15-
* - extraParameters - String to string map of custom query parameters added to outgoing token service requests
1614
* @public
1715
*/
1816
export type OnBehalfOfRequest = Partial<

lib/msal-node/test/client/ClientCredentialClient.spec.ts

Lines changed: 0 additions & 39 deletions
Original file line numberDiff line numberDiff line change
@@ -119,45 +119,6 @@ describe("ClientCredentialClient unit tests", () => {
119119
checkMockedNetworkRequest(returnVal, checks);
120120
});
121121

122-
it("Adds extraQueryParameters to the /token request", async () => {
123-
const badExecutePostToTokenEndpointMock = jest.spyOn(
124-
ClientCredentialClient.prototype,
125-
<any>"executePostToTokenEndpoint"
126-
);
127-
// no implementation has been mocked, the acquireToken call will fail
128-
129-
const fakeConfig: ClientConfiguration =
130-
await ClientTestUtils.createTestClientConfiguration();
131-
const client: ClientCredentialClient = new ClientCredentialClient(
132-
fakeConfig
133-
);
134-
135-
const clientCredentialRequest: CommonClientCredentialRequest = {
136-
authority: TEST_CONFIG.validAuthority,
137-
correlationId: TEST_CONFIG.CORRELATION_ID,
138-
scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE,
139-
extraQueryParameters: {
140-
testParam1: "testValue1",
141-
testParam2: "",
142-
testParam3: "testValue3",
143-
},
144-
};
145-
146-
await expect(
147-
client.acquireToken(clientCredentialRequest)
148-
).rejects.toThrow();
149-
150-
if (!badExecutePostToTokenEndpointMock.mock.lastCall) {
151-
fail("executePostToTokenEndpointMock was not called");
152-
}
153-
const url: string = badExecutePostToTokenEndpointMock.mock
154-
.lastCall[0] as string;
155-
expect(
156-
url.includes("/token?testParam1=testValue1&testParam3=testValue3")
157-
).toBeTruthy();
158-
expect(!url.includes("/token?testParam2=")).toBeTruthy();
159-
});
160-
161122
it("acquireToken's interactionRequiredAuthError error contains claims", async () => {
162123
const errorResponse = {
163124
error: "interaction_required",

lib/msal-node/test/client/OnBehalfOfClient.spec.ts

Lines changed: 0 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -232,46 +232,6 @@ describe("OnBehalfOf unit tests", () => {
232232
);
233233
});
234234

235-
it("Adds extraQueryParameters to the /token request", async () => {
236-
const badExecutePostToTokenEndpointMock = jest.spyOn(
237-
OnBehalfOfClient.prototype,
238-
<any>"executePostToTokenEndpoint"
239-
);
240-
// no implementation has been mocked, the acquireToken call will fail
241-
242-
const fakeConfig: ClientConfiguration =
243-
await ClientTestUtils.createTestClientConfiguration();
244-
const client: OnBehalfOfClient = new OnBehalfOfClient(fakeConfig);
245-
246-
const oboRequest: CommonOnBehalfOfRequest = {
247-
scopes: [...TEST_CONFIG.DEFAULT_GRAPH_SCOPE],
248-
authority: TEST_CONFIG.validAuthority,
249-
correlationId: TEST_CONFIG.CORRELATION_ID,
250-
oboAssertion: "user_assertion_hash",
251-
skipCache: true,
252-
claims: TEST_CONFIG.CLAIMS,
253-
extraQueryParameters: {
254-
testParam1: "testValue1",
255-
testParam2: "",
256-
testParam3: "testValue3",
257-
},
258-
};
259-
260-
await expect(client.acquireToken(oboRequest)).rejects.toThrow();
261-
262-
if (!badExecutePostToTokenEndpointMock.mock.lastCall) {
263-
fail("executePostToTokenEndpointMock was not called");
264-
}
265-
const url: string = badExecutePostToTokenEndpointMock.mock
266-
.lastCall[0] as string;
267-
expect(
268-
url.includes(
269-
"/token?testParam1=testValue1&testParam3=testValue3"
270-
)
271-
).toBeTruthy();
272-
expect(!url.includes("/token?testParam2=")).toBeTruthy();
273-
});
274-
275235
it("Does not add claims when empty object provided", async () => {
276236
const client = new OnBehalfOfClient(config);
277237

0 commit comments

Comments
 (0)