Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Move correlationId to query string for /token calls #7385

Merged
merged 5 commits into from
Oct 21, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "patch",
"comment": "Move correlationId to query string on /token calls",
"packageName": "@azure/msal-common",
"email": "[email protected]",
"dependentChangeType": "patch"
}
6 changes: 3 additions & 3 deletions lib/msal-common/apiReview/msal-common.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4284,9 +4284,9 @@ const X_MS_LIB_CAPABILITY = "x-ms-lib-capability";
// src/client/AuthorizationCodeClient.ts:228:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:229:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:307:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:512:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:735:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:795:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:507:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:730:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/AuthorizationCodeClient.ts:790:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/RefreshTokenClient.ts:193:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/RefreshTokenClient.ts:277:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
// src/client/RefreshTokenClient.ts:278:8 - (tsdoc-param-tag-missing-hyphen) The @param block should be followed by a parameter name and then a hyphen
Expand Down
5 changes: 0 additions & 5 deletions lib/msal-common/src/client/AuthorizationCodeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -420,11 +420,6 @@ export class AuthorizationCodeClient extends BaseClient {
}
}

const correlationId =
request.correlationId ||
this.config.cryptoInterface.createNewGuid();
parameterBuilder.addCorrelationId(correlationId);

if (
!StringUtils.isEmptyObj(request.claims) ||
(this.config.authOptions.clientCapabilities &&
Expand Down
2 changes: 2 additions & 0 deletions lib/msal-common/src/client/BaseClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ export abstract class BaseClient {
);
}

parameterBuilder.addCorrelationId(request.correlationId);

return parameterBuilder.createQueryString();
}
}
2 changes: 0 additions & 2 deletions lib/msal-common/src/client/RefreshTokenClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -380,8 +380,6 @@ export class RefreshTokenClient extends BaseClient {
parameterBuilder.addServerTelemetry(this.serverTelemetryManager);
}

parameterBuilder.addCorrelationId(correlationId);

parameterBuilder.addRefreshToken(request.refreshToken);

if (this.config.clientCredentials.clientSecret) {
Expand Down
40 changes: 40 additions & 0 deletions lib/msal-common/test/client/AuthorizationCodeClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2327,6 +2327,46 @@ describe("AuthorizationCodeClient unit tests", () => {
).toBe(true);
});

it("Adds correlationId to the /token query string", (done) => {
jest.spyOn(
Authority.prototype,
<any>"getEndpointMetadataFromNetwork"
).mockResolvedValue(DEFAULT_OPENID_CONFIG_RESPONSE.body);
jest.spyOn(
AuthorizationCodeClient.prototype,
<any>"executePostToTokenEndpoint"
// @ts-expect-error
).mockImplementation((url: string) => {
try {
expect(url).toContain(
`client-request-id=${RANDOM_TEST_GUID}`
);
done();
} catch (error) {
done(error);
}
});

const client = new AuthorizationCodeClient(config);
const authorizationCodeRequest: CommonAuthorizationCodeRequest = {
authority: Constants.DEFAULT_AUTHORITY,
scopes: [
...TEST_CONFIG.DEFAULT_GRAPH_SCOPE,
...TEST_CONFIG.DEFAULT_SCOPES,
],
redirectUri: TEST_URIS.TEST_REDIRECT_URI_LOCALHOST,
code: TEST_TOKENS.AUTHORIZATION_CODE,
codeVerifier: TEST_CONFIG.TEST_VERIFIER,
claims: TEST_CONFIG.CLAIMS,
correlationId: RANDOM_TEST_GUID,
authenticationScheme: AuthenticationScheme.BEARER,
};

client.acquireToken(authorizationCodeRequest).catch((error) => {
// Catch errors thrown after the function call this test is testing
});
});

it("Adds tokenQueryParameters to the /token request", (done) => {
jest.spyOn(
Authority.prototype,
Expand Down
38 changes: 38 additions & 0 deletions lib/msal-common/test/client/RefreshTokenClient.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,44 @@ describe("RefreshTokenClient unit tests", () => {
config = await ClientTestUtils.createTestClientConfiguration();
});

it("Adds correlationId to the /token query string", (done) => {
jest.spyOn(
RefreshTokenClient.prototype,
<any>"executePostToTokenEndpoint"
// @ts-expect-error
).mockImplementation((url: string) => {
try {
expect(url).toContain(
`client-request-id=${TEST_CONFIG.CORRELATION_ID}`
);
done();
} catch (error) {
done(error);
}
});

const client = new RefreshTokenClient(
config,
stubPerformanceClient
);
const refreshTokenRequest: CommonRefreshTokenRequest = {
scopes: TEST_CONFIG.DEFAULT_GRAPH_SCOPE,
refreshToken: TEST_TOKENS.REFRESH_TOKEN,
claims: TEST_CONFIG.CLAIMS,
authority: TEST_CONFIG.validAuthority,
correlationId: TEST_CONFIG.CORRELATION_ID,
authenticationScheme:
TEST_CONFIG.TOKEN_TYPE_BEARER as AuthenticationScheme,
tokenQueryParameters: {
testParam: "testValue",
},
};

client.acquireToken(refreshTokenRequest).catch((e) => {
// Catch errors thrown after the function call this test is testing
});
});

it("Adds tokenQueryParameters to the /token request", (done) => {
jest.spyOn(
RefreshTokenClient.prototype,
Expand Down
7 changes: 5 additions & 2 deletions lib/msal-node/test/config/ClientConfiguration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
ConfidentialClientApplication,
} from "../../src";
import { OnBehalfOfRequest } from "../../src/request/OnBehalfOfRequest";
import { RANDOM_TEST_GUID } from "../test_kit/StringConstants.js";

describe("ClientConfiguration tests", () => {
test("builds configuration and assigns default functions", () => {
Expand Down Expand Up @@ -203,6 +204,7 @@ describe("ClientConfiguration tests", () => {
const request: ClientCredentialRequest = {
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
skipCache: true,
correlationId: RANDOM_TEST_GUID,
};

await new ConfidentialClientApplication(
Expand All @@ -215,7 +217,7 @@ describe("ClientConfiguration tests", () => {
DEFAULT_OPENID_CONFIG_RESPONSE.body.token_endpoint.replace(
"{tenant}",
"tenantid"
),
) + `?client-request-id=${RANDOM_TEST_GUID}`,
expect.objectContaining({
body: expect.stringContaining("TEST-CAPABILITY"),
})
Expand All @@ -227,6 +229,7 @@ describe("ClientConfiguration tests", () => {
scopes: TEST_CONSTANTS.DEFAULT_GRAPH_SCOPE,
oboAssertion: "user_assertion_hash",
skipCache: true,
correlationId: RANDOM_TEST_GUID,
};

const appConfig: Configuration = {
Expand Down Expand Up @@ -258,7 +261,7 @@ describe("ClientConfiguration tests", () => {
DEFAULT_OPENID_CONFIG_RESPONSE.body.token_endpoint.replace(
"{tenant}",
"tenantid"
),
) + `?client-request-id=${RANDOM_TEST_GUID}`,
expect.objectContaining({
body: expect.stringContaining("TEST-CAPABILITY"),
})
Expand Down
Loading