From 9a9560fa6cc0056410dadf73c16e183ef544ecfc Mon Sep 17 00:00:00 2001
From: Roy Wetherall <roy.wetherall@alfresco.com>
Date: Tue, 21 Jul 2015 05:30:53 +0000
Subject: [PATCH] Fixes to classification enforcement:

 * only process services that start with an upper case character.  We were being over zealous in our checking which was causing problems, we only care about public services.
 * add enable/disable/isenabled methods to pre-processor
 * switch off pre-processing when post-processing
 * remove method black list
 * unit tests, integration tests and UI tests run locally

+review RM @taksoy



git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@108613 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261
---
 .../classified-content-context.xml            |  3 +-
 ...icationMethodInterceptorPostProcessor.java |  3 +-
 .../BasePostMethodInvocationProcessor.java    | 18 +++-
 .../PreMethodInvocationProcessor.java         | 45 ++++++---
 ...owseClassificationEnforcementTestBase.java |  2 +-
 .../EnforceClassificationTest.java            | 99 -------------------
 ...PostMethodInvocationProcessorUnitTest.java |  1 +
 7 files changed, 51 insertions(+), 120 deletions(-)
 delete mode 100644 rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/EnforceClassificationTest.java

diff --git a/rm-server/config/alfresco/module/org_alfresco_module_rm/classified-content-context.xml b/rm-server/config/alfresco/module/org_alfresco_module_rm/classified-content-context.xml
index 0d9fea42db..c1ec943eb9 100644
--- a/rm-server/config/alfresco/module/org_alfresco_module_rm/classified-content-context.xml
+++ b/rm-server/config/alfresco/module/org_alfresco_module_rm/classified-content-context.xml
@@ -43,8 +43,7 @@
    <!-- Classification Method Interceptor -->
 
    <bean id="preMethodInvocationProcessor"
-      class="org.alfresco.module.org_alfresco_module_rm.classification.interceptor.processor.PreMethodInvocationProcessor"
-      init-method="init">
+      class="org.alfresco.module.org_alfresco_module_rm.classification.interceptor.processor.PreMethodInvocationProcessor">
    </bean>
 
    <bean id="postMethodInvocationProcessor"
diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/ClassificationMethodInterceptorPostProcessor.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/ClassificationMethodInterceptorPostProcessor.java
index 5bb5be1062..dc81660d32 100644
--- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/ClassificationMethodInterceptorPostProcessor.java
+++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/ClassificationMethodInterceptorPostProcessor.java
@@ -60,7 +60,8 @@ public class ClassificationMethodInterceptorPostProcessor implements BeanFactory
             // only modify proxy factory beans that follow the public service naming postfix convention
             if (beanDefinition.getBeanClassName() != null &&
                 beanDefinition.getBeanClassName().endsWith(TYPE_PROXY_FACTORY_BEAN) &&
-                bean.endsWith(POSTFIX_SERVICE))
+                bean.endsWith(POSTFIX_SERVICE) &&
+                Character.isUpperCase(bean.charAt(0)))
             {
                 // get the property values for the bean definition
                 MutablePropertyValues propertyValues = beanDefinition.getPropertyValues();
diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/BasePostMethodInvocationProcessor.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/BasePostMethodInvocationProcessor.java
index 8870b632a8..058f09ad72 100644
--- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/BasePostMethodInvocationProcessor.java
+++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/BasePostMethodInvocationProcessor.java
@@ -56,6 +56,10 @@ public abstract class BasePostMethodInvocationProcessor
     /** Post method invocation processor */
     @Autowired
     private PostMethodInvocationProcessor postMethodInvocationProcessor;
+    
+    /** Pre method invocation processor */
+    @Autowired
+    private PreMethodInvocationProcessor preMethodInvocationProcessor;
 
     /**
      * @return the nodeService
@@ -133,11 +137,21 @@ public abstract class BasePostMethodInvocationProcessor
     {
         NodeRef filter = nodeRef;
 
-        if (getNodeService().exists(nodeRef) &&
+        // disable pre-method invocation processor
+        preMethodInvocationProcessor.disable();        
+        try
+        {
+            if (getNodeService().exists(nodeRef) &&
                 getDictionaryService().isSubClass(getNodeService().getType(nodeRef), TYPE_CONTENT) &&
                 !getContentClassificationService().hasClearance(nodeRef))
+            {
+                filter = null;
+            }
+        }
+        finally
         {
-            filter = null;
+            // re-enable pre-method invocation processor
+            preMethodInvocationProcessor.enable();
         }
 
         return filter;
diff --git a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/PreMethodInvocationProcessor.java b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/PreMethodInvocationProcessor.java
index b5d2f2b2e9..43f750f5fc 100644
--- a/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/PreMethodInvocationProcessor.java
+++ b/rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/PreMethodInvocationProcessor.java
@@ -137,17 +137,33 @@ public class PreMethodInvocationProcessor implements ApplicationContextAware
     {
         return this.methodNames;
     }
-
+    
     /**
-     * Init method to populate the list of method names which will be checked before invocation
+     * is pre-processing enabled?
+     * 
+     * @return boolean  true if enabled, false otherwise
      */
-    public void init()
+    public boolean isEnabled()
     {
-        getMethodNames().add("NodeService.exists");
-        getMethodNames().add("NodeService.getType");
-        getMethodNames().add("NodeService.hasAspect");
-        getMethodNames().add("NodeService.getAspects");
-        getMethodNames().add("NodeService.getProperties");
+        return (getAlfrescoTransactionSupport().getResource(KEY_PROCESSING) == null);
+    }
+    
+    /**
+     * disable pre-processing for this transaction
+     */
+    public void disable()
+    {
+        // mark the transaction as processing a classification check
+        getAlfrescoTransactionSupport().bindResource(KEY_PROCESSING, TRUE);        
+    }
+    
+    /**
+     * enable pre-processing for this transaction
+     */
+    public void enable()
+    {
+        // clear the transaction as processed a classification check
+        getAlfrescoTransactionSupport().unbindResource(KEY_PROCESSING);
     }
 
     /**
@@ -169,9 +185,8 @@ public class PreMethodInvocationProcessor implements ApplicationContextAware
                 // ensure classification service has been bootstrapped
                 if (getClassificationServiceBootstrap().isInitialised())
                 {
-                    // check that we are not already processing a classification check
-                    Object value = getAlfrescoTransactionSupport().getResource(KEY_PROCESSING);
-                    if (value == null)
+                    // if pre-processing is enabled
+                    if (isEnabled())
                     {
                         Method method = invocation.getMethod();
                         Class[] params = method.getParameterTypes();
@@ -188,8 +203,8 @@ public class PreMethodInvocationProcessor implements ApplicationContextAware
 
                                 if (!getMethodNames().contains(name))
                                 {
-                                    // mark the transaction as processing a classification check
-                                    getAlfrescoTransactionSupport().bindResource(KEY_PROCESSING, TRUE);
+                                    // disable pre-processing
+                                    disable();
                                     try
                                     {
                                         // get the value of the parameter
@@ -200,8 +215,8 @@ public class PreMethodInvocationProcessor implements ApplicationContextAware
                                     }
                                     finally
                                     {
-                                        // clear the transaction as processed a classification check
-                                        getAlfrescoTransactionSupport().unbindResource(KEY_PROCESSING);
+                                        // re-enable pre-processing
+                                        enable();
                                     }
                                 }
                             }
diff --git a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/BrowseClassificationEnforcementTestBase.java b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/BrowseClassificationEnforcementTestBase.java
index ebfc0e51ed..47b29d7b64 100644
--- a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/BrowseClassificationEnforcementTestBase.java
+++ b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/BrowseClassificationEnforcementTestBase.java
@@ -32,7 +32,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
  * @author Tuna Aksoy
  * @since 3.0
  */
-public class BrowseClassificationEnforcementTestBase extends BaseRMTestCase
+public abstract class BrowseClassificationEnforcementTestBase extends BaseRMTestCase
 {
     protected String testUser;
     protected static final String LEVEL1 = "level1";
diff --git a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/EnforceClassificationTest.java b/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/EnforceClassificationTest.java
deleted file mode 100644
index 870e3086cb..0000000000
--- a/rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/classification/interceptor/EnforceClassificationTest.java
+++ /dev/null
@@ -1,99 +0,0 @@
-/*
- * Copyright (C) 2005-2015 Alfresco Software Limited.
- *
- * This file is part of Alfresco
- *
- * Alfresco is free software: you can redistribute it and/or modify
- * it under the terms of the GNU Lesser General Public License as published by
- * the Free Software Foundation, either version 3 of the License, or
- * (at your option) any later version.
- *
- * Alfresco is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
- * GNU Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public License
- * along with Alfresco. If not, see <http://www.gnu.org/licenses/>.
- */
-package org.alfresco.module.org_alfresco_module_rm.test.integration.classification.interceptor;
-
-import org.alfresco.module.org_alfresco_module_rm.classification.ContentClassificationService;
-import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
-
-/**
- * Enforce classification integration test
- *
- * @author Roy Wetherall
- * @since 3.0
- */
-public class EnforceClassificationTest extends BaseRMTestCase
-{
-	/** test data */
-//	private static final String CLASSIFICATION_LEVEL1 = "level1";
-//	private static final String CLASSIFICATION_LEVEL2 = "level2";
-//	private static final String CLASSIFICATION_LEVEL3 = "level3";
-//	private static final String CLASSIFICATION_LEVEL4 = "level4";
-//
-//	private static final String CLASSIFICATION_REASON = "Test Reason 1";
-//	private static final String CLASSIFICATION_AUTHORITY = "classification.authority";
-//	private static final String RECORD_NAME = "recordname.txt";
-//
-//	private ContentClassificationService contentClassificationService;
-
-	@Override
-	protected void initServices()
-	{
-	    super.initServices();
-	    contentClassificationService = (ContentClassificationService)applicationContext.getBean("contentClassificationService");
-	}
-
-	@Override
-	protected boolean isCollaborationSiteTest()
-	{
-	    return true;
-	}
-
-	/**
-	 *
-	 */
-    public void testUserNotClearedDocument() throws Exception
-    {
-//        doBehaviourDrivenTest(new BehaviourDrivenTest(AccessDeniedException.class)
-//        {
-//        	private String userName;
-//
-//            public void given() throws Exception
-//            {
-//                // create test person and assign read permission to document
-//                userName = GUID.generate();
-//                createPerson(userName, true);
-//                permissionService.setPermission(dmDocument, userName , PermissionService.READ, true);
-//
-//                // assign security clearance
-//                securityClearanceService.setUserSecurityClearance(userName, CLASSIFICATION_LEVEL3);
-//
-//                // classify document
-//                contentClassificationService.classifyContent(
-//                            CLASSIFICATION_LEVEL1,
-//                            CLASSIFICATION_AUTHORITY,
-//                            Collections.singleton(CLASSIFICATION_REASON),
-//                            dmDocument);
-//
-//            }
-//
-//            public void when() throws Exception
-//            {
-//                AuthenticationUtil.runAs(new RunAsWork<Void>()
-//                {
-//                    public Void doWork() throws Exception
-//                    {
-//                        nodeService.getAspects(dmDocument);
-//
-//                        return null;
-//                    }
-//                }, userName);
-//            }
-//        });
-    }
-}
diff --git a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/NodeRefPostMethodInvocationProcessorUnitTest.java b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/NodeRefPostMethodInvocationProcessorUnitTest.java
index c8ddc99bd7..733aae3fe9 100644
--- a/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/NodeRefPostMethodInvocationProcessorUnitTest.java
+++ b/rm-server/unit-test/java/org/alfresco/module/org_alfresco_module_rm/classification/interceptor/processor/NodeRefPostMethodInvocationProcessorUnitTest.java
@@ -39,6 +39,7 @@ public class NodeRefPostMethodInvocationProcessorUnitTest extends BaseUnitTest
 {
     @InjectMocks private NodeRefPostMethodInvocationProcessor nodeRefPostMethodInvocationProcessor;
     @Mock private ContentClassificationService mockedContentClassificationService;
+    @Mock private PreMethodInvocationProcessor mockedPreMethodInvocationProcessor;
 
     @Test
     public void testProcessingNonExistingNode()