Return-Path: X-Original-To: apmail-jackrabbit-oak-commits-archive@minotaur.apache.org Delivered-To: apmail-jackrabbit-oak-commits-archive@minotaur.apache.org Received: from mail.apache.org (hermes.apache.org [140.211.11.3]) by minotaur.apache.org (Postfix) with SMTP id C530510020 for ; Fri, 5 Apr 2013 15:34:48 +0000 (UTC) Received: (qmail 56412 invoked by uid 500); 5 Apr 2013 15:34:48 -0000 Delivered-To: apmail-jackrabbit-oak-commits-archive@jackrabbit.apache.org Received: (qmail 56381 invoked by uid 500); 5 Apr 2013 15:34:48 -0000 Mailing-List: contact oak-commits-help@jackrabbit.apache.org; run by ezmlm Precedence: bulk List-Help: List-Unsubscribe: List-Post: List-Id: Reply-To: oak-dev@jackrabbit.apache.org Delivered-To: mailing list oak-commits@jackrabbit.apache.org Received: (qmail 56373 invoked by uid 99); 5 Apr 2013 15:34:48 -0000 Received: from nike.apache.org (HELO nike.apache.org) (192.87.106.230) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 05 Apr 2013 15:34:48 +0000 X-ASF-Spam-Status: No, hits=-2000.0 required=5.0 tests=ALL_TRUSTED X-Spam-Check-By: apache.org Received: from [140.211.11.4] (HELO eris.apache.org) (140.211.11.4) by apache.org (qpsmtpd/0.29) with ESMTP; Fri, 05 Apr 2013 15:34:43 +0000 Received: from eris.apache.org (localhost [127.0.0.1]) by eris.apache.org (Postfix) with ESMTP id 1827023889BB; Fri, 5 Apr 2013 15:34:22 +0000 (UTC) Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: svn commit: r1465011 - in /jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak: core/RootImpl.java core/TreeImpl.java kernel/KernelNodeState.java spi/state/SecureNodeState.java Date: Fri, 05 Apr 2013 15:34:21 -0000 To: oak-commits@jackrabbit.apache.org From: angela@apache.org X-Mailer: svnmailer-1.0.8-patched Message-Id: <20130405153422.1827023889BB@eris.apache.org> X-Virus-Checked: Checked by ClamAV on apache.org Author: angela Date: Fri Apr 5 15:34:21 2013 New Revision: 1465011 URL: http://svn.apache.org/r1465011 Log: OAK-709: Consider moving permission evaluation to the node state level (intermediate commit to finally move down the permission evaluation. open TODOs/FIXMEs marked in the code and listed in the issue) Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/SecureNodeState.java 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/TreeImpl.java jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.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=1465011&r1=1465010&r2=1465011&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 Fri Apr 5 15:34:21 2013 @@ -18,19 +18,12 @@ */ package org.apache.jackrabbit.oak.core; -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; -import static org.apache.jackrabbit.oak.commons.PathUtils.elements; -import static org.apache.jackrabbit.oak.commons.PathUtils.getName; -import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath; - import java.io.IOException; import java.io.InputStream; import java.security.PrivilegedAction; import java.util.ArrayList; import java.util.Collections; import java.util.List; - import javax.annotation.Nonnull; import javax.security.auth.Subject; @@ -65,6 +58,13 @@ import org.apache.jackrabbit.oak.spi.sta import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; import org.apache.jackrabbit.oak.spi.state.NodeStore; import org.apache.jackrabbit.oak.spi.state.NodeStoreBranch; +import org.apache.jackrabbit.oak.spi.state.SecureNodeState; + +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static org.apache.jackrabbit.oak.commons.PathUtils.elements; +import static org.apache.jackrabbit.oak.commons.PathUtils.getName; +import static org.apache.jackrabbit.oak.commons.PathUtils.getParentPath; public class RootImpl implements Root { @@ -242,7 +242,7 @@ public class RootImpl implements Root { if (!store.getRoot().equals(rootTree.getBaseState())) { purgePendingChanges(); branch.rebase(); - rootTree.reset(branch.getHead()); + rootTree.reset(getSecureHead()); permissionProvider = null; } } @@ -251,7 +251,7 @@ public class RootImpl implements Root { public final void refresh() { checkLive(); branch = store.branch(); - rootTree.reset(branch.getHead()); + rootTree.reset(getSecureHead()); modCount = 0; if (permissionProvider != null) { permissionProvider.refresh(); @@ -324,7 +324,7 @@ public class RootImpl implements Root { @Override public boolean hasPendingChanges() { checkLive(); - return !getBaseState().equals(rootTree.getNodeState()); + return !getBaseState().equals(getRootState()); } @Nonnull @@ -349,7 +349,7 @@ public class RootImpl implements Root { @Override protected NodeState getRootState() { - return rootTree.getNodeState(); + return RootImpl.this.getRootState(); } @Override @@ -378,7 +378,7 @@ public class RootImpl implements Root { private QueryIndexProvider getIndexProvider() { if (hasPendingChanges()) { return new UUIDDiffIndexProviderWrapper(indexProvider, - getBaseState(), rootTree.getNodeState()); + getBaseState(), getRootState()); } return indexProvider; } @@ -397,7 +397,7 @@ public class RootImpl implements Root { @Nonnull NodeBuilder createRootBuilder() { - return branch.getHead().builder(); + return getSecureHead().builder(); } // TODO better way to determine purge limit. See OAK-175 @@ -427,7 +427,7 @@ public class RootImpl implements Root { * Purge all pending changes to the underlying {@link NodeStoreBranch}. */ private void purgePendingChanges() { - branch.setRoot(rootTree.getNodeState()); + branch.setRoot(getRootState()); reset(); } @@ -435,13 +435,30 @@ public class RootImpl implements Root { * Reset the root builder to the branch's current root state */ private void reset() { - rootTree.getNodeBuilder().reset(branch.getHead()); + rootTree.getNodeBuilder().reset(getSecureHead()); } + @Nonnull private PermissionProvider createPermissionProvider() { return securityProvider.getAccessControlConfiguration().getPermissionProvider(this, subject.getPrincipals()); } + @Nonnull + private ImmutableTree.TypeProvider getTypeProvider() { + return new ImmutableTree.DefaultTypeProvider(securityProvider.getAccessControlConfiguration().getContext()); + } + + @Nonnull + private NodeState getSecureHead() { + return new SecureNodeState(branch.getHead(), getPermissionProvider(), getTypeProvider()); + } + + @Nonnull + private NodeState getRootState() { + // FIXME: should not return a state being (based on) SecureNodeState (see OAK-709) + return rootTree.getNodeState(); + } + //---------------------------------------------------------< MoveRecord >--- /** Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java?rev=1465011&r1=1465010&r2=1465011&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core/TreeImpl.java Fri Apr 5 15:34:21 2013 @@ -18,16 +18,9 @@ */ package org.apache.jackrabbit.oak.core; -import static com.google.common.base.Preconditions.checkArgument; -import static com.google.common.base.Preconditions.checkNotNull; -import static com.google.common.base.Preconditions.checkState; -import static org.apache.jackrabbit.oak.api.Type.STRING; -import static org.apache.jackrabbit.oak.commons.PathUtils.elements; - import java.util.Collections; import java.util.Iterator; import java.util.Set; - import javax.annotation.CheckForNull; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -45,12 +38,17 @@ import org.apache.jackrabbit.oak.commons import org.apache.jackrabbit.oak.core.RootImpl.Move; import org.apache.jackrabbit.oak.plugins.memory.MemoryPropertyBuilder; import org.apache.jackrabbit.oak.plugins.memory.MultiStringPropertyState; -import org.apache.jackrabbit.oak.spi.security.authorization.permission.ReadStatus; import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateUtils; import org.apache.jackrabbit.oak.spi.state.PropertyBuilder; +import static com.google.common.base.Preconditions.checkArgument; +import static com.google.common.base.Preconditions.checkNotNull; +import static com.google.common.base.Preconditions.checkState; +import static org.apache.jackrabbit.oak.api.Type.STRING; +import static org.apache.jackrabbit.oak.commons.PathUtils.elements; + public class TreeImpl implements Tree { /** @@ -81,8 +79,6 @@ public class TreeImpl implements Tree { /** Pointer into the list of pending moves */ private Move pendingMoves; - private ReadStatus readStatus = null; - TreeImpl(RootImpl root, Move pendingMoves) { this.root = checkNotNull(root); this.name = ""; @@ -96,12 +92,6 @@ public class TreeImpl implements Tree { this.name = checkNotNull(name); this.nodeBuilder = parent.getNodeBuilder().child(name); this.pendingMoves = checkNotNull(pendingMoves); - - if (getBaseState().exists()) { - readStatus = ReadStatus.getChildStatus(parent.readStatus); - } else { - readStatus = ReadStatus.ALLOW_ALL; // new items are always readable - } } @Override @@ -565,30 +555,13 @@ public class TreeImpl implements Tree { } private boolean canRead(TreeImpl tree) { - // FIXME: access control eval must have full access to the tree - // FIXME: special handling for access control item and version content - if (tree.readStatus == null) { - tree.readStatus = root.getPermissionProvider().getReadStatus(tree, null); - } - return tree.readStatus.includes(ReadStatus.ALLOW_THIS); + // TODO: OAK-753 TreeImpl exposes hidden child trees + // return tree.getNodeState().exists() && !NodeStateUtils.isHidden(tree.getName()); + return tree.getNodeState().exists(); } private boolean canRead(PropertyState property) { - // FIXME: access control eval must have full access to the tree/property - // FIXME: special handling for access control item and version content - if (property == null || NodeStateUtils.isHidden(property.getName())) { - return false; - } - if (readStatus == null || readStatus.appliesToThis()) { - ReadStatus rs = root.getPermissionProvider().getReadStatus(this, property); - if (rs.appliesToThis()) { - // status applies to this property only -> recalc for others - return rs.isAllow(); - } else { - readStatus = rs; - } - } - return readStatus.includes(ReadStatus.ALLOW_PROPERTIES); + return property != null && !NodeStateUtils.isHidden(property.getName()); } /** Modified: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java?rev=1465011&r1=1465010&r2=1465011&view=diff ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java (original) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/kernel/KernelNodeState.java Fri Apr 5 15:34:21 2013 @@ -18,11 +18,6 @@ */ package org.apache.jackrabbit.oak.kernel; -import static com.google.common.base.Preconditions.checkNotNull; -import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; -import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; -import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; - import java.lang.reflect.InvocationHandler; import java.lang.reflect.Method; import java.lang.reflect.Proxy; @@ -34,7 +29,6 @@ import java.util.List; import java.util.Map; import java.util.Set; import java.util.concurrent.ExecutionException; - import javax.annotation.Nonnull; import javax.jcr.PropertyType; @@ -63,6 +57,12 @@ import org.apache.jackrabbit.oak.spi.sta import org.apache.jackrabbit.oak.spi.state.NodeBuilder; import org.apache.jackrabbit.oak.spi.state.NodeState; import org.apache.jackrabbit.oak.spi.state.NodeStateDiff; +import org.apache.jackrabbit.oak.spi.state.SecureNodeState; + +import static com.google.common.base.Preconditions.checkNotNull; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.EMPTY_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.EmptyNodeState.MISSING_NODE; +import static org.apache.jackrabbit.oak.plugins.memory.PropertyStates.createProperty; /** * Basic {@link NodeState} implementation based on the {@link MicroKernel} @@ -369,6 +369,11 @@ public final class KernelNodeState exten */ @Override public boolean equals(Object object) { + // FIXME: temporary solution (see discussion in OAK-709) + if (object instanceof SecureNodeState) { + return object.equals(this); + } + if (this == object) { return true; } else if (object instanceof KernelNodeState) { Added: jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/SecureNodeState.java URL: http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/SecureNodeState.java?rev=1465011&view=auto ============================================================================== --- jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/SecureNodeState.java (added) +++ jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/spi/state/SecureNodeState.java Fri Apr 5 15:34:21 2013 @@ -0,0 +1,218 @@ +/* + * 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.spi.state; + +import java.util.Collections; +import javax.annotation.CheckForNull; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import com.google.common.base.Function; +import com.google.common.base.Predicates; +import com.google.common.collect.Iterables; +import org.apache.jackrabbit.oak.api.PropertyState; +import org.apache.jackrabbit.oak.core.ImmutableTree; +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; + +/** + * SecureNodeState... + * + * TODO: clarify if HIDDEN items should be filtered by this NodeState implementation + * TODO: add proper implementation for getPropertyCount and getChildrenCount + * TODO: clarify usage of ReadStatus in getChildNodeEntries + * TODO: add proper equals/hashcode implementation + */ +public class SecureNodeState extends AbstractNodeState { + + private final ImmutableTree base; + private final PermissionProvider permissionProvider; + + private ReadStatus readStatus; + + public SecureNodeState(@Nonnull NodeState rootState, + @Nonnull PermissionProvider permissionProvider, + @Nonnull ImmutableTree.TypeProvider typeProvider) { + this.base = new ImmutableTree(rootState, typeProvider); + this.permissionProvider = permissionProvider; + } + + private SecureNodeState(@Nonnull SecureNodeState parent, @Nonnull String name, + @Nonnull NodeState nodeState) { + this.base = new ImmutableTree(parent.base, name, nodeState); + this.permissionProvider = parent.permissionProvider; + if (base.getType() == parent.base.getType()) { + this.readStatus = (ReadStatus.getChildStatus(parent.readStatus)); + } + } + + @Override + public boolean exists() { + return getReadStatus().includes(ReadStatus.ALLOW_THIS); + } + + @Override + @CheckForNull + public PropertyState getProperty(String name) { + PropertyState property = getBaseState().getProperty(name); + if (canReadProperty(property)) { + return property; + } else { + return null; + } + } + + @Override + public long getPropertyCount() { + // TODO: make sure cnt respects read permissions (OAK-708) + return getBaseState().getPropertyCount(); + } + + @Nonnull + @Override + public Iterable getProperties() { + ReadStatus rs = getReadStatus(); + Iterable properties = getBaseState().getProperties(); + if (rs.includes(ReadStatus.ALLOW_PROPERTIES)) { + return properties; + } else if (rs.includes(ReadStatus.DENY_PROPERTIES)) { + return Collections.emptySet(); + } else { + Iterable readable = Iterables.transform(properties, new ReadableProperties()); + return Iterables.filter(readable, Predicates.notNull()); + } + } + + @Override + public boolean hasChildNode(@Nonnull String name) { + return getChildNode(name).exists(); + } + + @Override + public NodeState getChildNode(@Nonnull String name) { + NodeState child = getBaseState().getChildNode(name); + if (child.exists()) { + return new SecureNodeState(this, name, child); + } else { + // a non-existing child node + return child; + } + } + + @Override + public long getChildNodeCount() { + // TODO: make sure cnt respects read permissions (OAK-708) + return getBaseState().getChildNodeCount(); + } + + @Override + public Iterable getChildNodeNames() { + return Iterables.transform(getChildNodeEntries(), new Function() { + @Override + public String apply(@Nullable ChildNodeEntry cnEntry) { + return (cnEntry == null) ? null : cnEntry.getName(); + } + }); + } + + @Override + @Nonnull + public Iterable getChildNodeEntries() { + ReadStatus rs = getReadStatus(); + if (rs.includes(ReadStatus.DENY_CHILDREN)) { + 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 readable = Iterables.transform(getBaseState().getChildNodeEntries(), + new ReadableChildNodeEntries()); + return Iterables.filter(readable, Predicates.notNull()); + } + } + + @Nonnull + @Override + public NodeBuilder builder() { + return new MemoryNodeBuilder(this); + } + + @Override + public void compareAgainstBaseState(NodeState base, NodeStateDiff diff) { + getBaseState().compareAgainstBaseState(base, diff); + } + + //-------------------------------------------------------------------------- + private NodeState getBaseState() { + return base.getNodeState(); + } + + private ReadStatus getReadStatus() { + if (readStatus == null) { + readStatus = permissionProvider.getReadStatus(base, null); + } + return readStatus; + } + + private boolean canReadProperty(@Nullable PropertyState property) { + if (property == null) { + return false; + } + if (readStatus == null || readStatus.appliesToThis()) { + ReadStatus rs = permissionProvider.getReadStatus(this.base, property); + if (rs.appliesToThis()) { + // status applies to this property only -> recalc for others + return rs.isAllow(); + } else { + readStatus = rs; + } + } + return readStatus.includes(ReadStatus.ALLOW_PROPERTIES); + } + + private class ReadableProperties implements Function { + @Override + public PropertyState apply(PropertyState property) { + if (canReadProperty(property)) { + return property; + } else { + return null; + } + } + } + + private class ReadableChildNodeEntries implements Function { + @Override + public ChildNodeEntry apply(ChildNodeEntry input) { + String name = input.getName(); + NodeState child = new SecureNodeState(SecureNodeState.this, name, input.getNodeState()); + if (child.exists()) { + return new MemoryChildNodeEntry(name, child); + } else { + return null; + } + } + } + + //-------------------------------------------------------------< Object >--- + // FIXME: add proper equals/hashcode implementation (see OAK-709) + @Override + public boolean equals(Object obj) { + return getBaseState().equals(obj); + } +}