You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by ro...@apache.org on 2019/09/17 12:43:01 UTC
[sling-org-apache-sling-jcr-repoinit] branch master updated:
SLING-8604 - AclUtil.setAcl: invalid assumptions regarding principal lookup
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 <an...@apache.org>
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);
}