RM-3074: Contributor didn't get write in-place access as expected

* owner derived from cm:creator was not added to writer list
* fully authenticated user in base test was being set incorrectly
* transaction executed outside authentication in BDT classes

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/integration/record/InplaceRecordPermissionTest.java

@@ -34,6 +34,7 @@ import java.util.Map;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
+import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.action.impl.CutOffAction;
 import org.alfresco.module.org_alfresco_module_rm.action.impl.DeclareRecordAction;
 import org.alfresco.module.org_alfresco_module_rm.action.impl.DestroyAction;
@@ -45,6 +46,7 @@ import org.alfresco.module.org_alfresco_module_rm.test.util.CommonRMTestUtils;
 import org.alfresco.module.org_alfresco_module_rm.test.util.bdt.BehaviourTest;
 import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.security.AccessStatus;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.util.GUID;
@@ -64,6 +66,12 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
             RMPermissionModel.EDIT_RECORD_METADATA)
         .collect(Collectors.toList());
     
+    /** test data */
+    NodeRef contribDoc;    
+    
+    /** services */
+    private NodeService dbNodeService;
+    
     /** capabilities */
     private Capability viewRecordsCapability;
     private Capability editNonRecordMetadataCapability;
@@ -84,6 +92,9 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
         // initialise behaviour tests
         BehaviourTest.initBehaviourTests(retryingTransactionHelper);
         
+        // get services
+        dbNodeService = (NodeService)applicationContext.getBean("dbNodeService");
+        
         // get capability references
         viewRecordsCapability = capabilityService.getCapability(RMPermissionModel.VIEW_RECORDS);
         editNonRecordMetadataCapability = capabilityService.getCapability(RMPermissionModel.EDIT_NON_RECORD_METADATA);
@@ -121,6 +132,36 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                         .because("The user does not have write permission on the document.");
     }
     
