Skip to content

Commit ccb50b1

Browse files
committed
Merge remote-tracking branch 'origin/avdunn/nimbus-removal' into avdunn/json-removal
# Conflicts: # msal4j-sdk/src/main/java/com/microsoft/aad/msal4j/JsonHelper.java
2 parents aa30533 + 169c2b0 commit ccb50b1

File tree

4 files changed

+241
-16
lines changed

4 files changed

+241
-16
lines changed

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -215,7 +215,7 @@ public AuthenticationResult build() {
215215
}
216216

217217
public String toString() {
218-
return "AuthenticationResult.AuthenticationResultBuilder(accessToken=" + this.accessToken + ", expiresOn=" + this.expiresOn + ", extExpiresOn=" + this.extExpiresOn + ", refreshToken=" + this.refreshToken + ", refreshOn=" + this.refreshOn + ", familyId=" + this.familyId + ", idToken=" + this.idToken + ", accountCacheEntity=" + this.accountCacheEntity + ", environment=" + this.environment + ", scopes=" + this.scopes + ", metadata" + this.metadata + ", isPopAuthorization=" + this.isPopAuthorization + ")";
218+
return "AuthenticationResult.AuthenticationResultBuilder(accessToken=" + this.accessToken + ", expiresOn=" + this.expiresOn + ", extExpiresOn=" + this.extExpiresOn + ", refreshToken=" + this.refreshToken + ", refreshOn=" + this.refreshOn + ", familyId=" + this.familyId + ", idToken=" + this.idToken + ", accountCacheEntity=" + this.accountCacheEntity + ", environment=" + this.environment + ", scopes=" + this.scopes + ", metadata=" + this.metadata + ", isPopAuthorization=" + this.isPopAuthorization + ")";
219219
}
220220
}
221221

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

