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 d3ef340b26
commit d5d0be36f4
4 changed files with 56 additions and 18 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) { if (source == null) {
return 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) { if (source instanceof Collection) {
Collection<String> results = new ArrayList<>(); Collection<String> results = new ArrayList<>();
for (Object object : ((Collection<?>) source)) { for (Object object : ((Collection<?>) source)) {

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

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

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

@ -26,6 +26,8 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import com.nimbusds.jose.shaded.json.JSONArray;
import com.nimbusds.jose.shaded.json.JSONObject;
import org.assertj.core.util.Lists; import org.assertj.core.util.Lists;
import org.junit.Test; import org.junit.Test;
@ -145,9 +147,9 @@ public class ClaimConversionServiceTests {
} }
@Test @Test
public void convertCollectionStringWhenListStringThenReturnSame() { public void convertCollectionStringWhenListStringThenReturnNotSameButEqual() {
List<String> list = Lists.list("1", "2", "3", "4"); 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 @Test
@ -156,6 +158,17 @@ public class ClaimConversionServiceTests {
.isEqualTo(Lists.list("1", "2", "3", "4")); .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 @Test
public void convertCollectionStringWhenNotConvertibleThenReturnSingletonList() { public void convertCollectionStringWhenNotConvertibleThenReturnSingletonList() {
String string = "not-convertible-collection"; String string = "not-convertible-collection";
@ -169,9 +182,9 @@ public class ClaimConversionServiceTests {
} }
@Test @Test
public void convertListStringWhenListStringThenReturnSame() { public void convertListStringWhenListStringThenReturnNotSameButEqual() {
List<String> list = Lists.list("1", "2", "3", "4"); 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 @Test
@ -192,7 +205,7 @@ public class ClaimConversionServiceTests {
} }
@Test @Test
public void convertMapStringObjectWhenMapStringObjectThenReturnSame() { public void convertMapStringObjectWhenMapStringObjectThenReturnNotSameButEqual() {
Map<String, Object> mapStringObject = new HashMap<String, Object>() { Map<String, Object> mapStringObject = new HashMap<String, Object>() {
{ {
put("key1", "value1"); put("key1", "value1");
@ -200,7 +213,8 @@ public class ClaimConversionServiceTests {
put("key3", "value3"); put("key3", "value3");
} }
}; };
assertThat(this.conversionService.convert(mapStringObject, Map.class)).isSameAs(mapStringObject); assertThat(this.conversionService.convert(mapStringObject, Map.class)).isNotSameAs(mapStringObject)
.isEqualTo(mapStringObject);
} }
@Test @Test
@ -222,6 +236,22 @@ public class ClaimConversionServiceTests {
assertThat(this.conversionService.convert(mapIntegerObject, Map.class)).isEqualTo(mapStringObject); 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 @Test
public void convertMapStringObjectWhenNotConvertibleThenReturnNull() { public void convertMapStringObjectWhenNotConvertibleThenReturnNull() {
List<String> notConvertibleList = Lists.list("1", "2", "3", "4"); 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

@ -23,7 +23,10 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import com.nimbusds.jose.shaded.json.JSONArray;
import com.nimbusds.jose.shaded.json.JSONObject;
import org.assertj.core.util.Lists; import org.assertj.core.util.Lists;
import org.assertj.core.util.Maps;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
@ -55,6 +58,10 @@ public class ClaimTypeConverterTests {
private static final String MAP_STRING_OBJECT_CLAIM = "map-string-object-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; private ClaimTypeConverter claimTypeConverter;
@Before @Before
@ -115,6 +122,12 @@ public class ClaimTypeConverterTests {
mapIntegerObject.put(1, "value1"); mapIntegerObject.put(1, "value1");
Map<String, Object> mapStringObject = new HashMap<>(); Map<String, Object> mapStringObject = new HashMap<>();
mapStringObject.put("1", "value1"); 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<>(); Map<String, Object> claims = new HashMap<>();
claims.put(STRING_CLAIM, Boolean.TRUE); claims.put(STRING_CLAIM, Boolean.TRUE);
claims.put(BOOLEAN_CLAIM, "true"); claims.put(BOOLEAN_CLAIM, "true");
@ -123,6 +136,8 @@ public class ClaimTypeConverterTests {
claims.put(COLLECTION_STRING_CLAIM, listNumber); claims.put(COLLECTION_STRING_CLAIM, listNumber);
claims.put(LIST_STRING_CLAIM, listNumber); claims.put(LIST_STRING_CLAIM, listNumber);
claims.put(MAP_STRING_OBJECT_CLAIM, mapIntegerObject); claims.put(MAP_STRING_OBJECT_CLAIM, mapIntegerObject);
claims.put(JSON_ARRAY_CLAIM, jsonArray);
claims.put(JSON_OBJECT_CLAIM, jsonObject);
claims = this.claimTypeConverter.convert(claims); claims = this.claimTypeConverter.convert(claims);
assertThat(claims.get(STRING_CLAIM)).isEqualTo("true"); assertThat(claims.get(STRING_CLAIM)).isEqualTo("true");
assertThat(claims.get(BOOLEAN_CLAIM)).isEqualTo(Boolean.TRUE); assertThat(claims.get(BOOLEAN_CLAIM)).isEqualTo(Boolean.TRUE);
@ -131,6 +146,8 @@ public class ClaimTypeConverterTests {
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isEqualTo(listString); assertThat(claims.get(COLLECTION_STRING_CLAIM)).isEqualTo(listString);
assertThat(claims.get(LIST_STRING_CLAIM)).isEqualTo(listString); assertThat(claims.get(LIST_STRING_CLAIM)).isEqualTo(listString);
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isEqualTo(mapStringObject); 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 @Test
@ -155,9 +172,9 @@ public class ClaimTypeConverterTests {
assertThat(claims.get(BOOLEAN_CLAIM)).isSameAs(bool); assertThat(claims.get(BOOLEAN_CLAIM)).isSameAs(bool);
assertThat(claims.get(INSTANT_CLAIM)).isSameAs(instant); assertThat(claims.get(INSTANT_CLAIM)).isSameAs(instant);
assertThat(claims.get(URL_CLAIM)).isSameAs(url); assertThat(claims.get(URL_CLAIM)).isSameAs(url);
assertThat(claims.get(COLLECTION_STRING_CLAIM)).isSameAs(listString); assertThat(claims.get(COLLECTION_STRING_CLAIM)).isNotSameAs(listString).isEqualTo(listString);
assertThat(claims.get(LIST_STRING_CLAIM)).isSameAs(listString); assertThat(claims.get(LIST_STRING_CLAIM)).isNotSameAs(listString).isEqualTo(listString);
assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isSameAs(mapStringObject); assertThat(claims.get(MAP_STRING_OBJECT_CLAIM)).isNotSameAs(mapStringObject).isEqualTo(mapStringObject);
} }
@Test @Test