Skip to content

Commit de10e08

Browse files
committed
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
1 parent bd119ac commit de10e08

File tree

4 files changed

+44
-23
lines changed

4 files changed

+44
-23
lines changed

config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/LdapAuthenticationProviderBuilderSecurityBuilderTests.java

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818

1919
import java.io.IOException;
2020
import java.net.ServerSocket;
21-
import java.util.Collections;
2221
import java.util.List;
2322

2423
import javax.naming.directory.SearchControls;
@@ -39,7 +38,6 @@
3938
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
4039
import org.springframework.security.config.test.SpringTestContext;
4140
import org.springframework.security.config.test.SpringTestContextExtension;
42-
import org.springframework.security.core.authority.SimpleGrantedAuthority;
4341
import org.springframework.security.core.authority.mapping.GrantedAuthoritiesMapper;
4442
import org.springframework.security.crypto.bcrypt.BCryptPasswordEncoder;
4543
import org.springframework.security.ldap.DefaultSpringSecurityContextSource;
@@ -120,8 +118,7 @@ public void bindAuthentication() throws Exception {
120118
this.spring.register(BindAuthenticationConfig.class).autowire();
121119

122120
this.mockMvc.perform(formLogin().user("bob").password("bobspassword"))
123-
.andExpect(authenticated().withUsername("bob")
124-
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_DEVELOPERS"))));
121+
.andExpect(authenticated().withUsername("bob").withRoles("DEVELOPERS"));
125122
}
126123

127124
// SEC-2472
@@ -130,8 +127,7 @@ public void canUseCryptoPasswordEncoder() throws Exception {
130127
this.spring.register(PasswordEncoderConfig.class).autowire();
131128

132129
this.mockMvc.perform(formLogin().user("bcrypt").password("password"))
133-
.andExpect(authenticated().withUsername("bcrypt")
134-
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_DEVELOPERS"))));
130+
.andExpect(authenticated().withUsername("bcrypt").withRoles("DEVELOPERS"));
135131
}
136132

137133
private LdapAuthenticationProvider ldapProvider() {

config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/LdapAuthenticationProviderConfigurerTests.java

Lines changed: 3 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616

1717
package org.springframework.security.config.annotation.authentication.ldap;
1818

19-
import java.util.Collections;
20-
2119
import org.junit.jupiter.api.Test;
2220
import org.junit.jupiter.api.extension.ExtendWith;
2321

@@ -28,8 +26,6 @@
2826
import org.springframework.security.config.annotation.web.configuration.EnableWebSecurity;
2927
import org.springframework.security.config.test.SpringTestContext;
3028
import org.springframework.security.config.test.SpringTestContextExtension;
31-
import org.springframework.security.core.authority.AuthorityUtils;
32-
import org.springframework.security.core.authority.SimpleGrantedAuthority;
3329
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders;
3430
import org.springframework.security.test.web.servlet.response.SecurityMockMvcResultMatchers;
3531
import org.springframework.test.web.servlet.MockMvc;
@@ -64,7 +60,7 @@ public void authenticationManagerSupportMultipleLdapContextWithDefaultRolePrefix
6460
.password("bobspassword");
6561
SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated()
6662
.withUsername("bob")
67-
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_DEVELOPERS")));
63+
.withRoles("DEVELOPERS");
6864
// @formatter:on
6965
this.mockMvc.perform(request).andExpect(expectedUser);
7066
}
@@ -79,7 +75,7 @@ public void authenticationManagerSupportMultipleLdapContextWithCustomRolePrefix(
7975
.password("bobspassword");
8076
SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated()
8177
.withUsername("bob")
82-
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROL_DEVELOPERS")));
78+
.withRoles("ROL_", new String[] { "DEVELOPERS" });
8379
// @formatter:on
8480
this.mockMvc.perform(request).andExpect(expectedUser);
8581
}
@@ -108,8 +104,7 @@ public void authenticationManagerWhenSearchSubtreeThenNestedGroupFound() throws
108104
.password("otherbenspassword");
109105
SecurityMockMvcResultMatchers.AuthenticatedMatcher expectedUser = authenticated()
110106
.withUsername("otherben")
111-
.withAuthorities(
112-
AuthorityUtils.createAuthorityList("ROLE_SUBMANAGERS", "ROLE_MANAGERS", "ROLE_DEVELOPERS"));
107+
.withRoles("SUBMANAGERS", "MANAGERS", "DEVELOPERS");
113108
// @formatter:on
114109
this.mockMvc.perform(request).andExpect(expectedUser);
115110
}

config/src/integration-test/java/org/springframework/security/config/annotation/authentication/ldap/NamespaceLdapAuthenticationProviderTests.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616

1717
package org.springframework.security.config.annotation.authentication.ldap;
1818

19-
import java.util.Collections;
2019
import java.util.HashSet;
2120
import java.util.Set;
2221

@@ -34,7 +33,6 @@
3433
import org.springframework.security.config.test.SpringTestContextExtension;
3534
import org.springframework.security.core.GrantedAuthority;
3635
import org.springframework.security.core.authority.AuthorityUtils;
37-
import org.springframework.security.core.authority.SimpleGrantedAuthority;
3836
import org.springframework.security.ldap.DefaultSpringSecurityContextSource;
3937
import org.springframework.security.ldap.userdetails.DefaultLdapAuthoritiesPopulator;
4038
import org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestBuilders;
@@ -79,7 +77,7 @@ public void ldapAuthenticationProviderCustom() throws Exception {
7977
.user("bob")
8078
.password("bobspassword");
8179
SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated()
82-
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("PREFIX_DEVELOPERS")));
80+
.withRoles("PREFIX_", new String[] { "DEVELOPERS" });
8381
// @formatter:on
8482
this.mockMvc.perform(request).andExpect(user);
8583
}
@@ -103,7 +101,7 @@ protected Set<GrantedAuthority> getAdditionalRoles(DirContextOperations user, St
103101
.user("bob")
104102
.password("bobspassword");
105103
SecurityMockMvcResultMatchers.AuthenticatedMatcher user = authenticated()
106-
.withAuthorities(Collections.singleton(new SimpleGrantedAuthority("ROLE_EXTRA")));
104+
.withRoles("EXTRA");
107105
// @formatter:on
108106
this.mockMvc.perform(request).andExpect(user);
109107
}

