RM-600: A user with the "EditDocumentMetaData" capability can only edit the properties of the records content.

* record service extended with isPropertyEditable method .. indicates, based on the capabilities of the current user and the nature of the property, whether it can be edited
  * the record service monitors record aspect property updates and throws ModelExceptions if a user tries to update a record or non-record property if they don't have the appropriate capability
  * form filter updated to protect record properties the current user may not be able to edit
  * unit tests

So what does this all mean?  

It means we can control the write permissions of individual properties on a record based on capabilities.  In this case we slice the capability to edit a record in two .. you can either edit the content properties (ie the normal meta-data) or the record properties .. or both of course!



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

rm-server/config/alfresco/module/org_alfresco_module_rm/capability/groups/rm-capability-groups-context.xml

@@ -58,12 +58,6 @@
       <property name="index" value="80"/>
    </bean>
 
-   <bean id="metadataControlGroup"
-      parent="groupBase">
-      <property name="id" value="metadataControl"/>
-      <property name="index" value="90"/>
-   </bean>
-
    <bean id="referencesGroup"
       parent="groupBase">
       <property name="id" value="references"/>

rm-server/config/alfresco/module/org_alfresco_module_rm/capability/rm-capabilities-record-context.xml

@@ -183,35 +183,38 @@
       <property name="conditions">
          <map>
             <entry key="capabilityCondition.filling" value="true"/>
-            <entry key="capabilityCondition.cutoff" value="false"/>
             <entry key="capabilityCondition.frozen" value="false"/>
             <entry key="capabilityCondition.declared" value="true"/>
          </map>
       </property>
-      <property name="group"><ref bean="metadataControlGroup"/></property>
+      <property name="group"><ref bean="recordsGroup"/></property>
       <property name="index" value="10" />
    </bean>
 
-   <!-- TODO .. this is being used for the wrong thing! ... should indicate that the use can edit the non-record meta-data of a record -->
-   <!-- TODO .. may need some kind of 'prep' record capability you get with file? .. I think this is what this is being used for atm -->
    <bean id="rmEditNonRecordMetadataCapability"
-      parent="declarativeCapability"
+      parent="declarativeCapability">
-      class="org.alfresco.module.org_alfresco_module_rm.capability.impl.EditCapability">
       <property name="name" value="EditNonRecordMetadata"/>
       <property name="permission" value="EditNonRecordMetadata"/>
+      <property name="kinds">
+         <list>
+            <value>RECORD</value>
+         </list>
+      </property>
       <property name="conditions">
          <map>
-            <entry key="capabilityCondition.fileable" value="true"/>
+            <entry key="capabilityCondition.filling" value="true"/>
+            <entry key="capabilityCondition.cutoff" value="false"/>
             <entry key="capabilityCondition.frozen" value="false"/>
+            <entry key="capabilityCondition.closed" value="false"/>
+            <entry key="capabilityCondition.declared" value="false"/>
          </map>
       </property>
-      <property name="group"><ref bean="metadataControlGroup"/></property>
+      <property name="group"><ref bean="recordsGroup"/></property>
       <property name="index" value="20" />
    </bean>
 
-   <bean id="rmEditRecordMetadataCapability"
+   <bean id="rmEditRecordMetadataCapability"      
-      parent="rmBaseCapability"
+      parent="declarativeCapability">
-      class="org.alfresco.module.org_alfresco_module_rm.capability.impl.EditCapability">
       <property name="name" value="EditRecordMetadata"/>
       <property name="permission" value="EditRecordMetadata"/>
       <property name="kinds">
@@ -228,7 +231,7 @@
             <entry key="capabilityCondition.declared" value="false"/>
          </map>
       </property>
-      <property name="group"><ref bean="metadataControlGroup"/></property>
+      <property name="group"><ref bean="recordsGroup"/></property>
       <property name="index" value="30" />
    </bean>
 
@@ -246,7 +249,7 @@
          </map>
       </property>
       <property name="targetCapability" ref="rmFileRecordsCapability"/>
-      <property name="group"><ref bean="metadataControlGroup"/></property>
+      <property name="group"><ref bean="recordsGroup"/></property>
       <property name="index" value="40" />
    </bean>
 

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

@@ -255,7 +255,6 @@
             <aspect>rma:recordComponentIdentifier</aspect>
             <aspect>rma:commonRecordDetails</aspect>
             <aspect>rma:filePlanComponent</aspect>
-        <!-- <aspect>rma:vitalRecordDefinition</aspect> -->
          </mandatory-aspects>
 
       </type>
@@ -590,6 +589,7 @@
             <property name="rma:rootNodeRef">
                <title>Root node reference</title>
                <type>d:noderef</type>
+               <protected>true</protected>
             </property>
        	</properties>
       </aspect>
@@ -659,6 +659,7 @@
             <property name="rma:identifier">
                <title>Record Component Identifier</title>
                <type>d:text</type>
+               <protected>true</protected>
                <mandatory>true</mandatory>
                <index enabled="true">
 		          <atomic>true</atomic>
@@ -837,9 +838,11 @@
       	<properties>
       		<property name="rma:readers">
       			<type>d:any</type>
+      			<protected>true</protected>
       		</property>
       		<property name="rma:writers">
       			<type>d:any</type>
+      			<protected>true</protected>
       		</property>
       	</properties>
       </aspect>
@@ -857,6 +860,7 @@
             <property name="rma:notificationIssued">
                <title>Indicates whether a notification that this record is due for review has been issued</title>
                <type>d:boolean</type>
+      		   <protected>true</protected>
                <mandatory>false</mandatory>
                <default>false</default>
             </property>
@@ -1025,9 +1029,11 @@
          <properties>
             <property name="rma:recordSearchHasDispositionSchedule">
                <type>d:boolean</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordSearchDispositionActionName">
                <type>d:text</type>
+      			<protected>true</protected>
                <index enabled="true">
 		          <atomic>true</atomic>
 		          <stored>false</stored>
@@ -1036,37 +1042,43 @@
             </property>
             <property name="rma:recordSearchDispositionActionAsOf">
                <type>d:date</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordSearchDispositionPeriod">
-               <type>d:text</type>
+               	<type>d:text</type>
-               <index enabled="true">
+      			<protected>true</protected>
+               	<index enabled="true">
 		          <atomic>true</atomic>
 		          <stored>false</stored>
 		          <tokenised>false</tokenised>
-		       </index>
+		       	</index>
             </property>
             <property name="rma:recordSearchDispositionPeriodExpression">
-               <type>d:text</type>
+               	<type>d:text</type>
-               <index enabled="true">
+      			<protected>true</protected>
+               	<index enabled="true">
 		          <atomic>true</atomic>
 		          <stored>false</stored>
 		          <tokenised>false</tokenised>
-		       </index>
+		       	</index>
             </property>
             <property name="rma:recordSearchDispositionEventsEligible">
-               <type>d:boolean</type>
+               	<type>d:boolean</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordSearchDispositionEvents">
-               <type>d:text</type>
+               	<type>d:text</type>
-               <multiple>true</multiple>
+      			<protected>true</protected>
-               <index enabled="true">
+               	<multiple>true</multiple>
+               	<index enabled="true">
 		          <atomic>true</atomic>
 		          <stored>false</stored>
 		          <tokenised>false</tokenised>
-		       </index>
+		       	</index>
             </property>
             <property name="rma:recordSearchDispositionAuthority">
-               <type>d:text</type>
+                <type>d:text</type>
+      			<protected>true</protected>
                 <index enabled="true">
                     <atomic>true</atomic>
                     <stored>false</stored>
@@ -1075,12 +1087,15 @@
             </property>
             <property name="rma:recordSearchDispositionInstructions">
                <type>d:text</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordSearchHoldReason">
                <type>d:text</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordSearchVitalRecordReviewPeriod">
                <type>d:text</type>
