Merge branch 'feature/RM-7062_NoReadOnHoldCanSeeAuditEvent' into 'master'

RM-7062 Check hold permission for view audit event

Closes RM-7062

See merge request records-management/records-management!1324

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditAddToHoldTests.java

@@ -151,21 +151,6 @@ public class AuditAddToHoldTests extends BaseRMRestTest
         };
     }
 
-    /**
-     * Data provider with invalid users that can not add content to a hold
-     *
-     * @return the userModel
-     */
-    @DataProvider (name = "invalidUsersForAddToHold")
-    public Object[][] getInvalidUsersForAddToHold()
-    {
-        return new UserModel[][]
-        {
-            { rmManagerNoReadOnHold },
-            { rmManagerNoReadOnNode }
-        };
-    }
-
     /**
      * Given a document/record/record folder is added to a hold
      * When I view the audit log
@@ -269,11 +254,11 @@ public class AuditAddToHoldTests extends BaseRMRestTest
 
     /**
      * Given a document is added to a hold
-     * When I view the audit log as an user with no Read permissions over the hold or the document
+     * When I view the audit log as an user with no Read permissions over the document
      * Then the add to hold entry isn't visible
      */
-    @Test (dataProvider = "invalidUsersForAddToHold")
+    @Test
-    public void addToHoldAuditEntryNotVisible(UserModel user)
+    public void addToHoldAuditEntryNotVisible()
     {
         STEP("Create a new file");
         FileModel contentToBeAdded = dataContent.usingAdmin().usingSite(privateSite)
@@ -285,7 +270,33 @@ public class AuditAddToHoldTests extends BaseRMRestTest
 
         STEP("Check that an user with no Read permissions can't see the entry for the add to hold event.");
         assertTrue("The list of events should not contain Add to Hold entry ",
-                rmAuditService.getAuditEntriesFilteredByEvent(user, ADD_TO_HOLD).isEmpty());
+            rmAuditService.getAuditEntriesFilteredByEvent(rmManagerNoReadOnNode, ADD_TO_HOLD).isEmpty());
+    }
+
+    /**
+     * Given a document is added to a hold
+     * When I view the audit log as an user with no Read permissions over the hold
+     * Then the the hold name is replaced in the add to hold entry
+     */
+    @Test
+    public void addToHoldAuditEntryHoldNameNotVisible()
+    {
+        STEP("Create a new file");
+        FileModel contentToBeAdded = dataContent.usingAdmin().usingSite(privateSite)
+                                                .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
+        rmAuditService.clearAuditLog();
+
+        STEP("Add file to hold.");
+        holdsAPI.addItemToHold(rmAdmin.getUsername(), rmAdmin.getPassword(), contentToBeAdded.getNodeRefWithoutVersion(), HOLD1);
+
+        auditEntries = rmAuditService.getAuditEntriesFilteredByEvent(rmManagerNoReadOnHold, ADD_TO_HOLD);
+
+        STEP("Check that an user with no Read permissions can't see the hold name in the add to hold event.");
+        String replacementHoldName = "You don't have permission to view this hold.";
+        assertEquals("The list of events should contain the Add to Hold entry", 1, auditEntries.size());
+        assertTrue("The hold name should not be visible in the Add to Hold entry ",
+            auditEntries.stream().anyMatch(entry -> entry.getChangedValues().contains(
+                ImmutableMap.of("new", replacementHoldName, "previous", "", "name", "Hold Name"))));
     }
 
     @AfterClass (alwaysRun = true)

rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditRemoveFromHoldTests.java

@@ -162,21 +162,6 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
         };
     }
 
