RM-3074: Initial refactoring of the extended security service implementation

rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml

@@ -573,7 +573,9 @@
           parent="baseService">
         <property name="nodeService" ref="nodeService"/>
         <property name="filePlanService" ref="filePlanService" />
-        <property name="filePlanRoleService" ref="filePlanRoleService" />
+        <property name="filePlanRoleService" ref="filePlanRoleService" />        
+        <property name="authorityService" ref="authorityService"/>        
+        <property name="permissionService" ref="permissionService"/>
     </bean>
 
     <bean id="ExtendedSecurityService" class="org.springframework.aop.framework.ProxyFactoryBean">

rm-community/rm-community-repo/source/compatibility/org/alfresco/module/org_alfresco_module_rm/security/ExtendedReaderDynamicAuthority.java

@@ -41,6 +41,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
  * @author Roy Wetherall
  * @since 2.1
  */
+@Deprecated
 public class ExtendedReaderDynamicAuthority extends ExtendedSecurityBaseDynamicAuthority
 {
     /** Extended reader role */

rm-community/rm-community-repo/source/compatibility/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityBaseDynamicAuthority.java

@@ -48,6 +48,7 @@ import org.springframework.context.ApplicationContextAware;
  * @author Roy Wetherall
  * @since 2.1
  */
+@Deprecated
 public abstract class ExtendedSecurityBaseDynamicAuthority implements DynamicAuthority,
                                                                       RecordsManagementModel,
                                                                       ApplicationContextAware

rm-community/rm-community-repo/source/compatibility/org/alfresco/module/org_alfresco_module_rm/security/ExtendedWriterDynamicAuthority.java

@@ -42,6 +42,7 @@ import org.alfresco.service.cmr.repository.NodeRef;
  * @author Roy Wetherall
  * @since 2.1
  */
+@Deprecated
 public class ExtendedWriterDynamicAuthority extends ExtendedSecurityBaseDynamicAuthority
 {
     /** Extended writer role */

rm-community/rm-community-repo/source/compatibility/org/alfresco/module/org_alfresco_module_rm/security/FilePlanAuthenticationServiceImpl.java

@@ -34,6 +34,7 @@ import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
  * @author Roy Wetherall
  * @since 2.1
  */
+@Deprecated
 public class FilePlanAuthenticationServiceImpl implements FilePlanAuthenticationService
 {
     /** Default rm admin user values */

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/fileplan/FilePlanServiceImpl.java

@@ -360,8 +360,6 @@ public class FilePlanServiceImpl extends ServiceBaseImpl
         // set inheritance to false
         getPermissionService().setInheritParentPermissions(container, false);
         getPermissionService().setPermission(container, allRoles, RMPermissionModel.READ_RECORDS, true);
-        getPermissionService().setPermission(container, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS, true);
-        getPermissionService().setPermission(container, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING, true);
         
         // prevent inheritance of rules
         nodeService.addAspect(container, RuleModel.ASPECT_IGNORE_INHERITED_RULES, null);

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/model/RecordsManagementModel.java

@@ -253,9 +253,10 @@ public interface RecordsManagementModel extends RecordsManagementCustomModel
     QName PROP_LOADED_DATA_SET_IDS = QName.createQName(RM_URI, "loadedDataSetIds");
 
     // Extended security aspect
-    QName ASPECT_EXTENDED_SECURITY = QName.createQName(RM_URI, "extendedSecurity");
+    // @deprecated as of 2.5, because of performance issues
-    QName PROP_READERS = QName.createQName(RM_URI, "readers");
+    @Deprecated QName ASPECT_EXTENDED_SECURITY = QName.createQName(RM_URI, "extendedSecurity");
-    QName PROP_WRITERS = QName.createQName(RM_URI, "writers");
+    @Deprecated QName PROP_READERS = QName.createQName(RM_URI, "readers");
+    @Deprecated QName PROP_WRITERS = QName.createQName(RM_URI, "writers");
 
     // Originating details of a record
     QName ASPECT_RECORD_ORIGINATING_DETAILS = QName.createQName(RM_URI, "recordOriginatingDetails");

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/patch/v21/RMv21InPlacePatch.java

@@ -164,10 +164,6 @@ public class RMv21InPlacePatch extends RMv21PatchComponent
                 ruleService.disableRules();
                 try
                 {
-                    // set permissions
-                    filePlanPermissionService.setPermission(filePlan, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS);
-                    filePlanPermissionService.setPermission(filePlan, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING);
-
                     // create fileplan containers
                     filePlanService.createHoldContainer(filePlan);
                     filePlanService.createTransferContainer(filePlan);

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

@@ -609,7 +609,7 @@ public class RecordServiceImpl extends BaseBehaviourBean
             nodeService.getType(childAssocRef.getParentRef()).equals(ContentModel.TYPE_FOLDER))
         {
             // ..then remove the extended readers and writers up the tree for this remaining node
-            extendedSecurityService.removeExtendedSecurity(childAssocRef.getChildRef(), extendedSecurityService.getExtendedReaders(childAssocRef.getChildRef()), extendedSecurityService.getExtendedWriters(childAssocRef.getChildRef()), true);
+            extendedSecurityService.removeAllExtendedSecurity(childAssocRef.getChildRef());
         }
     }
 

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/role/FilePlanRoleServiceImpl.java

@@ -195,8 +195,6 @@ public class FilePlanRoleServiceImpl implements FilePlanRoleService,
                     // Set the permissions
                     permissionService.setInheritParentPermissions(filePlan, false);
                     permissionService.setPermission(filePlan, allRoles, RMPermissionModel.READ_RECORDS, true);
-                    permissionService.setPermission(filePlan, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS, true);
-                    permissionService.setPermission(filePlan, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING, true);
 
                     // Create the transfer and hold containers
                     systemContainers.add(filePlanService.createHoldContainer(filePlan));

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityService.java

@@ -41,11 +41,13 @@ import org.alfresco.service.cmr.repository.NodeRef;
 @AlfrescoPublicApi
 public interface ExtendedSecurityService
 {
+    static final String IPR_GROUP_PREFIX = "IPR_";
+    
 	/**
 	 * Indicates whether a node has extended security.
 	 *
 	 * @param nodeRef      node reference
-	 * @return boolean     true if the node has extedned security, false otherwise
+	 * @return boolean     true if the node has extended security, false otherwise
 	 */
     boolean hasExtendedSecurity(NodeRef nodeRef);
 
@@ -67,13 +69,25 @@ public interface ExtendedSecurityService
 
     /**
      * Add extended security for the specified authorities to a node.
+     * 
+     * As of, 2.5 this method no longer applies the extended security to parents.
      *
      * @param nodeRef   node reference
      * @param readers   set of authorities to add extended read permissions
      * @param writers   set of authorities to add extended write permissions
+     *
      */
+    // TODO rename to setExtendedSecurity to reflect that this doesn't update the extended security any more 
     void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers);
 
+    /**
+     * Remove all extended readers and writers from the given node reference.
+     *
+     * @param nodeRef   node reference
+     */
+    // TODO rename to removeExtendedSecurity 
+    void removeAllExtendedSecurity(NodeRef nodeRef);
+    
     /**
      * Add extended security for the specified authorities to a node.
      * <p>
@@ -84,17 +98,27 @@ public interface ExtendedSecurityService
      * @param readers   set of authorities to add extended read permissions
      * @param writers   set of authorities to add extended write permissions
      * @param applyToParents true if extended security applied to parents (read only) false otherwise.
+     * 
+     * @deprecated as of 2.5, because extended security is no longer applied to parents.  Note that calling this method will
+     * only apply the exetended securiyt to the node and the applyToParents parameter value will be ignored.
+     * 
+     * @see #addExtendedSecurity(NodeRef, Set, Set)
      */
-    void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents);
+    @Deprecated void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents);
-
+    
     /**
      * Remove the extended security for the specified authorities from a node.
      *
      * @param nodeRef   node reference
      * @param readers   set of authorities to remove as extended readers
      * @param writers   set of authorities to remove as extended writers
+     * 
+     * @deprecated as of 2.5, because partial removal of readers and writers from node or parents is no longer supported.
+     * Note that calling this method will now remove all extended security from the node and never applied to parents.
+     * 
+     * @see #removeAllExtendedSecurity(NodeRef)
      */