+      			<protected>true</protected>
                <index enabled="true">
 		          <atomic>true</atomic>
 		          <stored>false</stored>
@@ -1089,6 +1104,7 @@
             </property>
             <property name="rma:recordSearchVitalRecordReviewPeriodExpression">
                <type>d:text</type>
+      			<protected>true</protected>
                <index enabled="true">
 		          <atomic>true</atomic>
 		          <stored>false</stored>
@@ -1147,13 +1163,16 @@
          <title>The originating details of a record</title>
          <properties>
             <property name="rma:recordOriginatingUserId">
-               <type>d:text</type>
+               	<type>d:text</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordOriginatingCreationDate">
-               <type>d:date</type>
+               	<type>d:date</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordOriginatingLocation">
-               <type>d:any</type>
+               	<type>d:any</type>
+      			<protected>true</protected>
             </property>
          </properties>
       </aspect>
@@ -1163,13 +1182,16 @@
          <title>The rejection details of a record</title>
          <properties>
             <property name="rma:recordRejectionUserId">
-               <type>d:text</type>
+               	<type>d:text</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordRejectionDate">
-               <type>d:date</type>
+               	<type>d:date</type>
+      			<protected>true</protected>
             </property>
             <property name="rma:recordRejectionReason">
-               <type>d:text</type>
+               	<type>d:text</type>
+      			<protected>true</protected>
             </property>
          </properties>
       </aspect>

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

@@ -481,8 +481,11 @@
       </property>
    </bean>
 
-   <bean id="broadcastVitalRecordDefinition" class="org.alfresco.module.org_alfresco_module_rm.vital.BroadcastVitalRecordDefinitionAction"
+   <bean id="broadcastVitalRecordDefinition" 
-                                            parent="rmAction" />
+         class="org.alfresco.module.org_alfresco_module_rm.vital.BroadcastVitalRecordDefinitionAction"
+         parent="rmAction">
+      <property name="filePlanAuthenticationService" ref="FilePlanAuthenticationService"/>
+   </bean>
 
    <!-- broadcast disposition action definition update -->
    <!-- bound to policy: allow -->

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

@@ -1101,6 +1101,7 @@
         <property name="filePlanService" ref="FilePlanService" />
         <property name="ownableService" ref="OwnableService" />
         <property name="notificationHelper" ref="recordsManagementNotificationHelper"/>
+        <property name="capabilityService" ref="CapabilityService" />
     </bean>
 
     <bean id="RecordService" class="org.springframework.aop.framework.ProxyFactoryBean">
@@ -1149,6 +1150,7 @@
                 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.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.*=RM_DENY
                 ]]>
             </value>

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/action/impl/DeclareRecordAction.java

@@ -30,6 +30,7 @@ import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.module.org_alfresco_module_rm.action.RMActionExecuterAbstractBase;
 import org.alfresco.repo.action.executer.ActionExecuterAbstractBase;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.service.cmr.action.Action;
 import org.alfresco.service.cmr.dictionary.AspectDefinition;
 import org.alfresco.service.cmr.dictionary.PropertyDefinition;
@@ -59,7 +60,7 @@ public class DeclareRecordAction extends RMActionExecuterAbstractBase
     * @see org.alfresco.repo.action.executer.ActionExecuterAbstractBase#executeImpl(org.alfresco.service.cmr.action.Action, org.alfresco.service.cmr.repository.NodeRef)
     */
    @Override
-   protected void executeImpl(Action action, NodeRef actionedUponNodeRef)
+   protected void executeImpl(final Action action, final NodeRef actionedUponNodeRef)
    {
       if (recordService.isRecord(actionedUponNodeRef) == true)
       {
@@ -75,8 +76,16 @@ public class DeclareRecordAction extends RMActionExecuterAbstractBase
                declaredProps.put(PROP_DECLARED_BY, AuthenticationUtil.getRunAsUser());
                this.nodeService.addAspect(actionedUponNodeRef, ASPECT_DECLARED_RECORD, declaredProps);
 
-               // remove all owner related rights 
+               AuthenticationUtil.runAsSystem(new RunAsWork<Void>()
-               this.ownableService.setOwner(actionedUponNodeRef, OwnableService.NO_OWNER);
+               {
+                  @Override
+                  public Void doWork() throws Exception
+                  {
+                      // remove all owner related rights 
+                      ownableService.setOwner(actionedUponNodeRef, OwnableService.NO_OWNER);
+                      return null;
+                   }
+                });
             }
             else
             {

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

@@ -1,100 +0,0 @@
-/*
- * Copyright (C) 2005-2011 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.capability.impl;
-
-import net.sf.acegisecurity.vote.AccessDecisionVoter;
-
-import org.alfresco.module.org_alfresco_module_rm.capability.declarative.DeclarativeCapability;
-import org.alfresco.repo.security.authentication.AuthenticationUtil;
-import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
-import org.alfresco.service.cmr.repository.NodeRef;
-import org.alfresco.service.cmr.security.OwnableService;
-
-/**
- * Edit capability, checks the permission and whether the current user is the owner of the 
- * object.
- * 
- * @author Roy Wetherall
- */
-public class EditCapability extends DeclarativeCapability 
-{
-    private OwnableService ownableService;
-
-    private OwnableService getOwnableService()
-    {
-        if (ownableService == null)
-        {
-            ownableService = (OwnableService)applicationContext.getBean("OwnableService");
-        }
-        return ownableService;
-    }
-    
-    /**
-     * @see org.alfresco.module.org_alfresco_module_rm.capability.declarative.DeclarativeCapability#evaluate(org.alfresco.service.cmr.repository.NodeRef)
-     */
-    public int evaluate(final NodeRef nodeRef)
-    {
-        // The default state is not knowing anything
-        int result = AccessDecisionVoter.ACCESS_ABSTAIN;
-        
-        // Check we are dealing with a file plan component
-        if (rmService.isFilePlanComponent(nodeRef) == true)
-        {
-            // Now the default state is denied
-            result = AccessDecisionVoter.ACCESS_DENIED;
-            
-            // Check the kind of the object, the permissions and the conditions
-            if (checkKinds(nodeRef) == true && checkConditions(nodeRef) == true)
-            {
-                if (checkPermissions(nodeRef) == true)
-                {
-                    result = AccessDecisionVoter.ACCESS_GRANTED;
-                }
-                else
-                {
-                    result = AuthenticationUtil.runAs(new RunAsWork<Integer>()
-                    {
-                        @Override
-                        public Integer doWork() throws Exception
-                        {
-                            Integer result = Integer.valueOf(AccessDecisionVoter.ACCESS_DENIED);
-                                
-                            // Since we know this is undeclared if you are the owner then you should be able to 
-                            // edit the records meta-data (otherwise how can it be declared by the user?)
-                            if (getOwnableService().hasOwner(nodeRef) == true)
-                            {
-                                String user = AuthenticationUtil.getFullyAuthenticatedUser();
-                                if (user != null &&
-                                    getOwnableService().getOwner(nodeRef).equals(user) == true)
-                                {
-                                    result = Integer.valueOf(AccessDecisionVoter.ACCESS_GRANTED);
-                                }
-                            }
-                            
-                            return result;
-                        }
-                        
-                    }, AuthenticationUtil.getSystemUserName()).intValue();
-                }
-            }
-        }
- 
-        return result;        
-    }
-}

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/forms/RecordsManagementNodeFormFilter.java

@@ -38,6 +38,7 @@ import org.alfresco.repo.forms.Form;
 import org.alfresco.repo.forms.PropertyFieldDefinition;
 import org.alfresco.repo.forms.processor.node.FieldUtils;
 import org.alfresco.repo.forms.processor.node.FormFieldConstants;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.service.cmr.dictionary.DataTypeDefinition;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
 import org.alfresco.service.cmr.dictionary.PropertyDefinition;
@@ -91,9 +92,10 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
         this.dispositionService = dispositionService;
     }
 
-    /*
+    /**
      * @see org.alfresco.repo.forms.processor.Filter#afterGenerate(java.lang.Object, java.util.List, java.util.List, org.alfresco.repo.forms.Form, java.util.Map)
      */
+    @Override
     public void afterGenerate(
             NodeRef nodeRef, 
             List<String> fields, 
@@ -118,6 +120,9 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
                 // add the supplemental marking list property
                 forceSupplementalMarkingListProperty(form, nodeRef);
                 
+                // protect uneditable properties
+                protectRecordProperties(form, nodeRef);
+                
                 // if the record is the result of an email we need to 'protect' some fields
                 if (this.nodeService.hasAspect(nodeRef, ImapModel.ASPECT_IMAP_CONTENT))
                 {
@@ -147,6 +152,11 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
         }
     }
     
+    /**
+     * 
+     * @param form
+     * @param nodeRef
+     */
     protected void addCustomPropertyFieldsToGroup(Form form, NodeRef nodeRef)
     {
         Set<QName> customisables = rmAdminService.getCustomisable(nodeRef);
@@ -165,6 +175,11 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
         }
     }
     
