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/11/05 16:10:29 UTC
svn commit: r1539022 - in /jackrabbit/oak/trunk:
oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/
oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/
Author: angela
Date: Tue Nov 5 15:10:29 2013
New Revision: 1539022
URL: http://svn.apache.org/r1539022
Log:
OAK-527: bug in readstatus calculation (filtered entries may affect accessibility of properties and child nodes)
Added:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ReadStatus.java
Modified:
jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadPropertyTest.java
Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java?rev=1539022&r1=1539021&r2=1539022&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java (original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/CompiledPermissionImpl.java Tue Nov 5 15:10:29 2013
@@ -69,6 +69,7 @@ final class CompiledPermissionImpl imple
private static final Logger log = LoggerFactory.getLogger(CompiledPermissionImpl.class);
private static final Map<Long, PrivilegeBits> READ_BITS = ImmutableMap.of(
+ Permissions.READ, PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ),
Permissions.READ_NODE, PrivilegeBits.BUILT_IN.get(PrivilegeConstants.REP_READ_NODES),
Permissions.READ_PROPERTY, PrivilegeBits.BUILT_IN.get(PrivilegeConstants.REP_READ_PROPERTIES),
Permissions.READ_ACCESS_CONTROL, PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ_ACCESS_CONTROL));
@@ -421,6 +422,7 @@ final class CompiledPermissionImpl imple
private Collection<PermissionEntry> userEntries;
private Collection<PermissionEntry> groupEntries;
+ private boolean skipped;
private ReadStatus readStatus;
private TreePermissionImpl(Tree tree, int treeType, TreePermission parentPermission) {
@@ -441,13 +443,15 @@ final class CompiledPermissionImpl imple
}
if (readStatus == null) {
readStatus = ReadStatus.DENY_THIS;
+
long permission = (isAcTree) ? Permissions.READ_ACCESS_CONTROL : Permissions.READ_NODE;
PrivilegeBits requiredBits = READ_BITS.get(permission);
+
Iterator<PermissionEntry> it = getIterator(null);
while (it.hasNext()) {
PermissionEntry entry = it.next();
if (entry.privilegeBits.includes(requiredBits)) {
- readStatus = ReadStatus.create(entry, permission);
+ readStatus = ReadStatus.create(entry, permission, skipped);
break;
}
}
@@ -498,9 +502,8 @@ final class CompiledPermissionImpl imple
}
private Iterator<PermissionEntry> getIterator(@Nullable PropertyState property) {
- return Iterators.filter(
- concat(new LazyIterator(this, true), new LazyIterator(this, false)),
- new EntryPredicate(tree, property));
+ EntryPredicate predicate = new EntryPredicate(tree, property);
+ return concat(new LazyIterator(this, true, predicate), new LazyIterator(this, false, predicate));
}
private Iterator<PermissionEntry> getUserEntries() {
@@ -520,23 +523,34 @@ final class CompiledPermissionImpl imple
private static final class LazyIterator extends AbstractEntryIterator {
+ private final TreePermissionImpl treePermission;
private final boolean isUser;
+ private final EntryPredicate predicate;
+
// the ordered permission entries at a given path in the hierarchy
private Iterator<PermissionEntry> nextEntries = Iterators.emptyIterator();
private TreePermissionImpl tp;
- private LazyIterator(@Nonnull TreePermissionImpl tp, boolean isUser) {
- this.tp = tp;
+ private LazyIterator(@Nonnull TreePermissionImpl treePermission, boolean isUser, @Nonnull EntryPredicate predicate) {
+ this.treePermission = treePermission;
this.isUser = isUser;
+ this.predicate = predicate;
+
+ tp = treePermission;
}
@Override
protected void seekNext() {
for (next = null; next == null;) {
if (nextEntries.hasNext()) {
- next = nextEntries.next();
+ PermissionEntry pe = nextEntries.next();
+ if (predicate.apply(pe)) {
+ next = pe;
+ } else {
+ treePermission.skipped = true;
+ }
} else {
if (tp == null) {
break;
@@ -644,62 +658,5 @@ final class CompiledPermissionImpl imple
}
}
- private static final class ReadStatus {
- private static final int THIS = 1;
- private static final int PROPERTIES = 2;
- private static final int CHILD_NODES = 4;
- private static final int THIS_PROPERTIES = THIS | PROPERTIES;
- private static final int ALL = THIS | PROPERTIES | CHILD_NODES;
-
- private static final ReadStatus ALLOW_THIS = new ReadStatus(THIS, true);
- private static final ReadStatus ALLOW_THIS_PROPERTIES = new ReadStatus(THIS_PROPERTIES, true);
- private static final ReadStatus ALLOW_ALL = new ReadStatus(ALL, true);
- private static final ReadStatus DENY_THIS = new ReadStatus(THIS, false);
- private static final ReadStatus DENY_THIS_PROPERTIES = new ReadStatus(THIS_PROPERTIES, false);
- private static final ReadStatus DENY_ALL = new ReadStatus(ALL, false);
-
- private static final PrivilegeBits READ_BITS = PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ);
- private static final PrivilegeBits READ_PROPERTIES_BITS = PrivilegeBits.BUILT_IN.get(PrivilegeConstants.REP_READ_PROPERTIES);
-
- private final int status;
- private final boolean isAllow;
-
- private ReadStatus(int status, boolean isAllow) {
- this.status = status;
- this.isAllow = isAllow;
- }
-
- private static ReadStatus create(PermissionEntry pe, long permission) {
- // best effort: read status is only calculated if the first matching
- // entry doesn't define any restrictions and it's a regular tree
- if (permission == Permissions.READ_ACCESS_CONTROL) {
- return (pe.isAllow) ? ALLOW_THIS : DENY_THIS;
- } else {
- if (pe.restriction == RestrictionPattern.EMPTY) {
- if (pe.privilegeBits.includes(READ_BITS)) {
- return (pe.isAllow) ? ALLOW_ALL : DENY_ALL;
- } else if (pe.privilegeBits.includes(READ_PROPERTIES_BITS)) {
- return (pe.isAllow) ? ALLOW_THIS_PROPERTIES : DENY_THIS_PROPERTIES;
- } else {
- return (pe.isAllow) ? ALLOW_THIS : DENY_THIS;
- }
- } else {
- return (pe.isAllow) ? ALLOW_THIS : DENY_THIS;
- }
- }
- }
-
- private boolean allowsThis() {
- return isAllow && ((status & THIS) == THIS);
- }
-
- private boolean allowsProperties() {
- return isAllow && ((status & PROPERTIES) == PROPERTIES);
- }
-
- private boolean allowsAll() {
- return false;
- }
- }
}
Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ReadStatus.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ReadStatus.java?rev=1539022&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ReadStatus.java (added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/permission/ReadStatus.java Tue Nov 5 15:10:29 2013
@@ -0,0 +1,85 @@
+/*
+ * 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.jackrabbit.oak.security.authorization.permission;
+
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.Permissions;
+import org.apache.jackrabbit.oak.spi.security.authorization.restriction.RestrictionPattern;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBits;
+import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
+
+/**
+ * ReadStatus... TODO
+ */
+final class ReadStatus {
+
+ private static final int THIS = 1;
+ private static final int PROPERTIES = 2;
+ private static final int CHILD_NODES = 4;
+ private static final int THIS_PROPERTIES = THIS | PROPERTIES;
+ private static final int ALL = THIS | PROPERTIES | CHILD_NODES;
+
+ static final ReadStatus ALLOW_THIS = new ReadStatus(THIS, true);
+ static final ReadStatus ALLOW_THIS_PROPERTIES = new ReadStatus(THIS_PROPERTIES, true);
+ static final ReadStatus ALLOW_ALL = new ReadStatus(ALL, true);
+ static final ReadStatus DENY_THIS = new ReadStatus(THIS, false);
+ static final ReadStatus DENY_THIS_PROPERTIES = new ReadStatus(THIS_PROPERTIES, false);
+ static final ReadStatus DENY_ALL = new ReadStatus(ALL, false);
+
+ private static final PrivilegeBits READ_BITS = PrivilegeBits.BUILT_IN.get(PrivilegeConstants.JCR_READ);
+ private static final PrivilegeBits READ_PROPERTIES_BITS = PrivilegeBits.BUILT_IN.get(PrivilegeConstants.REP_READ_PROPERTIES);
+
+ private final int status;
+ private final boolean isAllow;
+
+ private ReadStatus(int status, boolean isAllow) {
+ this.status = status;
+ this.isAllow = isAllow;
+ }
+
+ static ReadStatus create(PermissionEntry pe, long permission, boolean skipped) {
+ /*
+ best effort: read status is only calculated if
+ - no permission entries have been filtered out (e.g. an entry that
+ only applies to certain properties and thus not to the target tree itself)
+ - the target does not define access control content
+ - the matching entry doesn't contain any restrictions
+ */
+ if (skipped || permission == Permissions.READ_ACCESS_CONTROL || pe.restriction != RestrictionPattern.EMPTY) {
+ return (pe.isAllow) ? ALLOW_THIS : DENY_THIS;
+ } else {
+ if (pe.privilegeBits.includes(READ_BITS)) {
+ return (pe.isAllow) ? ALLOW_ALL : DENY_ALL;
+ } else if (pe.privilegeBits.includes(READ_PROPERTIES_BITS)) {
+ return (pe.isAllow) ? ALLOW_THIS_PROPERTIES : DENY_THIS_PROPERTIES;
+ } else {
+ return (pe.isAllow) ? ALLOW_THIS : DENY_THIS;
+ }
+ }
+ }
+
+ boolean allowsThis() {
+ return isAllow && ((status & THIS) == THIS);
+ }
+
+ boolean allowsProperties() {
+ return isAllow && ((status & PROPERTIES) == PROPERTIES);
+ }
+
+ boolean allowsAll() {
+ return false; // TODO: calculation of allows-all requires knowledge of permissions defined in the subtree
+ }
+}
\ No newline at end of file
Modified: jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadPropertyTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadPropertyTest.java?rev=1539022&r1=1539021&r2=1539022&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadPropertyTest.java (original)
+++ jackrabbit/oak/trunk/oak-jcr/src/test/java/org/apache/jackrabbit/oak/jcr/security/authorization/ReadPropertyTest.java Tue Nov 5 15:10:29 2013
@@ -21,6 +21,7 @@ import java.util.List;
import javax.jcr.AccessDeniedException;
import javax.jcr.Node;
import javax.jcr.Property;
+import javax.jcr.PropertyIterator;
import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConstants;
import org.junit.Test;
@@ -53,7 +54,27 @@ public class ReadPropertyTest extends Ab
}
@Test
- public void testDenyReadProperty() throws Exception {
+ public void testReadProperty2() throws Exception {
+ deny(path, privilegesFromName(PrivilegeConstants.REP_READ_NODES));
+
+ assertFalse(testSession.nodeExists(path));
+ assertFalse(testSession.itemExists(path));
+ assertFalse(testSession.nodeExists(childNPath));
+
+ List<String> propertyPaths = new ArrayList<String>();
+ propertyPaths.add(childPPath);
+ propertyPaths.add(childchildPPath);
+ propertyPaths.add(path + "/jcr:primaryType");
+
+ for (String pPath : propertyPaths) {
+ assertTrue(testSession.itemExists(pPath));
+ assertTrue(testSession.propertyExists(pPath));
+ Property p = testSession.getProperty(pPath);
+ }
+ }
+
+ @Test
+ public void testDenyReadProperties() throws Exception {
allow(path, privilegesFromName(PrivilegeConstants.JCR_READ));
deny(path, privilegesFromName(PrivilegeConstants.REP_READ_PROPERTIES));
@@ -70,25 +91,40 @@ public class ReadPropertyTest extends Ab
assertFalse(testSession.itemExists(pPath));
assertFalse(testSession.propertyExists(pPath));
}
+
+ Node target = testSession.getNode(path);
+ assertFalse(target.getProperties().hasNext());
}
@Test
- public void testReadProperty2() throws Exception {
- deny(path, privilegesFromName(PrivilegeConstants.REP_READ_NODES));
-
- assertFalse(testSession.nodeExists(path));
- assertFalse(testSession.itemExists(path));
- assertFalse(testSession.nodeExists(childNPath));
+ public void testDenySingleProperty() throws Exception {
+ allow(path, privilegesFromName(PrivilegeConstants.JCR_READ));
+ deny(path, privilegesFromName(PrivilegeConstants.REP_READ_PROPERTIES), createGlobRestriction("*/" +propertyName1));
- List<String> propertyPaths = new ArrayList<String>();
- propertyPaths.add(childPPath);
- propertyPaths.add(childchildPPath);
- propertyPaths.add(path + "/jcr:primaryType");
+ assertTrue(testSession.nodeExists(path));
+ assertTrue(testSession.itemExists(path));
+ assertTrue(testSession.nodeExists(childNPath));
- for (String pPath : propertyPaths) {
+ List<String> allowed = new ArrayList<String>();
+ allowed.add(path + "/jcr:primaryType");
+ allowed.add(childNPath + "/jcr:primaryType");
+ for (String pPath : allowed) {
assertTrue(testSession.itemExists(pPath));
assertTrue(testSession.propertyExists(pPath));
- Property p = testSession.getProperty(pPath);
+ }
+
+ List<String> denied = new ArrayList<String>();
+ denied.add(childPPath);
+ denied.add(childchildPPath);
+ for (String pPath : denied) {
+ assertFalse(testSession.itemExists(pPath));
+ assertFalse(testSession.propertyExists(pPath));
+ }
+
+ Node target = testSession.getNode(path);
+ PropertyIterator pit = target.getProperties();
+ while (pit.hasNext()) {
+ assertFalse(propertyName1.equals(pit.nextProperty().getName()));
}
}