You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@sling.apache.org by kw...@apache.org on 2021/12/09 09:39:15 UTC

[sling-org-apache-sling-jcr-contentloader] branch feature/cleanup-overwrite created (now 0a8c454)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a change to branch feature/cleanup-overwrite
in repository https://gitbox.apache.org/repos/asf/sling-org-apache-sling-jcr-contentloader.git.


      at 0a8c454  SLING-10983 never overwrite nodes on root level

This branch includes the following new commits:

     new 0a8c454  SLING-10983 never overwrite nodes on root level

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[sling-org-apache-sling-jcr-contentloader] 01/01: SLING-10983 never overwrite nodes on root level

Posted by kw...@apache.org.
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 <kw...@apache.org>
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