You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2017/08/24 13:44:15 UTC

svn commit: r1806048 - in /sling/trunk/bundles/jcr/jackrabbit-accessmanager: ./ src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/ src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/ src/test/ src/test/java/ src/test/java/o...

Author: rombert
Date: Thu Aug 24 13:44:15 2017
New Revision: 1806048

URL: http://svn.apache.org/viewvc?rev=1806048&view=rev
Log:
SLING-3224 - GetAclTest integration test fails due to incorrect privilege expansion in AbstractGetAclServlet

Extracted a PrivilegesHelper from the AbstractGetAclServlet and added fix + tests.

Added:
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelperTest.java
Modified:
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/pom.xml
    sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractGetAclServlet.java

Modified: sling/trunk/bundles/jcr/jackrabbit-accessmanager/pom.xml
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/pom.xml?rev=1806048&r1=1806047&r2=1806048&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/pom.xml (original)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/pom.xml Thu Aug 24 13:44:15 2017
@@ -76,13 +76,13 @@
         <dependency>
             <groupId>org.apache.sling</groupId>
             <artifactId>org.apache.sling.api</artifactId>
-            <version>2.0.8</version>
+            <version>2.11.0</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
             <groupId>org.apache.jackrabbit</groupId>
             <artifactId>jackrabbit-api</artifactId>
-            <version>2.0.0</version>
+            <version>2.12.0</version>
             <scope>provided</scope>
         </dependency>
         <dependency>
@@ -112,5 +112,19 @@
             <version>1.0-alpha-1</version>
             <scope>provided</scope>
         </dependency>
+        <dependency>
+            <groupId>junit</groupId>
+            <artifactId>junit</artifactId>
+        </dependency>
+        <dependency>
+            <groupId>org.apache.sling</groupId>
+            <artifactId>org.apache.sling.testing.sling-mock-oak</artifactId>
+            <version>2.0.2</version>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>org.slf4j</groupId>
+            <artifactId>slf4j-simple</artifactId>
+        </dependency>
     </dependencies>
 </project>