+    /**
+     * 
+     * @param form
+     * @param nodeRef
+     */
     protected void addRecordMetadataPropertyFieldsToGroup(Form form, NodeRef nodeRef)
     {
         Set<QName> aspects = recordService.getRecordMetaDataAspects();
@@ -210,7 +225,11 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
         }
     }
 
-    
+    /**
+     * 
+     * @param form
+     * @param nodeRef
+     */
     protected void addTransientProperties(Form form, NodeRef nodeRef)
     {
         if (recordService.isRecord(nodeRef) == true)
@@ -239,6 +258,13 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
         }        
     }
     
+    /**
+     * 
+     * @param form
+     * @param name
+     * @param type
+     * @param value
+     */
     protected void addTransientPropertyField(Form form, String name, QName type, Object value)
     {
         String dataKeyName = FormFieldConstants.PROP_DATA_PREFIX + name;
@@ -251,6 +277,47 @@ public class RecordsManagementNodeFormFilter extends RecordsManagementFormFilter
         form.addData(dataKeyName, value);
     }
     
+    /**
+     * 
+     * @param form
+     * @param nodeRef
+     */
+    protected void protectRecordProperties(Form form, NodeRef nodeRef)
+    {
+        List<FieldDefinition> fieldDefs = form.getFieldDefinitions();
+        for (FieldDefinition fieldDef : fieldDefs)
+        {
+            if (fieldDef.isProtectedField() == false)
+            {
+                String name = fieldDef.getName();
+                String prefixName = null;
+                if ("size".equals(name) || "mimetype".equals(name) || "encoding".equals(name))
+                {
+                    prefixName = "cm:content";
+                }
+                else
+                {                
+                    prefixName = fieldDef.getName();
+                }
+                
+                if (logger.isDebugEnabled() == true)
+                {
+                    logger.debug("Checking property " + prefixName + " is editable by user " + AuthenticationUtil.getFullyAuthenticatedUser());
+                }
+                
+                QName qname = QName.createQName(prefixName, namespaceService);
+                if (recordService.isPropertyEditable(nodeRef, qname) == false)
+                {
+                    if (logger.isDebugEnabled() == true)
+                    {
+                        logger.debug("   ... protected property");
+                    }
+                    fieldDef.setProtectedField(true);
+                }
+            }
+        }        
+    }
+    
     /**
      * Marks all the fields that contain data extracted from an email
      * as protected fields.

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/model/behaviour/RecordContainerType.java

@@ -128,7 +128,7 @@ public class RecordContainerType implements RecordsManagementModel,
     @Override
     public void onCreateChildAssociation(final ChildAssociationRef childAssocRef, boolean isNewNode)
     {   
-        filePlanAuthenticationService.runAsRmAdmin(new RunAsWork<Void>()
+        AuthenticationUtil.runAsSystem(new RunAsWork<Void>()
         {
             @Override
             public Void doWork() throws Exception

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

@@ -45,44 +45,69 @@ public abstract class ProtectedModelArtifact
     /** Set of capabilities */
     private Set<Capability> capabilities;
     
+    /** Capability names */
     private Set<String> capabilityNames;
     
+    /**
+     * @param namespaceService  namespace service
+     */
     public void setNamespaceService(NamespaceService namespaceService)
     {
         this.namespaceService = namespaceService;
     }
     
+    /**
+     * @param modelSecurityService  model security service
+     */
     public void setModelSecurityService(ModelSecurityService modelSecurityService)
     {
         this.modelSecurityService = modelSecurityService;
     }
     
+    /**
+     * Init method
+     */
     public void init()
     {
         modelSecurityService.register(this);
     }
     
+    /**
+     * @param name  artifact name (in cm:content form)
+     */
     public void setName(String name)
     {
         QName qname = QName.createQName(name, namespaceService);
         this.name = qname;
     }
     
+    /**
+     * @return  artifact QName
+     */
     public QName getQName()
     {
         return name;
     }
     
+    /**
+     * @param capabilities  capabilities
+     */
     public void setCapabilities(Set<Capability> capabilities)
     {
         this.capabilities = capabilities;
     }
     
+    /**
+     * @return  capabilities
+     */
     public Set<Capability> getCapabilities()
     {
         return capabilities;
     }
     
+    /**
+     * @return  capability names
+     */
     public Set<String> getCapilityNames()
     {
         if (capabilityNames == null && capabilities != null)

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

@@ -96,4 +96,13 @@ public interface RecordService
     * @param reason    The reason for rejection
     */
    void rejectRecord(NodeRef nodeRef, String reason);
+   
+   /**
+    * Indicates whether a property of a record is editable for the current user or not.
+    * 
+    * @param record     record
+    * @param property   property
+    * @return boolean   true if editable, false otherwise.
+    */
+   boolean isPropertyEditable(NodeRef record, QName property);
 }

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

@@ -31,11 +31,16 @@ import java.util.Set;
 import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.RecordsManagementService;
+import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
+import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule;
 import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService;
+import org.alfresco.module.org_alfresco_module_rm.dod5015.DOD5015Model;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.identifier.IdentifierService;
+import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementCustomModel;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.module.org_alfresco_module_rm.model.security.ModelAccessDeniedException;
 import org.alfresco.module.org_alfresco_module_rm.notification.RecordsManagementNotificationHelper;
 import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
 import org.alfresco.module.org_alfresco_module_rm.vital.VitalRecordServiceImpl;
@@ -48,31 +53,80 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.repo.security.permissions.AccessDeniedException;
 import org.alfresco.repo.security.permissions.impl.ExtendedPermissionService;
 import org.alfresco.service.cmr.dictionary.AspectDefinition;
+import org.alfresco.service.cmr.dictionary.ClassDefinition;
 import org.alfresco.service.cmr.dictionary.DictionaryService;
+import org.alfresco.service.cmr.dictionary.PropertyDefinition;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 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.OwnableService;
 import org.alfresco.service.cmr.security.PermissionService;
+import org.alfresco.service.namespace.NamespaceService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.util.EqualsHelper;
 import org.alfresco.util.ParameterCheck;
+import org.apache.commons.lang.ArrayUtils;
 import org.apache.commons.lang.StringUtils;
