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 c7b4e4a493
commit 174b71c017
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) { 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

@ -52,9 +52,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;

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; 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.Lists;
import org.junit.Test; import org.junit.Test;
import org.springframework.core.convert.ConversionService;
import java.net.URL; import java.net.URL;
import java.time.Instant; import java.time.Instant;
@ -29,6 +30,8 @@ import java.util.HashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;
import org.springframework.core.convert.ConversionService;
import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThat;
/** /**
@ -141,9 +144,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
@ -152,6 +155,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";
@ -165,9 +179,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
@ -189,7 +203,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");
@ -197,7 +211,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
@ -219,6 +234,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

@ -15,7 +15,10 @@
*/ */
package org.springframework.security.oauth2.core.converter; 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.Lists;
import org.assertj.core.util.Maps;
import org.junit.Before; import org.junit.Before;
import org.junit.Test; import org.junit.Test;
import org.springframework.core.convert.TypeDescriptor; 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 COLLECTION_STRING_CLAIM = "collection-string-claim";
private static final String LIST_STRING_CLAIM = "list-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 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
@ -107,7 +112,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");
@ -116,7 +126,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");
@ -126,6 +137,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
@ -153,9 +166,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