-    void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers);
+    @Deprecated void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers);
 
     /**
      * Remove the extended security for the specified authorities from a node.
@@ -108,21 +132,24 @@ public interface ExtendedSecurityService
      * @param writers           set of authorities to remove as extedned writers
      * @param applyToParents    true if removal of extended security is applied to parent hierarchy (read only), false
      *                          otherwise
+     * 
+     * @deprecated as of 2.5, because partial removal of readers and writers from node or parents is no longer supported.
+     * Note that calling this method will now remove all extended security from the node and never applied to parents.
+     * 
+     * @see #removeAllExtendedSecurity(NodeRef)
      */
-    void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents);
+    @Deprecated void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents);    
-
-    /**
-     * Remove all extended readers and writers from the given node reference.
-     *
-     * @param nodeRef   node reference
-     */
-    void removeAllExtendedSecurity(NodeRef nodeRef);
 
     /**
      * Remove all extended readers and writers from the given node reference.
      *
      * @param nodeRef           node reference
      * @param applyToParents    if true then apply removal to parent hierarchy (read only) false otherwise.
+     * 
+     * @deprecated as of 2.5, because partial removal of readers and writers from node or parents is no longer supported.
+     * Note that calling this method will now remove all extended security from the node and never applied to parents.
+     * 
+     * @see #removeAllExtendedSecurity(NodeRef)
      */
-    void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents);
+    @Deprecated void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents);
 }

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java

@@ -27,23 +27,28 @@
 
 package org.alfresco.module.org_alfresco_module_rm.security;
 
-import java.io.Serializable;
+import java.util.Collections;
-import java.util.HashMap;
 import java.util.List;
-import java.util.Map;
 import java.util.Set;
 
+import org.alfresco.error.AlfrescoRuntimeException;
 import org.alfresco.model.RenditionModel;
+import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.fileplan.FilePlanService;
 import org.alfresco.module.org_alfresco_module_rm.model.RecordsManagementModel;
 import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
 import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.NodeRef;
+import org.alfresco.service.cmr.security.AccessPermission;
+import org.alfresco.service.cmr.security.AuthorityService;
+import org.alfresco.service.cmr.security.AuthorityType;
 import org.alfresco.service.cmr.security.PermissionService;
-import org.alfresco.service.namespace.QName;
 import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.util.ParameterCheck;
+import org.springframework.extensions.webscripts.ui.common.StringUtils;
+
+import com.google.gdata.util.common.base.Pair;
 
 /**
  * Extended security service implementation.
@@ -55,11 +60,20 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
                                          implements ExtendedSecurityService,
                                                     RecordsManagementModel
 {
+    private static final String READER_GROUP_PREFIX = ExtendedSecurityService.IPR_GROUP_PREFIX + "R_";
+    private static final String WRITER_GROUP_PREFIX = ExtendedSecurityService.IPR_GROUP_PREFIX + "W_";
+    
     /** File plan service */
     private FilePlanService filePlanService;
 
     /** File plan role service */
     private FilePlanRoleService filePlanRoleService;
