From e3249dfda0ff5fbc5b7497f455804f84e5cd56ff Mon Sep 17 00:00:00 2001 From: chanbinme Date: Sun, 15 Jun 2025 00:58:29 +0900 Subject: [PATCH 1/2] Improve authoritiesClaimName validation in JwtGrantedAuthoritiesConverter Use StringUtils.hasText() instead of null check to properly handle empty strings and whitespace-only strings. Signed-off-by: chanbinme --- .../JwtGrantedAuthoritiesConverter.java | 2 +- .../JwtGrantedAuthoritiesConverterTests.java | 21 +++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java index bc625793ef8..29642e47bcf 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java @@ -106,7 +106,7 @@ public void setAuthoritiesClaimName(String authoritiesClaimName) { } private String getAuthoritiesClaimName(Jwt jwt) { - if (this.authoritiesClaimName != null) { + if (StringUtils.hasText(this.authoritiesClaimName)) { return this.authoritiesClaimName; } for (String claimName : WELL_KNOWN_AUTHORITIES_CLAIM_NAMES) { diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java index 2cb5390907f..67e4ffe72b7 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java @@ -21,11 +21,15 @@ import java.util.Collections; import org.junit.jupiter.api.Test; +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.NullSource; +import org.junit.jupiter.params.provider.ValueSource; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.jwt.TestJwts; +import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -270,4 +274,21 @@ public void convertWithCustomAuthoritiesSplitRegexWhenTokenHasScopeAttributeThen new SimpleGrantedAuthority("SCOPE_message:write")); } + @ParameterizedTest + @ValueSource(strings = { "", " " }) + @NullSource + public void convertWhenAuthoritiesClaimNameIsBlankThenUsesWellKnownClaims(String invalidClaimName) + throws Exception { + // @formatter:off + Jwt jwt = TestJwts.jwt() + .claim("scope", "message:read message:write") + .build(); + // @formatter:on + JwtGrantedAuthoritiesConverter jwtGrantedAuthoritiesConverter = new JwtGrantedAuthoritiesConverter(); + ReflectionTestUtils.setField(jwtGrantedAuthoritiesConverter, "authoritiesClaimName", invalidClaimName); + Collection authorities = jwtGrantedAuthoritiesConverter.convert(jwt); + assertThat(authorities).containsExactly(new SimpleGrantedAuthority("SCOPE_message:read"), + new SimpleGrantedAuthority("SCOPE_message:write")); + } + } From 39b5cf545c5b8ff4b05e3d58b9f0b3490545491f Mon Sep 17 00:00:00 2001 From: chanbinme Date: Thu, 19 Jun 2025 00:25:57 +0900 Subject: [PATCH 2/2] Refactor authoritiesClaimName to use Collection and remove null checks - Change authoritiesClaimName field to Collection authoritiesClaimNames - Add isExplicitlySet flag to preserve original behavior - Remove null checks by ensuring authoritiesClaimNames is always initialized - Maintain backward compatibility for explicit vs default claim name handling - Delete unnecessary test code related to previous null-checking logic Signed-off-by: chanbinme --- .../JwtGrantedAuthoritiesConverter.java | 15 +++++++------ .../JwtGrantedAuthoritiesConverterTests.java | 21 ------------------- 2 files changed, 9 insertions(+), 27 deletions(-) diff --git a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java index 29642e47bcf..943aa11441f 100644 --- a/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java +++ b/oauth2/oauth2-resource-server/src/main/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverter.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2022 the original author or authors. + * Copyright 2002-2025 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. @@ -53,7 +53,9 @@ public final class JwtGrantedAuthoritiesConverter implements Converter authoritiesClaimNames = WELL_KNOWN_AUTHORITIES_CLAIM_NAMES; + + private boolean isExplicitlySet = false; /** * Extract {@link GrantedAuthority}s from the given {@link Jwt}. @@ -102,14 +104,15 @@ public void setAuthoritiesClaimDelimiter(String authoritiesClaimDelimiter) { */ public void setAuthoritiesClaimName(String authoritiesClaimName) { Assert.hasText(authoritiesClaimName, "authoritiesClaimName cannot be empty"); - this.authoritiesClaimName = authoritiesClaimName; + this.authoritiesClaimNames = Collections.singletonList(authoritiesClaimName); + this.isExplicitlySet = true; } private String getAuthoritiesClaimName(Jwt jwt) { - if (StringUtils.hasText(this.authoritiesClaimName)) { - return this.authoritiesClaimName; + if (this.isExplicitlySet) { + return this.authoritiesClaimNames.iterator().next(); } - for (String claimName : WELL_KNOWN_AUTHORITIES_CLAIM_NAMES) { + for (String claimName : this.authoritiesClaimNames) { if (jwt.hasClaim(claimName)) { return claimName; } diff --git a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java index 67e4ffe72b7..2cb5390907f 100644 --- a/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java +++ b/oauth2/oauth2-resource-server/src/test/java/org/springframework/security/oauth2/server/resource/authentication/JwtGrantedAuthoritiesConverterTests.java @@ -21,15 +21,11 @@ import java.util.Collections; import org.junit.jupiter.api.Test; -import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.NullSource; -import org.junit.jupiter.params.provider.ValueSource; import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.security.oauth2.jwt.Jwt; import org.springframework.security.oauth2.jwt.TestJwts; -import org.springframework.test.util.ReflectionTestUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException; @@ -274,21 +270,4 @@ public void convertWithCustomAuthoritiesSplitRegexWhenTokenHasScopeAttributeThen new SimpleGrantedAuthority("SCOPE_message:write")); } - @ParameterizedTest - @ValueSource(strings = { "", " " }) - @NullSource - public void convertWhenAuthoritiesClaimNameIsBlankThenUsesWellKnownClaims(String invalidClaimName) - throws Exception { - // @formatter:off - Jwt jwt = TestJwts.jwt() - .claim("scope", "message:read message:write") - .build(); - // @formatter:on - JwtGrantedAuthoritiesConverter jwtGrantedAuthoritiesConverter = new JwtGrantedAuthoritiesConverter(); - ReflectionTestUtils.setField(jwtGrantedAuthoritiesConverter, "authoritiesClaimName", invalidClaimName); - Collection authorities = jwtGrantedAuthoritiesConverter.convert(jwt); - assertThat(authorities).containsExactly(new SimpleGrantedAuthority("SCOPE_message:read"), - new SimpleGrantedAuthority("SCOPE_message:write")); - } - }