From 5d129ce43aa571da306352300bb9feebcf7434ee Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Thu, 5 Dec 2019 05:11:53 +0000 Subject: [PATCH 1/7] RM-7062 Check hold permission for view audit event --- .../rm-service-context.xml | 2 +- .../RecordsManagementAuditServiceImpl.java | 79 +++++++++++++++++-- .../hold/HoldServiceImpl.java | 12 --- 3 files changed, 73 insertions(+), 20 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index ccaba88ffc..3b5c7eee06 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -941,6 +941,7 @@ + cm:lastThumbnailModification @@ -1533,7 +1534,6 @@ - diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java index 770d05d3b1..9d191de947 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java @@ -47,6 +47,7 @@ import java.util.Calendar; import java.util.Collections; import java.util.Date; import java.util.HashMap; +import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -60,10 +61,14 @@ 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; import org.alfresco.repo.policy.PolicyComponent; +import org.alfresco.repo.security.authentication.AuthenticationUtil; +import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; @@ -194,6 +199,8 @@ 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 PolicyComponent policyComponent; private DictionaryService dictionaryService; private TransactionService transactionService; @@ -207,6 +214,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean private NamespaceService namespaceService; protected CapabilityService capabilityService; protected PermissionService permissionService; + protected HoldService holdService; private boolean shutdown = false; @@ -332,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) */ @@ -686,7 +703,8 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean /** * Helper method to remove system properties from maps * - * @param properties + * @param before + * @param after */ private void removeAuditProperties(Map before, Map after) { @@ -997,13 +1015,33 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean 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))))) + if (nodeRef != null && nodeService.exists(nodeRef)) { - return true; + if ((filePlanService.isFilePlanComponent(nodeRef) && + !AccessStatus.ALLOWED.equals( + capabilityService.getCapabilityAccessState(nodeRef, ACCESS_AUDIT_CAPABILITY))) || + (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(nodeRef, PermissionService.READ)))) + { + return true; + } + // must have read permission on hold to see hold events + else + { + // get hold names, if any, from event properties + Set holdNames = new HashSet<>(2); + addHoldNameFromProperties(holdNames, beforeProperties); + addHoldNameFromProperties(holdNames, afterProperties); + + // check permission for all hold names found in event properties + for (String holdName: holdNames) + { + if (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(getHold(holdName), + PermissionService.READ))) + { + return true; + } + } + } } // TODO: Refactor this to use the builder pattern @@ -1039,6 +1077,33 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean return true; } + /** + * Helper method to extract the hold name, if any, from the given event properties + * @param holdNames set of hold names + * @param eventProperties event properties + */ + private void addHoldNameFromProperties(Set holdNames, Map eventProperties) + { + String name = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; + if (name != null) + { + holdNames.add(name); + } + } + + /** + * Helper method to get the hold for a given hold name + * @param holdName hold name + * @return node ref of hold + */ + private NodeRef getHold(String holdName) + { + return AuthenticationUtil.runAsSystem(() -> { + NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); + return holdService.getHold(filePlan, holdName); + }); + } + private void writeEntryToFile(RecordsManagementAuditEntry entry) { if (writer == null) 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 756dda7b85..1eea10cee3 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 @@ -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 */ From 740e9fda42574d51ffc9f669cf6d1623820ae2e8 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Fri, 13 Dec 2019 07:21:35 +0000 Subject: [PATCH 2/7] RM-7062 Unit tests --- .../messages/audit-service.properties | 1 + .../RecordsManagementAuditServiceImpl.java | 541 +++++++++--------- 2 files changed, 277 insertions(+), 265 deletions(-) diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties index b5e7c4079c..acafc32979 100644 --- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/messages/audit-service.properties +++ b/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 diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java index 9d191de947..2706f9a01c 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java @@ -47,7 +47,6 @@ import java.util.Calendar; import java.util.Collections; import java.util.Date; import java.util.HashMap; -import java.util.HashSet; import java.util.List; import java.util.Locale; import java.util.Map; @@ -68,7 +67,6 @@ import org.alfresco.repo.audit.model.AuditApplication; import org.alfresco.repo.content.MimetypeMap; import org.alfresco.repo.policy.PolicyComponent; import org.alfresco.repo.security.authentication.AuthenticationUtil; -import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; @@ -200,6 +198,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean 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 String HOLD_PERMISSION_DENIED_MSG = "rm.audit.holdPermission-Error"; private PolicyComponent policyComponent; private DictionaryService dictionaryService; @@ -881,267 +880,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean } // define the callback - AuditQueryCallback callback = new AuditQueryCallback() - { - 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 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 beforeProperties = null; - Map 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) values.get(RM_AUDIT_DATA_NODE_CHANGES_BEFORE); - afterProperties = (Map) 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) values.get( DOD5015_AUDIT_DATA_NODE_CHANGES_BEFORE); - afterProperties = (Map) 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)) - { - if ((filePlanService.isFilePlanComponent(nodeRef) && - !AccessStatus.ALLOWED.equals( - capabilityService.getCapabilityAccessState(nodeRef, ACCESS_AUDIT_CAPABILITY))) || - (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(nodeRef, PermissionService.READ)))) - { - return true; - } - // must have read permission on hold to see hold events - else - { - // get hold names, if any, from event properties - Set holdNames = new HashSet<>(2); - addHoldNameFromProperties(holdNames, beforeProperties); - addHoldNameFromProperties(holdNames, afterProperties); - - // check permission for all hold names found in event properties - for (String holdName: holdNames) - { - if (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(getHold(holdName), - 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; - } - - /** - * Helper method to extract the hold name, if any, from the given event properties - * @param holdNames set of hold names - * @param eventProperties event properties - */ - private void addHoldNameFromProperties(Set holdNames, Map eventProperties) - { - String name = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; - if (name != null) - { - holdNames.add(name); - } - } - - /** - * Helper method to get the hold for a given hold name - * @param holdName hold name - * @return node ref of hold - */ - private NodeRef getHold(String holdName) - { - return AuthenticationUtil.runAsSystem(() -> { - NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); - return holdService.getHold(filePlan, holdName); - }); - } - - 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); - } - } - }; + AuditQueryCallback callback = new AuditTrailQueryCallback(results, writer, reportFormat); String user = params.getUser(); Long fromTime = getFromDateTime(params.getDateFrom()); @@ -1238,7 +977,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. @@ -1280,7 +1019,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. @@ -1968,4 +1707,276 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean { auditEvent(nodeRef, action.getName()); } + + private class AuditTrailQueryCallback implements AuditQueryCallback + { + private final List results; + private final Writer writer; + private final ReportFormat reportFormat; + private boolean firstEntry; + + public AuditTrailQueryCallback(List 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 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 beforeProperties = null; + Map 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) values.get(RM_AUDIT_DATA_NODE_CHANGES_BEFORE); + afterProperties = (Map) 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) values.get( DOD5015_AUDIT_DATA_NODE_CHANGES_BEFORE); + afterProperties = (Map) 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)) + { + if ((filePlanService.isFilePlanComponent(nodeRef) && + !AccessStatus.ALLOWED.equals( + capabilityService.getCapabilityAccessState(nodeRef, ACCESS_AUDIT_CAPABILITY))) || + (!AccessStatus.ALLOWED.equals(permissionService.hasPermission(nodeRef, PermissionService.READ)))) + { + return true; + } + else + { + // must have read permission on hold to see hold name in event + Map holdNamesToHide = new HashMap<>(2); + replaceHoldNameIfRequired(beforeProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + replaceHoldNameIfRequired(afterProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + } + } + + // 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 replace the hold name, if required, in the given event properties + * @param eventProperties event properties + * @param holdNamesToHide map of hold names and their hide name status + * @param replacementText text to replace hidden hold name + */ + private void replaceHoldNameIfRequired(Map eventProperties, + Map holdNamesToHide, String replacementText) + { + // get hold name, if any, from audit event properties + String holdName = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; + if (holdName != null) + { + holdNamesToHide.putIfAbsent(holdName, + !AccessStatus.ALLOWED.equals(permissionService.hasPermission(getHold(holdName), + PermissionService.READ))); + + if (holdNamesToHide.get(holdName) == true) + { + eventProperties.replace(PROPERTY_HOLD_NAME, replacementText); + } + } + } + + /** + * Helper method to get the hold for a given hold name + * @param holdName hold name + * @return node ref of hold + */ + private NodeRef getHold(String holdName) + { + return AuthenticationUtil.runAsSystem(() -> { + NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); + return holdService.getHold(filePlan, holdName); + }); + } + + 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); + } + } + } } From f4dce089df9a098cb875f7ba4aab90662b4dcdb0 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Sun, 15 Dec 2019 07:17:59 +0000 Subject: [PATCH 3/7] RM-7062 Updates from review --- .../audit/RecordsManagementAuditServiceImpl.java | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java index 2706f9a01c..f309c0a8ce 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java @@ -702,8 +702,8 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean /** * Helper method to remove system properties from maps * - * @param before - * @param after + * @param before properties before event + * @param after properties after event */ private void removeAuditProperties(Map before, Map after) { @@ -1783,7 +1783,6 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean afterProperties = (Map) 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); @@ -1792,7 +1791,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean } else if (values.containsKey(DOD5015_AUDIT_DATA_EVENT_NAME)) { - // This data is /RM/event/... + // 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); @@ -1805,7 +1804,6 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean afterProperties = (Map) 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); @@ -1921,7 +1919,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean !AccessStatus.ALLOWED.equals(permissionService.hasPermission(getHold(holdName), PermissionService.READ))); - if (holdNamesToHide.get(holdName) == true) + if (holdNamesToHide.get(holdName)) { eventProperties.replace(PROPERTY_HOLD_NAME, replacementText); } @@ -1941,6 +1939,10 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean }); } + /** + * Helper method to write the audit entry to file + * @param entry audit entry + */ private void writeEntryToFile(RecordsManagementAuditEntry entry) { if (writer == null) From 0e743a8b31481ab0c98dbefb292d090603ceb71d Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Sun, 15 Dec 2019 07:18:33 +0000 Subject: [PATCH 4/7] RM-7062 Fix automation tests --- .../community/audit/AuditAddToHoldTests.java | 49 +++++++++++------- .../audit/AuditRemoveFromHoldTests.java | 51 ++++++++++++------- 2 files changed, 62 insertions(+), 38 deletions(-) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditAddToHoldTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditAddToHoldTests.java index 0d63ac5002..9a5e03def8 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditAddToHoldTests.java +++ b/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") - public void addToHoldAuditEntryNotVisible(UserModel user) + @Test + 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) diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditRemoveFromHoldTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditRemoveFromHoldTests.java index 7fa655f79b..13905a6208 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/audit/AuditRemoveFromHoldTests.java +++ b/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") - public void removeFromHoldAuditEntryNotVisible(UserModel user) + @Test + 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) From 2c3a3e3726352481ffbbea0a99abfc82b5728fb2 Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Tue, 17 Dec 2019 08:37:15 +0000 Subject: [PATCH 5/7] RM-7062 fix integration tests --- .../RecordsManagementAuditServiceImpl.java | 74 ++++++++++++++++--- 1 file changed, 65 insertions(+), 9 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java index f78f5f2f8b..dbf57693e6 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java @@ -1871,10 +1871,29 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean } else { - // must have read permission on hold to see hold name in event + // find any hold names in event properties Map holdNamesToHide = new HashMap<>(2); - replaceHoldNameIfRequired(beforeProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); - replaceHoldNameIfRequired(afterProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + buildHoldNamesToHide(holdNamesToHide, beforeProperties); + buildHoldNamesToHide(holdNamesToHide, afterProperties); + + // if audit event contains any hold names, check if any hold name should be hidden + if (!holdNamesToHide.isEmpty()) + { + // get file plan + NodeRef filePlan = getFilePlan(nodeRef); + + // check permissions for each hold name + for (String holdName : holdNamesToHide.keySet()) + { + boolean hideHoldName = !AccessStatus.ALLOWED.equals( + permissionService.hasPermission(getHold(filePlan, holdName), PermissionService.READ)); + holdNamesToHide.replace(holdName, hideHoldName); + } + + // hide hold names, if required, in event properties + replaceHoldNameIfRequired(beforeProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + replaceHoldNameIfRequired(afterProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + } } } @@ -1911,6 +1930,46 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean return true; } + /** + * Helper method to extract the hold name, if any, from the given event properties + * @param holdNamesToHide map of hold names and their hide name status. True if hold name is to be hidden. + * @param eventProperties event properties + */ + private void buildHoldNamesToHide(Map holdNamesToHide, Map eventProperties) + { + // get hold name, if any, from audit event properties + String holdName = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; + if (holdName != null) + { + // assume hold name should be hidden + holdNamesToHide.putIfAbsent(holdName, true); + } + } + + /** + * Helper method to get the file plan + * @param nodeRef node ref for which to get file plan + * @return node ref of file plan + */ + private NodeRef getFilePlan(NodeRef nodeRef) + { + NodeRef filePlan = filePlanService.getFilePlan(nodeRef); + if (filePlan == null) + { + Set filePlans = filePlanService.getFilePlans(); + if (filePlans != null && !filePlans.isEmpty()) + { + filePlan = filePlans.iterator().next(); + } + + if (filePlan == null) + { + filePlan = getDefaultFilePlan(); + } + } + return filePlan; + } + /** * Helper method to replace the hold name, if required, in the given event properties * @param eventProperties event properties @@ -1924,10 +1983,6 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean String holdName = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; if (holdName != null) { - holdNamesToHide.putIfAbsent(holdName, - !AccessStatus.ALLOWED.equals(permissionService.hasPermission(getHold(holdName), - PermissionService.READ))); - if (holdNamesToHide.get(holdName)) { eventProperties.replace(PROPERTY_HOLD_NAME, replacementText); @@ -1937,13 +1992,14 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean /** * Helper method to get the hold for a given hold name + * @param filePlan file plan * @param holdName hold name * @return node ref of hold */ - private NodeRef getHold(String holdName) + private NodeRef getHold(NodeRef filePlan, String holdName) { return AuthenticationUtil.runAsSystem(() -> { - NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); + //NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); return holdService.getHold(filePlan, holdName); }); } From 105da8eecfa8b755e3ff6558029dc9e4770f036e Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Thu, 19 Dec 2019 22:46:07 +0000 Subject: [PATCH 6/7] RM-7062 add noderef to audit entry for hold --- .../RecordsManagementAuditServiceImpl.java | 44 +++++++++---------- .../audit/event/HoldUtils.java | 8 +++- 2 files changed, 28 insertions(+), 24 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java index dbf57693e6..048942cd88 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java @@ -199,6 +199,7 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean 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; @@ -1869,31 +1870,30 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean { return true; } - else + + // get hold nodeRef, if any, from audit event properties + NodeRef holdNodeRef = beforeProperties != null ? (NodeRef) beforeProperties.get(PROPERTY_HOLD_NODEREF) : null; + if (holdNodeRef != null) { - // find any hold names in event properties - Map holdNamesToHide = new HashMap<>(2); - buildHoldNamesToHide(holdNamesToHide, beforeProperties); - buildHoldNamesToHide(holdNamesToHide, afterProperties); - - // if audit event contains any hold names, check if any hold name should be hidden - if (!holdNamesToHide.isEmpty()) + if (!AccessStatus.ALLOWED.equals( + permissionService.hasPermission(holdNodeRef, PermissionService.READ))) { - // get file plan - NodeRef filePlan = getFilePlan(nodeRef); - - // check permissions for each hold name - for (String holdName : holdNamesToHide.keySet()) - { - boolean hideHoldName = !AccessStatus.ALLOWED.equals( - permissionService.hasPermission(getHold(filePlan, holdName), PermissionService.READ)); - holdNamesToHide.replace(holdName, hideHoldName); - } - - // hide hold names, if required, in event properties - replaceHoldNameIfRequired(beforeProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); - replaceHoldNameIfRequired(afterProperties, holdNamesToHide, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + beforeProperties.replace(PROPERTY_HOLD_NAME, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); } + // remove hold node ref from view + beforeProperties.remove(PROPERTY_HOLD_NODEREF); + } + + holdNodeRef = afterProperties != null ? (NodeRef) afterProperties.get(PROPERTY_HOLD_NODEREF) : null; + if (holdNodeRef != null) + { + if (!AccessStatus.ALLOWED.equals( + permissionService.hasPermission(holdNodeRef, PermissionService.READ))) + { + afterProperties.replace(PROPERTY_HOLD_NAME, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); + } + // remove hold node ref from view + afterProperties.remove(PROPERTY_HOLD_NODEREF); } } diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java index 641b7f016d..d8faecc72e 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/event/HoldUtils.java +++ b/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 makePropertiesMap(NodeRef nodeRef, NodeService nodeService) { Map auditProperties = new HashMap<>(); auditProperties.put(HOLD_NAME, nodeService.getProperty(nodeRef, ContentModel.PROP_NAME)); + auditProperties.put(HOLD_NODEREF, nodeRef); return auditProperties; } From b35dcb14a3359abb170473f8948525dfd4b8990c Mon Sep 17 00:00:00 2001 From: Sara Aspery Date: Fri, 20 Dec 2019 02:20:13 +0000 Subject: [PATCH 7/7] RM-7062 fix for failing tests --- .../RecordsManagementAuditServiceImpl.java | 104 ++++-------------- 1 file changed, 20 insertions(+), 84 deletions(-) diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java index 048942cd88..924c4ec25d 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/audit/RecordsManagementAuditServiceImpl.java @@ -67,7 +67,6 @@ import org.alfresco.repo.audit.AuditComponent; import org.alfresco.repo.audit.model.AuditApplication; import org.alfresco.repo.content.MimetypeMap; import org.alfresco.repo.policy.PolicyComponent; -import org.alfresco.repo.security.authentication.AuthenticationUtil; import org.alfresco.repo.transaction.AlfrescoTransactionSupport; import org.alfresco.repo.transaction.RetryingTransactionHelper; import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback; @@ -1871,32 +1870,14 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean return true; } - // get hold nodeRef, if any, from audit event properties - NodeRef holdNodeRef = beforeProperties != null ? (NodeRef) beforeProperties.get(PROPERTY_HOLD_NODEREF) : null; - if (holdNodeRef != null) - { - if (!AccessStatus.ALLOWED.equals( - permissionService.hasPermission(holdNodeRef, PermissionService.READ))) - { - beforeProperties.replace(PROPERTY_HOLD_NAME, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); - } - // remove hold node ref from view - beforeProperties.remove(PROPERTY_HOLD_NODEREF); - } - - holdNodeRef = afterProperties != null ? (NodeRef) afterProperties.get(PROPERTY_HOLD_NODEREF) : null; - if (holdNodeRef != null) - { - if (!AccessStatus.ALLOWED.equals( - permissionService.hasPermission(holdNodeRef, PermissionService.READ))) - { - afterProperties.replace(PROPERTY_HOLD_NAME, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); - } - // remove hold node ref from view - afterProperties.remove(PROPERTY_HOLD_NODEREF); - } + 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, @@ -1931,77 +1912,32 @@ public class RecordsManagementAuditServiceImpl extends AbstractLifecycleBean } /** - * Helper method to extract the hold name, if any, from the given event properties - * @param holdNamesToHide map of hold names and their hide name status. True if hold name is to be hidden. + * Helper method to check permission on the hold, if any, from the given event properties * @param eventProperties event properties */ - private void buildHoldNamesToHide(Map holdNamesToHide, Map eventProperties) + private void checkPermissionIfHoldInProperties(Map eventProperties) { - // get hold name, if any, from audit event properties - String holdName = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; - if (holdName != null) + NodeRef holdNodeRef = eventProperties != null ? (NodeRef) eventProperties.get(PROPERTY_HOLD_NODEREF) : null; + if (holdNodeRef != null) { - // assume hold name should be hidden - holdNamesToHide.putIfAbsent(holdName, true); - } - } - - /** - * Helper method to get the file plan - * @param nodeRef node ref for which to get file plan - * @return node ref of file plan - */ - private NodeRef getFilePlan(NodeRef nodeRef) - { - NodeRef filePlan = filePlanService.getFilePlan(nodeRef); - if (filePlan == null) - { - Set filePlans = filePlanService.getFilePlans(); - if (filePlans != null && !filePlans.isEmpty()) - { - filePlan = filePlans.iterator().next(); - } - - if (filePlan == null) - { - filePlan = getDefaultFilePlan(); - } - } - return filePlan; - } - - /** - * Helper method to replace the hold name, if required, in the given event properties - * @param eventProperties event properties - * @param holdNamesToHide map of hold names and their hide name status - * @param replacementText text to replace hidden hold name - */ - private void replaceHoldNameIfRequired(Map eventProperties, - Map holdNamesToHide, String replacementText) - { - // get hold name, if any, from audit event properties - String holdName = eventProperties != null ? (String) eventProperties.get(PROPERTY_HOLD_NAME) : null; - if (holdName != null) - { - if (holdNamesToHide.get(holdName)) + if (!AccessStatus.ALLOWED.equals( + permissionService.hasPermission(holdNodeRef, PermissionService.READ))) { - eventProperties.replace(PROPERTY_HOLD_NAME, replacementText); + eventProperties.replace(PROPERTY_HOLD_NAME, I18NUtil.getMessage(HOLD_PERMISSION_DENIED_MSG)); } } } /** - * Helper method to get the hold for a given hold name - * @param filePlan file plan - * @param holdName hold name - * @return node ref of hold + * Helper method to remove the hold node ref, if any, from the given event properties + * @param eventProperties event properties */ - private NodeRef getHold(NodeRef filePlan, String holdName) + private void removeHoldNodeRefIfPresent(Map eventProperties) { - return AuthenticationUtil.runAsSystem(() -> { - //NodeRef filePlan = filePlanService.getFilePlanBySiteId(FilePlanService.DEFAULT_RM_SITE_ID); - return holdService.getHold(filePlan, holdName); - }); + if (eventProperties != null) + { + eventProperties.remove(PROPERTY_HOLD_NODEREF); + } } /**