+    
+    /** authority service */
+    private AuthorityService authorityService;
+    
+    /** permission service */
+    private PermissionService permissionService;
 
     /**
      * @param filePlanService   file plan service
@@ -76,13 +90,29 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     {
         this.filePlanRoleService = filePlanRoleService;
     }
+    
+    /**
+     * @param authorityService  authority service
+     */
+    public void setAuthorityService(AuthorityService authorityService)
+    {
+        this.authorityService = authorityService;
+    }
+    
+    /**
+     * @param permissionService permission service
+     */
+    public void setPermissionService(PermissionService permissionService)
+    {
+        this.permissionService = permissionService;
+    }
 
 	/**
      * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#hasExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef)
      */
     public boolean hasExtendedSecurity(NodeRef nodeRef)
     {
-        return nodeService.hasAspect(nodeRef, ASPECT_EXTENDED_SECURITY);
+        return (getIPRGroups(nodeRef) != null);
     }
 
     /**
@@ -93,13 +123,18 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     public Set<String> getExtendedReaders(NodeRef nodeRef)
     {
         Set<String> result = null;
-
+        
-        Map<String, Integer> readerMap = (Map<String, Integer>)nodeService.getProperty(nodeRef, PROP_READERS);
+        Pair<String, String> iprGroups = getIPRGroups(nodeRef);
-        if (readerMap != null)
+        if (iprGroups != null)
         {
-            result = readerMap.keySet();
+            result = authorityService.getContainedAuthorities(null, iprGroups.first, true);
+            result.remove(iprGroups.second);
         }
-
+        else
+        {
+            result = Collections.EMPTY_SET;
+        }
+        
         return result;
     }
 
@@ -111,14 +146,19 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     public Set<String> getExtendedWriters(NodeRef nodeRef)
     {
         Set<String> result = null;
-
+        
-        Map<String, Integer> map = (Map<String, Integer>)nodeService.getProperty(nodeRef, PROP_WRITERS);
+        Pair<String, String> iprGroups = getIPRGroups(nodeRef);
-        if (map != null)
+        if (iprGroups != null)
         {
-            result = map.keySet();
+            result = authorityService.getContainedAuthorities(null, iprGroups.second, true);
         }
-
+        else
+        {
+            result = Collections.EMPTY_SET;
+        }
+        
         return result;
+        
     }
 
     /**
@@ -126,25 +166,15 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
      */
     @Override
     public void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers)
-    {
-        addExtendedSecurity(nodeRef, readers, writers, true);
-    }
-
-    /**
-     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean)
-     */
-    @Override
-    public void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents)
     {
         ParameterCheck.mandatory("nodeRef", nodeRef);
-        ParameterCheck.mandatory("applyToParents", applyToParents);
 
         if (nodeRef != null)
         {
-            addExtendedSecurityImpl(nodeRef, readers, writers, applyToParents);
+            addExtendedSecurityImpl(nodeRef, readers, writers);
 
             // add to the extended security roles
-	        addExtendedSecurityRoles(nodeRef, readers, writers);
+            addExtendedSecurityRoles(nodeRef, readers, writers);
         }
     }
 
@@ -156,38 +186,16 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
      * @param writers
      * @param applyToParents
      */
-    @SuppressWarnings("unchecked")
+    private void addExtendedSecurityImpl(final NodeRef nodeRef, Set<String> readers, Set<String> writers)
-    private void addExtendedSecurityImpl(final NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents)
     {
         ParameterCheck.mandatory("nodeRef", nodeRef);
-        ParameterCheck.mandatory("applyToParents", applyToParents);
+        
-
+        // find groups
-        // get the properties
+        Pair<String, String> iprGroups = getIPRGroups(readers, writers);
-        final Map<QName, Serializable> properties = nodeService.getProperties(nodeRef);
+        
-
+        // assign groups to node
-        // update the readers map
+        assignIPRGroupsToNode(iprGroups, nodeRef);
-        if (readers != null && readers.size() != 0)
+        
-        {
-            // get reader map
-            Map<String, Integer> readersMap = (Map<String, Integer>)properties.get(PROP_READERS);
-
-            // set the readers property (this will in turn apply the aspect if required)
-            properties.put(PROP_READERS, (Serializable)addToMap(readersMap, readers));
-        }
-
-        // update the writers map
-	    if (writers != null && writers.size() != 0)
-	    {
-	    	// get writer map
-	        Map<String, Integer> writersMap = (Map<String, Integer>)properties.get(PROP_WRITERS);
-
-	        // set the writers property (this will in turn apply the aspect if required)
-	        properties.put(PROP_WRITERS, (Serializable)addToMap(writersMap, writers));
-	    }
-
-	    // set properties
-	    nodeService.setProperties(nodeRef, properties);
-
         // apply the readers to any renditions of the content
         if (isRecord(nodeRef))
         {
@@ -195,10 +203,188 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
             for (ChildAssociationRef assoc : assocs)
             {
                 NodeRef child = assoc.getChildRef();
-                addExtendedSecurityImpl(child, readers, writers, false);
+                assignIPRGroupsToNode(iprGroups, child);
             }
         }
     }
