diff --git a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java index b6d920021f..169c25b03b 100644 --- a/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java +++ b/rm-community/rm-community-repo/source/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImpl.java @@ -34,7 +34,6 @@ import java.util.HashSet; import java.util.List; 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; @@ -280,19 +279,23 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl String iprReaderGroup = null; String iprWriterGroup = null; + // get all the set permissions Set permissions = permissionService.getAllSetPermissions(nodeRef); for (AccessPermission permission : permissions) { + // look for the presence of the reader group if (permission.getAuthority().startsWith(GROUP_PREFIX + READER_GROUP_PREFIX)) { iprReaderGroup = permission.getAuthority(); } + // look for the presence of the writer group else if (permission.getAuthority().startsWith(GROUP_PREFIX + WRITER_GROUP_PREFIX)) { iprWriterGroup = permission.getAuthority(); } } + // assuming the are both present then return if (iprReaderGroup != null && iprWriterGroup != null) { result = new Pair(iprReaderGroup, iprWriterGroup); @@ -318,40 +321,49 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl */ private Pair createOrFindIPRGroups(Set readers, Set writers) { - Pair result = null; - - // find read group or determine what the next index is if no group exists or there is a clash - Pair readGroupResult = findIPRGroup(READER_GROUP_PREFIX, readers, writers); - - if (readGroupResult.getFirst() == null) - { - // create inplace record reader and writer groups - result = createIPRGroups(readers, writers, readGroupResult.getSecond()); - } - else - { - // set result - result = new Pair(readGroupResult.getFirst(), - getIRPWriteGroupNameFromReadGroupName(readGroupResult.getFirst(), readers, writers)); - } - - return result; + return new Pair( + createOrFindIPRGroup(READER_GROUP_PREFIX, readers), + createOrFindIPRGroup(WRITER_GROUP_PREFIX, writers)); } /** - * Give a group name prefix and the read/write authorities, finds the exact match existing read group - * (containing the exact match write group). + * Create or find an IPR group based on the provided prefix and authorities. + * + * @param groupPrefix group prefix + * @param authorities authorities + * @return String full group name + */ + private String createOrFindIPRGroup(String groupPrefix, Set authorities) + { + String group = null; + + // find group or determine what the next index is if no group exists or there is a clash + Pair groupResult = findIPRGroup(groupPrefix, authorities); + + if (groupResult.getFirst() == null) + { + group = createIPRGroup(groupPrefix, authorities, groupResult.getSecond()); + } + else + { + group = groupResult.getFirst(); + } + + return group; + } + + /** + * Given a group name prefix and the authorities, finds the exact match existing group. *

