RM-1231: Could not Revert the rejected record

* record security was not being correctly reset during record reject
 * added additional logging to help debug access denied exceptions



git-svn-id: https://svn.alfresco.com/repos/alfresco-enterprise/modules/recordsmanagement/HEAD@73455 c4b6b30b-aa2e-2d43-bbcb-ca4b014f7261

rm-server/config/alfresco/module/org_alfresco_module_rm/extended-repository-context.xml

@@ -3,29 +3,12 @@
 
 <beans>
 
-   <!-- AVM Remote Store to ADM Remote Store migration patch -->
+   <!-- extend node service security to report capability details on failure -->
-  <!-- <bean id="patch.avmToAdmRemoteStore" class="org.alfresco.repo.admin.patch.impl.AVMToADMRemoteStorePatch" parent="basePatch">
+   <bean id="rm.NodeService_security" abstract="true" class="org.alfresco.module.org_alfresco_module_rm.security.RMMethodSecurityInterceptor"/>
-        <property name="id"><value>patch.avmToAdmRemoteStore</value></property>
+   <bean class="org.alfresco.module.org_alfresco_module_rm.util.BeanExtender">
-        <property name="description"><value>patch.avmToAdmRemoteStore.description</value></property>
+      <property name="beanName" value="NodeService_security" />
-        <property name="fixesFromSchema"><value>0</value></property>
+      <property name="extendingBeanName" value="rm.NodeService_security" />
-        <property name="fixesToSchema"><value>5011</value></property>
+   </bean>
-        <property name="targetSchema"><value>5012</value></property>
-        <property name="requiresTransaction"><value>false</value></property>
-        <property name="fileFolderService" ref="fileFolderService" />
-        <property name="contentService" ref="contentService" />
-        <property name="siteService" ref="SiteService" />
-        <property name="ruleService" ref="ruleService" />
-        <property name="avmService" ref="AVMService" />
-        <property name="hiddenAspect" ref="hiddenAspect" />
-        <property name="avmStore"><value>sitestore</value></property>
-        <property name="avmRootPath"><value>/alfresco/site-data</value></property>
-        <property name="dependsOn" >
-            <list>
-                <ref bean="patch.migrateTenantsFromAttrsToTable" />
-                <ref bean="patch.migrateAttrTenants" />
-            </list>
-        </property>
-    </bean> -->
 
    <bean id="ExtendedPermissionService" class="org.springframework.aop.framework.ProxyFactoryBean">
         <property name="proxyInterfaces">

rm-server/config/alfresco/module/org_alfresco_module_rm/model/recordsModel.xml

@@ -1102,7 +1102,7 @@
       			<protected>true</protected>
             </property>
             <property name="rma:recordOriginatingLocation">
-               	<type>d:any</type>
+               	<type>d:noderef</type>
       			<protected>true</protected>
             </property>
          </properties>

rm-server/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml

@@ -1089,17 +1089,23 @@
             <value>
                 <![CDATA[
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.registerRecordMetadataAspect=RM_ALLOW                
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.disablePropertyEditableCheck=RM_ALLOW
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.enablePropertyEditableCheck=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.getRecordMetaDataAspects=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.getRecordMetadataAspects=RM.Read.0
-                org.alfresco.module.org_alfresco_module_rm.record.RecordService.isRecord=RM_ALLOW
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.isRecord=ACL_ALLOW,RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.isDeclared=RM.Read.0
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.isFiled=RM.Read.0
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.createRecord=RM_ALLOW
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.file=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.hideRecord=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.isPropertyEditable=RM.Read.0
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.isMetadataStub=RM.Read.0
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.rejectRecord=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.getRecords=RM.Read.0,AFTER_RM.FilterNode
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.addRecordType=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.makeRecord=RM_ALLOW
+                org.alfresco.module.org_alfresco_module_rm.record.RecordService.link=RM_ALLOW
                 org.alfresco.module.org_alfresco_module_rm.record.RecordService.*=RM_DENY
                 ]]>
             </value>
@@ -1421,56 +1427,6 @@
         <property name="path" value="alfresco/module/org_alfresco_module_rm/dod5015/DODExampleFilePlan.xml" />
     </bean>
 
