diff --git a/core/src/main/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImpl.java b/core/src/main/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImpl.java index e159d3c4ce..f2e0e25e07 100755 --- a/core/src/main/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImpl.java +++ b/core/src/main/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImpl.java @@ -14,21 +14,16 @@ package org.acegisecurity.userdetails.hierarchicalroles; + import org.acegisecurity.GrantedAuthority; import org.acegisecurity.GrantedAuthorityImpl; - -import org.apache.commons.collections.CollectionUtils; import org.apache.commons.logging.Log; import org.apache.commons.logging.LogFactory; -import java.util.HashMap; -import java.util.HashSet; -import java.util.Iterator; -import java.util.Map; -import java.util.Set; -import java.util.Arrays; -import java.util.regex.Pattern; import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import java.util.*; /** *

@@ -183,8 +178,8 @@ public class RoleHierarchyImpl implements RoleHierarchy { if (rolesReachableInOneStepMap.containsKey(aRole)) { Set newReachableRoles = (Set) rolesReachableInOneStepMap.get(aRole); - if (CollectionUtils.containsAny(rolesToVisitSet, newReachableRoles) - || CollectionUtils.containsAny(visitedRolesSet, newReachableRoles)) { + // definition of a cycle: you can reach the role you are starting from + if (rolesToVisitSet.contains(role) || visitedRolesSet.contains(role)) { throw new CycleInRoleHierarchyException(); } else { // no cycle @@ -200,4 +195,4 @@ public class RoleHierarchyImpl implements RoleHierarchy { } -} \ No newline at end of file +} diff --git a/core/src/test/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImplTests.java b/core/src/test/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImplTests.java index 59c8c10594..d32f65b489 100755 --- a/core/src/test/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImplTests.java +++ b/core/src/test/java/org/acegisecurity/userdetails/hierarchicalroles/RoleHierarchyImplTests.java @@ -34,10 +34,6 @@ public class RoleHierarchyImplTests extends TestCase { super(testCaseName); } - public static void main(String[] args) { - TestRunner.run(RoleHierarchyImplTests.class); - } - public void testSimpleRoleHierarchy() { GrantedAuthority[] authorities0 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_0") }; GrantedAuthority[] authorities1 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_A") }; @@ -66,6 +62,26 @@ public class RoleHierarchyImplTests extends TestCase { assertTrue(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(roleHierarchyImpl.getReachableGrantedAuthorities(authorities1), authorities3)); } + public void testComplexRoleHierarchy() { + GrantedAuthority[] authoritiesInput1 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_A") }; + GrantedAuthority[] authoritiesOutput1 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_A"), new GrantedAuthorityImpl("ROLE_B"), new GrantedAuthorityImpl("ROLE_C"), + new GrantedAuthorityImpl("ROLE_D") }; + GrantedAuthority[] authoritiesInput2 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_B") }; + GrantedAuthority[] authoritiesOutput2 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_B"), new GrantedAuthorityImpl("ROLE_D") }; + GrantedAuthority[] authoritiesInput3 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_C") }; + GrantedAuthority[] authoritiesOutput3 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_C"), new GrantedAuthorityImpl("ROLE_D") }; + GrantedAuthority[] authoritiesInput4 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_D") }; + GrantedAuthority[] authoritiesOutput4 = new GrantedAuthority[] { new GrantedAuthorityImpl("ROLE_D") }; + + RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl(); + roleHierarchyImpl.setHierarchy("ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_C > ROLE_D\nROLE_B > ROLE_D"); + + assertTrue(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(roleHierarchyImpl.getReachableGrantedAuthorities(authoritiesInput1), authoritiesOutput1)); + assertTrue(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(roleHierarchyImpl.getReachableGrantedAuthorities(authoritiesInput2), authoritiesOutput2)); + assertTrue(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(roleHierarchyImpl.getReachableGrantedAuthorities(authoritiesInput3), authoritiesOutput3)); + assertTrue(HierarchicalRolesTestHelper.containTheSameGrantedAuthorities(roleHierarchyImpl.getReachableGrantedAuthorities(authoritiesInput4), authoritiesOutput4)); + } + public void testCyclesInRoleHierarchy() { RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl(); @@ -83,6 +99,21 @@ public class RoleHierarchyImplTests extends TestCase { roleHierarchyImpl.setHierarchy("ROLE_A > ROLE_B\nROLE_B > ROLE_C\nROLE_C > ROLE_A"); fail("Cycle in role hierarchy was not detected!"); } catch (CycleInRoleHierarchyException e) {} + + try { + roleHierarchyImpl.setHierarchy("ROLE_A > ROLE_B\nROLE_B > ROLE_C\nROLE_C > ROLE_E\nROLE_E > ROLE_D\nROLE_D > ROLE_B"); + fail("Cycle in role hierarchy was not detected!"); + } catch (CycleInRoleHierarchyException e) {} + } + + public void testNoCyclesInRoleHierarchy() { + RoleHierarchyImpl roleHierarchyImpl = new RoleHierarchyImpl(); + + try { + roleHierarchyImpl.setHierarchy("ROLE_A > ROLE_B\nROLE_A > ROLE_C\nROLE_C > ROLE_D\nROLE_B > ROLE_D"); + } catch (CycleInRoleHierarchyException e) { + fail("A cycle in role hierarchy was incorrectly detected!"); + } } } \ No newline at end of file