+    /**
+     * Given a document in a collaboration site that is not a record
+     * And a contributor the didn't create the document
+     * When the contributor tries to declare the document as a record
+     * Then the document does not become a record
+     */
+    public void testContributorThatIsntOwnerDeclareInPlaceRecord()
+    {
+        test()
+            .given()
+            
+                // Given a document in a collaboration site that is not a record            
+                .expect(false)
+                    .from(() -> recordService.isRecord(dmDocument))
+                    .because("The document is not a record.")
+            
+                // And a contributor the didn't create the document
+                .as(dmContributor)
+                    .expect(AccessStatus.DENIED.toString())
+                        .from(() -> permissionService.hasPermission(dmDocument, PermissionService.WRITE).toString())
+                        .because("Contributor does not have write access to document.")
+            
+            // When the user tries to declare the record
+            // When the contributor tries to declare the document as a record
+            .when()
+                .as(dmContributor)
+                    .expectException(AccessDeniedException.class)
+                        .from(() -> recordService.createRecord(filePlan, dmDocument))
+                        .because("The contributor does not have write permission on the document.");        
+    }    
 
     /**
      * Given a document in a collaboration site is not a record
@@ -166,6 +207,16 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                                 AccessStatus.ALLOWED,  // edit non record metadata capability
                                 AccessStatus.DENIED))  // edit record metadata capability
                 
+                // And a site contributor has read and view    
+                .as(dmContributor)
+                    .perform(() ->               
+                        checkInPlaceAccess(dmDocument, 
+                                AccessStatus.ALLOWED,  // read record permission 
+                                AccessStatus.DENIED,  // filing permission
+                                AccessStatus.ALLOWED,  // view record capability 
+                                AccessStatus.DENIED,  // edit non record metadata capability
+                                AccessStatus.DENIED))  // edit record metadata capability
+                    
                 // And a site consumer has read permissions and view record capability on the record
                 .as(dmConsumer)
                     .perform(() -> 
@@ -203,6 +254,91 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
         assertEquals(accessStatus[4], access.get(editRecordMetadataCapability));        
     }
     
+    /**
+     * 
+     */
+    public void testCreateInplaceRecordFromCollabSiteWhenContribIsCreatorOfDocument()
+    {
+        test()
+        
+            // Given that a document is created by contributor        
+            .given()                
+                .as(dmContributor)
+                    .perform(() -> 
+                    {   
+                        contribDoc = fileFolderService.create(dmFolder, "contrib.txt" , ContentModel.TYPE_CONTENT).getNodeRef();
+                        dbNodeService.addAspect(contribDoc, ContentModel.ASPECT_AUDITABLE, null);
+                    })
+                    .expect(false)
+                        .from(() -> recordService.isRecord(contribDoc))
+                        .because("It is not a record.")
+                .asAdmin()
+                    .expect(dmContributor)
+                        .from(() -> ownableService.getOwner(contribDoc))
+                        .because("As the creator of the document the contributor is also the owner")
+                .as(dmContributor)
+                    .expect(AccessStatus.ALLOWED.toString())
+                        .from(() -> permissionService.hasPermission(contribDoc, PermissionService.WRITE).toString())
+                        .because("Contrib user has write permissions on created document as the owner.")
+                        
+            // When it is declared as an inplace record            
+            .when()        
+                .as(dmContributor)
+                    .perform(() -> recordService.createRecord(filePlan, contribDoc))
+            
+            .then()        
+                .asAdmin()
+                    // Then it becomes a record
+                    .expect(true)
+                        .from(() -> recordService.isRecord(contribDoc))
+                        .because("The document is a record")
+                    
+                    // And it isn't filed
+                    .expect(false)
+                        .from(() -> recordService.isFiled(contribDoc))
+                        .because("The record is not filed")
+                
+                // And a site collaborator has filling permissions and filling capability on the record     
+                .as(dmCollaborator)
+                    .perform(() ->               
+                        checkInPlaceAccess(contribDoc, 
+                                AccessStatus.ALLOWED,  // read record permission 
+                                AccessStatus.ALLOWED,  // filing permission
+                                AccessStatus.ALLOWED,  // view record capability 
+                                AccessStatus.ALLOWED,  // edit non record metadata capability
+                                AccessStatus.DENIED))  // edit record metadata capability
+                
+                // And a site contributor has filling capability and permissions  
+                .as(dmContributor)
+                    .perform(() ->               
+                        checkInPlaceAccess(contribDoc, 
+                                AccessStatus.ALLOWED,  // read record permission 
+                                AccessStatus.ALLOWED,  // filing permission
+                                AccessStatus.ALLOWED,  // view record capability 
+                                AccessStatus.ALLOWED,  // edit non record metadata capability
+                                AccessStatus.DENIED))  // edit record metadata capability
+                    
+                // And a site consumer has read permissions and view record capability on the record
+                .as(dmConsumer)
+                    .perform(() -> 
+                        checkInPlaceAccess(contribDoc, 
+                                AccessStatus.ALLOWED,  // read record permission 
+                                AccessStatus.DENIED,   // filing permission
+                                AccessStatus.ALLOWED,  // view record capability 
+                                AccessStatus.DENIED,   // edit non record metadata capability
+                                AccessStatus.DENIED))  // edit record metadata capability   
+            
+                // And a user that is not a member of the site has no access to the inplace record     
+                .as(userName)
+                    .perform(() ->    
+                        checkInPlaceAccess(contribDoc, 
+                                AccessStatus.DENIED,   // read record permission 
+                                AccessStatus.DENIED,   // filing permission
+                                AccessStatus.DENIED,   // view record capability 
+                                AccessStatus.DENIED,   // edit non record metadata capability
+                                AccessStatus.DENIED));  // edit record metadata capability   
+    }
+    
     /**
      * Given an unfiled in-place record
      * When the record is moved to the file plan (ie filed)
@@ -245,6 +381,16 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                             AccessStatus.ALLOWED,  // view record capability 
                             AccessStatus.ALLOWED,  // edit non record metadata capability
                             AccessStatus.DENIED))  // edit record metadata capability
+                    
+                 // And a site contributor has read and view    
+                .as(dmContributor)
+                    .perform(() ->               
+                        checkInPlaceAccess(dmDocument, 
+                            AccessStatus.ALLOWED,  // read record permission 
+                            AccessStatus.DENIED,  // filing permission
+                            AccessStatus.ALLOWED,  // view record capability 
+                            AccessStatus.DENIED,  // edit non record metadata capability
+                            AccessStatus.DENIED))  // edit record metadata capability    
             
                 // And the consumer has read permissions and view record capability on the record       
                 .as(dmConsumer)
@@ -304,6 +450,16 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                             AccessStatus.ALLOWED,  // view record capability 
                             AccessStatus.DENIED,  // edit non record metadata capability
                             AccessStatus.DENIED))  // edit record metadata capability
+                    
+                // And a site contributor has read and view    
+                .as(dmContributor)
+                    .perform(() ->               
+                        checkInPlaceAccess(dmDocument, 
+                            AccessStatus.ALLOWED,  // read record permission 
+                            AccessStatus.DENIED,  // filing permission
+                            AccessStatus.ALLOWED,  // view record capability 
+                            AccessStatus.DENIED,  // edit non record metadata capability
+                            AccessStatus.DENIED))  // edit record metadata capability    
             
                 // And the consumer has read permissions and view record capability on the record       
                 .as(dmConsumer)
@@ -386,6 +542,16 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                             AccessStatus.DENIED,  // view record capability 
                             AccessStatus.DENIED,  // edit non record metadata capability
                             AccessStatus.DENIED))  // edit record metadata capability
+                    
+                // And a site contributor has no permissions or capabilities
+                .as(dmContributor)
+                    .perform(() ->               
+                        checkInPlaceAccess(dmDocument, 
+                            AccessStatus.DENIED,  // read record permission 
+                            AccessStatus.DENIED,  // filing permission
+                            AccessStatus.DENIED,  // view record capability 
+                            AccessStatus.DENIED,  // edit non record metadata capability
+                            AccessStatus.DENIED))  // edit record metadata capability
             
                 // And the consumer has no permissions or capabilities   
                 .as(dmConsumer)
@@ -496,6 +662,16 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                              AccessStatus.DENIED,  // view record capability 
                              AccessStatus.DENIED,  // edit non record metadata capability
                              AccessStatus.DENIED))  // edit record metadata capability
+                     
+                 // And a site contributor has read and view    
+                 .as(dmContributor)
+                     .perform(() ->               
+                         checkInPlaceAccess(dmDocument, 
+                             AccessStatus.DENIED,  // read record permission 
+                             AccessStatus.DENIED,  // filing permission
+                             AccessStatus.DENIED,  // view record capability 
+                             AccessStatus.DENIED,  // edit non record metadata capability
+                             AccessStatus.DENIED))  // edit record metadata capability
              
                  // And the consumer has no access to the record   
                  .as(dmConsumer)
@@ -554,6 +730,16 @@ public class InplaceRecordPermissionTest extends BaseRMTestCase
                              AccessStatus.DENIED,  // view record capability 
                              AccessStatus.DENIED,  // edit non record metadata capability
                              AccessStatus.DENIED))  // edit record metadata capability
+                     
+                 // And a site contributor has read and view    
+                 .as(dmContributor)
+                     .perform(() ->               
+                         checkInPlaceAccess(dmDocument, 
+                             AccessStatus.DENIED,  // read record permission 
+                             AccessStatus.DENIED,  // filing permission
+                             AccessStatus.DENIED,  // view record capability 
+                             AccessStatus.DENIED,  // edit non record metadata capability
+                             AccessStatus.DENIED))  // edit record metadata capability
              
                  // And the consumer has no access to the record   
                  .as(dmConsumer)

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

@@ -423,11 +423,12 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
             @Override
             public Object execute() throws Throwable
             {
-                // As system user
-                AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getSystemUserName());
-
                 // Do the tear down
-                tearDownImpl();
+                AuthenticationUtil.runAsSystem(() -> 
+                {
+                    tearDownImpl();
+                    return null;
+                });
 
                 return null;
             }
@@ -607,8 +608,13 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
             @Override
             public Object execute() throws Throwable
             {
-                AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getAdminUserName());
+                AuthenticationUtil.runAs(() -> 
-                setupTestUsersImpl(filePlan);
+                {
+                    setupTestUsersImpl(filePlan);
+                    return null;
+                }, 
+                AuthenticationUtil.getAdminUserName());
+                
                 return null;
             }
         });
@@ -693,13 +699,13 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
             @Override
             public Object execute() throws Throwable
             {
-                // As system user
+                return AuthenticationUtil.runAsSystem(() ->
-                AuthenticationUtil.setFullyAuthenticatedUser(AuthenticationUtil.getSystemUserName());
+                {
-
+                    // Do setup
-                // Do setup
+                    setupMultiHierarchyTestDataImpl();
-                setupMultiHierarchyTestDataImpl();
+                    
-
+                    return null;
-                return null;
+                });
             }
         });
     }

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/bdt/BehaviourTest.java

@@ -235,14 +235,15 @@ public class BehaviourTest
      */
     public BehaviourTest perform(Work work)
     {
-        return retryingTransactionHelper.doInTransaction(() -> 
+        return AuthenticationUtil.runAs(() -> 
         {
-            return AuthenticationUtil.runAs(() -> 
+            return retryingTransactionHelper.doInTransaction(() -> 
             {
                 work.doIt();                
                 return this;
-            }, this.asUser);
+            });
-        });        
+        },
+        this.asUser);  
     }
     
     /**

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/util/bdt/ExpectedValue.java

@@ -60,13 +60,14 @@ public class ExpectedValue<T>
     
     public BehaviourTest because(String message)
     {
-        T actualValue = (T)test.getRetryingTransactionHelper().doInTransaction(() -> 
+        T actualValue = (T)AuthenticationUtil.runAs(() -> 
         {
-            return AuthenticationUtil.runAs(() -> 
+            return test.getRetryingTransactionHelper().doInTransaction(() -> 
             {
                 return evaluation.eval();
-            }, test.getAsUser());
+            });
-        });        
+        }, 
+        test.getAsUser());        
         
         if (message != null)
         {