+import org.apache.commons.logging.Log;
+import org.apache.commons.logging.LogFactory;
 import org.springframework.beans.BeansException;
 import org.springframework.context.ApplicationContext;
 import org.springframework.context.ApplicationContextAware;
 
 /**
- * Record service implementation
+ * Record service implementation.
  *
  * @author Roy Wetherall
  * @since 2.1
  */
 public class RecordServiceImpl implements RecordService,
                                           RecordsManagementModel,
+                                          RecordsManagementCustomModel,
                                           NodeServicePolicies.OnCreateChildAssociationPolicy,
+                                          NodeServicePolicies.OnUpdatePropertiesPolicy,
                                           ApplicationContextAware
 {
+    /** Logger */
+    private static Log logger = LogFactory.getLog(RecordServiceImpl.class);
+    
+    /** Always edit property array */
+    private static final QName[] ALWAYS_EDIT_PROPERTIES = new QName[]
+    {
+       ContentModel.PROP_LAST_THUMBNAIL_MODIFICATION_DATA
+    };
+    
+    private static final String[] ALWAYS_EDIT_URIS = new String[]
+    {
+        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
+     }; 
+    
+    private static final String[] RECORD_MODEL_URIS = new String[]
+    {
+       RM_URI,
+       RM_CUSTOM_URI,
+       DOD5015Model.DOD_URI
+    };
+    
+    private static final String[] NON_RECORD_MODEL_URIS = new String[]
+    {
+        NamespaceService.AUDIO_MODEL_1_0_URI,
+        NamespaceService.CONTENT_MODEL_1_0_URI,
+        NamespaceService.EMAILSERVER_MODEL_URI,
+        NamespaceService.EXIF_MODEL_1_0_URI,
+        NamespaceService.FORUMS_MODEL_1_0_URI,
+        NamespaceService.LINKS_MODEL_1_0_URI,
+        NamespaceService.REPOSITORY_VIEW_1_0_URI
+
+    };
+    
     /** Application context */
     private ApplicationContext applicationContext;
 
@@ -108,15 +162,22 @@ public class RecordServiceImpl implements RecordService,
 
     /** Ownable service */
     private OwnableService ownableService;
+    
+    /** Capability service */
+    private CapabilityService capabilityService;
 
     /** List of available record meta-data aspects */
-    private Set<QName> recordMetaDataAspects;
+    private Set<QName> recordMetaDataAspects;    
 
     /** Behaviours */
     private JavaBehaviour onCreateChildAssociation = new JavaBehaviour(
                                                             this,
                                                             "onCreateChildAssociation",
                                                             NotificationFrequency.FIRST_EVENT);
+    private JavaBehaviour onUpdateProperties = new JavaBehaviour(
+                                                            this,
+                                                            "onUpdateProperties",
+                                                            NotificationFrequency.EVERY_EVENT);
 
     @Override
     public void setApplicationContext(ApplicationContext applicationContext) throws BeansException
@@ -211,6 +272,14 @@ public class RecordServiceImpl implements RecordService,
     {
         this.ownableService = ownableService;
     }
+    
+    /**
+     * @param capabilityService capability service
+     */
+    public void setCapabilityService(CapabilityService capabilityService)
+    {
+        this.capabilityService = capabilityService;
+    }
 
     /**
      * Init method
@@ -222,6 +291,10 @@ public class RecordServiceImpl implements RecordService,
                 TYPE_RECORD_FOLDER,
                 ContentModel.ASSOC_CONTAINS,
                 onCreateChildAssociation);
+        policyComponent.bindClassBehaviour(
+                NodeServicePolicies.OnUpdatePropertiesPolicy.QNAME, 
+                ASPECT_RECORD, 
+                onUpdateProperties);
     }
 
     /**
@@ -239,6 +312,56 @@ public class RecordServiceImpl implements RecordService,
             file(nodeRef);
         }
     }
+    
+    /**
+     * Ensure that the user only updates record properties that they have permission to.
+     * 
+     * @see org.alfresco.repo.node.NodeServicePolicies.OnUpdatePropertiesPolicy#onUpdateProperties(org.alfresco.service.cmr.repository.NodeRef, java.util.Map, java.util.Map)
+     */
+    @Override
+    public void onUpdateProperties(final NodeRef nodeRef, final Map<QName, Serializable> before, final Map<QName, Serializable> after)
+    {
+        onUpdateProperties.disable();
+        try
+        {
+            if (AuthenticationUtil.getFullyAuthenticatedUser() != null &&
+                AuthenticationUtil.isRunAsUserTheSystemUser() == false &&
+                nodeService.exists(nodeRef) == true)
+            {
+                if (isRecord(nodeRef) == true)
+                {
+                    for (QName property : after.keySet())
+                    {
+                        Serializable beforeValue = null;
+                        if (before != null)
+                        {
+                            beforeValue = before.get(property);
+                        }
+                        
+                        Serializable afterValue = null;
+                        if (after != null)
+                        {
+                            afterValue = after.get(property);
+                        }
+    
+                        if (EqualsHelper.nullSafeEquals(beforeValue, afterValue) == false &&
+                            isPropertyEditable(nodeRef, property) == false)
+                        {
+                            // the user can't edit the record property
+                            throw new ModelAccessDeniedException(
+                                "The user " + AuthenticationUtil.getFullyAuthenticatedUser() +
+                                " does not have the permission to edit the record property " + property.toString() +
+                                " on the node " + nodeRef.toString());
+                        }
+                    }
+                }
+            }
+        }
+        finally
+        {
+            onUpdateProperties.enable();
+        }
+    }    
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.record.RecordService#getRecordMetaDataAspects()
@@ -579,4 +702,120 @@ public class RecordServiceImpl implements RecordService,
             }
         });
     }
+
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.record.RecordService#isPropertyEditable(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.namespace.QName)
+     */
+    @Override
+    public boolean isPropertyEditable(NodeRef record, QName property)
+    {
+        ParameterCheck.mandatory("record", record);
+        ParameterCheck.mandatory("property", property);
+        
+        if (isRecord(record) == false)
+        {
+            throw new AlfrescoRuntimeException("Can not check if the property " + property.toString() + " is editable, because node reference is not a record.");
+        }
+        
+        if (logger.isDebugEnabled() == true)
+        {
+            logger.debug("Checking whether property " + property.toString() + " is editable for user " + AuthenticationUtil.getRunAsUser());
+        }
+        
+        boolean result = alwaysEditProperty(property);
+        if (result == false)
+        {
+            boolean allowRecordEdit = false;
+            boolean allowNonRecordEdit = false;
+
+            AccessStatus accessNonRecord = capabilityService.getCapabilityAccessState(record, RMPermissionModel.EDIT_NON_RECORD_METADATA);
+            AccessStatus accessDeclaredRecord = capabilityService.getCapabilityAccessState(record, RMPermissionModel.EDIT_DECLARED_RECORD_METADATA);
+            AccessStatus accessRecord = capabilityService.getCapabilityAccessState(record, RMPermissionModel.EDIT_RECORD_METADATA);
+            
+            if (AccessStatus.ALLOWED.equals(accessNonRecord) == true)
+            {
+                allowNonRecordEdit = true;
+            }
+            
+            if (AccessStatus.ALLOWED.equals(accessRecord) == true ||
+                AccessStatus.ALLOWED.equals(accessDeclaredRecord) == true)
+            {
+                allowRecordEdit = true;
+            }
+            
+            if (allowNonRecordEdit == true && allowRecordEdit == true)
+            {
+                result = true;
+            }
+            else if (allowNonRecordEdit == true && allowRecordEdit == false)
+            {
+                // can only edit non record properties
+                if (isRecordMetadata(property) == false)                    
+                {
+                    result = true;
+                }
+            }
+            else if (allowNonRecordEdit == false && allowRecordEdit == true)
+            {
+                // can only edit record properties
+                if (isRecordMetadata(property) == true)
+                {
+                    result = true;
+                }            
+            }
+            // otherwise we can't edit any properties so just return the empty set
+        }
+        return result;
+    }
+    
+    private boolean isRecordMetadata(QName property)
+    {
+        boolean result = ArrayUtils.contains(RECORD_MODEL_URIS, property.getNamespaceURI());
+        
+        if (result == false && ArrayUtils.contains(NON_RECORD_MODEL_URIS, property.getNamespaceURI()) == false)
+        {
+            PropertyDefinition def = dictionaryService.getProperty(property);
+            if (def != null)
+            {
+                ClassDefinition parent = def.getContainerClass();
+                if (parent != null && parent.isAspect() == true)
+                {
+                    result = getRecordMetaDataAspects().contains(parent.getName());
+                }
+            }
+        }
+        
+        return result;
+    }
+    
+    /**
+     * Determines whether the property should always be allowed to be edited or not.
+     * 
+     * @param property
+     * @return
+     */
+    private boolean alwaysEditProperty(QName property)
+    {
+        return (ArrayUtils.contains(ALWAYS_EDIT_URIS, property.getNamespaceURI()) ||
+                ArrayUtils.contains(ALWAYS_EDIT_PROPERTIES, property) ||
+                isProtectedProperty(property));
+    }
+    
+    /**
+     * Helper method to determine whether a property is protected at a dictionary definition
+     * level.
+     * 
+     * @param property  property qualified name
+     * @return booelan  true if protected, false otherwise
+     */
+    private boolean isProtectedProperty(QName property)
+    {
+        boolean result = false;        
+        PropertyDefinition def = dictionaryService.getProperty(property);
+        if (def != null)
+        {
+            result = def.isProtected();
+        }        
+        return result;
+    }
 }

