Skip to content

Commit

Permalink
Deprecated SHA-1 Thumbprints and Added SHA-256 support in Auth Flow w…
Browse files Browse the repository at this point in the history
…ith Certificate (#7185)

SHA-1 thumbprints have been deprecated. It's now up to the developer to
pass in a SHA-256 thumbprint. A warning will be shown to the dev that
states the deprecation.

This is a quick-fix that puts the burden of providing a SHA-2 thumbprint
on the developer.

Manual testing has been completed.
  • Loading branch information
Robbie-Microsoft authored Jul 12, 2024
1 parent 2adfb68 commit 2ff18f2
Show file tree
Hide file tree
Showing 11 changed files with 225 additions and 42 deletions.
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"type": "minor",
"comment": "Deprecated SHA-1 thumbprint for clientCertificate #7185",
"packageName": "@azure/msal-node",
"email": "[email protected]",
"dependentChangeType": "patch"
}
5 changes: 4 additions & 1 deletion lib/msal-node/apiReview/msal-node.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -169,7 +169,9 @@ export abstract class ClientApplication {
// @public
export class ClientAssertion {
static fromAssertion(assertion: string): ClientAssertion;
// @deprecated (undocumented)
static fromCertificate(thumbprint: string, privateKey: string, publicCertificate?: string): ClientAssertion;
static fromCertificateWithSha256Thumbprint(thumbprint: string, privateKey: string, publicCertificate?: string): ClientAssertion;
getJwt(cryptoProvider: CryptoProvider, issuer: string, jwtAudience: string): string;
static parseCertificate(publicCertificate: string): Array<string>;
}
Expand Down Expand Up @@ -475,7 +477,8 @@ export type NodeAuthOptions = {
clientSecret?: string;
clientAssertion?: string | ClientAssertionCallback;
clientCertificate?: {
thumbprint: string;
thumbprint?: string;
thumbprintSha256?: string;
privateKey: string;
x5c?: string;
};
Expand Down
38 changes: 36 additions & 2 deletions lib/msal-node/src/client/ClientAssertion.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ export class ClientAssertion {
private jwt: string;
private privateKey: string;
private thumbprint: string;
private useSha256: boolean;
private expirationTime: number;
private issuer: string;
private jwtAudience: string;
Expand All @@ -38,6 +39,7 @@ export class ClientAssertion {
}

/**
* @deprecated Use fromCertificateWithSha256Thumbprint instead, with a SHA-256 thumprint
* Initialize the ClientAssertion class from the certificate passed by the user
* @param thumbprint - identifier of a certificate
* @param privateKey - secret key
Expand All @@ -51,6 +53,29 @@ export class ClientAssertion {
const clientAssertion = new ClientAssertion();
clientAssertion.privateKey = privateKey;
clientAssertion.thumbprint = thumbprint;
clientAssertion.useSha256 = false;
if (publicCertificate) {
clientAssertion.publicCertificate =
this.parseCertificate(publicCertificate);
}
return clientAssertion;
}

/**
* Initialize the ClientAssertion class from the certificate passed by the user
* @param thumbprint - identifier of a certificate
* @param privateKey - secret key
* @param publicCertificate - electronic document provided to prove the ownership of the public key
*/
public static fromCertificateWithSha256Thumbprint(
thumbprint: string,
privateKey: string,
publicCertificate?: string
): ClientAssertion {
const clientAssertion = new ClientAssertion();
clientAssertion.privateKey = privateKey;
clientAssertion.thumbprint = thumbprint;
clientAssertion.useSha256 = true;
if (publicCertificate) {
clientAssertion.publicCertificate =
this.parseCertificate(publicCertificate);
Expand Down Expand Up @@ -109,12 +134,21 @@ export class ClientAssertion {

const header: jwt.JwtHeader = {
alg: JwtConstants.RSA_256,
x5t: EncodingUtils.base64EncodeUrl(this.thumbprint, "hex"),
};

const thumbprintHeader = this.useSha256
? JwtConstants.X5T_256
: JwtConstants.X5T;
Object.assign(header, {
[thumbprintHeader]: EncodingUtils.base64EncodeUrl(
this.thumbprint,
"hex"
),
} as Partial<jwt.JwtHeader>);

if (this.publicCertificate) {
Object.assign(header, {
x5c: this.publicCertificate,
[JwtConstants.X5C]: this.publicCertificate,
} as Partial<jwt.JwtHeader>);
}

Expand Down
43 changes: 24 additions & 19 deletions lib/msal-node/src/client/ConfidentialClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import {
AuthenticationResult,
AzureRegionConfiguration,
AuthError,
Constants,
IAppTokenProvider,
OIDC_DEFAULT_SCOPES,
UrlString,
Expand Down Expand Up @@ -67,7 +66,7 @@ export class ConfidentialClientApplication
*/
constructor(configuration: Configuration) {
super(configuration);
this.setClientCredential(this.config);
this.setClientCredential();
this.appTokenProvider = undefined;
}

Expand Down Expand Up @@ -218,15 +217,13 @@ export class ConfidentialClientApplication
}
}

private setClientCredential(configuration: Configuration): void {
const clientSecretNotEmpty = !!configuration.auth.clientSecret;
const clientAssertionNotEmpty = !!configuration.auth.clientAssertion;
const certificate = configuration.auth.clientCertificate || {
thumbprint: Constants.EMPTY_STRING,
privateKey: Constants.EMPTY_STRING,
};
private setClientCredential(): void {
const clientSecretNotEmpty = !!this.config.auth.clientSecret;
const clientAssertionNotEmpty = !!this.config.auth.clientAssertion;
const certificateNotEmpty =
!!certificate.thumbprint || !!certificate.privateKey;
(!!this.config.auth.clientCertificate.thumbprint ||
!!this.config.auth.clientCertificate.thumbprintSha256) &&
!!this.config.auth.clientCertificate.privateKey;

/*
* If app developer configures this callback, they don't need a credential
Expand All @@ -247,14 +244,14 @@ export class ConfidentialClientApplication
);
}

if (configuration.auth.clientSecret) {
this.clientSecret = configuration.auth.clientSecret;
if (this.config.auth.clientSecret) {
this.clientSecret = this.config.auth.clientSecret;
return;
}

if (configuration.auth.clientAssertion) {
if (this.config.auth.clientAssertion) {
this.developerProvidedClientAssertion =
configuration.auth.clientAssertion;
this.config.auth.clientAssertion;
return;
}

Expand All @@ -263,11 +260,19 @@ export class ConfidentialClientApplication
ClientAuthErrorCodes.invalidClientCredential
);
} else {
this.clientAssertion = ClientAssertion.fromCertificate(
certificate.thumbprint,
certificate.privateKey,
configuration.auth.clientCertificate?.x5c
);
this.clientAssertion = !!this.config.auth.clientCertificate
.thumbprintSha256
? ClientAssertion.fromCertificateWithSha256Thumbprint(
this.config.auth.clientCertificate.thumbprintSha256,
this.config.auth.clientCertificate.privateKey,
this.config.auth.clientCertificate.x5c
)
: ClientAssertion.fromCertificate(
// guaranteed to be a string, due to prior error checking in this function
this.config.auth.clientCertificate.thumbprint as string,
this.config.auth.clientCertificate.privateKey,
this.config.auth.clientCertificate.x5c
);
}
}
}
20 changes: 18 additions & 2 deletions lib/msal-node/src/config/Configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,14 +27,15 @@ import {
} from "../utils/Constants.js";
import { LinearRetryPolicy } from "../retry/LinearRetryPolicy.js";
import { HttpClientWithRetries } from "../network/HttpClientWithRetries.js";
import { NodeAuthError } from "../error/NodeAuthError.js";

/**
* - clientId - Client id of the application.
* - authority - Url of the authority. If no value is set, defaults to https://login.microsoftonline.com/common.
* - knownAuthorities - Needed for Azure B2C and ADFS. All authorities that will be used in the client application. Only the host of the authority should be passed in.
* - clientSecret - Secret string that the application uses when requesting a token. Only used in confidential client applications. Can be created in the Azure app registration portal.
* - clientAssertion - 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). Only used in confidential client applications.
* - clientCertificate - Certificate that the application uses when requesting a token. Only used in confidential client applications. Requires hex encoded X.509 SHA-1 thumbprint of the certificiate, and the PEM encoded private key (string should contain -----BEGIN PRIVATE KEY----- ... -----END PRIVATE KEY----- )
* - clientCertificate - Certificate that the application uses when requesting a token. Only used in confidential client applications. Requires hex encoded X.509 SHA-1 or SHA-256 thumbprint of the certificate, and the PEM encoded private key (string should contain -----BEGIN PRIVATE KEY----- ... -----END PRIVATE KEY----- )
* - protocolMode - Enum that represents the protocol that msal follows. Used for configuring proper endpoints.
* - skipAuthorityMetadataCache - A flag to choose whether to use or not use the local metadata cache during authority initialization. Defaults to false.
* @public
Expand All @@ -45,7 +46,12 @@ export type NodeAuthOptions = {
clientSecret?: string;
clientAssertion?: string | ClientAssertionCallback;
clientCertificate?: {
thumbprint: string;
/**
* @deprecated Use thumbprintSha2 property instead. Thumbprint needs to be computed with SHA-256 algorithm.
* SHA-1 is only needed for backwards compatibility with older versions of ADFS.
*/
thumbprint?: string;
thumbprintSha256?: string;
privateKey: string;
x5c?: string;
};
Expand Down Expand Up @@ -138,6 +144,7 @@ const DEFAULT_AUTH_OPTIONS: Required<NodeAuthOptions> = {
clientAssertion: Constants.EMPTY_STRING,
clientCertificate: {
thumbprint: Constants.EMPTY_STRING,
thumbprintSha256: Constants.EMPTY_STRING,
privateKey: Constants.EMPTY_STRING,
x5c: Constants.EMPTY_STRING,
},
Expand Down Expand Up @@ -217,6 +224,15 @@ export function buildAppConfiguration({
disableInternalRetries: system?.disableInternalRetries || false,
};

// if client certificate was provided, ensure that at least one of the SHA-1 or SHA-256 thumbprints were provided
if (
!!auth.clientCertificate &&
!!!auth.clientCertificate.thumbprint &&
!!!auth.clientCertificate.thumbprintSha256
) {
throw NodeAuthError.createStateNotFoundError();
}

return {
auth: { ...DEFAULT_AUTH_OPTIONS, ...auth },
broker: { ...broker },
Expand Down
14 changes: 14 additions & 0 deletions lib/msal-node/src/error/NodeAuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,10 @@ export const NodeAuthErrorMessage = {
code: "state_not_found",
desc: "State not found. Please verify that the request originated from msal.",
},
thumbprintMissing: {
code: "thumbprint_missing_from_client_certificate",
desc: "Client certificate does not contain a SHA-1 or SHA-256 thumbprint.",
},
};

export class NodeAuthError extends AuthError {
Expand Down Expand Up @@ -114,4 +118,14 @@ export class NodeAuthError extends AuthError {
NodeAuthErrorMessage.stateNotFoundError.desc
);
}

/**
* Creates an error thrown when client certificate was provided, but neither the SHA-1 or SHA-256 thumbprints were provided
*/
static createThumbprintMissingError(): NodeAuthError {
return new NodeAuthError(
NodeAuthErrorMessage.thumbprintMissing.code,
NodeAuthErrorMessage.thumbprintMissing.desc
);
}
}
1 change: 1 addition & 0 deletions lib/msal-node/src/utils/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,7 @@ export type ApiId = (typeof ApiId)[keyof typeof ApiId];
export const JwtConstants = {
ALGORITHM: "alg",
RSA_256: "RS256",
X5T_256: "x5t#S256",
X5T: "x5t",
X5C: "x5c",
AUDIENCE: "aud",
Expand Down
Loading

0 comments on commit 2ff18f2

Please sign in to comment.