From fefbb15eeb9fdf801ce3f25783f7a64682d204ef Mon Sep 17 00:00:00 2001 From: Roy Wetherall Date: Wed, 25 Jun 2014 04:14:18 +0000 Subject: [PATCH] RM-1460: Hold Reson is not displayed in Audit Log RM-1505: Search Results: Hold Reason field is empty * specific filtering for ViewHoldReason capability removed .. no longer required as we specify user permissions at the hold level * deprecated hold related actions are no longer auditable (and so don't appear in the audit view) * hold service Audits add/remove to/from holds * hold reason is no longer rolled up onto disposable item .. it no longer makes sense for multiple holds and was a security hole * hold reason removed from search options git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@74611 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../messages/actions.properties | 3 + .../model/recordsModel.xml | 1 + .../rm-action-context.xml | 203 +++++++++--------- .../rm-service-context.xml | 4 +- .../security/rm-method-security.properties | 4 +- .../documentlibrary-v2/rm-filters.lib.js | 1 - .../capability/RMAfterInvocationProvider.java | 37 ---- .../capability/policy/ReadPropertyPolicy.java | 50 ----- .../hold/HoldServiceImpl.java | 44 +++- .../model/RecordsManagementModel.java | 2 + .../RecordsManagementSearchBehaviour.java | 64 ------ .../RecordsManagementSearchParameters.java | 1 - .../hold/HoldServiceImplUnitTest.java | 8 + .../test/util/BaseUnitTest.java | 2 + 14 files changed, 171 insertions(+), 253 deletions(-) delete mode 100644 rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/policy/ReadPropertyPolicy.java diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/messages/actions.properties b/rm-server/config/alfresco/module/org_alfresco_module_rm/messages/actions.properties index 1829e21563..690be465e8 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/messages/actions.properties +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/messages/actions.properties @@ -178,6 +178,9 @@ addRecordTypes.description=Adds the selected type(s) to the record # File report fileReport.title=File report fileReport.description=File report +# Delete Hold +deleteHold.title=Delete Hold +deleteHold.description=Delete hold # Action parameter constraints rm-ac-is-kind-kinds.record_category=Record Category diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/model/recordsModel.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/model/recordsModel.xml index fc5f506434..71ea265afe 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/model/recordsModel.xml +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/model/recordsModel.xml @@ -1024,6 +1024,7 @@ d:text true + d:text true diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml index 6ab10bc8e8..ae95fdc820 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-action-context.xml @@ -413,102 +413,7 @@ - - - - - - - - - - - - - - - - - org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.execute=RM_CAP.0.rma:filePlanComponent.ExtendRetentionPeriodOrFreeze - org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.*=RM_ALLOW - org.alfresco.repo.action.executer.ActionExecuter.*=RM_ALLOW - - - - - - - - - - - - - - - - - - - - - - - - - org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.execute=RM_CAP.0.rma:filePlanComponent.Unfreeze - org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.*=RM_ALLOW - org.alfresco.repo.action.executer.ActionExecuter.*=RM_ALLOW - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - - org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.execute=RM_CAP.0.rma:filePlanComponent.ViewUpdateReasonsForFreeze - org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.*=RM_ALLOW - org.alfresco.repo.action.executer.ActionExecuter.*=RM_ALLOW - - - - - + @@ -1111,5 +1016,111 @@ + + + + + + + + + + + + + + + + + + + + + org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.execute=RM_CAP.0.rma:filePlanComponent.ExtendRetentionPeriodOrFreeze + org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.*=RM_ALLOW + org.alfresco.repo.action.executer.ActionExecuter.*=RM_ALLOW + + + + + + + + + + + + + + + + + + + + + + + + + + org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.execute=RM_CAP.0.rma:filePlanComponent.Unfreeze + org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.*=RM_ALLOW + org.alfresco.repo.action.executer.ActionExecuter.*=RM_ALLOW + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.execute=RM_CAP.0.rma:filePlanComponent.ViewUpdateReasonsForFreeze + org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementAction.*=RM_ALLOW + org.alfresco.repo.action.executer.ActionExecuter.*=RM_ALLOW + + + + + + + + \ No newline at end of file diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index 3f9c036e60..c884558fad 100644 --- a/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -1418,11 +1418,13 @@ + class="org.alfresco.module.org_alfresco_module_rm.hold.HoldServiceImpl" + init-method="init"> + . - */ -package org.alfresco.module.org_alfresco_module_rm.capability.policy; - -import net.sf.acegisecurity.vote.AccessDecisionVoter; - -import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel; -import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; -import org.alfresco.service.cmr.repository.NodeRef; -import org.alfresco.service.namespace.QName; -import org.aopalliance.intercept.MethodInvocation; - -public class ReadPropertyPolicy extends AbstractBasePolicy -{ - - @SuppressWarnings("rawtypes") - public int evaluate( - MethodInvocation invocation, - Class[] params, - ConfigAttributeDefinition cad) - { - NodeRef nodeRef = getTestNode(invocation, params, cad.getParameters().get(0), cad.isParent()); - QName propertyQName = getQName(invocation, params, cad.getParameters().get(1)); - if(propertyQName.equals(RecordsManagementModel.PROP_HOLD_REASON)) - { - return getCapabilityService().getCapability(RMPermissionModel.VIEW_UPDATE_REASONS_FOR_FREEZE).evaluate(nodeRef); - } - else - { - return AccessDecisionVoter.ACCESS_GRANTED; - } - } - -} \ No newline at end of file diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java index be71499da3..360b064fbe 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImpl.java @@ -30,6 +30,8 @@ import java.util.Set; 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.audit.event.AuditEvent; import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel; import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService; import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel; @@ -71,6 +73,10 @@ public class HoldServiceImpl extends ServiceBaseImpl { /** Logger */ private static Log logger = LogFactory.getLog(HoldServiceImpl.class); + + /** Audit event keys */ + private static final String AUDIT_ADD_TO_HOLD = "addToHold"; + private static final String AUDIT_REMOVE_FROM_HOLD = "removeFromHold"; /** File Plan Service */ private FilePlanService filePlanService; @@ -83,6 +89,9 @@ public class HoldServiceImpl extends ServiceBaseImpl /** Permission service */ private PermissionService permissionService; + + /** records management audit service */ + private RecordsManagementAuditService recordsManagementAuditService; /** * Set the file plan service @@ -133,6 +142,31 @@ public class HoldServiceImpl extends ServiceBaseImpl { this.permissionService = permissionService; } + + /** + * @param recordsManagementAuditService records management audit service + */ + public void setRecordsManagementAuditService(RecordsManagementAuditService recordsManagementAuditService) + { + this.recordsManagementAuditService = recordsManagementAuditService; + } + + /** + * Initialise hold service + */ + public void init() + { + AuthenticationUtil.runAsSystem(new RunAsWork() + { + @Override + public Void doWork() throws Exception + { + recordsManagementAuditService.registerAuditEvent(new AuditEvent(AUDIT_ADD_TO_HOLD, "capability.AddToHold.title")); + recordsManagementAuditService.registerAuditEvent(new AuditEvent(AUDIT_REMOVE_FROM_HOLD, "capability.RemoveFromHold.title")); + return null; + } + }); + } /** * Behaviour unfreezes node's that will no longer he held after delete. @@ -555,6 +589,9 @@ public class HoldServiceImpl extends ServiceBaseImpl // Link the record to the hold nodeService.addChild(hold, nodeRef, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); + + // audit item being added to the hold + recordsManagementAuditService.auditEvent(nodeRef, AUDIT_ADD_TO_HOLD); // Mark all the folders contents as frozen if (isRecordFolder(nodeRef)) @@ -657,6 +694,11 @@ public class HoldServiceImpl extends ServiceBaseImpl { // remove from hold nodeService.removeChild(hold, nodeRef); + + // audit that the node has been remove from the hold + // TODO add details of the hold that the node was removed from + recordsManagementAuditService.auditEvent(nodeRef, AUDIT_REMOVE_FROM_HOLD); + return null; } }); @@ -672,7 +714,7 @@ public class HoldServiceImpl extends ServiceBaseImpl removeFreezeAspect(nodeRef, 0); return null; } - }); + }); } } diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/RecordsManagementModel.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/RecordsManagementModel.java index ea5cf3cf75..00887912cc 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/RecordsManagementModel.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/RecordsManagementModel.java @@ -233,6 +233,8 @@ public interface RecordsManagementModel extends RecordsManagementCustomModel QName PROP_RS_HAS_DISPOITION_SCHEDULE = QName.createQName(RM_URI, "recordSearchHasDispositionSchedule"); QName PROP_RS_DISPOITION_INSTRUCTIONS = QName.createQName(RM_URI, "recordSearchDispositionInstructions"); QName PROP_RS_DISPOITION_AUTHORITY = QName.createQName(RM_URI, "recordSearchDispositionAuthority"); + /** @depreacted as of 2.2, because disposable items can now be in multiple holds */ + @Deprecated QName PROP_RS_HOLD_REASON = QName.createQName(RM_URI, "recordSearchHoldReason"); // Loaded Data Set Ids diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/behaviour/RecordsManagementSearchBehaviour.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/behaviour/RecordsManagementSearchBehaviour.java index ab34232acd..5b5c8ad80a 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/behaviour/RecordsManagementSearchBehaviour.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/behaviour/RecordsManagementSearchBehaviour.java @@ -218,17 +218,6 @@ public class RecordsManagementSearchBehaviour implements RecordsManagementModel QName.createQName(NamespaceService.ALFRESCO_URI, "onUpdateProperties"), ASPECT_VITAL_RECORD_DEFINITION, new JavaBehaviour(this, "vitalRecordDefintionUpdateProperties", NotificationFrequency.TRANSACTION_COMMIT)); - - // Hold reason rollup - this.policyComponent.bindClassBehaviour( - QName.createQName(NamespaceService.ALFRESCO_URI, "onRemoveAspect"), - ASPECT_FROZEN, - new JavaBehaviour(this, "onRemoveFrozenAspect", NotificationFrequency.TRANSACTION_COMMIT)); - - this.policyComponent.bindClassBehaviour( - QName.createQName(NamespaceService.ALFRESCO_URI, "onUpdateProperties"), - TYPE_HOLD, - new JavaBehaviour(this, "frozenAspectUpdateProperties", NotificationFrequency.TRANSACTION_COMMIT)); } /** @@ -716,59 +705,6 @@ public class RecordsManagementSearchBehaviour implements RecordsManagementModel } } - /** - * On remove frozen aspect aspect behaviour implementation - * - * @param nodeRef node reference - * @param aspectTypeQName aspect type qname - */ - public void onRemoveFrozenAspect(NodeRef nodeRef, QName aspectTypeQName) - { - if (nodeService.exists(nodeRef) && - nodeService.hasAspect(nodeRef, ASPECT_RM_SEARCH)) - { - nodeService.setProperty(nodeRef, PROP_RS_HOLD_REASON, null); - } - } - - /** - * Frozen aspect properties update behavour implementation. - * - * @param nodeRef node reference - * @param before before properties - * @param after after proeprties - */ - public void frozenAspectUpdateProperties(final NodeRef nodeRef, final Map before, final Map after) - { - AuthenticationUtil.RunAsWork work = new AuthenticationUtil.RunAsWork() - { - @Override - public Void doWork() - { - if (nodeService.exists(nodeRef)) - { - // get the changed hold reason - String holdReason = (String)nodeService.getProperty(nodeRef, PROP_HOLD_REASON); - - // get all the frozen items the hold node has and change the hold reason - List holdAssocs = nodeService.getChildAssocs(nodeRef, ASSOC_FROZEN_RECORDS, RegexQNamePattern.MATCH_ALL); - for (ChildAssociationRef assoc : holdAssocs) - { - NodeRef frozenItem = assoc.getChildRef(); - - // ensure the search aspect is applied and set the hold reason - applySearchAspect(frozenItem); - nodeService.setProperty(frozenItem, PROP_RS_HOLD_REASON, holdReason); - } - } - - return null; - } - }; - - AuthenticationUtil.runAs(work, AuthenticationUtil.getSystemUserName()); - } - /** * Updates the disposition schedule properties * diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/search/RecordsManagementSearchParameters.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/search/RecordsManagementSearchParameters.java index 286cb02166..547210095c 100644 --- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/search/RecordsManagementSearchParameters.java +++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/search/RecordsManagementSearchParameters.java @@ -68,7 +68,6 @@ public class RecordsManagementSearchParameters put("hasDispositionSchedule", "%(rma:recordSearchHasDispositionSchedule)"); put("dispositionInstructions", "%(rma:recordSearchDispositionInstructions)"); put("dispositionAuthority", "%(rma:recordSearchDispositionAuthority)"); - put("holdReason", "%(rma:recordSearchHoldReason)"); put("vitalRecordReviewPeriod", "%(rma:recordSearchVitalRecordReviewPeriod)"); } }; diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java index 5a33ecc131..a275d7260b 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/hold/HoldServiceImplUnitTest.java @@ -288,11 +288,13 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString()); holdService.addToHold(hold, record); verify(mockedNodeService).addChild(hold, record, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, times(2)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(record), anyString()); } @SuppressWarnings("unchecked") @@ -306,6 +308,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, never()).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(recordFolder), anyString()); } @SuppressWarnings("unchecked") @@ -320,6 +323,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).addChild(hold, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService, never()).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, never()).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString()); } @SuppressWarnings("unchecked") @@ -351,6 +355,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).addChild(hold2, recordFolder, ASSOC_FROZEN_RECORDS, ASSOC_FROZEN_RECORDS); verify(mockedNodeService, times(1)).addAspect(eq(recordFolder), eq(ASPECT_FROZEN), any(Map.class)); verify(mockedNodeService, times(1)).addAspect(eq(record), eq(ASPECT_FROZEN), any(Map.class)); + verify(mockedRecordsManagementAuditService, times(2)).auditEvent(eq(recordFolder), anyString()); } @Test (expected=AlfrescoRuntimeException.class) @@ -367,6 +372,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, never()).removeChild(hold, recordFolder); verify(mockedNodeService, never()).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, never()).removeAspect(record, ASPECT_FROZEN); + verify(mockedRecordsManagementAuditService, never()).auditEvent(eq(recordFolder), anyString()); } @Test @@ -381,6 +387,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).removeChild(hold, recordFolder); verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN); + verify(mockedRecordsManagementAuditService, times(1)).auditEvent(eq(recordFolder), anyString()); } @Test @@ -402,6 +409,7 @@ public class HoldServiceImplUnitTest extends BaseUnitTest verify(mockedNodeService, times(1)).removeChild(hold2, recordFolder); verify(mockedNodeService, times(1)).removeAspect(recordFolder, ASPECT_FROZEN); verify(mockedNodeService, times(1)).removeAspect(record, ASPECT_FROZEN); + verify(mockedRecordsManagementAuditService, times(2)).auditEvent(any(NodeRef.class), anyString()); } @Test diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseUnitTest.java index 04f0a971e5..1686c07440 100644 --- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseUnitTest.java +++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseUnitTest.java @@ -31,6 +31,7 @@ import java.util.List; import org.alfresco.model.ContentModel; import org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementActionService; +import org.alfresco.module.org_alfresco_module_rm.audit.RecordsManagementAuditService; 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.identifier.IdentifierService; @@ -103,6 +104,7 @@ public class BaseUnitTest implements RecordsManagementModel @Mock(name="recordsManagementActionService") protected RecordsManagementActionService mockedRecordsManagementActionService; @Mock(name="reportService") protected ReportService mockedReportService; @Mock(name="filePlanRoleService") protected FilePlanRoleService mockedFilePlanRoleService; + @Mock(name="recordsManagementAuditService") protected RecordsManagementAuditService mockedRecordsManagementAuditService; /** application context mock */ @Mock(name="applicationContext") protected ApplicationContext mockedApplicationContext;