jackrabbit-oak-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From ju...@apache.org
Subject svn commit: r1468785 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core: RootImpl.java SecureNodeState.java SecurityContext.java
Date Wed, 17 Apr 2013 07:43:12 GMT
Author: jukka
Date: Wed Apr 17 07:43:12 2013
New Revision: 1468785

URL: http://svn.apache.org/r1468785
Log:
OAK-709: Consider moving permission evaluation to the node state level

Extract the security context to a separate SecurityContext class

Added:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java
  (with props)
Modified:
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
    jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java?rev=1468785&r1=1468784&r2=1468785&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/RootImpl.java
Wed Apr 17 07:43:12 2013
@@ -139,8 +139,8 @@ public class RootImpl implements Root {
         this.indexProvider = indexProvider;
 
         branch = this.store.branch();
-        secureHead = new SecureNodeState(
-                branch.getHead(), getPermissionProvider(), getTypeProvider());
+        NodeState root = branch.getHead();
+        secureHead = new SecureNodeState(root, getRootContext(root));
         rootTree = new TreeImpl(this, secureHead.builder(), lastMove);
     }
 
@@ -387,7 +387,8 @@ public class RootImpl implements Root {
      * @return secure base node state
      */
     NodeState getSecureBase() {
-        return new SecureNodeState(branch.getBase(), getPermissionProvider(), getTypeProvider());
+        NodeState root = branch.getBase();
+        return new SecureNodeState(root, getRootContext(root));
     }
 
     // TODO better way to determine purge limit. See OAK-175
@@ -424,6 +425,13 @@ public class RootImpl implements Root {
     }
 
     @Nonnull
+    private SecurityContext getRootContext(NodeState root) {
+        TreeTypeProvider typeProvider = new TreeTypeProviderImpl(
+                securityProvider.getAccessControlConfiguration().getContext());
+        return new SecurityContext(root, getPermissionProvider(), typeProvider);
+    }
+
+    @Nonnull
     private PermissionProvider getPermissionProvider() {
         if (permissionProvider == null) {
             permissionProvider = createPermissionProvider();
@@ -443,8 +451,8 @@ public class RootImpl implements Root {
      * Reset the root builder to the branch's current root state
      */
     private void reset() {
-        secureHead = new SecureNodeState(
-                branch.getHead(), getPermissionProvider(), getTypeProvider());
+        NodeState root = branch.getHead();
+        secureHead = new SecureNodeState(root, getRootContext(root));
         rootTree.reset(secureHead);
     }
 
@@ -453,11 +461,6 @@ public class RootImpl implements Root {
         return securityProvider.getAccessControlConfiguration().getPermissionProvider(this,
subject.getPrincipals());
     }
 
-    @Nonnull
-    private TreeTypeProviderImpl getTypeProvider() {
-        return new TreeTypeProviderImpl(securityProvider.getAccessControlConfiguration().getContext());
-    }
-
     //---------------------------------------------------------< MoveRecord >---
 
     /**

Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java?rev=1468785&r1=1468784&r2=1468785&view=diff
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
(original)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecureNodeState.java
Wed Apr 17 07:43:12 2013
@@ -22,12 +22,9 @@ import javax.annotation.Nonnull;
 
 import com.google.common.base.Function;
 import com.google.common.base.Predicate;
-import com.google.common.collect.Iterables;
 import org.apache.jackrabbit.oak.api.PropertyState;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryChildNodeEntry;
 import org.apache.jackrabbit.oak.plugins.memory.MemoryNodeBuilder;
-import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider;
-import org.apache.jackrabbit.oak.spi.security.authorization.permission.ReadStatus;
 import org.apache.jackrabbit.oak.spi.state.AbstractNodeState;
 import org.apache.jackrabbit.oak.spi.state.ChildNodeEntry;
 import org.apache.jackrabbit.oak.spi.state.NodeBuilder;
@@ -35,6 +32,8 @@ import org.apache.jackrabbit.oak.spi.sta
 import org.apache.jackrabbit.oak.spi.state.NodeStateDiff;
 
 import static com.google.common.base.Preconditions.checkNotNull;
+import static com.google.common.collect.Iterables.filter;
+import static com.google.common.collect.Iterables.transform;
 
 /**
  * SecureNodeState...
@@ -47,22 +46,14 @@ import static com.google.common.base.Pre
 class SecureNodeState extends AbstractNodeState {
 
     /**
-     * Underlying root state, used to optimize a common case
-     * in {@link #equals(Object)}.
-     */
-    private final NodeState root;
-
-    /**
      * Underlying node state.
      */
     private final NodeState state;
 
     /**
-     * Immutable tree based on the underlying node state.
+     * Security context of this subtree.
      */
-    private final ImmutableTree base;
-
-    private final PermissionProvider permissionProvider;
+    private final SecurityContext context;
 
     /**
      * Predicate for testing whether a given property is readable.
@@ -70,8 +61,7 @@ class SecureNodeState extends AbstractNo
     private final Predicate<PropertyState> isPropertyReadable = new Predicate<PropertyState>()
{
         @Override
         public boolean apply(@Nonnull PropertyState property) {
-            ReadStatus status = permissionProvider.getReadStatus(base, property);
-            return status.isAllow();
+            return context.canReadProperty(property);
         }
     };
 
@@ -96,16 +86,16 @@ class SecureNodeState extends AbstractNo
     * we can optimize access by skipping the security wrapper entirely.
     */
     private final Function<ChildNodeEntry, ChildNodeEntry> wrapChildNodeEntry = new
Function<ChildNodeEntry, ChildNodeEntry>() {
-       @Nonnull
-       @Override
+        @Override @Nonnull
         public ChildNodeEntry apply(@Nonnull ChildNodeEntry input) {
             String name = input.getName();
             NodeState child = input.getNodeState();
+            SecurityContext childContext = context.getChildContext(name, child);
             SecureNodeState secureChild =
-                    new SecureNodeState(SecureNodeState.this, name, child);
+                    new SecureNodeState(child, childContext);
             if (child.getChildNodeCount() == 0
-                    && secureChild.getReadStatus().includes(
-                            ReadStatus.ALLOW_THIS_PROPERTIES)) {
+                    && secureChild.context.canReadThisNode()
+                    && secureChild.context.canReadAllProperties()) {
                 // Since this is an accessible leaf node whose all properties
                 // are readable, we don't need the SecureNodeState wrapper
                 // TODO: A further optimization would be to return the raw
@@ -120,89 +110,53 @@ class SecureNodeState extends AbstractNo
         }
     };
 
-    private ReadStatus readStatus;
-
     private long childNodeCount = -1;
 
     private long propertyCount = -1;
 
-    public SecureNodeState(@Nonnull NodeState rootState,
-                           @Nonnull PermissionProvider permissionProvider,
-                           @Nonnull TreeTypeProvider typeProvider) {
-        this.root = checkNotNull(rootState);
-        this.state = rootState;
-        this.base = new ImmutableTree(rootState, typeProvider);
-        this.permissionProvider = permissionProvider;
-        // calculate the readstatus for the root
-        this.readStatus = permissionProvider.getReadStatus(base, null);
-    }
-
-    private SecureNodeState(
-            @Nonnull SecureNodeState parent,
-            @Nonnull String name, @Nonnull NodeState nodeState) {
-        this.root = checkNotNull(parent).root;
-        this.state = checkNotNull(nodeState);
-        this.base = new ImmutableTree(parent.base, name, nodeState);
-        this.permissionProvider = parent.permissionProvider;
-
-        if (base.getType() == parent.base.getType()) {
-            readStatus = ReadStatus.getChildStatus(parent.readStatus);
-        }
+    SecureNodeState(
+            @Nonnull NodeState state, @Nonnull SecurityContext context) {
+        this.state = checkNotNull(state);
+        this.context = checkNotNull(context);
     }
 
     @Override
     public boolean exists() {
-        return getReadStatus().includes(ReadStatus.ALLOW_THIS);
+        return context.canReadThisNode();
     }
 
     @Override @CheckForNull
     public PropertyState getProperty(String name) {
         PropertyState property = state.getProperty(name);
-        if (property == null) {
+        if (property != null && context.canReadProperty(property)) {
             return property;
-        }
-        ReadStatus rs = getReadStatus();
-        if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
-            return property;
-        } else if (rs.appliesToThis()) {
-            // calculate for property individually
-            return (isPropertyReadable.apply(property)) ? property : null;
         } else {
-            // property access is denied
             return null;
         }
     }
 
     @Override
     public synchronized long getPropertyCount() {
-        ReadStatus rs = getReadStatus();
         if (propertyCount == -1) {
-            if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
-                // all properties are readable
+            if (context.canReadAllProperties()) {
                 propertyCount = state.getPropertyCount();
-            } else if (rs.appliesToThis()) {
-                // some properties may be readable
-                propertyCount = count(Iterables.filter(state.getProperties(), isPropertyReadable));
+            } else if (context.canReadThisNode()) {
+                propertyCount = count(
+                        filter(state.getProperties(), isPropertyReadable));
             } else {
-                // property access is denied
-                return 0;
+                propertyCount = 0;
             }
         }
         return propertyCount;
     }
 
-    @Nonnull
-    @Override
+    @Override @Nonnull
     public Iterable<? extends PropertyState> getProperties() {
-        ReadStatus rs = getReadStatus();
-        if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
-            // all properties are readable
+        if (context.canReadAllProperties()) {
             return state.getProperties();
-        } else if (rs.appliesToThis()) {
-            // some properties may be readable
-            return Iterables.filter(state.getProperties(), isPropertyReadable);
+        } else if (context.canReadThisNode()) {
+            return filter(state.getProperties(), isPropertyReadable);
         } else {
-            // property access is denied
             return Collections.emptySet();
         }
     }
@@ -226,22 +180,20 @@ class SecureNodeState extends AbstractNo
         return childNodeCount;
     }
 
-    @Override
-    @Nonnull
+    @Override @Nonnull
     public Iterable<? extends ChildNodeEntry> getChildNodeEntries() {
-        if (getReadStatus().includes(ReadStatus.DENY_CHILDREN)) {
+        if (context.canNotReadChildNodes()) {
             return Collections.emptySet();
         } else {
             // TODO: review if ALLOW_CHILDREN could be used as well although we
             // don't know the type of all child-nodes where ac node would need special treatment
-            Iterable<ChildNodeEntry> readable = Iterables.transform(
-                    state.getChildNodeEntries(), wrapChildNodeEntry);
-            return Iterables.filter(readable, isIterableNode);
+            Iterable<ChildNodeEntry> readable =
+                    transform(state.getChildNodeEntries(), wrapChildNodeEntry);
+            return filter(readable, isIterableNode);
         }
     }
 
-    @Nonnull
-    @Override
+    @Override @Nonnull
     public NodeBuilder builder() {
         return new MemoryNodeBuilder(this);
     }
@@ -265,19 +217,11 @@ class SecureNodeState extends AbstractNo
             // different revisions (root states) as long as the path,
             // the subtree, and any security-related areas like the
             // permission store are equal for both states.
-            if (root.equals(that.root)
-                    && base.getPath().equals(that.base.getPath())) {
+            if (state.equals(that.state) && context.equals(that.context)) {
                 return true;
             }
         }
         return super.equals(object);
     }
 
-    //------------------------------------------------------------< private >---
-    private ReadStatus getReadStatus() {
-        if (readStatus == null) {
-            readStatus = permissionProvider.getReadStatus(base, null);
-        }
-        return readStatus;
-    }
 }

Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java
URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java?rev=1468785&view=auto
==============================================================================
--- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java
(added)
+++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java
Wed Apr 17 07:43:12 2013
@@ -0,0 +1,134 @@
+/*
+ * 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.core;
+
+import javax.annotation.Nonnull;
+
+import org.apache.jackrabbit.oak.api.PropertyState;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.PermissionProvider;
+import org.apache.jackrabbit.oak.spi.security.authorization.permission.ReadStatus;
+import org.apache.jackrabbit.oak.spi.state.NodeState;
+
+import static com.google.common.base.Preconditions.checkNotNull;
+
+/**
+ * The security context encapsulates all the information needed to make
+ * read permission checks within a specific subtree.
+ */
+class SecurityContext {
+
+    /**
+     * Underlying root state, used to optimize a common case
+     * in {@link #equals(Object)}.
+     */
+    private final NodeState root;
+
+    /**
+     * Immutable tree based on the underlying node state.
+     */
+    private final ImmutableTree base;
+
+    private final PermissionProvider permissionProvider;
+
+    private ReadStatus readStatus;
+
+    SecurityContext(
+            @Nonnull NodeState rootState,
+            @Nonnull PermissionProvider permissionProvider,
+            @Nonnull TreeTypeProvider typeProvider) {
+        this.root = checkNotNull(rootState);
+        this.base = new ImmutableTree(rootState, typeProvider);
+        this.permissionProvider = permissionProvider;
+        // calculate the readstatus for the root
+        this.readStatus = permissionProvider.getReadStatus(base, null);
+    }
+
+    private SecurityContext(
+            @Nonnull SecurityContext parent,
+            @Nonnull String name, @Nonnull NodeState nodeState) {
+        this.root = checkNotNull(parent).root;
+        this.base = new ImmutableTree(parent.base, name, nodeState);
+        this.permissionProvider = parent.permissionProvider;
+        if (base.getType() == parent.base.getType()) {
+            readStatus = ReadStatus.getChildStatus(parent.readStatus);
+        } else {
+            readStatus = null;
+        }
+    }
+
+    private synchronized ReadStatus getReadStatus() {
+        if (readStatus == null) {
+            readStatus = permissionProvider.getReadStatus(base, null);
+        }
+        return readStatus;
+    }
+
+    boolean canReadThisNode() {
+        return getReadStatus().includes(ReadStatus.ALLOW_THIS);
+    }
+
+    boolean canReadAllProperties() {
+        ReadStatus rs = getReadStatus();
+        return rs.includes(ReadStatus.ALLOW_PROPERTIES);
+    }
+
+    boolean canReadProperty(PropertyState property) {
+        ReadStatus rs = getReadStatus();
+        if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) {
+            return true;
+        } else if (rs.appliesToThis()) {
+            rs = permissionProvider.getReadStatus(base, property);
+            return rs.isAllow();
+        } else {
+            return false;
+        }
+    }
+
+    boolean canNotReadChildNodes() {
+        ReadStatus rs = getReadStatus();
+        return rs.includes(ReadStatus.DENY_CHILDREN);
+    }
+
+    SecurityContext getChildContext(String name, NodeState state) {
+        return new SecurityContext(this, name, state);
+    }
+
+    //-------------------------------------------------------------< Object >---
+
+    @Override
+    public boolean equals(Object object) {
+        if (object == this) {
+            return true;
+        } else if (object instanceof SecurityContext) {
+            SecurityContext that = (SecurityContext) object;
+            // TODO: We should be able to do this optimization also across
+            // different revisions (root states) as long as the path,
+            // the subtree, and any security-related areas like the
+            // permission store are equal for both states.
+            return root.equals(that.root)
+                    && base.getPath().equals(that.base.getPath());
+        } else {
+            return false;
+        }
+    }
+
+    @Override
+    public int hashCode() {
+        return 0;
+    }
+
+}

Propchange: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/SecurityContext.java
------------------------------------------------------------------------------
    svn:eol-style = native



Mime
View raw message