+    
+    /**
+     * 
+     * @param nodeRef
+     * @return
+     */
+    private Pair<String, String> getIPRGroups(NodeRef nodeRef)
+    {
+        Pair<String, String> result = null;
+        String iprReaderGroup = null;
+        String iprWriterGroup = null;
+        
+        Set<AccessPermission> permissions = permissionService.getAllSetPermissions(nodeRef);
+        for (AccessPermission permission : permissions)
+        {
+            if (permission.getAuthority().startsWith(PermissionService.GROUP_PREFIX + READER_GROUP_PREFIX))
+            {
+                iprReaderGroup = permission.getAuthority();
+            }   
+            else if (permission.getAuthority().startsWith(PermissionService.GROUP_PREFIX + WRITER_GROUP_PREFIX))
+            {
+                iprWriterGroup = permission.getAuthority();
+            }
+        }
+        
+        if (iprReaderGroup != null && iprWriterGroup != null)
+        {
+            result = new Pair<String, String>(iprReaderGroup, iprWriterGroup);
+        }
+            
+        return result;
+    }
+    
+    /**
+     * 
+     * @param readers
+     * @param writers
+     * @return
+     */
+    private Pair<String, String> getIPRGroups(Set<String> readers, Set<String> writers)
+    {
+        Pair<String, String> result = null;
+        
+        // see if the groups already exists or not
+        String readerGroupName = getIPRGroupName(READER_GROUP_PREFIX, readers, writers, false);
+        String writerGroupName = getIPRGroupName(WRITER_GROUP_PREFIX, readers, writers, false);
+        if (authorityService.authorityExists(readerGroupName) &&
+            authorityService.authorityExists(writerGroupName))
+        {
+            // check that the groups are a true match
+            if (authorityService.getContainingAuthorities(AuthorityType.GROUP, writerGroupName, true).contains(readerGroupName) &&
+                isIPRGroupTrueMatch(readers, readerGroupName) &&
+                isIPRGroupTrueMatch(writers, writerGroupName))
+            {     
+                // reuse the existing groups
+                result = new Pair<String, String>(readerGroupName, writerGroupName);
+            }
+            else
+            {
+                // TODO - CLASH
+                throw new AlfrescoRuntimeException("IPR Group Name Clash!");
+            }
+        }        
+        else
+        {
+            // create inplace record reader and writer groups
+            result = createIPRGroups(readers, writers);
+        }
+        
+        return result;
+    }
+    
+    /**
+     * 
+     * @param authorities
+     * @param group
+     * @return
+     */
+    private boolean isIPRGroupTrueMatch(Set<String> authorities, String group)
+    {
+        // TODO
+        return true;
+    }
+    
+    /**
+     * 
+     * @param prefix
+     * @param authorities
+     * @param shortName
+     * @return
+     */
+    private String getIPRGroupName(String prefix, Set<String> readers, Set<String> writers, boolean shortName)
+    {
+        StringBuilder builder = new StringBuilder(128);
+        
+        if (!shortName)
+        {
+            builder.append(PermissionService.GROUP_PREFIX);
+        }
+        
+        builder.append(prefix)
+               .append(getAuthoritySetHashCode(readers))
+               .append("-")
+               .append(getAuthoritySetHashCode(writers));
+        
+        return builder.toString();
+    }
+    
+    /**
+     * 
+     * @param authorities
+     * @return
+     */
+    private int getAuthoritySetHashCode(Set<String> authorities)
+    {
+        int result = 0;
+        if (authorities != null && !authorities.isEmpty())
+        {
+            result = StringUtils.join(authorities.toArray(), "").hashCode();   
+        }
+        return result;
+    }
+    
+    /**
+     * 
+     * @param readers
+     * @param writers
+     * @return
+     */
+    private Pair<String, String> createIPRGroups(Set<String> readers, Set<String> writers)
+    {
+        String iprReaderGroup = createIPRGroup(getIPRGroupName(READER_GROUP_PREFIX, readers, writers, true), null, readers); 
+        String iprWriterGroup = createIPRGroup(getIPRGroupName(WRITER_GROUP_PREFIX, readers, writers, true), iprReaderGroup, writers);
+        return new Pair<String, String>(iprReaderGroup, iprWriterGroup);
+    }
+    
+    /**
+     * 
+     * @param groupShortName
+     * @param parent
+     * @param children
+     * @return
+     */
+    private String createIPRGroup(String groupShortName, String parent, Set<String> children)
+    {
+        ParameterCheck.mandatory("groupShortName", groupShortName);
+        
+        String group = authorityService.createAuthority(AuthorityType.GROUP, groupShortName); // TODO set appropriate zone
+        
+        if (parent != null)
+        {
+            authorityService.addAuthority(parent, group);
+        }
+        
+        if (children != null)
+        {
+            for (String child : children)
+            {
+                if (!PermissionService.ALL_AUTHORITIES.equals(child))
+                {
+                    authorityService.addAuthority(group, child);
+                }
+            }
+        }
+        
+        return group;
+    }
+    
+    /**
+     * 
+     * @param iprGroups
+     * @param nodeRef
+     */
+    private void assignIPRGroupsToNode(Pair<String, String> iprGroups, NodeRef nodeRef)
+    {
+        permissionService.setPermission(nodeRef, iprGroups.first, RMPermissionModel.READ_RECORDS, true);
+        permissionService.setPermission(nodeRef, iprGroups.second, RMPermissionModel.FILING, true);
+    }
 
     /**
      *
@@ -235,60 +421,16 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
             }
         }
     }
-
+    
     /**
-     *
+     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef)
-     * @param map
-     * @param keys
-     * @return
-     */
-    private Map<String, Integer> addToMap(Map<String, Integer> map, Set<String> keys)
-    {
-        if (map == null)
-        {
-            // create map
-            map = new HashMap<String, Integer>(7);
-        }
-
-        for (String key : keys)
-        {
-            if (!key.equals(PermissionService.ALL_AUTHORITIES))
-            {
-                if (map.containsKey(key))
-                {
-                    // increment reference count
-                    Integer count = map.get(key);
-                    map.put(key, Integer.valueOf(count.intValue()+1));
-                }
-                else
-                {
-                    // add key with initial count
-                    map.put(key, Integer.valueOf(1));
-                }
-            }
-        }
-
-        return map;
-    }
-
-    /**
-     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set)
      */
     @Override
-    public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers)
+    public void removeAllExtendedSecurity(NodeRef nodeRef)
-    {
-        removeExtendedSecurity(nodeRef, readers, writers, true);
-    }
-
-    /**
-     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean)
-     */
-    @Override
-    public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String>writers, boolean applyToParents)
     {
         if (hasExtendedSecurity(nodeRef))
         {
-            removeExtendedSecurityImpl(nodeRef, readers, writers);
+            removeExtendedSecurityImpl(nodeRef);
 
             // remove the readers from any renditions of the content
             if (isRecord(nodeRef))
@@ -297,110 +439,62 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
                 for (ChildAssociationRef assoc : assocs)
                 {
                     NodeRef child = assoc.getChildRef();
-                    removeExtendedSecurityImpl(child, readers, writers);
+                    removeExtendedSecurityImpl(child);
-                }
-            }
-
-            if (applyToParents)
-            {
-                // apply the extended readers up the file plan primary hierarchy
-                NodeRef parent = nodeService.getPrimaryParent(nodeRef).getParentRef();
-                if (parent != null &&
-                    filePlanService.isFilePlanComponent(parent))
-                {
-                    removeExtendedSecurity(parent, readers, null, applyToParents);
-                    removeExtendedSecurity(parent, writers, null, applyToParents);
                 }
             }
         }
     }
 
     /**
-     * Removes a set of readers and writers from a node reference.
+     * 
-     * <p>
+     * @param nodeRef
-     * Removes the aspect and resets the property to null if all readers and writers are removed.
+     * @param readers
-     *
+     * @param writers
-     * @param nodeRef   node reference
-     * @param readers   {@link Set} of readers
-     * @param writers   {@link Set} of writers
      */