rm-server/source/java/org/alfresco/module/org_alfresco_module_rm/vital/BroadcastVitalRecordDefinitionAction.java

@@ -26,6 +26,8 @@ import java.util.Map;
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.action.RMActionExecuterAbstractBase;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
+import org.alfresco.module.org_alfresco_module_rm.security.FilePlanAuthenticationService;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.service.cmr.action.Action;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -43,14 +45,29 @@ import org.alfresco.service.namespace.RegexQNamePattern;
  */
 public class BroadcastVitalRecordDefinitionAction extends RMActionExecuterAbstractBase
 {
+    private FilePlanAuthenticationService filePlanAuthenticationService;
+    
+    public void setFilePlanAuthenticationService(FilePlanAuthenticationService filePlanAuthenticationService)
+    {
+        this.filePlanAuthenticationService = filePlanAuthenticationService;
+    }
+    
     /**
      * @see org.alfresco.repo.action.executer.ActionExecuterAbstractBase#executeImpl(org.alfresco.service.cmr.action.Action,
      *      org.alfresco.service.cmr.repository.NodeRef)
      */
     @Override
-    protected void executeImpl(Action action, NodeRef actionedUponNodeRef)
+    protected void executeImpl(Action action, final NodeRef actionedUponNodeRef)
     {
-        this.propagateChangeToChildrenOf(actionedUponNodeRef);
+        filePlanAuthenticationService.runAsRmAdmin(new RunAsWork<Void>()
+        {
+            @Override
+            public Void doWork() throws Exception
+            {
+                propagateChangeToChildrenOf(actionedUponNodeRef);
+                return null;
+            }
+        });        
     }
 
     /**

rm-server/test/java/org/alfresco/module/org_alfresco_module_rm/test/service/RecordServiceImplTest.java

@@ -22,10 +22,12 @@ import java.util.Arrays;
 import java.util.List;
 import java.util.Set;
 
+import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.capability.Capability;
 import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.dod5015.DOD5015Model;
 import org.alfresco.module.org_alfresco_module_rm.record.RecordService;
+import org.alfresco.module.org_alfresco_module_rm.role.Role;
 import org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService;
 import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
 import org.alfresco.repo.content.MimetypeMap;
@@ -37,10 +39,11 @@ import org.alfresco.service.cmr.repository.NodeRef;
 import org.alfresco.service.cmr.security.AccessStatus;
 import org.alfresco.service.cmr.security.PermissionService;
 import org.alfresco.service.namespace.QName;
+import org.alfresco.util.GUID;
 
 /**
  * Records Service Implementation Test
- *
+ * 
  * @author Roy Wetherall
  * @author Tuna Aksoy
  * @since 2.1
@@ -49,7 +52,9 @@ public class RecordServiceImplTest extends BaseRMTestCase
 {
     /** Services */
     protected ActionService dmActionService;
+
     protected PermissionService dmPermissionService;
+
     protected ExtendedSecurityService extendedSecurityService;
 
     /**
@@ -67,7 +72,7 @@ public class RecordServiceImplTest extends BaseRMTestCase
 
     /**
      * This is a user test
-     *
+     * 
      * @see org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase#isUserTest()
      */
     @Override
@@ -78,7 +83,7 @@ public class RecordServiceImplTest extends BaseRMTestCase
 
     /**
      * This is a record test
-     *
+     * 
      * @see org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase#isRecordTest()
      */
     @Override
@@ -116,19 +121,14 @@ public class RecordServiceImplTest extends BaseRMTestCase
 
             /**
              * Helper method for getting a list of record meta data aspects
-             *
+             * 
              * @return Record meta data aspects as list
              */
             private List<QName> getAspectList()
             {
-                QName[] aspects = new QName[]
+                QName[] aspects = new QName[] { DOD5015Model.ASPECT_DIGITAL_PHOTOGRAPH_RECORD,
-                {
+                        DOD5015Model.ASPECT_PDF_RECORD, DOD5015Model.ASPECT_WEB_RECORD,
-                    DOD5015Model.ASPECT_DIGITAL_PHOTOGRAPH_RECORD,
+                        DOD5015Model.ASPECT_SCANNED_RECORD, ASPECT_RECORD_META_DATA };
-                    DOD5015Model.ASPECT_PDF_RECORD,
-                    DOD5015Model.ASPECT_WEB_RECORD,
-                    DOD5015Model.ASPECT_SCANNED_RECORD,
-                    ASPECT_RECORD_META_DATA
-                };
 
                 return Arrays.asList(aspects);
             }
@@ -190,22 +190,21 @@ public class RecordServiceImplTest extends BaseRMTestCase
     }
 
     /**
-     * @see RecordService#createRecord(org.alfresco.service.cmr.repository.NodeRef, org.alfresco.service.cmr.repository.NodeRef)
+     * @see RecordService#createRecord(org.alfresco.service.cmr.repository.NodeRef,
+     *      org.alfresco.service.cmr.repository.NodeRef)
      */
     public void testCreateRecord() throws Exception
     {
         // show that users without WRITE can not create a record from a document
-        doTestInTransaction(new FailureTest
+        doTestInTransaction(new FailureTest(
-        (
+                "Can not create a record from a document if you do not have WRITE permissions.",
-            "Can not create a record from a document if you do not have WRITE permissions.",
+                AccessDeniedException.class)
-            AccessDeniedException.class
-        )
         {
             public void run() throws Exception
             {
                 recordService.createRecord(filePlan, dmDocument);
             }
-         }, dmConsumer);
+        }, dmConsumer);
 
         // create record from document
         doTestInTransaction(new Test<Void>()
@@ -218,107 +217,108 @@ public class RecordServiceImplTest extends BaseRMTestCase
                 assertFalse(recordService.isRecord(dmDocument));
                 assertFalse(extendedSecurityService.hasExtendedSecurity(dmDocument));
 
-                checkPermissions(READ_RECORDS,
+                checkPermissions(READ_RECORDS, AccessStatus.DENIED, // file plan
-                                 AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                                 AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                                 AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                                 AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED); // doc/record
-                                 AccessStatus.DENIED);  // doc/record
 
-                assertEquals(AccessStatus.DENIED,
+                assertEquals(AccessStatus.DENIED, dmPermissionService.hasPermission(filePlan,
-                        dmPermissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
+                        RMPermissionModel.VIEW_RECORDS));
 
-                checkPermissions(FILING,
+                checkPermissions(FILING, AccessStatus.DENIED, // file plan
-                        AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                        AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED); // doc/record
-                        AccessStatus.DENIED);  // doc/record
 
                 recordService.createRecord(filePlan, dmDocument);
 
-                checkPermissions(READ_RECORDS,
+                checkPermissions(READ_RECORDS, AccessStatus.ALLOWED, // file
-                        AccessStatus.ALLOWED,   // file plan
+                                                                     // plan
-                        AccessStatus.ALLOWED,   // unfiled container
+                        AccessStatus.ALLOWED, // unfiled container
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.ALLOWED);  // doc/record
+                        AccessStatus.ALLOWED); // doc/record
 
-                assertEquals(AccessStatus.ALLOWED,
+                assertEquals(AccessStatus.ALLOWED, dmPermissionService.hasPermission(filePlan,
-                        dmPermissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
+                        RMPermissionModel.VIEW_RECORDS));
 
-                checkPermissions(FILING,
+                checkPermissions(FILING, AccessStatus.DENIED, // file plan
-                        AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                        AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.ALLOWED); // doc/record
-                        AccessStatus.ALLOWED);  // doc/record
 
                 assertTrue(recordService.isRecord(dmDocument));
                 assertTrue(extendedSecurityService.hasExtendedSecurity(dmDocument));
                 assertFalse(recordService.isFiled(dmDocument));
 
-                // show that the record has meta-data about it's original location
+                // show that the record has meta-data about it's original
+                // location
                 assertTrue(nodeService.hasAspect(dmDocument, ASPECT_RECORD_ORIGINATING_DETAILS));
                 assertEquals(originalLocation, nodeService.getProperty(dmDocument, PROP_RECORD_ORIGINATING_LOCATION));
                 assertFalse(originalLocation == nodeService.getPrimaryParent(dmDocument).getParentRef());
 
                 // show that the record is linked to it's original location
                 assertEquals(2, nodeService.getParentAssocs(dmDocument).size());
-                
+
                 // ****
                 // Capability Tests
                 // ****
 
-                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
+                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan,
-                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan, RMPermissionModel.EDIT_RECORD_METADATA));
+                        RMPermissionModel.VIEW_RECORDS));
-                
+                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan,
+                        RMPermissionModel.EDIT_RECORD_METADATA));
+
                 Capability filling = capabilityService.getCapability("FileRecords");
                 assertEquals(AccessStatus.DENIED, filling.hasPermission(dmDocument));
