This is an automated email from the ASF dual-hosted git repository. kwin pushed a commit to branch feature/cleanup-overwrite in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-contentloader.git
commit 0a8c454d2e3be4818f6813696754589671a6c39c Author: Konrad Windszus <[email protected]> AuthorDate: Thu Dec 9 10:39:05 2021 +0100 SLING-10983 never overwrite nodes on root level SLING-10986 overwrite with a path directive should also overwite node at path itself --- .../internal/BundleContentLoader.java | 22 +++- .../internal/BundleContentLoaderTest.java | 111 ++++++++++++++++++++- .../SLING-INF2/apps/sling/validation/content.json | 3 + 3 files changed, 131 insertions(+), 5 deletions(-) diff --git a/src/main/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoader.java b/src/main/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoader.java index 8bbb26e..da76713 100644 --- a/src/main/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoader.java +++ b/src/main/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoader.java @@ -260,6 +260,10 @@ public class BundleContentLoader extends BaseImportLoader { continue; } + if (pathEntry.isOverwrite() && (pathEntry.getTarget() == null || "/".equals(pathEntry.getTarget()))) { + log.error("Path {} tries to overwrite on the repository root level which is not allowed, only use overwrite with a dedicated path directive having any value but '/'", pathEntry.getPath()); + continue; + } if (!contentAlreadyLoaded || pathEntry.isOverwrite()) { String workspace = pathEntry.getWorkspace(); final Session targetSession; @@ -274,7 +278,7 @@ public class BundleContentLoader extends BaseImportLoader { targetSession = defaultSession; } - final Node targetNode = getTargetNode(targetSession, pathEntry.getTarget()); + final Node targetNode = getTargetNode(targetSession, pathEntry.getTarget(), pathEntry.isOverwrite()); if (targetNode != null) { installFromPath(bundle, pathEntry.getPath(), pathEntry, targetNode, @@ -635,7 +639,7 @@ public class BundleContentLoader extends BaseImportLoader { return name; } - private Node getTargetNode(Session session, String path) throws RepositoryException { + private Node getTargetNode(Session session, String path, boolean overwrite) throws RepositoryException { // not specified path directive if (path == null) { @@ -658,9 +662,19 @@ public class BundleContentLoader extends BaseImportLoader { currentNode = currentNode.getNode(name); } return currentNode; + } else { + Item item = session.getItem(path); + if (!item.isNode()) { + log.warn("Cannot overwrite item at path {} as this is an existing property", path); + return null; + } + Node targetNode = session.getNode(path); + // overwrite target node itself? + if (overwrite) { + targetNode = createFolder(targetNode.getParent(), targetNode.getName(), true); + } + return targetNode; } - Item item = session.getItem(path); - return (item.isNode()) ? (Node) item : null; } private void uninstallContent(final Session defaultSession, final Bundle bundle, final String[] uninstallPaths) { diff --git a/src/test/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoaderTest.java b/src/test/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoaderTest.java index 6f6303c..fc5bb3c 100644 --- a/src/test/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoaderTest.java +++ b/src/test/java/org/apache/sling/jcr/contentloader/internal/BundleContentLoaderTest.java @@ -22,19 +22,42 @@ import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.notNullValue; import static org.hamcrest.CoreMatchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; import static org.junit.Assert.assertThat; +import static org.junit.Assert.assertTrue; import java.lang.annotation.Annotation; +import java.security.Principal; +import javax.jcr.AccessDeniedException; +import javax.jcr.NamespaceException; +import javax.jcr.RepositoryException; import javax.jcr.Session; - +import javax.jcr.Workspace; +import javax.jcr.nodetype.NodeType; +import javax.jcr.security.AccessControlEntry; +import javax.jcr.security.AccessControlException; +import javax.jcr.security.AccessControlList; +import javax.jcr.security.AccessControlManager; +import javax.jcr.security.Privilege; + +import org.apache.jackrabbit.JcrConstants; +import org.apache.jackrabbit.api.JackrabbitWorkspace; +import org.apache.jackrabbit.api.security.authorization.PrivilegeManager; +import org.apache.jackrabbit.commons.JcrUtils; +import org.apache.jackrabbit.oak.spi.security.principal.EveryonePrincipal; import org.apache.sling.api.resource.Resource; import org.apache.sling.jcr.contentloader.internal.readers.JsonReader; import org.apache.sling.jcr.contentloader.internal.readers.XmlReader; import org.apache.sling.jcr.contentloader.internal.readers.ZipReader; +import org.apache.sling.jcr.resource.internal.helper.JcrResourceUtil; import org.apache.sling.testing.mock.osgi.MockBundle; import org.apache.sling.testing.mock.sling.ResourceResolverType; import org.apache.sling.testing.mock.sling.junit.SlingContext; +import org.hamcrest.MatcherAssert; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Ignore; import org.junit.Rule; @@ -50,6 +73,7 @@ public class BundleContentLoaderTest { private ContentReaderWhiteboard whiteboard; + private static final String CUSTOM_PRIVILEGE_NAME = "customPrivilege"; @Before public void prepareContentLoader() throws Exception { // prepare content readers @@ -67,6 +91,91 @@ public class BundleContentLoaderTest { } + private static Privilege getOrRegisterCustomPrivilege(Session session) throws AccessDeniedException, NamespaceException, RepositoryException { + // register custom privilege + Workspace wsp = session.getWorkspace(); + if (!(wsp instanceof JackrabbitWorkspace)) { + throw new RepositoryException("Unable to register privileges. No JackrabbitWorkspace."); + } + PrivilegeManager mgr = ((JackrabbitWorkspace) wsp).getPrivilegeManager(); + try { + return mgr.getPrivilege(CUSTOM_PRIVILEGE_NAME); + } catch (AccessControlException e) { + return mgr.registerPrivilege(CUSTOM_PRIVILEGE_NAME, false, new String[0]); + } + } + + private AccessControlEntry[] createFolderNodeAndACL(String path) throws RepositoryException { + // use JCR API to add some node and acl + Session session = context.resourceResolver().adaptTo(Session.class); + + Privilege customPrivilege = getOrRegisterCustomPrivilege(session); + Privilege[] customPrivilegeSingleItemArray = new Privilege[]{ customPrivilege }; + + JcrUtils.getOrCreateByPath(path, NodeType.NT_FOLDER, session); + + AccessControlManager acMgr = session.getAccessControlManager(); + AccessControlList acl = (AccessControlList)acMgr.getApplicablePolicies(path).nextAccessControlPolicy(); + Principal everyone = EveryonePrincipal.getInstance(); + assertTrue(acl.addAccessControlEntry(everyone, customPrivilegeSingleItemArray)); + AccessControlEntry[] expectedAces = acl.getAccessControlEntries(); + acMgr.setPolicy(path, acl); + session.save(); + return expectedAces; + } + + private void assertFolderNodeAndACL(String path, AccessControlEntry[] expectedAces) throws RepositoryException { + Session session = context.resourceResolver().adaptTo(Session.class); + session.refresh(false); + assertTrue(session.nodeExists(path)); + assertEquals(JcrConstants.NT_FOLDER, session.getNode(path).getPrimaryNodeType().getName()); + AccessControlManager acMgr = session.getAccessControlManager(); + AccessControlList acl = (AccessControlList)acMgr.getPolicies(path)[0]; + AccessControlEntry[] aces = acl.getAccessControlEntries(); + MatcherAssert.assertThat(aces, Matchers.arrayContaining(expectedAces)); + } + + @Test + public void loadContentOverwriteWithoutPath() throws Exception { + AccessControlEntry[] expectedAces = createFolderNodeAndACL("/apps/child"); + + // import without path + BundleContentLoader contentLoader = new BundleContentLoader(bundleHelper, whiteboard, null); + Bundle mockBundle = newBundleWithInitialContent(context, "SLING-INF2;overwrite:=true"); + contentLoader.registerBundle(context.resourceResolver().adaptTo(Session.class), mockBundle, false); + Resource imported = context.resourceResolver().getResource("/apps/sling/validation/content"); + assertNull("Resource was unexpectedly imported", imported); + assertFolderNodeAndACL("/apps/child", expectedAces); + } + + @Test + public void loadContentOverwriteWithRootPath() throws Exception { + AccessControlEntry[] expectedAces = createFolderNodeAndACL("/apps/child"); + + // import without path + BundleContentLoader contentLoader = new BundleContentLoader(bundleHelper, whiteboard, null); + Bundle mockBundle = newBundleWithInitialContent(context, "SLING-INF2;overwrite:=true;path:=/"); + contentLoader.registerBundle(context.resourceResolver().adaptTo(Session.class), mockBundle, false); + Resource imported = context.resourceResolver().getResource("/apps/sling/validation/content"); + assertNull("Resource was unexpectedly imported", imported); + assertFolderNodeAndACL("/apps/child", expectedAces); + } + + @Test + public void loadContentOverwriteWith2ndLevelPath() throws Exception { + AccessControlEntry[] expectedAces = createFolderNodeAndACL("/apps/child"); + + // import without path + BundleContentLoader contentLoader = new BundleContentLoader(bundleHelper, whiteboard, null); + Bundle mockBundle = newBundleWithInitialContent(context, "SLING-INF2/apps;overwrite:=true;path:=/apps"); + contentLoader.registerBundle(context.resourceResolver().adaptTo(Session.class), mockBundle, false); + Resource imported = context.resourceResolver().getResource("/apps/sling/validation/content"); + + assertThat("Resource was not imported", imported, notNullValue()); + assertThat("sling:resourceType was not properly set", imported.getResourceType(), equalTo("sling:Folder")); + assertNull(context.resourceResolver().getResource("/apps/child")); + } + @Test public void loadContentWithSpecificPath() throws Exception { diff --git a/src/test/resources/SLING-INF2/apps/sling/validation/content.json b/src/test/resources/SLING-INF2/apps/sling/validation/content.json new file mode 100644 index 0000000..681774d --- /dev/null +++ b/src/test/resources/SLING-INF2/apps/sling/validation/content.json @@ -0,0 +1,3 @@ +{ + "jcr:primaryType" : "sling:Folder" +} \ No newline at end of file