-    /**
-     * Data provider with invalid users that can not remove content from a hold
-     *
-     * @return the userModel
-     */
-    @DataProvider (name = "invalidUsersForRemoveFromHold")
-    public Object[][] getInvalidUsersForRemoveFromHold()
-    {
-        return new UserModel[][]
-        {
-            { rmManagerNoReadOnHold },
-            { rmManagerNoReadOnNode }
-        };
-    }
-
     /**
      * Given a document/record/record folder is removed from a hold
      * When I view the audit log
@@ -280,11 +265,11 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
 
     /**
      * Given a document/record/record folder is removed from a hold
-     * When I view the audit log as an user with no Read permissions over the hold or the node
+     * When I view the audit log as an user with no Read permissions over the node
      * Then the remove from hold entry isn't visible
      */
-    @Test (dataProvider = "invalidUsersForRemoveFromHold")
+    @Test
-    public void removeFromHoldAuditEntryNotVisible(UserModel user)
+    public void removeFromHoldAuditEntryNotVisible()
     {
         STEP("Add content to a hold.");
         FileModel heldFile = dataContent.usingAdmin().usingSite(privateSite)
@@ -298,7 +283,35 @@ public class AuditRemoveFromHoldTests extends BaseRMRestTest
 
         STEP("Check that an user with no Read permissions can't see the entry for the remove from hold event.");
         assertTrue("The list of events should not contain Remove from Hold entry ",
-                rmAuditService.getAuditEntriesFilteredByEvent(user, REMOVE_FROM_HOLD).isEmpty());
+                rmAuditService.getAuditEntriesFilteredByEvent(rmManagerNoReadOnNode, REMOVE_FROM_HOLD).isEmpty());
+    }
+
+    /**
+     * Given a document/record/record folder is removed from a hold
+     * When I view the audit log as an user with no Read permissions over the hold
+     * Then the the hold name is replaced in the remove from hold entry
+     */
+    @Test
+    public void removeFromHoldAuditEntryHoldNameNotVisible()
+    {
+        STEP("Add content to a hold.");
+        FileModel heldFile = dataContent.usingAdmin().usingSite(privateSite)
+                                        .createContent(CMISUtil.DocumentType.TEXT_PLAIN);
+        holdsAPI.addItemToHold(rmAdmin.getUsername(), rmAdmin.getPassword(), heldFile.getNodeRefWithoutVersion(), HOLD1);
+
+        rmAuditService.clearAuditLog();
+
+        STEP("Remove held content from the hold.");
+        holdsAPI.removeItemFromHold(rmAdmin.getUsername(), rmAdmin.getPassword(), heldFile.getNodeRefWithoutVersion(), HOLD1);
+
+        auditEntries = rmAuditService.getAuditEntriesFilteredByEvent(rmManagerNoReadOnHold, REMOVE_FROM_HOLD);
+
+        STEP("Check that an user with no Read permissions can't see the hold name in the remove from hold event.");
+        String replacementHoldName = "You don't have permission to view this hold.";
+        assertEquals("The list of events should contain the Remove from Hold entry", 1, auditEntries.size());
+        assertTrue("The hold name should not be visible in the Remove from Hold entry ",
+            auditEntries.stream().anyMatch(entry -> entry.getChangedValues().contains(
+                ImmutableMap.of("new", "", "previous", replacementHoldName, "name", "Hold Name"))));
     }
 
     @AfterClass (alwaysRun = true)

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

@@ -17,6 +17,7 @@ rm.audit.createHold=Create Hold
 rm.audit.deleteHold=Delete Hold
 rm.audit.addToHold=Add To Hold
 rm.audit.removeFromHold=Remove From Hold
+rm.audit.holdPermission-Error=You don't have permission to view this hold.
 rm.audit.audit-start=Audit Start
 rm.audit.audit-stop=Audit Stop
 rm.audit.audit-clear=Audit Clear

rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml

@@ -941,6 +941,7 @@
         <property name="namespaceService" ref="NamespaceService" />
         <property name="capabilityService" ref="CapabilityService" />
         <property name="permissionService" ref="PermissionService" />
+        <property name="holdService" ref="HoldService" />
         <property name="ignoredAuditProperties">
             <list>
                 <value>cm:lastThumbnailModification</value>
