From 1f04baa4a3e76992c9142fe554b7a627fb6ae795 Mon Sep 17 00:00:00 2001 From: Steve Riesenberg Date: Tue, 13 Jun 2023 14:07:31 -0500 Subject: [PATCH] Polish gh-13290 Issue gh-12533 --- .../security/core/userdetails/User.java | 3 +- .../security/core/userdetails/UserTests.java | 44 +++++++++++-------- 2 files changed, 26 insertions(+), 21 deletions(-) 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 3629c8bd8e..141f65dde8 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 @@ -441,8 +441,7 @@ public class User implements UserDetails, CredentialsContainer { */ public UserBuilder authorities(Collection authorities) { Assert.notNull(authorities, "authorities cannot be null"); - this.authorities.clear(); - this.authorities.addAll(authorities); + this.authorities = new ArrayList<>(authorities); return this; } 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 9402856b57..13bcfb3c60 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,19 +18,21 @@ package org.springframework.security.core.userdetails; import java.io.ByteArrayOutputStream; import java.io.ObjectOutputStream; -import java.util.*; +import java.util.ArrayList; +import java.util.Collection; +import java.util.HashSet; +import java.util.List; +import java.util.Set; import java.util.function.Function; -import java.util.stream.Stream; import org.junit.jupiter.api.Test; - import org.junit.jupiter.params.ParameterizedTest; -import org.junit.jupiter.params.provider.Arguments; -import org.junit.jupiter.params.provider.MethodSource; +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.AuthorityUtils; import org.springframework.security.core.authority.SimpleGrantedAuthority; -import org.springframework.util.CollectionUtils; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatExceptionOfType; @@ -46,14 +48,6 @@ public class UserTests { private static final List ROLE_12 = AuthorityUtils.createAuthorityList("ROLE_ONE", "ROLE_TWO"); - public static Stream testNewAuthoritiesShouldReplacePreviousAuthorities() { - return Stream.of( - Arguments.of((Object) new String[0]), - Arguments.of((Object) new String[]{"B7", "C12", "role"}), - Arguments.of((Object) new String[]{"A1"}) - ); - } - @Test public void equalsReturnsTrueIfUsernamesAreTheSame() { User user1 = new User("rod", "koala", true, true, true, true, ROLE_12); @@ -107,16 +101,28 @@ public class UserTests { .authorities(new String[] { null, null }).build()); } + // gh-12533 @ParameterizedTest - @MethodSource - public void testNewAuthoritiesShouldReplacePreviousAuthorities(String[] authorities) { - UserDetails parent = User.builder().username("user").password("password").authorities("A1", "A2", "B1").build(); - User.UserBuilder builder = User.withUserDetails(parent).authorities(authorities); + @NullSource + @ValueSource(strings = { "ROLE_USER,ROLE_ADMIN,read", "read" }) + public void withUserDetailsWhenAuthoritiesThenOverridesPreviousAuthorities(String arg) { + // @formatter:off + UserDetails parent = User.builder() + .username("user") + .password("password") + .authorities("one", "two", "three") + .build(); + // @formatter:on + String[] authorities = (arg != null) ? arg.split(",") : new String[0]; + User.UserBuilder builder = User.withUserDetails(parent); UserDetails user = builder.build(); + assertThat(AuthorityUtils.authorityListToSet(user.getAuthorities())).containsOnly("one", "two", "three"); + user = builder.authorities(authorities).build(); assertThat(AuthorityUtils.authorityListToSet(user.getAuthorities())).containsOnly(authorities); user = builder.authorities(AuthorityUtils.createAuthorityList(authorities)).build(); assertThat(AuthorityUtils.authorityListToSet(user.getAuthorities())).containsOnly(authorities); - user = builder.authorities(AuthorityUtils.createAuthorityList(authorities).toArray(GrantedAuthority[]::new)).build(); + user = builder.authorities(AuthorityUtils.createAuthorityList(authorities).toArray(GrantedAuthority[]::new)) + .build(); assertThat(AuthorityUtils.authorityListToSet(user.getAuthorities())).containsOnly(authorities); }