Merged 5.1.N (5.1.2) to 5.2.N (5.2.1)

128355 jvonka: MNT-16446: Edit Comment permission (part 2)
   - for v1 api


git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/BRANCHES/DEV/5.2.N/root@128362 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

source/java/org/alfresco/rest/api/impl/CommentsImpl.java

@@ -37,11 +37,14 @@ import org.alfresco.model.ContentModel;
 import org.alfresco.query.PagingRequest;
 import org.alfresco.query.PagingResults;
 import org.alfresco.repo.forum.CommentService;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.site.SiteModel;
 import org.alfresco.rest.api.Comments;
 import org.alfresco.rest.api.Nodes;
 import org.alfresco.rest.api.model.Comment;
 import org.alfresco.rest.framework.core.exceptions.ConstraintViolatedException;
 import org.alfresco.rest.framework.core.exceptions.InvalidArgumentException;
+import org.alfresco.rest.framework.core.exceptions.PermissionDeniedException;
 import org.alfresco.rest.framework.core.exceptions.UnsupportedResourceOperationException;
 import org.alfresco.rest.framework.resource.parameters.CollectionWithPagingInfo;
 import org.alfresco.rest.framework.resource.parameters.Paging;
@@ -119,11 +122,24 @@ public class CommentsImpl implements Comments
 	        nodeProps.remove(ContentModel.PROP_CONTENT);
         }
 
-        boolean canEdit = true;
+        boolean canEdit = false;
-        boolean canDelete = true;
+        boolean canDelete = false;
 
-        boolean isNodeLocked = false;
+        if (! isWorkingCopyOrLocked(nodeRef))
+        {
+            canEdit = canEditPermission(commentNodeRef);
+            canDelete = canDeletePermission(commentNodeRef);
+        }
+
+
+        Comment comment = new Comment(commentNodeRef.getId(), nodeProps, canEdit, canDelete);
+        return comment;
+    }
+
+    private boolean isWorkingCopyOrLocked(NodeRef nodeRef)
+    {
         boolean isWorkingCopy = false;
+        boolean isLocked = false;
 
         if (nodeRef != null)
         {
@@ -137,25 +153,39 @@ public class CommentsImpl implements Comments
                     LockStatus lockStatus = lockService.getLockStatus(nodeRef);
                     if (lockStatus == LockStatus.LOCKED || lockStatus == LockStatus.LOCK_OWNER)
                     {
-		            	isNodeLocked = true;
+                        isLocked = true;
                     }
                 }
             }
         }
+        return (isWorkingCopy || isLocked);
+    }
 
-        if(isNodeLocked || isWorkingCopy)
+    private boolean canEdit(NodeRef nodeRef, NodeRef commentNodeRef)
     {
-        	canEdit = false;
+        return ((! isWorkingCopyOrLocked(nodeRef) && canEditPermission(commentNodeRef)));
-        	canDelete = false;
-        }
-        else
-        {
-        	canEdit = permissionService.hasPermission(commentNodeRef, PermissionService.WRITE) == AccessStatus.ALLOWED;
-        	canDelete = permissionService.hasPermission(commentNodeRef, PermissionService.DELETE) == AccessStatus.ALLOWED;
     }
 
-        Comment comment = new Comment(commentNodeRef.getId(), nodeProps, canEdit, canDelete);
+    // TODO refactor (ACE-5437) - see also CommentsPost
-        return comment;
+    private boolean canEditPermission(NodeRef commentNodeRef)
+    {
+        String creator = (String)nodeService.getProperty(commentNodeRef, ContentModel.PROP_CREATOR);
+        Serializable owner = nodeService.getProperty(commentNodeRef, ContentModel.PROP_OWNER);
+        String currentUser = AuthenticationUtil.getFullyAuthenticatedUser();
+
+        boolean isSiteManager = permissionService.hasPermission(commentNodeRef, SiteModel.SITE_MANAGER) == (AccessStatus.ALLOWED);
+        boolean isCoordinator = permissionService.hasPermission(commentNodeRef, PermissionService.COORDINATOR) == (AccessStatus.ALLOWED);
+        return (isSiteManager || isCoordinator || currentUser.equals(creator) || currentUser.equals(owner));
+    }
+
+    private boolean canDelete(NodeRef nodeRef, NodeRef commentNodeRef)
+    {
+        return ((! isWorkingCopyOrLocked(nodeRef) || canDeletePermission(commentNodeRef)));
+    }
+
+    private boolean canDeletePermission(NodeRef commentNodeRef)
+    {
+        return permissionService.hasPermission(commentNodeRef, PermissionService.DELETE) == AccessStatus.ALLOWED;
     }
 
     public Comment createComment(String nodeId, Comment comment)
@@ -194,6 +224,12 @@ public class CommentsImpl implements Comments
 				throw new InvalidArgumentException();
 			}
 