* If the group does not exist then the group returned is null and the index shows the next available * group index for creation. * * @param groupPrefix group name prefix - * @param readers authorities with read - * @param writers authorities with write + * @param authorities authorities * @return Pair where first is the name of the found group, null if none found and second * if the next available create index */ - private Pair findIPRGroup(String groupPrefix, Set readers, Set writers) + private Pair findIPRGroup(String groupPrefix, Set authorities) { String iprGroup = null; int nextGroupIndex = 0; @@ -359,7 +371,7 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl int pageCount = 0; // determine the short name prefix - String groupShortNamePrefix = getIPRGroupPrefixShortName(groupPrefix, readers, writers); + String groupShortNamePrefix = getIPRGroupPrefixShortName(groupPrefix, authorities); // iterate over the authorities to find a match while (hasMoreItems == true) @@ -376,22 +388,12 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl nextGroupIndex = nextGroupIndex + results.getPage().size(); // see if any of the matching groups exactly match - for (String readGroup : results.getPage()) + for (String group : results.getPage()) { - // get the corresponding write group name - String writeGroup = getIRPWriteGroupNameFromReadGroupName(readGroup, readers, writers); - - // check for existence - if (!authorityService.authorityExists(writeGroup)) + // if exists and matches we have found our group + if (isIPRGroupTrueMatch(group, authorities)) { - throw new AlfrescoRuntimeException("Missing inplace writer group for reader group " + readGroup); - } - - // if exists and matches we have found our groups - if (isIPRGroupTrueMatch(readGroup, readers, writeGroup) && - isIPRGroupTrueMatch(writeGroup, writers, null)) - { - iprGroup = readGroup; + iprGroup = group; break; } } @@ -409,33 +411,28 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl * * @param authorities list of authorities * @param group group - * @param excludeAuthority authority to exclude from comparision * @return */ - private boolean isIPRGroupTrueMatch(String group, Set authorities, String excludeAuthority) - { - Set contained = authorityService.getContainedAuthorities(null, group, true); - if (excludeAuthority != null) - { - contained.remove(excludeAuthority); - } + private boolean isIPRGroupTrueMatch(String group, Set authorities) + { + Set contained = authorityService.getContainedAuthorities(null, group, true); return contained.equals(authorities); } /** * Get IPR group prefix short name. + *

+ * 'package' scope to help testing. * * @param prefix prefix - * @param authorities read authorities - * @param shortName write authorities + * @param authorities authorities * @return String group prefix short name */ - private String getIPRGroupPrefixShortName(String prefix, Set readers, Set writers) + /*package*/ String getIPRGroupPrefixShortName(String prefix, Set authorities) { StringBuilder builder = new StringBuilder(128) .append(prefix) - .append(getAuthoritySetHashCode(readers)) - .append(getAuthoritySetHashCode(writers)); + .append(getAuthoritySetHashCode(authorities)); return builder.toString(); } @@ -453,9 +450,9 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl * @param index group index * @return String group short name */ - /*package*/ String getIPRGroupShortName(String prefix, Set readers, Set writers, int index) + /*package*/ String getIPRGroupShortName(String prefix, Set authorities, int index) { - return getIPRGroupShortName(prefix, readers, writers, Integer.toString(index)); + return getIPRGroupShortName(prefix, authorities, Integer.toString(index)); } /** @@ -469,36 +466,15 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl * @param index group index * @return String group short name */ - private String getIPRGroupShortName(String prefix, Set readers, Set writers, String index) + private String getIPRGroupShortName(String prefix, Set authorities, String index) { StringBuilder builder = new StringBuilder(128) - .append(prefix) - .append(getAuthoritySetHashCode(readers)) - .append(getAuthoritySetHashCode(writers)) + .append(getIPRGroupPrefixShortName(prefix, authorities)) .append(index); return builder.toString(); } - /** - * Get the IPR write group name from the read group name. - *

- * Note this doesn't test for existence of the group, instead determines the name based on the index and - * authorities. - *

- * Note this excludes the "GROUP_" prefix - * - * @param readGroupShortName read group short name - * @param readers read authorities - * @param writers write authorities - * @return String write group name - */ - private String getIRPWriteGroupNameFromReadGroupName(String readGroupnName, Set readers, Set writers) - { - String index = readGroupnName.substring(readGroupnName.length() - 1); - return PermissionService.GROUP_PREFIX + getIPRGroupShortName(WRITER_GROUP_PREFIX, readers, writers, index); - } - /** * Gets the hashcode value of a set of authorities. * @@ -515,41 +491,26 @@ public class ExtendedSecurityServiceImpl extends ServiceBaseImpl return result; } - /** - * Creates new IPR groups and assigns then to the correct RM roles. - * - * @param readers read authorities - * @param writers write authorities - * @param index index - * @return Pair where first if the full read group name and second is the full write group name - */ - private Pair createIPRGroups(Set readers, Set writers, int index) - { - String iprReaderGroup = createIPRGroup(getIPRGroupShortName(READER_GROUP_PREFIX, readers, writers, index), getRootIRPGroup(), readers); - String iprWriterGroup = createIPRGroup(getIPRGroupShortName(WRITER_GROUP_PREFIX, readers, writers, index), iprReaderGroup, writers); - return new Pair(iprReaderGroup, iprWriterGroup); - } - /** * Creates a new IPR group. * - * @param groupShortName group short name - * @param parent parent group, null if none + * @param groupNamePrefix group name prefix * @param children child authorities + * @param index group index * @return String full name of created group */ - private String createIPRGroup(String groupShortName, String parent, Set children) + private String createIPRGroup(String groupNamePrefix, Set children, int index) { - ParameterCheck.mandatory("groupShortName", groupShortName); + ParameterCheck.mandatory("groupNamePrefix", groupNamePrefix); + + // get the group name + String groupShortName = getIPRGroupShortName(groupNamePrefix, children, index); // create group String group = authorityService.createAuthority(AuthorityType.GROUP, groupShortName, groupShortName, Collections.singleton(RMAuthority.ZONE_APP_RM)); - // add parent if provided - if (parent != null) - { - authorityService.addAuthority(parent, group); - } + // add root parent + authorityService.addAuthority(getRootIRPGroup(), group); // add children if provided if (children != null) diff --git a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java index e7c125233d..4d782c1fb8 100644 --- a/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java +++ b/rm-community/rm-community-repo/unit-test/java/org/alfresco/module/org_alfresco_module_rm/security/ExtendedSecurityServiceImplUnitTest.java @@ -94,7 +94,8 @@ public class ExtendedSecurityServiceImplUnitTest @Mock private TransactionService mockedTransactionService; @Mock private RetryingTransactionHelper mockedRetryingTransactionHelper; @Mock private NodeService mockedNodeService; - @Mock private PagingResults mockedPagingResults; + @Mock private PagingResults mockedReadPagingResults; + @Mock private PagingResults mockedWritePagingResults; @Mock private ApplicationContext mockedApplicationContext; /** test component */ @@ -105,10 +106,10 @@ public class ExtendedSecurityServiceImplUnitTest private static final String WRITER_GROUP_FULL_PREFIX = GROUP_PREFIX + WRITER_GROUP_PREFIX; /** test authorities */ - private static final String USER = AlfMock.generateText(); - private static final String GROUP = GROUP_PREFIX + AlfMock.generateText(); - private static final String USER_W = AlfMock.generateText(); - private static final String GROUP_W = GROUP_PREFIX + AlfMock.generateText(); + private static final String USER = "USER"; + private static final String GROUP = GROUP_PREFIX + "GROUP"; + private static final String USER_W = "USER_W"; + private static final String GROUP_W = GROUP_PREFIX + "GROUP_W"; private static final Set READERS = Stream.of(USER, GROUP).collect(Collectors.toSet()); private static final Set WRITERS = Stream.of(USER_W, GROUP_W).collect(Collectors.toSet()); @@ -129,6 +130,8 @@ public class ExtendedSecurityServiceImplUnitTest /** test data */ private NodeRef nodeRef; private NodeRef filePlan; + private String readGroupPrefix; + private String writeGroupPrefix; /** * Before tests @@ -179,6 +182,10 @@ public class ExtendedSecurityServiceImplUnitTest }; when(mockedAuthorityService.createAuthority(any(AuthorityType.class), anyString(), anyString(), anySet())) .thenAnswer(createAuthorityAnswer); + + // setup group prefixes + readGroupPrefix = extendedSecurityService.getIPRGroupPrefixShortName(READER_GROUP_PREFIX, READERS); + writeGroupPrefix = extendedSecurityService.getIPRGroupPrefixShortName(WRITER_GROUP_PREFIX, WRITERS); } /** @@ -318,7 +325,7 @@ public class ExtendedSecurityServiceImplUnitTest when(mockedAuthorityService.getContainedAuthorities(null, READER_GROUP_FULL_PREFIX, true)) .thenReturn(Stream - .of(USER, GROUP, WRITER_GROUP_FULL_PREFIX) + .of(USER, GROUP) .collect(Collectors.toSet())); // get extended readers @@ -379,11 +386,11 @@ public class ExtendedSecurityServiceImplUnitTest @Test public void addExtendedSecurityForTheFirstTimeAndCreateGroups() { // group names - String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, WRITERS, 0); - String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, READERS, WRITERS, 0); + String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, 0); + String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, WRITERS, 0); // setup query results - when(mockedPagingResults.getPage()) + when(mockedReadPagingResults.getPage()) .thenReturn(Collections.emptyList()); when(mockedAuthorityService.getAuthorities( eq(AuthorityType.GROUP), @@ -392,7 +399,7 @@ public class ExtendedSecurityServiceImplUnitTest eq(false), eq(false), any(PagingRequest.class))) - .thenReturn(mockedPagingResults); + .thenReturn(mockedReadPagingResults); // add extended security extendedSecurityService.addExtendedSecurity(nodeRef, READERS, WRITERS); @@ -407,7 +414,7 @@ public class ExtendedSecurityServiceImplUnitTest // verify write group created correctly verify(mockedAuthorityService).createAuthority(AuthorityType.GROUP, writeGroup, writeGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); writeGroup = GROUP_PREFIX + writeGroup; - verify(mockedAuthorityService).addAuthority(readGroup, writeGroup); + verify(mockedAuthorityService).addAuthority(GROUP_PREFIX + ROOT_IPR_GROUP, writeGroup); verify(mockedAuthorityService).addAuthority(writeGroup, USER_W); verify(mockedAuthorityService).addAuthority(writeGroup, GROUP_W); @@ -430,28 +437,39 @@ public class ExtendedSecurityServiceImplUnitTest */ @Test public void addExtendedSecurityForTheFirstTimeAndReuseGroups() { - // group names - String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, WRITERS, 0); - String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, READERS, WRITERS, 0); + // group names + String readGroup = readGroupPrefix + "0"; + String writeGroup = writeGroupPrefix + "0"; // setup query results - when(mockedPagingResults.getPage()) + when(mockedReadPagingResults.getPage()) .thenReturn(Stream.of(GROUP_PREFIX + readGroup).collect(Collectors.toList())); when(mockedAuthorityService.getAuthorities( eq(AuthorityType.GROUP), eq(RMAuthority.ZONE_APP_RM), - any(String.class), + eq(readGroupPrefix), eq(false), eq(false), any(PagingRequest.class))) - .thenReturn(mockedPagingResults); + .thenReturn(mockedReadPagingResults); + + when(mockedWritePagingResults.getPage()) + .thenReturn(Stream.of(GROUP_PREFIX + writeGroup).collect(Collectors.toList())); + when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(writeGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedWritePagingResults); // setup exact match when(mockedAuthorityService.authorityExists(GROUP_PREFIX + writeGroup)) .thenReturn(true); when(mockedAuthorityService.getContainedAuthorities(null, GROUP_PREFIX + readGroup, true)) .thenReturn(Stream - .of(GROUP_PREFIX + writeGroup, USER, GROUP) + .of(USER, GROUP) .collect(Collectors.toSet())); when(mockedAuthorityService.getContainedAuthorities(null, GROUP_PREFIX + writeGroup, true)) .thenReturn(Stream @@ -495,28 +513,39 @@ public class ExtendedSecurityServiceImplUnitTest */ @Test public void addExtendedSecurityForTheFirstTimeAndCreateGroupsAfterClash() { - // group names - String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, WRITERS, 0); - String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, READERS, WRITERS, 0); + // group names + String readGroup = readGroupPrefix + "0"; + String writeGroup = writeGroupPrefix + "0"; // setup query results - when(mockedPagingResults.getPage()) + when(mockedReadPagingResults.getPage()) .thenReturn(Stream.of(GROUP_PREFIX + readGroup).collect(Collectors.toList())); when(mockedAuthorityService.getAuthorities( eq(AuthorityType.GROUP), eq(RMAuthority.ZONE_APP_RM), - any(String.class), + eq(readGroupPrefix), eq(false), eq(false), any(PagingRequest.class))) - .thenReturn(mockedPagingResults); + .thenReturn(mockedReadPagingResults); + + when(mockedWritePagingResults.getPage()) + .thenReturn(Stream.of(GROUP_PREFIX + writeGroup).collect(Collectors.toList())); + when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(writeGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedWritePagingResults); // setup exact match when(mockedAuthorityService.authorityExists(GROUP_PREFIX + writeGroup)) .thenReturn(true); when(mockedAuthorityService.getContainedAuthorities(null, GROUP_PREFIX + readGroup, true)) .thenReturn(Stream - .of(GROUP_PREFIX + writeGroup, USER, GROUP) + .of(USER, GROUP, AlfMock.generateText()) .collect(Collectors.toSet())); when(mockedAuthorityService.getContainedAuthorities(null, GROUP_PREFIX + writeGroup, true)) .thenReturn(Stream @@ -527,8 +556,8 @@ public class ExtendedSecurityServiceImplUnitTest extendedSecurityService.addExtendedSecurity(nodeRef, READERS, WRITERS); // new group names - readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, WRITERS, 1); - writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, READERS, WRITERS, 1); + readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, 1); + writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, WRITERS, 1); // verify read group created correctly verify(mockedAuthorityService).createAuthority(AuthorityType.GROUP, readGroup, readGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); @@ -540,7 +569,7 @@ public class ExtendedSecurityServiceImplUnitTest // verify write group created correctly verify(mockedAuthorityService).createAuthority(AuthorityType.GROUP, writeGroup, writeGroup, Collections.singleton(RMAuthority.ZONE_APP_RM)); writeGroup = GROUP_PREFIX + writeGroup; - verify(mockedAuthorityService).addAuthority(readGroup, writeGroup); + verify(mockedAuthorityService).addAuthority(GROUP_PREFIX + ROOT_IPR_GROUP, writeGroup); verify(mockedAuthorityService).addAuthority(writeGroup, USER_W); verify(mockedAuthorityService).addAuthority(writeGroup, GROUP_W); @@ -563,35 +592,48 @@ public class ExtendedSecurityServiceImplUnitTest */ @Test public void addExtendedSecurityWithResultPaging() { - // group names - String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_PREFIX, READERS, WRITERS, 0); - String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_PREFIX, READERS, WRITERS, 0); + // group names + String readGroup = readGroupPrefix + "0"; + String writeGroup = writeGroupPrefix + "0"; + // create fity results List fiftyResults = new ArrayList(50); for (int i = 0; i < 50; i++) { fiftyResults.add(AlfMock.generateText()); - } + } // setup query results - when(mockedPagingResults.getPage()) + when(mockedReadPagingResults.getPage()) .thenReturn(fiftyResults) .thenReturn(Stream.of(GROUP_PREFIX + readGroup).collect(Collectors.toList())); when(mockedAuthorityService.getAuthorities( eq(AuthorityType.GROUP), eq(RMAuthority.ZONE_APP_RM), - any(String.class), + eq(readGroupPrefix), eq(false), eq(false), any(PagingRequest.class))) - .thenReturn(mockedPagingResults); + .thenReturn(mockedReadPagingResults); + when(mockedWritePagingResults.getPage()) + .thenReturn(fiftyResults) + .thenReturn(Stream.of(GROUP_PREFIX + writeGroup).collect(Collectors.toList())); + when(mockedAuthorityService.getAuthorities( + eq(AuthorityType.GROUP), + eq(RMAuthority.ZONE_APP_RM), + eq(writeGroupPrefix), + eq(false), + eq(false), + any(PagingRequest.class))) + .thenReturn(mockedWritePagingResults); + // setup exact match when(mockedAuthorityService.authorityExists(GROUP_PREFIX + writeGroup)) .thenReturn(true); when(mockedAuthorityService.getContainedAuthorities(null, GROUP_PREFIX + readGroup, true)) .thenReturn(Stream - .of(GROUP_PREFIX + writeGroup, USER, GROUP) + .of(USER, GROUP) .collect(Collectors.toSet())); when(mockedAuthorityService.getContainedAuthorities(null, GROUP_PREFIX + writeGroup, true)) .thenReturn(Stream @@ -647,8 +689,8 @@ public class ExtendedSecurityServiceImplUnitTest @Test public void removeAllExtendedSecurity() { // group names - String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_FULL_PREFIX, READERS, WRITERS, 0); - String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_FULL_PREFIX, READERS, WRITERS, 0); + String readGroup = extendedSecurityService.getIPRGroupShortName(READER_GROUP_FULL_PREFIX, READERS, 0); + String writeGroup = extendedSecurityService.getIPRGroupShortName(WRITER_GROUP_FULL_PREFIX, WRITERS, 0); // setup permissions Set permissions = Stream