Skip to content

Commit 35faa26

Browse files
committed
Address PR feedback
1 parent d6acf80 commit 35faa26

File tree

2 files changed

+10
-16
lines changed

2 files changed

+10
-16
lines changed

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

Lines changed: 7 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -101,12 +101,7 @@ private void initClientAuthentication(IClientCredential clientCredential) {
101101
} else if (clientCredential instanceof ClientCertificate) {
102102
this.clientCertAuthentication = true;
103103
this.clientCertificate = (ClientCertificate) clientCredential;
104-
if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) {
105-
//When this was added, ADFS did not support SHA256 hashes for client certificates
106-
clientAuthentication = buildValidClientCertificateAuthority(true);
107-
} else {
108-
clientAuthentication = buildValidClientCertificateAuthority(false);
109-
}
104+
clientAuthentication = buildValidClientCertificateAuthority();
110105
} else if (clientCredential instanceof ClientAssertion) {
111106
clientAuthentication = createClientAuthFromClientAssertion((ClientAssertion) clientCredential);
112107
} else {
@@ -120,19 +115,17 @@ protected ClientAuthentication clientAuthentication() {
120115
final Date currentDateTime = new Date(System.currentTimeMillis());
121116
final Date expirationTime = ((PrivateKeyJWT) clientAuthentication).getJWTAuthenticationClaimsSet().getExpirationTime();
122117
if (expirationTime.before(currentDateTime)) {
123-
if (Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS) {
124-
clientAuthentication = buildValidClientCertificateAuthority(true);
125-
} else {
126-
clientAuthentication = buildValidClientCertificateAuthority(false);
127-
}
118+
clientAuthentication = buildValidClientCertificateAuthority();
128119
}
129120
}
130121
return clientAuthentication;
131122
}
132123

133-
//The library originally used SHA-1 for thumbprints as other algorithms were not supported server-side,
134-
// and while support for SHA-256 has been added certain flows still only allow SHA-1
135-
private ClientAuthentication buildValidClientCertificateAuthority(boolean useSha1) {
124+
private ClientAuthentication buildValidClientCertificateAuthority() {
125+
//The library originally used SHA-1 for thumbprints as other algorithms were not supported server-side.
126+
//When this was written support for SHA-256 had been added, however ADFS scenarios still only allowed SHA-1.
127+
boolean useSha1 = Authority.detectAuthorityType(this.authenticationAuthority.canonicalAuthorityUrl()) == AuthorityType.ADFS;
128+
136129
ClientAssertion clientAssertion = JwtHelper.buildJwt(
137130
clientId(),
138131
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
@@ -58,10 +58,11 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent
5858

5959
//SHA-256 is preferred, however certain flows still require SHA-1 due to what is supported server-side. If SHA-256
6060
// is not supported or the IClientCredential.publicCertificateHash256() method is not implemented, the library will default to SHA-1.
61-
if (useSha1 || credential.publicCertificateHash256() == null) {
61+
String hash256 = credential.publicCertificateHash256();
62+
if (useSha1 || hash256 == null) {
6263
builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHash()));
6364
} else {
64-
builder.x509CertSHA256Thumbprint(new Base64URL(credential.publicCertificateHash256()));
65+
builder.x509CertSHA256Thumbprint(new Base64URL(hash256));
6566
}
6667

6768
jwt = new SignedJWT(builder.build(), claimsSet);

0 commit comments

Comments
 (0)