From bef750f2d30f8a048a8c82efe85470179c75e866 Mon Sep 17 00:00:00 2001 From: Ross Gale Date: Tue, 29 Oct 2019 11:43:18 +0000 Subject: [PATCH] RM-6930 adding permissions check for active content in holds defore deletion --- .../messages/hold-service.properties | 4 +- .../hold/HoldServiceImpl.java | 19 ++++- .../hold/HoldServiceImplUnitTest.java | 76 +++++++++++++++++++ 3 files changed, 95 insertions(+), 4 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 20aedc40af..3295a49687 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 @@ -6,4 +6,6 @@ rm.hold.add-to-hold-locked-node=Locked content can't be added to a hold. rm.hold.delete-frozen-node=Frozen content can't be deleted. rm.hold.delete-node-frozen-children=Can't delete folder because it contains frozen content. rm.hold.move-frozen-node=Frozen content can't be moved. -rm.hold.update-frozen-node=Frozen content can't be updated. \ No newline at end of file +rm.hold.update-frozen-node=Frozen content can't be updated. +rm.hold.generic-permission-error=Can't delete hold, because you don't have the correct permissions for all the items within the hold. +rm.hold.detailed-permission-error=Can't delete hold, because filing permissions for the following items are needed: \ 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 9dde7263d9..ae7249c39f 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 @@ -94,6 +94,8 @@ public class HoldServiceImpl extends ServiceBaseImpl /** I18N */ private static final String MSG_ERR_ACCESS_DENIED = "permissions.err_access_denied"; + private static final String MSG_ERR_HOLD_PERMISSION_GENERIC_ERROR = "rm.hold.generic-permission-error"; + private static final String MSG_ERR_HOLD_PERMISSION_DETAILED_ERROR = "rm.hold.detailed-permission-error"; /** File Plan Service */ private FilePlanService filePlanService; @@ -496,14 +498,25 @@ public class HoldServiceImpl extends ServiceBaseImpl { try { - if (permissionService.hasPermission(nodeRef, RMPermissionModel.FILING) == AccessStatus.DENIED) + String permission; + + if (recordService.isRecord(nodeRef) || recordFolderService.isRecordFolder(nodeRef)) + { + permission = RMPermissionModel.FILING; + } + else + { + permission = PermissionService.READ; + } + + if (permissionService.hasPermission(nodeRef, permission) == AccessStatus.DENIED) { heldNames.add((String) nodeService.getProperty(nodeRef, ContentModel.PROP_NAME)); } } catch (AccessDeniedException ade) { - throw new AlfrescoRuntimeException("Can't delete hold, because you don't have filling permissions on all the items held within the hold.", ade); + throw new AlfrescoRuntimeException(I18NUtil.getMessage(MSG_ERR_HOLD_PERMISSION_GENERIC_ERROR), ade); } } @@ -517,7 +530,7 @@ public class HoldServiceImpl extends ServiceBaseImpl sb.append(name); sb.append("'"); } - throw new AlfrescoRuntimeException("Can't delete hold, because filing permissions for the following items are needed: " + sb.toString()); + throw new AlfrescoRuntimeException(I18NUtil.getMessage(MSG_ERR_HOLD_PERMISSION_DETAILED_ERROR) + sb.toString()); } // delete the hold node 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 15af11203a..fcca1eddd6 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 @@ -95,6 +95,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest @Mock private CapabilityService mockedCapabilityService; + @Spy @InjectMocks HoldServiceImpl holdService; @Before @@ -566,4 +567,79 @@ public class HoldServiceImplUnitTest extends BaseUnitTest holds.add(hold2); holdService.removeFromHolds(holds, activeContent); } + + /** + * test delete hold throws exception for failed read permission check for content + */ + @Test (expected = AlfrescoRuntimeException.class) + public void testDeleteHoldThrowsExceptionForActiveContentWithoutReadPermission() + { + NodeRef holdNode = generateNodeRef(TYPE_HOLD); + NodeRef heldContent = generateNodeRef(TYPE_CONTENT); + List holds = new ArrayList<>(2); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); + when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + when(mockedRecordService.isRecord(heldContent)).thenReturn(false); + when(mockedRecordFolderService.isRecordFolder(heldContent)).thenReturn(false); + when(mockedPermissionService.hasPermission(heldContent, PermissionService.READ)).thenReturn(AccessStatus.DENIED); + when(mockedNodeService.getProperty(heldContent, ContentModel.PROP_NAME)).thenReturn("foo"); + + holdService.deleteHold(holdNode); + } + + /** + * test delete hold throws exception for failed read permission check for records + */ + @Test (expected = AlfrescoRuntimeException.class) + public void testDeleteHoldThrowsExceptionForARecordWithoutReadPermission() + { + NodeRef holdNode = generateNodeRef(TYPE_HOLD); + NodeRef heldContent = generateNodeRef(); + List holds = new ArrayList<>(2); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); + when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + when(mockedRecordService.isRecord(heldContent)).thenThrow(new AccessDeniedException("")); + + holdService.deleteHold(holdNode); + + } + + /** + * test delete hold throws exception for failed file permission check for records + */ + @Test (expected = AlfrescoRuntimeException.class) + public void testDeleteHoldThrowsExceptionForARecordWithoutFilePermission() + { + NodeRef holdNode = generateNodeRef(TYPE_HOLD); + NodeRef heldContent = generateNodeRef(); + List holds = new ArrayList<>(2); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); + when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + when(mockedRecordService.isRecord(heldContent)).thenReturn(true); + when(mockedPermissionService.hasPermission(heldContent, RMPermissionModel.FILING)).thenReturn(AccessStatus.DENIED); + when(mockedNodeService.getProperty(heldContent, ContentModel.PROP_NAME)).thenReturn("foo"); + + holdService.deleteHold(holdNode); + } + + /** + * Test hold deleted for active content with read permission + */ + @Test + public void testDeleteHoldChecksReadPermissionForActiveContent() + { + NodeRef holdNode = generateNodeRef(TYPE_HOLD); + NodeRef heldContent = generateNodeRef(TYPE_CONTENT); + List holds = new ArrayList<>(2); + holds.add(new ChildAssociationRef(ASSOC_FROZEN_CONTENT, holdNode, ASSOC_FROZEN_CONTENT, heldContent, true, 1)); + when(mockedNodeService.getChildAssocs(holdNode, ASSOC_FROZEN_CONTENT, RegexQNamePattern.MATCH_ALL)).thenReturn(holds); + when(mockedRecordService.isRecord(heldContent)).thenReturn(false); + when(mockedRecordFolderService.isRecordFolder(heldContent)).thenReturn(false); + when(mockedPermissionService.hasPermission(heldContent, PermissionService.READ)).thenReturn(AccessStatus.ALLOWED); + when(mockedNodeService.getProperty(heldContent, ContentModel.PROP_NAME)).thenReturn("foo"); + + holdService.deleteHold(holdNode); + + verify(mockedNodeService, times(1)).deleteNode(holdNode); + } }