Make withRoles Check Only Roles

This commit clarifies the semantics of withRoles,
which is to check the role-based authorities in an
authentication.

Closes gh-17843
This commit is contained in:
Josh Cummings 2025-09-03 16:25:16 -06:00
parent bd119ac411
commit de10e08348
No known key found for this signature in database
GPG Key ID: 869B37A20E876129
4 changed files with 44 additions and 23 deletions

View File

@ -18,7 +18,6 @@ package org.springframework.security.config.annotation.authentication.ldap;
import java.io.IOException; import java.io.IOException;
import java.net.ServerSocket; import java.net.ServerSocket;
import java.util.Collections;
import java.util.List; import java.util.List;
import javax.naming.directory.SearchControls; import javax.naming.directory.SearchControls;
@ -39,7 +38,6 @@ import org.springframework.security.config.annotation.configuration.ObjectPostPr
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContext;
import org.springframework.security.config.test.SpringTestContextExtension; import org.springframework.security.config.test.SpringTestContextExtension;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper; import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder; import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
import org.springframework.security.ldap.DefaultSpringSecurityContextSource; import org.springframework.security.ldap.DefaultSpringSecurityContextSource;
@ -120,8 +118,7 @@ public class LdapAuthenticationProviderBuilderSecurityBuilderTests {
this.spring.register(BindAuthenticationConfig.class).autowire(); this.spring.register(BindAuthenticationConfig.class).autowire();
this.mockMvc.perform(formLogin().user("bob").password("bobspassword")) this.mockMvc.perform(formLogin().user("bob").password("bobspassword"))
.andExpect(authenticated().withUsername("bob") .andExpect(authenticated().withUsername("bob").withRoles("DEVELOPERS"));
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_DEVELOPERS"))));
} }
// SEC-2472 // SEC-2472
@ -130,8 +127,7 @@ public class LdapAuthenticationProviderBuilderSecurityBuilderTests {
this.spring.register(PasswordEncoderConfig.class).autowire(); this.spring.register(PasswordEncoderConfig.class).autowire();
this.mockMvc.perform(formLogin().user("bcrypt").password("password")) this.mockMvc.perform(formLogin().user("bcrypt").password("password"))
.andExpect(authenticated().withUsername("bcrypt") .andExpect(authenticated().withUsername("bcrypt").withRoles("DEVELOPERS"));
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_DEVELOPERS"))));
} }
private LdapAuthenticationProvider ldapProvider() { private LdapAuthenticationProvider ldapProvider() {

View File

@ -16,8 +16,6 @@
package org.springframework.security.config.annotation.authentication.ldap; package org.springframework.security.config.annotation.authentication.ldap;
import java.util.Collections;
import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Test;
import org.junit.jupiter.api.extension.ExtendWith; import org.junit.jupiter.api.extension.ExtendWith;
@ -28,8 +26,6 @@ import org.springframework.security.config.annotation.authentication.ldap.LdapAu
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity; import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
import org.springframework.security.config.test.SpringTestContext; import org.springframework.security.config.test.SpringTestContext;
import org.springframework.security.config.test.SpringTestContextExtension; import org.springframework.security.config.test.SpringTestContextExtension;
import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders; import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders;
import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers; import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers;
import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.MockMvc;
@ -64,7 +60,7 @@ public class LdapAuthenticationProviderConfigurerTests {
.password("bobspassword"); .password("bobspassword");
SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated() SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated()
.withUsername("bob") .withUsername("bob")
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_DEVELOPERS"))); .withRoles("DEVELOPERS");
// @formatter:on // @formatter:on
this.mockMvc.perform(request).andExpect(expectedUser); this.mockMvc.perform(request).andExpect(expectedUser);
} }
@ -79,7 +75,7 @@ public class LdapAuthenticationProviderConfigurerTests {
.password("bobspassword"); .password("bobspassword");
SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated() SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated()
.withUsername("bob") .withUsername("bob")
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROL_DEVELOPERS"))); .withRoles("ROL_", new String[] { "DEVELOPERS" });
// @formatter:on // @formatter:on
this.mockMvc.perform(request).andExpect(expectedUser); this.mockMvc.perform(request).andExpect(expectedUser);
} }
@ -108,8 +104,7 @@ public class LdapAuthenticationProviderConfigurerTests {
.password("otherbenspassword"); .password("otherbenspassword");
SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated() SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated()
.withUsername("otherben") .withUsername("otherben")
.withAuthorities( .withRoles("SUBMANAGERS", "MANAGERS", "DEVELOPERS");
AuthorityUtils.createAuthorityList("ROLE_SUBMANAGERS", "ROLE_MANAGERS", "ROLE_DEVELOPERS"));
// @formatter:on // @formatter:on
this.mockMvc.perform(request).andExpect(expectedUser); this.mockMvc.perform(request).andExpect(expectedUser);
} }

View File

