fix: allow removing under conditions files owned by other users from personal drives - EXO-89504

documents-api/src/main/java/org/exoplatform/documents/storage/JCRDeleteFileStorage.java

@@ -42,8 +42,9 @@ public interface JCRDeleteFileStorage {
    * @param delay
    * @param acIdentity
    * @param userIdentityId
+   * @throws IllegalAccessException if the acting user may not move this document to trash
    */
-  void deleteDocument(String documentPath, String documentId, boolean favorite, boolean checkToMoveToTrash, long delay, Identity acIdentity, long userIdentityId);
+  void deleteDocument(String documentPath, String documentId, boolean favorite, boolean checkToMoveToTrash, long delay, Identity acIdentity, long userIdentityId) throws IllegalAccessException;
 
   /**
    * Undo delete document

documents-services/src/test/java/org/exoplatform/documents/rest/DocumentFileRestTest.java

@@ -891,7 +891,7 @@ public void testCreateFolder() throws Exception {
   }
 
   @Test
-  public void testDeleteDocument() {
+  public void testDeleteDocument() throws IllegalAccessException {
     String username = "testuser";
     org.exoplatform.services.security.Identity root = new org.exoplatform.services.security.Identity(username);
     ConversationState.setCurrent(new ConversationState(root));

documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageImpl.java

@@ -131,7 +131,7 @@ public Map<String, String> getDocumentsToDelete(){
     return documentsToDeleteQueue;
   }
   @Override
-  public void deleteDocument(String folderPath, String documentId, boolean favorite, boolean checkToMoveToTrash, long delay, Identity identity, long userIdentityId) {
+  public void deleteDocument(String folderPath, String documentId, boolean favorite, boolean checkToMoveToTrash, long delay, Identity identity, long userIdentityId) throws IllegalAccessException {
     SessionProvider sessionProvider = null;
     try {
       ManageableRepository manageableRepository = repositoryService.getCurrentRepository();
@@ -140,6 +140,11 @@ public void deleteDocument(String folderPath, String documentId, boolean favorit
       deleteDocument(session, folderPath, documentId, favorite, checkToMoveToTrash, delay, identity, userIdentityId);
     } catch (PathNotFoundException path) {
       LOG.error("The document with this path is not found" + folderPath, path);
+    } catch (AccessDeniedException accessDenied) {
+      // Surfaced rather than logged-and-ignored: a permission refusal must reach the REST
+      // layer as a real 401, not as a silent no-op "success".
+      throw new IllegalAccessException("User " + identity.getUserId() + " is not allowed to delete document "
+          + documentId);
     } catch (Exception e) {
       LOG.error("Error when deleting the document" + folderPath, e);
     }
@@ -394,6 +399,22 @@ private void processRemoveNode(Node node)
       }
       node.remove();
       parentNode.save();
+    } catch (AccessDeniedException e) {
+      // Not swallowed like the other failure modes here: this method returning normally
+      // is what makes the caller report "0" (node removed), so a permission refusal has
+      // to surface as a real error instead of a silent false success.
+      if (LOG.isErrorEnabled()) {
+        LOG.error("access denied, can't remove node:" + node.getPath());
+      }
+      throw e;
+    } catch (AccessControlException e) {
+      // The same refusal, raised by the session's permission check rather than by the
+      // removal itself. Normalized so that it reaches the caller as the one exception
+      // type the delete flow maps to a failure response.
+      if (LOG.isErrorEnabled()) {
+        LOG.error("access denied, can't remove node:" + node.getPath());
+      }
+      throw new AccessDeniedException("access denied, can't remove node:" + node.getPath(), e);
     } catch (Exception e) {
       if (LOG.isErrorEnabled()) {
         LOG.error("an unexpected error occurs while removing the node", e);
@@ -438,10 +459,13 @@ private String moveToTrash(Node node) throws RepositoryException {
       removeMixinRestoreLocation(node);
       ret = false;
     } catch (AccessDeniedException e) {
+      // Not swallowed like the other failure modes here: a permission refusal must reach
+      // the caller as an actual error rather than a logged-and-ignored "-1", so that it
+      // surfaces as a real failure response instead of a false "moved to trash".
       if (LOG.isErrorEnabled()) {
         LOG.error("access denied, can't move to trash node:" + node.getPath());
       }
-      ret = false;
+      throw e;
     } catch (Exception e) {
       if (LOG.isErrorEnabled()) {
         LOG.error("an unexpected error occurs", e);
@@ -452,7 +476,11 @@ private String moveToTrash(Node node) throws RepositoryException {
   }
 
   public static boolean canRemoveNode(Node node) throws RepositoryException {
-    return checkPermission(node, PermissionType.REMOVE);
+    // A user must always be able to clear their own Personal Documents space, even when
+    // a node inside it carries an ACL that does not grant them REMOVE (e.g. a node
+    // created there on somebody else's behalf) — see JCRDocumentsUtil#isInUserPrivateSpace.
+    return checkPermission(node, PermissionType.REMOVE)
+        || JCRDocumentsUtil.isInUserPrivateSpace(node, node.getSession().getUserID());
   }
 
   private static boolean checkPermission(Node node,String permissionType) throws RepositoryException {

documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/TrashStorageImpl.java

@@ -21,6 +21,7 @@
 import org.exoplatform.container.xml.InitParams;
 import org.exoplatform.documents.model.TrashElementNodeFilter;
 import org.exoplatform.documents.storage.TrashStorage;
+import org.exoplatform.documents.storage.jcr.util.JCRDocumentsUtil;
 import org.exoplatform.documents.storage.jcr.util.NodeTypeConstants;
 import org.exoplatform.services.jcr.RepositoryService;
 import org.exoplatform.services.jcr.access.PermissionType;
@@ -41,6 +42,8 @@
 import org.gatein.pc.api.info.PortletInfo;
 import org.gatein.pc.api.info.PreferencesInfo;
 
+import java.security.AccessControlException;
+
 import javax.jcr.*;
 import javax.jcr.query.Query;
 import javax.jcr.query.QueryManager;
@@ -121,7 +124,18 @@ public String moveToTrash(Node node,
     String trashId = null;
     String nodeName = node.getName();
     Session nodeSession = node.getSession();
-    nodeSession.checkPermission(node.getPath(), PermissionType.REMOVE);
+    try {
+      nodeSession.checkPermission(node.getPath(), PermissionType.REMOVE);
+    } catch (AccessControlException e) {
+      // A user must always be able to clear their own Personal Documents space, even when
+      // a node inside it carries an ACL that does not grant them REMOVE (e.g. a node
+      // created there on somebody else's behalf) — see JCRDocumentsUtil#isInUserPrivateSpace.
+      // The caller's own canRemoveNode already allows for this; this is the same rule
+      // applied at the actual mutation, which checks the node's real ACL independently.
+      if (!JCRDocumentsUtil.isInUserPrivateSpace(node, nodeSession.getUserID())) {
+        throw e;
+      }
+    }
     if (deep == 0 && !node.isNodeType(NodeTypeConstants.EXO_SYMLINK)) {
       try {
         removeDeadSymlinks(node);

documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtil.java

@@ -84,8 +84,12 @@ public class JCRDocumentsUtil {
 
   private static final String                           DEFAULT_GROUPS_HOME_PATH      = "/Groups";                             // NOSONAR
 
+  private static final String                           DEFAULT_USERS_HOME_PATH       = "/Users";                              // NOSONAR
+
   public static final String                            GROUPS_PATH_ALIAS             = "groupsPath";
 
+  public static final String                            USERS_PATH_ALIAS              = "usersPath";
+
   public static final String                            DOCUMENTS_NODE                = "Documents";
 
   private static final String                           JCR_DATASOURCE_NAME           = "jcr";
@@ -114,6 +118,8 @@ public class JCRDocumentsUtil {
 
   private static String groupsPath = null;
 
+  private static String usersPath = null;
+
   private JCRDocumentsUtil() {
     // Utils class, no constructor will be needed
   }
@@ -140,6 +146,21 @@ public static String getGroupsPath(NodeHierarchyCreator nodeHierarchyCreator) {
     return groupsPath;
   }
 
+  /**
+   * @return the JCR path holding every user's home node, e.g. {@code /Users}.
+   */
+  public static String getUsersPath() {
+    if (usersPath != null) {
+      return usersPath;
+    }
+    NodeHierarchyCreator nodeHierarchyCreator = CommonsUtils.getService(NodeHierarchyCreator.class);
+    usersPath = nodeHierarchyCreator == null ? null : nodeHierarchyCreator.getJcrPath(USERS_PATH_ALIAS);
+    if (StringUtils.isBlank(usersPath)) {
+      usersPath = DEFAULT_USERS_HOME_PATH;
+    }
+    return usersPath;
+  }
+
   public static List<FileNode> toFileNodes(IdentityManager identityManager,
                                            NodeIterator nodeIterator,
                                            Identity aclIdentity,
@@ -633,6 +654,9 @@ public static void computeDocumentAcl(Node node, AbstractNode documentNode, Iden
       }
 
     }
+    // Owning the containing space outranks a node's own ACL for delete purposes — see
+    // isInUserPrivateSpace.
+    canDelete = canDelete || isInUserPrivateSpace(node, userId);
     documentNode.setAcl(new NodePermission(true, canEdit, canDelete, isPublic, permissions,null, null,null));
   }
 