+            // MNT-16446 (pending future ACE-5437)
+            if (! canEdit(nodeRef, commentNodeRef))
+            {
+                throw new PermissionDeniedException("Cannot edit comment");
+            }
+
             commentService.updateComment(commentNodeRef, title, content);
 	        return toComment(nodeRef, commentNodeRef);
 		}
@@ -243,8 +279,15 @@ public class CommentsImpl implements Comments
     {
     	try
     	{
-	    	nodes.validateNode(nodeId);
+            NodeRef nodeRef = nodes.validateNode(nodeId);
 	        NodeRef commentNodeRef = nodes.validateNode(commentNodeId);
+
+            // MNT-16446 (pending future ACE-5437)
+            if (! canDelete(nodeRef, commentNodeRef))
+            {
+                throw new PermissionDeniedException("Cannot delete comment");
+            }
+
             commentService.deleteComment(commentNodeRef);
 		}
 		catch(IllegalArgumentException e)

source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java

@@ -55,6 +55,7 @@ import org.alfresco.rest.api.tests.client.PublicApiException;
 import org.alfresco.rest.api.tests.client.RequestContext;
 import org.alfresco.rest.api.tests.client.data.Activity;
 import org.alfresco.rest.api.tests.client.data.Comment;
+import org.alfresco.rest.api.tests.client.data.SiteRole;
 import org.alfresco.rest.api.tests.client.data.Tag;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.site.SiteVisibility;
@@ -73,6 +74,9 @@ public class TestNodeComments extends EnterpriseTestApi
 
 	private TestPerson person11;
 	private TestPerson person12;
+	private TestPerson person13;
+	private TestPerson person14;
+
 	private TestPerson person21;
 	private TestPerson person22;
 	
@@ -101,11 +105,20 @@ public class TestNodeComments extends EnterpriseTestApi
 				people.add(person);
 				person = network1.createUser();
 				people.add(person);
+				person = network1.createUser();
+				people.add(person);
+				person = network1.createUser();
+				people.add(person);
 
 				return null;
 			}
 		}, network1.getId());
 
+		this.person11 = people.get(0);
+		this.person12 = people.get(1);
+		this.person13 = people.get(2);
+		this.person14 = people.get(3);
+
 		TenantUtil.runAsSystemTenant(new TenantRunAsWork<Void>()
 		{
 			@Override
@@ -120,10 +133,8 @@ public class TestNodeComments extends EnterpriseTestApi
 			}
 		}, network2.getId());
 
-		this.person11 = people.get(0);
+		this.person21 = people.get(4);
-		this.person12 = people.get(1);
+		this.person22 = people.get(5);
-		this.person21 = people.get(2);
-		this.person22 = people.get(3);
 
 		TenantUtil.runAsUserTenant(new TenantRunAsWork<Void>()
 		{
@@ -133,6 +144,9 @@ public class TestNodeComments extends EnterpriseTestApi
 				TestSite site = network1.createSite(SiteVisibility.PRIVATE);
 				sites.add(site);
 
+				site.updateMember(person13.getId(), SiteRole.SiteCollaborator);
+				site.updateMember(person14.getId(), SiteRole.SiteCollaborator);
+				
 				return null;
 			}
 		}, person11.getId(), network1.getId());
@@ -824,4 +838,37 @@ public class TestNodeComments extends EnterpriseTestApi
 			}, person11.getId(), network1.getId());
 		}
 	}
+	
+	@Test
+	public void test_MNT_16446() throws Exception
+	{
+		Comments commentsProxy = publicApiClient.comments();
+
+		// in a site
+
+		publicApiClient.setRequestContext(new RequestContext(network1.getId(), person13.getId()));
+		Comment comment = new Comment("Test Comment 1", "Test Comment 1");
+		Comment resp = commentsProxy.createNodeComment(nodeRef1.getId(), comment);
+		String commentId = resp.getId();
+
+		// MNT-16446: another site collaborator should not be able to edit
+		try
+		{
+			publicApiClient.setRequestContext(new RequestContext(network1.getId(), person14.getId()));
+			Comment update = new Comment("Test Comment 4", "Test Comment 4");
+			commentsProxy.updateNodeComment(nodeRef1.getId(), commentId, update);
+			fail();
+		}
+		catch (PublicApiException e)
+		{
+			assertEquals(HttpStatus.SC_FORBIDDEN, e.getHttpResponse().getStatusCode());
+		}
+
+		publicApiClient.setRequestContext(new RequestContext(network1.getId(), person13.getId()));
+
+		Comment update = new Comment("Updated comment", "Updated comment");
+		commentsProxy.updateNodeComment(nodeRef1.getId(), commentId, update);
+
+		commentsProxy.removeNodeComment(nodeRef1.getId(), commentId);
+	}
 }