diff --git a/acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionGrantingStrategy.java b/acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionGrantingStrategy.java index a6f9d2bf5e..3f54678873 100644 --- a/acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionGrantingStrategy.java +++ b/acl/src/main/java/org/springframework/security/acls/domain/DefaultPermissionGrantingStrategy.java @@ -87,7 +87,7 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra for (AccessControlEntry ace : aces) { - if (comparePermissionMasks(ace, p) + if (isGranted(ace, p) && ace.getSid().equals(sid)) { // Found a matching ACE, so its authorization decision will // prevail @@ -159,7 +159,7 @@ public class DefaultPermissionGrantingStrategy implements PermissionGrantingStra * @param p the Permission we are checking against. * @return true, if the respective masks are considered to be equal. */ - protected boolean comparePermissionMasks(AccessControlEntry ace, Permission p) { + protected boolean isGranted(AccessControlEntry ace, Permission p) { return ace.getPermission().getMask() == p.getMask(); } 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 c0d6b7a49b..140915a948 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 @@ -49,6 +49,7 @@ public class AclImplTests { PermissionGrantingStrategy pgs; AuditLogger mockAuditLogger; ObjectIdentity objectIdentity = new ObjectIdentityImpl(TARGET_CLASS, 100); + private DefaultPermissionFactory permissionFactory; // ~ Methods // ======================================================================================================== @@ -60,6 +61,7 @@ public class AclImplTests { mockAuditLogger = mock(AuditLogger.class); pgs = new DefaultPermissionGrantingStrategy(mockAuditLogger); auth.setAuthenticated(true); + permissionFactory = new DefaultPermissionFactory(); } @After @@ -559,9 +561,39 @@ public class AclImplTests { childAcl.setParent(changeParentAcl); } + // SEC-2342 + @Test + public void maskPermissionGrantingStrategy() { + DefaultPermissionGrantingStrategy maskPgs = new MaskPermissionGrantingStrategy(mockAuditLogger); + MockAclService service = new MockAclService(); + AclImpl acl = new AclImpl(objectIdentity, 1, authzStrategy, maskPgs, null, null, + true, new PrincipalSid("joe")); + Permission permission = permissionFactory.buildFromMask(BasePermission.READ.getMask() | BasePermission.WRITE.getMask()); + Sid sid = new PrincipalSid("ben"); + acl.insertAce(0, permission, sid, true); + service.updateAcl(acl); + List permissions = Arrays.asList(BasePermission.READ); + List sids = Arrays.asList(sid); + assertThat(acl.isGranted(permissions, sids, false)).isTrue(); + } + // ~ Inner Classes // ================================================================================================== + private static class MaskPermissionGrantingStrategy extends DefaultPermissionGrantingStrategy { + public MaskPermissionGrantingStrategy(AuditLogger auditLogger) { + super(auditLogger); + } + + @Override + protected boolean isGranted(AccessControlEntry ace, Permission p) { + if (p.getMask() != 0) { + return (p.getMask() & ace.getPermission().getMask()) != 0; + } + return super.isGranted(ace, p); + } + } + private class MockAclService implements MutableAclService { public MutableAcl createAcl(ObjectIdentity objectIdentity) throws AlreadyExistsException {