diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java index 0597f561fed..c1a70f96511 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToListStringConverter.java @@ -56,12 +56,6 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t 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 results = new ArrayList<>(); for (Object object : ((Collection) source)) { diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java index 08d4f6ef971..2a6e2418124 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ObjectToMapStringObjectConverter.java @@ -53,9 +53,6 @@ public Object convert(Object source, TypeDescriptor sourceType, TypeDescriptor t return null; } Map sourceMap = (Map) source; - if (!sourceMap.isEmpty() && sourceMap.keySet().iterator().next() instanceof String) { - return source; - } Map result = new HashMap<>(); sourceMap.forEach((k, v) -> result.put(k.toString(), v)); return result; diff --git a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimConversionServiceTests.java b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimConversionServiceTests.java index 6a07f8f37f4..f77687a4284 100644 --- a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimConversionServiceTests.java +++ b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimConversionServiceTests.java @@ -26,6 +26,8 @@ import java.util.List; 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.junit.Test; @@ -145,9 +147,9 @@ public void convertCollectionStringWhenNullThenReturnNull() { } @Test - public void convertCollectionStringWhenListStringThenReturnSame() { + public void convertCollectionStringWhenListStringThenReturnNotSameButEqual() { List 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 @@ -156,6 +158,17 @@ public void convertCollectionStringWhenListNumberThenConverts() { .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"; @@ -169,9 +182,9 @@ public void convertListStringWhenNullThenReturnNull() { } @Test - public void convertListStringWhenListStringThenReturnSame() { + public void convertListStringWhenListStringThenReturnNotSameButEqual() { List 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 @@ -192,7 +205,7 @@ public void convertMapStringObjectWhenNullThenReturnNull() { } @Test - public void convertMapStringObjectWhenMapStringObjectThenReturnSame() { + public void convertMapStringObjectWhenMapStringObjectThenReturnNotSameButEqual() { Map mapStringObject = new HashMap() { { put("key1", "value1"); @@ -200,7 +213,8 @@ public void convertMapStringObjectWhenMapStringObjectThenReturnSame() { put("key3", "value3"); } }; - assertThat(this.conversionService.convert(mapStringObject, Map.class)).isSameAs(mapStringObject); + assertThat(this.conversionService.convert(mapStringObject, Map.class)).isNotSameAs(mapStringObject) + .isEqualTo(mapStringObject); } @Test @@ -222,6 +236,22 @@ public void convertMapStringObjectWhenMapIntegerObjectThenConverts() { 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 mapStringObject = new HashMap() { + { + put("1", "value1"); + put("2", "value2"); + } + }; + assertThat(this.conversionService.convert(jsonObject, Map.class)).isNotInstanceOf(JSONObject.class) + .isEqualTo(mapStringObject); + } + @Test public void convertMapStringObjectWhenNotConvertibleThenReturnNull() { List notConvertibleList = Lists.list("1", "2", "3", "4"); diff --git a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverterTests.java b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverterTests.java index 0b0d9f0acd7..2a1f329df17 100644 --- a/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverterTests.java +++ b/oauth2/oauth2-core/src/test/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverterTests.java @@ -23,7 +23,10 @@ import java.util.List; 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.Maps; import org.junit.Before; 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 JSON_ARRAY_CLAIM = "json-array-claim"; + + private static final String JSON_OBJECT_CLAIM = "json-object-claim"; + private ClaimTypeConverter claimTypeConverter; @Before @@ -115,6 +122,12 @@ public void convertWhenAllClaimsRequireConversionThenConvertAll() throws Excepti mapIntegerObject.put(1, "value1"); Map mapStringObject = new HashMap<>(); mapStringObject.put("1", "value1"); + JSONArray jsonArray = new JSONArray(); + jsonArray.add("1"); + List jsonArrayListString = Lists.list("1"); + JSONObject jsonObject = new JSONObject(); + jsonObject.put("1", "value1"); + Map jsonObjectMap = Maps.newHashMap("1", "value1"); Map claims = new HashMap<>(); claims.put(STRING_CLAIM, Boolean.TRUE); claims.put(BOOLEAN_CLAIM, "true"); @@ -123,6 +136,8 @@ public void convertWhenAllClaimsRequireConversionThenConvertAll() throws Excepti 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"); assertThat(claims.get(BOOLEAN_CLAIM)).isEqualTo(Boolean.TRUE); @@ -131,6 +146,8 @@ public void convertWhenAllClaimsRequireConversionThenConvertAll() throws Excepti 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 @@ -155,9 +172,9 @@ public void convertWhenNoClaimsRequireConversionThenConvertNone() throws Excepti 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