Skip to content

Commit

Permalink
Client Assertion implementation now accepts an async callback as well…
Browse files Browse the repository at this point in the history
… as a string argument (#7014)

Client assertion can currently be provided by a developer as a string.
This PR allows a developer to provide an async callback (which will
resolve to a string) as the client assertion.

The client assertion initialization has been removed from
ConfidentialClient's constructor and is now initialized inside of
buildOauthClientConfiguration, called during every acquireToken call.

Note: Applied Boy-Scout-Rule on UsernamePasswordClient.spec.ts. Only the
last test in the file is relevant for this review. Now this test file is
up-to-date and mirrors ClientCredentialsClient.spec.ts and
OnBehalfOfClient.spec.ts.
  • Loading branch information
Robbie-Microsoft authored Apr 30, 2024
1 parent 37633b4 commit e6d18c5
Show file tree
Hide file tree
Showing 24 changed files with 843 additions and 467 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Client Assertion Implementation now accepts a callback instead of a string argument",
"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": "Client Assertion Implementation now accepts a callback instead of a string argument",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
11 changes: 10 additions & 1 deletion lib/msal-common/src/account/ClientCredentials.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,20 @@
* Licensed under the MIT License.
*/

export type ClientAssertionConfig = {
clientId: string;
tokenEndpoint?: string;
};

export type ClientAssertionCallback = (
config: ClientAssertionConfig
) => Promise<string>;

/**
* Client Assertion credential for Confidential Clients
*/
export type ClientAssertion = {
assertion: string;
assertion: string | ClientAssertionCallback;
assertionType: string;
};

Expand Down
13 changes: 11 additions & 2 deletions lib/msal-common/src/client/AuthorizationCodeClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,8 @@ import { RequestValidator } from "../request/RequestValidator";
import { IPerformanceClient } from "../telemetry/performance/IPerformanceClient";
import { PerformanceEvents } from "../telemetry/performance/PerformanceEvent";
import { invokeAsync } from "../utils/FunctionWrappers";
import { ClientAssertion } from "../account/ClientCredentials";
import { getClientAssertion } from "../utils/ClientAssertionUtils";

/**
* Oauth2.0 Authorization Code client
Expand Down Expand Up @@ -364,9 +366,16 @@ export class AuthorizationCodeClient extends BaseClient {
}

if (this.config.clientCredentials.clientAssertion) {
const clientAssertion =
const clientAssertion: ClientAssertion =
this.config.clientCredentials.clientAssertion;
parameterBuilder.addClientAssertion(clientAssertion.assertion);

parameterBuilder.addClientAssertion(
await getClientAssertion(
clientAssertion.assertion,
this.config.authOptions.clientId,
request.resourceRequestUri
)
);
parameterBuilder.addClientAssertionType(
clientAssertion.assertionType
);
Expand Down
13 changes: 11 additions & 2 deletions lib/msal-common/src/client/RefreshTokenClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ import { PerformanceEvents } from "../telemetry/performance/PerformanceEvent";
import { IPerformanceClient } from "../telemetry/performance/IPerformanceClient";
import { invoke, invokeAsync } from "../utils/FunctionWrappers";
import { generateCredentialKey } from "../cache/utils/CacheHelpers";
import { ClientAssertion } from "../account/ClientCredentials";
import { getClientAssertion } from "../utils/ClientAssertionUtils";

const DEFAULT_REFRESH_TOKEN_EXPIRATION_OFFSET_SECONDS = 300; // 5 Minutes

Expand Down Expand Up @@ -385,9 +387,16 @@ export class RefreshTokenClient extends BaseClient {
}

if (this.config.clientCredentials.clientAssertion) {
const clientAssertion =
const clientAssertion: ClientAssertion =
this.config.clientCredentials.clientAssertion;
parameterBuilder.addClientAssertion(clientAssertion.assertion);

parameterBuilder.addClientAssertion(
await getClientAssertion(
clientAssertion.assertion,
this.config.authOptions.clientId,
request.resourceRequestUri
)
);
parameterBuilder.addClientAssertionType(
clientAssertion.assertionType
);
Expand Down
8 changes: 7 additions & 1 deletion lib/msal-common/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,11 @@ export { NativeRequest } from "./request/NativeRequest";
export { NativeSignOutRequest } from "./request/NativeSignOutRequest";
export { RequestParameterBuilder } from "./request/RequestParameterBuilder";
export { StoreInCache } from "./request/StoreInCache";
export { ClientAssertion } from "./account/ClientCredentials";
export {
ClientAssertion,
ClientAssertionConfig,
ClientAssertionCallback,
} from "./account/ClientCredentials";
// Response
export { AzureRegion } from "./authority/AzureRegion";
export { AzureRegionConfiguration } from "./authority/AzureRegionConfiguration";
Expand Down Expand Up @@ -182,6 +186,7 @@ export {
createClientConfigurationError,
} from "./error/ClientConfigurationError";
// Constants and Utils
export { getClientAssertion } from "./utils/ClientAssertionUtils";
export {
Constants,
OIDC_DEFAULT_SCOPES,
Expand Down Expand Up @@ -218,6 +223,7 @@ export {
} from "./utils/ProtocolUtils";
export * as TimeUtils from "./utils/TimeUtils";
export * as UrlUtils from "./utils/UrlUtils";
export * as ClientAssertionUtils from "./utils/ClientAssertionUtils";
export * from "./utils/FunctionWrappers";
// Server Telemetry
export { ServerTelemetryManager } from "./telemetry/server/ServerTelemetryManager";
Expand Down
25 changes: 25 additions & 0 deletions lib/msal-common/src/utils/ClientAssertionUtils.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
/*
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License.
*/

import {
ClientAssertionCallback,
ClientAssertionConfig,
} from "../account/ClientCredentials";

export async function getClientAssertion(
clientAssertion: string | ClientAssertionCallback,
clientId: string,
tokenEndpoint?: string
): Promise<string> {
if (typeof clientAssertion === "string") {
return clientAssertion;
} else {
const config: ClientAssertionConfig = {
clientId: clientId,
tokenEndpoint: tokenEndpoint,
};
return clientAssertion(config);
}
}
101 changes: 95 additions & 6 deletions lib/msal-common/test/request/RequestParameterBuilder.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,9 @@ import {
ClientConfigurationErrorMessage,
createClientConfigurationError,
} from "../../src/error/ClientConfigurationError";
import { ClientAssertion, ClientAssertionCallback } from "../../src";
import { getClientAssertion } from "../../src/utils/ClientAssertionUtils";
import { ClientAssertionConfig } from "../../src/account/ClientCredentials";

describe("RequestParameterBuilder unit tests", () => {
it("constructor", () => {
Expand Down Expand Up @@ -379,14 +382,20 @@ describe("RequestParameterBuilder unit tests", () => {
sinon.restore();
});

it("adds clientAssertion and assertionType if they are passed in as strings", () => {
const clientAssertion = {
it("adds clientAssertion (string) and assertionType if they are provided by the developer", async () => {
const clientAssertion: ClientAssertion = {
assertion: "testAssertion",
assertionType: "jwt-bearer",
};

const requestParameterBuilder = new RequestParameterBuilder();
requestParameterBuilder.addClientAssertion(clientAssertion.assertion);
requestParameterBuilder.addClientAssertion(
await getClientAssertion(
clientAssertion.assertion,
"client_id",
"optional_token_endpoint"
)
);
requestParameterBuilder.addClientAssertionType(
clientAssertion.assertionType
);
Expand All @@ -407,14 +416,94 @@ describe("RequestParameterBuilder unit tests", () => {
).toBe(true);
});

it("doesn't add client assertion and client assertion type if they are empty strings", () => {
const clientAssertion = {
it("doesn't add client assertion (string) and client assertion type if they are empty strings", async () => {
const clientAssertion: ClientAssertion = {
assertion: "",
assertionType: "",
};

const requestParameterBuilder = new RequestParameterBuilder();
requestParameterBuilder.addClientAssertion(clientAssertion.assertion);
requestParameterBuilder.addClientAssertion(
await getClientAssertion(
clientAssertion.assertion,
"client_id",
"optional_token_endpoint"
)
);
requestParameterBuilder.addClientAssertionType(
clientAssertion.assertionType
);
const requestQueryString = requestParameterBuilder.createQueryString();
expect(
requestQueryString.includes(AADServerParamKeys.CLIENT_ASSERTION)
).toBe(false);
expect(
requestQueryString.includes(
AADServerParamKeys.CLIENT_ASSERTION_TYPE
)
).toBe(false);
});

it("adds clientAssertion (ClientAssertionCallback) and assertionType if they are provided by the developer", async () => {
const ClientAssertionCallback: ClientAssertionCallback = (
_config: ClientAssertionConfig
) => {
return Promise.resolve("testAssertion");
};

const clientAssertion: ClientAssertion = {
assertion: ClientAssertionCallback,
assertionType: "jwt-bearer",
};

const requestParameterBuilder = new RequestParameterBuilder();
requestParameterBuilder.addClientAssertion(
await getClientAssertion(
clientAssertion.assertion,
"client_id",
"optional_token_endpoint"
)
);
requestParameterBuilder.addClientAssertionType(
clientAssertion.assertionType
);
const requestQueryString = requestParameterBuilder.createQueryString();
expect(
requestQueryString.includes(
`${AADServerParamKeys.CLIENT_ASSERTION}=${encodeURIComponent(
"testAssertion"
)}`
)
).toBe(true);
expect(
requestQueryString.includes(
`${
AADServerParamKeys.CLIENT_ASSERTION_TYPE
}=${encodeURIComponent("jwt-bearer")}`
)
).toBe(true);
});

it("doesn't add client assertion (ClientAssertionCallback) and client assertion type if they are empty strings", async () => {
const ClientAssertionCallback: ClientAssertionCallback = (
_config: ClientAssertionConfig
) => {
return Promise.resolve("");
};

const clientAssertion: ClientAssertion = {
assertion: ClientAssertionCallback,
assertionType: "",
};

const requestParameterBuilder = new RequestParameterBuilder();
requestParameterBuilder.addClientAssertion(
await getClientAssertion(
clientAssertion.assertion,
"client_id",
"optional_token_endpoint"
)
);
requestParameterBuilder.addClientAssertionType(
clientAssertion.assertionType
);
Expand Down
14 changes: 12 additions & 2 deletions lib/msal-node/docs/initialize-confidential-client-application.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,16 @@ See the MSAL sample: [auth-code-with-certs](../../../samples/msal-node-samples/a
import * as msal from "@azure/msal-node";
import "dotenv/config"; // process.env now has the values defined in a .env file

const clientAssertionCallback: msal.ClientAssertionCallback = async (
config: msal.ClientAssertionConfig
): Promise<string> => {
// network request that uses config.clientId and (optionally) config.tokenEndpoint
const result: Promise<string> = await Promise.resolve(
"network request which gets assertion"
);
return result;
};

const clientConfig = {
auth: {
clientId: "your_client_id",
Expand All @@ -38,7 +48,7 @@ const clientConfig = {
thumbprint: process.env.thumbprint,
privateKey: process.env.privateKey,
}, // OR
clientAssertion: "assertion",
clientAssertion: clientAssertionCallback, // or a predetermined clientAssertion string
},
};
const pca = new msal.ConfidentialClientApplication(clientConfig);
Expand All @@ -53,7 +63,7 @@ const pca = new msal.ConfidentialClientApplication(clientConfig);
- A Client credential is mandatory for confidential clients. Client credential can be a:
- `clientSecret` is secret string generated set on the app registration.
- `clientCertificate` is a certificate set on the app registration. The `thumbprint` is a X.509 SHA-1 thumbprint of the certificate, and the `privateKey` is the PEM encoded private key. `x5c` is the optional X.509 certificate chain used in [subject name/issuer auth scenarios](https://github.com/AzureAD/microsoft-authentication-library-for-js/blob/dev/lib/msal-node/docs/sni.md).
- `clientAssertion` is string that the application uses when requesting a token. The certificate used to sign the assertion should be set on the app registration. Assertion should be of type urn:ietf:params:oauth:client-assertion-type:jwt-bearer.
- `clientAssertion` is a ClientAssertion object containing an assertion string or a callback function that returns an assertion string that the application uses when requesting a token, as well as the assertion's type (urn:ietf:params:oauth:client-assertion-type:jwt-bearer). The callback is invoked every time MSAL needs to acquire a token from the token issuer. App developers should generally use the callback because assertions expire and new assertions need to be created. App developers are responsible for the assertion lifetime. Use [this mechanism](https://learn.microsoft.com/entra/workload-id/workload-identity-federation-create-trust) to get tokens for a downstream API using a Federated Identity Credential.

## Configure Authority

Expand Down
25 changes: 19 additions & 6 deletions lib/msal-node/src/client/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,9 @@ import {
createClientAuthError,
ClientAuthErrorCodes,
buildStaticAuthorityOptions,
ClientAssertion as ClientAssertionType,
getClientAssertion,
ClientAssertionCallback,
} from "@azure/msal-common";
import {
Configuration,
Expand Down Expand Up @@ -77,6 +80,9 @@ export abstract class ClientApplication {
* Client assertion passed by the user for confidential client flows
*/
protected clientAssertion: ClientAssertion;
protected developerProvidedClientAssertion:
| string
| ClientAssertionCallback;
/**
* Client secret passed by the user for confidential client flows
*/
Expand Down Expand Up @@ -451,8 +457,8 @@ export abstract class ClientApplication {
serverTelemetryManager: serverTelemetryManager,
clientCredentials: {
clientSecret: this.clientSecret,
clientAssertion: this.clientAssertion
? this.getClientAssertion(discoveredAuthority)
clientAssertion: this.developerProvidedClientAssertion
? await this.getClientAssertion(discoveredAuthority)

This comment has been minimized.

Copy link
@ericcan

ericcan May 9, 2024

@Robbie-Microsoft

I'm thinking that this change may be related to the problem in #7082 . The certificate is translated into a client assertion (in msal-node\src\client\ConfidentialClientApplication.ts in the setClientCredential method:

this.clientAssertion = ClientAssertion.fromCertificate( certificate.thumbprint, certificate.privateKey, configuration.auth.clientCertificate?.x5c );

With the change to developerProvidedClientAssertion, I am thinking the clientAssertion is no longer included in the configuration. The changes to the getClientAssertion method overwrite the clientAssertion property so that method may also need to address the existence of a clientAssertion for the certificate

This comment has been minimized.

Copy link
@Robbie-Microsoft

Robbie-Microsoft May 9, 2024

Author Collaborator

excellent, thanks for this @ericcan. I will be working on this today.

: undefined,
},
libraryInfo: {
Expand All @@ -469,10 +475,17 @@ export abstract class ClientApplication {
return clientConfiguration;
}

private getClientAssertion(authority: Authority): {
assertion: string;
assertionType: string;
} {
private async getClientAssertion(
authority: Authority
): Promise<ClientAssertionType> {
this.clientAssertion = ClientAssertion.fromAssertion(
await getClientAssertion(
this.developerProvidedClientAssertion,
this.config.auth.clientId,
authority.tokenEndpoint
)
);

return {
assertion: this.clientAssertion.getJwt(
this.cryptoProvider,
Expand Down
Loading

0 comments on commit e6d18c5

Please sign in to comment.