-    <!--
-    <bean id="authorityDAO" class="org.alfresco.repo.security.authority.RMAuthorityDAOImpl" init-method="init">
-        <property name="storeUrl">
-            <value>${spaces.store}</value>
-        </property>
-        <property name="nodeService">
-            <ref bean="mlAwareNodeService" />
-        </property>
-        <property name="searchService">
-            <ref bean="admSearchService" />
-        </property>
-        <property name="namespacePrefixResolver">
-            <ref bean="namespaceService" />
-        </property>
-        <property name="dictionaryService">
-            <ref bean="dictionaryService" />
-        </property>
-        <property name="personService">
-            <ref bean="personService" />
-        </property>
-        <property name="tenantService">
-            <ref bean="tenantService" />
-        </property>
-        <property name="authorityLookupCache">
-            <ref bean="authorityLookupCache" />
-        </property>
-        <property name="userAuthorityCache">
-            <ref bean="userToAuthorityCache" />
-        </property>
-        <property name="childAuthorityCache">
-            <ref bean="authorityToChildAuthorityCache" />
-        </property>
-        <property name="zoneAuthorityCache">
-            <ref bean="zoneToAuthorityCache" />
-        </property>
-        <property name="singletonCache">
-            <ref bean="immutableSingletonCache" />
-        </property>
-        <property name="policyComponent">
-            <ref bean="policyComponent"/>
-        </property>
-        <property name="cannedQueryRegistry" ref="cannedQueryRegistry" />
-        <property name="cannedQueryDAO" ref="cannedQueryDAO" />
-        <property name="aclDAO" ref="aclDAO" />
-        <property name="authorityBridgeDAO" ref="authorityBridgeDAO" />
-        <property name="authorityBridgeTableCache" ref="authorityBridgeTableCache" />
-        <property name="useBridgeTable" value="${authority.useBridgeTable}" />
-    </bean>
-    -->
-
    <!--  Hold Service -->
 
    <bean id="holdService"

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMEntryVoter.java

@@ -31,6 +31,8 @@ import net.sf.acegisecurity.vote.AccessDecisionVoter;
 
 import org.alfresco.module.org_alfresco_module_rm.capability.policy.ConfigAttributeDefinition;
 import org.alfresco.module.org_alfresco_module_rm.capability.policy.Policy;
+import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.module.org_alfresco_module_rm.security.RMMethodSecurityInterceptor;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport;
 import org.alfresco.repo.transaction.TransactionalResourceHelper;
