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 tr...@apache.org on 2015/09/04 19:13:03 UTC
svn commit: r1701297 - in /jackrabbit/oak/branches/1.0/oak-core/src:
main/java/org/apache/jackrabbit/oak/security/authorization/permission/
test/java/org/apache/jackrabbit/oak/security/authorization/restriction/
Author: tripod
Date: Fri Sep 4 17:13:02 2015
New Revision: 1701297
URL: http://svn.apache.org/r1701297
Log:
OAK-3324 Evaluation with restriction is not consistent with parent ACLs
- fixed it, so that the behavior is consisten between single node privs and dual node privs
Modified:
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java
jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/CustomRestrictionProviderTest.java
jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/PermissionTest.java
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java?rev=1701297&r1=1701296&r2=1701297&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java Fri Sep 4 17:13:02 2015
@@ -141,7 +141,8 @@ final class CompiledPermissionImpl imple
return new RepositoryPermission() {
@Override
public boolean isGranted(long repositoryPermissions) {
- return hasPermissions(getEntryIterator(new EntryPredicate()), repositoryPermissions, null);
+ EntryPredicate predicate = new EntryPredicate();
+ return hasPermissions(getEntryIterator(predicate), predicate, repositoryPermissions, null);
}
};
}
@@ -237,8 +238,8 @@ final class CompiledPermissionImpl imple
@Override
public boolean isGranted(@Nonnull String path, long permissions) {
- Iterator<PermissionEntry> it = getEntryIterator(new EntryPredicate(path, Permissions.respectParentPermissions(permissions)));
- return hasPermissions(it, permissions, path);
+ EntryPredicate predicate = new EntryPredicate(path, Permissions.respectParentPermissions(permissions));
+ return hasPermissions(getEntryIterator(predicate), predicate, permissions, path);
}
@Override
@@ -254,11 +255,12 @@ final class CompiledPermissionImpl imple
//------------------------------------------------------------< private >---
private boolean internalIsGranted(@Nonnull Tree tree, @Nullable PropertyState property, long permissions) {
- Iterator<PermissionEntry> it = getEntryIterator(tree, property, permissions);
- return hasPermissions(it, permissions, tree.getPath());
+ EntryPredicate predicate = new EntryPredicate(tree, property, Permissions.respectParentPermissions(permissions));
+ return hasPermissions(getEntryIterator(predicate), predicate, permissions, tree.getPath());
}
private boolean hasPermissions(@Nonnull Iterator<PermissionEntry> entries,
+ @Nonnull EntryPredicate predicate,
long permissions, @Nullable String path) {
// calculate readable paths if the given permissions includes any read permission.
boolean isReadable = Permissions.diff(Permissions.READ, permissions) != Permissions.READ && readPolicy.isReadablePath(path, false);
@@ -304,14 +306,18 @@ final class CompiledPermissionImpl imple
}
if (entry.isAllow) {
- allowBits.addDifference(entry.privilegeBits, denyBits);
+ if (!respectParent || predicate.apply(entry, false)) {
+ allowBits.addDifference(entry.privilegeBits, denyBits);
+ }
long ap = PrivilegeBits.calculatePermissions(allowBits, parentAllowBits, true);
allows |= Permissions.diff(ap, denies);
if ((allows | ~permissions) == -1) {
return true;
}
} else {
- denyBits.addDifference(entry.privilegeBits, allowBits);
+ if (!respectParent || predicate.apply(entry, false)) {
+ denyBits.addDifference(entry.privilegeBits, allowBits);
+ }
long dp = PrivilegeBits.calculatePermissions(denyBits, parentDenyBits, false);
denies |= Permissions.diff(dp, allows);
if (Permissions.includes(denies, permissions)) {
@@ -371,11 +377,6 @@ final class CompiledPermissionImpl imple
}
@Nonnull
- private Iterator<PermissionEntry> getEntryIterator(@Nonnull Tree tree, @Nullable PropertyState property, long permissions) {
- return getEntryIterator(new EntryPredicate(tree, property, Permissions.respectParentPermissions(permissions)));
- }
-
- @Nonnull
private Iterator<PermissionEntry> getEntryIterator(@Nonnull EntryPredicate predicate) {
Iterator<PermissionEntry> userEntries = userStore.getEntryIterator(predicate);
Iterator<PermissionEntry> groupEntries = groupStore.getEntryIterator(predicate);
@@ -519,12 +520,16 @@ final class CompiledPermissionImpl imple
@Override
public boolean isGranted(long permissions) {
- return hasPermissions(getIterator(null, permissions), permissions, tree.getPath());
+ EntryPredicate predicate = new EntryPredicate(tree, null, Permissions.respectParentPermissions(permissions));
+ Iterator<PermissionEntry> it = concat(new LazyIterator(this, true, predicate), new LazyIterator(this, false, predicate));
+ return hasPermissions(it, predicate, permissions, tree.getPath());
}
@Override
public boolean isGranted(long permissions, @Nonnull PropertyState property) {
- return hasPermissions(getIterator(property, permissions), permissions, tree.getPath());
+ EntryPredicate predicate = new EntryPredicate(tree, property, Permissions.respectParentPermissions(permissions));
+ Iterator<PermissionEntry> it = concat(new LazyIterator(this, true, predicate), new LazyIterator(this, false, predicate));
+ return hasPermissions(it, predicate, permissions, tree.getPath());
}
//--------------------------------------------------------< private >---
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java?rev=1701297&r1=1701296&r2=1701297&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/EntryPredicate.java Fri Sep 4 17:13:02 2015
@@ -37,6 +37,7 @@ final class EntryPredicate implements Pr
private final String parentPath;
private final Tree parent;
+ private final boolean respectParent;
public EntryPredicate(@Nonnull Tree tree, @Nullable PropertyState property,
boolean respectParent) {
@@ -64,6 +65,7 @@ final class EntryPredicate implements Pr
parentPath = null;
parent = null;
}
+ this.respectParent = parent != null || parentPath != null;
}
@CheckForNull
@@ -73,25 +75,22 @@ final class EntryPredicate implements Pr
@Override
public boolean apply(@Nullable PermissionEntry entry) {
+ return apply(entry, true);
+ }
+
+ public boolean apply(@Nullable PermissionEntry entry, boolean respectParent) {
if (entry == null) {
return false;
}
+ respectParent &= this.respectParent;
+
if (tree != null) {
- return entry.matches(tree, property) || applyToParent(entry);
+ return entry.matches(tree, property) || (respectParent && parent != null && entry.matches(parent, null));
} else if (path != null) {
- return entry.matches(path) || applyToParent(entry);
+ return entry.matches(path) || (respectParent && parentPath != null && entry.matches(parentPath));
} else {
return entry.matches();
}
}
- private boolean applyToParent(@Nonnull PermissionEntry entry) {
- if (parent != null) {
- return entry.matches(parent, null);
- } else if (parentPath != null) {
- return entry.matches(parentPath);
- } else {
- return false;
- }
- }
}
\ No newline at end of file
Modified: jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry.java?rev=1701297&r1=1701296&r2=1701297&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/PermissionEntry.java Fri Sep 4 17:13:02 2015
@@ -122,4 +122,16 @@ final class PermissionEntry implements C
public int hashCode() {
return Objects.hashCode(privilegeBits, index, path, isAllow, restriction);
}
+
+ @Override
+ public String toString() {
+ final StringBuilder sb = new StringBuilder("PermissionEntry{");
+ sb.append("isAllow=").append(isAllow);
+ sb.append(", privilegeBits=").append(privilegeBits);
+ sb.append(", index=").append(index);
+ sb.append(", path='").append(path).append('\'');
+ sb.append(", restriction=").append(restriction);
+ sb.append('}');
+ return sb.toString();
+ }
}
\ No newline at end of file
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/CustomRestrictionProviderTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/CustomRestrictionProviderTest.java?rev=1701297&r1=1701296&r2=1701297&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/CustomRestrictionProviderTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/CustomRestrictionProviderTest.java Fri Sep 4 17:13:02 2015
@@ -54,7 +54,6 @@ import org.apache.jackrabbit.oak.util.No
import org.apache.jackrabbit.value.StringValue;
import org.junit.After;
import org.junit.Before;
-import org.junit.Ignore;
import org.junit.Test;
import com.google.common.collect.ImmutableMap;
@@ -63,6 +62,10 @@ import static org.apache.jackrabbit.JcrC
import static org.junit.Assert.assertEquals;
import static org.junit.Assert.fail;
+/**
+ * Test suite for a custom restriction provider. The restriction is enabled based on the (non) existence of a property.
+ * The test creates nodes along '/testRoot/a/b/c/d/e' and sets the 'protect-me' property on '/testRoot/a/b/c'.
+ */
public class CustomRestrictionProviderTest extends AbstractSecurityTest {
private static final String TEST_ROOT_PATH = "/testRoot";
@@ -145,12 +148,9 @@ public class CustomRestrictionProviderTe
* @throws Exception
*/
@Test
- @Ignore("OAK-3324")
public void testProtectByRestriction() throws Exception {
- // create permissions
// allow rep:write /testroot
- // deny jcr:removeNode /testroot/a hasProperty = protect-me
-
+ // deny jcr:removeNode /testroot/a hasProperty=protect-me
addEntry(TEST_ROOT_PATH, true, "", PrivilegeConstants.JCR_READ, PrivilegeConstants.REP_WRITE);
addEntry(TEST_A_PATH, false, PROP_NAME_PROTECT_ME, PrivilegeConstants.JCR_REMOVE_NODE);
@@ -158,11 +158,11 @@ public class CustomRestrictionProviderTe
try {
Root testRoot = testSession.getLatestRoot();
PermissionProvider pp = getPermissionProvider(testSession);
- assertIsGranted(pp, testRoot, true, TEST_A_PATH, Permissions.REMOVE_NODE);
- assertIsGranted(pp, testRoot, true, TEST_B_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_A_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_B_PATH, Permissions.REMOVE_NODE);
assertIsGranted(pp, testRoot, false, TEST_C_PATH, Permissions.REMOVE_NODE);
assertIsGranted(pp, testRoot, true , TEST_D_PATH, Permissions.REMOVE_NODE);
- assertIsGranted(pp, testRoot, true, TEST_E_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_E_PATH, Permissions.REMOVE_NODE);
// should be able to remove /a/b/c/d
testRoot.getTree(TEST_D_PATH).remove();
@@ -186,10 +186,8 @@ public class CustomRestrictionProviderTe
*/
@Test
public void testProtectPropertiesByRestriction() throws Exception {
- // create permissions
- // allow rep:write /testroot
- // deny jcr:modifyProperties /testroot/a hasProperty = protect-me
-
+ // allow rep:write /testroot
+ // deny jcr:modifyProperties /testroot/a hasProperty=protect-me
addEntry(TEST_ROOT_PATH, true, "", PrivilegeConstants.JCR_READ, PrivilegeConstants.REP_WRITE);
addEntry(TEST_A_PATH, false, PROP_NAME_PROTECT_ME, PrivilegeConstants.JCR_MODIFY_PROPERTIES);
@@ -199,10 +197,10 @@ public class CustomRestrictionProviderTe
PermissionProvider pp = getPermissionProvider(testSession);
assertIsGranted(pp, testRoot, true , TEST_A_PATH, Permissions.MODIFY_PROPERTY);
- assertIsGranted(pp, testRoot, true, TEST_B_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, true , TEST_B_PATH, Permissions.MODIFY_PROPERTY);
assertIsGranted(pp, testRoot, false, TEST_C_PATH, Permissions.MODIFY_PROPERTY);
- assertIsGranted(pp, testRoot, true, TEST_D_PATH, Permissions.MODIFY_PROPERTY);
- assertIsGranted(pp, testRoot, true, TEST_E_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, true , TEST_D_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, true , TEST_E_PATH, Permissions.MODIFY_PROPERTY);
} finally {
testSession.close();
@@ -210,17 +208,14 @@ public class CustomRestrictionProviderTe
}
/**
- * Tests the custom restriction provider that checks on the existence of a property.
+ * Tests the custom restriction provider that checks on the absence of a property.
* @throws Exception
*/
@Test
- @Ignore("OAK-3324")
public void testUnProtectByRestriction() throws Exception {
- // create permissions
// allow rep:write /testroot
// deny jcr:removeNode /testroot
- // allow jcr:removeNode /testroot/a hasProperty = !protect-me
-
+ // allow jcr:removeNode /testroot/a hasProperty=!protect-me
addEntry(TEST_ROOT_PATH, true, "", PrivilegeConstants.JCR_READ, PrivilegeConstants.REP_WRITE);
addEntry(TEST_ROOT_PATH, false, "", PrivilegeConstants.JCR_REMOVE_NODE);
addEntry(TEST_A_PATH, true, "!" + PROP_NAME_PROTECT_ME, PrivilegeConstants.JCR_REMOVE_NODE);
@@ -229,17 +224,20 @@ public class CustomRestrictionProviderTe
try {
Root testRoot = testSession.getLatestRoot();
PermissionProvider pp = getPermissionProvider(testSession);
- assertIsGranted(pp, testRoot, true, TEST_A_PATH, Permissions.REMOVE_NODE);
- assertIsGranted(pp, testRoot, true, TEST_B_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_A_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_B_PATH, Permissions.REMOVE_NODE);
assertIsGranted(pp, testRoot, false, TEST_C_PATH, Permissions.REMOVE_NODE);
- assertIsGranted(pp, testRoot, true, TEST_D_PATH, Permissions.REMOVE_NODE);
- assertIsGranted(pp, testRoot, true, TEST_E_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_D_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true , TEST_E_PATH, Permissions.REMOVE_NODE);
} finally {
testSession.close();
}
}
+ /**
+ * Customs restriction provider that matches restrictions based on the existence of a property.
+ */
public static class PropertyRestrictionProvider extends AbstractRestrictionProvider {
public static final String RESTRICTION_NAME = "hasProperty";
Modified: jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/PermissionTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/PermissionTest.java?rev=1701297&r1=1701296&r2=1701297&view=diff
==============================================================================
--- jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/PermissionTest.java (original)
+++ jackrabbit/oak/branches/1.0/oak-core/src/test/java/org/apache/jackrabbit/oak/security/authorization/restriction/PermissionTest.java Fri Sep 4 17:13:02 2015
@@ -51,6 +51,7 @@ public class PermissionTest extends Abst
private static final String TEST_B_PATH = "/testRoot/a/b";
private static final String TEST_C_PATH = "/testRoot/a/b/c";
private static final String TEST_D_PATH = "/testRoot/a/b/c/d";
+ private static final String TEST_E_PATH = "/testRoot/a/b/c/d/e";
private Principal testPrincipal;
@@ -64,7 +65,8 @@ public class PermissionTest extends Abst
NodeUtil a = testRootNode.addChild("a", NT_UNSTRUCTURED);
NodeUtil b = a.addChild("b", NT_UNSTRUCTURED);
NodeUtil c = b.addChild("c", NT_UNSTRUCTURED);
- c.addChild("d", NT_UNSTRUCTURED);
+ NodeUtil d = c.addChild("d", NT_UNSTRUCTURED);
+ d.addChild("e", NT_UNSTRUCTURED);
root.commit();
testPrincipal = getTestUser().getPrincipal();
@@ -152,7 +154,6 @@ public class PermissionTest extends Abst
* of /a/b/c yields a deny, which terminates the iteration.
*/
@Test
- @Ignore("OAK-3324")
public void testHasPermissionWithRestrictions() throws Exception {
// create permissions
// allow rep:write /testroot
@@ -172,6 +173,7 @@ public class PermissionTest extends Abst
assertIsGranted(pp, testRoot, true, TEST_B_PATH, Permissions.REMOVE_NODE);
assertIsGranted(pp, testRoot, false, TEST_C_PATH, Permissions.REMOVE_NODE);
assertIsGranted(pp, testRoot, true, TEST_D_PATH, Permissions.REMOVE_NODE);
+ assertIsGranted(pp, testRoot, true, TEST_E_PATH, Permissions.REMOVE_NODE);
// should be able to remove /a/b/c/d
testRoot.getTree(TEST_D_PATH).remove();
@@ -197,7 +199,6 @@ public class PermissionTest extends Abst
* since the 'deny' on /a/b is after the 'allow' on a/b/c, the deny wins.
*/
@Test
- @Ignore("OAK-3324")
public void testHasPermissionWithRestrictions2() throws Exception {
// create permissions
// allow rep:write /testroot
@@ -236,4 +237,35 @@ public class PermissionTest extends Abst
testSession.close();
}
}
+
+ /**
+ * Tests the custom restriction provider that checks on the existence of a property.
+ * @throws Exception
+ */
+ @Test
+ public void testProtectPropertiesByRestriction() throws Exception {
+ // create permissions
+ // allow rep:write /testroot
+ // deny jcr:modifyProperties /testroot/a glob = */c
+
+ addEntry(TEST_ROOT_PATH, true, "", PrivilegeConstants.JCR_READ, PrivilegeConstants.REP_WRITE);
+ addEntry(TEST_A_PATH, false, "*/c", PrivilegeConstants.JCR_MODIFY_PROPERTIES);
+
+ ContentSession testSession = createTestSession();
+ try {
+ Root testRoot = testSession.getLatestRoot();
+
+ PermissionProvider pp = getPermissionProvider(testSession);
+ assertIsGranted(pp, testRoot, true , TEST_A_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, true, TEST_B_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, false, TEST_C_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, true, TEST_D_PATH, Permissions.MODIFY_PROPERTY);
+ assertIsGranted(pp, testRoot, true, TEST_E_PATH, Permissions.MODIFY_PROPERTY);
+
+ } finally {
+ testSession.close();
+ }
+ }
+
+
}
\ No newline at end of file