You are viewing a plain text version of this content. The canonical link for it is here.
Posted to oak-commits@jackrabbit.apache.org by an...@apache.org on 2013/05/17 16:11:04 UTC
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
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