diff --git a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/fileplancomponents/UpdateRecordsTests.java b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/fileplancomponents/UpdateRecordsTests.java index e2738c2fca..82ff77fabe 100644 --- a/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/fileplancomponents/UpdateRecordsTests.java +++ b/rm-automation/rm-automation-community-rest-api/src/test/java/org/alfresco/rest/rm/community/fileplancomponents/UpdateRecordsTests.java @@ -29,8 +29,8 @@ package org.alfresco.rest.rm.community.fileplancomponents; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.IMAGE_FILE; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createElectronicRecordModel; import static org.alfresco.rest.rm.community.utils.FilePlanComponentsUtil.createNonElectronicRecordModel; -import static org.springframework.http.HttpStatus.BAD_REQUEST; import static org.springframework.http.HttpStatus.CREATED; +import static org.springframework.http.HttpStatus.FORBIDDEN; import static org.springframework.http.HttpStatus.OK; import static org.testng.Assert.assertEquals; @@ -62,10 +62,10 @@ public class UpdateRecordsTests extends BaseRMRestTest { @Autowired private RMUserAPI rmUserAPI; - + /* to be used to append to modifications */ private final String MODIFIED_PREFIX = "modified_"; - + /** *
      * Given an incomplete record
@@ -83,14 +83,14 @@ public class UpdateRecordsTests extends BaseRMRestTest
     public void incompleteRecordsCanBeUpdated(FilePlanComponent recordFolder) throws Exception
     {
         FilePlanComponentAPI filePlanComponentsAPI = getRestAPIFactory().getFilePlanComponentsAPI();
-        
+
         // create electronic and non-electronic records in a folder
         FilePlanComponent electronicRecord = filePlanComponentsAPI.createElectronicRecord(createElectronicRecordModel(), IMAGE_FILE, recordFolder.getId());
         assertStatusCode(CREATED);
         FilePlanComponent nonElectronicRecord = filePlanComponentsAPI.createFilePlanComponent(createNonElectronicRecordModel(), recordFolder.getId());
         assertStatusCode(CREATED);
-        
-        for (FilePlanComponent record: Arrays.asList(electronicRecord, nonElectronicRecord)) {            
+
+        for (FilePlanComponent record: Arrays.asList(electronicRecord, nonElectronicRecord)) {
             // generate update metadata
             String newName = getModifiedPropertyValue(record.getName());
             String newTitle = getModifiedPropertyValue(record.getProperties().getTitle());
@@ -115,7 +115,7 @@ public class UpdateRecordsTests extends BaseRMRestTest
             assertEquals(updatedRecord.getProperties().getDescription(), newDescription);
         }
     }
-    
+
     /**
      * 
      * Given an incomplete record
@@ -131,7 +131,7 @@ public class UpdateRecordsTests extends BaseRMRestTest
     )
     @AlfrescoTest(jira="RM-4362")
     public void userWithEditMetadataCapsCanUpdateMetadata() throws Exception
-    {   
+    {
         // create test user and add it with collab. privileges
         UserModel updateUser = getDataUser().createRandomTestUser("updateuser");
         updateUser.setUserRole(UserRole.SiteCollaborator);
@@ -151,21 +151,21 @@ public class UpdateRecordsTests extends BaseRMRestTest
         rmUserAPI.addUserPermission(filePlanComponentsAPIAsAdmin.getFilePlanComponent(randomFolder.getParentId()),
             updateUser, UserPermissions.PERMISSION_FILING);
         rmUserAPI.usingRestWrapper().assertStatusCodeIs(OK);
-        
+
         // create electronic and non-electronic records in a folder
         FilePlanComponentAPI filePlanComponentsAPI = getRestAPIFactory().getFilePlanComponentsAPI();
         FilePlanComponent electronicRecord = filePlanComponentsAPI.createElectronicRecord(createElectronicRecordModel(), IMAGE_FILE, randomFolder.getId());
         assertStatusCode(CREATED);
         FilePlanComponent nonElectronicRecord = filePlanComponentsAPI.createFilePlanComponent(createNonElectronicRecordModel(), randomFolder.getId());
         assertStatusCode(CREATED);
-        
+
         // get FilePlanComponentAPI instance initialised to updateUser
         FilePlanComponentAPI filePlanComponentsAPIAsUser = getRestAPIFactory().getFilePlanComponentsAPI(updateUser);
-        
+
         for (FilePlanComponent record: Arrays.asList(electronicRecord, nonElectronicRecord)) {
             filePlanComponentsAPIAsUser.getFilePlanComponent(record.getId());
             assertStatusCode(OK);
-            
+
             // generate update metadata
             String newName = getModifiedPropertyValue(record.getName());
             String newTitle = getModifiedPropertyValue(record.getProperties().getTitle());
@@ -191,7 +191,7 @@ public class UpdateRecordsTests extends BaseRMRestTest
             assertEquals(updatedRecord.getModifiedByUser().getId(), updateUser.getUsername());
         }
     }
-    
+
     /**
      * 
      * Given a complete record
@@ -210,16 +210,16 @@ public class UpdateRecordsTests extends BaseRMRestTest
     public void completeRecordsCantBeUpdated(FilePlanComponent recordFolder) throws Exception
     {
         FilePlanComponentAPI filePlanComponentsAPI = getRestAPIFactory().getFilePlanComponentsAPI();
-        
+
         // create electronic and non-electronic records in a folder
         FilePlanComponent electronicRecord = filePlanComponentsAPI.createElectronicRecord(createElectronicRecordModel(), IMAGE_FILE, recordFolder.getId());
         assertStatusCode(CREATED);
         closeRecord(electronicRecord);
-       
+
         FilePlanComponent nonElectronicRecord = filePlanComponentsAPI.createFilePlanComponent(createNonElectronicRecordModel(), recordFolder.getId());
         assertStatusCode(CREATED);
         closeRecord(nonElectronicRecord);
-        
+
         for (FilePlanComponent record: Arrays.asList(electronicRecord, nonElectronicRecord)) {
             // generate update metadata
             String newName = getModifiedPropertyValue(record.getName());
@@ -236,7 +236,7 @@ public class UpdateRecordsTests extends BaseRMRestTest
 
             // attempt to update record
             filePlanComponentsAPI.updateFilePlanComponent(updateRecord, record.getId());
-            assertStatusCode(BAD_REQUEST);
+            assertStatusCode(FORBIDDEN);
 
             // verify the original record metatada has been retained
             FilePlanComponent updatedRecord = filePlanComponentsAPI.getFilePlanComponent(record.getId());
@@ -245,7 +245,7 @@ public class UpdateRecordsTests extends BaseRMRestTest
             assertEquals(updatedRecord.getProperties().getDescription(), record.getProperties().getTitle());
         }
     }
-    
+
     /**
      * Helper method to generate modified property value based on original value
      * @param originalValue original value
diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml
index 4ef9447a72..63242f2ab1 100644
--- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml
+++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/module-context.xml
@@ -78,6 +78,7 @@
    
       
       
+      
    
 
    
diff --git a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml
index 56d5b0f6af..0301dd0644 100644
--- a/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml
+++ b/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml
@@ -1,7 +1,18 @@
 
-
-
-
+
 
    
 
@@ -569,9 +580,9 @@
           parent="baseService">
         
         
-                
-                
-           
+        
+        
+        
         
     
 
@@ -1060,8 +1071,21 @@
         
         
         
+        
     
 
+   
+   
+      http://www.alfresco.org/model/security/1.0
+      http://www.alfresco.org/model/system/1.0
+      http://www.alfresco.org/model/workflow/1.0
+      http://www.alfresco.org/model/application/1.0
+      http://www.alfresco.org/model/datalist/1.0
+      http://www.alfresco.org/model/dictionary/1.0
+      http://www.alfresco.org/model/bpm/1.0
+      http://www.alfresco.org/model/rendition/1.0
+   
+
     
     	
     	
diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessor.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessor.java
index 660cb6bc90..9fc6e6bc1f 100644
--- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessor.java
+++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessor.java
@@ -31,6 +31,8 @@ import java.util.List;
 
 import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.repo.security.permissions.PermissionReference;
+import org.alfresco.repo.security.permissions.impl.model.PermissionModel;
 import org.alfresco.repo.security.permissions.processor.impl.PermissionPostProcessorBaseImpl;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
@@ -39,47 +41,81 @@ import org.alfresco.service.cmr.security.PermissionService;
 
 /**
  * Records management permission post processor.
- * 
+ *
  * @author Roy Wetherall
  * @since 2.4.a
  */
-public class RecordsManagementPermissionPostProcessor extends PermissionPostProcessorBaseImpl 
+public class RecordsManagementPermissionPostProcessor extends PermissionPostProcessorBaseImpl
 {
-	/** node service */
-	private NodeService nodeService;
-	public void setNodeService(NodeService nodeService) {this.nodeService=nodeService;}
-	
-	/** permission service */
-	private PermissionService permissionService;
-	public void setPermissionService(PermissionService permissionService) {this.permissionService=permissionService;}
-	
-	/**
-	 * @see org.alfresco.repo.security.permissions.processor.PermissionPostProcessor#process(org.alfresco.service.cmr.security.AccessStatus, org.alfresco.service.cmr.repository.NodeRef, java.lang.String)
-	 */
-	@Override
-	public AccessStatus process(AccessStatus accessStatus, NodeRef nodeRef, String perm,
-								List configuredReadPermissions, List configuredFilePermissions)
-	{
-		AccessStatus result = accessStatus;
-		if (AccessStatus.DENIED.equals(accessStatus) &&
+    /** node service */
+    private NodeService nodeService;
+    public void setNodeService(NodeService nodeService) {this.nodeService=nodeService;}
+
+    /** permission service */
+    private PermissionService permissionService;
+    public void setPermissionService(PermissionService permissionService) {this.permissionService=permissionService;}
+
+    /** The permission model DAO. */
+    private PermissionModel permissionModel;
+    public void setPermissionModel(PermissionModel permissionModel) {this.permissionModel=permissionModel;}
+
+    /**
+     * @see org.alfresco.repo.security.permissions.processor.PermissionPostProcessor#process(org.alfresco.service.cmr.security.AccessStatus, org.alfresco.service.cmr.repository.NodeRef, java.lang.String)
+     */
+    @Override
+    public AccessStatus process(AccessStatus accessStatus, NodeRef nodeRef, String perm,
+                                List configuredReadPermissions, List configuredFilePermissions)
+    {
+        AccessStatus result = accessStatus;
+        if (AccessStatus.DENIED.equals(accessStatus) &&
             nodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT))
-		{        
-			// if read denied on rm artifact
-	        if (PermissionService.READ.equals(perm) || configuredReadPermissions.contains(perm))
-	        {
-	        	// check for read record
-	            result = permissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS);
-	        }
-	        // if write deinied on rm artificat
-	        else if (PermissionService.WRITE.equals(perm) || configuredFilePermissions.contains(perm))
-	        {
-	        	// check for file record
-	        	result = permissionService.hasPermission(nodeRef, RMPermissionModel.FILE_RECORDS);
-	        }
-		}
-		
-		return result;
-	
-	}
+        {
+            // if read denied on rm artifact
+            if (PermissionService.READ.equals(perm) || isPermissionContained(perm, configuredReadPermissions))
+            {
+                // check for read record
+                result = permissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS);
+            }
+            // if write denied on rm artifact
+            else if (PermissionService.WRITE.equals(perm) || isPermissionContained(perm, configuredFilePermissions))
+            {
+                // check for file record
+                result = permissionService.hasPermission(nodeRef, RMPermissionModel.FILE_RECORDS);
+            }
+        }
+
+        return result;
+
+    }
+
+    /**
+     * Check if a given permission is implied by a list of permission groups.
+     *
+     * @param perm The name of the permission in question.
+     * @param configuredPermissions The list of permission group names.
+     * @return true if the permission is contained or implied by the list of permissions.
+     */
+    private boolean isPermissionContained(String perm, List configuredPermissions)
+    {
+        // Check if the permission is explicitly in the list
+        if (configuredPermissions.contains(perm))
+        {
+            return true;
+        }
+        // Check if the permission is implied by one from the list.
+        for (String configuredPermission : configuredPermissions)
+        {
+            // TODO: Here we are assuming the permission name is unique across all contexts (but I think we're doing that in the properties file anyway).
+            PermissionReference permissionReference = permissionModel.getPermissionReference(null, configuredPermission);
+            for (PermissionReference granteePermission : permissionModel.getGranteePermissions(permissionReference))
+            {
+                if (granteePermission.getName().equals(perm))
+                {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
 
 }
diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java
index b40c64b197..d274d78e8f 100644
--- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java
+++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/record/RecordServiceImpl.java
@@ -27,8 +27,6 @@
 
 package org.alfresco.module.org_alfresco_module_rm.record;
 
-import static com.google.common.collect.Lists.newArrayList;
-
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -158,18 +156,22 @@ public class RecordServiceImpl extends BaseBehaviourBean
     };
 
     /** always edit model URI's */
+    private List alwaysEditURIs;
+
+    /**
+     * @param alwaysEditURIs the alwaysEditURIs to set
+     */
+    public void setAlwaysEditURIs(List alwaysEditURIs)
+    {
+        this.alwaysEditURIs = alwaysEditURIs;
+    }
+
+    /**
+     * @return the alwaysEditURIs
+     */
     protected List getAlwaysEditURIs()
     {
-        return newArrayList(
-            NamespaceService.SECURITY_MODEL_1_0_URI,
-            NamespaceService.SYSTEM_MODEL_1_0_URI,
-            NamespaceService.WORKFLOW_MODEL_1_0_URI,
-            NamespaceService.APP_MODEL_1_0_URI,
-            NamespaceService.DATALIST_MODEL_1_0_URI,
-            NamespaceService.DICTIONARY_MODEL_1_0_URI,
-            NamespaceService.BPM_MODEL_1_0_URI,
-            NamespaceService.RENDITION_MODEL_1_0_URI
-        );
+        return this.alwaysEditURIs;
     }
 
     /** record model URI's */
diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/repo/security/permissions/processor/PermissionPostProcessor.java b/rm-community/rm-community-repo/source/java/org/alfresco/repo/security/permissions/processor/PermissionPostProcessor.java
index 438b3bbd4a..3a38462f5a 100644
--- a/rm-community/rm-community-repo/source/java/org/alfresco/repo/security/permissions/processor/PermissionPostProcessor.java
+++ b/rm-community/rm-community-repo/source/java/org/alfresco/repo/security/permissions/processor/PermissionPostProcessor.java
@@ -52,5 +52,5 @@ public interface PermissionPostProcessor
 	 * @return {@link AccessStatus}
 	 */
 	AccessStatus process(AccessStatus accessStatus, NodeRef nodeRef, String perm,
-						List configuredReadPermissions, List configuredFilePermissions);
+						List configuredReadPermissions, List configuredFilePermissions);
 }
diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessorUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessorUnitTest.java
index cfa99cae76..f0c25e9e96 100644
--- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessorUnitTest.java
+++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/permission/RecordsManagementPermissionPostProcessorUnitTest.java
@@ -28,8 +28,8 @@
 package org.alfresco.module.org_alfresco_module_rm.permission;
 
 import static java.util.Arrays.asList;
-
 import static org.junit.Assert.assertEquals;
+import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.when;
 
 import java.util.List;
@@ -37,6 +37,8 @@ import java.util.List;
 import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.test.util.AlfMock;
+import org.alfresco.repo.security.permissions.PermissionReference;
+import org.alfresco.repo.security.permissions.impl.model.PermissionModel;
 import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.repository.NodeService;
 import org.alfresco.service.cmr.security.AccessStatus;
@@ -47,20 +49,26 @@ import org.mockito.InjectMocks;
 import org.mockito.Mock;
 import org.mockito.MockitoAnnotations;
 
+import com.google.common.collect.Sets;
+
 /**
  * Unit tests for {@link RecordsManagementPermissionPostProcessor}.
  *
  * @author David Webster
+ * @author Tom Page
  * @since 2.4.1
  */
 public class RecordsManagementPermissionPostProcessorUnitTest
 {
+    @InjectMocks
+    private RecordsManagementPermissionPostProcessor recordsManagementPermissionPostProcessor = new RecordsManagementPermissionPostProcessor();
 
-    private @InjectMocks
-    RecordsManagementPermissionPostProcessor recordsManagementPermissionPostProcessor = new RecordsManagementPermissionPostProcessor();
-
-    private @Mock NodeService nodeService;
-    private @Mock PermissionService permissionService;
+    @Mock
+    private NodeService mockNodeService;
+    @Mock
+    private PermissionService mockPermissionService;
+    @Mock
+    private PermissionModel mockPermissionModel;
 
     @Before
     public void setup()
@@ -83,9 +91,9 @@ public class RecordsManagementPermissionPostProcessorUnitTest
         List configuredReadPermissions = asList("ReadProperties", "ReadChildren", perm);
         List configuredFilePermissions = asList("WriteProperties", "AddChildren");
 
-        when(nodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT))
+        when(mockNodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT))
             .thenReturn(true);
