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 ff28223311a..c5a3d47a501 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 @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -87,7 +87,7 @@ public boolean isGranted(Acl acl, List permission, List sids, for (AccessControlEntry ace : aces) { - if ((ace.getPermission().getMask() == p.getMask()) + if (isGranted(ace, p) && ace.getSid().equals(sid)) { // Found a matching ACE, so its authorization decision will // prevail @@ -142,4 +142,25 @@ public boolean isGranted(Acl acl, List permission, List sids, } } + /** + * Compares an ACE Permission to the given Permission. + * By default, we compare the Permission masks for exact match. + * Subclasses of this strategy can override this behavior and implement + * more sophisticated comparisons, e.g. a bitwise comparison for ACEs that grant access. + *
{@code
+	 * if (ace.isGranting() && p.getMask() != 0) {
+	 *    return (ace.getPermission().getMask() & p.getMask()) != 0;
+	 * } else {
+	 *    return ace.getPermission().getMask() == p.getMask();
+	 * }
+	 * }
+ * + * @param ace the ACE from the Acl holding the mask. + * @param p the Permission we are checking against. + * @return true, if the respective masks are considered to be equal. + */ + 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 50cbf3ad6ad..785c09078e0 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 void setUp() throws Exception { mockAuditLogger = mock(AuditLogger.class); pgs = new DefaultPermissionGrantingStrategy(mockAuditLogger); auth.setAuthenticated(true); + permissionFactory = new DefaultPermissionFactory(); } @After @@ -560,9 +562,39 @@ public void changingParentIsSuccessful() throws Exception { 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 {