From 7d7e0063562f1f2daaa87b378880199a86d17af5 Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Mon, 27 Jun 2016 07:05:38 +0000 Subject: [PATCH] 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 --- .../alfresco/rest/api/impl/CommentsImpl.java | 161 +++++++++++------- .../rest/api/tests/TestNodeComments.java | 111 ++++++++---- 2 files changed, 181 insertions(+), 91 deletions(-) diff --git a/source/java/org/alfresco/rest/api/impl/CommentsImpl.java b/source/java/org/alfresco/rest/api/impl/CommentsImpl.java index cffc27c72a..0156d1cf69 100644 --- a/source/java/org/alfresco/rest/api/impl/CommentsImpl.java +++ b/source/java/org/alfresco/rest/api/impl/CommentsImpl.java @@ -1,28 +1,28 @@ -/* - * #%L - * Alfresco Remote API - * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2016 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ package org.alfresco.rest.api.impl; import java.io.Serializable; @@ -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,45 +122,72 @@ public class CommentsImpl implements Comments nodeProps.remove(ContentModel.PROP_CONTENT); } - boolean canEdit = true; - boolean canDelete = true; + boolean canEdit = false; + boolean canDelete = false; - boolean isNodeLocked = false; - boolean isWorkingCopy = false; - - if(nodeRef != null) + if (! isWorkingCopyOrLocked(nodeRef)) { - Set aspects = nodeService.getAspects(nodeRef); - - isWorkingCopy = aspects.contains(ContentModel.ASPECT_WORKING_COPY); - if(!isWorkingCopy) - { - if(aspects.contains(ContentModel.ASPECT_LOCKABLE)) - { - LockStatus lockStatus = lockService.getLockStatus(nodeRef); - if (lockStatus == LockStatus.LOCKED || lockStatus == LockStatus.LOCK_OWNER) - { - isNodeLocked = true; - } - } - } + canEdit = canEditPermission(commentNodeRef); + canDelete = canDeletePermission(commentNodeRef); } - if(isNodeLocked || isWorkingCopy) - { - canEdit = false; - 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); return comment; } + private boolean isWorkingCopyOrLocked(NodeRef nodeRef) + { + boolean isWorkingCopy = false; + boolean isLocked = false; + + if (nodeRef != null) + { + Set aspects = nodeService.getAspects(nodeRef); + + isWorkingCopy = aspects.contains(ContentModel.ASPECT_WORKING_COPY); + if(!isWorkingCopy) + { + if(aspects.contains(ContentModel.ASPECT_LOCKABLE)) + { + LockStatus lockStatus = lockService.getLockStatus(nodeRef); + if (lockStatus == LockStatus.LOCKED || lockStatus == LockStatus.LOCK_OWNER) + { + isLocked = true; + } + } + } + } + return (isWorkingCopy || isLocked); + } + + private boolean canEdit(NodeRef nodeRef, NodeRef commentNodeRef) + { + return ((! isWorkingCopyOrLocked(nodeRef) && canEditPermission(commentNodeRef))); + } + + // TODO refactor (ACE-5437) - see also CommentsPost + 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) { NodeRef nodeRef = nodes.validateNode(nodeId); @@ -193,8 +223,14 @@ public class CommentsImpl implements Comments { throw new InvalidArgumentException(); } - - commentService.updateComment(commentNodeRef, title, content); + + // 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); } catch(IllegalArgumentException e) @@ -243,9 +279,16 @@ public class CommentsImpl implements Comments { try { - nodes.validateNode(nodeId); + NodeRef nodeRef = nodes.validateNode(nodeId); NodeRef commentNodeRef = nodes.validateNode(commentNodeId); - commentService.deleteComment(commentNodeRef); + + // MNT-16446 (pending future ACE-5437) + if (! canDelete(nodeRef, commentNodeRef)) + { + throw new PermissionDeniedException("Cannot delete comment"); + } + + commentService.deleteComment(commentNodeRef); } catch(IllegalArgumentException e) { diff --git a/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java b/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java index eb68e35b02..ecfbc818ca 100644 --- a/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java +++ b/source/test-java/org/alfresco/rest/api/tests/TestNodeComments.java @@ -1,28 +1,28 @@ -/* - * #%L - * Alfresco Remote API - * %% - * Copyright (C) 2005 - 2016 Alfresco Software Limited - * %% - * This file is part of the Alfresco software. - * If the software was purchased under a paid Alfresco license, the terms of - * the paid license agreement will prevail. Otherwise, the software is - * provided under the following open source license terms: - * - * Alfresco is free software: you can redistribute it and/or modify - * it under the terms of the GNU Lesser General Public License as published by - * the Free Software Foundation, either version 3 of the License, or - * (at your option) any later version. - * - * Alfresco is distributed in the hope that it will be useful, - * but WITHOUT ANY WARRANTY; without even the implied warranty of - * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the - * GNU Lesser General Public License for more details. - * - * You should have received a copy of the GNU Lesser General Public License - * along with Alfresco. If not, see . - * #L% - */ +/* + * #%L + * Alfresco Remote API + * %% + * Copyright (C) 2005 - 2016 Alfresco Software Limited + * %% + * This file is part of the Alfresco software. + * If the software was purchased under a paid Alfresco license, the terms of + * the paid license agreement will prevail. Otherwise, the software is + * provided under the following open source license terms: + * + * Alfresco is free software: you can redistribute it and/or modify + * it under the terms of the GNU Lesser General Public License as published by + * the Free Software Foundation, either version 3 of the License, or + * (at your option) any later version. + * + * Alfresco is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public License + * along with Alfresco. If not, see . + * #L% + */ package org.alfresco.rest.api.tests; import static org.junit.Assert.assertEquals; @@ -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; @@ -70,9 +71,12 @@ public class TestNodeComments extends EnterpriseTestApi private List people = new ArrayList(); private List sites = new ArrayList(); - + 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() { @Override @@ -119,11 +132,9 @@ public class TestNodeComments extends EnterpriseTestApi return null; } }, network2.getId()); - - this.person11 = people.get(0); - this.person12 = people.get(1); - this.person21 = people.get(2); - this.person22 = people.get(3); + + this.person21 = people.get(4); + this.person22 = people.get(5); TenantUtil.runAsUserTenant(new TenantRunAsWork() { @@ -132,6 +143,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; } @@ -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); + } }