Lines changed: 46 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,15 @@ class JsonHelper {
2121
private JsonHelper() {
2222
}
2323

24+
static <T> T convertJsonToObject(final String json, final Class<T> tClass) {
25+
try {
26+
return mapper.readValue(json, tClass);
27+
} catch (Exception e) {
28+
LOG.error(String.format("Error converting JSON string into %s: %s", tClass, e.getMessage()));
29+
throw new MsalJsonParsingException(e.getMessage(), AuthenticationErrorCode.INVALID_JSON);
30+
}
31+
}
32+
2433
static IdToken createIdTokenFromEncodedTokenString(String token) {
2534
return convertJsonStringToJsonSerializableObject(getTokenPayloadClaims(token), IdToken::fromJson);
2635
}
@@ -29,11 +38,13 @@ static String getTokenPayloadClaims(String token) {
2938
try {
3039
return new String(Base64.getUrlDecoder().decode(token.split("\\.")[1]), StandardCharsets.UTF_8);
3140
} catch (ArrayIndexOutOfBoundsException e) {
41+
LOG.error("Error parsing ID token, missing payload section.");
3242
throw new MsalClientException("Error parsing ID token, missing payload section.",
3343
AuthenticationErrorCode.INVALID_JWT);
3444
}
3545
}
3646

47+
//Converts a generic JSON string to a Map<String, Object> with relevant types
3748
static Map<String, Object> parseJsonToMap(String jsonString) {
3849
if (StringHelper.isBlank(jsonString)) {
3950
return new HashMap<>();
@@ -43,31 +54,47 @@ static Map<String, Object> parseJsonToMap(String jsonString) {
4354
jsonReader.nextToken();
4455
return parseJsonObject(jsonReader);
4556
} catch (IOException e) {
57+
LOG.error("JSON parsing error when attempting to convert JSON into a Map.");
4658
throw new MsalJsonParsingException(e.getMessage(), AuthenticationErrorCode.INVALID_JSON);
4759
}
4860
}
4961

50-
private static List<Object> parseJsonArray(JsonReader jsonReader) throws IOException {
51-
List<Object> array = new ArrayList<>();
52-
53-
while (jsonReader.nextToken() != JsonToken.END_ARRAY) {
54-
array.add(parseValue(jsonReader));
55-
}
56-
57-
return array;
58-
}
59-
6062
private static Map<String, Object> parseJsonObject(JsonReader jsonReader) throws IOException {
6163
Map<String, Object> object = new HashMap<>();
6264

6365
while (jsonReader.nextToken() != JsonToken.END_OBJECT) {
6466
String fieldName = jsonReader.getFieldName();
65-
object.put(fieldName, parseValue(jsonReader));
67+
Object value = parseValue(jsonReader);
68+
object.put(fieldName, handleSpecialFields(fieldName, value));
6669
}
6770

6871
return object;
6972
}
7073

74+
//Due to the old usage of com.nimbusds for JWT parsing, customers may be relying on certain fields being treated as specific types.
75+
// This method handles those special cases to help ensure backwards compatibility.
76+
private static Object handleSpecialFields(String fieldName, Object value) {
77+
//nimbus always treated the "aud" field as an ArrayList, even when it was a single string
78+
if ("aud".equals(fieldName) && value instanceof String) {
79+
ArrayList<String> list = new ArrayList<>();
80+
list.add((String) value);
81+
return list;
82+
}
83+
84+
//nimbus converted certain unix timestamps to Date objects
85+
if (isTimestampField(fieldName) && value instanceof Number) {
86+
// Convert seconds to milliseconds for Date constructor
87+
return new Date(((Number) value).longValue() * 1000);
88+
}
89+
90+
return value;
91+
}
92+
93+
private static boolean isTimestampField(String fieldName) {
94+
return "exp".equals(fieldName) || "iat".equals(fieldName) ||
95+
"nbf".equals(fieldName);
96+
}
97+
7198
private static Object parseValue(JsonReader jsonReader) throws IOException {
7299
JsonToken token = jsonReader.currentToken();
73100

@@ -79,10 +106,14 @@ private static Object parseValue(JsonReader jsonReader) throws IOException {
79106
} catch (ArithmeticException e) {
80107
return jsonReader.getDouble();
81108
}
82-
case BOOLEAN: return jsonReader.getBoolean();
83-
case NULL: return null;
84-
case START_ARRAY: return parseJsonArray(jsonReader);
85-
case START_OBJECT: return parseJsonObject(jsonReader);
109+
case BOOLEAN:
110+
return jsonReader.getBoolean();
111+
case NULL:
112+
return null;
113+
case START_ARRAY:
114+
return jsonReader.readArray(JsonReader::readUntyped);
115+
case START_OBJECT:
116+
return parseJsonObject(jsonReader);
86117
default:
87118
jsonReader.skipChildren();
88119
return null;
Lines changed: 154 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,154 @@
1+
package com.microsoft.aad.msal4j;
2+
3+
import com.nimbusds.jwt.JWTParser;
4+
import org.junit.jupiter.api.Test;
5+
6+
import java.text.ParseException;
7+
import java.util.Base64;
8+
import java.util.List;
9+
import java.util.Map;
10+
11+
import static org.junit.jupiter.api.Assertions.*;
12+
13+
//These tests were added to ensure the new usages of com.azure.json are functionally the same as the old usages of com.nimbusds packages.
14+
//Once we are confident in the new behavior these should no longer be necessary.
15+
class JsonCompatibilityTests {
16+
17+
//New style, using helper methods in JsonHelper that use com.azure.json
18+
private final Map<String, Object> newStyleParsedClaims = JsonHelper.parseJsonToMap(JsonHelper.getTokenPayloadClaims(TestHelper.ENCODED_JWT));
19+
//Old style, using com.nimbusds methods
20+
private final Map<String, Object> oldStyleParsedClaims = JWTParser.parse(TestHelper.ENCODED_JWT).getJWTClaimsSet().getClaims();
21+
22+
JsonCompatibilityTests() throws ParseException {
23+
}
24+
25+
@Test
26+
void testBasicClaimsMatching() {
27+
// Test basic string claims
28+
assertEquals(oldStyleParsedClaims.get("aud"), newStyleParsedClaims.get("aud"));
29+
assertEquals(oldStyleParsedClaims.get("iss"), newStyleParsedClaims.get("iss"));
30+
assertEquals(oldStyleParsedClaims.get("name"), newStyleParsedClaims.get("name"));
31+
assertEquals(oldStyleParsedClaims.get("oid"), newStyleParsedClaims.get("oid"));
32+
assertEquals(oldStyleParsedClaims.get("preferred_username"), newStyleParsedClaims.get("preferred_username"));
33+
assertEquals(oldStyleParsedClaims.get("sub"), newStyleParsedClaims.get("sub"));
34+
assertEquals(oldStyleParsedClaims.get("tid"), newStyleParsedClaims.get("tid"));
35+
assertEquals(oldStyleParsedClaims.get("ver"), newStyleParsedClaims.get("ver"));
36+
}
37+
38+
@Test
39+
void testNullValues() {
40+
// Check null values are handled the same
41+
assertEquals(oldStyleParsedClaims.get("email"), newStyleParsedClaims.get("email"));
42+
}
43+
44+
@Test
45+
void testNumericValues() {
46+
// Test numeric claims
47+
assertEquals(oldStyleParsedClaims.get("exp"), newStyleParsedClaims.get("exp"));
48+
assertEquals(oldStyleParsedClaims.get("iat"), newStyleParsedClaims.get("iat"));
49+
assertEquals(oldStyleParsedClaims.get("nbf"), newStyleParsedClaims.get("nbf"));
50+
assertEquals(oldStyleParsedClaims.get("auth_time"), newStyleParsedClaims.get("auth_time"));
51+
}
52+
53+
@Test
54+
void testListValues() {
55+
// Test list claims
56+
List<?> oldGroups = (List<?>) oldStyleParsedClaims.get("groups");
57+
List<?> newGroups = (List<?>) newStyleParsedClaims.get("groups");
58+
assertEquals(oldGroups, newGroups);
59+
60+
List<?> oldAmr = (List<?>) oldStyleParsedClaims.get("amr");
61+
List<?> newAmr = (List<?>) newStyleParsedClaims.get("amr");
62+
assertEquals(oldAmr, newAmr);
63+
64+
List<?> oldRoles = (List<?>) oldStyleParsedClaims.get("roles");
65+
List<?> newRoles = (List<?>) newStyleParsedClaims.get("roles");
66+
assertEquals(oldRoles, newRoles);
67+
}
68+
69+
@Test
70+
void testNestedObjects() {
71+
// Test nested objects
72+
Map<?, ?> oldExtensionData = (Map<?, ?>) oldStyleParsedClaims.get("extension_data");
73+
Map<?, ?> newExtensionData = (Map<?, ?>) newStyleParsedClaims.get("extension_data");
74+
75+
assertEquals(oldExtensionData.get("department"), newExtensionData.get("department"));
76+
assertEquals(oldExtensionData.get("manager"), newExtensionData.get("manager"));
77+
assertEquals(oldExtensionData.get("employeeId"), newExtensionData.get("employeeId"));
78+
}
79+
80+
@Test
81+
void testCompleteTokenParsing() {
82+
assertEquals(oldStyleParsedClaims.size(), newStyleParsedClaims.size());
83+
84+
// Check that all keys and values match
85+
for (String key : oldStyleParsedClaims.keySet()) {
86+
assertTrue(newStyleParsedClaims.containsKey(key), "New claims should contain key: " + key);
87+
88+
Object oldValue = oldStyleParsedClaims.get(key);
89+
Object newValue = newStyleParsedClaims.get(key);
90+
91+
if (oldValue instanceof List) {
92+
assertListsEqual((List<?>) oldValue, (List<?>) newValue);
93+
} else if (oldValue instanceof Map) {
94+
assertMapsEqual((Map<?, ?>) oldValue, (Map<?, ?>) newValue);
95+
} else {
96+
assertEquals(oldValue, newValue, "Value mismatch for key: " + key);
97+
}
98+
}
99+
}
100+
101+
@Test
102+
void testInvalidJSONHandling() {
103+
// Test that both parsers throw exceptions for invalid JSON
104+
String invalidJson = "{ this is not valid json }";
105+
106+
assertThrows(Exception.class, () -> JsonHelper.parseJsonToMap(invalidJson));
107+
assertThrows(Exception.class, () -> JWTParser.parse("header." +
108+
Base64.getUrlEncoder().encodeToString(invalidJson.getBytes()) + ".signature"));
109+
}
110+
111+
/**
112+
* Utility method to compare lists deeply
113+
*/
114+
private void assertListsEqual(List<?> list1, List<?> list2) {
115+
assertEquals(list1.size(), list2.size());
116+
for (int i = 0; i < list1.size(); i++) {
117+
Object item1 = list1.get(i);
118+
Object item2 = list2.get(i);
119+
120+
if (item1 == null) {
121+
assertNull(item2);
122+
} else if (item1 instanceof List) {
123+
assertListsEqual((List<?>) item1, (List<?>) item2);
124+
} else if (item1 instanceof Map) {
125+
assertMapsEqual((Map<?, ?>) item1, (Map<?, ?>) item2);
126+
} else {
127+
assertEquals(item1, item2);
128+
}
129+
}
130+
}
131+
132+
/**
133+
* Utility method to compare maps deeply
134+
*/
135+
private void assertMapsEqual(Map<?, ?> map1, Map<?, ?> map2) {
136+
assertEquals(map1.size(), map2.size());
137+
for (Object key : map1.keySet()) {
138+
assertTrue(map2.containsKey(key));
139+
140+
Object value1 = map1.get(key);
141+
Object value2 = map2.get(key);
142+
143+
if (value1 == null) {
144+
assertNull(value2);
145+
} else if (value1 instanceof List) {
146+
assertListsEqual((List<?>) value1, (List<?>) value2);
147+
} else if (value1 instanceof Map) {
148+
assertMapsEqual((Map<?, ?>) value1, (Map<?, ?>) value2);
149+
} else {
150+
assertEquals(value1, value2);
151+
}
152+
}
153+
}
154+
}

msal4j-sdk/src/test/java/com/microsoft/aad/msal4j/TestHelper.java

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,38 @@ class TestHelper {
2626
//Signed JWT which should be enough to pass the parsing/validation in the library, useful if a unit test needs an
2727
// assertion but that is not the focus of the test
2828
static String signedAssertion = generateToken();
29+
30+
// Realistic token header
31+
static final String TOKEN_HEADER = "{\"alg\":\"PS256\",\"typ\":\"JWT\"}";
32+
33+
// Realistic JWT payload, containing strings, numbers, timestamps, arrays, nested JSON objects, and nulls
34+
static final String TOKEN_PAYLOAD = "{\n" +
35+
"\"aud\": \"e854a4a7-6c34-449c-b237-fc7a28093d84\",\n" +
36+
"\"iss\": \"https://login.microsoftonline.com/6c3d51dd-f0e5-4959-b4ea-a80c4e36fe5e/v2.0/\",\n" +
37+
"\"name\": \"John Doe\",\n" +
38+
"\"oid\": \"00000000-0000-0000-66f3-3332eca7ea81\",\n" +
39+
"\"preferred_username\": \"[email protected]\",\n" +
40+
"\"sub\": \"K4_SGGxKqW1SxUAmhg6C1F6VPiFzcx-Qd80ehIEdFus\",\n" +
41+
"\"tid\": \"6c3d51dd-f0e5-4959-b4ea-a80c4e36fe5e\",\n" +
42+
"\"ver\": \"2.0\",\n" +
43+
"\"exp\": 1735689600,\n" +
44+
"\"iat\": 1516239022,\n" +
45+
"\"nbf\": 1516239022,\n" +
46+
"\"email\": null,\n" +
47+
"\"groups\": [\"admin\", \"user\", null],\n" +
48+
"\"roles\": [],\n" +
49+
"\"amr\": [\"pwd\", \"mfa\"],\n" +
50+
"\"nonce\": \"12345\",\n" +
51+
"\"auth_time\": 1516239022,\n" +
52+
"\"given_name\": \"John\",\n" +
53+
"\"family_name\": \"Doe\",\n" +
54+
"\"at_hash\": \"jT3s9ygOQRtifgrpJgGz_w\",\n" +
55+
"\"extension_data\": {\"department\": \"Engineering\", \"manager\": null, \"employeeId\": 12345}\n" +
56+
"}";
57+
58+
// Base64URL encoded ID token, has a junk signature but is otherwise realistic and parsable
59+
static final String ENCODED_JWT = createEncodedJwt(TOKEN_HEADER, TOKEN_PAYLOAD);
60+
2961
private static final String successfulResponseFormat = "{\"access_token\":\"%s\",\"id_token\":\"%s\",\"refresh_token\":\"%s\"," +
3062
"\"client_id\":\"%s\",\"client_info\":\"%s\"," +
3163
"\"refresh_in\": %d,\"expires_on\": %d,\"expires_in\": %d," +
@@ -96,6 +128,14 @@ static String generateToken() {
96128
}
97129
}
98130

131+
private static String createEncodedJwt(String headerJson, String payloadJson) {
132+
String encodedHeader = Base64.getUrlEncoder().withoutPadding().encodeToString(headerJson.getBytes());
133+
String encodedPayload = Base64.getUrlEncoder().withoutPadding().encodeToString(payloadJson.getBytes());
134+
String signature = "signature"; // Simple signature for testing purposes
135+
136+
return encodedHeader + "." + encodedPayload + "." + signature;
137+
}
138+
99139
//Maps various values to the successfulResponseFormat string to create a valid token response
100140
static String getSuccessfulTokenResponse(HashMap<String, String> responseValues) {
101141
//Will default to expiring in one hour if expiry time values are not set

0 commit comments

Comments
 (0)