@@ -671,6 +695,56 @@ public static Node getNodeByPath(Session session, String nodePath) {
     return null;
   }
 
+  /**
+   * Whether the node sits inside {@code username}'s own Personal Documents ("Private")
+   * space.
+   * <p>
+   * Whoever owns the containing space must always be able to manage what is inside it,
+   * even a node whose own explicit ACL does not grant them {@code REMOVE}/
+   * {@code SET_PROPERTY} — an ACE that never named the space's owner (e.g. a node
+   * created there on somebody else's behalf) must not outrank ownership of the space
+   * itself.
+   *
+   * @param node the node to check
+   * @param username the user whose private space to check against
+   * @return true when the node's path sits under that user's own Private root
+   * @throws RepositoryException if the node's path cannot be read
+   */
+  public static boolean isInUserPrivateSpace(Node node, String username) throws RepositoryException {
+    if (node == null || StringUtils.isBlank(username)) {
+      return false;
+    }
+    return StringUtils.equals(username, getPrivateDriveOwner(node.getPath()));
+  }
+
+  /**
+   * The user whose personal drive holds the given path.
+   * <p>
+   * Resolved by anchoring on the users home ({@link #getUsersPath()}) and on the
+   * <em>first</em> {@code Private} segment of the path, so that the owner is the user
+   * whose home node the drive belongs to — never a folder further down that happens to
+   * be named after a user, wherever it sits.
+   *
+   * @param path the JCR path to resolve
+   * @return the owning username, or null when the path is not inside a personal drive
+   */
+  private static String getPrivateDriveOwner(String path) {
+    String usersHomePath = getUsersPath();
+    if (StringUtils.isBlank(path) || !path.startsWith(usersHomePath + "/")) {
+      return null;
+    }
+    String privateRootSuffix = "/" + USER_PRIVATE_ROOT_NODE;
+    int index = path.indexOf(privateRootSuffix + "/");
+    if (index < 0 && path.endsWith(privateRootSuffix)) {
+      index = path.length() - privateRootSuffix.length();
+    }
+    if (index <= usersHomePath.length()) {
+      return null;
+    }
+    String userHomePath = path.substring(0, index);
+    return userHomePath.substring(userHomePath.lastIndexOf('/') + 1);
+  }
+
   public static Node getIdentityRootNode(SpaceService spaceService,
                                          NodeHierarchyCreator nodeHierarchyCreator,
                                          String username,

documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandler.java

@@ -22,6 +22,7 @@
 
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
+import java.security.AccessControlException;
 import java.util.*;
 import java.util.Map.Entry;
 
@@ -45,6 +46,7 @@
 import org.exoplatform.commons.api.settings.data.Scope;
 import org.exoplatform.commons.utils.MimeTypeResolver;
 import org.exoplatform.documents.storage.TrashStorage;
+import org.exoplatform.documents.storage.jcr.util.JCRDocumentsUtil;
 import org.exoplatform.documents.webdav.model.WebDavException;
 import org.exoplatform.documents.webdav.model.WebDavItemOrder;
 import org.exoplatform.documents.webdav.model.WebDavItemProperty;
@@ -751,15 +753,23 @@ public String getParentWebDavPath(String webDavPath) {
     return index <= 0 ? "/" : normalizedPath.substring(0, index);
   }
 
+  @SneakyThrows
   private boolean canRemoveNode(Node node) {
-    return checkPermission(node, PermissionType.REMOVE);
+    // A user must always be able to clear their own Personal Documents space, even when
+    // a node inside it carries an ACL that does not grant them REMOVE (e.g. a node
+    // created there on somebody else's behalf) — see JCRDocumentsUtil#isInUserPrivateSpace.
+    return checkPermission(node, PermissionType.REMOVE)
+        || JCRDocumentsUtil.isInUserPrivateSpace(node, node.getSession().getUserID());
   }
 
   private boolean checkPermission(Node node, String permissionType) {
     try {
       ((ExtendedNode) node).checkPermission(permissionType);
       return true;
-    } catch (RepositoryException e) {
+    } catch (AccessControlException | RepositoryException e) {
+      // ExtendedNode#checkPermission declares both: a denial normally surfaces as the
+      // former, not the latter, and only catching RepositoryException let it escape
+      // uncaught instead of being reported as a plain "can't remove".
       return false;
     }
   }