diff --git a/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java b/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java index 58085ef0d8..de29b8026a 100644 --- a/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/content/CollectionServiceImpl.java @@ -920,8 +920,7 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i int defaultRead) throws SQLException, AuthorizeException { Group role = groupService.create(context); - groupService.setName(role, "COLLECTION_" + collection.getID().toString() + "_" + typeOfGroupString + - "_DEFAULT_READ"); + groupService.setName(role, getDefaultReadGroupName(collection, typeOfGroupString)); // Remove existing privileges from the anonymous group. authorizeService.removePoliciesActionFilter(context, collection, defaultRead); @@ -932,6 +931,12 @@ public class CollectionServiceImpl extends DSpaceObjectServiceImpl i return role; } + @Override + public String getDefaultReadGroupName(Collection collection, String typeOfGroupString) { + return "COLLECTION_" + collection.getID().toString() + "_" + typeOfGroupString + + "_DEFAULT_READ"; + } + @Override public List findCollectionsWithSubmit(String q, Context context, Community community, int offset, int limit) throws SQLException, SearchServiceException { diff --git a/dspace-api/src/main/java/org/dspace/content/service/CollectionService.java b/dspace-api/src/main/java/org/dspace/content/service/CollectionService.java index 3a48065795..522bdac224 100644 --- a/dspace-api/src/main/java/org/dspace/content/service/CollectionService.java +++ b/dspace-api/src/main/java/org/dspace/content/service/CollectionService.java @@ -360,6 +360,16 @@ public interface CollectionService Group createDefaultReadGroup(Context context, Collection collection, String typeOfGroupString, int defaultRead) throws SQLException, AuthorizeException; + /** + * This method will return the name to give to the group created by the + * {@link #createDefaultReadGroup(Context, Collection, String, int)} method + * + * @param collection The DSpace collection to use in the name generation + * @param typeOfGroupString The type of group to use in the name generation + * @return the name to give to the group that hold default read for the collection + */ + String getDefaultReadGroupName(Collection collection, String typeOfGroupString); + /** * Returns Collections for which the current user has 'submit' privileges. * NOTE: for better performance, this method retrieves its results from an diff --git a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java index 4c98ed20c8..be81cd9bd8 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java @@ -15,6 +15,7 @@ import java.util.Iterator; import java.util.List; import java.util.Map; import java.util.Objects; +import java.util.Optional; import java.util.Set; import java.util.UUID; @@ -735,13 +736,24 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements groups.add(group); List policies = resourcePolicyService.find(context, null, groups, Constants.DEFAULT_ITEM_READ, Constants.COLLECTION); - if (policies.size() > 0) { - return policies.get(0).getdSpaceObject(); + + Optional defaultPolicy = policies.stream().filter(p -> StringUtils.equals( + collectionService.getDefaultReadGroupName((Collection) p.getdSpaceObject(), "ITEM"), + group.getName())).findFirst(); + + if (defaultPolicy.isPresent()) { + return defaultPolicy.get().getdSpaceObject(); } policies = resourcePolicyService.find(context, null, groups, Constants.DEFAULT_BITSTREAM_READ, Constants.COLLECTION); - if (policies.size() > 0) { - return policies.get(0).getdSpaceObject(); + + defaultPolicy = policies.stream() + .filter(p -> StringUtils.equals(collectionService.getDefaultReadGroupName( + (Collection) p.getdSpaceObject(), "BITSTREAM"), group.getName())) + .findFirst(); + + if (defaultPolicy.isPresent()) { + return defaultPolicy.get().getdSpaceObject(); } } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java index f0cee34e40..7121e11953 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/GroupRestRepositoryIT.java @@ -53,6 +53,7 @@ import org.dspace.builder.CollectionBuilder; import org.dspace.builder.CommunityBuilder; import org.dspace.builder.EPersonBuilder; import org.dspace.builder.GroupBuilder; +import org.dspace.builder.ResourcePolicyBuilder; import org.dspace.content.Collection; import org.dspace.content.Community; import org.dspace.content.factory.ContentServiceFactory; @@ -3025,4 +3026,164 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { } + @Test + public void commAdminAndColAdminCannotExploitItemReadGroupTest() throws Exception { + + GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + + context.turnOffAuthorisationSystem(); + + EPerson adminChild1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Oliver", "Rossi") + .withEmail("adminChild1@example.com") + .withPassword(password) + .build(); + EPerson adminCol1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("James", "Rossi") + .withEmail("adminCol1@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community") + .withAdminGroup(adminChild1) + .build(); + + Collection col1 = CollectionBuilder.createCollection(context, child1) + .withName("Collection 1") + .withAdminGroup(adminCol1) + .withSubmitterGroup(eperson) + .build(); + + Group adminGroup = groupService.findByName(context, Group.ADMIN); + ResourcePolicyBuilder.createResourcePolicy(context).withAction(Constants.DEFAULT_ITEM_READ) + .withGroup(adminGroup).withDspaceObject(child1).build(); + ResourcePolicyBuilder.createResourcePolicy(context).withAction(Constants.DEFAULT_ITEM_READ) + .withGroup(adminGroup).withDspaceObject(col1).build(); + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminChild1.getEmail(), password); + + assertFalse(groupService.isMember(context, adminChild1, adminGroup)); + assertFalse(groupService.isMember(context, adminCol1, adminGroup)); + + getClient(tokenAdminCol) + .perform(post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + adminCol1.getID())) + .andExpect(status().isForbidden()); + + assertFalse(groupService.isMember(context, adminCol1, adminGroup)); + + getClient(tokenAdminComm) + .perform(post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + adminChild1.getID())) + .andExpect(status().isForbidden()); + + assertFalse(groupService.isMember(context, adminChild1, adminGroup)); + + } + + @Test + public void commAdminAndColAdminCannotExpoloitBitstreamReadGroupTest() throws Exception { + + GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + + context.turnOffAuthorisationSystem(); + + EPerson adminChild1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Oliver", "Rossi") + .withEmail("adminChild1@example.com") + .withPassword(password) + .build(); + EPerson adminCol1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("James", "Rossi") + .withEmail("adminCol1@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community") + .withAdminGroup(adminChild1) + .build(); + + Collection col1 = CollectionBuilder.createCollection(context, child1) + .withName("Collection 1") + .withAdminGroup(adminCol1) + .withSubmitterGroup(eperson) + .build(); + + Group adminGroup = groupService.findByName(context, Group.ADMIN); + ResourcePolicyBuilder.createResourcePolicy(context).withAction(Constants.DEFAULT_BITSTREAM_READ) + .withGroup(adminGroup).withDspaceObject(child1).build(); + ResourcePolicyBuilder.createResourcePolicy(context).withAction(Constants.DEFAULT_BITSTREAM_READ) + .withGroup(adminGroup).withDspaceObject(col1).build(); + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminChild1.getEmail(), password); + + assertFalse(groupService.isMember(context, adminChild1, adminGroup)); + assertFalse(groupService.isMember(context, adminCol1, adminGroup)); + + getClient(tokenAdminCol) + .perform(post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + adminCol1.getID())) + .andExpect(status().isForbidden()); + + assertFalse(groupService.isMember(context, adminCol1, adminGroup)); + + getClient(tokenAdminComm) + .perform(post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + adminChild1.getID())) + .andExpect(status().isForbidden()); + + assertFalse(groupService.isMember(context, adminChild1, adminGroup)); + } + + @Test + /** + * Test for bug https://github.com/DSpace/DSpace/issues/7928 + * @throws Exception + */ + public void anonymousGroupParentObjectTest() throws Exception { + + GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + Group anonGroup = groupService.findByName(context, Group.ANONYMOUS); + context.turnOffAuthorisationSystem(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .build(); + + Collection col1 = CollectionBuilder.createCollection(context, parentCommunity) + .withName("Collection 1") + .build(); + context.restoreAuthSystemState(); + + String tokenAdmin = getAuthToken(admin.getEmail(), password); + + getClient(tokenAdmin).perform(get("/api/eperson/groups/" + anonGroup.getID().toString()) + .param("projection", "full")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$", GroupMatcher.matchFullEmbeds())) + .andExpect(jsonPath("$", GroupMatcher.matchLinks(anonGroup.getID()))) + .andExpect(jsonPath("$", Matchers.is( + GroupMatcher.matchGroupEntry(anonGroup.getID(), anonGroup.getName()) + ))) + .andExpect(jsonPath("$._embedded.object").doesNotExist()) + ; + } + }