This is an automated email from the ASF dual-hosted git repository.
rombert pushed a commit to branch master
in repository
https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
The following commit(s) were added to refs/heads/master by this push:
new bdad3db SLING-8604 - AclUtil.setAcl: invalid assumptions regarding
principal lookup
bdad3db is described below
commit bdad3dbe22b13952209db113bf651e1b1e612d66
Author: Angela Schreiber <[email protected]>
AuthorDate: Tue Sep 17 14:37:41 2019 +0200
SLING-8604 - AclUtil.setAcl: invalid assumptions regarding principal lookup
---
.../apache/sling/jcr/repoinit/impl/AclUtil.java | 31 +++++----
.../sling/jcr/repoinit/impl/AclUtilTest.java | 77 ++++++++++++++++++++++
2 files changed, 92 insertions(+), 16 deletions(-)
diff --git a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
index 01c5d80..1adb0e7 100644
--- a/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
+++ b/src/main/java/org/apache/sling/jcr/repoinit/impl/AclUtil.java
@@ -24,6 +24,7 @@ import java.util.List;
import java.util.Map;
import java.util.Set;
+import javax.annotation.Nullable;
import javax.jcr.PathNotFoundException;
import javax.jcr.RepositoryException;
import javax.jcr.Session;
@@ -49,11 +50,7 @@ public class AclUtil {
private static final Logger LOG = LoggerFactory.getLogger(AclUtil.class);
public static JackrabbitAccessControlManager getJACM(Session s) throws
RepositoryException {
final AccessControlManager acm = s.getAccessControlManager();
- if(!(acm instanceof JackrabbitAccessControlManager)) {
- throw new IllegalStateException(
- "AccessControlManager is not a JackrabbitAccessControlManager:"
- + acm.getClass().getName());
- }
+ checkState((acm instanceof JackrabbitAccessControlManager),
"AccessControlManager is not a JackrabbitAccessControlManager:" +
acm.getClass().getName());
return (JackrabbitAccessControlManager) acm;
}
@@ -119,6 +116,7 @@ public class AclUtil {
final Privilege[] jcrPriv =
AccessControlUtils.privilegesFromNames(session, privArray);
JackrabbitAccessControlList acl =
AccessControlUtils.getAccessControlList(session, path);
+ checkState(acl != null, "No JackrabbitAccessControlList available for
path " + path);
LocalRestrictions localRestrictions =
createLocalRestrictions(restrictionClauses, acl, session);
@@ -126,19 +124,14 @@ public class AclUtil {
boolean changed = false;
for (String name : principals) {
- final Principal principal;
- if (EveryonePrincipal.NAME.equals(name)) {
- principal = AccessControlUtils.getPrincipal(session, name);
- } else {
+ Principal principal = AccessControlUtils.getPrincipal(session,
name);
+ if (principal == null) {
+ // backwards compatibility: fallback to original code treating
principal name as authorizable ID (see SLING-8604)
final Authorizable authorizable =
UserUtil.getAuthorizable(session, name);
- if (authorizable == null) {
- throw new IllegalStateException("Authorizable not found:"
+ name);
- }
+ checkState(authorizable != null, "Authorizable not found:" +
name);
principal = authorizable.getPrincipal();
}
- if (principal == null) {
- throw new IllegalStateException("Principal not found: " +
name);
- }
+ checkState(principal != null, "Principal not found: " + name);
LocalAccessControlEntry newAce = new
LocalAccessControlEntry(principal, jcrPriv, isAllow, localRestrictions);
if (contains(existingAces, newAce)) {
LOG.info("Not adding {} to path {} since an equivalent access
control entry already exists", newAce, path);
@@ -149,7 +142,7 @@ public class AclUtil {
changed = true;
}
if ( changed ) {
- getJACM(session).setPolicy(path, acl);
+ session.getAccessControlManager().setPolicy(path, acl);
}
}
@@ -176,6 +169,12 @@ public class AclUtil {
", isAllow: " + entry.isAllow() + ", restrictionNames: " +
entry.getRestrictionNames() + "]";
}
+ private static void checkState(boolean expression, @Nullable String msg) {
+ if (!expression) {
+ throw new IllegalStateException(msg);
+ }
+ }
+
/** Compare arrays a and b, which do not need to be ordered
* but are expected to be small.
* @param a might be sorted by this method
diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
index 782b68b..8ea5f38 100644
--- a/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
+++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java
@@ -17,15 +17,23 @@
package org.apache.sling.jcr.repoinit.impl;
import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
import java.security.Principal;
+import java.util.Collections;
import javax.jcr.RepositoryException;
+import javax.jcr.Session;
import javax.jcr.security.Privilege;
+import org.apache.jackrabbit.api.JackrabbitSession;
import org.apache.jackrabbit.api.security.JackrabbitAccessControlList;
+import org.apache.jackrabbit.api.security.user.UserManager;
import
org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils;
+import org.apache.jackrabbit.oak.commons.PathUtils;
+import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal;
+import org.apache.jackrabbit.oak.spi.security.principal.PrincipalImpl;
import org.apache.sling.repoinit.parser.RepoInitParsingException;
import org.apache.sling.testing.mock.sling.ResourceResolverType;
import org.apache.sling.testing.mock.sling.junit.SlingContext;
@@ -169,6 +177,75 @@ public class AclUtilTest {
assertArrayCompare(emptyA, emptyB, true);
}
+ /**
+ * Repo init should work for principals even if they are not present as
user/group with the user manager.
+ *
+ * @see <a
href="https://issues.apache.org/jira/browse/SLING-8604";>SLING-8604</>
+ */
+ @Test
+ public void testSetAclForEveryone() throws Exception {
+ Session session = U.adminSession;
+ try {
+ assertTrue(session instanceof JackrabbitSession);
+ assertNull(((JackrabbitSession)
session).getUserManager().getAuthorizable(EveryonePrincipal.getInstance()));
+
+ AclUtil.setAcl(session,
Collections.singletonList(EveryonePrincipal.NAME),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ), false);
+
+ assertIsContained(AccessControlUtils.getAccessControlList(session,
PathUtils.ROOT_PATH), EveryonePrincipal.NAME, new String[]
{Privilege.JCR_READ}, false);
+ } finally {
+ session.refresh(false);
+ }
+ }
+
+ /**
+ * @see <a
href="https://issues.apache.org/jira/browse/SLING-8604";>SLING-8604</>
+ */
+ @Test
+ public void testSetAclForPrincipalNameDifferentFromId() throws Exception {
+ U.cleanupUser();
+ try {
+ Principal principal = new PrincipalImpl(U.id + "_principalName");
+ assertTrue(U.adminSession instanceof JackrabbitSession);
+ UserManager uMgr = ((JackrabbitSession)
U.adminSession).getUserManager();
+ uMgr.createUser(U.username, null, principal, null);
+ U.adminSession.save();
+
+ AclUtil.setAcl(U.adminSession,
Collections.singletonList(principal.getName()),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ), false);
+
+
assertIsContained(AccessControlUtils.getAccessControlList(U.adminSession,
PathUtils.ROOT_PATH), principal.getName(), new String[] {Privilege.JCR_READ},
false);
+ } finally {
+ U.adminSession.refresh(false);
+ }
+ }
+
+ /**
+ * Test backwards compatibility of SLING-8604: test that the fallback
mechanism still tries to
+ * resolve the given 'principalName' to an authorizable by treating it as
the identifier, if there exists no principal
+ * for the given name (i.e ID is not equal to the principal name.
+ *
+ * @see <a
href="https://issues.apache.org/jira/browse/SLING-8604";>SLING-8604</>
+ */
+ @Test
+ public void testSetAclUsingUserId() throws Exception {
+ U.cleanupUser();
+ try {
+ Principal principal = new PrincipalImpl(U.id + "_principalName");
+ assertTrue(U.adminSession instanceof JackrabbitSession);
+ UserManager uMgr = ((JackrabbitSession)
U.adminSession).getUserManager();
+ uMgr.createUser(U.username, null, principal, null);
+ U.adminSession.save();
+
+ AclUtil.setAcl(U.adminSession,
Collections.singletonList(U.username),
Collections.singletonList(PathUtils.ROOT_PATH),
Collections.singletonList(Privilege.JCR_READ), false);
+
+ // assert that the entries is created for the principal name (and
not the identifier)
+
assertIsContained(AccessControlUtils.getAccessControlList(U.adminSession,
PathUtils.ROOT_PATH), principal.getName(), new String[] {Privilege.JCR_READ},
false);
+ // assert no entry has been created for the identifier (which
cannot be resolved to a principal)
+
assertIsNotContained(AccessControlUtils.getAccessControlList(U.adminSession,
PathUtils.ROOT_PATH), U.username, new String[] {Privilege.JCR_READ}, false);
+ } finally {
+ U.adminSession.refresh(false);
+ }
+ }
+
private void assertIsContained(JackrabbitAccessControlList acl, String
username, String[] privilegeNames, boolean isAllow) throws RepositoryException {
assertIsContained0(acl, username, privilegeNames, isAllow, true);
}
