From c75a5b727945e2e97a558243e0175196175ed723 Mon Sep 17 00:00:00 2001 From: Joe Grandja Date: Mon, 19 Sep 2016 13:30:10 -0400 Subject: [PATCH] Polish RoleHierarchyUtils and add tests --- .../hierarchicalroles/RoleHierarchyUtils.java | 87 ++---- .../RoleHierarchyUtilsTests.java | 253 +++--------------- 2 files changed, 66 insertions(+), 274 deletions(-) diff --git a/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java b/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java index ff5ce22e5c..0f716c07a8 100644 --- a/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java +++ b/core/src/main/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtils.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -15,84 +15,55 @@ */ package org.springframework.security.access.hierarchicalroles; +import org.springframework.util.Assert; + import java.io.PrintWriter; import java.io.StringWriter; import java.util.List; import java.util.Map; /** - * Utility method for working with {@link RoleHierarchy}. + * Utility methods for {@link RoleHierarchy}. * * @author Thomas Darimont - * @since + * @since 4.2.0 */ -public class RoleHierarchyUtils { +public final class RoleHierarchyUtils { + + private RoleHierarchyUtils() { + } /** - * Builds a {@link RoleHierarchy} representation from the given {@link Map} of role name to implied roles. - * The map key is the role name and the map value is a {@link List} of implied role names. + * Converts the supplied {@link Map} of role name to implied role name(s) to a string + * representation understood by {@link RoleHierarchyImpl#setHierarchy(String)}. + * The map key is the role name and the map value is a {@link List} of implied role name(s). * - *

- * Here is an example configuration of a role hierarchy configured via yaml. - * wich follows the pattern: - * {@code ROLE_NAME: List of implied role names} - *

- * 
-	 * 
+	 * @param roleHierarchyMap the mapping(s) of role name to implied role name(s)
+	 * @return a string representation of a role hierarchy
+	 * @throws IllegalArgumentException if roleHierarchyMap is null or empty or if a role name is null or
+	 * empty or if an implied role name(s) is null or empty
 	 *
-	 * security:
-	 *   roles:
-	 *     hierarchy:
-	 *       ROLE_ALL: ROLE_A, ROLE_C
-	 *       ROLE_A: ROLE_B
-	 *       ROLE_B: ROLE_AUTHENTICATED
-	 *       ROLE_C: ROLE_AUTHENTICATED
-	 *       ROLE_AUTHENTICATED: ROLE_UNAUTHENTICATED
-	 * 
-	 *
- *

This yaml configuration could then be mapped by the following {@literal ConfigurationProperties}

- * 
-	 * 
-	 *   {@literal @}ConfigurationProperties("security.roles")
-	 *   class SecurityPropertiesExtension {
-	 *     Map> hierarchy = new LinkedHashMap<>();
-	 *
-	 *     //getter | setter
-	 *   }
-	 * 
-	 *
- *

To define the role hierarchy just declare a {@link org.springframework.context.annotation.Bean} of - * type {@link RoleHierarchy} as follows:

- * 
-	 * 
-	 *   {@literal @}Bean
-	 *   RoleHierarchy roleHierarchy(SecurityPropertiesExtension spe) {
-	 *     return RoleHierarchyUtils.roleHierarchyFromMap(spe.getHierarchy());
-	 *   }
-	 * 
-	 *
- * - * @param roleHierarchyMapping the role name to implied role names mapping - * @return */ - public static RoleHierarchy roleHierarchyFromMap(Map> roleHierarchyMapping) { + public static String roleHierarchyFromMap(Map> roleHierarchyMap) { + Assert.notEmpty(roleHierarchyMap, "roleHierarchyMap cannot be empty"); - StringWriter roleHierachyDescriptionBuffer = new StringWriter(); - PrintWriter roleHierarchyDescriptionWriter = new PrintWriter(roleHierachyDescriptionBuffer); + StringWriter roleHierarchyBuffer = new StringWriter(); + PrintWriter roleHierarchyWriter = new PrintWriter(roleHierarchyBuffer); - for (Map.Entry> entry : roleHierarchyMapping.entrySet()) { + for (Map.Entry> roleHierarchyEntry : roleHierarchyMap.entrySet()) { + String role = roleHierarchyEntry.getKey(); + List impliedRoles = roleHierarchyEntry.getValue(); - String currentRole = entry.getKey(); - List impliedRoles = entry.getValue(); + Assert.hasLength(role, "role name must be supplied"); + Assert.notEmpty(impliedRoles, "implied role name(s) cannot be empty"); for (String impliedRole : impliedRoles) { - String roleMapping = currentRole + " > " + impliedRole; - roleHierarchyDescriptionWriter.println(roleMapping); + String roleMapping = role + " > " + impliedRole; + roleHierarchyWriter.println(roleMapping); } } - RoleHierarchyImpl roleHierarchy = new RoleHierarchyImpl(); - roleHierarchy.setHierarchy(roleHierachyDescriptionBuffer.toString()); - return roleHierarchy; + return roleHierarchyBuffer.toString(); } + } diff --git a/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtilsTests.java b/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtilsTests.java index b90e58f03b..0f369a0db4 100644 --- a/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtilsTests.java +++ b/core/src/test/java/org/springframework/security/access/hierarchicalroles/RoleHierarchyUtilsTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2012-2016 the original author or authors. * * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. @@ -16,251 +16,72 @@ package org.springframework.security.access.hierarchicalroles; import org.junit.Test; -import org.springframework.security.core.GrantedAuthority; -import org.springframework.security.core.authority.AuthorityUtils; -import java.util.ArrayList; -import java.util.HashMap; -import java.util.List; +import java.util.*; import static java.util.Arrays.asList; -import static java.util.Collections.singletonList; -import static java.util.Collections.singletonMap; import static org.assertj.core.api.Assertions.assertThat; -import static org.assertj.core.api.Assertions.fail; -import static org.springframework.security.access.hierarchicalroles.RoleHierarchyUtils.roleHierarchyFromMap; /** * Tests for {@link RoleHierarchyUtils}. * - * Copied from {@link RoleHierarchyImplTests} with adaptations for {@link RoleHierarchyUtils}. - * - * @author Thomas Darimont + * @author Joe Grandja */ public class RoleHierarchyUtilsTests { @Test - public void testRoleHierarchyWithNullOrEmptyAuthorities() { + public void roleHierarchyFromMapWhenMapValidThenConvertsCorrectly() throws Exception { + String expectedRoleHierarchy = "ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_B > ROLE_D\nROLE_C > ROLE_D\n"; - List authorities0 = null; - List authorities1 = new ArrayList(); + Map> roleHierarchyMap = new TreeMap>(); + roleHierarchyMap.put("ROLE_A", asList("ROLE_B", "ROLE_C")); + roleHierarchyMap.put("ROLE_B", asList("ROLE_D")); + roleHierarchyMap.put("ROLE_C", asList("ROLE_D")); - RoleHierarchy roleHierarchy = roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_B"))); + String roleHierarchy = RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap); - assertThat(roleHierarchy.getReachableGrantedAuthorities( - authorities0)).isNotNull(); - assertThat( - roleHierarchy.getReachableGrantedAuthorities(authorities0)).isEmpty(); - ; - assertThat(roleHierarchy.getReachableGrantedAuthorities( - authorities1)).isNotNull(); - assertThat( - roleHierarchy.getReachableGrantedAuthorities(authorities1)).isEmpty(); - ; + assertThat(roleHierarchy).isEqualTo(expectedRoleHierarchy); } - @Test - public void testSimpleRoleHierarchy() { - - List authorities0 = AuthorityUtils.createAuthorityList( - "ROLE_0"); - List authorities1 = AuthorityUtils.createAuthorityList( - "ROLE_A"); - List authorities2 = AuthorityUtils.createAuthorityList("ROLE_A", - "ROLE_B"); - - RoleHierarchy roleHierarchy = roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_B"))); - - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy.getReachableGrantedAuthorities(authorities0), - authorities0)).isTrue(); - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy.getReachableGrantedAuthorities(authorities1), - authorities2)).isTrue(); - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy.getReachableGrantedAuthorities(authorities2), - authorities2)).isTrue(); + @Test(expected = IllegalArgumentException.class) + public void roleHierarchyFromMapWhenMapNullThenThrowsIllegalArgumentException() throws Exception { + RoleHierarchyUtils.roleHierarchyFromMap(null); } - @Test - public void testTransitiveRoleHierarchies() { - List authorities1 = AuthorityUtils.createAuthorityList( - "ROLE_A"); - List authorities2 = AuthorityUtils.createAuthorityList("ROLE_A", - "ROLE_B", "ROLE_C"); - List authorities3 = AuthorityUtils.createAuthorityList("ROLE_A", - "ROLE_B", "ROLE_C", "ROLE_D"); - - RoleHierarchy roleHierarchy2Levels = roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B")); - put("ROLE_B", asList("ROLE_C")); - } - }); - - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy2Levels.getReachableGrantedAuthorities(authorities1), - authorities2)).isTrue(); - - RoleHierarchy roleHierarchy3Levels = roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B")); - put("ROLE_B", asList("ROLE_C")); - put("ROLE_C", asList("ROLE_D")); - } - }); - - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy3Levels.getReachableGrantedAuthorities(authorities1), - authorities3)).isTrue(); + @Test(expected = IllegalArgumentException.class) + public void roleHierarchyFromMapWhenMapEmptyThenThrowsIllegalArgumentException() throws Exception { + RoleHierarchyUtils.roleHierarchyFromMap(Collections.>emptyMap()); } - @Test - public void testComplexRoleHierarchy() { - List authoritiesInput1 = AuthorityUtils.createAuthorityList( - "ROLE_A"); - List authoritiesOutput1 = AuthorityUtils.createAuthorityList( - "ROLE_A", "ROLE_B", "ROLE_C", "ROLE_D"); - List authoritiesInput2 = AuthorityUtils.createAuthorityList( - "ROLE_B"); - List authoritiesOutput2 = AuthorityUtils.createAuthorityList( - "ROLE_B", "ROLE_D"); - List authoritiesInput3 = AuthorityUtils.createAuthorityList( - "ROLE_C"); - List authoritiesOutput3 = AuthorityUtils.createAuthorityList( - "ROLE_C", "ROLE_D"); - List authoritiesInput4 = AuthorityUtils.createAuthorityList( - "ROLE_D"); - List authoritiesOutput4 = AuthorityUtils.createAuthorityList( - "ROLE_D"); + @Test(expected = IllegalArgumentException.class) + public void roleHierarchyFromMapWhenRoleNullThenThrowsIllegalArgumentException() throws Exception { + Map> roleHierarchyMap = new HashMap>(); + roleHierarchyMap.put(null, asList("ROLE_B", "ROLE_C")); - - RoleHierarchy roleHierarchy3LevelsMultipleRoles = roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B","ROLE_C")); - put("ROLE_B", asList("ROLE_D")); - put("ROLE_C", asList("ROLE_D")); - } - }); - - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput1), - authoritiesOutput1)).isTrue(); - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput2), - authoritiesOutput2)).isTrue(); - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput3), - authoritiesOutput3)).isTrue(); - assertThat(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities( - roleHierarchy3LevelsMultipleRoles.getReachableGrantedAuthorities(authoritiesInput4), - authoritiesOutput4)).isTrue(); + RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap); } - @Test - public void testCyclesInRoleHierarchy() { + @Test(expected = IllegalArgumentException.class) + public void roleHierarchyFromMapWhenRoleEmptyThenThrowsIllegalArgumentException() throws Exception { + Map> roleHierarchyMap = new HashMap>(); + roleHierarchyMap.put("", asList("ROLE_B", "ROLE_C")); - try { - roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_A"))); - fail("Cycle in role hierarchy was not detected!"); - } - catch (CycleInRoleHierarchyException e) { - } - - try { - - roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B")); - put("ROLE_B", asList("ROLE_A")); - } - }); - - fail("Cycle in role hierarchy was not detected!"); - } - catch (CycleInRoleHierarchyException e) { - } - - try { - - roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B")); - put("ROLE_B", asList("ROLE_C")); - put("ROLE_C", asList("ROLE_A")); - } - }); - - fail("Cycle in role hierarchy was not detected!"); - } - catch (CycleInRoleHierarchyException e) { - } - - try { - - roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B")); - put("ROLE_B", asList("ROLE_C")); - put("ROLE_C", asList("ROLE_E")); - put("ROLE_E", asList("ROLE_D")); - put("ROLE_D", asList("ROLE_B")); - } - }); - - - fail("Cycle in role hierarchy was not detected!"); - } - catch (CycleInRoleHierarchyException e) { - } + RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap); } - @Test - public void testNoCyclesInRoleHierarchy() { - RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl(); + @Test(expected = IllegalArgumentException.class) + public void roleHierarchyFromMapWhenImpliedRolesNullThenThrowsIllegalArgumentException() throws Exception { + Map> roleHierarchyMap = new HashMap>(); + roleHierarchyMap.put("ROLE_A", null); - try { - roleHierarchyImpl.setHierarchy( - "ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_C > ROLE_D\nROLE_B > ROLE_D"); - - roleHierarchyFromMap(new HashMap>(){ - { - put("ROLE_A", asList("ROLE_B")); - put("ROLE_A", asList("ROLE_C")); - put("ROLE_C", asList("ROLE_D")); - put("ROLE_B", asList("ROLE_D")); - } - }); - - } - catch (CycleInRoleHierarchyException e) { - fail("A cycle in role hierarchy was incorrectly detected!"); - } + RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap); } - @Test - public void testSimpleRoleHierarchyWithCustomGrantedAuthorityImplementation() { + @Test(expected = IllegalArgumentException.class) + public void roleHierarchyFromMapWhenImpliedRolesEmptyThenThrowsIllegalArgumentException() throws Exception { + Map> roleHierarchyMap = new HashMap>(); + roleHierarchyMap.put("ROLE_A", Collections.emptyList()); - List authorities0 = HierarchicalRolesTestHelper.createAuthorityList( - "ROLE_0"); - List authorities1 = HierarchicalRolesTestHelper.createAuthorityList( - "ROLE_A"); - List authorities2 = HierarchicalRolesTestHelper.createAuthorityList( - "ROLE_A", "ROLE_B"); - - RoleHierarchy roleHierarchy = roleHierarchyFromMap(singletonMap("ROLE_A", singletonList("ROLE_B"))); - - assertThat( - HierarchicalRolesTestHelper.containTheSameGrantedAuthoritiesCompareByAuthorityString( - roleHierarchy.getReachableGrantedAuthorities(authorities0), - authorities0)).isTrue(); - assertThat( - HierarchicalRolesTestHelper.containTheSameGrantedAuthoritiesCompareByAuthorityString( - roleHierarchy.getReachableGrantedAuthorities(authorities1), - authorities2)).isTrue(); - assertThat( - HierarchicalRolesTestHelper.containTheSameGrantedAuthoritiesCompareByAuthorityString( - roleHierarchy.getReachableGrantedAuthorities(authorities2), - authorities2)).isTrue(); + RoleHierarchyUtils.roleHierarchyFromMap(roleHierarchyMap); } }