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 ju...@apache.org on 2013/04/17 09:43:12 UTC
svn commit: r1468785 - in
/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/core:
RootImpl.java SecureNodeState.java SecurityContext.java
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