Skip to content

Commit cb7c22b

Browse files
committed
Address PR feedback
1 parent 30a9eee commit cb7c22b

File tree

3 files changed

+9
-9
lines changed

3 files changed

+9
-9
lines changed

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientCertificate.java

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,8 +8,6 @@
88

99
import java.io.IOException;
1010
import java.io.InputStream;
11-
import java.lang.reflect.InvocationTargetException;
12-
import java.lang.reflect.Method;
1311
import java.security.KeyStore;
1412
import java.security.KeyStoreException;
1513
import java.security.MessageDigest;
@@ -21,7 +19,6 @@
2119
import java.security.cert.CertificateEncodingException;
2220
import java.security.cert.CertificateException;
2321
import java.security.cert.X509Certificate;
24-
import java.security.interfaces.RSAPrivateKey;
2522
import java.util.ArrayList;
2623
import java.util.Arrays;
2724
import java.util.Base64;
@@ -53,7 +50,7 @@ public String publicCertificateHash()
5350
throws CertificateEncodingException, NoSuchAlgorithmException {
5451

5552
return Base64.getEncoder().encodeToString(ClientCertificate
56-
.getHash(publicKeyCertificateChain.get(0).getEncoded()));
53+
.getHashSha256(publicKeyCertificateChain.get(0).getEncoded()));
5754
}
5855

5956
public String publicCertificateHashSha1()
@@ -132,7 +129,7 @@ private static byte[] getHashSha1(final byte[] inputBytes) throws NoSuchAlgorith
132129
return md.digest();
133130
}
134131

135-
private static byte[] getHash(final byte[] inputBytes) throws NoSuchAlgorithmException {
132+
private static byte[] getHashSha256(final byte[] inputBytes) throws NoSuchAlgorithmException {
136133
final MessageDigest md = MessageDigest.getInstance("SHA-256");
137134
md.update(inputBytes);
138135
return md.digest();

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ConfidentialClientApplication.java

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -102,7 +102,7 @@ private void initClientAuthentication(IClientCredential clientCredential) {
102102
this.clientCertAuthentication = true;
103103
this.clientCertificate = (ClientCertificate) clientCredential;
104104
if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) {
105-
clientAuthentication = buildValidClientCertificateAuthorityLegacySha1();
105+
clientAuthentication = buildValidClientCertificateAuthoritySha1();
106106
} else {
107107
clientAuthentication = buildValidClientCertificateAuthority();
108108
}
@@ -136,7 +136,9 @@ private ClientAuthentication buildValidClientCertificateAuthority() {
136136
return createClientAuthFromClientAssertion(clientAssertion);
137137
}
138138

139-
private ClientAuthentication buildValidClientCertificateAuthorityLegacySha1() {
139+
//The library originally used SHA-1 for thumbprints as other algorithms were not supported server-side,
140+
// and while support for SHA-256 has been added certain flows still only allow SHA-1
141+
private ClientAuthentication buildValidClientCertificateAuthoritySha1() {
140142
ClientAssertion clientAssertion = JwtHelper.buildJwt(
141143
clientId(),
142144
clientCertificate,

msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ final class JwtHelper {
2222

2323
static ClientAssertion buildJwt(String clientId, final ClientCertificate credential,
2424
final String jwtAudience, boolean sendX5c,
25-
boolean useLegacySha1) throws MsalClientException {
25+
boolean useSha1) throws MsalClientException {
2626
if (StringHelper.isBlank(clientId)) {
2727
throw new IllegalArgumentException("clientId is null or empty");
2828
}
@@ -56,7 +56,8 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent
5656
builder.x509CertChain(certs);
5757
}
5858

59-
if (useLegacySha1) {
59+
//SHA-256 is preferred, however certain flows still require SHA-1 due to what is supported server-side
60+
if (useSha1) {
6061
builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHashSha1()));
6162
} else {
6263
builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash()));

0 commit comments

Comments
 (0)