From 92d0f18668e4d2441637d58f99f029a2b20945ea Mon Sep 17 00:00:00 2001 From: Jukka Zitting Date: Mon, 2 Dec 2013 13:27:00 -0500 Subject: [PATCH] OAK-1252: Lazy privilege access in AccessControlValidator Defer privilege access to when they're really needed --- .../accesscontrol/AccessControlValidator.java | 26 ++++++++++++---------- .../AccessControlValidatorProvider.java | 25 ++------------------- 2 files changed, 16 insertions(+), 35 deletions(-) diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java index d6d2b19..95ab412 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidator.java @@ -18,7 +18,6 @@ package org.apache.jackrabbit.oak.security.authorization.accesscontrol; import java.util.Collection; import java.util.Collections; -import java.util.Map; import java.util.Set; import javax.jcr.RepositoryException; @@ -30,6 +29,7 @@ import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.api.security.authorization.PrivilegeManager; import org.apache.jackrabbit.oak.api.CommitFailedException; import org.apache.jackrabbit.oak.api.PropertyState; import org.apache.jackrabbit.oak.api.Tree; @@ -61,19 +61,19 @@ class AccessControlValidator extends DefaultValidator implements AccessControlCo private final Tree parentAfter; private final PrivilegeBitsProvider privilegeBitsProvider; - private final Map privileges; + private final PrivilegeManager privilegeManager; private final RestrictionProvider restrictionProvider; private final ReadOnlyNodeTypeManager ntMgr; AccessControlValidator(Tree parentBefore, Tree parentAfter, - Map privileges, + PrivilegeManager privilegeManager, PrivilegeBitsProvider privilegeBitsProvider, RestrictionProvider restrictionProvider, ReadOnlyNodeTypeManager ntMgr) { this.parentBefore = parentBefore; this.parentAfter = parentAfter; this.privilegeBitsProvider = privilegeBitsProvider; - this.privileges = privileges; + this.privilegeManager = privilegeManager; this.restrictionProvider = restrictionProvider; this.ntMgr = ntMgr; } @@ -109,7 +109,7 @@ class AccessControlValidator extends DefaultValidator implements AccessControlCo Tree treeAfter = checkNotNull(parentAfter.getChild(name)); checkValidTree(parentAfter, treeAfter, after); - return new AccessControlValidator(null, treeAfter, privileges, privilegeBitsProvider, restrictionProvider, ntMgr); + return new AccessControlValidator(null, treeAfter, privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr); } @Override @@ -118,7 +118,7 @@ class AccessControlValidator extends DefaultValidator implements AccessControlCo Tree treeAfter = checkNotNull(parentAfter.getChild(name)); checkValidTree(parentAfter, treeAfter, after); - return new AccessControlValidator(treeBefore, treeAfter, privileges, privilegeBitsProvider, restrictionProvider, ntMgr); + return new AccessControlValidator(treeBefore, treeAfter, privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr); } @Override @@ -220,13 +220,15 @@ class AccessControlValidator extends DefaultValidator implements AccessControlCo throw accessViolation(9, "Missing privileges."); } for (String privilegeName : privilegeNames) { - if (privilegeName == null || !privileges.containsKey(privilegeName)) { + try { + Privilege privilege = privilegeManager.getPrivilege(privilegeName); + if (privilege.isAbstract()) { + throw accessViolation(11, "Abstract privilege " + privilegeName); + } + } catch (AccessControlException e) { throw accessViolation(10, "Invalid privilege " + privilegeName); - } - - Privilege privilege = privileges.get(privilegeName); - if (privilege.isAbstract()) { - throw accessViolation(11, "Abstract privilege " + privilegeName); + } catch (RepositoryException e) { + throw new RuntimeException(e); } } } diff --git a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java index 0230993..953fd15 100644 --- a/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java +++ b/oak-core/src/main/java/org/apache/jackrabbit/oak/security/authorization/accesscontrol/AccessControlValidatorProvider.java @@ -16,12 +16,8 @@ */ package org.apache.jackrabbit.oak.security.authorization.accesscontrol; -import java.util.Map; import javax.annotation.Nonnull; -import javax.jcr.RepositoryException; -import javax.jcr.security.Privilege; -import com.google.common.collect.ImmutableMap; import org.apache.jackrabbit.api.security.authorization.PrivilegeManager; import org.apache.jackrabbit.oak.api.Root; import org.apache.jackrabbit.oak.api.Tree; @@ -37,8 +33,6 @@ import org.apache.jackrabbit.oak.spi.security.authorization.restriction.Restrict import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeBitsProvider; import org.apache.jackrabbit.oak.spi.security.privilege.PrivilegeConfiguration; import org.apache.jackrabbit.oak.spi.state.NodeState; -import org.slf4j.Logger; -import org.slf4j.LoggerFactory; /** * {@code AccessControlValidatorProvider} aimed to provide a root validator @@ -48,8 +42,6 @@ import org.slf4j.LoggerFactory; */ public class AccessControlValidatorProvider extends ValidatorProvider { - private static final Logger log = LoggerFactory.getLogger(AccessControlValidatorProvider.class); - private final SecurityProvider securityProvider; public AccessControlValidatorProvider(@Nonnull SecurityProvider securityProvider) { @@ -66,24 +58,11 @@ public class AccessControlValidatorProvider extends ValidatorProvider { RestrictionProvider restrictionProvider = getConfig(AuthorizationConfiguration.class).getRestrictionProvider(); Root root = new ImmutableRoot(before); - Map privileges = getPrivileges(root); + PrivilegeManager privilegeManager = getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root, NamePathMapper.DEFAULT); PrivilegeBitsProvider privilegeBitsProvider = new PrivilegeBitsProvider(root); ReadOnlyNodeTypeManager ntMgr = ReadOnlyNodeTypeManager.getInstance(before); - return new AccessControlValidator(rootBefore, rootAfter, privileges, privilegeBitsProvider, restrictionProvider, ntMgr); - } - - private Map getPrivileges(Root root) { - PrivilegeManager pMgr = getConfig(PrivilegeConfiguration.class).getPrivilegeManager(root, NamePathMapper.DEFAULT); - ImmutableMap.Builder privileges = ImmutableMap.builder(); - try { - for (Privilege privilege : pMgr.getRegisteredPrivileges()) { - privileges.put(privilege.getName(), privilege); - } - } catch (RepositoryException e) { - log.error("Unexpected error: failed to read privileges.", e); - } - return privileges.build(); + return new AccessControlValidator(rootBefore, rootAfter, privilegeManager, privilegeBitsProvider, restrictionProvider, ntMgr); } private T getConfig(Class configClass) { -- 1.8.3.msysgit.0