-                
+
                 Capability editRecordMetadata = capabilityService.getCapability("EditRecordMetadata");
                 assertEquals(AccessStatus.ALLOWED, editRecordMetadata.hasPermission(dmDocument));
-                
+
                 Capability updateProperties = capabilityService.getCapability("UpdateProperties");
                 assertEquals(AccessStatus.ALLOWED, updateProperties.hasPermission(dmDocument));
 
                 return null;
             }
         }, dmCollaborator);
-        
+
-        // check the consumer's permissions are correct for the newly created document
+        // check the consumer's permissions are correct for the newly created
+        // document
         doTestInTransaction(new Test<Void>()
         {
             @Override
             public Void run()
             {
-                checkPermissions(READ_RECORDS,
+                checkPermissions(READ_RECORDS, AccessStatus.ALLOWED, // file
-                        AccessStatus.ALLOWED,   // file plan
+                                                                     // plan
-                        AccessStatus.ALLOWED,   // unfiled container
+                        AccessStatus.ALLOWED, // unfiled container
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.ALLOWED);  // doc/record
+                        AccessStatus.ALLOWED); // doc/record
+
+                checkPermissions(FILING, AccessStatus.DENIED, // file plan
+                        AccessStatus.DENIED, // unfiled container
+                        AccessStatus.DENIED, // record category
+                        AccessStatus.DENIED, // record folder
+                        AccessStatus.DENIED); // doc/record
+
+                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan,
+                        RMPermissionModel.VIEW_RECORDS));
+                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan,
+                        RMPermissionModel.EDIT_RECORD_METADATA));
 
-                checkPermissions(FILING,
-                        AccessStatus.DENIED,   // file plan
-                        AccessStatus.DENIED,   // unfiled container
-                        AccessStatus.DENIED,   // record category
-                        AccessStatus.DENIED,   // record folder
-                        AccessStatus.DENIED);  // doc/record
-                
-                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
-                assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan, RMPermissionModel.EDIT_RECORD_METADATA));
-                
                 Capability filling = capabilityService.getCapability("FileRecords");
                 assertEquals(AccessStatus.DENIED, filling.hasPermission(dmDocument));
-                
+
                 Capability editRecordMetadata = capabilityService.getCapability("EditRecordMetadata");
                 assertEquals(AccessStatus.DENIED, editRecordMetadata.hasPermission(dmDocument));
-                
+
                 Capability updateProperties = capabilityService.getCapability("UpdateProperties");
                 assertEquals(AccessStatus.DENIED, updateProperties.hasPermission(dmDocument));
 
-            
                 return null;
             }
         }, dmConsumer);
