From eda69a6eb6de4656f7dd074db1944c15ebb45f70 Mon Sep 17 00:00:00 2001 From: cagache Date: Tue, 13 Aug 2019 17:10:06 +0300 Subject: [PATCH] Do not allow addition of locked content to holds --- .../messages/hold-service.properties | 3 +- .../hold/HoldServiceImpl.java | 21 +++++-------- .../script/hold/BaseHold.java | 18 +++++------ .../script/hold/HoldsGet.java | 30 +++++++++---------- .../hold/HoldServiceImplUnitTest.java | 21 +++++++++---- 5 files changed, 50 insertions(+), 43 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties index ae21a7ddb8..fa48e03455 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties @@ -1,4 +1,5 @@ rm.hold.not-hold=The node {0} is not a hold. rm.hold.add-to-hold-invalid-type={0} is neither a record nor a record folder nor active content. Only records, record \ folders or active content can be added to a hold. -rm.hold.add-to-hold-archived-node=Archived nodes can't be added to hold. \ No newline at end of file +rm.hold.add-to-hold-archived-node=Archived nodes can't be added to hold. +rm.hold.add-to-hold-locked-node=Locked nodes can't be added to hold. \ No newline at end of file diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index d4aadb59d9..643ec24370 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -27,6 +27,8 @@ package org.alfresco.module.org_alfresco_module_rm.hold; +import static org.alfresco.model.ContentModel.ASPECT_LOCKABLE; + import java.io.Serializable; import java.util.ArrayList; import java.util.Collections; @@ -59,7 +61,6 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.security.permissions.AccessDeniedException; import org.alfresco.service.cmr.repository.ChildAssociationRef; import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.PermissionService; import org.alfresco.service.namespace.NamespaceService; @@ -122,16 +123,6 @@ public class HoldServiceImpl extends ServiceBaseImpl this.filePlanService = filePlanService; } - /** - * Set the node service - * - * @param nodeService the node service - */ - public void setNodeService(NodeService nodeService) - { - this.nodeService = nodeService; - } - /** * Set the record service * @@ -575,8 +566,7 @@ public class HoldServiceImpl extends ServiceBaseImpl throw new IntegrityException(I18NUtil.getMessage("rm.hold.not-hold", holdName), null); } - if (permissionService.hasPermission(hold, RMPermissionModel.FILING) == AccessStatus.DENIED || - !AccessStatus.ALLOWED.equals( + if (!AccessStatus.ALLOWED.equals( capabilityService.getCapabilityAccessState(hold, RMPermissionModel.ADD_TO_HOLD))) { throw new AccessDeniedException(I18NUtil.getMessage(MSG_ERR_ACCESS_DENIED)); @@ -638,6 +628,11 @@ public class HoldServiceImpl extends ServiceBaseImpl { throw new IntegrityException(I18NUtil.getMessage("rm.hold.add-to-hold-archived-node"), null); } + + if (nodeService.hasAspect(nodeRef, ASPECT_LOCKABLE)) + { + throw new IntegrityException(I18NUtil.getMessage("rm.hold.add-to-hold-locked-node"), null); + } } /** diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHold.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHold.java index e09c2bc2c9..2d4beae7f1 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHold.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/BaseHold.java @@ -131,9 +131,9 @@ public abstract class BaseHold extends DeclarativeWebScript @Override protected Map executeImpl(WebScriptRequest req, Status status, Cache cache) { - JSONObject json = getJSONFromContent(req); - List holds = getHolds(json); - List nodeRefs = getItemNodeRefs(json); + final JSONObject json = getJSONFromContent(req); + final List holds = getHolds(json); + final List nodeRefs = getItemNodeRefs(json); doAction(holds, nodeRefs); return new HashMap<>(); } @@ -159,7 +159,7 @@ public abstract class BaseHold extends DeclarativeWebScript JSONObject json = null; try { - String content = req.getContent().getContent(); + final String content = req.getContent().getContent(); json = new JSONObject(new JSONTokener(content)); } catch (IOException iox) @@ -185,10 +185,10 @@ public abstract class BaseHold extends DeclarativeWebScript */ protected List getItemNodeRefs(JSONObject json) { - List nodeRefs = new ArrayList<>(); + final List nodeRefs = new ArrayList<>(); try { - JSONArray nodeRefsArray = json.getJSONArray("nodeRefs"); + final JSONArray nodeRefsArray = json.getJSONArray("nodeRefs"); for (int i = 0; i < nodeRefsArray.length(); i++) { NodeRef nodeReference = new NodeRef(nodeRefsArray.getString(i)); @@ -234,13 +234,13 @@ public abstract class BaseHold extends DeclarativeWebScript */ protected List getHolds(JSONObject json) { - List holds = new ArrayList<>(); + final List holds = new ArrayList<>(); try { - JSONArray holdsArray = json.getJSONArray("holds"); + final JSONArray holdsArray = json.getJSONArray("holds"); for (int i = 0; i < holdsArray.length(); i++) { - NodeRef nodeRef = new NodeRef(holdsArray.getString(i)); + final NodeRef nodeRef = new NodeRef(holdsArray.getString(i)); checkHoldNodeRef(nodeRef); holds.add(nodeRef); } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldsGet.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldsGet.java index e12e422acd..9338651dce 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldsGet.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/script/hold/HoldsGet.java @@ -116,33 +116,33 @@ public class HoldsGet extends DeclarativeWebScript @Override protected Map executeImpl(WebScriptRequest req, Status status, Cache cache) { - boolean fileOnly = getFileOnly(req); - NodeRef itemNodeRef = getItemNodeRef(req); - List holds = new ArrayList<>(); + final boolean fileOnly = getFileOnly(req); + final NodeRef itemNodeRef = getItemNodeRef(req); + final List holds = new ArrayList<>(); if (itemNodeRef == null) { - NodeRef filePlan = getFilePlan(req); + final NodeRef filePlan = getFilePlan(req); holds.addAll(holdService.getHolds(filePlan)); } else { - boolean includedInHold = getIncludedInHold(req); + final boolean includedInHold = getIncludedInHold(req); holds.addAll(holdService.heldBy(itemNodeRef, includedInHold)); } - List holdObjects = new ArrayList<>(holds.size()); + final List holdObjects = new ArrayList<>(holds.size()); for (NodeRef nodeRef : holds) { // only add if user has filling permission on the hold if (!fileOnly || permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.ALLOWED) { - String name = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); + final String name = (String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME); holdObjects.add(new Hold(name, nodeRef)); } } - Map model = new HashMap<>(1); + final Map model = new HashMap<>(1); sortHoldByName(holdObjects); model.put("holds", holdObjects); @@ -159,10 +159,10 @@ public class HoldsGet extends DeclarativeWebScript { NodeRef filePlan = null; - Map templateVars = req.getServiceMatch().getTemplateVars(); - String storeType = templateVars.get("store_type"); - String storeId = templateVars.get("store_id"); - String id = templateVars.get("id"); + final Map templateVars = req.getServiceMatch().getTemplateVars(); + final String storeType = templateVars.get("store_type"); + final String storeId = templateVars.get("store_id"); + final String id = templateVars.get("id"); if (StringUtils.isNotBlank(storeType) && StringUtils.isNotBlank(storeId) && StringUtils.isNotBlank(id)) { @@ -194,7 +194,7 @@ public class HoldsGet extends DeclarativeWebScript */ private NodeRef getItemNodeRef(WebScriptRequest req) { - String nodeRef = req.getParameter("itemNodeRef"); + final String nodeRef = req.getParameter("itemNodeRef"); NodeRef itemNodeRef = null; if (StringUtils.isNotBlank(nodeRef)) { @@ -212,7 +212,7 @@ public class HoldsGet extends DeclarativeWebScript private boolean getIncludedInHold(WebScriptRequest req) { boolean result = true; - String includedInHold = req.getParameter("includedInHold"); + final String includedInHold = req.getParameter("includedInHold"); if (StringUtils.isNotBlank(includedInHold)) { result = Boolean.parseBoolean(includedInHold); @@ -223,7 +223,7 @@ public class HoldsGet extends DeclarativeWebScript private boolean getFileOnly(WebScriptRequest req) { boolean result = false; - String fillingOnly = req.getParameter("fileOnly"); + final String fillingOnly = req.getParameter("fileOnly"); if (StringUtils.isNotBlank(fillingOnly)) { result = Boolean.parseBoolean(fillingOnly); diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java index c34ec088ec..05ae6f2606 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java @@ -93,8 +93,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest protected NodeRef hold2; protected NodeRef activeContent; - @Mock (name="capabilityService") - protected CapabilityService mockedCapabilityService; + @Mock + private CapabilityService mockedCapabilityService; @Spy @InjectMocks HoldServiceImpl holdService; @Before @@ -389,10 +389,14 @@ public class HoldServiceImplUnitTest extends BaseUnitTest } @Test (expected = AccessDeniedException.class) - public void addActiveContentToHoldNoPermissionsOnHold() + public void addActiveContentToHoldsNoPermissionsOnHold() { - when(mockedPermissionService.hasPermission(hold, RMPermissionModel.FILING)).thenReturn(AccessStatus.DENIED); - holdService.addToHold(hold, activeContent); + when(mockedCapabilityService.getCapabilityAccessState(hold, RMPermissionModel.ADD_TO_HOLD)).thenReturn(AccessStatus.DENIED); + // build a list of holds + List holds = new ArrayList<>(2); + holds.add(hold); + holds.add(hold2); + holdService.addToHolds(holds, activeContent); } @Test (expected = AccessDeniedException.class) @@ -409,6 +413,13 @@ public class HoldServiceImplUnitTest extends BaseUnitTest holdService.addToHold(hold, activeContent); } + @Test (expected = IntegrityException.class) + public void addLockedContentToHold() + { + when(mockedNodeService.hasAspect(activeContent, ContentModel.ASPECT_LOCKABLE)).thenReturn(true); + holdService.addToHold(hold, activeContent); + } + @SuppressWarnings("unchecked") @Test public void addToHolds()