-    @SuppressWarnings("unchecked")
+    private void removeExtendedSecurityImpl(NodeRef nodeRef)
-    private void removeExtendedSecurityImpl(NodeRef nodeRef, Set<String> readers, Set<String> writers)
     {
-        Map<String, Integer> readersMap = (Map<String, Integer>)nodeService.getProperty(nodeRef, PROP_READERS);
+        ParameterCheck.mandatory("nodeRef", nodeRef);
-        nodeService.setProperty(nodeRef, PROP_READERS, (Serializable)removeFromMap(readersMap, readers));
+        
-
+        Pair<String, String> iprGroups = getIPRGroups(nodeRef);
-        Map<String, Integer> writersMap = (Map<String, Integer>)nodeService.getProperty(nodeRef, PROP_WRITERS);
+        if (iprGroups != null)
-        nodeService.setProperty(nodeRef, PROP_WRITERS, (Serializable)removeFromMap(writersMap, writers));
-
-        if (readersMap == null && writersMap == null)
         {
-            // remove the aspect
+            // remove group permissions from node
-            nodeService.removeAspect(nodeRef, ASPECT_EXTENDED_SECURITY);
+            permissionService.clearPermission(nodeRef, iprGroups.first);
+            permissionService.clearPermission(nodeRef, iprGroups.second);
+            
+            // TODO delete the groups if they are no longer in use (easier said than done perhaps!)
         }
+    }        
+    
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean)
+     */
+    @Override @Deprecated public void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents)
+    {
+        addExtendedSecurity(nodeRef, readers, writers);
+    }
+    
+    /**
+     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set)
+     */
+    @Override @Deprecated public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers)
+    {
+        removeAllExtendedSecurity(nodeRef);
     }
 
     /**
-     * Helper method to remove items from map or reduce reference count
+     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean)
-     *
-     * @param map                       ref count map
-     * @param keys                      keys
-     * @return Map<String, Integer>     ref count map
      */
-    private Map<String, Integer> removeFromMap(Map<String, Integer> map, Set<String> keys)
+    @Override @Deprecated public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String>writers, boolean applyToParents)
     {
-        if (map != null && keys != null && keys.size() != 0)
+        removeAllExtendedSecurity(nodeRef);        
-        {
-            // remove the keys
-            for (String key : keys)
-            {
-                if (!key.equals(PermissionService.ALL_AUTHORITIES))
-                {
-                    Integer count = map.get(key);
-                    if (count != null)
-                    {
-                        if (count == 1)
-                        {
-                            // remove entry all together if the reference count is now 0
-                            map.remove(key);
-                        }
-                        else
-                        {
-                            // decrement the reference count by 1
-                            map.put(key, Integer.valueOf(count.intValue()-1));
-                        }
-                    }
-                }
-            }
-        }
-
-        // reset the map to null if now empty
-        if (map != null && map.isEmpty())
-        {
-            map = null;
-        }
-
-        return map;
-    }
-
-    /**
-     * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef)
-     */
-    @Override
-    public void removeAllExtendedSecurity(NodeRef nodeRef)
-    {
-        removeAllExtendedSecurity(nodeRef, true);
     }
 
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.ExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, boolean)
      */
-    @Override
+    @Override @Deprecated public void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents)
-    public void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents)
     {
-        if (hasExtendedSecurity(nodeRef))
+        removeAllExtendedSecurity(nodeRef); 
-        {
-            removeExtendedSecurity(nodeRef, getExtendedReaders(nodeRef), getExtendedWriters(nodeRef));
-        }
     }
 }

rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/FilePlanPermissionServiceImpl.java

@@ -27,8 +27,6 @@
 
 package org.alfresco.module.org_alfresco_module_rm.security;
 
-import static org.alfresco.module.org_alfresco_module_rm.security.ExtendedReaderDynamicAuthority.EXTENDED_READER;
-import static org.alfresco.module.org_alfresco_module_rm.security.ExtendedWriterDynamicAuthority.EXTENDED_WRITER;
 import static org.alfresco.repo.policy.Behaviour.NotificationFrequency.TRANSACTION_COMMIT;
 import static org.alfresco.repo.policy.annotation.BehaviourKind.CLASS;
 import static org.alfresco.repo.security.authentication.AuthenticationUtil.getSystemUserName;
@@ -383,13 +381,29 @@ public class FilePlanPermissionServiceImpl extends    ServiceBaseImpl
                     boolean inheritanceAllowed = isInheritanceAllowed(nodeRef, isParentNodeFilePlan);
                     getPermissionService().setInheritParentPermissions(nodeRef, inheritanceAllowed);
 
-                    // clear all existing permissions
+                    Set<AccessPermission> keepPerms = new HashSet<AccessPermission>(5);
+                    Set<AccessPermission> origionalPerms= getPermissionService().getAllSetPermissions(nodeRef);
+
+                    for (AccessPermission perm : origionalPerms)
+                    {
+                        if (perm.getAuthority().startsWith(PermissionService.GROUP_PREFIX + ExtendedSecurityService.IPR_GROUP_PREFIX))
+                        {
+                            // then we can assume this is a permission we want to preserve
+                            keepPerms.add(perm);
+                        }
+                    }
+
+                    // clear all existing permissions and start again
                     getPermissionService().clearPermission(nodeRef, null);
 
+                    // re-add keep'er permissions
+                    for (AccessPermission keeper : keepPerms)
+                    {
+                        setPermission(nodeRef, keeper.getAuthority(), keeper.getPermission());
+                    }
+
                     if (!inheritanceAllowed)
                     {
-                        getPermissionService().setPermission(nodeRef, EXTENDED_READER, READ_RECORDS, true);
-                        getPermissionService().setPermission(nodeRef, EXTENDED_WRITER, FILING, true);
                         String adminRole = getAdminRole(nodeRef);
                         getPermissionService().setPermission(nodeRef, adminRole, RMPermissionModel.FILING, true);
                     }
@@ -494,11 +508,8 @@ public class FilePlanPermissionServiceImpl extends    ServiceBaseImpl
                     for (AccessPermission recordPermission : origionalRecordPerms)
                     {
                         String permission = recordPermission.getPermission();
-                        String authority = recordPermission.getAuthority();
                         if ((RMPermissionModel.FILING.equals(permission) || RMPermissionModel.READ_RECORDS.equals(permission)) &&
-                                recordPermission.isSetDirectly() &&
+                             recordPermission.isSetDirectly())
-                                !ExtendedReaderDynamicAuthority.EXTENDED_READER.equals(authority) &&
-                                !ExtendedWriterDynamicAuthority.EXTENDED_WRITER.equals(authority))
                         {
                             // then we can assume this is a permission we want to preserve
                             keepPerms.add(recordPermission);
@@ -506,7 +517,7 @@ public class FilePlanPermissionServiceImpl extends    ServiceBaseImpl
                     }
 
                     // clear all existing permissions and start again
