Cwikius-Spring
/
spring-security
mirror of https://github.com/spring-projects/spring-security.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

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
This commit is contained in:
Ovidiu Popa 2020-11-27 09:10:54 +02:00 committed by Joe Grandja
parent 17276ad787
commit 7d31837af3
4 changed files with 56 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

6
oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java
View File

@ -56,12 +56,6 @@ final class ObjectToListStringConverter implements ConditionalGenericConverter {
if (source == null) {
return null;
}
if (source instanceof List) {
List<?> sourceList = (List<?>) source;
if (!sourceList.isEmpty() && sourceList.get(0) instanceof String) {
return source;
}
}
if (source instanceof Collection) {
Collection<String> results = new ArrayList<>();
for (Object object : ((Collection<?>) source)) {

3
oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java
View File

@ -52,9 +52,6 @@ final class ObjectToMapStringObjectConverter implements ConditionalGenericConver
return null;
}
Map<?, ?> sourceMap = (Map<?, ?>) source;
if (!sourceMap.isEmpty() && sourceMap.keySet().iterator().next() instanceof String) {
return source;
}
Map<String, Object> result = new HashMap<>();
sourceMap.forEach((k, v) -> result.put(k.toString(), v));
return result;

45
oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimConversionServiceTests.java
View File

@ -15,9 +15,10 @@
*/
package org.springframework.security.oauth2.core.converter;
import net.minidev.json.JSONArray;
import net.minidev.json.JSONObject;
import org.assertj.core.util.Lists;
import org.junit.Test;
import org.springframework.core.convert.ConversionService;
import java.net.URL;
import java.time.Instant;
@ -29,6 +30,8 @@ import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.springframework.core.convert.ConversionService;
import static org.assertj.core.api.Assertions.assertThat;
/**
@ -141,9 +144,9 @@ public class ClaimConversionServiceTests {
}
@Test
public void convertCollectionStringWhenListStringThenReturnSame() {
public void convertCollectionStringWhenListStringThenReturnNotSameButEqual() {
List<String> list = Lists.list("1", "2", "3", "4");
assertThat(this.conversionService.convert(list, Collection.class)).isSameAs(list);
assertThat(this.conversionService.convert(list, Collection.class)).isNotSameAs(list).isEqualTo(list);
}
@Test
@ -152,6 +155,17 @@ public class ClaimConversionServiceTests {
.isEqualTo(Lists.list("1", "2", "3", "4"));
}
@Test
public void convertListStringWhenJsonArrayThenConverts() {
JSONArray jsonArray = new JSONArray();
jsonArray.add("1");
jsonArray.add("2");
jsonArray.add("3");
jsonArray.add(null);
assertThat(this.conversionService.convert(jsonArray, List.class)).isNotInstanceOf(JSONArray.class)
.isEqualTo(Lists.list("1", "2", "3"));
}
@Test
public void convertCollectionStringWhenNotConvertibleThenReturnSingletonList() {
String string = "not-convertible-collection";
@ -165,9 +179,9 @@ public class ClaimConversionServiceTests {
}
@Test
public void convertListStringWhenListStringThenReturnSame() {
public void convertListStringWhenListStringThenReturnNotSameButEqual() {
List<String> list = Lists.list("1", "2", "3", "4");
assertThat(this.conversionService.convert(list, List.class)).isSameAs(list);
assertThat(this.conversionService.convert(list, List.class)).isNotSameAs(list).isEqualTo(list);
}
@Test
@ -189,7 +203,7 @@ public class ClaimConversionServiceTests {
}
@Test
public void convertMapStringObjectWhenMapStringObjectThenReturnSame() {
public void convertMapStringObjectWhenMapStringObjectThenReturnNotSameButEqual() {
Map<String, Object> mapStringObject = new HashMap<String, Object>() {
{
put("key1", "value1");
@ -197,7 +211,8 @@ public class ClaimConversionServiceTests {
put("key3", "value3");
}
};
assertThat(this.conversionService.convert(mapStringObject, Map.class)).isSameAs(mapStringObject);
assertThat(this.conversionService.convert(mapStringObject, Map.class)).isNotSameAs(mapStringObject)
.isEqualTo(mapStringObject);
}
@Test
@ -219,6 +234,22 @@ public class ClaimConversionServiceTests {
assertThat(this.conversionService.convert(mapIntegerObject, Map.class)).isEqualTo(mapStringObject);
}
@Test
public void convertMapStringObjectWhenJsonObjectThenConverts() {
JSONObject jsonObject = new JSONObject();
jsonObject.put("1", "value1");
jsonObject.put("2", "value2");
Map<String, Object> mapStringObject = new HashMap<String, Object>() {
{
put("1", "value1");
put("2", "value2");
}
};
assertThat(this.conversionService.convert(jsonObject, Map.class)).isNotInstanceOf(JSONObject.class)
.isEqualTo(mapStringObject);
}
@Test
public void convertMapStringObjectWhenNotConvertibleThenReturnNull() {
List<String> notConvertibleList = Lists.list("1", "2", "3", "4");

23
oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverterTests.java
View File

@ -15,7 +15,10 @@
*/
package org.springframework.security.oauth2.core.converter;
import net.minidev.json.JSONArray;
import net.minidev.json.JSONObject;
import org.assertj.core.util.Lists;
import org.assertj.core.util.Maps;
import org.junit.Before;
import org.junit.Test;
import org.springframework.core.convert.TypeDescriptor;
@ -45,6 +48,8 @@ public class ClaimTypeConverterTests {
private static final String COLLECTION_STRING_CLAIM = "collection-string-claim";
private static final String LIST_STRING_CLAIM = "list-string-claim";
private static final String MAP_STRING_OBJECT_CLAIM = "map-string-object-claim";
private static final String JSON_ARRAY_CLAIM = "json-array-claim";
private static final String JSON_OBJECT_CLAIM = "json-object-claim";
private ClaimTypeConverter claimTypeConverter;
@Before
@ -107,7 +112,12 @@ public class ClaimTypeConverterTests {
mapIntegerObject.put(1, "value1");
Map<String, Object> mapStringObject = new HashMap<>();
mapStringObject.put("1", "value1");
JSONArray jsonArray = new JSONArray();
jsonArray.add("1");
List<String> jsonArrayListString = Lists.list("1");
JSONObject jsonObject = new JSONObject();
jsonObject.put("1", "value1");
Map<String, Object> jsonObjectMap = Maps.newHashMap("1", "value1");
Map<String, Object> claims = new HashMap<>();
claims.put(STRING_CLAIM, Boolean.TRUE);
claims.put(BOOLEAN_CLAIM, "true");
@ -116,7 +126,8 @@ public class ClaimTypeConverterTests {
claims.put(COLLECTION_STRING_CLAIM, listNumber);
claims.put(LIST_STRING_CLAIM, listNumber);
claims.put(MAP_STRING_OBJECT_CLAIM, mapIntegerObject);
claims.put(JSON_ARRAY_CLAIM, jsonArray);
claims.put(JSON_OBJECT_CLAIM, jsonObject);
claims = this.claimTypeConverter.convert(claims);
assertThat(claims.get(STRING_CLAIM)).isEqualTo("true");
@ -126,6 +137,8 @@ public class ClaimTypeConverterTests {
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isEqualTo(listString);
assertThat(claims.get(LIST_STRING_CLAIM)).isEqualTo(listString);
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isEqualTo(mapStringObject);
assertThat(claims.get(JSON_ARRAY_CLAIM)).isEqualTo(jsonArrayListString);
assertThat(claims.get(JSON_OBJECT_CLAIM)).isEqualTo(jsonObjectMap);
}
@Test
@ -153,9 +166,9 @@ public class ClaimTypeConverterTests {
assertThat(claims.get(BOOLEAN_CLAIM)).isSameAs(bool);
assertThat(claims.get(INSTANT_CLAIM)).isSameAs(instant);
assertThat(claims.get(URL_CLAIM)).isSameAs(url);
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isSameAs(listString);
assertThat(claims.get(LIST_STRING_CLAIM)).isSameAs(listString);
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isSameAs(mapStringObject);
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isNotSameAs(listString).isEqualTo(listString);
assertThat(claims.get(LIST_STRING_CLAIM)).isNotSameAs(listString).isEqualTo(listString);
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isNotSameAs(mapStringObject).isEqualTo(mapStringObject);
}
@Test