@@ -1533,7 +1534,6 @@
       <property name="recordService" ref="RecordService" />
       <property name="recordFolderService" ref="RecordFolderService" />
       <property name="permissionService" ref="PermissionService"/>
-      <property name="recordsManagementAuditService" ref="RecordsManagementAuditService" />
       <property name="capabilityService" ref="CapabilityService"/>
       <property name="policyComponent" ref="policyComponent"/>
    </bean>

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java

@@ -61,6 +61,8 @@ import org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction
 import org.alfresco.module.org_alfresco_module_rm.audit.event.AuditEvent;
 import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
+import org.alfresco.module.org_alfresco_module_rm.hold.HoldService;
+import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.repo.audit.AuditComponent;
 import org.alfresco.repo.audit.model.AuditApplication;
 import org.alfresco.repo.content.MimetypeMap;
@@ -195,6 +197,10 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
     private static final String AUDIT_EVENT_VIEW = "audit.view";
     private static final String MSG_AUDIT_VIEW = "rm.audit.audit-view";
 
+    private static final QName PROPERTY_HOLD_NAME = QName.createQName(RecordsManagementModel.RM_URI, "Hold Name");
+    private static final QName PROPERTY_HOLD_NODEREF = QName.createQName(RecordsManagementModel.RM_URI, "Hold NodeRef");
+    private static final String HOLD_PERMISSION_DENIED_MSG = "rm.audit.holdPermission-Error";
+
     private PolicyComponent policyComponent;
     private DictionaryService dictionaryService;
     private TransactionService transactionService;
@@ -208,6 +214,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
     private NamespaceService namespaceService;
     protected CapabilityService capabilityService;
     protected PermissionService permissionService;
+    protected HoldService holdService;
 
     private boolean shutdown = false;
 
@@ -333,6 +340,15 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
         this.permissionService = permissionService;
     }
 
+    /**
+     *
+     * @param holdService
+     */
+    public void setHoldService(HoldService holdService)
+    {
+        this.holdService = holdService;
+    }
+
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditService#registerAuditEvent(java.lang.String, java.lang.String)
      */
@@ -687,7 +703,8 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
     /**
      * Helper method to remove system properties from maps
      *
-     * @param properties
+     * @param before    properties before event
+     * @param after     properties after event
      */
     private void removeAuditProperties(Map<QName, Serializable> before, Map<QName, Serializable> after)
     {
@@ -864,220 +881,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
         }
 
         // define the callback