Added: sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java?rev=1806048&view=auto
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java (added)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelper.java Thu Aug 24 13:44:15 2017
@@ -0,0 +1,181 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+package org.apache.sling.jcr.jackrabbit.accessmanager.impl;
+
+import java.util.Arrays;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.Privilege;
+
+import org.apache.sling.jcr.base.util.AccessControlUtil;
+
+/**
+ * Contains utility methods related to handling privileges 
+ *
+ */
+public abstract class PrivilegesHelper {
+    
+    /**
+     * Builds a map of aggregate privileges to privileges they aggregate
+     * 
+     * @param jcrSession a JCR session
+     * @param resourcePath the path used to look up the supported privileges
+     * @return a map, never <code>null</code>
+     * 
+     * @throws RepositoryException error accessing the repository
+     */
+    public static Map<Privilege, Set<Privilege>> buildPrivilegeToAncestorMap(Session jcrSession, String resourcePath)
+            throws RepositoryException {
+        AccessControlManager accessControlManager = AccessControlUtil.getAccessControlManager(jcrSession);
+        Map<Privilege, Set<Privilege>> privilegeToAncestorMap = new HashMap<Privilege, Set<Privilege>>();
+        Privilege[] supportedPrivileges = accessControlManager.getSupportedPrivileges(resourcePath);
+        for (Privilege privilege : supportedPrivileges) {
+            if (privilege.isAggregate()) {
+                Privilege[] ap = privilege.getAggregatePrivileges();
+                for (Privilege privilege2 : ap) {
+                    Set<Privilege> set = privilegeToAncestorMap.get(privilege2);
+                    if (set == null) {
+                        set = new HashSet<Privilege>();
+                        privilegeToAncestorMap.put(privilege2, set);
+                    }
+                    set.add(privilege);
+                }
+            }
+        }
+        return privilegeToAncestorMap;
+    }
+    
+    /**
+     * Update the granted and denied privilege sets by merging the result of adding
+     * the supplied privilege.
+     * 
+     * @param privilege the privilege to merge
+     * @param privilegeToAncestorMap mapping created using {@linkplain #buildPrivilegeToAncestorMap(Session, String)}
+     * @param add the first set to which the <tt>privilege</tt> should be added if missing
+     * @param remove the second set from which the <tt>privilege</tt> should be removed if present
+     */
+    // visible for testing
+    public static void mergePrivilegeSets(Privilege privilege,
+            Map<Privilege, Set<Privilege>> privilegeToAncestorMap,
+            Set<Privilege> add, Set<Privilege> remove) {
+        //1. remove duplicates and invalid privileges from the list
+        if (privilege.isAggregate()) {
+            Privilege[] aggregatePrivileges = privilege.getAggregatePrivileges();
+            //remove duplicates from the granted set
+            List<Privilege> asList = Arrays.asList(aggregatePrivileges);
+            add.removeAll(asList);
+            
+            // tentatively check for aggregate in the 'remov' set
+            for ( Privilege removeCandidate : remove ) {
+                if ( removeCandidate.isAggregate() && containsAny(removeCandidate.getAggregatePrivileges(), aggregatePrivileges) ) {
+                    remove.remove(removeCandidate);
+                    remove.addAll(Arrays.asList(removeCandidate.getAggregatePrivileges()));
+                    remove.removeAll(Arrays.asList(aggregatePrivileges));
+                }
+            }
+            
+
+            //remove from the denied set
+            remove.removeAll(asList);
+        }
+        remove.remove(privilege);
+
+        //2. check if the privilege is already contained in another granted privilege
+        boolean isAlreadyGranted = false;
+        Set<Privilege> ancestorSet = privilegeToAncestorMap.get(privilege);
+        if (ancestorSet != null) {
+            for (Privilege privilege2 : ancestorSet) {
+                if (add.contains(privilege2)) {
+                    isAlreadyGranted = true;
+                    break;
+                }
+            }
+        }
+
+        //3. add the privilege
+        if (!isAlreadyGranted) {
+            add.add(privilege);
+        }
+
+        //4. Deal with expanding existing aggregate privileges to remove the invalid
+        //  items and add the valid ones.
+        Set<Privilege> filterSet = privilegeToAncestorMap.get(privilege);
+        if (filterSet != null) {
+            //re-pack the denied set to compensate
+            for (Privilege privilege2 : filterSet) {
+                if (remove.contains(privilege2)) {
+                    remove.remove(privilege2);
+                    if (privilege2.isAggregate()) {
+                        filterAndMergePrivilegesFromAggregate(privilege2,
+                                add, remove, filterSet, privilege);
+                    }
+                }
+            }
+        }
+    }
+
+    private static boolean containsAny(Privilege[] aggregatePrivileges, Privilege[] aggregatePrivileges2) {
+        List<Privilege> privs = Arrays.asList(aggregatePrivileges);
+        for ( Privilege agg2 : aggregatePrivileges2) {
+            if ( privs.contains(agg2))
+                return true;
+        }
+        
+        return false;
+    }
+
+    /**
+     * Add all the declared aggregate privileges from the supplied privilege to the secondSet
+     * unless the privilege is already in the firstSet and not contained in the supplied filterSet.
+     */
+    private static void filterAndMergePrivilegesFromAggregate(Privilege privilege, Set<Privilege> firstSet,
+            Set<Privilege> secondSet, Set<Privilege> filterSet, Privilege ignorePrivilege) {
+        Privilege[] declaredAggregatePrivileges = privilege.getDeclaredAggregatePrivileges();
+        for (Privilege privilege3 : declaredAggregatePrivileges) {
+            if (ignorePrivilege.equals(privilege3)) {
+                continue; //skip it.
+            }
+            if (!firstSet.contains(privilege3) && !filterSet.contains(privilege3)) {
+                secondSet.add(privilege3);
+            }
+            if (privilege3.isAggregate()) {
+                Privilege[] declaredAggregatePrivileges2 = privilege3.getDeclaredAggregatePrivileges();
+                for (Privilege privilege2 : declaredAggregatePrivileges2) {
+                    if (!ignorePrivilege.equals(privilege2)) {
+                        if (privilege2.isAggregate()) {
+                            filterAndMergePrivilegesFromAggregate(privilege2,
+                                    firstSet, secondSet, filterSet, ignorePrivilege);
+                        }
+                    }
+                }
+            }
+        }
+    }    
+
+    private PrivilegesHelper() {
+        
+    }
+}