-                    permissionService.deletePermissions(record);
+                   // permissionService.deletePermissions(record);
 
                     // re-setup the records permissions
                     setupPermissions(destinationAssocRef.getParentRef(), record);

rm-community/rm-community-repo/source/java/org/alfresco/repo/security/permissions/impl/ExtendedPermissionServiceImpl.java

@@ -337,9 +337,6 @@ public class ExtendedPermissionServiceImpl extends PermissionServiceImpl impleme
         if (nodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_FILE_PLAN_COMPONENT) && isNotBlank(adminRole)
                     && !inheritParentPermissions)
         {
-            setPermission(nodeRef, ExtendedReaderDynamicAuthority.EXTENDED_READER, RMPermissionModel.READ_RECORDS,
-                        true);
-            setPermission(nodeRef, ExtendedWriterDynamicAuthority.EXTENDED_WRITER, RMPermissionModel.FILING, true);
             setPermission(nodeRef, adminRole, RMPermissionModel.FILING, true);
         }
         super.setInheritParentPermissions(nodeRef, inheritParentPermissions);

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

@@ -0,0 +1,186 @@
+/*
+ * #%L
+ * Alfresco Records Management Module
+ * %%
+ * Copyright (C) 2005 - 2016 Alfresco Software Limited
+ * %%
+ * This file is part of the Alfresco software.
+ * -
+ * If the software was purchased under a paid Alfresco license, the terms of
+ * the paid license agreement will prevail.  Otherwise, the software is
+ * provided under the following open source license terms:
+ * -
+ * 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/>.
+ * #L%
+ */
+
+package org.alfresco.module.org_alfresco_module_rm.test.integration.record;
+
+import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
+import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
+import org.alfresco.service.cmr.model.FileExistsException;
+import org.alfresco.service.cmr.model.FileNotFoundException;
+import org.alfresco.service.cmr.security.AccessStatus;
+
+/**
+ * Create Inplace Record Test
+ *
+ * @author Roy Wetherall
+ */
+public class CreateInplaceRecordTest extends BaseRMTestCase
+{
+    @Override
+    protected boolean isCollaborationSiteTest()
+    {
+        return true;
+    }
+
+    /**
+     * Given a document in a collaboration site
+     * When the document is declared by a site collaborator
+     * Then the document becomes a record
+     * And the site users have the appropriate in-place permissions on the record
+     */
+    public void testCreateInplaceRecordFromCollabSite()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                // Check that the document is not a record
+                assertFalse(recordService.isRecord(dmDocument));            
+            }
+
+            public void when()
+            {
+                // Declare the document as a record
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        // Declare record
+                        recordService.createRecord(filePlan, dmDocument);
+
+                        return null;
+                    }
+                 }, dmCollaborator);
+            }
+
+            public void then()
+            {
+                // Check that the document is a record now
+                assertTrue(recordService.isRecord(dmDocument));    
+                
+                // Check that the record is in the unfiled container
+                
+                // Check that the record is still a child of the collaboration folder
+                
+                // Check that the collaborator has filling permissions on the record                
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(dmDocument, RMPermissionModel.FILING));
+                        assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(dmDocument, RMPermissionModel.READ_RECORDS));
+                        return null;
+                    }
+                 }, dmCollaborator);
+                                
+                
+                // Check that the consumer has read permissions on the record
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        assertEquals(AccessStatus.DENIED, permissionService.hasPermission(dmDocument, RMPermissionModel.FILING));
+                        assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(dmDocument, RMPermissionModel.READ_RECORDS));
+                        return null;
+                    }
+                 }, dmConsumer);
+                
+            }
+        });
+    }
+    
+    public void testFileInplaceRecordFromCollabSite()
+    {
+        doBehaviourDrivenTest(new BehaviourDrivenTest()
+        {
+            public void given()
+            {
+                // Check that the document is not a record
+                assertFalse(recordService.isRecord(dmDocument));  
+                
+                // Declare the document as a record
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        // Declare record
+                        recordService.createRecord(filePlan, dmDocument);
+
+                        return null;
+                    }
+                 }, dmCollaborator);
+                
+                // Check that the document is a record
+                assertTrue(recordService.isRecord(dmDocument));
+                assertFalse(recordService.isFiled(dmDocument));
+            }
+
+            public void when() throws FileExistsException, FileNotFoundException
+            {
+                // file the document to a location in the file plan
+                fileFolderService.move(dmDocument, rmFolder, null);
+            }
+
+            public void then()
+            {
+                // Check that the document is a record now
+                assertTrue(recordService.isRecord(dmDocument));
+                assertTrue(recordService.isFiled(dmDocument));  
+                
+                // Check that the record is in the unfiled container
+                
+                // Check that the record is still a child of the collaboration folder
+                
+                // Check that the collaborator has filling permissions on the record                
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(dmDocument, RMPermissionModel.FILING));
+                        assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(dmDocument, RMPermissionModel.READ_RECORDS));
+                        return null;
+                    }
+                 }, dmCollaborator);
+                                
+                
+                // Check that the consumer has read permissions on the record
+                AuthenticationUtil.runAs(new RunAsWork<Void>()
+                {
+                    public Void doWork() throws Exception
+                    {
+                        assertEquals(AccessStatus.DENIED, permissionService.hasPermission(dmDocument, RMPermissionModel.FILING));
+                        assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(dmDocument, RMPermissionModel.READ_RECORDS));
+                        return null;
+                    }
+                 }, dmConsumer);
+                
+            }
+        });
+    }
+}

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/action/RejectActionTest.java

@@ -113,7 +113,8 @@ public class RejectActionTest extends BaseRMTestCase
                 assertTrue(nodeService.getParentAssocs(dmDocument).size() == 1);
 
                 // The extended reader information should be removed
-                assertNull(extendedSecurityService.getExtendedReaders(dmDocument));
+                assertFalse(extendedSecurityService.hasExtendedSecurity(dmDocument));
+                assertTrue(extendedSecurityService.getExtendedReaders(dmDocument).isEmpty());
 
                 return null;
             }

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/ExtendedSecurityServiceImplTest.java