-        when(permissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS))
+        when(mockPermissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS))
             .thenReturn(AccessStatus.ALLOWED);
 
         AccessStatus result = recordsManagementPermissionPostProcessor.process(accessStatus, nodeRef, perm, configuredReadPermissions, configuredFilePermissions);
@@ -108,13 +116,51 @@ public class RecordsManagementPermissionPostProcessorUnitTest
         List configuredReadPermissions = asList("ReadProperties", "ReadChildren");
         List configuredFilePermissions = asList("WriteProperties", "AddChildren");
 
-        when(nodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT))
+        when(mockNodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT))
             .thenReturn(true);
-        when(permissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS))
+        when(mockPermissionService.hasPermission(nodeRef, RMPermissionModel.READ_RECORDS))
             .thenReturn(AccessStatus.ALLOWED);
 
         AccessStatus result = recordsManagementPermissionPostProcessor.process(accessStatus, nodeRef, perm, configuredReadPermissions, configuredFilePermissions);
 
         assertEquals(AccessStatus.DENIED, result);
     }
+
+    /**
+     * Test that the permission groups configured in the global properties file imply descendant permission groups.
+     * 

+ * Given a configured permission is an ancestor of another permission P + * And the post processor checks if the user has P + * Then the post processor says that they do. + */ + @Test + public void permissionInherittedFromConfiguredGroup() + { + NodeRef nodeRef = new NodeRef("node://ref/"); + // permissions do not include perm created above + List configuredReadPermissions = asList(); + List configuredFilePermissions = asList("WriteProperties"); + + when(mockNodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT)) + .thenReturn(true); + when(mockPermissionService.hasPermission(nodeRef, RMPermissionModel.FILE_RECORDS)) + .thenReturn(AccessStatus.ALLOWED); + + // Set up "WriteProperties" to imply three other permission groups. + PermissionReference mockWritePropsPermRef = mock(PermissionReference.class); + when(mockPermissionModel.getPermissionReference(null, "WriteProperties")).thenReturn(mockWritePropsPermRef); + PermissionReference childOne = mock(PermissionReference.class); + when(childOne.getName()).thenReturn("Not this one"); + PermissionReference childTwo = mock(PermissionReference.class); + when(childTwo.getName()).thenReturn("This is the requested permission"); + PermissionReference childThree = mock(PermissionReference.class); + when(childThree.getName()).thenReturn("Not this one either"); + when(mockPermissionModel.getGranteePermissions(mockWritePropsPermRef)).thenReturn(Sets.newHashSet(childOne, childTwo, childThree)); + + // Call the method under test. + AccessStatus result = recordsManagementPermissionPostProcessor.process(AccessStatus.DENIED, nodeRef, + "This is the requested permission", configuredReadPermissions, configuredFilePermissions); + + assertEquals(AccessStatus.ALLOWED, result); + } }