Modified: sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractGetAclServlet.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractGetAclServlet.java?rev=1806048&r1=1806047&r2=1806048&view=diff
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractGetAclServlet.java (original)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/main/java/org/apache/sling/jcr/jackrabbit/accessmanager/post/AbstractGetAclServlet.java Thu Aug 24 13:44:15 2017
@@ -31,8 +31,10 @@ import java.util.Set;
 
 import javax.jcr.AccessDeniedException;
 import javax.jcr.Item;
+import javax.jcr.PathNotFoundException;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.UnsupportedRepositoryOperationException;
 import javax.jcr.security.AccessControlEntry;
 import javax.jcr.security.AccessControlManager;
 import javax.jcr.security.Privilege;
@@ -50,6 +52,7 @@ import org.apache.sling.api.SlingHttpSer
 import org.apache.sling.api.resource.ResourceNotFoundException;
 import org.apache.sling.api.servlets.SlingAllMethodsServlet;
 import org.apache.sling.jcr.base.util.AccessControlUtil;
+import org.apache.sling.jcr.jackrabbit.accessmanager.impl.PrivilegesHelper;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -119,22 +122,7 @@ public abstract class AbstractGetAclServ
 
 		// Calculate a map of privileges to all the aggregate privileges it is contained in.
 		// Use for fast lookup during the mergePrivilegeSets calls below.
-        AccessControlManager accessControlManager = AccessControlUtil.getAccessControlManager(jcrSession);
-		Map<Privilege, Set<Privilege>> privilegeToAncestorMap = new HashMap<Privilege, Set<Privilege>>();
-        Privilege[] supportedPrivileges = accessControlManager.getSupportedPrivileges(item.getPath());
-        for (Privilege privilege : supportedPrivileges) {
-			if (privilege.isAggregate()) {
-				Privilege[] ap = privilege.getAggregatePrivileges();
-				for (Privilege privilege2 : ap) {
-					Set<Privilege> set = privilegeToAncestorMap.get(privilege2);
-					if (set == null) {
-						set = new HashSet<Privilege>();
-						privilegeToAncestorMap.put(privilege2, set);
-					}
-					set.add(privilege);
-				}
-			}
-		}
+        Map<Privilege, Set<Privilege>> privilegeToAncestorMap = PrivilegesHelper.buildPrivilegeToAncestorMap(jcrSession, resourcePath);
 
         AccessControlEntry[] declaredAccessControlEntries = getAccessControlEntries(jcrSession, resourcePath);
         Map<String, Map<String, Object>> aclMap = new LinkedHashMap<String, Map<String,Object>>();
