RM-6930 adding permissions check for active content in holds defore deletion

rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/hold-service.properties

@@ -7,3 +7,5 @@ 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.
+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: 

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

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<ChildAssociationRef> 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<ChildAssociationRef> 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<ChildAssociationRef> 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<ChildAssociationRef> 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);
+    }
 }