Skip to content

Commit 7d31837

Browse files
ovidiupopa07jgrandja
ovidiupopa07
authored and
jgrandja
committed
OidcIdToken cannot be serialized to JSON if token contains claim of type JSONArray or JSONObject
ObjectToListStringConverter and ObjectToMapStringObjectConverter were checking if the source object is of type List or Map and if the first element or key is a String. If we have a JSONArray containing Strings the above check will pass, meaning that a JSONArray will be returned which is not serializable (same applies to JSONObject) With this change, even if the check is passing a new List or Map will be returned. Closes gh-9210
1 parent 17276ad commit 7d31837

4 files changed

Lines changed: 56 additions & 21 deletions

File tree

‎oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java‎

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -56,12 +56,6 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
5656
if (source == null) {
5757
return null;
5858
}
59-
if (source instanceof List) {
60-
List<?> sourceList = (List<?>) source;
61-
if (!sourceList.isEmpty() && sourceList.get(0) instanceof String) {
62-
return source;
63-
}
64-
}
6559
if (source instanceof Collection) {
6660
Collection<String> results = new ArrayList<>();
6761
for (Object object : ((Collection<?>) source)) {

‎oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java‎

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -52,9 +52,6 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t
5252
return null;
5353
}
5454
Map<?, ?> sourceMap = (Map<?, ?>) source;
55-
if (!sourceMap.isEmpty() && sourceMap.keySet().iterator().next() instanceof String) {
56-
return source;
57-
}
5855
Map<String, Object> result = new HashMap<>();
5956
sourceMap.forEach((k, v) -> result.put(k.toString(), v));
6057
return result;

‎oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimConversionServiceTests.java‎

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,10 @@
1515
*/
1616
package org.springframework.security.oauth2.core.converter;
1717

18+
import net.minidev.json.JSONArray;
19+
import net.minidev.json.JSONObject;
1820
import org.assertj.core.util.Lists;
1921
import org.junit.Test;
20-
import org.springframework.core.convert.ConversionService;
2122

2223
import java.net.URL;
2324
import java.time.Instant;
@@ -29,6 +30,8 @@
2930
import java.util.List;
3031
import java.util.Map;
3132

33+
import org.springframework.core.convert.ConversionService;
34+
3235
import static org.assertj.core.api.Assertions.assertThat;
3336

3437
/**
@@ -141,9 +144,9 @@ public void convertCollectionStringWhenNullThenReturnNull() {
141144
}
142145

143146
@Test
144-
public void convertCollectionStringWhenListStringThenReturnSame() {
147+
public void convertCollectionStringWhenListStringThenReturnNotSameButEqual() {
145148
List<String> list = Lists.list("1", "2", "3", "4");
146-
assertThat(this.conversionService.convert(list, Collection.class)).isSameAs(list);
149+
assertThat(this.conversionService.convert(list, Collection.class)).isNotSameAs(list).isEqualTo(list);
147150
}
148151

149152
@Test
@@ -152,6 +155,17 @@ public void convertCollectionStringWhenListNumberThenConverts() {
152155
.isEqualTo(Lists.list("1", "2", "3", "4"));
153156
}
154157

158+
@Test
159+
public void convertListStringWhenJsonArrayThenConverts() {
160+
JSONArray jsonArray = new JSONArray();
161+
jsonArray.add("1");
162+
jsonArray.add("2");
163+
jsonArray.add("3");
164+
jsonArray.add(null);
165+
assertThat(this.conversionService.convert(jsonArray, List.class)).isNotInstanceOf(JSONArray.class)
166+
.isEqualTo(Lists.list("1", "2", "3"));
167+
}
168+
155169
@Test
156170
public void convertCollectionStringWhenNotConvertibleThenReturnSingletonList() {
157171
String string = "not-convertible-collection";
@@ -165,9 +179,9 @@ public void convertListStringWhenNullThenReturnNull() {
165179
}
166180

167181
@Test
168-
public void convertListStringWhenListStringThenReturnSame() {
182+
public void convertListStringWhenListStringThenReturnNotSameButEqual() {
169183
List<String> list = Lists.list("1", "2", "3", "4");
170-
assertThat(this.conversionService.convert(list, List.class)).isSameAs(list);
184+
assertThat(this.conversionService.convert(list, List.class)).isNotSameAs(list).isEqualTo(list);
171185
}
172186

173187
@Test
@@ -189,15 +203,16 @@ public void convertMapStringObjectWhenNullThenReturnNull() {
189203
}
190204

191205
@Test
192-
public void convertMapStringObjectWhenMapStringObjectThenReturnSame() {
206+
public void convertMapStringObjectWhenMapStringObjectThenReturnNotSameButEqual() {
193207
Map<String, Object> mapStringObject = new HashMap<String, Object>() {
194208
{
195209
put("key1", "value1");
196210
put("key2", "value2");
197211
put("key3", "value3");
198212
}
199213
};
200-
assertThat(this.conversionService.convert(mapStringObject, Map.class)).isSameAs(mapStringObject);
214+
assertThat(this.conversionService.convert(mapStringObject, Map.class)).isNotSameAs(mapStringObject)
215+
.isEqualTo(mapStringObject);
201216
}
202217

203218
@Test
@@ -219,6 +234,22 @@ public void convertMapStringObjectWhenMapIntegerObjectThenConverts() {
219234
assertThat(this.conversionService.convert(mapIntegerObject, Map.class)).isEqualTo(mapStringObject);
220235
}
221236

237+
@Test
238+
public void convertMapStringObjectWhenJsonObjectThenConverts() {
239+
JSONObject jsonObject = new JSONObject();
240+
jsonObject.put("1", "value1");
241+
jsonObject.put("2", "value2");
242+
243+
Map<String, Object> mapStringObject = new HashMap<String, Object>() {
244+
{
245+
put("1", "value1");
246+
put("2", "value2");
247+
}
248+
};
249+
assertThat(this.conversionService.convert(jsonObject, Map.class)).isNotInstanceOf(JSONObject.class)
250+
.isEqualTo(mapStringObject);
251+
}
252+
222253
@Test
223254
public void convertMapStringObjectWhenNotConvertibleThenReturnNull() {
224255
List<String> notConvertibleList = Lists.list("1", "2", "3", "4");

‎oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverterTests.java‎

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,10 @@
1515
*/
1616
package org.springframework.security.oauth2.core.converter;
1717

18+
import net.minidev.json.JSONArray;
19+
import net.minidev.json.JSONObject;
1820
import org.assertj.core.util.Lists;
21+
import org.assertj.core.util.Maps;
1922
import org.junit.Before;
2023
import org.junit.Test;
2124
import org.springframework.core.convert.TypeDescriptor;
@@ -45,6 +48,8 @@ public class ClaimTypeConverterTests {
4548
private static final String COLLECTION_STRING_CLAIM = "collection-string-claim";
4649
private static final String LIST_STRING_CLAIM = "list-string-claim";
4750
private static final String MAP_STRING_OBJECT_CLAIM = "map-string-object-claim";
51+
private static final String JSON_ARRAY_CLAIM = "json-array-claim";
52+
private static final String JSON_OBJECT_CLAIM = "json-object-claim";
4853
private ClaimTypeConverter claimTypeConverter;
4954

5055
@Before
@@ -107,7 +112,12 @@ public void convertWhenAllClaimsRequireConversionThenConvertAll() throws Excepti
107112
mapIntegerObject.put(1, "value1");
108113
Map<String, Object> mapStringObject = new HashMap<>();
109114
mapStringObject.put("1", "value1");
110-
115+
JSONArray jsonArray = new JSONArray();
116+
jsonArray.add("1");
117+
List<String> jsonArrayListString = Lists.list("1");
118+
JSONObject jsonObject = new JSONObject();
119+
jsonObject.put("1", "value1");
120+
Map<String, Object> jsonObjectMap = Maps.newHashMap("1", "value1");
111121
Map<String, Object> claims = new HashMap<>();
112122
claims.put(STRING_CLAIM, Boolean.TRUE);
113123
claims.put(BOOLEAN_CLAIM, "true");
@@ -116,7 +126,8 @@ public void convertWhenAllClaimsRequireConversionThenConvertAll() throws Excepti
116126
claims.put(COLLECTION_STRING_CLAIM, listNumber);
117127
claims.put(LIST_STRING_CLAIM, listNumber);
118128
claims.put(MAP_STRING_OBJECT_CLAIM, mapIntegerObject);
119-
129+
claims.put(JSON_ARRAY_CLAIM, jsonArray);
130+
claims.put(JSON_OBJECT_CLAIM, jsonObject);
120131
claims = this.claimTypeConverter.convert(claims);
121132

122133
assertThat(claims.get(STRING_CLAIM)).isEqualTo("true");
@@ -126,6 +137,8 @@ public void convertWhenAllClaimsRequireConversionThenConvertAll() throws Excepti
126137
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isEqualTo(listString);
127138
assertThat(claims.get(LIST_STRING_CLAIM)).isEqualTo(listString);
128139
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isEqualTo(mapStringObject);
140+
assertThat(claims.get(JSON_ARRAY_CLAIM)).isEqualTo(jsonArrayListString);
141+
assertThat(claims.get(JSON_OBJECT_CLAIM)).isEqualTo(jsonObjectMap);
129142
}
130143

131144
@Test
@@ -153,9 +166,9 @@ public void convertWhenNoClaimsRequireConversionThenConvertNone() throws Excepti
153166
assertThat(claims.get(BOOLEAN_CLAIM)).isSameAs(bool);
154167
assertThat(claims.get(INSTANT_CLAIM)).isSameAs(instant);
155168
assertThat(claims.get(URL_CLAIM)).isSameAs(url);
156-
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isSameAs(listString);
157-
assertThat(claims.get(LIST_STRING_CLAIM)).isSameAs(listString);
158-
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isSameAs(mapStringObject);
169+
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isNotSameAs(listString).isEqualTo(listString);
170+
assertThat(claims.get(LIST_STRING_CLAIM)).isNotSameAs(listString).isEqualTo(listString);
171+
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isNotSameAs(mapStringObject).isEqualTo(mapStringObject);
159172
}
160173

161174
@Test

0 commit comments

Comments
 (0)