@@ -327,17 +327,15 @@ public class RecordServiceImplTest extends BaseRMTestCase
     public void testCreateRecordNoLink() throws Exception
     {
         // show that users without WRITE can not create a record from a document
-        doTestInTransaction(new FailureTest
+        doTestInTransaction(new FailureTest(
-        (
+                "Can not create a record from a document if you do not have WRITE permissions.",
-            "Can not create a record from a document if you do not have WRITE permissions.",
+                AccessDeniedException.class)
-            AccessDeniedException.class
-        )
         {
             public void run() throws Exception
             {
                 recordService.createRecord(filePlan, dmDocument, false);
             }
-         }, dmConsumer);
+        }, dmConsumer);
 
         // create record from document
         final NodeRef originalLocation = doTestInTransaction(new Test<NodeRef>()
@@ -347,44 +345,40 @@ public class RecordServiceImplTest extends BaseRMTestCase
             {
                 NodeRef originalLocation = nodeService.getPrimaryParent(dmDocument).getParentRef();
 
-                assertFalse(recordService.isRecord(dmDocument));
+                //assertFalse(recordService.isRecord(dmDocument));
-                assertFalse(extendedSecurityService.hasExtendedSecurity(dmDocument));
+                //assertFalse(extendedSecurityService.hasExtendedSecurity(dmDocument));
 
-                checkPermissions(READ_RECORDS,
+                checkPermissions(READ_RECORDS, AccessStatus.DENIED, // file plan
-                                 AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                                 AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                                 AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                                 AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED); // doc/record
-                                 AccessStatus.DENIED);  // doc/record
 
-                assertEquals(AccessStatus.DENIED,
+                assertEquals(AccessStatus.DENIED, dmPermissionService.hasPermission(filePlan,
-                        dmPermissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
+                        RMPermissionModel.VIEW_RECORDS));
 
-                checkPermissions(FILING,
+                checkPermissions(FILING, AccessStatus.DENIED, // file plan
-                        AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                        AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED); // doc/record
-                        AccessStatus.DENIED);  // doc/record
 
                 recordService.createRecord(filePlan, dmDocument, false);
 
-                checkPermissions(READ_RECORDS,
+                checkPermissions(READ_RECORDS, AccessStatus.DENIED, // file plan
-                        AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                        AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED); // doc/record
-                        AccessStatus.DENIED);  // doc/record
 
-                assertEquals(AccessStatus.DENIED,
+                assertEquals(AccessStatus.DENIED, dmPermissionService.hasPermission(filePlan,
-                        dmPermissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
+                        RMPermissionModel.VIEW_RECORDS));
 
-                checkPermissions(FILING,
+                checkPermissions(FILING, AccessStatus.DENIED, // file plan
-                        AccessStatus.DENIED,   // file plan
+                        AccessStatus.DENIED, // unfiled container
-                        AccessStatus.DENIED,   // unfiled container
+                        AccessStatus.DENIED, // record category
-                        AccessStatus.DENIED,   // record category
+                        AccessStatus.DENIED, // record folder
-                        AccessStatus.DENIED,   // record folder
+                        AccessStatus.DENIED); // doc/record
-                        AccessStatus.DENIED);  // doc/record
 
                 return originalLocation;
             }
@@ -399,7 +393,8 @@ public class RecordServiceImplTest extends BaseRMTestCase
                 assertFalse(extendedSecurityService.hasExtendedSecurity(dmDocument));
                 assertFalse(recordService.isFiled(dmDocument));
 
-                // show that the record has meta-data about it's original location
+                // show that the record has meta-data about it's original
+                // location
                 assertTrue(nodeService.hasAspect(dmDocument, ASPECT_RECORD_ORIGINATING_DETAILS));
                 assertEquals(originalLocation, nodeService.getProperty(dmDocument, PROP_RECORD_ORIGINATING_LOCATION));
                 assertFalse(originalLocation == nodeService.getPrimaryParent(dmDocument).getParentRef());
@@ -419,7 +414,7 @@ public class RecordServiceImplTest extends BaseRMTestCase
             @Override
             public NodeRef run()
             {
-                NodeRef record = fileFolderService.create(rmFolder, "test101.txt" , TYPE_CONTENT).getNodeRef();
+                NodeRef record = fileFolderService.create(rmFolder, "test101.txt", TYPE_CONTENT).getNodeRef();
 
                 ContentWriter writer = contentService.getWriter(record, PROP_CONTENT, true);
                 writer.setEncoding("UTF-8");
@@ -495,21 +490,245 @@ public class RecordServiceImplTest extends BaseRMTestCase
         }, AuthenticationUtil.getSystemUserName());
     }
 
-    private void checkPermissions(String permission, AccessStatus filePlanExpected,
+    private void checkPermissions(String permission, AccessStatus filePlanExpected, AccessStatus unfiledExpected,
-                                                     AccessStatus unfiledExpected,
+            AccessStatus recordCatExpected, AccessStatus recordFolderExpected, AccessStatus recordExpected)
-                                                     AccessStatus recordCatExpected,
-                                                     AccessStatus recordFolderExpected,
-                                                     AccessStatus recordExpected)
     {
-        assertEquals(filePlanExpected,
+        assertEquals(filePlanExpected, dmPermissionService.hasPermission(filePlan, permission));
-                    dmPermissionService.hasPermission(filePlan, permission));
+        assertEquals(unfiledExpected, dmPermissionService.hasPermission(unfiledContainer, permission));
-        assertEquals(unfiledExpected,
+        assertEquals(recordCatExpected, dmPermissionService.hasPermission(rmContainer, permission));
-                    dmPermissionService.hasPermission(unfiledContainer, permission));
+        assertEquals(recordFolderExpected, dmPermissionService.hasPermission(rmFolder, permission));
-        assertEquals(recordCatExpected,
+        assertEquals(recordExpected, dmPermissionService.hasPermission(dmDocument, permission));
-                    dmPermissionService.hasPermission(rmContainer, permission));
+    }
-        assertEquals(recordFolderExpected,
+
-                    dmPermissionService.hasPermission(rmFolder, permission));
+    private String createUserWithCapabilties(final String... capabiltyNames)
-        assertEquals(recordExpected,
+    {
-                dmPermissionService.hasPermission(dmDocument, permission));
+        return doTestInTransaction(new Test<String>()
+        {
+            @Override
+            public String run() throws Exception
+            {
+                Role role = utils.createRole(filePlan, GUID.generate(), capabiltyNames);
+
+                String userName = GUID.generate();
+                createPerson(userName);
+                filePlanRoleService.assignRoleToAuthority(filePlan, role.getName(), userName);
+
+                return userName;
+            }
+        }, AuthenticationUtil.getSystemUserName());
+
+    }
+
+    /**
+     * Test {@link RecordService#isPropertyEditable(NodeRef, QName)}
+     */
+    public void testIsPropertyEditable() throws Exception
+    {
+        final String nonRecordMetadata = createUserWithCapabilties(
+                RMPermissionModel.VIEW_RECORDS,
+                RMPermissionModel.EDIT_NON_RECORD_METADATA);
+        final String recordMetadata = createUserWithCapabilties(
+                RMPermissionModel.VIEW_RECORDS,
+                RMPermissionModel.EDIT_RECORD_METADATA);
+        final String declaredRecordMetadata = createUserWithCapabilties(
+                RMPermissionModel.VIEW_RECORDS,
+                RMPermissionModel.EDIT_DECLARED_RECORD_METADATA);
+
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                filePlanPermissionService.setPermission(rmFolder, rmUserName, RMPermissionModel.FILING);
+                filePlanPermissionService.setPermission(rmFolder, nonRecordMetadata, RMPermissionModel.FILING);
+                filePlanPermissionService.setPermission(rmFolder, recordMetadata, RMPermissionModel.FILING);
+                filePlanPermissionService.setPermission(rmFolder, declaredRecordMetadata, RMPermissionModel.FILING);
+            }
+        });
+
+        // test rmadmin
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                assertTrue(recordService.isPropertyEditable(recordOne, PROP_ORIGINATING_ORGANIZATION));
+                assertTrue(recordService.isPropertyEditable(recordOne, PROP_DESCRIPTION));
+                assertTrue(recordService.isPropertyEditable(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_DESCRIPTION));
+            }
+        });
+
+        // test normal user        
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                assertFalse(recordService.isPropertyEditable(recordOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordOne, PROP_DESCRIPTION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_DESCRIPTION));
+            }
+        }, rmUserName);
+
+        // test undeclared record with edit non-record metadata capability
+        // test declared record with edit non-record metadata capability
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                assertFalse(recordService.isPropertyEditable(recordOne, PROP_ORIGINATING_ORGANIZATION));
+                assertTrue(recordService.isPropertyEditable(recordOne, PROP_DESCRIPTION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_DESCRIPTION));                
+            }
+        }, nonRecordMetadata);
+
+        // test undeclared record with edit record metadata capability
+        // test declared record with edit record metadata capability
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                assertTrue(recordService.isPropertyEditable(recordOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordOne, PROP_DESCRIPTION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_DESCRIPTION));
+            }
+        }, recordMetadata);
+
+        // test undeclared record with edit declared record metadata capability
+        // test declared record with edit declared record metadata capability
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                assertFalse(recordService.isPropertyEditable(recordOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordOne, PROP_DESCRIPTION));
+                assertTrue(recordService.isPropertyEditable(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION));
+                assertFalse(recordService.isPropertyEditable(recordDeclaredOne, PROP_DESCRIPTION));
+            }
+        }, declaredRecordMetadata);
+    }
+    
+    public void testRecordPropertiesUpdate() throws Exception
+    {
+        final String nonRecordMetadata = createUserWithCapabilties(
+                RMPermissionModel.VIEW_RECORDS,
+                RMPermissionModel.EDIT_NON_RECORD_METADATA);
+        final String recordMetadata = createUserWithCapabilties(
+                RMPermissionModel.VIEW_RECORDS,
+                RMPermissionModel.EDIT_RECORD_METADATA);
+        final String declaredRecordMetadata = createUserWithCapabilties(
+                RMPermissionModel.VIEW_RECORDS,
+                RMPermissionModel.EDIT_DECLARED_RECORD_METADATA);
+
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                filePlanPermissionService.setPermission(rmFolder, rmUserName, RMPermissionModel.FILING);
+                filePlanPermissionService.setPermission(rmFolder, nonRecordMetadata, RMPermissionModel.FILING);
+                filePlanPermissionService.setPermission(rmFolder, recordMetadata, RMPermissionModel.FILING);
+                filePlanPermissionService.setPermission(rmFolder, declaredRecordMetadata, RMPermissionModel.FILING);
+            }
+        });
+        
+        // test rmadmin
+        canEditProperty(recordOne, ContentModel.PROP_DESCRIPTION, rmAdminName);
+        canEditProperty(recordOne, PROP_ORIGINATING_ORGANIZATION, rmAdminName);
+        cantEditProperty(recordDeclaredOne, ContentModel.PROP_DESCRIPTION, rmAdminName);
+        canEditProperty(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION, rmAdminName);
+        
+        // test normal user
+        cantEditProperty(recordOne, ContentModel.PROP_DESCRIPTION, rmUserName);
+        cantEditProperty(recordOne, PROP_ORIGINATING_ORGANIZATION, rmUserName);
+        cantEditProperty(recordDeclaredOne, ContentModel.PROP_DESCRIPTION, rmUserName);
+        cantEditProperty(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION, rmUserName);
+        
+        // test undeclared record with edit non-record metadata capability
+        canEditProperty(recordOne, ContentModel.PROP_DESCRIPTION, nonRecordMetadata);
+        cantEditProperty(recordOne, PROP_ORIGINATING_ORGANIZATION, nonRecordMetadata);
+        // test declared record with edit non-record metadata capability
+        cantEditProperty(recordDeclaredOne, ContentModel.PROP_DESCRIPTION, nonRecordMetadata);
+        cantEditProperty(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION, nonRecordMetadata);
+        
+        // test undeclared record with edit record metadata capability
+        cantEditProperty(recordOne, ContentModel.PROP_DESCRIPTION, recordMetadata);
+        canEditProperty(recordOne, PROP_ORIGINATING_ORGANIZATION, recordMetadata);
+        // test declared record with edit record metadata capability
+        cantEditProperty(recordDeclaredOne, ContentModel.PROP_DESCRIPTION, recordMetadata);
+        cantEditProperty(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION, recordMetadata);
+        
+        // test undeclared record with edit declared record metadata capability
+        cantEditProperty(recordOne, ContentModel.PROP_DESCRIPTION, declaredRecordMetadata);
+        cantEditProperty(recordOne, PROP_ORIGINATING_ORGANIZATION, declaredRecordMetadata);
+        // test declared record with edit declared record metadata capability
+        cantEditProperty(recordDeclaredOne, ContentModel.PROP_DESCRIPTION, declaredRecordMetadata);
+        canEditProperty(recordDeclaredOne, PROP_ORIGINATING_ORGANIZATION, declaredRecordMetadata);
+        
+    }
+    
+    public abstract class CommitPropertyFailTest extends Test<Void>
+    {
+        @Override
+        public Void run() throws Exception
+        {
+            // TODO Auto-generated method stub
+            return null;
+        }
+        
+        @Override
+        public void test(Void result) throws Exception
+        {
+            // TODO Auto-generated method stub
+            super.test(result);
+        }
+        
+    }
+    
+    private void cantEditProperty(final NodeRef nodeRef, final QName property, String user) throws Exception
+    {
+        boolean failure = false;
+        try
+        {
+            doTestInTransaction(new VoidTest()
+            {
+                @Override
+                public void runImpl() throws Exception
+                {
+                    nodeService.setProperty(nodeRef, property, GUID.generate());
+                }
+            
+            }, user);
+        }
+        catch (Throwable exception)
+        {
+            // expected
+            failure = true;
+        }
+        
+        // assert fail not failure
+        if (failure == false)
+        {
+            fail("Property should not have been editable.");
+        }
+    }
+    
+    private void canEditProperty(final NodeRef nodeRef, final QName property, String user) throws Exception
+    {
+        doTestInTransaction(new VoidTest()
+        {
+            @Override
+            public void runImpl() throws Exception
+            {
+                nodeService.setProperty(nodeRef, property, GUID.generate());
+            }
+        }, user);
     }
 }

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

