From 6dbedb40f82e45e3fd6d5f587fc5f8b609bd2f5f Mon Sep 17 00:00:00 2001 From: Ali HAMDI Date: Fri, 28 Aug 2026 15:15:04 +0200 Subject: [PATCH] fix: allow removing under conditions files owned by other users from personal drives - EXO-89504 (#2054) If under the personal drive, there are files owned by other users (including System) that the drive owner has not the permission to delete, those files are kept forever . Those files are added by mistake, or following some bugs, they didn't get the delete permission The fix adds the condition : ** if the file is under his own personal drive** , a user should be able to remove it even if he has not the permission to. --- .../storage/JCRDeleteFileStorage.java | 3 +- .../documents/rest/DocumentFileRestTest.java | 2 +- .../storage/jcr/JCRDeleteFileStorageImpl.java | 34 ++++- .../storage/jcr/TrashStorageImpl.java | 16 ++- .../storage/jcr/util/JCRDocumentsUtil.java | 74 ++++++++++ .../plugin/WebdavWriteCommandHandler.java | 14 +- .../storage/jcr/JCRDeleteFileStorageTest.java | 126 ++++++++++++++++++ .../storage/jcr/TrashStorageImplTest.java | 90 +++++++++++++ .../jcr/util/JCRDocumentsUtilTest.java | 40 ++++++ .../plugin/WebdavWriteCommandHandlerTest.java | 45 +++++++ 10 files changed, 436 insertions(+), 8 deletions(-) diff --git a/documents-api/src/main/java/org/exoplatform/documents/storage/JCRDeleteFileStorage.java b/documents-api/src/main/java/org/exoplatform/documents/storage/JCRDeleteFileStorage.java index 9797df9cb7..78e2ef97b2 100644 --- a/documents-api/src/main/java/org/exoplatform/documents/storage/JCRDeleteFileStorage.java +++ b/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 diff --git a/documents-services/src/test/java/org/exoplatform/documents/rest/DocumentFileRestTest.java b/documents-services/src/test/java/org/exoplatform/documents/rest/DocumentFileRestTest.java index dd07d6dbf6..b473fb5113 100644 --- a/documents-services/src/test/java/org/exoplatform/documents/rest/DocumentFileRestTest.java +++ b/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)); diff --git a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageImpl.java b/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageImpl.java index daca403304..61df1b2c5d 100644 --- a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageImpl.java +++ b/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageImpl.java @@ -131,7 +131,7 @@ public Map 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 { diff --git a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/TrashStorageImpl.java b/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/TrashStorageImpl.java index 822ae19821..4d7a4cf66f 100644 --- a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/TrashStorageImpl.java +++ b/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); diff --git a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtil.java b/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtil.java index 8c6c93a34d..a98c75de35 100644 --- a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtil.java +++ b/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 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. + *

+ * 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. + *

+ * Resolved by anchoring on the users home ({@link #getUsersPath()}) and on the + * first {@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, diff --git a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandler.java b/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandler.java index 31d1cdadfb..72239be739 100644 --- a/documents-storage-jcr/src/main/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandler.java +++ b/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; } } diff --git a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageTest.java b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageTest.java index 8bf6db694f..f2943689ed 100644 --- a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageTest.java +++ b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/JCRDeleteFileStorageTest.java @@ -36,6 +36,7 @@ import org.exoplatform.services.jcr.ext.app.SessionProviderService; import org.exoplatform.services.jcr.ext.common.SessionProvider; import org.exoplatform.services.jcr.impl.core.NodeImpl; +import org.exoplatform.services.jcr.impl.core.SessionImpl; import org.exoplatform.services.listener.ListenerService; import org.exoplatform.social.core.identity.model.Identity; import org.exoplatform.social.core.identity.model.Profile; @@ -52,6 +53,8 @@ import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; +import java.security.AccessControlException; +import javax.jcr.AccessDeniedException; import javax.jcr.Node; import javax.jcr.RepositoryException; import javax.jcr.Session; @@ -199,6 +202,129 @@ public void testDeleteDocument() throws Exception { verify(node, times(2)).removeMixin(NodeTypeConstants.EXO_RESTORE_LOCATION); } + @Test + public void testDeleteDocumentDeniedOutsideOwnPrivateSpace() throws Exception { + String username = "testuser"; + String currentRepository = "collaboration"; + String path = "/Users/otheruser/Private/file1"; + long currentOwnerId = 2; + org.exoplatform.services.security.Identity userID = new org.exoplatform.services.security.Identity(username); + + when(sessionProviderService.getSystemSessionProvider(any())).thenReturn(sessionProvider); + when(repositoryService.getCurrentRepository()).thenReturn(repository); + when(repository.getConfiguration()).thenReturn(repositoryEntry); + when(repositoryEntry.getDefaultWorkspaceName()).thenReturn(currentRepository); + + ExtendedSession session1 = mock(ExtendedSession.class); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.getUserSessionProvider(repositoryService, userID)).thenReturn(sessionProvider); + when(sessionProvider.getSession(Mockito.any(), Mockito.any())).thenReturn(session1); + + NodeImpl node = Mockito.mock(NodeImpl.class); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.getNodeByPath(session1, path)).thenReturn(node); + when(node.getPath()).thenReturn(path); + when(node.isCheckedOut()).thenReturn(true); + SessionImpl nodeSession = mock(SessionImpl.class); + when(node.getSession()).thenReturn(nodeSession); + when(nodeSession.getUserID()).thenReturn(username); + // Neither the node's own ACL nor the private-space override grant this user REMOVE here. + doThrow(new AccessControlException("denied")).when((ExtendedNode) node).checkPermission(anyString()); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.isInUserPrivateSpace(node, username)).thenReturn(false); + + // The refusal must reach the caller as a real failure, not a logged-and-ignored "success". + assertThrows(IllegalAccessException.class, + () -> jcrDeleteFileStorage.deleteDocument(path, "1", false, true, 0, userID, currentOwnerId)); + } + + @Test + public void testDeleteDocumentAllowedInsideOwnPrivateSpaceDespiteMissingAcl() throws Exception { + String username = "testuser"; + String currentRepository = "collaboration"; + String trashId = "999"; + String path = "/Users/testuser/Private/file1"; + long currentOwnerId = 2; + org.exoplatform.services.security.Identity userID = new org.exoplatform.services.security.Identity(username); + + when(sessionProviderService.getSystemSessionProvider(any())).thenReturn(sessionProvider); + when(repositoryService.getCurrentRepository()).thenReturn(repository); + when(repository.getConfiguration()).thenReturn(repositoryEntry); + when(repositoryEntry.getDefaultWorkspaceName()).thenReturn(currentRepository); + + ExtendedSession session1 = mock(ExtendedSession.class); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.getUserSessionProvider(repositoryService, userID)).thenReturn(sessionProvider); + when(sessionProvider.getSession(Mockito.any(), Mockito.any())).thenReturn(session1); + + NodeImpl node = Mockito.mock(NodeImpl.class); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.getNodeByPath(session1, path)).thenReturn(node); + when(node.getPath()).thenReturn(path); + when(node.isCheckedOut()).thenReturn(true); + SessionImpl nodeSession = mock(SessionImpl.class); + when(node.getSession()).thenReturn(nodeSession); + when(nodeSession.getUserID()).thenReturn(username); + // The node's own ACL does not grant REMOVE (e.g. it was created there on this user's + // behalf without one), but the node lives inside the acting user's own Private space. + doThrow(new AccessControlException("denied")).when((ExtendedNode) node).checkPermission(anyString()); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.isInUserPrivateSpace(node, username)).thenReturn(true); + when(trashStorage.moveToTrash(node, sessionProvider)).thenReturn(trashId); + when(trashStorage.getNodeByTrashId(trashId)).thenReturn(node); + NodeType nodeType = mock(NodeType.class); + when(nodeType.getName()).thenReturn(NodeTypeConstants.NT_FILE); + when(node.getPrimaryNodeType()).thenReturn(nodeType); + + jcrDeleteFileStorage.deleteDocument(path, "1", false, true, 0, userID, currentOwnerId); + + verify(trashStorage, times(1)).moveToTrash(node, sessionProvider); + } + + @Test + public void testDeleteDocumentSurfacesRemovalRefusal() throws Exception { + String username = "testuser"; + String path = "/document/file1"; + NodeImpl node = prepareNodeToRemove(username, path); + // The node is removed outright rather than moved to trash, and the user's own session + // refuses the removal. + doThrow(new AccessDeniedException("denied")).when(node).remove(); + + // The refusal must reach the caller: returning normally here is reported as "removed". + assertThrows(IllegalAccessException.class, + () -> jcrDeleteFileStorage.deleteDocument(path, "1", false, true, 0, + new org.exoplatform.services.security.Identity(username), 2)); + } + + @Test + public void testDeleteDocumentSurfacesRemovalRefusalRaisedByTheSession() throws Exception { + String username = "testuser"; + String path = "/document/file1"; + NodeImpl node = prepareNodeToRemove(username, path); + // Same refusal, raised by the session's permission check rather than by the removal. + doThrow(new AccessControlException("denied")).when(node).remove(); + + assertThrows(IllegalAccessException.class, + () -> jcrDeleteFileStorage.deleteDocument(path, "1", false, true, 0, + new org.exoplatform.services.security.Identity(username), 2)); + } + + private NodeImpl prepareNodeToRemove(String username, String path) throws Exception { + String currentRepository = "collaboration"; + org.exoplatform.services.security.Identity userID = new org.exoplatform.services.security.Identity(username); + + when(sessionProviderService.getSystemSessionProvider(any())).thenReturn(sessionProvider); + when(repositoryService.getCurrentRepository()).thenReturn(repository); + when(repository.getConfiguration()).thenReturn(repositoryEntry); + when(repositoryEntry.getDefaultWorkspaceName()).thenReturn(currentRepository); + + ExtendedSession session1 = mock(ExtendedSession.class); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.getUserSessionProvider(repositoryService, userID)).thenReturn(sessionProvider); + when(sessionProvider.getSession(Mockito.any(), Mockito.any())).thenReturn(session1); + + NodeImpl node = Mockito.mock(NodeImpl.class); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.getNodeByPath(session1, path)).thenReturn(node); + when(node.getPath()).thenReturn(path); + when(node.getParent()).thenReturn(node); + // Already in trash: the delete removes the node outright instead of moving it there. + when(trashStorage.isInTrash(node)).thenReturn(true); + return node; + } + @Test public void testGetDeletedDocuments() throws RepositoryException { // Mock input diff --git a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/TrashStorageImplTest.java b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/TrashStorageImplTest.java index 000344225d..a5d94ab0e4 100644 --- a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/TrashStorageImplTest.java +++ b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/TrashStorageImplTest.java @@ -19,15 +19,20 @@ import static org.exoplatform.documents.storage.jcr.util.NodeTypeConstants.RESTORE_PATH; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.lenient; import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.mockStatic; import static org.mockito.Mockito.times; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; +import java.security.AccessControlException; + import javax.jcr.Node; import javax.jcr.NodeIterator; import javax.jcr.Property; @@ -42,17 +47,21 @@ import org.exoplatform.documents.model.TrashElementNodeFilter; import org.exoplatform.services.jcr.impl.core.query.QueryImpl; import org.gatein.pc.api.PortletInvokerException; +import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; +import org.mockito.MockedStatic; import org.mockito.Mockito; import org.mockito.junit.MockitoJUnitRunner; import org.exoplatform.container.xml.InitParams; import org.exoplatform.container.xml.PropertiesParam; import org.exoplatform.container.xml.ValueParam; +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; import org.exoplatform.services.jcr.config.RepositoryEntry; import org.exoplatform.services.jcr.core.ManageableRepository; import org.exoplatform.services.jcr.ext.app.SessionProviderService; @@ -72,6 +81,13 @@ @RunWith(MockitoJUnitRunner.class) public class TrashStorageImplTest { + private static final MockedStatic JCR_DOCUMENTS_UTIL = mockStatic(JCRDocumentsUtil.class); + + @AfterClass + public static void afterRunBare() throws Exception { // NOSONAR + JCR_DOCUMENTS_UTIL.close(); + } + private IdentityManager identityManager; @@ -158,6 +174,80 @@ public void testMoveToTrash() throws Exception { assertNotNull(trashId); } + @Test + public void testMoveToTrashAllowedInsideOwnPrivateSpaceDespiteMissingAcl() throws Exception { + String username = "testuser"; + String currentRepository = "Collaboration"; + String path = "/Users/testuser/Private/name123"; + + Node node = Mockito.mock(NodeImpl.class); + Workspace workspace = Mockito.mock(WorkspaceImpl.class); + NodeIterator nodeIterator = Mockito.mock(NodeIterator.class); + Session session1 = Mockito.mock(SessionImpl.class); + NodeType nodeType = Mockito.mock(NodeType.class); + SessionActionInterceptor sessionActionInterceptor = Mockito.mock(SessionActionInterceptor.class); + + lenient().when(sessionProviderService.getSystemSessionProvider(any())).thenReturn(sessionProvider); + lenient().when(sessionProviderService.getSystemSessionProvider(any()).getSession("trashWorkspace", repository)).thenReturn(session); + lenient().when(sessionProviderService.getSystemSessionProvider(any()).getSession("trashWorkspace", repository).getWorkspace()).thenReturn(workspace); + lenient().when(sessionProviderService.getSystemSessionProvider(any()).getSession("trashWorkspace", repository).getItem(anyString())).thenReturn(node); + lenient().when(((Node) sessionProviderService.getSystemSessionProvider(any()).getSession("trashWorkspace", repository).getItem(anyString())).getNodes(anyString())).thenReturn(nodeIterator); + lenient().when(sessionProviderService.getSystemSessionProvider(any()).getSession("trashWorkspace", repository).getWorkspace().getName()).thenReturn(currentRepository); + lenient().when(sessionProviderService.getSessionProvider(any())).thenReturn(sessionProvider); + lenient().when(repositoryService.getCurrentRepository()).thenReturn(repository); + lenient().when(repository.getConfiguration()).thenReturn(repositoryEntry); + lenient().when(repositoryEntry.getDefaultWorkspaceName()).thenReturn(currentRepository); + lenient().when(sessionProvider.getSession(any(), any())).thenReturn(session); + + lenient().when(node.getUUID()).thenReturn("id123"); + lenient().when(node.getName()).thenReturn("name123"); + lenient().when(node.getPath()).thenReturn(path); + lenient().when(node.getSession()).thenReturn(session1); + lenient().when(node.getSession().getWorkspace()).thenReturn(workspace); + lenient().when(session1.getWorkspace().getName()).thenReturn(currentRepository); + lenient().when(node.getParent()).thenReturn(node); + lenient().when(((SessionImpl) node.getSession()).getActionHandler()).thenReturn(sessionActionInterceptor); + lenient().when(session.getNodeByUUID(eq("id123"))).thenReturn(node); + lenient().when(session.itemExists(anyString())).thenReturn(true); + lenient().when(session.getItem(anyString())).thenReturn(node); + lenient().when(node.isNodeType(NodeTypeConstants.EXO_SYMLINK)).thenReturn(false); + lenient().when(node.getNodes()).thenReturn(nodeIterator); + lenient().when(node.getPrimaryNodeType()).thenReturn(nodeType); + lenient().when(node.getPrimaryNodeType().getName()).thenReturn(NodeTypeConstants.NT_FILE); + + // The node's own ACL does not grant REMOVE (e.g. it was created there on this user's + // behalf without one), but the node lives inside the acting user's own Private space. + lenient().when(session1.getUserID()).thenReturn(username); + doThrow(new AccessControlException("Permission denied " + path + " : remove")).when(session1) + .checkPermission(path, PermissionType.REMOVE); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.isInUserPrivateSpace(node, username)).thenReturn(true); + + String trashId = trashStorage.moveToTrash(node, sessionProvider, 0); + + assertNotNull(trashId); + } + + @Test + public void testMoveToTrashDeniedOutsideOwnPrivateSpace() throws Exception { + String username = "testuser"; + String path = "/Users/otheruser/Private/name123"; + + Node node = Mockito.mock(NodeImpl.class); + Session session1 = Mockito.mock(SessionImpl.class); + SessionActionInterceptor sessionActionInterceptor = Mockito.mock(SessionActionInterceptor.class); + + lenient().when(node.getPath()).thenReturn(path); + lenient().when(node.getSession()).thenReturn(session1); + lenient().when(((SessionImpl) node.getSession()).getActionHandler()).thenReturn(sessionActionInterceptor); + + lenient().when(session1.getUserID()).thenReturn(username); + doThrow(new AccessControlException("Permission denied " + path + " : remove")).when(session1) + .checkPermission(path, PermissionType.REMOVE); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.isInUserPrivateSpace(node, username)).thenReturn(false); + + assertThrows(AccessControlException.class, () -> trashStorage.moveToTrash(node, sessionProvider, 0)); + } + @Test public void testRestoreFromTrash() throws Exception { String username = "testuser"; diff --git a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtilTest.java b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtilTest.java index 1581a2020d..656a11228a 100644 --- a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtilTest.java +++ b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/util/JCRDocumentsUtilTest.java @@ -105,6 +105,7 @@ public void testRetrieveFileProperties() throws IOException, RepositoryException when(((NodeImpl) parentNode).getIdentifier()).thenReturn("identifierOfParentNode"); when(node.getParent()).thenReturn(parentNode); when(node.getName()).thenReturn("NodeName.pdf"); + when(node.getPath()).thenReturn("/Users/root/Private/NodeName.pdf"); Property property = mock(Property.class); when(((ExtendedNode) node).getACL()).thenReturn(new AccessControlList()); when(node.isNodeType(NodeTypeConstants.MIX_VERSIONABLE)).thenReturn(true); @@ -478,4 +479,43 @@ public void testRetrieveTrashElementProperties() throws RepositoryException { assertEquals(1234L, result.getSize()); } + @Test + public void testIsInUserPrivateSpace() throws RepositoryException { + Node node = mock(NodeImpl.class); + + // The user's own personal drive. + when(node.getPath()).thenReturn("/Users/r___/ro__/root/Private/file.pdf"); + assertTrue(JCRDocumentsUtil.isInUserPrivateSpace(node, "root")); + + // The Private root itself. + when(node.getPath()).thenReturn("/Users/r___/ro__/root/Private"); + assertTrue(JCRDocumentsUtil.isInUserPrivateSpace(node, "root")); + + // Somebody else's personal drive. + assertFalse(JCRDocumentsUtil.isInUserPrivateSpace(node, "bob")); + + // The user's Public drive is not their Private one. + when(node.getPath()).thenReturn("/Users/r___/ro__/root/Public/file.pdf"); + assertFalse(JCRDocumentsUtil.isInUserPrivateSpace(node, "root")); + + assertFalse(JCRDocumentsUtil.isInUserPrivateSpace(null, "root")); + when(node.getPath()).thenReturn("/Users/r___/ro__/root/Private/file.pdf"); + assertFalse(JCRDocumentsUtil.isInUserPrivateSpace(node, "")); + } + + @Test + public void testIsInUserPrivateSpaceIsNotAPathSubstringMatch() throws RepositoryException { + Node node = mock(NodeImpl.class); + + // A folder named after another user, holding a "Private" folder, inside root's drive: + // the drive belongs to root, not to bob. + when(node.getPath()).thenReturn("/Users/r___/ro__/root/Private/bob/Private/file.pdf"); + assertTrue(JCRDocumentsUtil.isInUserPrivateSpace(node, "root")); + assertFalse(JCRDocumentsUtil.isInUserPrivateSpace(node, "bob")); + + // The same folder shape in a space drive is not a personal drive at all. + when(node.getPath()).thenReturn("/Groups/spaces/myspace/Documents/bob/Private/file.pdf"); + assertFalse(JCRDocumentsUtil.isInUserPrivateSpace(node, "bob")); + } + } diff --git a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandlerTest.java b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandlerTest.java index b9b95b3888..dc3df58563 100644 --- a/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandlerTest.java +++ b/documents-storage-jcr/src/test/java/org/exoplatform/documents/storage/jcr/webdav/plugin/WebdavWriteCommandHandlerTest.java @@ -16,16 +16,21 @@ */ package org.exoplatform.documents.storage.jcr.webdav.plugin; +import static org.junit.Assert.assertThrows; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.anyString; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.atLeastOnce; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; +import static org.mockito.Mockito.mockStatic; +import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import java.io.ByteArrayInputStream; import java.io.InputStream; +import java.security.AccessControlException; import java.util.Collections; import javax.jcr.Node; @@ -33,15 +38,18 @@ import javax.jcr.lock.Lock; import javax.jcr.version.Version; +import org.junit.AfterClass; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.InjectMocks; import org.mockito.Mock; +import org.mockito.MockedStatic; import org.mockito.junit.MockitoJUnitRunner; import org.exoplatform.commons.api.settings.SettingService; import org.exoplatform.documents.storage.TrashStorage; +import org.exoplatform.documents.storage.jcr.util.JCRDocumentsUtil; import org.exoplatform.documents.webdav.model.WebDavException; import org.exoplatform.services.jcr.RepositoryService; import org.exoplatform.services.jcr.access.PermissionType; @@ -56,6 +64,8 @@ @RunWith(MockitoJUnitRunner.Silent.class) public class WebdavWriteCommandHandlerTest { + private static final MockedStatic JCR_DOCUMENTS_UTIL = mockStatic(JCRDocumentsUtil.class); + private static final String JCR_CONTENT = "jcr:content"; private static final String JCR_PATH = "/jcr/path"; // NOSONAR @@ -112,6 +122,11 @@ public class WebdavWriteCommandHandlerTest { @InjectMocks private WebdavWriteCommandHandler handler; + @AfterClass + public static void afterRunBare() throws Exception { // NOSONAR + JCR_DOCUMENTS_UTIL.close(); + } + @Before @SneakyThrows public void setUp() { @@ -190,6 +205,36 @@ public void testDeleteDeletesPathMapping() { verify(session).save(); } + @Test + @SneakyThrows + public void testDeleteAllowedInsideOwnPrivateSpaceDespiteMissingAcl() { + when(session.getUserState()).thenReturn(conversationState); + when(node.getPath()).thenReturn(JCR_PATH); + when(session.getUserID()).thenReturn("testuser"); + // The node's own ACL does not grant REMOVE, but it lives inside the acting user's + // own Private space, which must be enough to move it to trash. + doThrow(new AccessControlException("denied")).when(node).checkPermission(PermissionType.REMOVE); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.isInUserPrivateSpace(node, "testuser")).thenReturn(true); + + handler.delete(session, JCR_PATH); + + verify(trashStorage).moveToTrash(eq(node), any(SessionProvider.class)); + verify(pathCommandHandler).deleteMapping(JCR_PATH); + } + + @Test + @SneakyThrows + public void testDeleteDeniedOutsideOwnPrivateSpace() { + when(node.getPath()).thenReturn(JCR_PATH); + when(session.getUserID()).thenReturn("testuser"); + doThrow(new AccessControlException("denied")).when(node).checkPermission(PermissionType.REMOVE); + JCR_DOCUMENTS_UTIL.when(() -> JCRDocumentsUtil.isInUserPrivateSpace(node, "testuser")).thenReturn(false); + + assertThrows(WebDavException.class, () -> handler.delete(session, JCR_PATH)); + + verify(trashStorage, never()).moveToTrash(eq(node), any(SessionProvider.class)); + } + @Test @SneakyThrows public void testEnableVersioningUsesResolvedJcrPath() {