@@ -171,14 +159,14 @@ public abstract class AbstractGetAclServ
             if (allow) {
                 Privilege[] privileges = ace.getPrivileges();
                 for (Privilege privilege : privileges) {
-                	mergePrivilegeSets(privilege,
+                	PrivilegesHelper.mergePrivilegeSets(privilege,
                 			privilegeToAncestorMap,
 							grantedSet, deniedSet);
                 }
             } else {
                 Privilege[] privileges = ace.getPrivileges();
                 for (Privilege privilege : privileges) {
-                	mergePrivilegeSets(privilege,
+                    PrivilegesHelper.mergePrivilegeSets(privilege,
                 			privilegeToAncestorMap,
 							deniedSet, grantedSet);
                 }
@@ -259,87 +247,6 @@ public abstract class AbstractGetAclServ
         return builder;
     }
 
-	/**
-	 * Update the granted and denied privilege sets by merging the result of adding
-	 * the supplied privilege.
-	 */
-	private void mergePrivilegeSets(Privilege privilege,
-			Map<Privilege, Set<Privilege>> privilegeToAncestorMap,
-			Set<Privilege> firstSet, Set<Privilege> secondSet) {
-		//1. remove duplicates and invalid privileges from the list
-		if (privilege.isAggregate()) {
-			Privilege[] aggregatePrivileges = privilege.getAggregatePrivileges();
-			//remove duplicates from the granted set
-			List<Privilege> asList = Arrays.asList(aggregatePrivileges);
-			firstSet.removeAll(asList);
-
-			//remove from the denied set
-			secondSet.removeAll(asList);
-		}
-		secondSet.remove(privilege);
-
-		//2. check if the privilege is already contained in another granted privilege
-		boolean isAlreadyGranted = false;
-		Set<Privilege> ancestorSet = privilegeToAncestorMap.get(privilege);
-		if (ancestorSet != null) {
-			for (Privilege privilege2 : ancestorSet) {
-				if (firstSet.contains(privilege2)) {
-					isAlreadyGranted = true;
-					break;
-				}
-			}
-		}
-
-		//3. add the privilege
-		if (!isAlreadyGranted) {
-		    firstSet.add(privilege);
-		}
-
-		//4. Deal with expanding existing aggregate privileges to remove the invalid
-		//  items and add the valid ones.
-		Set<Privilege> filterSet = privilegeToAncestorMap.get(privilege);
-		if (filterSet != null) {
-	    	//re-pack the denied set to compensate
-	    	for (Privilege privilege2 : filterSet) {
-	    		if (secondSet.contains(privilege2)) {
-	    			secondSet.remove(privilege2);
-	    			if (privilege2.isAggregate()) {
-		    			filterAndMergePrivilegesFromAggregate(privilege2,
-		    					firstSet, secondSet, filterSet, privilege);
-	    			}
-	    		}
-			}
-		}
-	}
-
-	/**
-	 * Add all the declared aggregate privileges from the supplied privilege to the secondSet
-	 * unless the privilege is already in the firstSet and not contained in the supplied filterSet.
-	 */
-	private void filterAndMergePrivilegesFromAggregate(Privilege privilege, Set<Privilege> firstSet,
-			Set<Privilege> secondSet, Set<Privilege> filterSet, Privilege ignorePrivilege) {
-		Privilege[] declaredAggregatePrivileges = privilege.getDeclaredAggregatePrivileges();
-		for (Privilege privilege3 : declaredAggregatePrivileges) {
-			if (ignorePrivilege.equals(privilege3)) {
-				continue; //skip it.
-			}
-			if (!firstSet.contains(privilege3) && !filterSet.contains(privilege3)) {
-				secondSet.add(privilege3);
-			}
-			if (privilege3.isAggregate()) {
-				Privilege[] declaredAggregatePrivileges2 = privilege3.getDeclaredAggregatePrivileges();
-				for (Privilege privilege2 : declaredAggregatePrivileges2) {
-					if (!ignorePrivilege.equals(privilege2)) {
-						if (privilege2.isAggregate()) {
-							filterAndMergePrivilegesFromAggregate(privilege2,
-									firstSet, secondSet, filterSet, ignorePrivilege);
-						}
-					}
-				}
-			}
-		}
-	}
-
     protected abstract AccessControlEntry[] getAccessControlEntries(Session session, String absPath) throws RepositoryException;
 
 }

Added: sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelperTest.java
URL: http://svn.apache.org/viewvc/sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelperTest.java?rev=1806048&view=auto
==============================================================================
--- sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelperTest.java (added)
+++ sling/trunk/bundles/jcr/jackrabbit-accessmanager/src/test/java/org/apache/sling/jcr/jackrabbit/accessmanager/impl/PrivilegesHelperTest.java Thu Aug 24 13:44:15 2017
@@ -0,0 +1,209 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with this
+ * work for additional information regarding copyright ownership. The ASF
+ * licenses this file to You under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+ * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+ * License for the specific language governing permissions and limitations under
+ * the License.
+ */
+package org.apache.sling.jcr.jackrabbit.accessmanager.impl;
+
+import static org.hamcrest.CoreMatchers.equalTo;
+import static org.hamcrest.CoreMatchers.hasItem;
+import static org.hamcrest.CoreMatchers.hasItems;
+import static org.hamcrest.CoreMatchers.not;
+import static org.junit.Assert.assertThat;
+import static org.junit.Assert.assertTrue;
+
+import java.util.HashSet;
+import java.util.Map;
+import java.util.Set;
+
+import javax.jcr.RepositoryException;
+import javax.jcr.Session;
+import javax.jcr.security.AccessControlManager;
+import javax.jcr.security.Privilege;
+
+import org.apache.sling.jcr.base.util.AccessControlUtil;
+import org.apache.sling.jcr.jackrabbit.accessmanager.impl.PrivilegesHelper;
+import org.apache.sling.testing.mock.sling.ResourceResolverType;
+import org.apache.sling.testing.mock.sling.junit.SlingContext;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+
+public class PrivilegesHelperTest {
+
+    @Rule
+    public final SlingContext context = new SlingContext(ResourceResolverType.JCR_OAK);
+    
+    private Map<Privilege, Set<Privilege>> privilegeToAncestorMap;
+    private Session session;
+    private AccessControlManager acm;
+
+    @Before
+    public void buildPrivilegesMap() throws Exception {
+        
+        session = context.resourceResolver().adaptTo(Session.class);
+        privilegeToAncestorMap = PrivilegesHelper.buildPrivilegeToAncestorMap(session, "/");
+        acm = AccessControlUtil.getAccessControlManager(session);
+    }
+    
+    @Test
+    public void mergeAddsMissingPrivilege() throws Exception {
+        
+        Privilege write = priv(Privilege.JCR_WRITE);
+        Privilege read = priv(Privilege.JCR_READ);
+        
+        Set<Privilege> allowed = new HashSet<>();
+        allowed.add(read);
+        Set<Privilege> denied = new HashSet<>();
+
+        PrivilegesHelper.mergePrivilegeSets(write, privilegeToAncestorMap, allowed, denied);
+
+        assertThat(allowed, hasItems(read, write));
+        assertThat(allowed.size(), equalTo(2));
+        
+        assertThat(denied.size(), equalTo(0));
+    }
+
+
+    private Privilege priv(String privilegeName) throws RepositoryException {
+        return acm.privilegeFromName(privilegeName);
+    }
+
+    @Test
+    public void mergeRemovesExistingDeniedPrivilege() throws Exception {
+        
+        Privilege write = priv(Privilege.JCR_WRITE);
+        
+        Set<Privilege> allowed = new HashSet<>();
+        Set<Privilege> denied = new HashSet<>();
+        denied.add(write);
+
+        PrivilegesHelper.mergePrivilegeSets(write, privilegeToAncestorMap, allowed, denied);
+
+        assertThat(allowed, hasItem(write));
+        assertThat(allowed.size(), equalTo(1));
+        
+        assertThat(denied.size(), equalTo(0));
+    }    
+    
+    @Test
+    public void mergeAggregateOverlappingPrivilegesOnBothSides() throws Exception {
+
+        Privilege all = priv(Privilege.JCR_ALL);
+        Privilege write = priv(Privilege.JCR_WRITE);
+        
+        Set<Privilege> allowed = new HashSet<>();
+        Set<Privilege> denied = new HashSet<>();
+        denied.add(all);
+
+        PrivilegesHelper.mergePrivilegeSets(write, privilegeToAncestorMap, allowed, denied);
+
+        assertThat(allowed, hasItem(write));
+        assertThat(allowed.size(), equalTo(1));
+        
+        assertThat(denied, hasItem(priv(Privilege.JCR_READ)));
+        assertThat(denied, not(hasItem(priv(Privilege.JCR_MODIFY_PROPERTIES))));
+    }
+    
+    @Test
+    public void mergeAggregateNonOverlappingPrivilegesOnBothSides() throws Exception {
+        
+        Privilege read = priv(Privilege.JCR_READ);
+        Privilege write = priv(Privilege.JCR_WRITE);
+        
+        assertTrue(read.isAggregate());
+        assertTrue(write.isAggregate());
+        
+        Set<Privilege> allowed = new HashSet<>();
+        Set<Privilege> denied = new HashSet<>();
+        denied.add(write);
+        
+        PrivilegesHelper.mergePrivilegeSets(read, privilegeToAncestorMap, allowed, denied);
+        
+        assertThat(allowed, hasItem(read));
+        assertThat(allowed.size(), equalTo(1));
+        
+        assertThat(denied, hasItem(write));
+        assertThat(denied.size(), equalTo(1));
+    }
+    
+    /**
+     * Validates that two identical privileges are merged
+     */
+    @Test
+    public void mergeIdenticalPrivileges() throws Exception {
+        
+        Privilege read = priv(Privilege.JCR_READ);
+        
+        Set<Privilege> allowed = new HashSet<>();
+        allowed.add(read);
+        
+        Set<Privilege> denied = new HashSet<>();
+        
+        PrivilegesHelper.mergePrivilegeSets(read, privilegeToAncestorMap, allowed, denied);
+        
+        assertThat(allowed, hasItem(read));
+        assertThat(allowed.size(), equalTo(1));
+        
+        assertThat(denied.size(), equalTo(0));
+    }
+    
+    /**
+     * 
+     * Validates that the <tt>jcr:modifyProperties</tt> is recognized as being aggregated into <tt>jcr:write</tt>
+     */
+    @Test
+    public void mergeAggregatePrivileges() throws Exception {
+        
+        Privilege write = priv(Privilege.JCR_WRITE);
+        Privilege modifyProps = priv(Privilege.JCR_MODIFY_PROPERTIES);
+        
+        Set<Privilege> allowed = new HashSet<>();
+        allowed.add(write);
+        
+        Set<Privilege> denied = new HashSet<>();
+        
+        PrivilegesHelper.mergePrivilegeSets(modifyProps, privilegeToAncestorMap, allowed, denied);
+        
+        assertThat(allowed, hasItem(write));
+        assertThat(allowed.size(), equalTo(1));
+        
+        assertThat(denied.size(), equalTo(0));
+    }
+    
+    /**
+     * Validates that when negating <tt>jcr:modifyProperties</tt> out of <tt>jcr:write</tt> the correct individual
+     * privileges are reported
+     */
+    @Test
+    public void mergeRemoveAggregatePrivileges() throws Exception {
+        
+        Privilege write = priv(Privilege.JCR_WRITE);
+        Privilege modifyProps = priv(Privilege.JCR_MODIFY_PROPERTIES);
+        
+        Set<Privilege> denied = new HashSet<>();
+        
+        Set<Privilege> second = new HashSet<>();
+        second.add(write);
+        
+        PrivilegesHelper.mergePrivilegeSets(modifyProps, privilegeToAncestorMap, denied, second);
+        
+        assertThat(denied, hasItem(modifyProps));
+        assertThat(denied.size(), equalTo(1));
+        
+        assertThat(second, hasItems(priv(Privilege.JCR_ADD_CHILD_NODES), priv(Privilege.JCR_REMOVE_CHILD_NODES), priv(Privilege.JCR_REMOVE_NODE)));
+        assertThat(second.size(), equalTo(3));
+    }
+
+}