This is an automated email from the ASF dual-hosted git repository. rombert pushed a commit to annotated tag org.apache.sling.jcr.repoinit-1.1.2 in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-repoinit.git
commit d043d8c4cb297ed6e6a16996bb5177575a555d80 Author: Robert Munteanu <[email protected]> AuthorDate: Thu Dec 15 10:13:11 2016 +0000 SLING-6368 - Repoinit should not attempt to create access control entries when no changes are needed git-svn-id: https://svn.apache.org/repos/asf/sling/trunk/bundles/jcr/repoinit@1774410 13f79535-47bb-0310-9956-ffa450edef68 --- .../apache/sling/jcr/repoinit/impl/AclUtil.java | 83 +++++++++++- .../sling/jcr/repoinit/impl/AclUtilTest.java | 141 +++++++++++++++++++++ .../apache/sling/jcr/repoinit/impl/TestUtil.java | 8 +- 3 files changed, 227 insertions(+), 5 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 43b9445..934fa5f 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 @@ -17,22 +17,31 @@ package org.apache.sling.jcr.repoinit.impl; import java.security.Principal; +import java.util.Arrays; +import java.util.HashSet; import java.util.List; +import java.util.Set; import javax.jcr.PathNotFoundException; import javax.jcr.RepositoryException; import javax.jcr.Session; import javax.jcr.UnsupportedRepositoryOperationException; +import javax.jcr.security.AccessControlEntry; import javax.jcr.security.AccessControlManager; import javax.jcr.security.Privilege; +import org.apache.jackrabbit.api.security.JackrabbitAccessControlEntry; import org.apache.jackrabbit.api.security.JackrabbitAccessControlList; import org.apache.jackrabbit.api.security.JackrabbitAccessControlManager; import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; /** Utilities for ACL management */ public class AclUtil { + private static final Logger LOG = LoggerFactory.getLogger(AclUtil.class); + public static JackrabbitAccessControlManager getJACM(Session s) throws UnsupportedRepositoryOperationException, RepositoryException { final AccessControlManager acm = s.getAccessControlManager(); if(!(acm instanceof JackrabbitAccessControlManager)) { @@ -49,20 +58,88 @@ public class AclUtil { final String [] privArray = privileges.toArray(new String[privileges.size()]); final Privilege[] jcrPriv = AccessControlUtils.privilegesFromNames(s, privArray); - for(String path : paths) { if(!s.nodeExists(path)) { throw new PathNotFoundException("Cannot set ACL on non-existent path " + path); } JackrabbitAccessControlList acl = AccessControlUtils.getAccessControlList(s, path); + AccessControlEntry[] existingAces = acl.getAccessControlEntries(); + boolean changed = false; for (String name : principals) { final Principal principal = AccessControlUtils.getPrincipal(s, name); if (principal == null) { throw new IllegalStateException("Principal not found: " + name); } - acl.addEntry(principal, jcrPriv, isAllow); + LocalAccessControlEntry newAce = new LocalAccessControlEntry(principal, jcrPriv, isAllow); + if (contains(existingAces, newAce)) { + LOG.info("Not adding {} to path {} since an equivalent access control entry already exists", newAce, path); + continue; + } + acl.addEntry(newAce.principal, newAce.privileges, newAce.isAllow); + changed = true; + } + if ( changed ) { + getJACM(s).setPolicy(path, acl); + } + + } + } + + // visible for testing + static boolean contains(AccessControlEntry[] existingAces, LocalAccessControlEntry newAce) throws RepositoryException { + for (int i = 0 ; i < existingAces.length; i++) { + JackrabbitAccessControlEntry existingEntry = (JackrabbitAccessControlEntry) existingAces[i]; + LOG.debug("Comparing {} with {}", newAce, toString(existingEntry)); + if (newAce.isContainedIn(existingEntry)) { + return true; } - getJACM(s).setPolicy(path, acl); + } + return false; + } + + private static String toString(JackrabbitAccessControlEntry entry) throws RepositoryException { + return "[" + entry.getClass().getSimpleName() + "# principal: " + + "" + entry.getPrincipal() + ", privileges: " + Arrays.toString(entry.getPrivileges()) + + ", isAllow: " + entry.isAllow() + ", restrictionNames: " + entry.getRestrictionNames() + "]"; + } + + /** + * Helper class which allows easy comparison of a local (proposed) access control entry with an existing one + */ + static class LocalAccessControlEntry { + + private final Principal principal; + private final Privilege[] privileges; + private final boolean isAllow; + + LocalAccessControlEntry(Principal principal, Privilege[] privileges, boolean isAllow) { + this.principal = principal; + this.privileges = privileges; + this.isAllow = isAllow; + } + + public boolean isContainedIn(JackrabbitAccessControlEntry other) throws RepositoryException { + return other.getPrincipal().equals(principal) && + contains(other.getPrivileges(), privileges) && + other.isAllow() == isAllow && + ( other.getRestrictionNames() == null || + other.getRestrictionNames().length == 0 ); + } + + private boolean contains(Privilege[] first, Privilege[] second) { + // we need to ensure that the privilege order is not taken into account, so we use sets + Set<Privilege> set1 = new HashSet<Privilege>(); + set1.addAll(Arrays.asList(first)); + + Set<Privilege> set2 = new HashSet<Privilege>(); + set2.addAll(Arrays.asList(second)); + + return set1.containsAll(set2); + } + + @Override + public String toString() { + return "[" + getClass().getSimpleName() + "# principal " + principal+ ", privileges: " + Arrays.toString(privileges) + ", isAllow : " + isAllow + "]"; } } } 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 new file mode 100644 index 0000000..0fb5a42 --- /dev/null +++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/AclUtilTest.java @@ -0,0 +1,141 @@ +/* + * 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.sling.jcr.repoinit.impl; + +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; + +import java.security.Principal; + +import javax.jcr.RepositoryException; +import javax.jcr.security.Privilege; + +import org.apache.jackrabbit.api.security.JackrabbitAccessControlList; +import org.apache.jackrabbit.commons.jackrabbit.authorization.AccessControlUtils; +import org.apache.sling.repoinit.parser.RepoInitParsingException; +import org.apache.sling.testing.mock.sling.ResourceResolverType; +import org.apache.sling.testing.mock.sling.junit.SlingContext; +import org.junit.After; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; + +public class AclUtilTest { + + @Rule + public final SlingContext context = new SlingContext(ResourceResolverType.JCR_OAK); + + private TestUtil U; + + private JackrabbitAccessControlList acl; + + @Before + public void setup() throws RepositoryException, RepoInitParsingException { + U = new TestUtil(context); + U.parseAndExecute("create service user " + U.username); + + final String aclSetup = + "set ACL for " + U.username + "\n" + + "allow jcr:read on /\n" + + "allow jcr:write on /\n" + + "end" + ; + + U.parseAndExecute(aclSetup); + acl = AccessControlUtils.getAccessControlList(U.adminSession, "/"); + } + + @After + public void cleanup() throws RepositoryException, RepoInitParsingException { + U.cleanupUser(); + } + + @Test + public void entryIsContained() throws Exception { + + // validates that an exact match of the username, privilege list and isAllow is contained + + assertIsContained(acl, U.username, new String[]{ Privilege.JCR_READ, Privilege.JCR_WRITE }, true); + } + + @Test + public void entryWithFewerPrivilegesIsContained() throws Exception { + + // validates that an exact match of the username and isAllow but with fewer privileges is contained + assertIsContained(acl, U.username, new String[]{ Privilege.JCR_WRITE }, true); + } + + @Test + public void entryWithDifferentPrivilegeIsNotContained() throws Exception { + // validates that an exact match of the username and isAllow but with different privileges is contained + assertIsNotContained(acl, U.username, new String[]{ Privilege.JCR_ALL }, true); + } + + @Test + public void entryWithPartiallyMatchingPrivilegesIsNotContained() throws Exception { + // validates that an exact match of the username and isAllow but with privileges partially overlapping is contained + // existing: JCR_READ, JCR_WRITE + // new: JCR_READ, JCR_MODIFY_PROPERTIES + assertIsNotContained(acl, U.username, new String[]{ Privilege.JCR_READ, Privilege.JCR_MODIFY_PROPERTIES }, true); + } + + @Test + public void entryWithDifferentUserIsNotContained() throws Exception { + + // validates that an exact match of the privileges and isAllow but with different username is not contained + String otherPrincipalName = U.username + "_"; + try { + U.parseAndExecute("create service user " + otherPrincipalName); + assertIsNotContained(acl, otherPrincipalName, new String[]{ Privilege.JCR_READ, Privilege.JCR_WRITE }, true); + } finally { + U.cleanupServiceUser(otherPrincipalName); + } + } + + @Test + public void entryWithDifferentIsAllowIsNotContained() throws Exception { + // validates that an exact match of the username and privileges but with different is allow is not contained + assertIsNotContained(acl, U.username, new String[]{ Privilege.JCR_READ, Privilege.JCR_WRITE }, false); + } + + private void assertIsContained(JackrabbitAccessControlList acl, String username, String[] privilegeNames, boolean isAllow) throws RepositoryException { + assertIsContained0(acl, username, privilegeNames, isAllow, true); + } + + private void assertIsNotContained(JackrabbitAccessControlList acl, String username, String[] privilegeNames, boolean isAllow) throws RepositoryException { + assertIsContained0(acl, username, privilegeNames, isAllow, false); + } + + private void assertIsContained0(JackrabbitAccessControlList acl, String username, String[] privilegeNames, boolean isAllow, boolean contained) throws RepositoryException { + AclUtil.LocalAccessControlEntry localAce = new AclUtil.LocalAccessControlEntry(principal(username), privileges(privilegeNames), isAllow); + + if ( contained ) { + assertTrue("ACL does not contain an entry for " + localAce, AclUtil.contains(acl.getAccessControlEntries(), localAce)); + } else { + assertFalse("ACL contains an entry for " + localAce, AclUtil.contains(acl.getAccessControlEntries(), localAce)); + } + + } + + private Principal principal(String principalName) throws RepositoryException { + return AccessControlUtils.getPrincipal(U.adminSession, principalName); + } + + private Privilege[] privileges(String... privilegeNames) throws RepositoryException { + return AccessControlUtils.privilegesFromNames(U.adminSession, privilegeNames); + } +} \ No newline at end of file diff --git a/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java b/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java index 2ff0101..24e0ae1 100644 --- a/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java +++ b/src/test/java/org/apache/sling/jcr/repoinit/impl/TestUtil.java @@ -112,8 +112,12 @@ public class TestUtil { } public void cleanupUser() throws RepositoryException, RepoInitParsingException { - parseAndExecute("delete service user " + username); - assertServiceUser("in cleanupUser()", username, false); + cleanupServiceUser(username); + } + + public void cleanupServiceUser(String principalName) throws RepositoryException, RepoInitParsingException { + parseAndExecute("delete service user " + principalName); + assertServiceUser("in cleanupUser()", principalName, false); } public Session loginService(String serviceUsername) throws RepositoryException { -- To stop receiving notification emails like this one, please contact "[email protected]" <[email protected]>.
