jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ang...@apache.org
Subject 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/
Date Tue, 05 Nov 2013 15:10:29 GMT
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()));
         }
     }
 



Mime
View raw message