@@ -27,9 +27,7 @@
 
 package org.alfresco.module.org_alfresco_module_rm.test.legacy.service;
 
-import java.util.HashMap;
 import java.util.HashSet;
-import java.util.Map;
 import java.util.Set;
 
 import org.alfresco.model.ContentModel;
@@ -101,64 +99,29 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase
                 assertFalse(extendedSecurityService.hasExtendedSecurity(rmFolder));
                 assertFalse(extendedSecurityService.hasExtendedSecurity(record));
 
-                assertNull(extendedSecurityService.getExtendedReaders(record));
+                assertTrue(extendedSecurityService.getExtendedReaders(record).isEmpty());
-                assertNull(extendedSecurityService.getExtendedWriters(record));
+                assertTrue(extendedSecurityService.getExtendedWriters(record).isEmpty());
 
                 Set<String> extendedReaders = new HashSet<String>(2);
                 extendedReaders.add(monkey);
                 extendedReaders.add(elephant);
 
                 extendedSecurityService.addExtendedSecurity(record, extendedReaders, null);
-
+                checkExtendedReaders(record, extendedReaders);
-                Map<String, Integer> testMap = new HashMap<String, Integer>(2);
-                testMap.put(monkey, Integer.valueOf(1));
-                testMap.put(elephant, Integer.valueOf(1));
-
-                checkExtendedReaders(record, testMap);
 
                 Set<String> extendedReadersToo = new HashSet<String>(2);
                 extendedReadersToo.add(monkey);
                 extendedReadersToo.add(snake);
 
                 extendedSecurityService.addExtendedSecurity(recordToo, extendedReadersToo, null);
+                checkExtendedReaders(recordToo, extendedReadersToo);
 
-                Map<String, Integer> testMapToo = new HashMap<String, Integer>(2);
+                // test remove
-                testMapToo.put(monkey, Integer.valueOf(1));
+                extendedSecurityService.removeAllExtendedSecurity(recordToo);
-                testMapToo.put(snake, Integer.valueOf(1));
+                
-
+                assertFalse(extendedSecurityService.hasExtendedSecurity(recordToo));
-                Map<String, Integer> testMapThree = new HashMap<String, Integer>(3);
+                assertTrue(extendedSecurityService.getExtendedReaders(recordToo).isEmpty());
-                testMapThree.put(monkey, Integer.valueOf(2));
+                assertTrue(extendedSecurityService.getExtendedWriters(recordToo).isEmpty());
-                testMapThree.put(elephant, Integer.valueOf(1));
-                testMapThree.put(snake, Integer.valueOf(1));
-
-                checkExtendedReaders(recordToo, testMapToo);
-
-                // test remove (with no parent inheritance)
-
-                Set<String> removeMap1 = new HashSet<String>(2);
-                removeMap1.add(elephant);
-                removeMap1.add(monkey);
-
-                extendedSecurityService.removeExtendedSecurity(rmFolder, removeMap1, null, false);
-
-                Map<String, Integer> testMapFour = new HashMap<String, Integer>(2);
-                testMapFour.put(monkey, Integer.valueOf(1));
-                testMapFour.put(snake, Integer.valueOf(1));
-
-                checkExtendedReaders(recordToo, testMapToo);
-
-                // test remove (apply to parents)
-
-                Set<String> removeMap2 = new HashSet<String>(1);
-                removeMap2.add(snake);
-
-                extendedSecurityService.removeExtendedSecurity(recordToo, removeMap2, null, true);
-
-                testMapThree.remove(snake);
-                testMapFour.remove(snake);
-                testMapToo.remove(snake);
-
-                checkExtendedReaders(recordToo, testMapToo);
 
                 return null;
             }
@@ -172,12 +135,12 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase
 
         doTestInTransaction(new Test<Void>()
         {
-            Map<String, Integer> testMap = new HashMap<String, Integer>(2);
+            Set<String> extendedReaders = new HashSet<String>(2);;
 
             public Void run() throws Exception
             {
-                testMap.put(monkey, Integer.valueOf(1));
+                extendedReaders.add(monkey);
-                testMap.put(elephant, Integer.valueOf(1));
+                extendedReaders.add(elephant);
 
                 assertFalse(extendedSecurityService.hasExtendedSecurity(filePlan));
                 assertFalse(extendedSecurityService.hasExtendedSecurity(rmContainer));
@@ -186,15 +149,11 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase
                 assertFalse(extendedSecurityService.hasExtendedSecurity(moveRecordCategory));
                 assertFalse(extendedSecurityService.hasExtendedSecurity(moveRecordFolder));
 
-                assertNull(extendedSecurityService.getExtendedReaders(record));
+                assertTrue(extendedSecurityService.getExtendedReaders(record).isEmpty());
-
-                Set<String> extendedReaders = new HashSet<String>(2);
-                extendedReaders.add(monkey);
-                extendedReaders.add(elephant);
 
                 extendedSecurityService.addExtendedSecurity(record, extendedReaders, null);
 
-                checkExtendedReaders(record, testMap);
+                checkExtendedReaders(record, extendedReaders);
                 assertFalse(extendedSecurityService.hasExtendedSecurity(moveRecordCategory));
                 assertFalse(extendedSecurityService.hasExtendedSecurity(moveRecordFolder));
 
@@ -206,31 +165,21 @@ public class ExtendedSecurityServiceImplTest extends BaseRMTestCase
             @Override
             public void test(Void result) throws Exception
             {
-                checkExtendedReaders(record, testMap);
+                checkExtendedReaders(record, extendedReaders);
             }
         });
     }
 
-
+    /**
-    @SuppressWarnings("unchecked")
+     * Check extended readers helper method
-    private void checkExtendedReaders(NodeRef nodeRef, Map<String, Integer> testMap)
+     */
+    private void checkExtendedReaders(NodeRef nodeRef, Set<String> testReaders)
     {
         assertTrue(extendedSecurityService.hasExtendedSecurity(nodeRef));
 
-        Map<String, Integer> readersMap = (Map<String,Integer>)nodeService.getProperty(nodeRef, PROP_READERS);
-        assertNotNull(readersMap);
-        assertEquals(testMap.size(), readersMap.size());
-
-        for (Map.Entry<String, Integer> entry: testMap.entrySet())
-        {
-            assertTrue(readersMap.containsKey(entry.getKey()));
-            assertEquals(entry.getKey(), entry.getValue(), readersMap.get(entry.getKey()));
-
-        }
-
         Set<String> readers = extendedSecurityService.getExtendedReaders(nodeRef);
         assertNotNull(readers);
-        assertEquals(testMap.size(), readers.size());
+        assertEquals(testReaders, readers);
     }
 
     public void testDifferentUsersDifferentPermissions()

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/FilePlanPermissionServiceImplTest.java