test/src/main/java/org/springframework/security/test/web/servlet/response/SecurityMockMvcResultMatchers.java

Lines changed: 37 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,9 @@
1818

1919
import java.util.ArrayList;
2020
import java.util.Collection;
21+
import java.util.List;
2122
import java.util.function.Consumer;
23+
import java.util.function.Predicate;
2224

2325
import org.jspecify.annotations.NullUnmarked;
2426
import org.jspecify.annotations.Nullable;
@@ -94,6 +96,8 @@ public static final class AuthenticatedMatcher extends AuthenticationMatcher<Aut
9496

9597
private @Nullable Collection<? extends GrantedAuthority> expectedGrantedAuthorities;
9698

99+
private Predicate<GrantedAuthority> ignoreAuthorities = (authority) -> false;
100+
97101
private @Nullable Consumer<Authentication> assertAuthentication;
98102

99103
AuthenticatedMatcher() {
@@ -132,7 +136,8 @@ public void match(MvcResult result) {
132136
}
133137
if (this.expectedGrantedAuthorities != null) {
134138
AssertionErrors.assertTrue("Authentication cannot be null", auth != null);
135-
Collection<? extends GrantedAuthority> authorities = auth.getAuthorities();
139+
Collection<? extends GrantedAuthority> authorities = new ArrayList<>(auth.getAuthorities());
140+
authorities.removeIf(this.ignoreAuthorities);
136141
AssertionErrors.assertTrue(
137142
authorities + " does not contain the same authorities as " + this.expectedGrantedAuthorities,
138143
authorities.containsAll(this.expectedGrantedAuthorities));
@@ -212,16 +217,43 @@ public AuthenticatedMatcher withAuthorities(Collection<? extends GrantedAuthorit
212217
}
213218

214219
/**
215-
* Specifies the {@link Authentication#getAuthorities()}
220+
* Specifies the expected roles.
221+
* <p>
222+
* Since a set of authorities can contain more than just roles, this method
223+
* differs from {@link #withAuthorities} in that it only verifies the authorities
224+
* prefixed by {@code ROLE_}. Other authorities are ignored.
225+
* <p>
226+
* If you want to validate more than just roles, please use
227+
* {@link #withAuthorities}.
216228
* @param roles the roles. Each value is automatically prefixed with "ROLE_"
217229
* @return the {@link AuthenticatedMatcher} for further customization
218230
*/
219231
public AuthenticatedMatcher withRoles(String... roles) {
220-
Collection<GrantedAuthority> authorities = new ArrayList<>();
232+
return withRoles("ROLE_", roles);
233+
}
234+
235+
/**
236+
* Specifies the expected roles.
237+
* <p>
238+
* Since a set of authorities can contain more than just roles, this method
239+
* differs from {@link #withAuthorities} in that it only verifies the authorities
240+
* prefixed by {@code ROLE_}. Other authorities are ignored.
241+
* <p>
242+
* If you want to validate more than just roles, please use
243+
* {@link #withAuthorities}.
244+
* @param rolePrefix the role prefix
245+
* @param roles the roles. Each value is automatically prefixed with the
246+
* {@code rolePrefix}
247+
* @return the {@link AuthenticatedMatcher} for further customization
248+
* @since 7.0
249+
*/
250+
public AuthenticatedMatcher withRoles(String rolePrefix, String[] roles) {
251+
List<GrantedAuthority> withPrefix = new ArrayList<>();
221252
for (String role : roles) {
222-
authorities.add(new SimpleGrantedAuthority("ROLE_" + role));
253+
withPrefix.add(new SimpleGrantedAuthority(rolePrefix + role));
223254
}
224-
return withAuthorities(authorities);
255+
this.ignoreAuthorities = (authority) -> !authority.getAuthority().startsWith(rolePrefix);
256+
return withAuthorities(withPrefix);
225257
}
226258

227259
}

0 commit comments

Comments
 (0)