From a3998de1ba2db222cc455061e6a6eeafed9de835 Mon Sep 17 00:00:00 2001 From: Eva Vasques Date: Mon, 28 Jul 2025 12:28:02 +0100 Subject: [PATCH] MNT-24975 - Repeated IPR groups due to casing inconsistencies (#3459) --- .../alfresco-global.properties | 5 + .../rm-service-context.xml | 1 + .../security/ExtendedSecurityServiceImpl.java | 81 ++++++++++++- .../ExtendedSecurityServiceImplUnitTest.java | 114 ++++++++++++++++-- 4 files changed, 191 insertions(+), 10 deletions(-) diff --git a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties index 13f938dc27..b936bba824 100644 --- a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties +++ b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/alfresco-global.properties @@ -119,6 +119,11 @@ rm.patch.v35.holdNewChildAssocPatch.batchSize=1000 rm.haspermissionmap.read=Read rm.haspermissionmap.write=WriteProperties,AddChildren,ReadContent +# Extended Permissions +# Enable matching the given username with the correct casing username when retrieving an IPR group. +# Only needs to be used if there are owners that don't have the username in the correct casing. +rm.extendedSecurity.enableUsernameNormalization=false + # # Extended auto-version behaviour. If true and other auto-version properties are satisfied, then # a document will be auto-versioned when its type is changed. diff --git a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml index dfc436146d..cd13c5ffbc 100644 --- a/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml +++ b/amps/ags/rm-community/rm-community-repo/config/alfresco/module/org_alfresco_module_rm/rm-service-context.xml @@ -611,6 +611,7 @@ + diff --git a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java index ff0d9f62b8..fefa403c5e 100644 --- a/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java +++ b/amps/ags/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java @@ -38,6 +38,7 @@ import org.springframework.context.ApplicationListener; import org.springframework.context.event.ContextRefreshedEvent; import org.springframework.extensions.webscripts.ui.common.StringUtils; +import org.alfresco.model.ContentModel; 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; @@ -96,6 +97,8 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl /** transaction service */ private TransactionService transactionService; + private boolean enableUsernameNormalization; + /** * @param filePlanService * file plan service @@ -141,6 +144,15 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl this.transactionService = transactionService; } + /** + * @param enableUsernameNormalization + * enable username normalization to ensure correct casing + */ + public void setEnableUsernameNormalization(boolean enableUsernameNormalization) + { + this.enableUsernameNormalization = enableUsernameNormalization; + } + /** * Application context refresh event handler */ @@ -392,14 +404,20 @@ 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 authoritySet = normalizeAuthorities(authorities); + // determine the short name prefix - String groupShortNamePrefix = getIPRGroupPrefixShortName(groupPrefix, authorities); + String groupShortNamePrefix = getIPRGroupPrefixShortName(groupPrefix, authoritySet); // iterate over the authorities to find a match while (hasMoreItems == true) { // get matching authorities - PagingResults results = authorityService.getAuthorities(AuthorityType.GROUP, + PagingResults results = authorityService.getAuthorities( + AuthorityType.GROUP, RMAuthority.ZONE_APP_RM, groupShortNamePrefix, false, @@ -413,7 +431,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl for (String group : results.getPage()) { // if exists and matches we have found our group - if (isIPRGroupTrueMatch(group, authorities)) + if (isIPRGroupTrueMatch(group, authoritySet)) { return new Pair(group, nextGroupIndex); } @@ -427,6 +445,63 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl return new Pair<>(iprGroup, nextGroupIndex); } + /** + * Given a set of authorities, normalizes the authority names to ensure correct casing. + * + * @param authNames + * @return + */ + private Set normalizeAuthorities(Set authNames) + { + // If disabled or no authorities, return as is + if (!enableUsernameNormalization || authNames == null || authNames.isEmpty()) + { + return authNames; + } + + Set normalizedAuthorities = new HashSet<>(); + for (String authorityName : authNames) + { + normalizedAuthorities.add(normalizeAuthorityName(authorityName)); + } + return normalizedAuthorities; + } + + /** + * Usernames are case insensitive but affect the IPR group matching when set with different casing. For a given authority of type user, this method normalizes the authority name. If group, it returns the name as-is. + * + * @param authorityName + * the authority name to normalize + * @return the normalized authority name + */ + private String normalizeAuthorityName(String authorityName) + { + if (authorityName == null || authorityName.startsWith(GROUP_PREFIX)) + { + return authorityName; + } + + // For users, attempt to get the correct casing from the username property of the user node + if (authorityService.authorityExists(authorityName)) + { + try + { + NodeRef authorityNodeRef = authorityService.getAuthorityNodeRef(authorityName); + if (authorityNodeRef != null) + { + String username = (String) nodeService.getProperty(authorityNodeRef, ContentModel.PROP_USERNAME); + return username != null ? username : authorityName; + } + } + catch (Exception e) + { + // If anything goes wrong, fallback to the original name + } + } + + return authorityName; + } + /** * Determines whether a group exactly matches a list of authorities. * diff --git a/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java b/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java index 213c7f9647..104fa80c24 100644 --- a/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java +++ b/amps/ags/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java @@ -52,6 +52,7 @@ import java.util.Set; import java.util.stream.Collectors; import java.util.stream.Stream; +import org.alfresco.model.ContentModel; 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; @@ -67,6 +68,7 @@ import org.alfresco.repo.transaction.RetryingTransactionHelper.RetryingTransacti 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.repository.StoreRef; import org.alfresco.service.cmr.security.AccessPermission; import org.alfresco.service.cmr.security.AccessStatus; import org.alfresco.service.cmr.security.AuthorityService; @@ -522,6 +524,104 @@ public class ExtendedSecurityServiceImplUnitTest verify(mockedPermissionService).setPermission(nodeRef, writeGroup, RMPermissionModel.FILING, true); } + + /** + * Given a node with no previous IPR groups assigned + * And having pre-existing IPR groups matching the ones we need + * When I add some read and write authorities but with a different casing + * Then the existing IPR groups are used + */ + @SuppressWarnings("unchecked") + @Test public void addExtendedSecurityWithMixedCasingUsernames() + { + // Have the usernames in the node as the correct usernames but with incorrect casing + String user1 = "UseR"; + String user2 = "UseR_w"; + + // Incorrect IPR Group names + Set diffCasingReaders = Stream.of(user1, GROUP).collect(Collectors.toSet()); + Set diffCasingWriters = Stream.of(user2, GROUP_W).collect(Collectors.toSet()); + String wrongReadGroupPrefix = extendedSecurityService.getIPRGroupPrefixShortName(READER_GROUP_PREFIX, diffCasingReaders); + String wrongWriteGroupPrefix = extendedSecurityService.getIPRGroupPrefixShortName(WRITER_GROUP_PREFIX, diffCasingWriters); + String wrongReadGroup = wrongReadGroupPrefix + "0"; + String wrongWriteGroup = wrongWriteGroupPrefix + "0"; + + // Correct Group names + String correctReadGroup = readGroupPrefix + "0"; + String correctWriteGroup = writeGroupPrefix + "0"; + + // If queried for the correct groups, return the results + PagingResults mockedCorrectReadPResults = mock(PagingResults.class); + PagingResults mockedCorrectWritePResults = mock(PagingResults.class); + when(mockedCorrectReadPResults.getPage()) + .thenReturn(Stream.of(GROUP_PREFIX + correctReadGroup).collect(Collectors.toList())); + when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(readGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedCorrectReadPResults); + + when(mockedCorrectWritePResults.getPage()) + .thenReturn(Stream.of(GROUP_PREFIX + correctWriteGroup).collect(Collectors.toList())); + when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(writeGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedCorrectWritePResults); + + // Don't return results for the incorrect groups (lenient as these may not be called with normalization enabled) + PagingResults mockedWrongReadPResults = mock(PagingResults.class); + PagingResults mockedWrongWritePResults = mock(PagingResults.class); + lenient().when(mockedWrongReadPResults.getPage()) + .thenReturn(Collections.emptyList()); + lenient().when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(wrongReadGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedWrongReadPResults); + + lenient().when(mockedWrongWritePResults.getPage()) + .thenReturn(Collections.emptyList()); + lenient().when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(wrongWriteGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedWrongWritePResults); + + // The users do exist, despite being in a different casing and are able to be retrieved + NodeRef noderefUser1 = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, USER); + when(mockedAuthorityService.authorityExists(user1)).thenReturn(true); + when(mockedAuthorityService.getAuthorityNodeRef(user1)).thenReturn(noderefUser1); + when(mockedNodeService.getProperty(noderefUser1, ContentModel.PROP_USERNAME)).thenReturn(USER); + + NodeRef noderefUser2 = new NodeRef(StoreRef.STORE_REF_WORKSPACE_SPACESSTORE, USER_W); + when(mockedAuthorityService.authorityExists(user2)).thenReturn(true); + when(mockedAuthorityService.getAuthorityNodeRef(user2)).thenReturn(noderefUser2); + when(mockedNodeService.getProperty(noderefUser2, ContentModel.PROP_USERNAME)).thenReturn(USER_W); + + // Set the extended security service to normalize usernames + extendedSecurityService.setEnableUsernameNormalization(true); + extendedSecurityService.set(nodeRef, diffCasingReaders, diffCasingWriters); + + // Verify that the incorrect read group is not created + verify(mockedAuthorityService, never()).createAuthority(AuthorityType.GROUP, wrongReadGroup, wrongReadGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); + + // Verify that the incorrect write group is not created + verify(mockedAuthorityService, never()).createAuthority(AuthorityType.GROUP, wrongWriteGroup, wrongWriteGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); + + } /** * Given a node with no previous IPR groups assigned @@ -571,7 +671,7 @@ public class ExtendedSecurityServiceImplUnitTest .thenReturn(Stream .of(USER_W, AlfMock.generateText()) .collect(Collectors.toSet())); - + // add extended security extendedSecurityService.set(nodeRef, READERS, WRITERS); @@ -895,7 +995,7 @@ public class ExtendedSecurityServiceImplUnitTest // group names String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_FULL_PREFIX, READERS, 0); String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_FULL_PREFIX, WRITERS, 0); - + // setup renditions NodeRef renditionNodeRef = AlfMock.generateNodeRef(mockedNodeService); when(mockedNodeService.hasAspect(nodeRef, RecordsManagementModel.ASPECT_RECORD)) @@ -904,7 +1004,7 @@ public class ExtendedSecurityServiceImplUnitTest .thenReturn(renditionNodeRef); when(mockedNodeService.getChildAssocs(nodeRef, RenditionModel.ASSOC_RENDITION, RegexQNamePattern.MATCH_ALL)) .thenReturn(Collections.singletonList(mockedChildAssociationRef)); - + // setup permissions Set permissions = Stream .of(new AccessPermissionImpl(AlfMock.generateText(), AccessStatus.ALLOWED, readGroup, 0), @@ -913,17 +1013,17 @@ public class ExtendedSecurityServiceImplUnitTest .collect(Collectors.toSet()); when(mockedPermissionService.getAllSetPermissions(nodeRef)) .thenReturn(permissions); - + // remove extended security extendedSecurityService.remove(nodeRef); - + // verify that the groups permissions have been removed verify(mockedPermissionService).clearPermission(nodeRef, readGroup); verify(mockedPermissionService).clearPermission(nodeRef, writeGroup); - + // verify that the groups permissions have been removed from the rendition verify(mockedPermissionService).clearPermission(renditionNodeRef, readGroup); verify(mockedPermissionService).clearPermission(renditionNodeRef, writeGroup); - + } } \ No newline at end of file