@@ -124,6 +126,9 @@ public class RMEntryVoter extends RMSecurityCommon
     @SuppressWarnings("rawtypes")
 	public int vote(Authentication authentication, Object object, net.sf.acegisecurity.ConfigAttributeDefinition config)
     {
+        // logging
+        RMMethodSecurityInterceptor.isRMSecurityChecked(true);
+        
     	MethodInvocation mi = (MethodInvocation)object;
 
     	if (TransactionalResourceHelper.isResourcePresent("voting"))
@@ -175,6 +180,9 @@ public class RMEntryVoter extends RMSecurityCommon
 	            // Whatever is found first takes precedence
 	            if (cad.getTypeString().equals(ConfigAttributeDefinition.RM_DENY))
 	            {
+	                // log message
+	                RMMethodSecurityInterceptor.addMessage("RM_DENY: check that a security policy has been set for this method");
+	                
 	                return AccessDecisionVoter.ACCESS_DENIED;
 	            }
 	            else if (cad.getTypeString().equals(ConfigAttributeDefinition.RM_ABSTAIN))
@@ -235,6 +243,9 @@ public class RMEntryVoter extends RMSecurityCommon
 	                {
 		                case  AccessDecisionVoter.ACCESS_DENIED:
 		                {
+		                    // log message
+		                    RMMethodSecurityInterceptor.addMessage("Policy " + cad.getPolicyName() + " denied.");
+		                    
 		                    return AccessDecisionVoter.ACCESS_DENIED;
 		                }
 		                case AccessDecisionVoter.ACCESS_ABSTAIN:

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/RMSecurityCommon.java

@@ -23,6 +23,7 @@ import net.sf.acegisecurity.vote.AccessDecisionVoter;
 import org.alfresco.module.org_alfresco_module_rm.caveat.RMCaveatConfigComponent;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.module.org_alfresco_module_rm.security.RMMethodSecurityInterceptor;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.transaction.AlfrescoTransactionSupport;
 import org.alfresco.service.cmr.repository.AssociationRef;
@@ -219,15 +220,22 @@ public class RMSecurityCommon implements ApplicationContextAware
 
         if (permissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS) == AccessStatus.DENIED)
         {
+            // log message
+            RMMethodSecurityInterceptor.addMessage("User does not have read record permission on node, access denied. (nodeRef={0}, user={1})", nodeRef, AuthenticationUtil.getRunAsUser());
+            
             if (logger.isDebugEnabled())
             {
                 logger.debug("\t\tUser does not have read record permission on node, access denied.  (nodeRef=" + nodeRef.toString() + ", user=" + AuthenticationUtil.getRunAsUser() + ")");
             }
+            
             return setTransactionCache("checkRmRead", nodeRef, AccessDecisionVoter.ACCESS_DENIED);
         }
 
         if (permissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS) == AccessStatus.DENIED)
         {
+            // log capability details
+            RMMethodSecurityInterceptor.reportCapabilityStatus(RMPermissionModel.VIEW_RECORDS, AccessDecisionVoter.ACCESS_DENIED);
+            
             if (logger.isDebugEnabled())
             {
                 logger.debug("\t\tUser does not have view records capability permission on node, access denied. (filePlan=" + filePlan.toString() + ", user=" + AuthenticationUtil.getRunAsUser() + ")");

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/capability/policy/ReadPolicy.java

@@ -23,7 +23,9 @@ import org.alfresco.service.cmr.repository.NodeRef;
 import org.aopalliance.intercept.MethodInvocation;
 
 /**
- * @author Roy Wethearll
+ * Read method security policy.
+ * 
+ * @author Roy Wetherall
  * @since 2.1
  */
 public class ReadPolicy extends AbstractBasePolicy

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java

@@ -728,13 +728,11 @@ public class RecordServiceImpl extends BaseBehaviourBean
         if (extendedPermissionService.hasPermission(nodeRef, PermissionService.WRITE) != AccessStatus.ALLOWED)
         {
             throw new AccessDeniedException("Can not create record from document, because the user " +
-                                            AuthenticationUtil.getFullyAuthenticatedUser() +
+                                            AuthenticationUtil.getRunAsUser() +
                                             " does not have Write permissions on the doucment " +
                                             nodeRef.toString());
         }
 
-        // Save the id of the currently logged in user
-        final String userId = AuthenticationUtil.getRunAsUser();
 
         // do the work of creating the record as the system user
         AuthenticationUtil.runAsSystem(new RunAsWork<Void>()
@@ -775,7 +773,7 @@ public class RecordServiceImpl extends BaseBehaviourBean
                         // save the information about the originating details
                         Map<QName, Serializable> aspectProperties = new HashMap<QName, Serializable>(3);
                         aspectProperties.put(PROP_RECORD_ORIGINATING_LOCATION, parentAssoc.getParentRef());
-                        aspectProperties.put(PROP_RECORD_ORIGINATING_USER_ID, userId);
+                        aspectProperties.put(PROP_RECORD_ORIGINATING_USER_ID, owner);
                         aspectProperties.put(PROP_RECORD_ORIGINATING_CREATION_DATE, new Date());
                         nodeService.addAspect(nodeRef, ASPECT_RECORD_ORIGINATING_DETAILS, aspectProperties);
                         
@@ -1111,6 +1109,12 @@ public class RecordServiceImpl extends BaseBehaviourBean
                     }
                     ownableService.setOwner(nodeRef, documentOwner);
                     
+                    // clear the existing permissions
+                    permissionService.clearPermission(nodeRef, null);
+                    
+                    // restore permission inheritance
+                    permissionService.setInheritParentPermissions(nodeRef, true);
+
                     // send an email to the record creator
                     notificationHelper.recordRejectedEmailNotification(nodeRef, recordId, documentOwner);
                 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/security/RMMethodSecurityInterceptor.java

@@ -1,15 +1,18 @@
 
 package org.alfresco.module.org_alfresco_module_rm.security;
 
+import java.text.MessageFormat;
+import java.util.ArrayList;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.List;
 import java.util.Map;
 
 import net.sf.acegisecurity.AccessDeniedException;
 import net.sf.acegisecurity.intercept.InterceptorStatusToken;
-import net.sf.acegisecurity.intercept.method.aopalliance.MethodSecurityInterceptor;
 import net.sf.acegisecurity.vote.AccessDecisionVoter;
 
+import org.alfresco.repo.security.permissions.impl.acegi.MethodSecurityInterceptor;
 import org.alfresco.service.cmr.security.AccessStatus;
 import org.aopalliance.intercept.MethodInvocation;
 import org.apache.commons.logging.Log;
@@ -63,7 +66,7 @@ public class RMMethodSecurityInterceptor extends MethodSecurityInterceptor
     /**
      * Current capability report details.
      * <p>
-     * Used to getnerate the capability error report.
+     * Used to generate the capability error report.
      */
     private static final ThreadLocal<Map<String, CapabilityReport>> CAPABILITIES = new ThreadLocal<Map<String, CapabilityReport>>()
     {
@@ -74,6 +77,22 @@ public class RMMethodSecurityInterceptor extends MethodSecurityInterceptor
         };
     };
 
+    /**
+     * Indicates whether this is an RM security check or not
+     */
+    private static final ThreadLocal<Boolean> IS_RM_SECURITY_CHECK = new ThreadLocal<Boolean>()
+    {
+        protected Boolean initialValue() {return false;};
+    };
+    
+    /**
+     * Messages to display in error report.
+     */
+    private static final ThreadLocal<List<String>> MESSAGES = new ThreadLocal<List<String>>()
+    {
+        protected List<String> initialValue() {return new ArrayList<String>();};
+    };
+    
     /**
      * Get capability report object from the thread local, creating one for
      * the given capability name if one does not already exist.
@@ -95,6 +114,41 @@ public class RMMethodSecurityInterceptor extends MethodSecurityInterceptor
         return capability;
     }
 
+    /**
+     * Indicates whether this is a RM security check or not
+     * 
+     * @param newValue  true if RM security check, false otherwise
+     */
+    public static void isRMSecurityChecked(boolean newValue)
+    {
+        if (logger.isDebugEnabled())
+        {
+            RMMethodSecurityInterceptor.IS_RM_SECURITY_CHECK.set(newValue);
+        }
+    }
+    
+    /**
+     * Add a message to be displayed in the error report.
+     * 
+     * @param message   error message
+     */
+    public static void addMessage(String message)
+    {
+        if (logger.isDebugEnabled())
+        {
+            List<String> messages = RMMethodSecurityInterceptor.MESSAGES.get();
+            messages.add(message);
+        }
+    }
+    
+    public static void addMessage(String message, Object ... params)
+    {
+        if (logger.isDebugEnabled())
+        {
+            addMessage(MessageFormat.format(message, params));
+        }
+    }
+    
     /**
      * Report capability status.
      *
@@ -186,20 +240,73 @@ public class RMMethodSecurityInterceptor extends MethodSecurityInterceptor
         {
             // clear the capability report information
             RMMethodSecurityInterceptor.CAPABILITIES.remove();
+            RMMethodSecurityInterceptor.IS_RM_SECURITY_CHECK.remove();
+            RMMethodSecurityInterceptor.MESSAGES.remove();
 
+            // before invocation (where method security check takes place)
             result = super.beforeInvocation(object);
         }
         catch (AccessDeniedException exception)
         {
+            if (logger.isDebugEnabled())
+            {
+                MethodInvocation mi = (MethodInvocation)object;
+                
+                StringBuilder methodDetails = new StringBuilder("\n");
+                if (RMMethodSecurityInterceptor.IS_RM_SECURITY_CHECK.get())
+                {
+                    methodDetails.append("RM method security check was performed.\n");
+                }
+                else
+                {
+                    methodDetails.append("Standard DM method security check was performed.\n");                    
+                }
+                
+                boolean first = true;
+                methodDetails.append("Failed on method:  ").append(mi.getMethod().getName()).append("(");
+                for (Object arg : mi.getArguments())
+                {
+                    if (first)
+                    {
+                        first = false;
+                    }
+                    else
+                    {
+                        methodDetails.append(", ");
+                    }
+                    
+                    if (arg != null)
+                    {
+                        methodDetails.append(arg.toString());
+                    }
+                    else
+                    {
+                        methodDetails.append("null");
+                    }
+                }
+                methodDetails.append(")\n");
+                
+                List<String> messages = RMMethodSecurityInterceptor.MESSAGES.get();
+                for (String message : messages)
+                {
+                    methodDetails.append(message).append("\n");
+                }
+                
                 String failureReport = getFailureReport();
                 if (failureReport == null)
                 {
-                throw exception;
+                    // rethrow with additional information
+                    throw new AccessDeniedException(exception.getMessage() + methodDetails, exception);
                 }
                 else
                 {
                     // rethrow with additional information
-                throw new AccessDeniedException(exception.getMessage() + getFailureReport(), exception);
+                    throw new AccessDeniedException(exception.getMessage() + methodDetails + getFailureReport(), exception);
+                }
+            }
+            else
+            {
+                throw exception;
             }
         }
         return result;

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/IntegrationTestSuite.java

@@ -22,6 +22,7 @@ import org.alfresco.module.org_alfresco_module_rm.test.integration.disposition.D
 import org.alfresco.module.org_alfresco_module_rm.test.integration.dod.DoD5015TestSuite;
 import org.alfresco.module.org_alfresco_module_rm.test.integration.event.EventTestSuite;
 import org.alfresco.module.org_alfresco_module_rm.test.integration.issue.IssueTestSuite;
+import org.alfresco.module.org_alfresco_module_rm.test.integration.record.RejectRecordTest;
 import org.alfresco.module.org_alfresco_module_rm.test.integration.report.ReportTestSuite;
 import org.junit.runner.RunWith;
 import org.junit.runners.Suite;
@@ -41,7 +42,8 @@ import org.junit.runners.Suite.SuiteClasses;
     IssueTestSuite.class,
     EventTestSuite.class,
     ReportTestSuite.class,
-    DispositionTestSuite.class
+    DispositionTestSuite.class,
+    RejectRecordTest.class
 })
 public class IntegrationTestSuite
 {

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/record/RecordTestSuite.java

@@ -0,0 +1,38 @@
+/*
+ * Copyright (C) 2005-2014 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.record;
+
+import org.junit.runner.RunWith;
+import org.junit.runners.Suite;
+import org.junit.runners.Suite.SuiteClasses;
+
+/**
+ * Record integration test suite
+ *
+ * @author Roy Wetherall
+ * @since 2.2
+ */
+@RunWith(Suite.class)
+@SuiteClasses(
+{
+    RejectRecordTest.class
+})
+public class RecordTestSuite
+{
+}

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/record/RejectRecordTest.java

@@ -0,0 +1,192 @@
+/*
+ * Copyright (C) 2005-2014 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.record;
+
+import org.alfresco.model.ContentModel;
+import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
+import org.alfresco.service.cmr.repository.ContentWriter;
+import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.version.Version;
+import org.alfresco.service.cmr.version.VersionHistory;
+import org.alfresco.service.cmr.version.VersionService;
+import org.springframework.extensions.webscripts.GUID;
+
+/**
+ * reject record tests.
+ * 
+ * @author Roy Wetherall
+ * @since 2.2
+ */
+public class RejectRecordTest extends BaseRMTestCase
+{    
+    private VersionService versionService;    
+    
+    private static final String REASON = GUID.generate();
+    
+    @Override
+    protected boolean isUserTest()
+    {
+        return true;
+    }
+    
+    @Override
+    protected boolean isCollaborationSiteTest()
+    {
+        return true;
+    }
+    
+    @Override
+    protected void initServices()
+    {
+        super.initServices();
+        
+        versionService = (VersionService)applicationContext.getBean("VersionService");
+    }
+    
+    /**
+     * 
+     */
+    public void testRejectedRecordInCorrectState()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                assertFalse(recordService.isRecord(dmDocument));
+                ownableService.setOwner(dmDocument, userName);    
+                
+                // document is declared as a record by user
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        // declare record
+                        recordService.createRecord(filePlan, dmDocument);                        
+                        return null;
+                    }
+                 }, userName);
+            }
+            
+            public void when()
+            {
+                // sanity checks
+                assertTrue(recordService.isRecord(dmDocument));
+                assertFalse(permissionService.getInheritParentPermissions(dmDocument));
+                
+                // declare record
+                recordService.rejectRecord(dmDocument, REASON);                               
+            }
+            
+            public void then()
+            {
+                // document is no longer a record
+                assertFalse(recordService.isRecord(dmDocument));
+
+                // expected owner has be re-set
+                assertEquals(userName, ownableService.getOwner(dmDocument));
+                assertTrue(permissionService.getInheritParentPermissions(dmDocument));
+                assertFalse(nodeService.hasAspect(dmDocument, ASPECT_FILE_PLAN_COMPONENT));
+            }
+        });       
+    }
+    
+    /**
+     * 
+     */
+    public void testRevertAfterReject()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {;
+            private NodeRef document;
+            
+            public void given()
+            {
+                NodeRef folder = fileFolderService.create(documentLibrary, GUID.generate(), TYPE_FOLDER).getNodeRef();
+                document = fileFolderService.create(folder, GUID.generate(), TYPE_CONTENT).getNodeRef();
+                                
+                assertFalse(recordService.isRecord(document));
+                ownableService.setOwner(document, userName);    
+                versionService.ensureVersioningEnabled(document, null);
+                
+                // document is declared as a record by user
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        // declare record
+                        recordService.createRecord(filePlan, document);                        
+                        return null;
+                    }
+                 }, userName);
+                
+                assertTrue(nodeService.hasAspect(document, ASPECT_FILE_PLAN_COMPONENT));                               
+            }
+            
+            public void when()
+            {    
+                // reject the record
+                recordService.rejectRecord(document, REASON);                
+                assertFalse(nodeService.hasAspect(document, ASPECT_FILE_PLAN_COMPONENT));
+                
+                // upload a new version of the document
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        ContentWriter writer = contentService.getWriter(document, ContentModel.PROP_CONTENT, true);
+                        writer.putContent("This is a change to the content and should force a new version");
+                        versionService.createVersion(document, null);
+                        
+                        return null;
+                    }
+                }, userName);
+                
+                assertFalse(nodeService.hasAspect(document, ASPECT_FILE_PLAN_COMPONENT));
+                
+                VersionHistory history = versionService.getVersionHistory(document);
+                assertEquals(2, history.getAllVersions().size());
+                final Version initial = history.getRootVersion();
+                
+                assertFalse(nodeService.hasAspect(initial.getFrozenStateNodeRef(), ASPECT_FILE_PLAN_COMPONENT));
+                
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {                        
+                        // revert the document to a previous version
+                        versionService.revert(document, initial);
+                        
+                        return null;
+                    }
+                 }, userName);
+            }
+            
+            public void then()
+            {
+                // document is no longer a record
+                assertFalse(recordService.isRecord(document));
+
+                // expected owner has be re-set
+                assertEquals(userName, ownableService.getOwner(document));
+            }
+        });       
+    }  
+}

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/BaseRMTestCase.java

