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 e0db4a7330..1b8bbe0401 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 @@ -712,13 +712,20 @@ public class HoldServiceImpl extends ServiceBaseImpl ParameterCheck.mandatory("holds", holds); ParameterCheck.mandatory("nodeRef", nodeRef); - if (holds != null && !holds.isEmpty()) + if (!holds.isEmpty()) { for (final NodeRef hold : holds) { - if (!instanceOf(hold, TYPE_HOLD)) + if (!isHold(hold)) { - throw new AlfrescoRuntimeException("Can't remove from hold, because it isn't a hold. (hold=" + hold + ")"); + final String holdName = (String) nodeService.getProperty(hold, ContentModel.PROP_NAME); + throw new IntegrityException(I18NUtil.getMessage("rm.hold.not-hold", holdName), null); + } + + if (!AccessStatus.ALLOWED.equals( + capabilityService.getCapabilityAccessState(hold, RMPermissionModel.REMOVE_FROM_HOLD))) + { + throw new AccessDeniedException(I18NUtil.getMessage(MSG_ERR_ACCESS_DENIED)); } if (getHeld(hold).contains(nodeRef)) 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 d4d3183a06..21e2d4b0d2 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 @@ -110,6 +110,8 @@ public class HoldServiceImplUnitTest extends BaseUnitTest when(mockedCapabilityService.getCapabilityAccessState(hold, RMPermissionModel.ADD_TO_HOLD)).thenReturn(AccessStatus.ALLOWED); when(mockedCapabilityService.getCapabilityAccessState(hold2, RMPermissionModel.ADD_TO_HOLD)).thenReturn(AccessStatus.ALLOWED); + when(mockedCapabilityService.getCapabilityAccessState(hold, RMPermissionModel.REMOVE_FROM_HOLD)).thenReturn(AccessStatus.ALLOWED); + when(mockedCapabilityService.getCapabilityAccessState(hold2, RMPermissionModel.REMOVE_FROM_HOLD)).thenReturn(AccessStatus.ALLOWED); activeContent = generateNodeRef(); QName contentSubtype = QName.createQName("contentSubtype", "contentSubtype"); @@ -452,7 +454,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedRecordsManagementAuditService, times(2)).auditEvent(eq(recordFolder), anyString()); } - @Test (expected=AlfrescoRuntimeException.class) + @Test (expected = IntegrityException.class) public void removeFromHoldNotAHold() { holdService.removeFromHold(recordFolder, recordFolder); @@ -550,4 +552,18 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN); } + + @Test (expected = AccessDeniedException.class) + public void removeActiveContentFromHoldsNoPermissionsOnHold() + { + doReturn(Collections.singletonList(activeContent)).when(holdService).getHeld(hold); + doReturn(Collections.singletonList(activeContent)).when(holdService).getHeld(hold2); + doReturn(true).when(mockedNodeService).hasAspect(activeContent, ASPECT_FROZEN); + when(mockedCapabilityService.getCapabilityAccessState(hold, RMPermissionModel.REMOVE_FROM_HOLD)).thenReturn(AccessStatus.DENIED); + // build a list of holds + List holds = new ArrayList<>(2); + holds.add(hold); + holds.add(hold2); + holdService.removeFromHolds(holds, activeContent); + } }