@@ -454,8 +454,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
                 unfiledContainer = filePlanService.getUnfiledContainer(filePlan);
                 assertNotNull(unfiledContainer);
             }
-        },
+        }, AuthenticationUtil.getSystemUserName());
-        AuthenticationUtil.getAdminUserName());
     }
 
     /**
@@ -566,6 +565,7 @@ public abstract class BaseRMTestCase extends RetryingTransactionHelperTestCase
             {
                 filePlanPermissionService.setPermission(filePlan, user, FILING);
                 filePlanPermissionService.setPermission(rmContainer, user, FILING);
+                filePlanPermissionService.setPermission(rmFolder, user, FILING);
                 filePlanPermissionService.setPermission(unfiledContainer, user, FILING);
             }
         }

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

@@ -7,16 +7,23 @@ import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Date;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 
+import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.ContentModel;
 import org.alfresco.module.org_alfresco_module_rm.action.RecordsManagementActionService;
 import org.alfresco.module.org_alfresco_module_rm.action.impl.FreezeAction;
+import org.alfresco.module.org_alfresco_module_rm.capability.Capability;
+import org.alfresco.module.org_alfresco_module_rm.capability.CapabilityService;
 import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionSchedule;
 import org.alfresco.module.org_alfresco_module_rm.disposition.DispositionService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.model.security.ModelSecurityService;
+import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
+import org.alfresco.module.org_alfresco_module_rm.role.Role;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
@@ -38,6 +45,8 @@ public class CommonRMTestUtils implements RecordsManagementModel
 	private ContentService contentService;
 	private RecordsManagementActionService actionService;
 	private ModelSecurityService modelSecurityService;
+	private FilePlanRoleService filePlanRoleService;
+	private CapabilityService capabilityService;
 	
     /** test values */
     public static final String DEFAULT_DISPOSITION_AUTHORITY = "disposition authority";
@@ -54,6 +63,8 @@ public class CommonRMTestUtils implements RecordsManagementModel
 		contentService = (ContentService)applicationContext.getBean("ContentService");
 		actionService = (RecordsManagementActionService)applicationContext.getBean("RecordsManagementActionService");
 		modelSecurityService = (ModelSecurityService)applicationContext.getBean("ModelSecurityService");
+		filePlanRoleService = (FilePlanRoleService)applicationContext.getBean("FilePlanRoleService");
+		capabilityService = (CapabilityService)applicationContext.getBean("CapabilityService");
 	}
 	
     /**
@@ -231,4 +242,20 @@ public class CommonRMTestUtils implements RecordsManagementModel
             
         }, AuthenticationUtil.getSystemUserName());
     }
+    
+    public Role createRole(NodeRef filePlan, String roleName, String ... capabilityNames)
+    {
+        Set<Capability> capabilities = new HashSet<Capability>(capabilityNames.length);
+        for (String name : capabilityNames)
+        {
+            Capability capability = capabilityService.getCapability(name);
+            if (capability == null)
+            {
+                throw new AlfrescoRuntimeException("capability " + name + " not found.");
+            }
+            capabilities.add(capability);
+        }
+        
+        return filePlanRoleService.createRole(filePlan, roleName, roleName, capabilities);
+    }
 }