Skip to content

Commit

Permalink
[Identity] What if we accepted the PEM certificate string contents? (#…
Browse files Browse the repository at this point in the history
…18017)

This PR serves to explore the idea of changing the public API of ClientCertificateCredential before we release v2, to allow users to either pass the path to a PEM certificate in the filesystem, or the string certificate contents directly.

.NET allows a similar set of alternatives (with more flexibility on the type of certificates than us): https://github.com/Azure/azure-sdk-for-net/blob/main/sdk/identity/Azure.Identity/api/Azure.Identity.netstandard2.0.cs#L96

This could help us provide a better experience for issues similar to #17715

Feedback appreciated 🙏
  • Loading branch information
sadasant authored Oct 12, 2021
1 parent 68fbad6 commit 8066272
Show file tree
Hide file tree
Showing 11 changed files with 300 additions and 47 deletions.
1 change: 1 addition & 0 deletions sdk/identity/identity/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,7 @@ Azure Service Fabric support hasn't been added on the initial version 2 of Ident

#### Other features

- `ClientCertificateCredential` now optionally accepts a configuration object as its third constructor parameter, instead of the PEM certificate path. This new object, called `ClientCertificateCredentialPEMConfiguration`, can contain either the PEM certificate path with the `certificatePath` property, or the contents of the PEM certificate with the `certificate` property..
- The Node.js version of `InteractiveBrowserCredential` has [Proof Key for Code Exchange (PKCE)](https://datatracker.ietf.org/doc/html/rfc7636) enabled by default.
- `InteractiveBrowserCredential` has a new `loginHint` constructor option, which allows a username to be pre-selected for interactive logins.
- In `AzureCliCredential`, we allow specifying a `tenantId` in the parameters through the `AzureCliCredentialOptions`.
Expand Down

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions sdk/identity/identity/review/identity.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ export class ChainedTokenCredential implements TokenCredential {
// @public
export class ClientCertificateCredential implements TokenCredential {
constructor(tenantId: string, clientId: string, certificatePath: string, options?: ClientCertificateCredentialOptions);
constructor(tenantId: string, clientId: string, configuration: ClientCertificateCredentialPEMConfiguration, options?: ClientCertificateCredentialOptions);
getToken(scopes: string | string[], options?: GetTokenOptions): Promise<AccessToken>;
}

Expand All @@ -113,6 +114,15 @@ export interface ClientCertificateCredentialOptions extends TokenCredentialOptio
sendCertificateChain?: boolean;
}

// @public
export type ClientCertificateCredentialPEMConfiguration = {
certificate: string;
certificatePath?: never;
} | {
certificate?: never;
certificatePath: string;
};

// @public
export class ClientSecretCredential implements TokenCredential {
constructor(tenantId: string, clientId: string, clientSecret: string, options?: ClientSecretCredentialOptions);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,33 @@ import { trace } from "../util/tracing";
import { MsalFlow } from "../msal/flows";
import { ClientCertificateCredentialOptions } from "./clientCertificateCredentialOptions";

const logger = credentialLogger("ClientCertificateCredential");
const credentialName = "ClientCertificateCredential";
const logger = credentialLogger(credentialName);

/**
* Required configuration options for the {@link ClientCertificateCredential}, with either the string contents of a PEM certificate, or the path to a PEM certificate.
*/
export type ClientCertificateCredentialPEMConfiguration =
| {
/**
* The PEM-encoded public/private key certificate on the filesystem.
*/
certificate: string;
/**
* The PEM-encoded public/private key certificate on the filesystem should not be provided if `certificate` is provided.
*/
certificatePath?: never;
}
| {
/**
* The PEM-encoded public/private key certificate on the filesystem should not be provided if `certificatePath` is provided.
*/
certificate?: never;
/**
* The path to the PEM-encoded public/private key certificate on the filesystem.
*/
certificatePath: string;
};

/**
* Enables authentication to Azure Active Directory using a PEM-encoded
Expand All @@ -35,16 +61,53 @@ export class ClientCertificateCredential implements TokenCredential {
tenantId: string,
clientId: string,
certificatePath: string,
options?: ClientCertificateCredentialOptions
);
/**
* Creates an instance of the ClientCertificateCredential with the details
* needed to authenticate against Azure Active Directory with a certificate.
*
* @param tenantId - The Azure Active Directory tenant (directory) ID.
* @param clientId - The client (application) ID of an App Registration in the tenant.
* @param configuration - Other parameters required, including the PEM-encoded certificate as a string, or as a path on the filesystem.
* If the type is ignored, we will throw if both the value of the PEM certificate and the path to a PEM certificate are provided at the same time.
* @param options - Options for configuring the client which makes the authentication request.
*/
constructor(
tenantId: string,
clientId: string,
configuration: ClientCertificateCredentialPEMConfiguration,
options?: ClientCertificateCredentialOptions
);
constructor(
tenantId: string,
clientId: string,
certificatePathOrConfiguration: string | ClientCertificateCredentialPEMConfiguration,
options: ClientCertificateCredentialOptions = {}
) {
if (!tenantId || !clientId || !certificatePath) {
if (!tenantId || !clientId) {
throw new Error(`${credentialName}: tenantId and clientId are required parameters.`);
}
const configuration: ClientCertificateCredentialPEMConfiguration = {
...(typeof certificatePathOrConfiguration === "string"
? {
certificatePath: certificatePathOrConfiguration
}
: certificatePathOrConfiguration)
};
if (!configuration || !(configuration.certificate || configuration.certificatePath)) {
throw new Error(
`${credentialName}: Provide either a PEM certificate in string form, or the path to that certificate in the filesystem.`
);
}
if (configuration.certificate && configuration.certificatePath) {
throw new Error(
"ClientCertificateCredential: tenantId, clientId, and certificatePath are required parameters."
`${credentialName}: To avoid unexpected behaviors, providing both the contents of a PEM certificate and the path to a PEM certificate is forbidden.`
);
}
this.msalFlow = new MsalClientCertificate({
...options,
certificatePath,
configuration,
logger,
clientId,
tenantId,
Expand All @@ -62,7 +125,7 @@ export class ClientCertificateCredential implements TokenCredential {
* TokenCredential implementation might make.
*/
async getToken(scopes: string | string[], options: GetTokenOptions = {}): Promise<AccessToken> {
return trace(`${this.constructor.name}.getToken`, options, async (newOptions) => {
return trace(`${credentialName}.getToken`, options, async (newOptions) => {
const arrayScopes = Array.isArray(scopes) ? scopes : [scopes];
return this.msalFlow.getToken(arrayScopes, newOptions);
});
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,7 @@ export class EnvironmentCredential implements TokenCredential {
this._credential = new ClientCertificateCredential(
tenantId,
clientId,
certificatePath,
{ certificatePath },
options
);
return;
Expand Down
5 changes: 4 additions & 1 deletion sdk/identity/identity/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ export {
} from "./credentials/environmentCredential";
export { ClientSecretCredential } from "./credentials/clientSecretCredential";
export { ClientSecretCredentialOptions } from "./credentials/clientSecretCredentialOptions";
export { ClientCertificateCredential } from "./credentials/clientCertificateCredential";
export {
ClientCertificateCredential,
ClientCertificateCredentialPEMConfiguration
} from "./credentials/clientCertificateCredential";
export { ClientCertificateCredentialOptions } from "./credentials/clientCertificateCredentialOptions";
export { CredentialPersistenceOptions } from "./credentials/credentialPersistenceOptions";
export { AzureCliCredential } from "./credentials/azureCliCredential";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { AccessToken } from "@azure/core-auth";
import { MsalNodeOptions, MsalNode } from "./nodeCommon";
import { formatError } from "../../util/logging";
import { CredentialFlowGetTokenOptions } from "../credentials";
import { ClientCertificateCredentialPEMConfiguration } from "../../credentials/clientCertificateCredential";

const readFileAsync = promisify(readFile);

Expand All @@ -20,7 +21,7 @@ export interface MSALClientCertificateOptions extends MsalNodeOptions {
/**
* Location of the PEM certificate.
*/
certificatePath: string;
configuration: ClientCertificateCredentialPEMConfiguration;
/**
* Option to include x5c header for SubjectName and Issuer name authorization.
* Set this option to send base64 encoded public certificate in the client assertion header as an x5c claim
Expand Down Expand Up @@ -50,18 +51,19 @@ interface CertificateParts {
/**
* Tries to asynchronously load a certificate from the given path.
*
* @param certificatePath - Path to the certificate.
* @param configuration - Either the PEM value or the path to the certificate.
* @param sendCertificateChain - Option to include x5c header for SubjectName and Issuer name authorization.
* @returns - The certificate parts, or `undefined` if the certificate could not be loaded.
* @internal
*/
export async function parseCertificate(
certificatePath: string,
configuration: ClientCertificateCredentialPEMConfiguration,
sendCertificateChain?: boolean
): Promise<CertificateParts> {
const certificateParts: Partial<CertificateParts> = {};

certificateParts.certificateContents = await readFileAsync(certificatePath, "utf8");
certificateParts.certificateContents =
configuration.certificate || (await readFileAsync(configuration.certificatePath!, "utf8"));
if (sendCertificateChain) {
certificateParts.x5c = certificateParts.certificateContents;
}
Expand Down Expand Up @@ -95,20 +97,20 @@ export async function parseCertificate(
* @internal
*/
export class MsalClientCertificate extends MsalNode {
private certificatePath: string;
private configuration: ClientCertificateCredentialPEMConfiguration;
private sendCertificateChain?: boolean;

constructor(options: MSALClientCertificateOptions) {
super(options);
this.requiresConfidential = true;
this.certificatePath = options.certificatePath;
this.configuration = options.configuration;
this.sendCertificateChain = options.sendCertificateChain;
}

// Changing the MSAL configuration asynchronously
async init(options?: CredentialFlowGetTokenOptions): Promise<void> {
try {
const parts = await parseCertificate(this.certificatePath, this.sendCertificateChain);
const parts = await parseCertificate(this.configuration, this.sendCertificateChain);
this.msalConfig.auth.clientCertificate = {
thumbprint: parts.thumbprint,
privateKey: parts.certificateContents,
Expand Down
5 changes: 4 additions & 1 deletion sdk/identity/identity/src/msal/nodeFlows/msalOnBehalfOf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@ export class MsalOnBehalfOf extends MsalNode {
async init(options?: CredentialFlowGetTokenOptions): Promise<void> {
if (this.certificatePath) {
try {
const parts = await parseCertificate(this.certificatePath, this.sendCertificateChain);
const parts = await parseCertificate(
{ certificatePath: this.certificatePath },
this.sendCertificateChain
);
this.msalConfig.auth.clientCertificate = {
thumbprint: parts.thumbprint,
privateKey: parts.certificateContents,
Expand Down
Loading

0 comments on commit 8066272

Please sign in to comment.