@@ -33,8 +33,6 @@ import java.util.Set;
 
 import org.alfresco.module.org_alfresco_module_rm.capability.RMPermissionModel;
 import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
-import org.alfresco.module.org_alfresco_module_rm.security.ExtendedReaderDynamicAuthority;
-import org.alfresco.module.org_alfresco_module_rm.security.ExtendedWriterDynamicAuthority;
 import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -1241,10 +1239,6 @@ public class FilePlanPermissionServiceImplTest extends BaseRMTestCase
             accessPermissions.put(permission.getAuthority(),  permission.getPermission());
         }
 
-        assertTrue(accessPermissions.containsKey(ExtendedReaderDynamicAuthority.EXTENDED_READER));
-        assertEquals(RMPermissionModel.READ_RECORDS, accessPermissions.get(ExtendedReaderDynamicAuthority.EXTENDED_READER));
-        assertTrue(accessPermissions.containsKey(ExtendedWriterDynamicAuthority.EXTENDED_WRITER));
-        assertEquals(RMPermissionModel.FILING, accessPermissions.get(ExtendedWriterDynamicAuthority.EXTENDED_WRITER));
         String adminRole = authorityService.getName(AuthorityType.GROUP, FilePlanRoleService.ROLE_ADMIN + filePlan.getId());
         assertTrue(accessPermissions.containsKey(adminRole));
         assertEquals(RMPermissionModel.FILING, accessPermissions.get(adminRole));

rm-community/rm-community-repo/test/java/org/alfresco/module/org_alfresco_module_rm/test/legacy/service/RecordServiceImplTest.java

@@ -38,8 +38,6 @@ 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.record.RecordService;
 import org.alfresco.module.org_alfresco_module_rm.role.Role;
-import org.alfresco.module.org_alfresco_module_rm.security.ExtendedReaderDynamicAuthority;
-import org.alfresco.module.org_alfresco_module_rm.security.ExtendedWriterDynamicAuthority;
 import org.alfresco.module.org_alfresco_module_rm.test.util.BaseRMTestCase;
 import org.alfresco.repo.content.MimetypeMap;
 import org.alfresco.repo.security.authentication.AuthenticationUtil;
@@ -181,22 +179,13 @@ public class RecordServiceImplTest extends BaseRMTestCase
 
     public void testExtendedWriters() throws Exception
     {
-        final ExtendedReaderDynamicAuthority readerDy = (ExtendedReaderDynamicAuthority)applicationContext.getBean("extendedReaderDynamicAuthority");
-        final ExtendedWriterDynamicAuthority writerDy = (ExtendedWriterDynamicAuthority)applicationContext.getBean("extendedWriterDynamicAuthority");
-
         doTestInTransaction(new Test<Void>()
         {
             @Override
             public Void run()
             {
-                assertNull(extendedSecurityService.getExtendedReaders(recordOne));
+                assertTrue(extendedSecurityService.getExtendedReaders(recordOne).isEmpty());
-                assertNull(extendedSecurityService.getExtendedWriters(recordOne));
+                assertTrue(extendedSecurityService.getExtendedWriters(recordOne).isEmpty());
-
-                assertFalse(readerDy.hasAuthority(recordOne, dmCollaborator));
-                assertFalse(writerDy.hasAuthority(recordOne, dmCollaborator));
-
-                assertFalse(readerDy.hasAuthority(filePlan, dmCollaborator));
-                assertFalse(writerDy.hasAuthority(filePlan, dmCollaborator));
 
                 return null;
             }
@@ -209,16 +198,9 @@ public class RecordServiceImplTest extends BaseRMTestCase
             {
                 assertEquals(AccessStatus.DENIED, permissionService.hasPermission(recordOne, RMPermissionModel.READ_RECORDS));
                 assertEquals(AccessStatus.DENIED, permissionService.hasPermission(recordOne, RMPermissionModel.FILING));
-
-                assertFalse(readerDy.hasAuthority(recordOne, dmCollaborator));
-                assertFalse(writerDy.hasAuthority(recordOne, dmCollaborator));
-
                 assertEquals(AccessStatus.DENIED, permissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
                 assertEquals(AccessStatus.DENIED, permissionService.hasPermission(filePlan, RMPermissionModel.EDIT_NON_RECORD_METADATA));
 
-                assertFalse(readerDy.hasAuthority(filePlan, dmCollaborator));
-                assertFalse(writerDy.hasAuthority(filePlan, dmCollaborator));
-
                 return null;
             }
         }, dmCollaborator);
@@ -232,7 +214,7 @@ public class RecordServiceImplTest extends BaseRMTestCase
                 writers.add(dmCollaborator);
                 extendedSecurityService.addExtendedSecurity(recordOne, null, writers);
 
-                assertNull(extendedSecurityService.getExtendedReaders(recordOne));
+                assertTrue(extendedSecurityService.getExtendedReaders(recordOne).isEmpty());
                 assertFalse(extendedSecurityService.getExtendedWriters(recordOne).isEmpty());
 
                 return null;
@@ -247,9 +229,7 @@ public class RecordServiceImplTest extends BaseRMTestCase
                 assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(recordOne, RMPermissionModel.READ_RECORDS));
                 assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(recordOne, RMPermissionModel.FILING));
 
-                assertFalse(readerDy.hasAuthority(recordOne, dmCollaborator));
+                // ALLOWED, becuase users have been added to the in-place roles
-                assertTrue(writerDy.hasAuthority(recordOne, dmCollaborator));
-
                 assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan, RMPermissionModel.VIEW_RECORDS));
                 assertEquals(AccessStatus.ALLOWED, permissionService.hasPermission(filePlan, RMPermissionModel.EDIT_NON_RECORD_METADATA));