diff --git a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java index fada8bf9649..4835ac0f48f 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/AclImpl.java @@ -247,7 +247,11 @@ public void setOwner(Sid newOwner) { @Override public void setParent(@Nullable Acl newParent) { this.aclAuthorizationStrategy.securityCheck(this, AclAuthorizationStrategy.CHANGE_GENERAL); - Assert.isTrue(newParent == null || !newParent.equals(this), "Cannot be the parent of yourself"); + Acl parent = newParent; + while (parent != null) { + Assert.isTrue(parent != this, "Cannot create a circular parent relationship"); + parent = parent.getParentAcl(); + } this.parentAcl = newParent; } diff --git a/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java b/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java index 2baace5b344..e02db533eba 100644 --- a/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java +++ b/acl/src/test/java/org/springframework/security/acls/domain/AclImplTests.java @@ -391,6 +391,33 @@ public void gettersAndSettersAreConsistent() { assertThat(new PrincipalSid("ben")).isEqualTo(acl.getOwner()); } + @Test + public void setParentRejectsAncestor() { + MutableAcl parentAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 101), 101, + this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe")); + MutableAcl childAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 102), 102, + this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe")); + + childAcl.setParent(parentAcl); + + assertThatIllegalArgumentException().isThrownBy(() -> parentAcl.setParent(childAcl)); + } + + @Test + public void setParentRejectsIndirectAncestor() { + MutableAcl grandParentAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 101), 101, + this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe")); + MutableAcl parentAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 102), 102, + this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe")); + MutableAcl childAcl = new AclImpl(new ObjectIdentityImpl(TARGET_CLASS, 103), 103, + this.authzStrategy, this.pgs, null, null, true, new PrincipalSid("joe")); + + parentAcl.setParent(grandParentAcl); + childAcl.setParent(parentAcl); + + assertThatIllegalArgumentException().isThrownBy(() -> grandParentAcl.setParent(childAcl)); + } + @Test public void isSidLoadedBehavesAsExpected() { List loadedSids = Arrays.asList(new PrincipalSid("ben"), new GrantedAuthoritySid("ROLE_IGNORED"));