@@ -66,6 +66,7 @@ import org.alfresco.service.cmr.repository.StoreRef;
 import org.alfresco.service.cmr.search.SearchService;
 import org.alfresco.service.cmr.security.AuthorityService;
 import org.alfresco.service.cmr.security.MutableAuthenticationService;
+import org.alfresco.service.cmr.security.OwnableService;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.cmr.security.PersonService;
 import org.alfresco.service.cmr.site.SiteInfo;
@@ -126,6 +127,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
     protected PermissionService permissionService;
     protected TaggingService taggingService;
     protected ActionService actionService;
+    protected OwnableService ownableService;
 
     /** RM Services */
     protected DispositionService dispositionService;
@@ -376,6 +378,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
         permissionService = (PermissionService)applicationContext.getBean("PermissionService");
         taggingService = (TaggingService)applicationContext.getBean("TaggingService");
         actionService = (ActionService)applicationContext.getBean("ActionService");
+        ownableService = (OwnableService)applicationContext.getBean("OwnableService");
 
         // Get RM services
         dispositionService = (DispositionService)applicationContext.getBean("DispositionService");
@@ -772,13 +775,13 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
     @Override
     protected <A> A doTestInTransaction(Test<A> test)
     {
-        return super.doTestInTransaction(test, rmAdminUserName);
+        return super.doTestInTransaction(test, AuthenticationUtil.getAdminUserName());
     }
 
     @Override
     protected void doTestInTransaction(FailureTest test)
     {
-        super.doTestInTransaction(test, rmAdminUserName);
+        super.doTestInTransaction(test, AuthenticationUtil.getAdminUserName());
     }
 
     /**