From 23182c909db907e98bde834b20aabb50f9b23c7f Mon Sep 17 00:00:00 2001 From: Alan Davis Date: Wed, 2 Apr 2014 19:53:27 +0000 Subject: [PATCH] Merged HEAD-BUG-FIX (4.3/Cloud) to HEAD (4.3/Cloud) 64785: Merged V4.2-BUG-FIX (4.2.2) to HEAD-BUG-FIX (4.3/Cloud) 64131: Merged DEV to V4.2-BUG-FIX (4.2.2) 63233: MNT-10767: Guard in AuditMethodInterceptor is too restrictive preventing subordinate service calls from producing data. - Modify AuditMethodInterceptor to audit subordinate public services 64123: MNT-10767: Guard in AuditMethodInterceptor is too restrictive preventing subordinate service calls from producing data. - Add unit test git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/alfresco/HEAD/root@66188 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261 --- .../repo/audit/AuditMethodInterceptor.java | 19 +++- .../repo/audit/AuditComponentTest.java | 94 ++++++++++++++++++- .../audit/TestCreateFileFolderService.java | 1 + .../TestCreateFileFolderServiceImpl.java | 1 + .../alfresco-audit-test-mnt-10767-context.xml | 24 +++++ .../alfresco-audit-test-mnt-10767.xml | 26 +++++ 6 files changed, 159 insertions(+), 6 deletions(-) create mode 100644 source/test-java/org/alfresco/repo/audit/TestCreateFileFolderService.java create mode 100644 source/test-java/org/alfresco/repo/audit/TestCreateFileFolderServiceImpl.java create mode 100644 source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml create mode 100644 source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767.xml diff --git a/source/java/org/alfresco/repo/audit/AuditMethodInterceptor.java b/source/java/org/alfresco/repo/audit/AuditMethodInterceptor.java index e9cc98d6a7..3d9213c92a 100644 --- a/source/java/org/alfresco/repo/audit/AuditMethodInterceptor.java +++ b/source/java/org/alfresco/repo/audit/AuditMethodInterceptor.java @@ -187,7 +187,7 @@ public class AuditMethodInterceptor implements MethodInterceptor try { // If we are already in a nested audit call, there is nothing to do - if (wasInAudit != null) + if (Boolean.TRUE.equals(wasInAudit)) { return mi.proceed(); } @@ -211,8 +211,6 @@ public class AuditMethodInterceptor implements MethodInterceptor } String methodName = mi.getMethod().getName(); - // Record that we have entered an audit method - inAudit.set(Boolean.TRUE); return proceedWithAudit(mi, auditableDef, serviceName, methodName, namedArguments); } finally @@ -228,7 +226,10 @@ public class AuditMethodInterceptor implements MethodInterceptor String methodName, Map namedArguments) throws Throwable { + Boolean wasInAudit = inAudit.get(); Map preAuditedData = null; + // Record that we have entered an audit method + inAudit.set(Boolean.TRUE); try { preAuditedData = auditInvocationBefore(serviceName, methodName, namedArguments); @@ -241,7 +242,10 @@ public class AuditMethodInterceptor implements MethodInterceptor " Invocation: " + mi, e); } - + finally + { + inAudit.set(wasInAudit); + } // Execute the call Object ret = null; Throwable thrown = null; @@ -256,6 +260,8 @@ public class AuditMethodInterceptor implements MethodInterceptor // We don't ALWAYS want to record the return value Object auditRet = auditableDef.recordReturnedObject() ? ret : null; + // Record that we have entered an audit method + inAudit.set(Boolean.TRUE); try { auditInvocationAfter(serviceName, methodName, namedArguments, auditRet, thrown, preAuditedData); @@ -268,7 +274,10 @@ public class AuditMethodInterceptor implements MethodInterceptor " Invocation: " + mi, e); } - + finally + { + inAudit.set(wasInAudit); + } // Done if (thrown != null) { diff --git a/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java b/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java index d84a1165a4..ba2f5e3244 100644 --- a/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java +++ b/source/test-java/org/alfresco/repo/audit/AuditComponentTest.java @@ -31,6 +31,7 @@ import java.util.Map; import junit.framework.TestCase; import org.alfresco.error.AlfrescoRuntimeException; +import org.alfresco.model.ContentModel; import org.alfresco.repo.audit.model.AuditApplication; import org.alfresco.repo.audit.model.AuditModelException; import org.alfresco.repo.audit.model.AuditModelRegistryImpl; @@ -48,6 +49,8 @@ import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.cmr.repository.NodeService; import org.alfresco.service.cmr.repository.StoreRef; import org.alfresco.service.cmr.security.MutableAuthenticationService; +import org.alfresco.service.namespace.NamespaceService; +import org.alfresco.service.namespace.QName; import org.alfresco.service.transaction.TransactionService; import org.alfresco.test_category.OwnJVMTestsCategory; import org.alfresco.util.ApplicationContextHelper; @@ -75,10 +78,11 @@ public class AuditComponentTest extends TestCase private static final String APPLICATION_ACTIONS_TEST = "Actions Test"; private static final String APPLICATION_API_TEST = "Test AuthenticationService"; private static final String APPLICATION_ALF12638_TEST = "Test ALF-12638"; + private static final String APPLICATION_MNT10767_TEST = "Test MNT-10767"; private static final Log logger = LogFactory.getLog(AuditComponentTest.class); - private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(); + private static ApplicationContext ctx = ApplicationContextHelper.getApplicationContext(new String[] {ApplicationContextHelper.CONFIG_LOCATIONS[0], "classpath:alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml"}); private AuditModelRegistryImpl auditModelRegistry; private AuditComponent auditComponent; @@ -787,6 +791,94 @@ public class AuditComponentTest extends TestCase assertTrue("There should be exactly one audit entry for the API test", success); } + /** + * See MNT-10767 + */ + public void testAuditSubordinateCall() throws Exception + { + AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getSystemUserName()); + + AuditQueryParameters params = new AuditQueryParameters(); + params.setForward(true); + params.setApplicationName(APPLICATION_MNT10767_TEST); + + // Load in the config for this specific test: alfresco-audit-test-mnt-10767 + URL testModelUrl = ResourceUtils.getURL("classpath:alfresco/testaudit/alfresco-audit-test-mnt-10767.xml"); + auditModelRegistry.registerModel(testModelUrl); + auditModelRegistry.loadAuditModels(); + + // There should be a log entry for the application + final List results = new ArrayList(5); + final StringBuilder sb = new StringBuilder(); + AuditQueryCallback auditQueryCallback = new AuditQueryCallback() + { + public boolean valuesRequired() + { + return true; + } + + public boolean handleAuditEntry(Long entryId, String applicationName, String user, long time, Map values) + { + results.add(entryId); + sb.append("Row: ").append(entryId).append(" | ").append(applicationName).append(" | ").append(user).append(" | ").append(new Date(time)).append(" | ").append( + values).append(" | ").append("\n"); + ; + return true; + } + + public boolean handleAuditEntryError(Long entryId, String errorMsg, Throwable error) + { + throw new AlfrescoRuntimeException(errorMsg, error); + } + }; + + clearAuditLog(APPLICATION_MNT10767_TEST); + results.clear(); + sb.delete(0, sb.length()); + queryAuditLog(auditQueryCallback, params, -1); + assertTrue("There should be no audit entries for the API test after a clear", results.isEmpty()); + + TestCreateFileFolderService testCreateFileFolderService = (TestCreateFileFolderService) ctx.getBean("TestCreateFileFolderService"); + NodeRef workingRootNodeRef = null; + try + { + workingRootNodeRef = nodeService.createNode(nodeRef, ContentModel.ASSOC_CHILDREN, + QName.createQName(NamespaceService.ALFRESCO_URI, "working_root" + System.currentTimeMillis()), ContentModel.TYPE_FOLDER).getChildRef(); + testCreateFileFolderService.create(workingRootNodeRef, "TestFolder-" + System.currentTimeMillis(), ContentModel.TYPE_FOLDER); + + // Try this for a while until we get a result + boolean success = false; + for (int i = 0; i < 30; i++) + { + queryAuditLog(auditQueryCallback, params, -1); + if (results.size() > 1) + { + logger.debug(sb.toString()); + success = true; + break; + } + synchronized (this) + { + try + { + this.wait(1000L); + } + catch (InterruptedException e) + { + } + } + } + assertTrue("There should be audit entry for the API test", success); + } + finally + { + if (workingRootNodeRef != null) + { + nodeService.deleteNode(workingRootNodeRef); + } + } + } + public void testAuditOverlimitProperties() throws Exception { final int OVERLIMIT_SIZE = 1500; diff --git a/source/test-java/org/alfresco/repo/audit/TestCreateFileFolderService.java b/source/test-java/org/alfresco/repo/audit/TestCreateFileFolderService.java new file mode 100644 index 0000000000..95eebf98c9 --- /dev/null +++ b/source/test-java/org/alfresco/repo/audit/TestCreateFileFolderService.java @@ -0,0 +1 @@ +package org.alfresco.repo.audit; import org.alfresco.service.Auditable; import org.alfresco.service.cmr.model.FileInfo; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; public interface TestCreateFileFolderService { @Auditable(parameters = {"parentNodeRef", "name", "typeQName"}) public FileInfo create(NodeRef parentNodeRef, String name, QName typeQName); } \ No newline at end of file diff --git a/source/test-java/org/alfresco/repo/audit/TestCreateFileFolderServiceImpl.java b/source/test-java/org/alfresco/repo/audit/TestCreateFileFolderServiceImpl.java new file mode 100644 index 0000000000..d069d14178 --- /dev/null +++ b/source/test-java/org/alfresco/repo/audit/TestCreateFileFolderServiceImpl.java @@ -0,0 +1 @@ +package org.alfresco.repo.audit; import org.alfresco.service.cmr.model.FileFolderService; import org.alfresco.service.cmr.model.FileInfo; import org.alfresco.service.cmr.repository.NodeRef; import org.alfresco.service.namespace.QName; public class TestCreateFileFolderServiceImpl implements TestCreateFileFolderService { // Public FileFolderService with AuditMethodInterceptor private FileFolderService fileFolderService; public void setFileFolderService(FileFolderService fileFolderService) { this.fileFolderService = fileFolderService; } @Override public FileInfo create(NodeRef parentNodeRef, String name, QName typeQName) { return fileFolderService.create(parentNodeRef, name, typeQName); } } \ No newline at end of file diff --git a/source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml b/source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml new file mode 100644 index 0000000000..cf03bf57d6 --- /dev/null +++ b/source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767-context.xml @@ -0,0 +1,24 @@ + + + + + + + + + + + + org.alfresco.repo.audit.TestCreateFileFolderService + + + + + + + AuditMethodInterceptor + + + + + \ No newline at end of file diff --git a/source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767.xml b/source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767.xml new file mode 100644 index 0000000000..bd03cded28 --- /dev/null +++ b/source/test-resources/alfresco/testaudit/alfresco-audit-test-mnt-10767.xml @@ -0,0 +1,26 @@ + + + + + + + + + + + + + + + + + + + + + + + + \ No newline at end of file