diff --git a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverter.java b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverter.java index 1eb661f2a4..06d2804408 100644 --- a/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverter.java +++ b/oauth2/oauth2-core/src/main/java/org/springframework/security/oauth2/core/converter/ClaimTypeConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2019 the original author or authors. + * Copyright 2002-2021 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -56,10 +56,7 @@ public final class ClaimTypeConverter implements Converter, this.claimTypeConverters.forEach((claimName, typeConverter) -> { if (claims.containsKey(claimName)) { Object claim = claims.get(claimName); - Object mappedClaim = typeConverter.convert(claim); - if (mappedClaim != null) { - result.put(claimName, mappedClaim); - } + result.put(claimName, typeConverter.convert(claim)); } }); return result; 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 586ab8653a..19544027cd 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 @@ -62,6 +62,8 @@ public class ClaimTypeConverterTests { private static final String JSON_OBJECT_CLAIM = "json-object-claim"; + private static final String NULL_OBJECT_CLAIM = "null-object-claim"; + private ClaimTypeConverter claimTypeConverter; @BeforeEach @@ -77,6 +79,7 @@ public class ClaimTypeConverterTests { TypeDescriptor.collection(List.class, TypeDescriptor.valueOf(String.class))); Converter mapStringObjectConverter = getConverter(TypeDescriptor.map(Map.class, TypeDescriptor.valueOf(String.class), TypeDescriptor.valueOf(Object.class))); + Converter nullConverter = (value) -> null; Map> claimTypeConverters = new HashMap<>(); claimTypeConverters.put(STRING_CLAIM, stringConverter); claimTypeConverters.put(BOOLEAN_CLAIM, booleanConverter); @@ -85,6 +88,7 @@ public class ClaimTypeConverterTests { claimTypeConverters.put(COLLECTION_STRING_CLAIM, collectionStringConverter); claimTypeConverters.put(LIST_STRING_CLAIM, listStringConverter); claimTypeConverters.put(MAP_STRING_OBJECT_CLAIM, mapStringObjectConverter); + claimTypeConverters.put(NULL_OBJECT_CLAIM, nullConverter); this.claimTypeConverter = new ClaimTypeConverter(claimTypeConverters); } @@ -138,6 +142,7 @@ public class ClaimTypeConverterTests { claims.put(MAP_STRING_OBJECT_CLAIM, mapIntegerObject); claims.put(JSON_ARRAY_CLAIM, jsonArray); claims.put(JSON_OBJECT_CLAIM, jsonObject); + claims.put(NULL_OBJECT_CLAIM, instant.toString()); claims = this.claimTypeConverter.convert(claims); assertThat(claims.get(STRING_CLAIM)).isEqualTo("true"); assertThat(claims.get(BOOLEAN_CLAIM)).isEqualTo(Boolean.TRUE); @@ -148,6 +153,7 @@ public class ClaimTypeConverterTests { 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); + assertThat(claims.get(NULL_OBJECT_CLAIM)).isNull(); } @Test diff --git a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java index 309559d5f5..29a5a5b536 100644 --- a/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java +++ b/oauth2/oauth2-jose/src/test/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverterTests.java @@ -123,8 +123,18 @@ public class MappedJwtClaimSetConverterTests { assertThat(target.get(JwtClaimNames.SUB)).isEqualTo("1234"); } + // gh-10135 @Test public void convertWhenConverterReturnsNullThenClaimIsRemoved() { + MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter + .withDefaults(Collections.singletonMap(JwtClaimNames.NBF, (nbfClaimValue) -> null)); + Map source = Collections.singletonMap(JwtClaimNames.NBF, Instant.now()); + Map target = converter.convert(source); + assertThat(target).doesNotContainKey(JwtClaimNames.NBF); + } + + @Test + public void convertWhenClaimValueIsNullThenClaimIsRemoved() { MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter.withDefaults(Collections.emptyMap()); Map source = Collections.singletonMap(JwtClaimNames.ISS, null); Map target = converter.convert(source);