diff --git a/core/src/main/java/org/springframework/security/core/userdetails/User.java b/core/src/main/java/org/springframework/security/core/userdetails/User.java index 86d57704dfb..8ed14bf1dc8 100644 --- a/core/src/main/java/org/springframework/security/core/userdetails/User.java +++ b/core/src/main/java/org/springframework/security/core/userdetails/User.java @@ -329,7 +329,7 @@ public static final class UserBuilder { private String password; - private List authorities; + private List authorities = new ArrayList<>(); private boolean accountExpired; @@ -427,6 +427,7 @@ public UserBuilder roles(String... roles) { * @see #roles(String...) */ public UserBuilder authorities(GrantedAuthority... authorities) { + Assert.notNull(authorities, "authorities cannot be null"); return authorities(Arrays.asList(authorities)); } @@ -439,7 +440,8 @@ public UserBuilder authorities(GrantedAuthority... authorities) { * @see #roles(String...) */ public UserBuilder authorities(Collection authorities) { - this.authorities = new ArrayList<>(authorities); + Assert.notNull(authorities, "authorities cannot be null"); + this.authorities.addAll(authorities); return this; } @@ -452,6 +454,7 @@ public UserBuilder authorities(Collection authoritie * @see #roles(String...) */ public UserBuilder authorities(String... authorities) { + Assert.notNull(authorities, "authorities cannot be null"); return authorities(AuthorityUtils.createAuthorityList(authorities)); } diff --git a/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java b/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java index 8c8855718fc..8d3825edc63 100644 --- a/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java +++ b/core/src/test/java/org/springframework/security/core/userdetails/UserTests.java @@ -18,6 +18,8 @@ import java.io.ByteArrayOutputStream; import java.io.ObjectOutputStream; +import java.util.ArrayList; +import java.util.Collection; import java.util.HashSet; import java.util.List; import java.util.Set; @@ -37,6 +39,7 @@ * Tests {@link User}. * * @author Ben Alex + * @author Ilya Starchenko */ public class UserTests { @@ -68,6 +71,33 @@ public void testNoArgConstructorDoesntExist() { .isThrownBy(() -> User.class.getDeclaredConstructor((Class[]) null)); } + @Test + public void testBuildUserWithNoAuthorities() { + UserDetails user = User.builder().username("user").password("password").build(); + assertThat(user.getAuthorities()).isEmpty(); + } + + @Test + public void testNullWithinUserAuthoritiesIsRejected() { + assertThatIllegalArgumentException().isThrownBy(() -> User.builder().username("user").password("password") + .authorities((Collection) null).build()); + List authorities = new ArrayList<>(); + authorities.add(null); + authorities.add(null); + assertThatIllegalArgumentException().isThrownBy( + () -> User.builder().username("user").password("password").authorities(authorities).build()); + + assertThatIllegalArgumentException().isThrownBy(() -> User.builder().username("user").password("password") + .authorities((GrantedAuthority[]) null).build()); + assertThatIllegalArgumentException().isThrownBy(() -> User.builder().username("user").password("password") + .authorities(new GrantedAuthority[] { null, null }).build()); + + assertThatIllegalArgumentException().isThrownBy( + () -> User.builder().username("user").password("password").authorities((String[]) null).build()); + assertThatIllegalArgumentException().isThrownBy(() -> User.builder().username("user").password("password") + .authorities(new String[] { null, null }).build()); + } + @Test public void testNullValuesRejected() { assertThatIllegalArgumentException().isThrownBy(() -> new User(null, "koala", true, true, true, true, ROLE_12));