@ -16,7 +16,6 @@
package org.springframework.security.config.annotation.authentication.ldap; package org.springframework.security.config.annotation.authentication.ldap;
import java.util.Collections;
import java.util.HashSet; import java.util.HashSet;
import java.util.Set; import java.util.Set;
@ -34,7 +33,6 @@ import org.springframework.security.config.test.SpringTestContext;
import org.springframework.security.config.test.SpringTestContextExtension; import org.springframework.security.config.test.SpringTestContextExtension;
import org.springframework.security.core.GrantedAuthority; import org.springframework.security.core.GrantedAuthority;
import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.AuthorityUtils;
import org.springframework.security.core.authority.SimpleGrantedAuthority;
import org.springframework.security.ldap.DefaultSpringSecurityContextSource; import org.springframework.security.ldap.DefaultSpringSecurityContextSource;
import org.springframework.security.ldap.userdetails.DefaultLdapAuthoritiesPopulator; import org.springframework.security.ldap.userdetails.DefaultLdapAuthoritiesPopulator;
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders; import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders;
@ -79,7 +77,7 @@ public class NamespaceLdapAuthenticationProviderTests {
.user("bob") .user("bob")
.password("bobspassword"); .password("bobspassword");
SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated() SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated()
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("PREFIX_DEVELOPERS"))); .withRoles("PREFIX_", new String[] { "DEVELOPERS" });
// @formatter:on // @formatter:on
this.mockMvc.perform(request).andExpect(user); this.mockMvc.perform(request).andExpect(user);
} }
@ -103,7 +101,7 @@ public class NamespaceLdapAuthenticationProviderTests {
.user("bob") .user("bob")
.password("bobspassword"); .password("bobspassword");
SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated() SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated()
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_EXTRA"))); .withRoles("EXTRA");
// @formatter:on // @formatter:on
this.mockMvc.perform(request).andExpect(user); this.mockMvc.perform(request).andExpect(user);
} }

View File

@ -18,7 +18,9 @@ package org.springframework.security.test.web.servlet.response;
import java.util.ArrayList; import java.util.ArrayList;
import java.util.Collection; import java.util.Collection;
import java.util.List;
import java.util.function.Consumer; import java.util.function.Consumer;
import java.util.function.Predicate;
import org.jspecify.annotations.NullUnmarked; import org.jspecify.annotations.NullUnmarked;
import org.jspecify.annotations.Nullable; import org.jspecify.annotations.Nullable;
@ -94,6 +96,8 @@ public final class SecurityMockMvcResultMatchers {
private @Nullable Collection<? extends GrantedAuthority> expectedGrantedAuthorities; private @Nullable Collection<? extends GrantedAuthority> expectedGrantedAuthorities;
private Predicate<GrantedAuthority> ignoreAuthorities = (authority) -> false;
private @Nullable Consumer<Authentication> assertAuthentication; private @Nullable Consumer<Authentication> assertAuthentication;
AuthenticatedMatcher() { AuthenticatedMatcher() {
@ -132,7 +136,8 @@ public final class SecurityMockMvcResultMatchers {
} }
if (this.expectedGrantedAuthorities != null) { if (this.expectedGrantedAuthorities != null) {
AssertionErrors.assertTrue("Authentication cannot be null", auth != null); AssertionErrors.assertTrue("Authentication cannot be null", auth != null);
Collection<? extends GrantedAuthority> authorities = auth.getAuthorities(); Collection<? extends GrantedAuthority> authorities = new ArrayList<>(auth.getAuthorities());
authorities.removeIf(this.ignoreAuthorities);
AssertionErrors.assertTrue( AssertionErrors.assertTrue(
authorities + " does not contain the same authorities as " + this.expectedGrantedAuthorities, authorities + " does not contain the same authorities as " + this.expectedGrantedAuthorities,
authorities.containsAll(this.expectedGrantedAuthorities)); authorities.containsAll(this.expectedGrantedAuthorities));
@ -212,16 +217,43 @@ public final class SecurityMockMvcResultMatchers {
} }
/** /**
* Specifies the {@link Authentication#getAuthorities()} * Specifies the expected roles.
* <p>
* Since a set of authorities can contain more than just roles, this method
* differs from {@link #withAuthorities} in that it only verifies the authorities
* prefixed by {@code ROLE_}. Other authorities are ignored.
* <p>
* If you want to validate more than just roles, please use
* {@link #withAuthorities}.
* @param roles the roles. Each value is automatically prefixed with "ROLE_" * @param roles the roles. Each value is automatically prefixed with "ROLE_"
* @return the {@link AuthenticatedMatcher} for further customization * @return the {@link AuthenticatedMatcher} for further customization
*/ */
public AuthenticatedMatcher withRoles(String... roles) { public AuthenticatedMatcher withRoles(String... roles) {
Collection<GrantedAuthority> authorities = new ArrayList<>(); return withRoles("ROLE_", roles);
}
/**
* Specifies the expected roles.
* <p>
* Since a set of authorities can contain more than just roles, this method
* differs from {@link #withAuthorities} in that it only verifies the authorities
* prefixed by {@code ROLE_}. Other authorities are ignored.
* <p>
* If you want to validate more than just roles, please use
* {@link #withAuthorities}.
* @param rolePrefix the role prefix
* @param roles the roles. Each value is automatically prefixed with the
* {@code rolePrefix}
* @return the {@link AuthenticatedMatcher} for further customization
* @since 7.0
*/
public AuthenticatedMatcher withRoles(String rolePrefix, String[] roles) {
List<GrantedAuthority> withPrefix = new ArrayList<>();
for (String role : roles) { for (String role : roles) {
authorities.add(new SimpleGrantedAuthority("ROLE_" + role)); withPrefix.add(new SimpleGrantedAuthority(rolePrefix + role));
} }
return withAuthorities(authorities); this.ignoreAuthorities = (authority) -> !authority.getAuthority().startsWith(rolePrefix);
return withAuthorities(withPrefix);
} }
} }