From f33598946fb9848826a64a4068e785dfe4f61d50 Mon Sep 17 00:00:00 2001 From: Fabio Guenci Date: Tue, 27 Jul 2021 18:24:11 +0200 Subject: [PATCH] Preserve Null Claim Values Prior to this commit ClaimTypeConverter returned the claims with the original value for all the claims with a null converted value. The changes allows ClaimTypeConverter to overwrite and return claims with converted value of null. Closes gh-10135 --- .../core/converter/ClaimTypeConverter.java | 2 +- .../jwt/MappedJwtClaimSetConverter.java | 45 +++++++------------ .../jwt/MappedJwtClaimSetConverterTests.java | 12 ++++- 3 files changed, 28 insertions(+), 31 deletions(-) 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 098cb86a01..863f19ad10 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. diff --git a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java index 50a6d382dc..64d2658313 100644 --- a/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.java +++ b/oauth2/oauth2-jose/src/main/java/org/springframework/security/oauth2/jwt/MappedJwtClaimSetConverter.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. @@ -45,19 +45,20 @@ public final class MappedJwtClaimSetConverter implements Converter> claimTypeConverters; - private final Converter, Map> delegate; /** * Constructs a {@link MappedJwtClaimSetConverter} with the provided arguments * * This will completely replace any set of default converters. * + * A converter that returns {@code null} removes the claim from the claim set. A + * converter that returns a non-{@code null} value adds or replaces that claim in the + * claim set. * @param claimTypeConverters The {@link Map} of converters to use */ public MappedJwtClaimSetConverter(Map> claimTypeConverters) { Assert.notNull(claimTypeConverters, "claimTypeConverters cannot be null"); this.claimTypeConverters = claimTypeConverters; - this.delegate = new ClaimTypeConverter(claimTypeConverters); } /** @@ -81,6 +82,9 @@ public final class MappedJwtClaimSetConverter implements Converter convert(Map claims) { Assert.notNull(claims, "claims cannot be null"); - - Map mappedClaims = this.delegate.convert(claims); - - mappedClaims = removeClaims(mappedClaims); - mappedClaims = addClaims(mappedClaims); - + Map mappedClaims = new HashMap<>(claims); + for (Map.Entry> entry : this.claimTypeConverters.entrySet()) { + String claimName = entry.getKey(); + Converter converter = entry.getValue(); + if (converter != null) { + Object claim = claims.get(claimName); + Object mappedClaim = converter.convert(claim); + mappedClaims.compute(claimName, (key, value) -> mappedClaim); + } + } Instant issuedAt = (Instant) mappedClaims.get(JwtClaimNames.IAT); Instant expiresAt = (Instant) mappedClaims.get(JwtClaimNames.EXP); if (issuedAt == null && expiresAt != null) { @@ -159,23 +167,4 @@ public final class MappedJwtClaimSetConverter implements Converter removeClaims(Map claims) { - Map result = new HashMap<>(); - for (Map.Entry entry : claims.entrySet()) { - if (entry.getValue() != null) { - result.put(entry.getKey(), entry.getValue()); - } - } - return result; - } - - private Map addClaims(Map claims) { - Map result = new HashMap<>(claims); - for (Map.Entry> entry : claimTypeConverters.entrySet()) { - if (!claims.containsKey(entry.getKey()) && entry.getValue().convert(null) != null) { - result.put(entry.getKey(), entry.getValue().convert(null)); - } - } - return result; - } } 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 322890449d..bbe917fabb 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2018 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. @@ -140,11 +140,19 @@ public class MappedJwtClaimSetConverterTests { assertThat(target.get(JwtClaimNames.SUB)).isEqualTo("1234"); } + // gh-10135 @Test public void convertWhenConverterReturnsNullThenClaimIsRemoved() { MappedJwtClaimSetConverter converter = MappedJwtClaimSetConverter - .withDefaults(Collections.emptyMap()); + .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);