jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject svn commit: r1483814 - in /jackrabbit/oak/trunk/oak-core/src: main/java/org/apache/jackrabbit/oak/security/authorization/ACL.java test/java/org/apache/jackrabbit/oak/security/authorization/ACLTest.java
Date Fri, 17 May 2013 14:11:04 GMT
Author: angela
Date: Fri May 17 14:11:04 2013
New Revision: 1483814

URL: http://svn.apache.org/r1483814
Log:
OAK-814 : Update/Insertion of ACEs for Group principals

Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/ACL.java
    jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/ACLTest.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/ACL.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/ACL.java?rev=1483814&r1=1483813&r2=1483814&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/ACL.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/ACL.java
Fri May 17 14:11:04 2013
@@ -17,7 +17,6 @@
 package org.apache.jackrabbit.oak.security.authorization;
 
 import java.security.Principal;
-import java.security.acl.Group;
 import java.util.ArrayList;
 import java.util.Collections;
 import java.util.HashSet;
@@ -35,7 +34,6 @@ import javax.jcr.security.Privilege;
 import com.google.common.base.Predicate;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
-import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
 import org.apache.jackrabbit.api.security.authorization.PrivilegeManager;
 import org.apache.jackrabbit.api.security.principal.PrincipalManager;
 import org.apache.jackrabbit.oak.namepath.NamePathMapper;
@@ -190,13 +188,13 @@ abstract class ACL extends AbstractAcces
             }
         }));
 
-        for (JackrabbitAccessControlEntry ace : subList) {
-            ACE existing = (ACE) ace;
+        for (ACE existing : subList) {
             PrivilegeBits existingBits = getPrivilegeBits(existing);
             PrivilegeBits entryBits = getPrivilegeBits(entry);
             if (entry.getRestrictions().equals(existing.getRestrictions())) {
-                if (isRedundantOrExtending(existing, entry)) {
+                if (entry.isAllow() == existing.isAllow()) {
                     if (existingBits.includes(entryBits)) {
+                        // no changes
                         return false;
                     } else {
                         // merge existing and new ace
@@ -206,34 +204,27 @@ abstract class ACL extends AbstractAcces
                         entries.add(index, createACE(existing, existingBits));
                         return true;
                     }
+                } else {
+                    // existing is complementary entry -> clean up redundant
+                    // privileges defined by the existing entry
+                    PrivilegeBits updated = PrivilegeBits.getInstance(existingBits).diff(entryBits);
+                    if (updated.isEmpty()) {
+                        // remove the existing entry as the new entry covers all privileges
+                        entries.remove(existing);
+                    } else if (!updated.includes(existingBits)) {
+                        // replace the existing entry having it's privileges adjusted
+                        int index = entries.indexOf(existing);
+                        entries.remove(existing);
+                        entries.add(index, createACE(existing, updated));
+                    } /* else: no collision that requires adjusting the existing entry.*/
                 }
-
-                // clean up redundant privileges defined by the existing entry
-                // and append the new entry at the end of the list.
-                PrivilegeBits updated = PrivilegeBits.getInstance(existingBits).diff(entryBits);
-                if (updated.isEmpty()) {
-                    // remove the existing entry as the new entry covers all privileges
-                    entries.remove(ace);
-                } else if (!updated.includes(existingBits)) {
-                    // replace the existing entry having it's privileges adjusted
-                    int index = entries.indexOf(existing);
-                    entries.remove(ace);
-                    entries.add(index, createACE(existing, updated));
-                } /* else: no collision that requires adjusting the existing entry.*/
             }
         }
-
         // finally add the new entry at the end of the list
         entries.add(entry);
         return true;
     }
 
-    // TODO: OAK-814
-    private boolean isRedundantOrExtending(ACE existing, ACE entry) {
-        return existing.isAllow() == entry.isAllow()
-                && (!(existing.getPrincipal() instanceof Group) || entries.indexOf(existing)
== entries.size() - 1);
-    }
-
     private ACE createACE(ACE existing, PrivilegeBits newPrivilegeBits) throws RepositoryException
{
         return new ACE(existing.getPrincipal(), getPrivileges(newPrivilegeBits), existing.isAllow(),
existing.getRestrictions(), getNamePathMapper());
     }

Modified: jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/ACLTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/ACLTest.java?rev=1483814&r1=1483813&r2=1483814&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/ACLTest.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/ACLTest.java
Fri May 17 14:11:04 2013
@@ -33,7 +33,7 @@ import javax.jcr.security.AccessControlE
 import javax.jcr.security.AccessControlException;
 import javax.jcr.security.Privilege;
 
-import com.google.common.collect.ImmutableList;
+import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Sets;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry;
 import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
@@ -56,7 +56,6 @@ import org.apache.jackrabbit.oak.spi.sec
 import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
 import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
 import org.junit.Before;
-import org.junit.Ignore;
 import org.junit.Test;
 
 import static org.junit.Assert.assertArrayEquals;
@@ -412,7 +411,6 @@ public class ACLTest extends AbstractAcc
         assertFalse(acl.getEntries().get(0).isAllow());
     }
 
-    @Ignore("OAK-814") // TODO
     @Test
     public void testUpdateGroupEntry() throws Exception {
         Privilege[] readPriv = privilegesFromNames(JCR_READ);
@@ -434,7 +432,6 @@ public class ACLTest extends AbstractAcc
         assertACE(princ2AllowEntry, true, privilegesFromNames(JCR_READ, JCR_WRITE));
     }
 
-    @Ignore("OAK-814") // TODO
     @Test
     public void testComplementaryGroupEntry() throws Exception {
         Privilege[] readPriv = privilegesFromNames(JCR_READ);
@@ -461,11 +458,10 @@ public class ACLTest extends AbstractAcc
         assertACE(second, false, privilegesFromNames(JCR_READ, JCR_WRITE));
     }
 
-    @Ignore("OAK-814") // TODO
     @Test
     public void testAllowWriteDenyRemoveGroupEntries() throws Exception {
         Principal everyone = principalManager.getEveryone();
-        Privilege[] grPriv = privilegesFromNames("rep:write");
+        Privilege[] grPriv = privilegesFromNames(REP_WRITE);
         Privilege[] dePriv = privilegesFromNames(JCR_REMOVE_CHILD_NODES);
 
         acl.addEntry(everyone, grPriv, true, Collections.<String, Value>emptyMap());
@@ -488,10 +484,10 @@ public class ACLTest extends AbstractAcc
 
         Privilege[] expected = privilegesFromNames(JCR_ADD_CHILD_NODES, JCR_REMOVE_NODE,
JCR_MODIFY_PROPERTIES, JCR_NODE_TYPE_MANAGEMENT);
         assertEquals(expected.length, allows.size());
-        assertTrue(allows.containsAll(ImmutableList.of(expected)));
+        assertEquals(ImmutableSet.copyOf(expected), allows);
 
         assertEquals(1, denies.size());
-        assertEquals(privilegesFromNames(JCR_REMOVE_CHILD_NODES)[0], denies.iterator().next());
+        assertArrayEquals(privilegesFromNames(JCR_REMOVE_CHILD_NODES), denies.toArray(new
Privilege[denies.size()]));
     }
 
     @Test



Mime
View raw message