You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by tr...@apache.org on 2014/01/23 19:56:25 UTC
svn commit: r1560783 - in /jackrabbit/trunk/jackrabbit-core/src:
main/java/org/apache/jackrabbit/core/security/authorization/acl/
main/java/org/apache/jackrabbit/core/xml/
test/java/org/apache/jackrabbit/core/security/authorization/acl/
test/java/org/a...
Author: tripod
Date: Thu Jan 23 18:56:25 2014
New Revision: 1560783
URL: http://svn.apache.org/r1560783
Log:
JCR-3718 Inconsistent Principal Validation between API and Import behavior
- make principal check based on ACLProvider configuration
- adding new ACLProvider configuration parameter: "allow-unknown-principals", default: "false"
- make missing-principal bypass in AccessControlImporter based on configuration
- adding new AccessControlImporter configuration parameter: "importBehavior", default: "bestEffort"
Modified:
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEditor.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java
jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/xml/AccessControlImporter.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateEntryTest.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java
jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/xml/AccessControlImporterTest.java
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEditor.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEditor.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEditor.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLEditor.java Thu Jan 23 18:56:25 2014
@@ -68,8 +68,9 @@ public class ACLEditor extends Protected
*/
private final SessionImpl session;
private final AccessControlUtils utils;
+ private final boolean allowUnknownPrincipals;
- ACLEditor(Session editingSession, AccessControlUtils utils) {
+ ACLEditor(Session editingSession, AccessControlUtils utils, boolean allowUnknownPrincipals) {
super(Permission.MODIFY_AC);
if (editingSession instanceof SessionImpl) {
session = ((SessionImpl) editingSession);
@@ -77,6 +78,7 @@ public class ACLEditor extends Protected
throw new IllegalArgumentException("org.apache.jackrabbit.core.SessionImpl expected. Found " + editingSession.getClass());
}
this.utils = utils;
+ this.allowUnknownPrincipals = allowUnknownPrincipals;
}
/**
@@ -87,7 +89,7 @@ public class ACLEditor extends Protected
* @throws RepositoryException if an error occurs
*/
ACLTemplate getACL(NodeImpl aclNode, String path) throws RepositoryException {
- return new ACLTemplate(aclNode, path);
+ return new ACLTemplate(aclNode, path, allowUnknownPrincipals);
}
//------------------------------------------------< AccessControlEditor >---
@@ -151,7 +153,7 @@ public class ACLEditor extends Protected
PrivilegeManager privMgr = ((JackrabbitWorkspace) session.getWorkspace()).getPrivilegeManager();
if (controlledNode.isNodeType(mixin) || controlledNode.canAddMixin(mixin)) {
acl = new ACLTemplate(nodePath, session.getPrincipalManager(),
- privMgr, session.getValueFactory(), session);
+ privMgr, session.getValueFactory(), session, allowUnknownPrincipals);
} else {
log.warn("Node {} cannot be made access controllable.", nodePath);
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLProvider.java Thu Jan 23 18:56:25 2014
@@ -78,6 +78,17 @@ import java.util.Set;
public class ACLProvider extends AbstractAccessControlProvider implements AccessControlConstants {
/**
+ * Constant for the name of the configuration option {@code allow-unknown-principals}.
+ * The option is a flag indicating whether access control entries with principals not known to the system
+ * can be added to an ACL. the default is {@code false}.
+ * <p/>
+ * Please note that the current implementation does only check principal existence when adding a new access
+ * control entry, but does not validate all ACEs when removing a principal. So even if this flag is {@code false},
+ * it's possible to create an ACL with a unknown principal.
+ */
+ public static final String PARAM_ALLOW_UNKNOWN_PRINCIPALS = "allow-unknown-principals";
+
+ /**
* the default logger
*/
private static final Logger log = LoggerFactory.getLogger(ACLProvider.class);
@@ -94,6 +105,11 @@ public class ACLProvider extends Abstrac
*/
private EntryCollector entryCollector;
+ /**
+ * controls if unknown principals are allowed in ACLs
+ */
+ private boolean allowUnknownPrincipals;
+
//----------------------------------------------< AccessControlProvider >---
/**
* @see org.apache.jackrabbit.core.security.authorization.AccessControlProvider#init(Session, Map)
@@ -101,12 +117,13 @@ public class ACLProvider extends Abstrac
@Override
public void init(Session systemSession, Map configuration) throws RepositoryException {
super.init(systemSession, configuration);
+ allowUnknownPrincipals = "true".equals(configuration.get(PARAM_ALLOW_UNKNOWN_PRINCIPALS));
// make sure the workspace of the given systemSession has a
// minimal protection on the root node.
NodeImpl root = (NodeImpl) session.getRootNode();
rootNodeId = root.getNodeId();
- ACLEditor systemEditor = new ACLEditor(session, this);
+ ACLEditor systemEditor = new ACLEditor(session, this, allowUnknownPrincipals);
// TODO: replace by configurable default policy (see JCR-2331)
boolean initializedWithDefaults = !configuration.containsKey(PARAM_OMIT_DEFAULT_PERMISSIONS);
@@ -221,7 +238,7 @@ public class ACLProvider extends Abstrac
*/
public AccessControlEditor getEditor(Session session) {
checkInitialized();
- return new ACLEditor(session, this);
+ return new ACLEditor(session, this, allowUnknownPrincipals);
}
/**
@@ -299,7 +316,7 @@ public class ACLProvider extends Abstrac
private AccessControlList getACL(NodeImpl accessControlledNode, Name policyName, String path) throws RepositoryException {
// collect the aces of that node.
NodeImpl aclNode = accessControlledNode.getNode(policyName);
- AccessControlList acl = new ACLTemplate(aclNode, path);
+ AccessControlList acl = new ACLTemplate(aclNode, path, allowUnknownPrincipals);
return new UnmodifiableAccessControlList(acl);
}
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplate.java Thu Jan 23 18:56:25 2014
@@ -90,6 +90,11 @@ class ACLTemplate extends AbstractACLTem
private final String jcrRepGlob;
/**
+ * controls if unknown principals can be used in access control entries.
+ */
+ private final boolean allowUnknownPrincipals;
+
+ /**
* Construct a new empty {@link ACLTemplate}.
*
* @param path path
@@ -101,11 +106,12 @@ class ACLTemplate extends AbstractACLTem
*/
ACLTemplate(String path, PrincipalManager principalMgr,
PrivilegeManager privilegeMgr, ValueFactory valueFactory,
- NamePathResolver resolver) throws NamespaceException {
+ NamePathResolver resolver, boolean allowUnknownPrincipals) throws NamespaceException {
super(path, valueFactory);
this.principalMgr = principalMgr;
this.privilegeMgr = (PrivilegeManagerImpl) privilegeMgr;
this.resolver = resolver;
+ this.allowUnknownPrincipals = allowUnknownPrincipals;
jcrRepGlob = resolver.getJCRName(P_GLOB);
}
@@ -118,7 +124,7 @@ class ACLTemplate extends AbstractACLTem
* @param path The path as exposed by {@link JackrabbitAccessControlList#getPath()}
* @throws RepositoryException if an error occurs
*/
- ACLTemplate(NodeImpl aclNode, String path) throws RepositoryException {
+ ACLTemplate(NodeImpl aclNode, String path, boolean allowUnknownPrincipals) throws RepositoryException {
super(path, (aclNode != null) ? aclNode.getSession().getValueFactory() : null);
if (aclNode == null || !NT_REP_ACL.equals(aclNode.getPrimaryNodeTypeName())) {
throw new IllegalArgumentException("Node must be of type 'rep:ACL'");
@@ -126,6 +132,7 @@ class ACLTemplate extends AbstractACLTem
SessionImpl sImpl = (SessionImpl) aclNode.getSession();
principalMgr = sImpl.getPrincipalManager();
privilegeMgr = (PrivilegeManagerImpl) ((JackrabbitWorkspace) sImpl.getWorkspace()).getPrivilegeManager();
+ this.allowUnknownPrincipals = allowUnknownPrincipals;
this.resolver = sImpl;
jcrRepGlob = sImpl.getJCRName(P_GLOB);
@@ -293,7 +300,10 @@ class ACLTemplate extends AbstractACLTem
if (principal instanceof UnknownPrincipal) {
log.debug("Consider fallback principal as valid: {}", principal.getName());
} else if (!principalMgr.hasPrincipal(principal.getName())) {
- throw new AccessControlException("Principal " + principal.getName() + " does not exist.");
+ if (!allowUnknownPrincipals) {
+ throw new AccessControlException("Principal " + principal.getName() + " does not exist.");
+ }
+ log.debug("Consider fallback principal as valid: {}", principal.getName());
}
if (path == null && restrictions != null && !restrictions.isEmpty()) {
Modified: jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/xml/AccessControlImporter.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/xml/AccessControlImporter.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/xml/AccessControlImporter.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/main/java/org/apache/jackrabbit/core/xml/AccessControlImporter.java Thu Jan 23 18:56:25 2014
@@ -44,6 +44,7 @@ import org.apache.jackrabbit.api.Jackrab
import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager;
import org.apache.jackrabbit.core.NodeImpl;
+import org.apache.jackrabbit.core.security.principal.PrincipalImpl;
import org.apache.jackrabbit.core.util.ReferenceChangeTracker;
import org.apache.jackrabbit.core.id.NodeId;
import org.apache.jackrabbit.core.security.authorization.AccessControlConstants;
@@ -89,6 +90,9 @@ public class AccessControlImporter exten
private boolean initialized = false;
+ // keep best-effort for backward compatibility reasons
+ private ImportBehavior importBehavior = ImportBehavior.BEST_EFFORT;
+
/**
* the ACL for non-principal based
*/
@@ -367,8 +371,13 @@ public class AccessControlImporter exten
String pName = values[0].getString();
principal = session.getPrincipalManager().getPrincipal(pName);
if (principal == null) {
- // create "fake" principal
- principal = new UnknownPrincipal(pName);
+ if (importBehavior == ImportBehavior.BEST_EFFORT) {
+ // create "fake" principal that is always accepted in ACLTemplate.checkValidEntry()
+ principal = new UnknownPrincipal(pName);
+ } else {
+ // create "fake" principal. this is checked again in ACLTemplate.checkValidEntry()
+ principal = new PrincipalImpl(pName);
+ }
}
} else if (AccessControlConstants.P_PRIVILEGES.equals(name)) {
Value[] values = pInfo.getValues(PropertyType.NAME, resolver);
@@ -429,4 +438,54 @@ public class AccessControlImporter exten
// could not apply the ACE. No suitable ACL found.
throw new ConstraintViolationException("Cannot handle childInfo " + childInfo + "; No policy found to apply the ACE.");
}
+
+ //---------------------------------------------------------< BeanConfig >---
+ /**
+ * @return human readable representation of the <code>importBehavior</code> value.
+ */
+ public String getImportBehavior() {
+ return importBehavior.getString();
+ }
+
+ /**
+ *
+ * @param importBehaviorStr
+ */
+ public void setImportBehavior(String importBehaviorStr) {
+ this.importBehavior = ImportBehavior.fromString(importBehaviorStr);
+ }
+
+
+ public static enum ImportBehavior {
+
+ /**
+ * Default behavior that does not try to prevent errors or incompatibilities between the content
+ * and the ACL manager (eg. does not try to fix missing principals)
+ */
+ DEFAULT("default"),
+
+ /**
+ * Tries to minimize errors by adapting the content and bypassing validation checks (e.g. allows adding
+ * ACEs with missing principals, even if ACL manager would not allow this).
+ */
+ BEST_EFFORT("bestEffort");
+
+ private final String value;
+
+ ImportBehavior(String value) {
+ this.value = value;
+ }
+
+ public static ImportBehavior fromString(String str) {
+ if (str.equals("bestEffort")) {
+ return BEST_EFFORT;
+ } else {
+ return ImportBehavior.valueOf(str.toUpperCase());
+ }
+ }
+
+ public String getString() {
+ return value;
+ }
+ }
}
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateEntryTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateEntryTest.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateEntryTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateEntryTest.java Thu Jan 23 18:56:25 2014
@@ -45,7 +45,7 @@ public class ACLTemplateEntryTest extend
SessionImpl s = (SessionImpl) superuser;
PrivilegeManager privMgr = ((JackrabbitWorkspace) superuser.getWorkspace()).getPrivilegeManager();
- acl = new ACLTemplate(testPath, s.getPrincipalManager(), privMgr, s.getValueFactory(), s);
+ acl = new ACLTemplate(testPath, s.getPrincipalManager(), privMgr, s.getValueFactory(), s, false);
}
@Override
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/security/authorization/acl/ACLTemplateTest.java Thu Jan 23 18:56:25 2014
@@ -56,7 +56,7 @@ public class ACLTemplateTest extends Abs
@Override
protected JackrabbitAccessControlList createEmptyTemplate(String path) throws RepositoryException {
SessionImpl sImpl = (SessionImpl) superuser;
- return new ACLTemplate(path, principalMgr, privilegeMgr, sImpl.getValueFactory(), sImpl);
+ return new ACLTemplate(path, principalMgr, privilegeMgr, sImpl.getValueFactory(), sImpl, false);
}
@Override
Modified: jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/xml/AccessControlImporterTest.java
URL: http://svn.apache.org/viewvc/jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/xml/AccessControlImporterTest.java?rev=1560783&r1=1560782&r2=1560783&view=diff
==============================================================================
--- jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/xml/AccessControlImporterTest.java (original)
+++ jackrabbit/trunk/jackrabbit-core/src/test/java/org/apache/jackrabbit/core/xml/AccessControlImporterTest.java Thu Jan 23 18:56:25 2014
@@ -45,6 +45,7 @@ import javax.jcr.security.AccessControlP
import javax.jcr.security.Privilege;
import java.io.ByteArrayInputStream;
+import java.io.IOException;
import java.io.InputStream;
import java.util.Arrays;
import java.util.Collections;
@@ -476,6 +477,33 @@ public class AccessControlImporterTest e
}
/**
+ * Imports a resource-based ACL containing a single entry.
+ *
+ * @throws Exception
+ */
+ public void testImportACLUnknownFail() throws Exception {
+ try {
+ NodeImpl target = (NodeImpl) testRootNode.addNode(nodeName1);
+ target.addMixin("rep:AccessControllable");
+
+ InputStream in = new ByteArrayInputStream(XML_POLICY_TREE_4.getBytes("UTF-8"));
+ PseudoConfig config = new PseudoConfig();
+ ((AccessControlImporter) config.getProtectedItemImporters().get(0)).setImportBehavior("default");
+ SessionImporter importer = new SessionImporter(target, sImpl,
+ ImportUUIDBehavior.IMPORT_UUID_COLLISION_THROW, config);
+ ImportHandler ih = new ImportHandler(importer, sImpl);
+ try {
+ new ParsingContentHandler(ih).parse(in);
+ fail("importing unknown principal should fail based on configuration.");
+ } catch (Exception e) {
+ // ok
+ }
+ } finally {
+ superuser.refresh(false);
+ }
+ }
+
+ /**
* Imports a resource-based ACL containing a single entry for a policy that
* already exists.
*