diff --git a/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java b/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java index 30ea1db90e..e4a5c77fe5 100755 --- a/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java +++ b/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImpl.java @@ -24,7 +24,6 @@ import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.Set; -import java.util.stream.Stream; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; @@ -35,8 +34,6 @@ import org.springframework.security.core.authority.AuthorityUtils; import org.springframework.security.core.authority.SimpleGrantedAuthority; import org.springframework.util.Assert; -import static org.springframework.security.access.hierarchicalroles.RoleHierarchyUtils.roleHierarchyFromMap; - /** *

* This class defines a role hierarchy for use with various access checking components. @@ -109,6 +106,7 @@ public class RoleHierarchyImpl implements RoleHierarchy { * Factory method that creates a {@link Builder} instance with the default role prefix * "ROLE_" * @return a {@link Builder} instance with the default role prefix "ROLE_" + * @since 6.3 */ public static Builder withDefaultRolePrefix() { return withRolePrefix("ROLE_"); @@ -120,20 +118,13 @@ public class RoleHierarchyImpl implements RoleHierarchy { * @param rolePrefix the prefix to be used for the roles in the hierarchy. * @return a new {@link Builder} instance with the specified role prefix * @throws IllegalArgumentException if the provided role prefix is null + * @since 6.3 */ public static Builder withRolePrefix(String rolePrefix) { Assert.notNull(rolePrefix, "rolePrefix must not be null"); return new Builder(rolePrefix); } - /** - * Factory method that creates a {@link Builder} instance with no role prefix. - * @return a new {@link Builder} instance with no role prefix. - */ - public static Builder withNoRolePrefix() { - return withRolePrefix(""); - } - /** * Set the role hierarchy and pre-calculate for every role the set of all reachable * roles, i.e. all roles lower in the hierarchy of every given role. Pre-calculation @@ -259,22 +250,22 @@ public class RoleHierarchyImpl implements RoleHierarchy { private final String rolePrefix; - private final Map> roleBranches; + private final Map> hierarchy; private Builder(String rolePrefix) { this.rolePrefix = rolePrefix; - this.roleBranches = new LinkedHashMap<>(); + this.hierarchy = new LinkedHashMap<>(); } /** * Creates a new hierarchy branch to define a role and its child roles. * @param role the highest role in this branch - * @return a {@link RoleBranchBuilder} to define the child roles for the + * @return a {@link ImpliedRoles} to define the child roles for the * role */ - public RoleBranchBuilder role(String role) { + public ImpliedRoles role(String role) { Assert.hasText(role, "role must not be empty"); - return new RoleBranchBuilder(this, rolePrefix.concat(role)); + return new ImpliedRoles(role); } /** @@ -283,23 +274,29 @@ public class RoleHierarchyImpl implements RoleHierarchy { * @return a {@link RoleHierarchyImpl} */ public RoleHierarchyImpl build() { - String roleHierarchyRepresentation = roleHierarchyFromMap(roleBranches); + String roleHierarchyRepresentation = RoleHierarchyUtils.roleHierarchyFromMap(this.hierarchy); RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl(); roleHierarchy.setHierarchy(roleHierarchyRepresentation); return roleHierarchy; } + private Builder addHierarchy(String role, String... impliedRoles) { + List withPrefix = new ArrayList<>(); + for (String impliedRole : impliedRoles) { + withPrefix.add(this.rolePrefix.concat(impliedRole)); + } + this.hierarchy.put(this.rolePrefix.concat(role), withPrefix); + return this; + } + /** * Builder class for constructing child roles within a role hierarchy branch. */ - public static final class RoleBranchBuilder { - - private final Builder parentBuilder; + public final class ImpliedRoles { private final String role; - private RoleBranchBuilder(Builder parentBuilder, String role) { - this.parentBuilder = parentBuilder; + private ImpliedRoles(String role) { this.role = role; } @@ -313,9 +310,7 @@ public class RoleHierarchyImpl implements RoleHierarchy { public Builder implies(String... impliedRoles) { Assert.notEmpty(impliedRoles, "at least one implied role must be provided"); Assert.noNullElements(impliedRoles, "implied role name(s) cannot be empty"); - parentBuilder.roleBranches.put(role, - Stream.of(impliedRoles).map(parentBuilder.rolePrefix::concat).toList()); - return parentBuilder; + return Builder.this.addHierarchy(this.role, impliedRoles); } } diff --git a/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java b/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java index 4e03b40d0f..977781231a 100644 --- a/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java +++ b/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyImplTests.java @@ -208,9 +208,15 @@ public class RoleHierarchyImplTests { @Test public void testBuilderWithDefaultRolePrefix() { - RoleHierarchyImpl roleHierarchyImpl = RoleHierarchyImpl.withDefaultRolePrefix().role("A").implies("B").build(); + RoleHierarchyImpl roleHierarchyImpl = RoleHierarchyImpl.withDefaultRolePrefix() + .role("A") + .implies("B") + .role("B") + .implies("C", "D") + .build(); List flatAuthorities = AuthorityUtils.createAuthorityList("ROLE_A"); - List allAuthorities = AuthorityUtils.createAuthorityList("ROLE_A", "ROLE_B"); + List allAuthorities = AuthorityUtils.createAuthorityList("ROLE_A", "ROLE_B", "ROLE_C", + "ROLE_D"); assertThat(roleHierarchyImpl).isNotNull(); assertThat(roleHierarchyImpl.getReachableGrantedAuthorities(flatAuthorities)) @@ -232,17 +238,6 @@ public class RoleHierarchyImplTests { .containsExactlyInAnyOrderElementsOf(allAuthorities); } - @Test - public void testBuilderWithNoRolePrefix() { - RoleHierarchyImpl roleHierarchyImpl = RoleHierarchyImpl.withNoRolePrefix().role("A").implies("B").build(); - List flatAuthorities = AuthorityUtils.createAuthorityList("A"); - List allAuthorities = AuthorityUtils.createAuthorityList("A", "B"); - - assertThat(roleHierarchyImpl).isNotNull(); - assertThat(roleHierarchyImpl.getReachableGrantedAuthorities(flatAuthorities)) - .containsExactlyInAnyOrderElementsOf(allAuthorities); - } - @Test public void testBuilderThrowIllegalArgumentExceptionWhenPrefixRoleNull() { assertThatIllegalArgumentException().isThrownBy(() -> RoleHierarchyImpl.withRolePrefix(null)); diff --git a/docs/modules/ROOT/pages/servlet/authorization/architecture.adoc b/docs/modules/ROOT/pages/servlet/authorization/architecture.adoc index d162ff5b81..edbcbc7e72 100644 --- a/docs/modules/ROOT/pages/servlet/authorization/architecture.adoc +++ b/docs/modules/ROOT/pages/servlet/authorization/architecture.adoc @@ -253,11 +253,10 @@ Java:: ---- @Bean static RoleHierarchy roleHierarchy() { - RoleHierarchyImpl hierarchy = new RoleHierarchyImpl(); - hierarchy.setHierarchy("ROLE_ADMIN > ROLE_STAFF\n" + - "ROLE_STAFF > ROLE_USER\n" + - "ROLE_USER > ROLE_GUEST"); - return hierarchy; + return RoleHierarchyImpl.withDefaultRolePrefix() + .role("ADMIN").implies("STAFF") + .role("STAFF").implies("USER") + .role("USER").implies("GUEST"); } // and, if using method security also add