-        AuditQueryCallback callback = new AuditQueryCallback()
+        AuditQueryCallback callback = new AuditTrailQueryCallback(results, writer, reportFormat);
-        {
-            private boolean firstEntry = true;
-
-
-            @Override
-            public boolean valuesRequired()
-            {
-                return true;
-            }
-
-            /**
-             * Just log the error, but continue
-             */
-            @Override
-            public boolean handleAuditEntryError(Long entryId, String errorMsg, Throwable error)
-            {
-                logger.warn(errorMsg, error);
-                return true;
-            }
-
-            @Override
-            @SuppressWarnings("unchecked")
-            public boolean handleAuditEntry(
-                    Long entryId,
-                    String applicationName,
-                    String user,
-                    long time,
-                    Map<String, Serializable> values)
-            {
-                // Check for context shutdown
-                if (shutdown)
-                {
-                    return false;
-                }
-
-
-                Date timestamp = new Date(time);
-                String eventName = null;
-                String fullName = null;
-                String userRoles = null;
-                NodeRef nodeRef = null;
-                String nodeName = null;
-                String nodeType = null;
-                String nodeIdentifier = null;
-                String namePath = null;
-                Map<QName, Serializable> beforeProperties = null;
-                Map<QName, Serializable> afterProperties = null;
-
-                if (values.containsKey(RM_AUDIT_DATA_EVENT_NAME))
-                {
-                    // This data is /RM/event/...
-                    eventName = (String) values.get(RM_AUDIT_DATA_EVENT_NAME);
-                    fullName = (String) values.get(RM_AUDIT_DATA_PERSON_FULLNAME);
-                    userRoles = (String) values.get(RM_AUDIT_DATA_PERSON_ROLES);
-                    nodeRef = (NodeRef) values.get(RM_AUDIT_DATA_NODE_NODEREF);
-                    nodeName = (String) values.get(RM_AUDIT_DATA_NODE_NAME);
-                    QName nodeTypeQname = (QName) values.get(RM_AUDIT_DATA_NODE_TYPE);
-                    nodeIdentifier = (String) values.get(RM_AUDIT_DATA_NODE_IDENTIFIER);
-                    namePath = (String) values.get(RM_AUDIT_DATA_NODE_NAMEPATH);
-                    beforeProperties = (Map<QName, Serializable>) values.get(RM_AUDIT_DATA_NODE_CHANGES_BEFORE);
-                    afterProperties = (Map<QName, Serializable>) values.get(RM_AUDIT_DATA_NODE_CHANGES_AFTER);
-
-                    // Convert some of the values to recognizable forms
-                    nodeType = null;
-                    if (nodeTypeQname != null)
-                    {
-                        TypeDefinition typeDef = dictionaryService.getType(nodeTypeQname);
-                        nodeType = (typeDef != null) ? typeDef.getTitle(dictionaryService) : null;
-                    }
-                }
-                else if (values.containsKey(DOD5015_AUDIT_DATA_EVENT_NAME))
-                {
-                    // This data is /RM/event/...
-                    eventName = (String) values.get(DOD5015_AUDIT_DATA_EVENT_NAME);
-                    fullName = (String) values.get(DOD5015_AUDIT_DATA_PERSON_FULLNAME);
-                    userRoles = (String) values.get(DOD5015_AUDIT_DATA_PERSON_ROLES);
-                    nodeRef = (NodeRef) values.get(DOD5015_AUDIT_DATA_NODE_NODEREF);
-                    nodeName = (String) values.get(DOD5015_AUDIT_DATA_NODE_NAME);
-                    QName nodeTypeQname = (QName) values.get(DOD5015_AUDIT_DATA_NODE_TYPE);
-                    nodeIdentifier = (String) values.get(DOD5015_AUDIT_DATA_NODE_IDENTIFIER);
-                    namePath = (String) values.get(DOD5015_AUDIT_DATA_NODE_NAMEPATH);
-                    beforeProperties = (Map<QName, Serializable>) values.get( DOD5015_AUDIT_DATA_NODE_CHANGES_BEFORE);
-                    afterProperties = (Map<QName, Serializable>) values.get(DOD5015_AUDIT_DATA_NODE_CHANGES_AFTER);
-
-                    // Convert some of the values to recognizable forms
-                    nodeType = null;
-                    if (nodeTypeQname != null)
-                    {
-                        TypeDefinition typeDef = dictionaryService.getType(nodeTypeQname);
-                        nodeType = (typeDef != null) ? typeDef.getTitle(dictionaryService) : null;
-                    }
-                }
-                else if (values.containsKey(RM_AUDIT_DATA_LOGIN_USERNAME))
-                {
-                    user = (String) values.get(RM_AUDIT_DATA_LOGIN_USERNAME);
-                    if (values.containsKey(RM_AUDIT_DATA_LOGIN_ERROR))
-                    {
-                        eventName = RM_AUDIT_EVENT_LOGIN_FAILURE;
-                        // The user didn't log in
-                        fullName = user;
-                    }
-                    else
-                    {
-                        eventName = RM_AUDIT_EVENT_LOGIN_SUCCESS;
-                        fullName = (String) values.get(RM_AUDIT_DATA_LOGIN_FULLNAME);
-                    }
-                }
-                else if (values.containsKey(DOD5015_AUDIT_DATA_LOGIN_USERNAME))
-                {
-                    user = (String) values.get(DOD5015_AUDIT_DATA_LOGIN_USERNAME);
-                    if (values.containsKey(DOD5015_AUDIT_DATA_LOGIN_ERROR))
-                    {
-                        eventName = RM_AUDIT_EVENT_LOGIN_FAILURE;
-                        // The user didn't log in
-                        fullName = user;
-                    }
-                    else
-                    {
-                        eventName = RM_AUDIT_EVENT_LOGIN_SUCCESS;
-                        fullName = (String) values.get(DOD5015_AUDIT_DATA_LOGIN_FULLNAME);
-                    }
-                }
-                else
-                {
-                    // This is not recognisable data
-                    logger.warn(
-                            "Unable to process audit entry for RM.  Unexpected data: \n" +
-                            "   Entry: " + entryId + "\n" +
-                            "   Data:  " + values);
-                    // Skip it
-                    return true;
-                }
-
-                if (nodeRef != null && nodeService.exists(nodeRef) &&
-                        ((filePlanService.isFilePlanComponent(nodeRef) &&
-                                !AccessStatus.ALLOWED.equals(
-                                        capabilityService.getCapabilityAccessState(nodeRef, ACCESS_AUDIT_CAPABILITY)))
-                                || (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(nodeRef, PermissionService.READ)))))
-                {
-                    return true;
-                }
-
-                // TODO: Refactor this to use the builder pattern
-                RecordsManagementAuditEntry entry = new RecordsManagementAuditEntry(
-                        timestamp,
-                        user,
-                        fullName,
-                        // A concatenated string of roles
-                        userRoles,
-                        nodeRef,
-                        nodeName,
-                        nodeType,
-                        eventName,
-                        nodeIdentifier,
-                        namePath,
-                        beforeProperties,
-                        afterProperties);
-
-                // write out the entry to the file in requested format
-                writeEntryToFile(entry);
-
-                if (results != null)
-                {
-                    results.add(entry);
-                }
-
-                if (logger.isDebugEnabled())
-                {
-                    logger.debug("   " + entry);
-                }
-
-                // Keep going
-                return true;
-            }
-
-            private void writeEntryToFile(RecordsManagementAuditEntry entry)
-            {
-                if (writer == null)
-                {
-                    return;
-                }
-                try
-                {
-                    if (!firstEntry)
-                    {
-                        if (reportFormat == ReportFormat.HTML)
-                        {
-                            writer.write("\n");
-                        }
-                        else
-                        {
-                            writer.write(",");
-                        }
-                    }
-                    else
-                    {
-                        firstEntry = false;
-                    }
-
-                    // write the entry to the file
-                    if (reportFormat == ReportFormat.JSON)
-                    {
-                        writer.write("\n\t\t");
-                    }
-
-                    writeAuditTrailEntry(writer, entry, reportFormat);
-                }
-                catch (IOException ioe)
-                {
-                    throw new AlfrescoRuntimeException(MSG_TRAIL_FILE_FAIL, ioe);
-                }
-            }
-        };
 
         String user = params.getUser();
         Long fromTime = getFromDateTime(params.getDateFrom());
