MNT-24975 - Repeated IPR groups due to casing inconsistencies on creation (#3509)

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

@@ -47,7 +47,10 @@ import org.alfresco.module.org_alfresco_module_rm.role.FilePlanRoleService;
 import org.alfresco.module.org_alfresco_module_rm.util.ServiceBaseImpl;
 import org.alfresco.query.PagingRequest;
 import org.alfresco.query.PagingResults;
+import org.alfresco.repo.security.authentication.AuthenticationUtil;
+import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
 import org.alfresco.repo.security.authority.RMAuthority;
+import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 import org.alfresco.service.cmr.repository.ChildAssociationRef;
 import org.alfresco.service.cmr.repository.DuplicateChildNodeNameException;
 import org.alfresco.service.cmr.repository.NodeRef;
@@ -59,12 +62,6 @@ import org.alfresco.service.namespace.RegexQNamePattern;
 import org.alfresco.service.transaction.TransactionService;
 import org.alfresco.util.Pair;
 import org.alfresco.util.ParameterCheck;
-import org.springframework.context.ApplicationListener;
-import org.springframework.context.event.ContextRefreshedEvent;
-import org.springframework.extensions.webscripts.ui.common.StringUtils;
-import org.alfresco.repo.security.authentication.AuthenticationUtil;
-import org.alfresco.repo.security.authentication.AuthenticationUtil.RunAsWork;
-import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransactionCallback;
 
 /**
  * Extended security service implementation.
@@ -103,7 +100,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     private boolean enableUsernameNormalization;
 
     /**
-     * @param filePlanService   file plan service
+     * @param filePlanService
+     *            file plan service
      */
     public void setFilePlanService(FilePlanService filePlanService)
     {
@@ -111,7 +109,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     }
 
     /**
-     * @param filePlanRoleService   file plan role service
+     * @param filePlanRoleService
+     *            file plan role service
      */
     public void setFilePlanRoleService(FilePlanRoleService filePlanRoleService)
     {
@@ -119,7 +118,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     }
 
     /**
-     * @param authorityService  authority service
+     * @param authorityService
+     *            authority service
      */
     public void setAuthorityService(AuthorityService authorityService)
     {
@@ -127,7 +127,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     }
 
     /**
-     * @param permissionService permission service
+     * @param permissionService
+     *            permission service
      */
     public void setPermissionService(PermissionService permissionService)
     {
@@ -135,7 +136,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     }
 
     /**
-     * @param transactionService    transaction service
+     * @param transactionService
+     *            transaction service
      */
     public void setTransactionService(TransactionService transactionService)
     {
@@ -158,12 +160,10 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     public void onApplicationEvent(ContextRefreshedEvent contextRefreshedEvent)
     {
         // run as System on bootstrap
-        AuthenticationUtil.runAs(new RunAsWork<Object>()
+        AuthenticationUtil.runAs(new RunAsWork<Object>() {
-        {
             public Object doWork()
             {
-                RetryingTransactionCallback<Void> callback = new RetryingTransactionCallback<Void>()
+                RetryingTransactionCallback<Void> callback = new RetryingTransactionCallback<Void>() {
-                {
                     public Void execute()
                     {
                         // if the root group doesn't exist then create it
@@ -240,7 +240,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Helper to get authorities for a given group
      *
-     * @param group         group name
+     * @param group
+     *            group name
      * @return Set<String> immediate authorities
      */
     private Set<String> getAuthorities(String group)
@@ -300,7 +301,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
      * <p>
      * Return null if none found.
      *
-     * @param nodeRef                node reference
+     * @param nodeRef
+     *            node reference
      * @return Pair<String, String> where first is the read group and second if the write group, null if none found
      */
     private Pair<String, String> getIPRGroups(NodeRef nodeRef)
@@ -337,17 +339,17 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Given a set of readers and writers find or create the appropriate IPR groups.
      * <p>
-     * The IPR groups are named with hashes of the authority lists in order to reduce
+     * The IPR groups are named with hashes of the authority lists in order to reduce the set of groups that require exact match. A further index is used to handle a situation where there is a hash clash, but a difference in the authority lists.
-     * the set of groups that require exact match.  A further index is used to handle
-     * a situation where there is a hash clash, but a difference in the authority lists.
      * <p>
      * When no match is found the groups are created. Once created
      *
-     * @param filePlan              file plan
+     * @param filePlan
-     * @param readers               authorities with read
+     *            file plan
-     * @param writers               authorities with write
+     * @param readers
-     * @return Pair<String, String> where first is the full name of the read group and
+     *            authorities with read
-     *                              second is the full name of the write group
+     * @param writers
+     *            authorities with write
+     * @return Pair<String, String> where first is the full name of the read group and second is the full name of the write group
      */
     private Pair<String, String> createOrFindIPRGroups(Set<String> readers, Set<String> writers)
     {
@@ -359,20 +361,28 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Create or find an IPR group based on the provided prefix and authorities.
      *
-     * @param groupPrefix   group prefix
+     * @param groupPrefix
-     * @param authorities   authorities
+     *            group prefix
+     * @param authorities
+     *            authorities
      * @return String full group name
      */
     private String createOrFindIPRGroup(String groupPrefix, Set<String> authorities)
     {
         String group = null;
 
+        // If enabled, the authorities are forced to match the correct casing of the usernames in case they were set
+        // with the incorrect casing.
+        // If not, it will just use the authorities as they are.
+        // In normal circumstances, the authorities are in the correct casing, so this is disabled by default.
+        Set<String> authoritySet = normalizeAuthorities(authorities);
+
         // find group or determine what the next index is if no group exists or there is a clash
-        Pair<String, Integer> groupResult = findIPRGroup(groupPrefix, authorities);
+        Pair<String, Integer> groupResult = findIPRGroup(groupPrefix, authoritySet);
 
         if (groupResult.getFirst() == null)
         {
-            group = createIPRGroup(groupPrefix, authorities, groupResult.getSecond());
+            group = createIPRGroup(groupPrefix, authoritySet, groupResult.getSecond());
         }
         else
         {
@@ -385,13 +395,13 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Given a group name prefix and the authorities, finds the exact match existing group.
      * <p>
-     * If the group does not exist then the group returned is null and the index shows the next available
+     * If the group does not exist then the group returned is null and the index shows the next available group index for creation.
-     * group index for creation.
      *
-     * @param groupPrefix             group name prefix
+     * @param groupPrefix
-     * @param authorities             authorities
+     *            group name prefix
-     * @return Pair<String, Integer>  where first is the name of the found group, null if none found and second
+     * @param authorities
-     *                                if the next available create index
+     *            authorities
+     * @return Pair<String, Integer> where first is the name of the found group, null if none found and second if the next available create index
      */
     private Pair<String, Integer> findIPRGroup(String groupPrefix, Set<String> authorities)
     {
@@ -400,13 +410,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
         boolean hasMoreItems = true;
         int pageCount = 0;
 
-        // If enabled, the authorities are forced to match the correct casing of the usernames in case they were set with the incorrect casing.
-        // If not, it will just use the authorities as they are.
-        // In normal circumstances, the authorities are in the correct casing, so this is disabled by default.
-        Set<String> authoritySet = normalizeAuthorities(authorities);
-
         // determine the short name prefix
-        String groupShortNamePrefix = getIPRGroupPrefixShortName(groupPrefix, authoritySet);
+        String groupShortNamePrefix = getIPRGroupPrefixShortName(groupPrefix, authorities);
 
         // iterate over the authorities to find a match
         while (hasMoreItems == true)
@@ -427,7 +432,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
             for (String group : results.getPage())
             {
                 // if exists and matches we have found our group
-                if (isIPRGroupTrueMatch(group, authoritySet))
+                if (isIPRGroupTrueMatch(group, authorities))
                 {
                     return new Pair<String, Integer>(group, nextGroupIndex);
                 }
@@ -501,8 +506,10 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Determines whether a group exactly matches a list of authorities.
      *
-     * @param authorities           list of authorities
+     * @param authorities
-     * @param group                 group
+     *            list of authorities
+     * @param group
+     *            group
      * @return
      */
     private boolean isIPRGroupTrueMatch(String group, Set<String> authorities)
@@ -523,8 +530,10 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
      * <p>
      * 'package' scope to help testing.
      *
-     * @param prefix        prefix
+     * @param prefix
-     * @param authorities   authorities
+     *            prefix
+     * @param authorities
+     *            authorities
      * @return String group prefix short name
      */
     /* package */ String getIPRGroupPrefixShortName(String prefix, Set<String> authorities)
@@ -543,10 +552,14 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
      * <p>
      * 'package' scope to help testing.
      *
-     * @param prefix    prefix
+     * @param prefix
-     * @param readers   read authorities
+     *            prefix
-     * @param writers   write authorities
+     * @param readers
-     * @param index     group index
+     *            read authorities
+     * @param writers
+     *            write authorities
+     * @param index
+     *            group index
      * @return String group short name
      */
     /* package */ String getIPRGroupShortName(String prefix, Set<String> authorities, int index)
@@ -559,10 +572,14 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
      * <p>
      * Note this excludes the "GROUP_" prefix.
      *
-     * @param prefix    prefix
+     * @param prefix
-     * @param readers   read authorities
+     *            prefix
-     * @param writers   write authorities
+     * @param readers
-     * @param index     group index
+     *            read authorities
+     * @param writers
+     *            write authorities
+     * @param index
+     *            group index
      * @return String group short name
      */
     private String getIPRGroupShortName(String prefix, Set<String> authorities, String index)
@@ -577,7 +594,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Gets the hashcode value of a set of authorities.
      *
-     * @param authorities   set of authorities
+     * @param authorities
+     *            set of authorities
      * @return int hash code
      */
     private int getAuthoritySetHashCode(Set<String> authorities)
@@ -593,9 +611,12 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Creates a new IPR group.
      *
-     * @param groupNamePrefix   group name prefix
+     * @param groupNamePrefix
-     * @param children          child authorities
+     *            group name prefix
-     * @param index             group index
+     * @param children
+     *            child authorities
+     * @param index
+     *            group index
      * @return String full name of created group
      */
     private String createIPRGroup(String groupNamePrefix, Set<String> children, int index)
@@ -638,8 +659,10 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Assign IPR groups to a node reference with the correct permissions.
      *
-     * @param iprGroups iprGroups, first read and second write
+     * @param iprGroups
-     * @param nodeRef   node reference
+     *            iprGroups, first read and second write
+     * @param nodeRef
+     *            node reference
      */
     private void assignIPRGroupsToNode(Pair<String, String> iprGroups, NodeRef nodeRef)
     {
@@ -677,7 +700,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * Clear the nodes IPR permissions
      *
-     * @param nodeRef   node reference
+     * @param nodeRef
+     *            node reference
      */
     private void clearPermissions(NodeRef nodeRef, Pair<String, String> iprGroups)
     {
@@ -689,7 +713,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#getExtendedReaders(org.alfresco.service.cmr.repository.NodeRef)
      */
-    @Override @Deprecated public Set<String> getExtendedReaders(NodeRef nodeRef)
+    @Override
+    @Deprecated
+    public Set<String> getExtendedReaders(NodeRef nodeRef)
     {
         return getReaders(nodeRef);
     }
@@ -697,7 +723,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#getExtendedWriters(org.alfresco.service.cmr.repository.NodeRef)
      */
-    @Override @Deprecated public Set<String> getExtendedWriters(NodeRef nodeRef)
+    @Override
+    @Deprecated
+    public Set<String> getExtendedWriters(NodeRef nodeRef)
     {
         return getWriters(nodeRef);
     }
@@ -705,7 +733,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#addExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set)
      */
-    @Override @Deprecated public void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers)
+    @Override
+    @Deprecated
+    public void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers)
     {
         set(nodeRef, readers, writers);
     }
@@ -713,7 +743,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#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)
+    @Override
+    @Deprecated
+    public void addExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents)
     {
         set(nodeRef, readers, writers);
     }
@@ -721,7 +753,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef)
      */
-    @Override @Deprecated public void removeAllExtendedSecurity(NodeRef nodeRef)
+    @Override
+    @Deprecated
+    public void removeAllExtendedSecurity(NodeRef nodeRef)
     {
         remove(nodeRef);
     }
@@ -729,7 +763,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#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)
+    @Override
+    @Deprecated
+    public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers)
     {
         remove(nodeRef);
     }
@@ -737,7 +773,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, java.util.Set, java.util.Set, boolean)
      */
-    @Override @Deprecated public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String>writers, boolean applyToParents)
+    @Override
+    @Deprecated
+    public void removeExtendedSecurity(NodeRef nodeRef, Set<String> readers, Set<String> writers, boolean applyToParents)
     {
         remove(nodeRef);
     }
@@ -745,7 +783,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl
     /**
      * @see org.alfresco.module.org_alfresco_module_rm.security.DeprecatedExtendedSecurityService#removeAllExtendedSecurity(org.alfresco.service.cmr.repository.NodeRef, boolean)
      */
-    @Override @Deprecated public void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents)
+    @Override
+    @Deprecated
+    public void removeAllExtendedSecurity(NodeRef nodeRef, boolean applyToParents)
     {
         remove(nodeRef);
     }