From f2644562396a2fcde0521be4948954540073580c Mon Sep 17 00:00:00 2001 From: avdunn Date: Tue, 22 Apr 2025 17:18:48 -0700 Subject: [PATCH 1/2] Remove and replace various Nimbus imports --- .../aad/msal4j/AuthenticationErrorCode.java | 5 + .../microsoft/aad/msal4j/ClientAssertion.java | 3 +- .../com/microsoft/aad/msal4j/ClientInfo.java | 6 +- .../com/microsoft/aad/msal4j/IBroker.java | 18 ++-- .../com/microsoft/aad/msal4j/IdToken.java | 41 -------- .../com/microsoft/aad/msal4j/JwtHelper.java | 93 ++++++++++--------- .../aad/msal4j/SAML11BearerGrant.java | 33 ------- .../aad/msal4j/TokenRequestExecutor.java | 10 +- .../aad/msal4j/HelperAndUtilityTests.java | 72 ++++++++++++++ 9 files changed, 149 insertions(+), 132 deletions(-) delete mode 100644 msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SAML11BearerGrant.java create mode 100644 msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java index 5c568831..690a1e2c 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/AuthenticationErrorCode.java @@ -93,6 +93,11 @@ public class AuthenticationErrorCode { */ public final static String INVALID_REDIRECT_URI = "invalid_redirect_uri"; + /** + * Indicates token endpoint is invalid. Ensure authority and tenant are correctly set, as this endpoint is typically created using those values. + */ + public final static String INVALID_ENDPOINT_URI = "invalid_endpoint_uri"; + /** * MSAL was unable to open the user-default browser. This is either because the current platform * does not support {@link java.awt.Desktop} or {@link java.awt.Desktop.Action#BROWSE}. Interactive diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java index 12c9e566..7c4e456b 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java @@ -3,7 +3,6 @@ package com.microsoft.aad.msal4j; -import com.nimbusds.oauth2.sdk.auth.JWTAuthentication; import lombok.EqualsAndHashCode; import lombok.Getter; import lombok.experimental.Accessors; @@ -13,7 +12,7 @@ @EqualsAndHashCode final class ClientAssertion implements IClientAssertion { - static final String assertionType = JWTAuthentication.CLIENT_ASSERTION_TYPE; + static final String ASSERTION_TYPE = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; private final String assertion; ClientAssertion(final String assertion) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientInfo.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientInfo.java index c9b2f83a..f09d2a93 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientInfo.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientInfo.java @@ -4,8 +4,8 @@ package com.microsoft.aad.msal4j; import com.fasterxml.jackson.annotation.JsonProperty; -import com.nimbusds.jose.util.StandardCharset; +import java.nio.charset.StandardCharsets; import java.util.Base64; import static com.microsoft.aad.msal4j.Constants.POINT_DELIMITER; @@ -23,9 +23,9 @@ public static ClientInfo createFromJson(String clientInfoJsonBase64Encoded) { return null; } - byte[] decodedInput = Base64.getUrlDecoder().decode(clientInfoJsonBase64Encoded.getBytes(StandardCharset.UTF_8)); + byte[] decodedInput = Base64.getUrlDecoder().decode(clientInfoJsonBase64Encoded.getBytes(StandardCharsets.UTF_8)); - return JsonHelper.convertJsonToObject(new String(decodedInput, StandardCharset.UTF_8), ClientInfo.class); + return JsonHelper.convertJsonToObject(new String(decodedInput, StandardCharsets.UTF_8), ClientInfo.class); } String toAccountIdentifier() { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java index ab3f0ce0..9f50fc7a 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java @@ -3,9 +3,9 @@ package com.microsoft.aad.msal4j; -import com.nimbusds.jwt.JWTParser; - import java.net.URL; +import java.nio.charset.StandardCharsets; +import java.util.Base64; import java.util.concurrent.CompletableFuture; /** @@ -67,11 +67,17 @@ default IAuthenticationResult parseBrokerAuthResult(String authority, String idT if (idToken != null) { builder.idToken(idToken); if (accountId != null) { - String idTokenJson = - JWTParser.parse(idToken).getParsedParts()[1].decodeToString(); + String idTokenJson; + + try { + idTokenJson = new String(Base64.getDecoder().decode(idToken.split("\\.")[1]), StandardCharsets.UTF_8); + } catch (ArrayIndexOutOfBoundsException e) { + throw new MsalServiceException("Error parsing ID token, missing payload section. Ensure that the ID token is following the JWT format.", + AuthenticationErrorCode.INVALID_JWT); + } + builder.accountCacheEntity(AccountCacheEntity.create(clientInfo, - Authority.createAuthority(new URL(authority)), JsonHelper.convertJsonToObject(idTokenJson, - IdToken.class), null)); + Authority.createAuthority(new URL(authority)), JsonHelper.convertJsonToObject(idTokenJson, IdToken.class), null)); } } if (accessToken != null) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IdToken.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IdToken.java index 04964b36..0425f368 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IdToken.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IdToken.java @@ -4,29 +4,10 @@ package com.microsoft.aad.msal4j; import com.fasterxml.jackson.annotation.JsonProperty; -import com.nimbusds.jwt.JWTClaimsSet; - -import java.text.ParseException; -import java.util.HashMap; -import java.util.Map; - import java.io.Serializable; class IdToken implements Serializable { - static final String ISSUER = "iss"; - static final String SUBJECT = "sub"; - static final String AUDIENCE = "aud"; - static final String EXPIRATION_TIME = "exp"; - static final String ISSUED_AT = "issuedAt"; - static final String NOT_BEFORE = "nbf"; - static final String NAME = "name"; - static final String PREFERRED_USERNAME = "preferred_username"; - static final String OBJECT_IDENTIFIER = "oid"; - static final String TENANT_IDENTIFIER = "tid"; - static final String UPN = "upn"; - static final String UNIQUE_NAME = "unique_name"; - @JsonProperty("iss") protected String issuer; @@ -62,26 +43,4 @@ class IdToken implements Serializable { @JsonProperty("unique_name") protected String uniqueName; - - static IdToken createFromJWTClaims(final JWTClaimsSet claims) throws ParseException { - IdToken idToken = new IdToken(); - - idToken.issuer = claims.getStringClaim(ISSUER); - idToken.subject = claims.getStringClaim(SUBJECT); - idToken.audience = claims.getStringClaim(AUDIENCE); - - idToken.expirationTime = claims.getLongClaim(EXPIRATION_TIME); - idToken.issuedAt = claims.getLongClaim(ISSUED_AT); - idToken.notBefore = claims.getLongClaim(NOT_BEFORE); - - idToken.name = claims.getStringClaim(NAME); - idToken.preferredUsername = claims.getStringClaim(PREFERRED_USERNAME); - idToken.objectIdentifier = claims.getStringClaim(OBJECT_IDENTIFIER); - idToken.tenantIdentifier = claims.getStringClaim(TENANT_IDENTIFIER); - - idToken.upn = claims.getStringClaim(UPN); - idToken.uniqueName = claims.getStringClaim(UNIQUE_NAME); - - return idToken; - } } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java index 2e928809..bca0cb82 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java @@ -3,76 +3,85 @@ package com.microsoft.aad.msal4j; +import java.nio.charset.StandardCharsets; +import java.security.Signature; import java.util.ArrayList; -import java.util.Collections; -import java.util.Date; +import java.util.Base64; +import java.util.HashMap; import java.util.List; +import java.util.Map; import java.util.UUID; -import com.nimbusds.jose.JWSAlgorithm; -import com.nimbusds.jose.JWSHeader; -import com.nimbusds.jose.JWSHeader.Builder; -import com.nimbusds.jose.crypto.RSASSASigner; -import com.nimbusds.jose.util.Base64; -import com.nimbusds.jose.util.Base64URL; -import com.nimbusds.jwt.JWTClaimsSet; -import com.nimbusds.jwt.SignedJWT; - final class JwtHelper { static ClientAssertion buildJwt(String clientId, final ClientCertificate credential, final String jwtAudience, boolean sendX5c, boolean useSha1) throws MsalClientException { - if (StringHelper.isBlank(clientId)) { - throw new IllegalArgumentException("clientId is null or empty"); - } - - if (credential == null) { - throw new IllegalArgumentException("credential is null"); - } - final long time = System.currentTimeMillis(); + ParameterValidationUtils.validateNotBlank("clientId", clientId); + ParameterValidationUtils.validateNotNull("credential", clientId); - final JWTClaimsSet claimsSet = new JWTClaimsSet.Builder() - .audience(Collections.singletonList(jwtAudience)) - .issuer(clientId) - .jwtID(UUID.randomUUID().toString()) - .notBeforeTime(new Date(time)) - .expirationTime(new Date(time - + Constants.AAD_JWT_TOKEN_LIFETIME_SECONDS - * 1000)) - .subject(clientId) - .build(); - - SignedJWT jwt; try { - JWSHeader.Builder builder = new Builder(JWSAlgorithm.RS256); + final long time = System.currentTimeMillis(); + + // Build header + Map header = new HashMap<>(); + header.put("alg", "RS256"); + header.put("typ", "JWT"); if (sendX5c) { - List certs = new ArrayList<>(); + List certs = new ArrayList<>(); for (String cert : credential.getEncodedPublicKeyCertificateChain()) { - certs.add(new Base64(cert)); + certs.add(cert); } - builder.x509CertChain(certs); + header.put("x5c", certs); } //SHA-256 is preferred, however certain flows still require SHA-1 due to what is supported server-side. If SHA-256 // is not supported or the IClientCredential.publicCertificateHash256() method is not implemented, the library will default to SHA-1. String hash256 = credential.publicCertificateHash256(); if (useSha1 || hash256 == null) { - builder.x509CertThumbprint(new Base64URL(credential.publicCertificateHash())); + header.put("x5t", credential.publicCertificateHash()); } else { - builder.x509CertSHA256Thumbprint(new Base64URL(hash256)); + header.put("x5t#S256", hash256); } - jwt = new SignedJWT(builder.build(), claimsSet); - final RSASSASigner signer = new RSASSASigner(credential.privateKey()); + // Build payload + Map payload = new HashMap<>(); + payload.put("aud", jwtAudience); + payload.put("iss", clientId); + payload.put("jti", UUID.randomUUID().toString()); + payload.put("nbf", time / 1000); + payload.put("exp", time / 1000 + Constants.AAD_JWT_TOKEN_LIFETIME_SECONDS); + payload.put("sub", clientId); + + // Concatenate header and payload + String jsonHeader = JsonHelper.mapper.writeValueAsString(header); + String jsonPayload = JsonHelper.mapper.writeValueAsString(payload); - jwt.sign(signer); + String encodedHeader = base64UrlEncode(jsonHeader.getBytes(StandardCharsets.UTF_8)); + String encodedPayload = base64UrlEncode(jsonPayload.getBytes(StandardCharsets.UTF_8)); + + // Create signature + String dataToSign = encodedHeader + "." + encodedPayload; + + Signature sig = Signature.getInstance("SHA256withRSA"); + sig.initSign(credential.privateKey()); + sig.update(dataToSign.getBytes(StandardCharsets.UTF_8)); + byte[] signatureBytes = sig.sign(); + + String encodedSignature = base64UrlEncode(signatureBytes); + + // Build the JWT + String jwt = dataToSign + "." + encodedSignature; + + return new ClientAssertion(jwt); } catch (final Exception e) { throw new MsalClientException(e); } + } - return new ClientAssertion(jwt.serialize()); + private static String base64UrlEncode(byte[] data) { + return Base64.getUrlEncoder().withoutPadding().encodeToString(data); } -} +} \ No newline at end of file diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SAML11BearerGrant.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SAML11BearerGrant.java deleted file mode 100644 index 54f10d49..00000000 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/SAML11BearerGrant.java +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.microsoft.aad.msal4j; - -import java.util.Collections; -import java.util.List; -import java.util.Map; - -import com.nimbusds.jose.util.Base64URL; -import com.nimbusds.oauth2.sdk.GrantType; -import com.nimbusds.oauth2.sdk.SAML2BearerGrant; - -class SAML11BearerGrant extends SAML2BearerGrant { - - /** - * The grant type. - */ - public static GrantType grantType = new GrantType( - "urn:ietf:params:oauth:grant-type:saml1_1-bearer"); - - public SAML11BearerGrant(Base64URL assertion) { - super(assertion); - } - - @Override - public Map> toParameters() { - - Map> params = super.toParameters(); - params.put("grant_type", Collections.singletonList(grantType.getValue())); - return params; - } -} diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java index b86c58a2..ad0ac1d7 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java @@ -31,7 +31,7 @@ class TokenRequestExecutor { msalRequest.requestContext().apiParameters().tenant() ; } - AuthenticationResult executeTokenRequest() throws ParseException, IOException { + AuthenticationResult executeTokenRequest() throws IOException { log.debug("Sending token request to: {}", requestAuthority.canonicalAuthorityUrl()); OAuthHttpRequest oAuthHttpRequest = createOauthHttpRequest(); @@ -39,10 +39,11 @@ AuthenticationResult executeTokenRequest() throws ParseException, IOException { return createAuthenticationResultFromOauthHttpResponse(oauthHttpResponse); } - OAuthHttpRequest createOauthHttpRequest() throws SerializeException, MalformedURLException { + OAuthHttpRequest createOauthHttpRequest() throws MalformedURLException { if (requestAuthority.tokenEndpointUrl() == null) { - throw new SerializeException("The endpoint URI is not specified"); + throw new MsalClientException("The endpoint URI is not specified", + AuthenticationErrorCode.INVALID_ENDPOINT_URI); } final OAuthHttpRequest oauthHttpRequest = new OAuthHttpRequest( @@ -113,8 +114,7 @@ private void addJWTBearerAssertionParams(Map> queryParamete queryParameters.put("client_assertion_type", Collections.singletonList("urn:ietf:params:oauth:client-assertion-type:jwt-bearer")); } - private AuthenticationResult createAuthenticationResultFromOauthHttpResponse( - HttpResponse oauthHttpResponse) throws ParseException { + private AuthenticationResult createAuthenticationResultFromOauthHttpResponse(HttpResponse oauthHttpResponse) { AuthenticationResult result; if (oauthHttpResponse.statusCode() == HttpHelper.HTTP_STATUS_200) { diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java new file mode 100644 index 00000000..b81f9831 --- /dev/null +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java @@ -0,0 +1,72 @@ +package com.microsoft.aad.msal4j; + +import org.junit.jupiter.api.Test; + +import java.security.NoSuchAlgorithmException; +import java.security.cert.CertificateEncodingException; +import java.util.*; + +import static org.junit.jupiter.api.Assertions.*; +import static org.junit.jupiter.api.Assertions.assertTrue; +import static org.mockito.Mockito.*; + +class HelperAndUtilityTests { + + @Test + void JwtHelper_buildJwt_ValidSha1AndSha256Assertions() throws MsalClientException, CertificateEncodingException, NoSuchAlgorithmException { + ClientCertificate clientCertificateMock = mock(ClientCertificate.class); + when(clientCertificateMock.privateKey()).thenReturn(TestHelper.getPrivateKey()); + when(clientCertificateMock.publicCertificateHash()).thenReturn("certificateHash"); + when(clientCertificateMock.publicCertificateHash256()).thenReturn("certificateHash256"); + when(clientCertificateMock.getEncodedPublicKeyCertificateChain()).thenReturn(Arrays.asList("cert1", "cert2")); + + String clientId = "clientId"; + String audience = "https://login.microsoftonline.com/common/oauth2/v2.0/token"; + + //Sha256 assertion + ClientAssertion clientAssertion = JwtHelper.buildJwt(clientId, clientCertificateMock, audience, true, false); + + assertNotNull(clientAssertion); + assertNotNull(clientAssertion.assertion()); + + // Verify JWT structure (header.payload.signature) + String jwt = clientAssertion.assertion(); + String[] jwtParts = jwt.split("\\."); + assertEquals(3, jwtParts.length, "JWT should have three parts"); + + // Decode and verify headers + String headerJson = new String(Base64.getUrlDecoder().decode(jwtParts[0])); + assertTrue(headerJson.contains("\"alg\":\"RS256\""), "Header should specify RS256 algorithm"); + assertTrue(headerJson.contains("\"typ\":\"JWT\""), "Header should specify JWT type"); + assertTrue(headerJson.contains("\"x5t#S256\":\"certificateHash256\""), "Header should contain x5t#S256"); + assertTrue(headerJson.contains("\"x5c\":[\"cert1\",\"cert2\"]"), "Header should contain x5c"); + + // Decode and verify payload + String payloadJson = new String(Base64.getUrlDecoder().decode(jwtParts[1])); + assertTrue(payloadJson.contains("\"aud\":\"" + audience + "\""), "Payload should contain correct audience"); + assertTrue(payloadJson.contains("\"iss\":\"" + clientId + "\""), "Payload should contain correct issuer"); + assertTrue(payloadJson.contains("\"sub\":\"" + clientId + "\""), "Payload should contain correct subject"); + assertTrue(payloadJson.contains("\"nbf\":"), "Payload should contain nbf claim"); + assertTrue(payloadJson.contains("\"exp\":"), "Payload should contain exp claim"); + assertTrue(payloadJson.contains("\"jti\":"), "Payload should contain jti claim"); + + // Verify certificate parameters were accessed + verify(clientCertificateMock).privateKey(); + verify(clientCertificateMock).publicCertificateHash256(); + verify(clientCertificateMock).getEncodedPublicKeyCertificateChain(); + + // Sha1 assertion, used in certain legacy flows + clientAssertion = JwtHelper.buildJwt(clientId, clientCertificateMock, audience, true, true); + + jwt = clientAssertion.assertion(); + jwtParts = jwt.split("\\."); + + // Verify header uses SHA1 hash/x5t header and not SHA256/x5t#S256 + headerJson = new String(Base64.getUrlDecoder().decode(jwtParts[0])); + assertTrue(headerJson.contains("\"x5t\":\"certificateHash\""), "Header should contain x5t (SHA1)"); + assertFalse(headerJson.contains("\"x5t#S256\""), "Header should not contain x5t#S256"); + + // Verify the correct certificate hash method was called + verify(clientCertificateMock).publicCertificateHash(); + } +} From 8d1f418a51c13bd8d271281dbf44734e5dcc3380 Mon Sep 17 00:00:00 2001 From: avdunn Date: Fri, 25 Apr 2025 15:45:30 -0700 Subject: [PATCH 2/2] Address PR feedback --- .../microsoft/aad/msal4j/ClientAssertion.java | 2 +- .../com/microsoft/aad/msal4j/IBroker.java | 13 +----- .../com/microsoft/aad/msal4j/JsonHelper.java | 18 +++++++-- .../com/microsoft/aad/msal4j/JwtHelper.java | 5 +-- .../aad/msal4j/TokenRequestExecutor.java | 15 +------ .../aad/msal4j/HelperAndUtilityTests.java | 40 +++++++++++++++++++ 6 files changed, 60 insertions(+), 33 deletions(-) diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java index 7c4e456b..f5fe64e9 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/ClientAssertion.java @@ -12,7 +12,7 @@ @EqualsAndHashCode final class ClientAssertion implements IClientAssertion { - static final String ASSERTION_TYPE = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; + static final String ASSERTION_TYPE_JWT_BEARER = "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"; private final String assertion; ClientAssertion(final String assertion) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java index 9f50fc7a..7175d8d6 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/IBroker.java @@ -4,8 +4,6 @@ package com.microsoft.aad.msal4j; import java.net.URL; -import java.nio.charset.StandardCharsets; -import java.util.Base64; import java.util.concurrent.CompletableFuture; /** @@ -67,17 +65,10 @@ default IAuthenticationResult parseBrokerAuthResult(String authority, String idT if (idToken != null) { builder.idToken(idToken); if (accountId != null) { - String idTokenJson; - - try { - idTokenJson = new String(Base64.getDecoder().decode(idToken.split("\\.")[1]), StandardCharsets.UTF_8); - } catch (ArrayIndexOutOfBoundsException e) { - throw new MsalServiceException("Error parsing ID token, missing payload section. Ensure that the ID token is following the JWT format.", - AuthenticationErrorCode.INVALID_JWT); - } + IdToken idTokenObj = JsonHelper.createIdTokenFromEncodedTokenString(idToken); builder.accountCacheEntity(AccountCacheEntity.create(clientInfo, - Authority.createAuthority(new URL(authority)), JsonHelper.convertJsonToObject(idTokenJson, IdToken.class), null)); + Authority.createAuthority(new URL(authority)), idTokenObj, null)); } } if (accessToken != null) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java index d0038014..aa6a74e1 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java @@ -15,10 +15,8 @@ import com.fasterxml.jackson.databind.JsonNode; import java.io.IOException; -import java.util.ArrayList; -import java.util.Iterator; -import java.util.Map; -import java.util.Set; +import java.nio.charset.StandardCharsets; +import java.util.*; class JsonHelper { static ObjectMapper mapper; @@ -41,6 +39,18 @@ static T convertJsonToObject(final String json, final Class tClass) { } } + static IdToken createIdTokenFromEncodedTokenString(String token) { + String idTokenJson; + try { + idTokenJson = new String(Base64.getUrlDecoder().decode(token.split("\\.")[1]), StandardCharsets.UTF_8); + } catch (ArrayIndexOutOfBoundsException e) { + throw new MsalClientException("Error parsing ID token, missing payload section.", + AuthenticationErrorCode.INVALID_JWT); + } + + return JsonHelper.convertJsonToObject(idTokenJson, IdToken.class); + } + //This method is used to convert a JSON string to an object which implements the JsonSerializable interface from com.azure.json static > T convertJsonStringToJsonSerializableObject(String jsonResponse, ReadValueCallback readFunction) { try (JsonReader jsonReader = JsonProviders.createReader(jsonResponse)) { diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java index bca0cb82..9d1a4fca 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JwtHelper.java @@ -30,10 +30,7 @@ static ClientAssertion buildJwt(String clientId, final ClientCertificate credent header.put("typ", "JWT"); if (sendX5c) { - List certs = new ArrayList<>(); - for (String cert : credential.getEncodedPublicKeyCertificateChain()) { - certs.add(cert); - } + List certs = new ArrayList<>(credential.getEncodedPublicKeyCertificateChain()); header.put("x5c", certs); } diff --git a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java index e08744fc..75342c38 100644 --- a/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java +++ b/msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/TokenRequestExecutor.java @@ -3,14 +3,11 @@ package com.microsoft.aad.msal4j; -import com.nimbusds.oauth2.sdk.ParseException; -import com.nimbusds.oauth2.sdk.SerializeException; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; import java.net.MalformedURLException; -import java.nio.charset.StandardCharsets; import java.util.*; class TokenRequestExecutor { @@ -110,7 +107,7 @@ private void addQueryParameters(OAuthHttpRequest oauthHttpRequest) { private void addJWTBearerAssertionParams(Map queryParameters, String assertion) { queryParameters.put("client_assertion", assertion); - queryParameters.put("client_assertion_type", "urn:ietf:params:oauth:client-assertion-type:jwt-bearer"); + queryParameters.put("client_assertion_type", ClientAssertion.ASSERTION_TYPE_JWT_BEARER); } private AuthenticationResult createAuthenticationResultFromOauthHttpResponse(HttpResponse oauthHttpResponse) { @@ -121,15 +118,7 @@ private AuthenticationResult createAuthenticationResultFromOauthHttpResponse(Htt AccountCacheEntity accountCacheEntity = null; if (!StringHelper.isNullOrBlank(response.idToken())) { - String idTokenJson; - try { - idTokenJson = new String(Base64.getUrlDecoder().decode(response.idToken().split("\\.")[1]), StandardCharsets.UTF_8); - } catch (ArrayIndexOutOfBoundsException e) { - throw new MsalServiceException("Error parsing ID token, missing payload section. Ensure that the ID token is following the JWT format.", - AuthenticationErrorCode.INVALID_JWT); - } - - IdToken idToken = JsonHelper.convertJsonToObject(idTokenJson, IdToken.class); + IdToken idToken = JsonHelper.createIdTokenFromEncodedTokenString(response.idToken()); AuthorityType type = msalRequest.application().authenticationAuthority.authorityType; if (!StringHelper.isBlank(response.getClientInfo())) { diff --git a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java index 32939513..08ca2e39 100644 --- a/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java +++ b/msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/HelperAndUtilityTests.java @@ -5,6 +5,7 @@ import org.junit.jupiter.api.Test; +import java.nio.charset.StandardCharsets; import java.security.NoSuchAlgorithmException; import java.security.cert.CertificateEncodingException; import java.util.*; @@ -147,4 +148,43 @@ void JwtHelper_buildJwt_ValidSha1AndSha256Assertions() throws MsalClientExceptio // Verify the correct certificate hash method was called verify(clientCertificateMock).publicCertificateHash(); } + + @Test + void JsonHelper_createIdTokenFromEncodedTokenString_Base64URLCharacters() { + HashMap tokenParameters = new HashMap<>(); + tokenParameters.put("preferred_username", "~nameWith~specialChars"); + String encodedIDToken = TestHelper.createIdToken(tokenParameters); + + try { + //TestHelper.createIdToken() should use Base64URL encoding, so first we prove that the encoded token cannot be decoded with Base64 decoder + Base64.getDecoder().decode(encodedIDToken); + + fail("IllegalArgumentException was expected but not thrown."); + } catch (IllegalArgumentException e) { + //Encoded token should have some "-" characters in it + assertTrue(e.getMessage().contains("Illegal base64 character 2e")); + } + + // Act + IdToken idToken = JsonHelper.createIdTokenFromEncodedTokenString(encodedIDToken); + + // Assert + assertNotNull(idToken); + assertEquals("~nameWith~specialChars", idToken.preferredUsername); + } + + @Test + void JsonHelper_createIdTokenFromEncodedTokenString_InvalidJsonInToken() { + // Arrange + String invalidPayload = "{not-valid-json}"; + String encodedPayload = Base64.getUrlEncoder().withoutPadding() + .encodeToString(invalidPayload.getBytes(StandardCharsets.UTF_8)); + String invalidToken = "header." + encodedPayload + ".signature"; + + // Act & Assert + MsalJsonParsingException exception = assertThrows(MsalJsonParsingException.class, + () -> JsonHelper.createIdTokenFromEncodedTokenString(invalidToken)); + + assertEquals(AuthenticationErrorCode.INVALID_JSON, exception.errorCode()); + } }