@@ -1174,7 +978,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
 
     /**
      * Gets the start of the from date
-     * @see org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditServiceImpl.getStartOfDay()
+     * @see org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditServiceImpl#getStartOfDay(java.util.Date)
      *
      * @param date The date for which the start should be retrieved.
      * @return Returns null if the given date is null, otherwise the start of the given day.
@@ -1216,7 +1020,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
 
     /**
      * Gets the end of the from date
-     * @see org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditServiceImpl.getEndOfDay()
+     * @see org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditServiceImpl#getEndOfDay(java.util.Date)
      *
      * @param date The date for which the end should be retrieved.
      * @return Returns null if the given date is null, otherwise the end of the given day.
@@ -1912,4 +1716,270 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean
     {
         auditEvent(nodeRef, action.getName());
     }
+
+    private class AuditTrailQueryCallback implements AuditQueryCallback
+    {
+        private final List<RecordsManagementAuditEntry> results;
+        private final Writer writer;
+        private final ReportFormat reportFormat;
+        private boolean firstEntry;
+
+        public AuditTrailQueryCallback(List<RecordsManagementAuditEntry> results, Writer writer, ReportFormat reportFormat)
+        {
+            this.results = results;
+            this.writer = writer;
+            this.reportFormat = reportFormat;
+            firstEntry = true;
+        }
+
+
+        @Override
+        public boolean valuesRequired()
+        {
+            return true;
+        }
+
+        /**
+         * Just log the error, but continue
+         */
+        @Override
+        public boolean handleAuditEntryError(Long entryId, String errorMsg, Throwable error)
+        {
+            logger.warn(errorMsg, error);
+            return true;
+        }
+
+        @Override
+        @SuppressWarnings("unchecked")
+        public boolean handleAuditEntry(
+                Long entryId,
+                String applicationName,
+                String user,
+                long time,
+                Map<String, Serializable> values)
+        {
+            // Check for context shutdown
+            if (shutdown)
+            {
+                return false;
+            }
+
+
+            Date timestamp = new Date(time);
+            String eventName = null;
+            String fullName = null;
+            String userRoles = null;
+            NodeRef nodeRef = null;
+            String nodeName = null;
+            String nodeType = null;
+            String nodeIdentifier = null;
+            String namePath = null;
+            Map<QName, Serializable> beforeProperties = null;
+            Map<QName, Serializable> afterProperties = null;
+
+            if (values.containsKey(RM_AUDIT_DATA_EVENT_NAME))
+            {
+                // This data is /RM/event/...
+                eventName = (String) values.get(RM_AUDIT_DATA_EVENT_NAME);
+                fullName = (String) values.get(RM_AUDIT_DATA_PERSON_FULLNAME);
+                userRoles = (String) values.get(RM_AUDIT_DATA_PERSON_ROLES);
+                nodeRef = (NodeRef) values.get(RM_AUDIT_DATA_NODE_NODEREF);
+                nodeName = (String) values.get(RM_AUDIT_DATA_NODE_NAME);
+                QName nodeTypeQname = (QName) values.get(RM_AUDIT_DATA_NODE_TYPE);
+                nodeIdentifier = (String) values.get(RM_AUDIT_DATA_NODE_IDENTIFIER);
+                namePath = (String) values.get(RM_AUDIT_DATA_NODE_NAMEPATH);
+                beforeProperties = (Map<QName, Serializable>) values.get(RM_AUDIT_DATA_NODE_CHANGES_BEFORE);
+                afterProperties = (Map<QName, Serializable>) values.get(RM_AUDIT_DATA_NODE_CHANGES_AFTER);
+
+                // Convert some of the values to recognizable forms
+                if (nodeTypeQname != null)
+                {
+                    TypeDefinition typeDef = dictionaryService.getType(nodeTypeQname);
+                    nodeType = (typeDef != null) ? typeDef.getTitle(dictionaryService) : null;
+                }
+            }
+            else if (values.containsKey(DOD5015_AUDIT_DATA_EVENT_NAME))
+            {
+                // This data is /DOD5015/event/...
+                eventName = (String) values.get(DOD5015_AUDIT_DATA_EVENT_NAME);
+                fullName = (String) values.get(DOD5015_AUDIT_DATA_PERSON_FULLNAME);
+                userRoles = (String) values.get(DOD5015_AUDIT_DATA_PERSON_ROLES);
+                nodeRef = (NodeRef) values.get(DOD5015_AUDIT_DATA_NODE_NODEREF);
+                nodeName = (String) values.get(DOD5015_AUDIT_DATA_NODE_NAME);
+                QName nodeTypeQname = (QName) values.get(DOD5015_AUDIT_DATA_NODE_TYPE);
+                nodeIdentifier = (String) values.get(DOD5015_AUDIT_DATA_NODE_IDENTIFIER);
+                namePath = (String) values.get(DOD5015_AUDIT_DATA_NODE_NAMEPATH);
+                beforeProperties = (Map<QName, Serializable>) values.get( DOD5015_AUDIT_DATA_NODE_CHANGES_BEFORE);
+                afterProperties = (Map<QName, Serializable>) values.get(DOD5015_AUDIT_DATA_NODE_CHANGES_AFTER);
+
+                // Convert some of the values to recognizable forms
+                if (nodeTypeQname != null)
+                {
+                    TypeDefinition typeDef = dictionaryService.getType(nodeTypeQname);
+                    nodeType = (typeDef != null) ? typeDef.getTitle(dictionaryService) : null;
+                }
+            }
+            else if (values.containsKey(RM_AUDIT_DATA_LOGIN_USERNAME))
+            {
+                user = (String) values.get(RM_AUDIT_DATA_LOGIN_USERNAME);
+                if (values.containsKey(RM_AUDIT_DATA_LOGIN_ERROR))
+                {
+                    eventName = RM_AUDIT_EVENT_LOGIN_FAILURE;
+                    // The user didn't log in
+                    fullName = user;
+                }
+                else
+                {
+                    eventName = RM_AUDIT_EVENT_LOGIN_SUCCESS;
+                    fullName = (String) values.get(RM_AUDIT_DATA_LOGIN_FULLNAME);
+                }
+            }
+            else if (values.containsKey(DOD5015_AUDIT_DATA_LOGIN_USERNAME))
+            {
+                user = (String) values.get(DOD5015_AUDIT_DATA_LOGIN_USERNAME);
+                if (values.containsKey(DOD5015_AUDIT_DATA_LOGIN_ERROR))
+                {
+                    eventName = RM_AUDIT_EVENT_LOGIN_FAILURE;
+                    // The user didn't log in
+                    fullName = user;
+                }
+                else
+                {
+                    eventName = RM_AUDIT_EVENT_LOGIN_SUCCESS;
+                    fullName = (String) values.get(DOD5015_AUDIT_DATA_LOGIN_FULLNAME);
+                }
+            }
+            else
+            {
+                // This is not recognisable data
+                logger.warn(
+                        "Unable to process audit entry for RM.  Unexpected data: \n" +
+                        "   Entry: " + entryId + "\n" +
+                        "   Data:  " + values);
+                // Skip it
+                return true;
+            }
+
+            if (nodeRef != null && nodeService.exists(nodeRef))
+            {
+                if ((filePlanService.isFilePlanComponent(nodeRef) &&
+                        !AccessStatus.ALLOWED.equals(
+                                capabilityService.getCapabilityAccessState(nodeRef, ACCESS_AUDIT_CAPABILITY))) ||
+                        (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(nodeRef, PermissionService.READ))))
+                {
+                    return true;
+                }
+
+                checkPermissionIfHoldInProperties(beforeProperties);
+                checkPermissionIfHoldInProperties(afterProperties);
+            }
+
+            // remove any hold node refs from view
+            removeHoldNodeRefIfPresent(beforeProperties);
+            removeHoldNodeRefIfPresent(afterProperties);
+
+            // TODO: Refactor this to use the builder pattern
+            RecordsManagementAuditEntry entry = new RecordsManagementAuditEntry(
+                    timestamp,
+                    user,
+                    fullName,
+                    // A concatenated string of roles
+                    userRoles,
+                    nodeRef,
+                    nodeName,
+                    nodeType,
+                    eventName,
+                    nodeIdentifier,
+                    namePath,
+                    beforeProperties,
+                    afterProperties);
+
+            // write out the entry to the file in requested format
+            writeEntryToFile(entry);
+
+            if (results != null)
+            {
+                results.add(entry);
+            }
+
+            if (logger.isDebugEnabled())
+            {
+                logger.debug("   " + entry);
+            }
+
+            // Keep going
+            return true;
+        }
+
+        /**
+         * Helper method to check permission on the hold, if any, from the given event properties
+         * @param eventProperties   event properties
+         */
+        private void checkPermissionIfHoldInProperties(Map<QName, Serializable> eventProperties)
+        {
+            NodeRef holdNodeRef = eventProperties != null ? (NodeRef) eventProperties.get(PROPERTY_HOLD_NODEREF) : null;
+            if (holdNodeRef != null)
+            {
+                if (!AccessStatus.ALLOWED.equals(
+                        permissionService.hasPermission(holdNodeRef, PermissionService.READ)))
+                {
+                    eventProperties.replace(PROPERTY_HOLD_NAME, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG));
+                }
+            }
+        }
+
+        /**
+         * Helper method to remove the hold node ref, if any, from the given event properties
+         * @param eventProperties   event properties
+         */
+        private void removeHoldNodeRefIfPresent(Map<QName, Serializable> eventProperties)
+        {
+            if (eventProperties != null)
+            {
+                eventProperties.remove(PROPERTY_HOLD_NODEREF);
+            }
+        }
+
+        /**
+         * Helper method to write the audit entry to file
+         * @param entry      audit entry
+         */
+        private void writeEntryToFile(RecordsManagementAuditEntry entry)
+        {
+            if (writer == null)
+            {
+                return;
+            }
+            try
+            {
+                if (!firstEntry)
+                {
+                    if (reportFormat == ReportFormat.HTML)
+                    {
+                        writer.write("\n");
+                    }
+                    else
+                    {
+                        writer.write(",");
+                    }
+                }
+                else
+                {
+                    firstEntry = false;
+                }
+
+                // write the entry to the file
+                if (reportFormat == ReportFormat.JSON)
+                {
+                    writer.write("\n\t\t");
+                }
+
+                writeAuditTrailEntry(writer, entry, reportFormat);
+            }
+            catch (IOException ioe)
+            {
+                throw new AlfrescoRuntimeException(MSG_TRAIL_FILE_FAIL, ioe);
+            }
+        }
+    }
 }

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java

@@ -47,18 +47,22 @@ class HoldUtils
 {
     /** A QName to display for the hold name. */
     public static final QName HOLD_NAME = QName.createQName(RecordsManagementModel.RM_URI, "Hold Name");
+    /** A QName to display for the hold node ref. */
+    public static final QName HOLD_NODEREF = QName.createQName(RecordsManagementModel.RM_URI, "Hold NodeRef");
+
     /**
-     * Create a properties map containing the hold name for the given hold.
+     * Create a properties map containing the hold name and node ref for the given hold.
      *
      * @param nodeRef The nodeRef of the hold.
      * @param nodeService The node service.
-     * @return A map containing the name of the hold.
+     * @return A map containing the name and noderef of the hold.
      */
     static Map<QName, Serializable> makePropertiesMap(NodeRef nodeRef, NodeService nodeService)
     {
         Map<QName, Serializable> auditProperties = new HashMap<>();
 
         auditProperties.put(HOLD_NAME, nodeService.getProperty(nodeRef, ContentModel.PROP_NAME));
+        auditProperties.put(HOLD_NODEREF, nodeRef);
 
         return auditProperties;
     }

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java

@@ -43,7 +43,6 @@ import java.util.stream.Stream;
 
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
-import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditService;
 import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
 import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
@@ -119,9 +118,6 @@ public class HoldServiceImpl extends ServiceBaseImpl
     /** Permission service */
     private PermissionService permissionService;
 
-    /** records management audit service */
-    private RecordsManagementAuditService recordsManagementAuditService;
-
     /** Capability service */
     private CapabilityService capabilityService;
 
@@ -168,14 +164,6 @@ public class HoldServiceImpl extends ServiceBaseImpl
         this.permissionService = permissionService;
     }
 
-    /**
-     * @param recordsManagementAuditService records management audit service
-     */
-    public void setRecordsManagementAuditService(RecordsManagementAuditService recordsManagementAuditService)
-    {
-        this.recordsManagementAuditService = recordsManagementAuditService;
-    }
-
      /**
      * @param capabilityService capability service
      */