From ee120b415f8de2a478ebea50e0795a7352d6ba10 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Wed, 27 May 2020 10:41:28 +0200 Subject: [PATCH 01/24] [Task 71143] initial implementation of the preauthorize annotations in the converterservice --- .../app/rest/converter/ConverterService.java | 76 +++++++++++++++++-- .../ExternalSourceHalLinkFactory.java | 1 - .../rest/repository/ItemRestRepository.java | 1 - 3 files changed, 71 insertions(+), 7 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java index 59ee666cfe..26eae66e65 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java @@ -7,8 +7,10 @@ */ package org.dspace.app.rest.converter; +import java.lang.annotation.Annotation; import java.lang.reflect.Constructor; import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; import java.util.HashMap; import java.util.LinkedList; import java.util.List; @@ -17,6 +19,7 @@ import java.util.Set; import javax.annotation.Nullable; import javax.annotation.PostConstruct; +import org.apache.commons.lang3.StringUtils; import org.apache.log4j.Logger; import org.dspace.app.rest.link.HalLinkFactory; import org.dspace.app.rest.link.HalLinkService; @@ -26,17 +29,20 @@ import org.dspace.app.rest.model.RestModel; import org.dspace.app.rest.model.hateoas.HALResource; import org.dspace.app.rest.projection.DefaultProjection; import org.dspace.app.rest.projection.Projection; +import org.dspace.app.rest.repository.DSpaceRestRepository; import org.dspace.app.rest.security.DSpacePermissionEvaluator; import org.dspace.app.rest.utils.Utils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider; +import org.springframework.core.annotation.AnnotationUtils; import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.hateoas.EntityModel; import org.springframework.hateoas.Link; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Component; import org.springframework.stereotype.Service; @@ -94,11 +100,19 @@ public class ConverterService { DSpaceConverter converter = requireConverter(modelObject.getClass()); R restObject = converter.convert(transformedModel, projection); if (restObject instanceof BaseObjectRest) { - if (!dSpacePermissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), - restObject, "READ")) { - log.debug("Access denied on " + restObject.getClass() + " with id: " + - ((BaseObjectRest) restObject).getId()); - return null; + String permission = getPermissionForRestObject((BaseObjectRest) restObject); + if (!StringUtils.equalsIgnoreCase(permission, "permitAll")) { + if (StringUtils.equalsIgnoreCase(permission, "admin")) { + //TODO + } else if (StringUtils.equalsIgnoreCase(permission, "authenticated")) { + //TODO + } else { + if (!dSpacePermissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), + restObject, permission)) { + log.info("Access denied on " + restObject.getClass()); + return null; + } + } } } if (restObject instanceof RestModel) { @@ -107,6 +121,58 @@ public class ConverterService { return restObject; } + private String getPermissionForRestObject(BaseObjectRest restObject) { + Annotation preAuthorize = getAnnotationForRestObject(restObject); + if (preAuthorize == null) { + preAuthorize = getDefaultFindOnePreAuthorize(); + + } + String permission = "READ"; + permission = parseAnnotation(preAuthorize); + return permission; + } + + private String parseAnnotation(Annotation preAuthorize) { + String permission = ""; + if (preAuthorize != null) { + String annotationValue = (String) AnnotationUtils.getValue(preAuthorize); + if (StringUtils.contains(annotationValue, "permitAll")) { + permission = "permitAll"; + } else if (StringUtils.contains(annotationValue, "hasAuthority")) { + permission = StringUtils.substringBetween(annotationValue, "hasAuthority('", "')"); + } else if (StringUtils.contains(annotationValue,"hasPermission")) { + permission = StringUtils.split(annotationValue, ",")[2]; + permission = StringUtils.substringBetween(permission, "'"); + } + } + return permission; + } + + private Annotation getAnnotationForRestObject(BaseObjectRest restObject) { + BaseObjectRest baseObjectRest = restObject; + DSpaceRestRepository repositoryToUse = utils + .getResourceRepositoryByCategoryAndModel(baseObjectRest.getCategory(), baseObjectRest.getType()); + Annotation preAuthorize = null; + for (Method m : repositoryToUse.getClass().getMethods()) { + if (StringUtils.equalsIgnoreCase(m.getName(), "findOne")) { + preAuthorize = AnnotationUtils.findAnnotation(m, PreAuthorize.class); + } + } + return preAuthorize; + } + + private Annotation getDefaultFindOnePreAuthorize() { + for (Method m : DSpaceRestRepository.class.getMethods()) { + if (StringUtils.equalsIgnoreCase(m.getName(), "findOne")) { + Annotation annotation = AnnotationUtils.findAnnotation(m, PreAuthorize.class); + if (annotation != null) { + return annotation; + } + } + } + return null; + } + /** * Converts a list of model objects to a page of rest objects using the given {@link Projection}. * diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java index e192c95404..7931593a31 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java @@ -26,7 +26,6 @@ public class ExternalSourceHalLinkFactory extends @Override protected void addLinks(ExternalSourceResource halResource, Pageable pageable, LinkedList list) throws Exception { - list.add(buildLink("entries", getMethodOn() .getExternalSourceEntries(halResource.getContent().getName(), "", null, null, null))); diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java index c6643496ae..a3128e0afe 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java @@ -99,7 +99,6 @@ public class ItemRestRepository extends DSpaceObjectRestRepository Date: Wed, 27 May 2020 10:43:31 +0200 Subject: [PATCH 02/24] undo unnecessary changes --- .../rest/link/externalsources/ExternalSourceHalLinkFactory.java | 1 + .../java/org/dspace/app/rest/repository/ItemRestRepository.java | 1 + 2 files changed, 2 insertions(+) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java index 7931593a31..e192c95404 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/link/externalsources/ExternalSourceHalLinkFactory.java @@ -26,6 +26,7 @@ public class ExternalSourceHalLinkFactory extends @Override protected void addLinks(ExternalSourceResource halResource, Pageable pageable, LinkedList list) throws Exception { + list.add(buildLink("entries", getMethodOn() .getExternalSourceEntries(halResource.getContent().getName(), "", null, null, null))); diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java index a3128e0afe..49b468e298 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/ItemRestRepository.java @@ -99,6 +99,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository Date: Wed, 27 May 2020 11:21:08 +0200 Subject: [PATCH 03/24] added implementations for support for Collection/Community role-based groups for Community/Collection Admins --- .../org/dspace/app/util/AuthorizeUtil.java | 34 +++++++++++++++++++ .../authorize/AuthorizeServiceImpl.java | 6 ++++ .../authorize/service/AuthorizeService.java | 20 +++++++++++ .../repository/EPersonRestRepository.java | 2 +- .../rest/repository/GroupRestRepository.java | 2 +- .../EPersonRestAuthenticationProvider.java | 10 ++++++ .../EPersonRestPermissionEvaluatorPlugin.java | 11 ++++-- .../GroupRestPermissionEvaluatorPlugin.java | 16 ++++++++- 8 files changed, 95 insertions(+), 6 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java b/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java index 39fb713970..6b5d7f9003 100644 --- a/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java +++ b/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java @@ -601,4 +601,38 @@ public class AuthorizeUtil { throw new AuthorizeException("not authorized to manage this group"); } + + /** + * This method checks if the community Admin can manage accounts + * + * @return true if is able + */ + public static boolean canCommunityAdminManageAccounts() { + boolean isAble = false; + if (AuthorizeConfiguration.canCommunityAdminManagePolicies() + || AuthorizeConfiguration.canCommunityAdminManageAdminGroup() + || AuthorizeConfiguration.canCommunityAdminManageCollectionPolicies() + || AuthorizeConfiguration.canCommunityAdminManageCollectionSubmitters() + || AuthorizeConfiguration.canCommunityAdminManageCollectionWorkflows() + || AuthorizeConfiguration.canCommunityAdminManageCollectionAdminGroup()) { + isAble = true; + } + return isAble; + } + + /** + * This method checks if the Collection Admin can manage accounts + * + * @return true if is able + */ + public static boolean canCollectionAdminManageAccounts() { + boolean isAble = false; + if (AuthorizeConfiguration.canCollectionAdminManagePolicies() + || AuthorizeConfiguration.canCollectionAdminManageSubmitters() + || AuthorizeConfiguration.canCollectionAdminManageWorkflows() + || AuthorizeConfiguration.canCollectionAdminManageAdminGroup()) { + isAble = true; + } + return isAble; + } } diff --git a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java index 2384a260da..2ebecf2005 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java @@ -430,7 +430,10 @@ public class AuthorizeServiceImpl implements AuthorizeService { public boolean isCommunityAdmin(Context c) throws SQLException { EPerson e = c.getCurrentUser(); + return isCommunityAdmin(c, e); + } + public boolean isCommunityAdmin(Context c, EPerson e) throws SQLException { if (e != null) { List policies = resourcePolicyService.find(c, e, groupService.allMemberGroups(c, e), @@ -446,7 +449,10 @@ public class AuthorizeServiceImpl implements AuthorizeService { public boolean isCollectionAdmin(Context c) throws SQLException { EPerson e = c.getCurrentUser(); + return isCollectionAdmin(c, e); + } + public boolean isCollectionAdmin(Context c, EPerson e) throws SQLException { if (e != null) { List policies = resourcePolicyService.find(c, e, groupService.allMemberGroups(c, e), diff --git a/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java b/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java index 9e739e2585..f3ede72ac1 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java +++ b/dspace-api/src/main/java/org/dspace/authorize/service/AuthorizeService.java @@ -213,6 +213,26 @@ public interface AuthorizeService { public boolean isCollectionAdmin(Context c) throws SQLException; + /** + * Check to see if a specific user is Community admin + * + * @param c current context + * @param e the user to check + * @return true if user is an admin of some community + * @throws SQLException + */ + public boolean isCommunityAdmin(Context c, EPerson e) throws SQLException; + + /** + * Check to see if a specific user is Collection admin + * + * @param c current context + * @param e the user to check + * @return true if user is an admin of some collection + * @throws SQLException if database error + */ + public boolean isCollectionAdmin(Context c, EPerson e) throws SQLException; + /////////////////////////////////////////////// // policy manipulation methods /////////////////////////////////////////////// diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java index 073d1b25bd..aed31fd6ce 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java @@ -147,7 +147,7 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository findByMetadata(@Parameter(value = "query", required = true) String query, Pageable pageable) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java index 8310533597..f150ec0f3c 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java @@ -131,7 +131,7 @@ public class GroupRestRepository extends DSpaceObjectRestRepository findByMetadata(@Parameter(value = "query", required = true) String query, Pageable pageable) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java index 7cfd451045..576c7e7e7d 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java @@ -18,6 +18,7 @@ import javax.servlet.http.HttpServletRequest; import org.apache.commons.lang3.StringUtils; import org.dspace.app.rest.utils.ContextUtil; +import org.dspace.app.util.AuthorizeUtil; import org.dspace.authenticate.AuthenticationMethod; import org.dspace.authenticate.service.AuthenticationService; import org.dspace.authorize.service.AuthorizeService; @@ -47,6 +48,8 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider private static final Logger log = LoggerFactory.getLogger(EPersonRestAuthenticationProvider.class); + public static final String ACCOUNT_ADMIN_GRANT = "ACCOUNT_ADMIN"; + @Autowired private AuthenticationService authenticationService; @@ -140,14 +143,21 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider if (eperson != null) { boolean isAdmin = false; + boolean isCommunityAdmin = false; + boolean isColectionAdmin = false; try { isAdmin = authorizeService.isAdmin(context, eperson); + isCommunityAdmin = authorizeService.isCommunityAdmin(context, eperson); + isColectionAdmin = authorizeService.isCollectionAdmin(context, eperson); } catch (SQLException e) { log.error("SQL error while checking for admin rights", e); } if (isAdmin) { authorities.add(new SimpleGrantedAuthority(ADMIN_GRANT)); + } else if ((isCommunityAdmin && AuthorizeUtil.canCommunityAdminManageAccounts()) + || (isColectionAdmin && AuthorizeUtil.canCollectionAdminManageAccounts())) { + authorities.add(new SimpleGrantedAuthority(ACCOUNT_ADMIN_GRANT)); } authorities.add(new SimpleGrantedAuthority(AUTHENTICATED_GRANT)); diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java index ca13277b04..00c2c60cb2 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java @@ -17,6 +17,7 @@ import org.dspace.app.rest.model.patch.Patch; import org.dspace.app.rest.repository.patch.operation.DSpaceObjectMetadataPatchUtils; import org.dspace.app.rest.repository.patch.operation.EPersonPasswordReplaceOperation; import org.dspace.app.rest.utils.ContextUtil; +import org.dspace.app.util.AuthorizeUtil; import org.dspace.authorize.service.AuthorizeService; import org.dspace.core.Constants; import org.dspace.core.Context; @@ -74,9 +75,13 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv // anonymous user if (ePerson == null) { return false; - } - - if (dsoId.equals(ePerson.getID())) { + } else if (dsoId.equals(ePerson.getID())) { + return true; + } else if (authorizeService.isCommunityAdmin(context, ePerson) + && AuthorizeUtil.canCommunityAdminManageAccounts()) { + return true; + } else if (authorizeService.isCollectionAdmin(context, ePerson) + && AuthorizeUtil.canCollectionAdminManageAccounts()) { return true; } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java index 91cc302aa1..7f793c3b8d 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java @@ -12,6 +12,8 @@ import java.sql.SQLException; import java.util.UUID; import org.dspace.app.rest.utils.ContextUtil; +import org.dspace.app.util.AuthorizeUtil; +import org.dspace.authorize.service.AuthorizeService; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.eperson.EPerson; @@ -44,6 +46,9 @@ public class GroupRestPermissionEvaluatorPlugin extends RestObjectPermissionEval @Autowired private EPersonService ePersonService; + @Autowired + AuthorizeService authorizeService; + @Override public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, DSpaceRestPermission permission) { @@ -64,7 +69,16 @@ public class GroupRestPermissionEvaluatorPlugin extends RestObjectPermissionEval Group group = groupService.find(context, dsoId); - if (groupService.isMember(context, ePerson, group)) { + // anonymous user + if (ePerson == null) { + return false; + } else if (groupService.isMember(context, ePerson, group)) { + return true; + } else if (authorizeService.isCommunityAdmin(context, ePerson) + && AuthorizeUtil.canCommunityAdminManageAccounts()) { + return true; + } else if (authorizeService.isCollectionAdmin(context, ePerson) + && AuthorizeUtil.canCollectionAdminManageAccounts()) { return true; } From 141284e7fab5d74fad0f200f61759b6f61031d9b Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Wed, 27 May 2020 11:25:01 +0200 Subject: [PATCH 04/24] added ITs for search method byMetadata by community admin and by collection admin --- .../app/rest/EPersonRestRepositoryIT.java | 167 ++++++++++++++++ .../app/rest/GroupRestRepositoryIT.java | 182 ++++++++++++++++++ 2 files changed, 349 insertions(+) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java index 75bffd3828..9de5eabe94 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java @@ -25,6 +25,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; @@ -48,15 +49,20 @@ import org.dspace.app.rest.model.patch.ReplaceOperation; import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.app.rest.test.MetadataPatchSuite; import org.dspace.content.Collection; +import org.dspace.content.Community; import org.dspace.content.Item; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; +import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; import org.junit.Test; +import org.springframework.beans.factory.annotation.Autowired; public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { + @Autowired + private ConfigurationService configurationService; @Test public void createTest() throws Exception { @@ -1786,4 +1792,165 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { ); } + + @Test + public void findByMetadataByCommAdminAndByColAdminTest() throws Exception { + 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(); + EPerson colSubmitter = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("colSubmitter@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .withAdminGroup(eperson) + .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(colSubmitter) + .build(); + + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + String tokencolSubmitter = getAuthToken(colSubmitter.getEmail(), password); + + getClient(tokenAdminComm).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( + EPersonMatcher.matchEPersonEntry(adminChild1), + EPersonMatcher.matchEPersonEntry(adminCol1), + EPersonMatcher.matchEPersonEntry(colSubmitter) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + getClient(tokenAdminCol).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( + EPersonMatcher.matchEPersonEntry(adminChild1), + EPersonMatcher.matchEPersonEntry(adminCol1), + EPersonMatcher.matchEPersonEntry(colSubmitter) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + getClient(tokencolSubmitter).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isForbidden()); + } + + @Test + public void findByMetadataByCommAdminAndByColAdminWithoutAuthorizationsTest() throws Exception { + context.turnOffAuthorisationSystem(); + + List confPropsCollectionAdmins = new LinkedList<>(); + confPropsCollectionAdmins.add("core.authorization.collection-admin.policies"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.workflows"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.submitters"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.admin-group"); + + List confPropsCommunityAdmins = new LinkedList<>(); + confPropsCommunityAdmins.add("core.authorization.community-admin.policies"); + confPropsCommunityAdmins.add("core.authorization.community-admin.admin-group"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.policies"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.workflows"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.submitters"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.admin-group"); + + EPerson adminChild1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Oliver", "Rossi") + .withEmail("adminChild1@example.com") + .withPassword(password) + .build(); + EPerson adminCol = EPersonBuilder.createEPerson(context) + .withNameInMetadata("James", "Rossi") + .withEmail("adminCol1@example.com") + .withPassword(password) + .build(); + EPerson col1Submitter = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("col1Submitter@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .withAdminGroup(eperson) + .build(); + Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) + .withName("Sub Community") + .withAdminGroup(adminChild1) + .build(); + + Collection col1 = CollectionBuilder.createCollection(context, child1) + .withName("Collection 1") + .withAdminGroup(adminCol) + .withSubmitterGroup(col1Submitter) + .build(); + + context.restoreAuthSystemState(); + + String tokenAdminCol = getAuthToken(adminCol.getEmail(), password); + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + + getClient(tokenAdminCol).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( + EPersonMatcher.matchEPersonEntry(adminChild1), + EPersonMatcher.matchEPersonEntry(adminCol), + EPersonMatcher.matchEPersonEntry(col1Submitter) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + for (String prop : confPropsCollectionAdmins) { + configurationService.setProperty(prop, false); + } + + getClient(tokenAdminCol).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isForbidden()); + + getClient(tokenAdminComm).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( + EPersonMatcher.matchEPersonEntry(adminChild1), + EPersonMatcher.matchEPersonEntry(adminCol), + EPersonMatcher.matchEPersonEntry(col1Submitter) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + for (String prop : confPropsCommunityAdmins) { + configurationService.setProperty(prop, false); + } + + getClient(tokenAdminComm).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isForbidden()); + } } 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 a1b2f9cf14..b142266d86 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 @@ -24,6 +24,7 @@ import static org.springframework.test.web.servlet.result.MockMvcResultMatchers. import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; import java.util.ArrayList; +import java.util.LinkedList; import java.util.List; import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; @@ -54,6 +55,7 @@ import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.service.EPersonService; import org.dspace.eperson.service.GroupService; +import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -66,6 +68,8 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { @Autowired ResourcePolicyService resourcePolicyService; + @Autowired + private ConfigurationService configurationService; @Test public void createTest() @@ -1914,5 +1918,183 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(status().isOk()); } + @Test + public void findByMetadataByCommAdminAndByColAdminTest() throws Exception { + 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(); + EPerson colSubmitter = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("colSubmitter@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(colSubmitter) + .build(); + + Group group1 = GroupBuilder.createGroup(context) + .withName("Test group") + .build(); + + Group group2 = GroupBuilder.createGroup(context) + .withName("Test group 2") + .build(); + + Group group3 = GroupBuilder.createGroup(context) + .withName("Test group 3") + .build(); + + Group group4 = GroupBuilder.createGroup(context) + .withName("Test other group") + .build(); + + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + String tokenSubmitterCol = getAuthToken(colSubmitter.getEmail(), password); + + getClient(tokenAdminComm).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + getClient(tokenAdminCol).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()).andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups", Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + getClient(tokenSubmitterCol).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isForbidden()); + } + + @Test + public void findByMetadataByCommAdminAndByColAdminWithoutAuthorizationsTest() throws Exception { + context.turnOffAuthorisationSystem(); + + List confPropsCollectionAdmins = new LinkedList<>(); + confPropsCollectionAdmins.add("core.authorization.collection-admin.policies"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.submitters"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.workflows"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.admin-group"); + + List confPropsCommunityAdmins = new LinkedList<>(); + confPropsCommunityAdmins.add("core.authorization.community-admin.policies"); + confPropsCommunityAdmins.add("core.authorization.community-admin.admin-group"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.policies"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.workflows"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.submitters"); + confPropsCommunityAdmins.add("core.authorization.community-admin.collection.admin-group"); + + 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) + .build(); + + Group group1 = GroupBuilder.createGroup(context) + .withName("Test group") + .build(); + + Group group2 = GroupBuilder.createGroup(context) + .withName("Test group 2") + .build(); + + Group group3 = GroupBuilder.createGroup(context) + .withName("Test group 3") + .build(); + + Group group4 = GroupBuilder.createGroup(context) + .withName("Test other group") + .build(); + + context.restoreAuthSystemState(); + + String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + + getClient(tokenAdminCol).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + for (String prop : confPropsCollectionAdmins) { + configurationService.setProperty(prop, false); + } + + getClient(tokenAdminCol).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isForbidden()); + + getClient(tokenAdminComm).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + + for (String prop : confPropsCommunityAdmins) { + configurationService.setProperty(prop, false); + } + + getClient(tokenAdminCol).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isForbidden()); + } } From 6df58917b4d3011d9e458015e2b30bbfb6c929cb Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Wed, 27 May 2020 18:56:51 +0200 Subject: [PATCH 05/24] added ITs to prove that admins of community/collection can manage submitters --- .../app/rest/GroupRestRepositoryIT.java | 293 ++++++++++++++++++ 1 file changed, 293 insertions(+) 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 b142266d86..7a415e2af0 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 @@ -2097,4 +2097,297 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .param("query", group1.getName())) .andExpect(status().isForbidden()); } + + @Test + public void colAdminManageSubmitterGroupAndAdminGroupTest() 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(); + EPerson submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@example.com") + .withPassword(password) + .build(); + EPerson submitter2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Robert", "Clarks") + .withEmail("submitter2@example.com") + .withPassword(password) + .build(); + EPerson submitter3 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Jack", "Brown") + .withEmail("submitter3@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .withAdminGroup(eperson) + .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 groupSubmitters = col1.getSubmitters(); + Group groupAdmins = col1.getAdministrators(); + + context.restoreAuthSystemState(); + + String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + + assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + + getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + submitter2.getID() + )) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, groupSubmitters)); + assertTrue(groupService.isMember(context, submitter2, groupSubmitters)); + + assertFalse(groupService.isMember(context, submitter3, groupAdmins)); + + getClient(tokenAdminCol).perform( + post("/api/eperson/groups/" + groupAdmins.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter3.getID() + )) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter3, groupAdmins)); + } + + @Test + public void colAdminWithoutRightsTest() throws Exception { + + GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); + + context.turnOffAuthorisationSystem(); + + List confPropsCollectionAdmins = new LinkedList<>(); + confPropsCollectionAdmins.add("core.authorization.collection-admin.policies"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.submitters"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.workflows"); + confPropsCollectionAdmins.add("core.authorization.collection-admin.admin-group"); + + 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(); + EPerson submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@example.com") + .withPassword(password) + .build(); + EPerson submitter2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Robert", "Clarks") + .withEmail("submitter2@example.com") + .withPassword(password) + .build(); + EPerson submitter3 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Jack", "Brown") + .withEmail("submitter3@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .withAdminGroup(eperson) + .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(submitter2) + .build(); + + Group groupSubmitters = col1.getSubmitters(); + + context.restoreAuthSystemState(); + + String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + + assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + + getClient(tokenAdminCol).perform( + post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() + )) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, groupSubmitters)); + + for (String prop : confPropsCollectionAdmins) { + configurationService.setProperty(prop, false); + } + + assertFalse(groupService.isMember(context, submitter3, groupSubmitters)); + + getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter3.getID() + )) + .andExpect(status().isForbidden()); + + assertFalse(groupService.isMember(context, submitter3, groupSubmitters)); + } + + @Test + public void commAdminManageSubmitterGroupAndAdminGroupTest() 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(); + EPerson submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@example.com") + .withPassword(password) + .build(); + EPerson submitter2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Robert", "Clarks") + .withEmail("submitter2@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .withAdminGroup(eperson) + .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 groupSubmitters = col1.getSubmitters(); + + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + + assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + + getClient(tokenAdminComm).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() + "/\n" + + REST_SERVER_URL + "eperson/groups/" + submitter2.getID() + )) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, groupSubmitters)); + assertTrue(groupService.isMember(context, submitter2, groupSubmitters)); + + } + + @Test + public void commAdminDeleteColAdminFromAdminGroupTest() 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(); + EPerson submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@example.com") + .withPassword(password) + .build(); + + parentCommunity = CommunityBuilder.createCommunity(context) + .withName("Parent Community") + .withAdminGroup(eperson) + .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 groupAdministrators = col1.getAdministrators(); + Group groupSubmitters = col1.getSubmitters(); + + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + + assertTrue(groupService.isMember(context, adminCol1, groupAdministrators)); + + getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + + groupAdministrators.getID() + "/epersons/" + adminCol1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, adminCol1, groupAdministrators)); + assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + + getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() + )) + .andExpect(status().isForbidden()); + + assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + + } } From 34b61f3b98eb22436a652f6270ff32e85cd6eb2f Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Thu, 28 May 2020 19:07:50 +0200 Subject: [PATCH 06/24] added ITs to prove that admins of community/collection can manage their own groups --- .../org/dspace/app/util/AuthorizeUtil.java | 7 +- .../org/dspace/eperson/GroupServiceImpl.java | 19 + .../rest/CollectionGroupRestControllerIT.java | 39 -- .../app/rest/EPersonRestRepositoryIT.java | 44 +-- .../app/rest/GroupRestRepositoryIT.java | 335 +++++++++++++++--- 5 files changed, 340 insertions(+), 104 deletions(-) diff --git a/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java b/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java index 6b5d7f9003..ea1fb87ff4 100644 --- a/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java +++ b/dspace-api/src/main/java/org/dspace/app/util/AuthorizeUtil.java @@ -590,8 +590,11 @@ public class AuthorizeUtil { authorizeManageAdminGroup(context, collection); return; } - - + // if we reach this point, it means that the group is related + // to a collection but as it is not the submitters, nor the administrators, + // nor a workflow groups it must be a default item/bitstream groups + authorizeManageDefaultReadGroup(context, collection); + return; } if (parentObject.getType() == Constants.COMMUNITY) { Community community = (Community) parentObject; 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 7c23216458..449ddca973 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java @@ -23,7 +23,9 @@ import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.tuple.Pair; import org.dspace.authorize.AuthorizeConfiguration; import org.dspace.authorize.AuthorizeException; +import org.dspace.authorize.ResourcePolicy; import org.dspace.authorize.service.AuthorizeService; +import org.dspace.authorize.service.ResourcePolicyService; import org.dspace.content.Collection; import org.dspace.content.DSpaceObject; import org.dspace.content.DSpaceObjectServiceImpl; @@ -76,6 +78,8 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements @Autowired(required = true) protected AuthorizeService authorizeService; + @Autowired(required = true) + protected ResourcePolicyService resourcePolicyService; protected GroupServiceImpl() { super(); @@ -654,6 +658,21 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements return collectionService.getParentObject(context, collection); } } + } else { + if (AuthorizeConfiguration.canCollectionAdminManagePolicies()) { + List groups = new ArrayList(); + 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(); + } + policies = resourcePolicyService.find(context, null, groups, + Constants.DEFAULT_BITSTREAM_READ, Constants.COLLECTION); + if (policies.size() > 0) { + return policies.get(0).getdSpaceObject(); + } + } } } if (AuthorizeConfiguration.canCommunityAdminManageAdminGroup()) { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java index 58d5ff93ad..7464e9c38c 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CollectionGroupRestControllerIT.java @@ -35,7 +35,6 @@ import org.dspace.eperson.Group; import org.dspace.eperson.service.GroupService; import org.dspace.workflow.WorkflowService; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -971,11 +970,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati jsonPath("$", GroupMatcher.matchGroupEntry(role.getID(), role.getName()))); } - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void getCollectionDefaultItemReadGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); String itemGroupString = "ITEM"; @@ -993,11 +988,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati jsonPath("$", GroupMatcher.matchGroupEntry(role.getID(), role.getName()))); } - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void getCollectionDefaultItemReadGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); String itemGroupString = "ITEM"; @@ -1120,13 +1111,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati } - - - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void postCollectionDefaultItemReadGroupCreateDefaultItemReadGroupSuccessParentCommunityAdmin() throws Exception { @@ -1161,12 +1146,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati } - - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void postCollectionDefaultItemReadGroupCreateDefaultItemReadGroupSuccessCollectionAdmin() throws Exception { ObjectMapper mapper = new ObjectMapper(); @@ -1475,11 +1455,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati jsonPath("$", GroupMatcher.matchGroupEntry(role.getID(), role.getName()))); } - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void getCollectionDefaultBitstreamReadGroupTestParentCommunityAdmin() throws Exception { context.turnOffAuthorisationSystem(); String bitstreamGroupString = "BITSTREAM"; @@ -1497,11 +1473,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati jsonPath("$", GroupMatcher.matchGroupEntry(role.getID(), role.getName()))); } - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void getCollectionDefaultBitstreamReadGroupTestCollectionAdmin() throws Exception { context.turnOffAuthorisationSystem(); String bitstreamGroupString = "BITSTREAM"; @@ -1627,13 +1599,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati } - - - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void postCollectionDefaultBitstreamReadGroupCreateDefaultBitstreamReadGroupSuccessParentCommunityAdmin() throws Exception { @@ -1668,12 +1634,7 @@ public class CollectionGroupRestControllerIT extends AbstractControllerIntegrati } - - // Put on ignore because there's no support to identify read rights on a group for a user in a special - // com/coll admin group - // Please refer to: https://jira.lyrasis.org/browse/DS-4505 @Test - @Ignore public void postCollectionDefaultBitstreamReadGroupCreateDefaultBitstreamReadGroupSuccessCollectionAdmin() throws Exception { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java index 9de5eabe94..8d42788df4 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java @@ -1915,18 +1915,18 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { String tokenAdminCol = getAuthToken(adminCol.getEmail(), password); String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); - getClient(tokenAdminCol).perform(get("/api/eperson/epersons/search/byMetadata") - .param("query", "Rossi")) - .andExpect(status().isOk()) - .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( - EPersonMatcher.matchEPersonEntry(adminChild1), - EPersonMatcher.matchEPersonEntry(adminCol), - EPersonMatcher.matchEPersonEntry(col1Submitter) - ))) - .andExpect(jsonPath("$.page.totalElements", is(3))); - for (String prop : confPropsCollectionAdmins) { + getClient(tokenAdminCol).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( + EPersonMatcher.matchEPersonEntry(adminChild1), + EPersonMatcher.matchEPersonEntry(adminCol), + EPersonMatcher.matchEPersonEntry(col1Submitter) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + configurationService.setProperty(prop, false); } @@ -1934,18 +1934,18 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { .param("query", "Rossi")) .andExpect(status().isForbidden()); - getClient(tokenAdminComm).perform(get("/api/eperson/epersons/search/byMetadata") - .param("query", "Rossi")) - .andExpect(status().isOk()) - .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( - EPersonMatcher.matchEPersonEntry(adminChild1), - EPersonMatcher.matchEPersonEntry(adminCol), - EPersonMatcher.matchEPersonEntry(col1Submitter) - ))) - .andExpect(jsonPath("$.page.totalElements", is(3))); - for (String prop : confPropsCommunityAdmins) { + getClient(tokenAdminComm).perform(get("/api/eperson/epersons/search/byMetadata") + .param("query", "Rossi")) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.epersons", Matchers.containsInAnyOrder( + EPersonMatcher.matchEPersonEntry(adminChild1), + EPersonMatcher.matchEPersonEntry(adminCol), + EPersonMatcher.matchEPersonEntry(col1Submitter) + ))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + configurationService.setProperty(prop, false); } 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 7a415e2af0..2799d88e2e 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 @@ -44,10 +44,12 @@ import org.dspace.app.rest.model.patch.Operation; import org.dspace.app.rest.model.patch.ReplaceOperation; import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.app.rest.test.MetadataPatchSuite; +import org.dspace.authorize.service.AuthorizeService; import org.dspace.authorize.service.ResourcePolicyService; import org.dspace.content.Collection; import org.dspace.content.Community; import org.dspace.content.factory.ContentServiceFactory; +import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; import org.dspace.core.Constants; import org.dspace.eperson.EPerson; @@ -70,6 +72,10 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { ResourcePolicyService resourcePolicyService; @Autowired private ConfigurationService configurationService; + @Autowired + private CollectionService collectionService; + @Autowired + private AuthorizeService authorizeService; @Test public void createTest() @@ -2061,17 +2067,17 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); - getClient(tokenAdminCol).perform(get("/api/eperson/groups/search/byMetadata") - .param("query", group1.getName())) - .andExpect(status().isOk()) - .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( - GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), - GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), - GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) - .andExpect(jsonPath("$.page.totalElements", is(3))); - for (String prop : confPropsCollectionAdmins) { + getClient(tokenAdminCol).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + configurationService.setProperty(prop, false); } @@ -2079,17 +2085,17 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .param("query", group1.getName())) .andExpect(status().isForbidden()); - getClient(tokenAdminComm).perform(get("/api/eperson/groups/search/byMetadata") - .param("query", group1.getName())) - .andExpect(status().isOk()) - .andExpect(content().contentType(contentType)) - .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( - GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), - GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), - GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) - .andExpect(jsonPath("$.page.totalElements", is(3))); - for (String prop : confPropsCommunityAdmins) { + getClient(tokenAdminComm).perform(get("/api/eperson/groups/search/byMetadata") + .param("query", group1.getName())) + .andExpect(status().isOk()) + .andExpect(content().contentType(contentType)) + .andExpect(jsonPath("$._embedded.groups",Matchers.containsInAnyOrder( + GroupMatcher.matchGroupEntry(group1.getID(), group1.getName()), + GroupMatcher.matchGroupEntry(group2.getID(), group2.getName()), + GroupMatcher.matchGroupEntry(group3.getID(), group3.getName())))) + .andExpect(jsonPath("$.page.totalElements", is(3))); + configurationService.setProperty(prop, false); } @@ -2098,6 +2104,57 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(status().isForbidden()); } + @Test + public void commAdminManageOwnerAdminGroupTest() 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 submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@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(); + + Group groupAdmins = child1.getAdministrators(); + + context.restoreAuthSystemState(); + + String tokenCommAdmin = getAuthToken(adminChild1.getEmail(), password); + + assertFalse(groupService.isMember(context, submitter1, groupAdmins)); + + getClient(tokenCommAdmin).perform(post("/api/eperson/groups/" + groupAdmins.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() + )) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, groupAdmins)); + + getClient(tokenCommAdmin).perform(delete("/api/eperson/groups/" + + groupAdmins.getID() + "/epersons/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter1, groupAdmins)); + } + @Test public void colAdminManageSubmitterGroupAndAdminGroupTest() throws Exception { @@ -2133,7 +2190,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { parentCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") - .withAdminGroup(eperson) .build(); Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) .withName("Sub Community") @@ -2154,6 +2210,7 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + assertFalse(groupService.isMember(context, submitter2, groupSubmitters)); getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) @@ -2218,7 +2275,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { parentCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") - .withAdminGroup(eperson) .build(); Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) .withName("Sub Community") @@ -2264,7 +2320,7 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { } @Test - public void commAdminManageSubmitterGroupAndAdminGroupTest() throws Exception { + public void commAdminManageSunCollectionOfSubmittersAndAdminsTest() throws Exception { GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); @@ -2293,7 +2349,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { parentCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") - .withAdminGroup(eperson) .build(); Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) .withName("Sub Community") @@ -2307,12 +2362,14 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .build(); Group groupSubmitters = col1.getSubmitters(); + Group groupAdministrators = col1.getAdministrators(); context.restoreAuthSystemState(); String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + assertFalse(groupService.isMember(context, submitter2, groupSubmitters)); getClient(tokenAdminComm).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) @@ -2324,10 +2381,25 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { assertTrue(groupService.isMember(context, submitter1, groupSubmitters)); assertTrue(groupService.isMember(context, submitter2, groupSubmitters)); + getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + + groupSubmitters.getID() + "/epersons/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + assertTrue(groupService.isMember(context, submitter2, groupSubmitters)); + + assertTrue(groupService.isMember(context, adminCol1, groupAdministrators)); + getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + + groupAdministrators.getID() + "/epersons/" + adminCol1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, adminCol1, groupAdministrators)); + } + @Test - public void commAdminDeleteColAdminFromAdminGroupTest() throws Exception { + public void commAdminAndColAdminCanManageItemReadGroupTest() throws Exception { GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); @@ -2348,10 +2420,14 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .withEmail("submitter1@example.com") .withPassword(password) .build(); + EPerson submitter2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Robert", "Clarks") + .withEmail("submitter2@example.com") + .withPassword(password) + .build(); parentCommunity = CommunityBuilder.createCommunity(context) .withName("Parent Community") - .withAdminGroup(eperson) .build(); Community child1 = CommunityBuilder.createSubCommunity(context, parentCommunity) .withName("Sub Community") @@ -2364,30 +2440,207 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .withSubmitterGroup(eperson) .build(); - Group groupAdministrators = col1.getAdministrators(); - Group groupSubmitters = col1.getSubmitters(); + String itemGroupString = "ITEM"; + int defaultItemRead = Constants.DEFAULT_ITEM_READ; + Group itemReadGroup = collectionService.createDefaultReadGroup(context, col1, itemGroupString, defaultItemRead); context.restoreAuthSystemState(); String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); - String tokenAdminCol = getAuthToken(adminCol1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminChild1.getEmail(), password); - assertTrue(groupService.isMember(context, adminCol1, groupAdministrators)); + assertFalse(groupService.isMember(context, submitter1, itemReadGroup)); + assertFalse(groupService.isMember(context, submitter2, itemReadGroup)); - getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" - + groupAdministrators.getID() + "/epersons/" + adminCol1.getID())) - .andExpect(status().isNoContent()); + getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + itemReadGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID())) + .andExpect(status().isNoContent()); - assertFalse(groupService.isMember(context, adminCol1, groupAdministrators)); - assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + assertTrue(groupService.isMember(context, submitter1, itemReadGroup)); - getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + groupSubmitters.getID() + "/epersons") - .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) - .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() - )) - .andExpect(status().isForbidden()); - assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); + getClient(tokenAdminComm).perform(post("/api/eperson/groups/" + itemReadGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter2.getID())) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter2, itemReadGroup)); + + getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + + itemReadGroup.getID() + "/epersons/" + submitter2.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter2, itemReadGroup)); + + getClient(tokenAdminCol).perform(delete("/api/eperson/groups/" + + itemReadGroup.getID() + "/epersons/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter1, itemReadGroup)); } + + @Test + public void commAdminAndColAdminCanManageBitstreamReadGroupTest() 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(); + EPerson submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@example.com") + .withPassword(password) + .build(); + EPerson submitter2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Robert", "Clarks") + .withEmail("submitter2@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(); + + String bitstreamGroupString = "BITSTREAM"; + int defaultBitstreamRead = Constants.DEFAULT_BITSTREAM_READ; + + Group bitstreamReadGroup = collectionService.createDefaultReadGroup(context, col1, bitstreamGroupString, + defaultBitstreamRead); + + context.restoreAuthSystemState(); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminChild1.getEmail(), password); + + assertFalse(groupService.isMember(context, submitter1, bitstreamReadGroup)); + assertFalse(groupService.isMember(context, submitter2, bitstreamReadGroup)); + + getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + bitstreamReadGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, bitstreamReadGroup)); + + + getClient(tokenAdminComm).perform(post("/api/eperson/groups/" + bitstreamReadGroup.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter2.getID())) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter2, bitstreamReadGroup)); + + getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + + bitstreamReadGroup.getID() + "/epersons/" + submitter2.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter2, bitstreamReadGroup)); + + getClient(tokenAdminCol).perform(delete("/api/eperson/groups/" + + bitstreamReadGroup.getID() + "/epersons/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter1, bitstreamReadGroup)); + + } + + @Test + public void commAdminAndColAdminCanManageWorkflowGroupsTest() 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(); + EPerson submitter1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Carl", "Rossi") + .withEmail("submitter1@example.com") + .withPassword(password) + .build(); + EPerson submitter2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Robert", "Clarks") + .withEmail("submitter2@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) + .withWorkflowGroup(1, eperson) + .withWorkflowGroup(2, eperson) + .build(); + + Group workflowGroupStep1 = col1.getWorkflowStep1(context); + Group workflowGroupStep2 = col1.getWorkflowStep2(context); + + context.restoreAuthSystemState(); + + assertFalse(groupService.isMember(context, submitter1, workflowGroupStep1)); + assertFalse(groupService.isMember(context, submitter2, workflowGroupStep2)); + + String tokenAdminComm = getAuthToken(adminChild1.getEmail(), password); + String tokenAdminCol = getAuthToken(adminChild1.getEmail(), password); + + getClient(tokenAdminComm).perform(post("/api/eperson/groups/" + workflowGroupStep1.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, workflowGroupStep1)); + + getClient(tokenAdminCol).perform(post("/api/eperson/groups/" + workflowGroupStep2.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter2.getID())) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter2, workflowGroupStep2)); + + getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + + workflowGroupStep2.getID() + "/epersons/" + submitter2.getID())) + .andExpect(status().isNoContent()); + + getClient(tokenAdminCol).perform(delete("/api/eperson/groups/" + + workflowGroupStep1.getID() + "/epersons/" + submitter1.getID())) + .andExpect(status().isNoContent()); + + assertFalse(groupService.isMember(context, submitter1, workflowGroupStep1)); + assertFalse(groupService.isMember(context, submitter2, workflowGroupStep2)); + } } From 361086710537d061df59e6c7b17ea1aa47466870 Mon Sep 17 00:00:00 2001 From: Kevin Van de Velde Date: Fri, 29 May 2020 13:19:49 +0200 Subject: [PATCH 07/24] Add missing test from https://github.com/DSpace/DSpace/pull/2770 --- .../CommunityAdminGroupRestControllerIT.java | 392 ++++++++++++++++++ .../app/rest/GroupRestRepositoryIT.java | 280 ++++++++++++- 2 files changed, 671 insertions(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java index 7d5e1a393b..bf606def8c 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java @@ -10,6 +10,7 @@ package org.dspace.app.rest; import static com.jayway.jsonpath.JsonPath.read; import static org.dspace.app.rest.matcher.MetadataMatcher.matchMetadata; import static org.hamcrest.Matchers.allOf; +import static org.springframework.http.MediaType.parseMediaType; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; @@ -20,21 +21,31 @@ import java.util.UUID; import java.util.concurrent.atomic.AtomicReference; import com.fasterxml.jackson.databind.ObjectMapper; +import org.dspace.app.rest.builder.CollectionBuilder; import org.dspace.app.rest.builder.CommunityBuilder; +import org.dspace.app.rest.builder.EPersonBuilder; +import org.dspace.app.rest.builder.GroupBuilder; +import org.dspace.app.rest.matcher.EPersonMatcher; import org.dspace.app.rest.matcher.GroupMatcher; import org.dspace.app.rest.model.GroupRest; import org.dspace.app.rest.model.MetadataRest; import org.dspace.app.rest.model.MetadataValueRest; import org.dspace.app.rest.test.AbstractControllerIntegrationTest; import org.dspace.authorize.service.AuthorizeService; +import org.dspace.content.Collection; import org.dspace.content.Community; +import org.dspace.content.service.CollectionService; import org.dspace.content.service.CommunityService; import org.dspace.core.Constants; +import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.service.GroupService; +import org.dspace.services.ConfigurationService; +import org.hamcrest.Matchers; import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.http.MediaType; public class CommunityAdminGroupRestControllerIT extends AbstractControllerIntegrationTest { @@ -48,10 +59,19 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg @Autowired private AuthorizeService authorizeService; + @Autowired + private CollectionService collectionService; + + @Autowired + private ConfigurationService configurationService; + + Collection collection; + @Before public void setup() { context.turnOffAuthorisationSystem(); parentCommunity = CommunityBuilder.createCommunity(context).withName("test").build(); + collection = CollectionBuilder.createCollection(context, parentCommunity).withName("Collection 1").build(); context.restoreAuthSystemState(); } @@ -437,4 +457,376 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg getClient(token).perform(delete("/api/core/communities/" + UUID.randomUUID() + "/adminGroup")) .andExpect(status().isNotFound()); } + + @Test + public void communityAdminAddMembersToCommunityAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = communityService.createAdministrators(context, parentCommunity); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + configurationService.setProperty("core.authorization.community-admin.admin-group", false); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes.TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.not(Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + )))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void communityAdminRemoveMembersFromCommunityAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = communityService.createAdministrators(context, parentCommunity); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", false); + context.restoreAuthSystemState(); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void communityAdminAddChildGroupToCommunityAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = communityService.createAdministrators(context, parentCommunity); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + configurationService.setProperty("core.authorization.community-admin.admin-group", false); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + )))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void communityAdminRemoveChildGroupFromCommunityAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = communityService.createAdministrators(context, parentCommunity); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", false); + context.restoreAuthSystemState(); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", true); + context.restoreAuthSystemState(); + } + + @Test + public void communityAdminAddChildGroupToCollectionAdminGroupSuccess() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(MediaType.parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + } + + @Test + public void communityAdminRemoveChildGroupFromCollectionAdminGroupSuccess() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(MediaType.parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) + .andExpect(status().isNoContent()); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + )))); + + } + + @Test + public void communityAdminAddMembersToCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(MediaType.parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes.TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.not(Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + )))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void communityAdminRemoveMembersFromCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(MediaType.parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void communityAdminAddChildGroupToCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(MediaType.parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + )))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void communityAdminRemoveChildGroupFromCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, parentCommunity, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(MediaType.parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + } } 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 2799d88e2e..ff5e69c396 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 @@ -35,6 +35,7 @@ import org.dspace.app.rest.builder.CollectionBuilder; import org.dspace.app.rest.builder.CommunityBuilder; import org.dspace.app.rest.builder.EPersonBuilder; import org.dspace.app.rest.builder.GroupBuilder; +import org.dspace.app.rest.matcher.EPersonMatcher; import org.dspace.app.rest.matcher.GroupMatcher; import org.dspace.app.rest.matcher.HalMatcher; import org.dspace.app.rest.model.GroupRest; @@ -59,6 +60,7 @@ import org.dspace.eperson.service.EPersonService; import org.dspace.eperson.service.GroupService; import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; +import org.junit.Before; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -74,9 +76,21 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { private ConfigurationService configurationService; @Autowired private CollectionService collectionService; + @Autowired private AuthorizeService authorizeService; + Collection collection; + + @Before + public void setup() { + context.turnOffAuthorisationSystem(); + parentCommunity = CommunityBuilder.createCommunity(context).withName("test").build(); + collection = CollectionBuilder.createCollection(context, parentCommunity).withName("Collection 1").build(); + + context.restoreAuthSystemState(); + } + @Test public void createTest() throws Exception { @@ -2320,7 +2334,7 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { } @Test - public void commAdminManageSunCollectionOfSubmittersAndAdminsTest() throws Exception { + public void communityAdminCanManageCollectionSubmittersGroupAndAdminsGroupsTest() throws Exception { GroupService groupService = EPersonServiceFactory.getInstance().getGroupService(); @@ -2388,6 +2402,13 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { assertFalse(groupService.isMember(context, submitter1, groupSubmitters)); assertTrue(groupService.isMember(context, submitter2, groupSubmitters)); + getClient(tokenAdminComm).perform(post("/api/eperson/groups/" + groupAdministrators.getID() + "/epersons") + .contentType(parseMediaType(TEXT_URI_LIST_VALUE)) + .content(REST_SERVER_URL + "eperson/groups/" + submitter1.getID() + )) + .andExpect(status().isNoContent()); + + assertTrue(groupService.isMember(context, submitter1, groupAdministrators)); assertTrue(groupService.isMember(context, adminCol1, groupAdministrators)); getClient(tokenAdminComm).perform(delete("/api/eperson/groups/" + groupAdministrators.getID() + "/epersons/" + adminCol1.getID())) @@ -2643,4 +2664,261 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { assertFalse(groupService.isMember(context, submitter1, workflowGroupStep1)); assertFalse(groupService.isMember(context, submitter2, workflowGroupStep2)); } + + @Test + public void collectionAdminRemoveMembersFromCollectionAdminGroupSuccess() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) + .andExpect(status().isNoContent()); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.not(Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + )))); + + } + + @Test + public void collectionAdminAddChildGroupToCollectionAdminGroupSuccess() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + } + + @Test + public void collectionAdminRemoveChildGroupFromCollectionAdminGroupSuccess() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) + .andExpect(status().isNoContent()); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + )))); + + } + + @Test + public void collectionAdminAddMembersToCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes.TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.not(Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + )))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void collectionAdminRemoveMembersFromCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + EPerson ePerson = EPersonBuilder.createEPerson(context).withEmail("testToAdd@test.com").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/epersons") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/epersons/" + ePerson.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/epersons")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( + EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void collectionAdminAddChildGroupToCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + )))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + + } + + @Test + public void collectionAdminRemoveChildGroupFromCollectionAdminGroupPropertySetToFalse() throws Exception { + + context.turnOffAuthorisationSystem(); + Group adminGroup = collectionService.createAdministrators(context, collection); + authorizeService.addPolicy(context, collection, Constants.ADMIN, eperson); + Group group = GroupBuilder.createGroup(context).withName("testGroup").build(); + context.restoreAuthSystemState(); + + + String token = getAuthToken(eperson.getEmail(), password); + getClient(token).perform( + post("/api/eperson/groups/" + adminGroup.getID() + "/subgroups") + .contentType(parseMediaType + (org.springframework.data.rest.webmvc.RestMediaTypes + .TEXT_URI_LIST_VALUE)) + .content("https://localhost:8080/server/api/eperson/groups/" + group.getID())); + + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); + configurationService.setProperty("core.authorization.collection-admin.admin-group", false); + context.restoreAuthSystemState(); + + getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) + .andExpect(status().isForbidden()); + + token = getAuthToken(admin.getEmail(), password); + getClient(token).perform(get("/api/eperson/groups/" + adminGroup.getID() + "/subgroups")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( + GroupMatcher.matchGroupWithName(group.getName()) + ))); + + context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); + configurationService.setProperty("core.authorization.collection-admin.admin-group", true); + context.restoreAuthSystemState(); + } + } From 6c0ee98fa616ea63af7aa68f6a500f7ef25be780 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Tue, 2 Jun 2020 14:56:00 +0200 Subject: [PATCH 08/24] SPEL parsing of the preAuthorize annotation in converterService work --- .../app/rest/converter/ConverterService.java | 16 ++++ .../spel/ExpressionValidationException.java | 7 ++ .../app/rest/spel/ExpressionValidator.java | 86 +++++++++++++++++++ 3 files changed, 109 insertions(+) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java index 26eae66e65..cbff22f9a2 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java @@ -31,6 +31,8 @@ import org.dspace.app.rest.projection.DefaultProjection; import org.dspace.app.rest.projection.Projection; import org.dspace.app.rest.repository.DSpaceRestRepository; import org.dspace.app.rest.security.DSpacePermissionEvaluator; +import org.dspace.app.rest.spel.ExpressionValidationException; +import org.dspace.app.rest.spel.ExpressionValidator; import org.dspace.app.rest.utils.Utils; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; @@ -40,8 +42,12 @@ import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; +import org.springframework.expression.Expression; +import org.springframework.expression.ExpressionParser; +import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.hateoas.EntityModel; import org.springframework.hateoas.Link; +import org.springframework.security.access.expression.SecurityExpressionRoot; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Component; @@ -136,6 +142,16 @@ public class ConverterService { String permission = ""; if (preAuthorize != null) { String annotationValue = (String) AnnotationUtils.getValue(preAuthorize); + ExpressionValidator validator = new ExpressionValidator(); + try { + validator.validate("hasPermission(123, 'ITEM', 'WRITE')", SecurityExpressionRoot.class); + } catch (ExpressionValidationException e) { + e.printStackTrace(); + } + +// ExpressionParser parser = new SpelExpressionParser(); +// Expression exp = parser.parseExpression("hasPermission('123', 'ITEM', 'WRITE')"); +// exp.getValue(SecurityExpressionRoot.class, boolean.class); if (StringUtils.contains(annotationValue, "permitAll")) { permission = "permitAll"; } else if (StringUtils.contains(annotationValue, "hasAuthority")) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java new file mode 100644 index 0000000000..9fbe73f19f --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java @@ -0,0 +1,7 @@ +package org.dspace.app.rest.spel; + +public class ExpressionValidationException extends Exception { + public ExpressionValidationException(String message) { + super(message); + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java new file mode 100644 index 0000000000..8cf1dc2a6c --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java @@ -0,0 +1,86 @@ +package org.dspace.app.rest.spel; + +import org.springframework.expression.ExpressionParser; +import org.springframework.expression.ParseException; +import org.springframework.expression.spel.SpelNode; +import org.springframework.expression.spel.ast.BeanReference; +import org.springframework.expression.spel.ast.MethodReference; +import org.springframework.expression.spel.ast.Operator; +import org.springframework.expression.spel.ast.TypeReference; +import org.springframework.expression.spel.ast.VariableReference; +import org.springframework.expression.spel.standard.SpelExpression; +import org.springframework.expression.spel.standard.SpelExpressionParser; +import org.springframework.security.access.expression.SecurityExpressionRoot; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; + +import java.lang.reflect.InvocationTargetException; +import java.lang.reflect.Method; + +public class ExpressionValidator { + private final ExpressionParser parser = new SpelExpressionParser(); + + public void validate(String expression, Class expressionRoot) throws ExpressionValidationException, ParseException { + SpelExpression exp = (SpelExpression) parser.parseExpression(expression); + if (expressionRoot != null) { + SpelNode node = exp.getAST(); + handle(node, expressionRoot); + } + } + + private void handle(SpelNode node, Class expressionRoot) throws ExpressionValidationException{ + if (node instanceof MethodReference) { + verify((MethodReference) node, expressionRoot); + } else if (node instanceof Operator) { + Operator operator = (Operator) node; + handle(operator.getLeftOperand(), expressionRoot); + handle(operator.getRightOperand(), expressionRoot); + } else if (node != null) { + for(int i=0; i expressionRoot) throws ExpressionValidationException { + String methodName = node.getName(); + int args = node.getChildCount(); + Method[] methods = expressionRoot.getDeclaredMethods(); + for(Method m : methods) { + if (m.getName().equals(methodName)) { + // exact match on the args + if (args == m.getParameterCount()) { + try { + SecurityExpressionRoot.class.getConstructor(Authentication.class).newInstance( + SecurityContextHolder.getContext().getAuthentication()); + } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { + e.printStackTrace(); + } + return; + } + Class[] parameterTypes = m.getParameterTypes(); + if (m.getName().equals(methodName) && + parameterTypes != null && + parameterTypes.length>=1 && + parameterTypes[parameterTypes.length-1].isArray()) { + // allow the number of params to be one less or >= the reported length + if(args == m.getParameterCount()-1 || args >= m.getParameterCount()) { + return; + } + } + } + } + // if we get here, then we were unable to match the method call + String pattern = "Unable to match method %s with %d params"; + throw new ExpressionValidationException(String.format(pattern, methodName, args)); + } + +} From 397a156186f33333e83e16bebb3b3940761fe5e7 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Fri, 5 Jun 2020 13:56:39 +0200 Subject: [PATCH 09/24] [Task 71143] Intermediate work on PreAuthorize annotation parsing --- .../app/rest/converter/ConverterService.java | 78 ++++++----------- .../WebSecurityExpressionEvaluator.java | 56 ++++++++++++ .../spel/ExpressionValidationException.java | 7 -- .../app/rest/spel/ExpressionValidator.java | 86 ------------------- 4 files changed, 82 insertions(+), 145 deletions(-) create mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java index cbff22f9a2..e1d282fc8b 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java @@ -31,9 +31,9 @@ import org.dspace.app.rest.projection.DefaultProjection; import org.dspace.app.rest.projection.Projection; import org.dspace.app.rest.repository.DSpaceRestRepository; import org.dspace.app.rest.security.DSpacePermissionEvaluator; -import org.dspace.app.rest.spel.ExpressionValidationException; -import org.dspace.app.rest.spel.ExpressionValidator; +import org.dspace.app.rest.security.WebSecurityExpressionEvaluator; import org.dspace.app.rest.utils.Utils; +import org.dspace.services.RequestService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.beans.factory.config.BeanDefinition; import org.springframework.context.annotation.ClassPathScanningCandidateComponentProvider; @@ -42,14 +42,9 @@ import org.springframework.core.type.filter.AssignableTypeFilter; import org.springframework.data.domain.Page; import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; -import org.springframework.expression.Expression; -import org.springframework.expression.ExpressionParser; -import org.springframework.expression.spel.standard.SpelExpressionParser; import org.springframework.hateoas.EntityModel; import org.springframework.hateoas.Link; -import org.springframework.security.access.expression.SecurityExpressionRoot; import org.springframework.security.access.prepost.PreAuthorize; -import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.stereotype.Component; import org.springframework.stereotype.Service; @@ -83,6 +78,12 @@ public class ConverterService { @Autowired private DSpacePermissionEvaluator dSpacePermissionEvaluator; + @Autowired + private WebSecurityExpressionEvaluator webSecurityExpressionEvaluator; + + @Autowired + private RequestService requestService; + /** * Converts the given model object to a rest object, using the appropriate {@link DSpaceConverter} and * the given projection. @@ -106,19 +107,12 @@ public class ConverterService { DSpaceConverter converter = requireConverter(modelObject.getClass()); R restObject = converter.convert(transformedModel, projection); if (restObject instanceof BaseObjectRest) { - String permission = getPermissionForRestObject((BaseObjectRest) restObject); - if (!StringUtils.equalsIgnoreCase(permission, "permitAll")) { - if (StringUtils.equalsIgnoreCase(permission, "admin")) { - //TODO - } else if (StringUtils.equalsIgnoreCase(permission, "authenticated")) { - //TODO - } else { - if (!dSpacePermissionEvaluator.hasPermission(SecurityContextHolder.getContext().getAuthentication(), - restObject, permission)) { - log.info("Access denied on " + restObject.getClass()); - return null; - } - } + String preAuthorizeValue = getPreAuthorizeAnnotationForBaseObject((BaseObjectRest) restObject); + if (!webSecurityExpressionEvaluator + .evaluate(preAuthorizeValue, requestService.getCurrentRequest().getHttpServletRequest(), + requestService.getCurrentRequest().getHttpServletResponse())) { + log.info("Access denied on " + restObject.getClass()); + return null; } } if (restObject instanceof RestModel) { @@ -127,41 +121,21 @@ public class ConverterService { return restObject; } - private String getPermissionForRestObject(BaseObjectRest restObject) { + private String getPreAuthorizeAnnotationForBaseObject(BaseObjectRest restObject) { Annotation preAuthorize = getAnnotationForRestObject(restObject); if (preAuthorize == null) { preAuthorize = getDefaultFindOnePreAuthorize(); } - String permission = "READ"; - permission = parseAnnotation(preAuthorize); - return permission; + return parseAnnotation(preAuthorize); + } private String parseAnnotation(Annotation preAuthorize) { - String permission = ""; if (preAuthorize != null) { - String annotationValue = (String) AnnotationUtils.getValue(preAuthorize); - ExpressionValidator validator = new ExpressionValidator(); - try { - validator.validate("hasPermission(123, 'ITEM', 'WRITE')", SecurityExpressionRoot.class); - } catch (ExpressionValidationException e) { - e.printStackTrace(); - } - -// ExpressionParser parser = new SpelExpressionParser(); -// Expression exp = parser.parseExpression("hasPermission('123', 'ITEM', 'WRITE')"); -// exp.getValue(SecurityExpressionRoot.class, boolean.class); - if (StringUtils.contains(annotationValue, "permitAll")) { - permission = "permitAll"; - } else if (StringUtils.contains(annotationValue, "hasAuthority")) { - permission = StringUtils.substringBetween(annotationValue, "hasAuthority('", "')"); - } else if (StringUtils.contains(annotationValue,"hasPermission")) { - permission = StringUtils.split(annotationValue, ",")[2]; - permission = StringUtils.substringBetween(permission, "'"); - } + return (String) AnnotationUtils.getValue(preAuthorize); } - return permission; + return null; } private Annotation getAnnotationForRestObject(BaseObjectRest restObject) { @@ -410,19 +384,19 @@ public class ConverterService { ClassPathScanningCandidateComponentProvider provider = new ClassPathScanningCandidateComponentProvider(false); provider.addIncludeFilter(new AssignableTypeFilter(EntityModel.class)); Set beanDefinitions = provider.findCandidateComponents( - HALResource.class.getPackage().getName().replaceAll("\\.", "/")); + HALResource.class.getPackage().getName().replaceAll("\\.", "/")); for (BeanDefinition beanDefinition : beanDefinitions) { String resourceClassName = beanDefinition.getBeanClassName(); String resourceClassSimpleName = resourceClassName.substring(resourceClassName.lastIndexOf(".") + 1); String restClassSimpleName = resourceClassSimpleName - .replaceAll("ResourceWrapper$", "RestWrapper") - .replaceAll("Resource$", "Rest"); + .replaceAll("ResourceWrapper$", "RestWrapper") + .replaceAll("Resource$", "Rest"); String restClassName = RestModel.class.getPackage().getName() + "." + restClassSimpleName; try { Class restClass = - (Class) Class.forName(restClassName); + (Class) Class.forName(restClassName); Class> resourceClass = - (Class>) Class.forName(resourceClassName); + (Class>) Class.forName(resourceClassName); Constructor compatibleConstructor = null; for (Constructor constructor : resourceClass.getDeclaredConstructors()) { if (constructor.getParameterCount() == 2 && constructor.getParameterTypes()[1] == Utils.class) { @@ -436,11 +410,11 @@ public class ConverterService { resourceConstructors.put(restClass, compatibleConstructor); } else { log.warn("Skipping registration of resource class " + resourceClassName - + "; compatible constructor not found"); + + "; compatible constructor not found"); } } catch (ClassNotFoundException e) { log.warn("Skipping registration of resource class " + resourceClassName - + "; rest class not found: " + restClassName); + + "; rest class not found: " + restClassName); } } } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java new file mode 100644 index 0000000000..ce8668e89f --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java @@ -0,0 +1,56 @@ +package org.dspace.app.rest.security; + +import java.util.List; +import javax.servlet.FilterChain; +import javax.servlet.http.HttpServletRequest; +import javax.servlet.http.HttpServletResponse; + +import org.springframework.core.GenericTypeResolver; +import org.springframework.expression.EvaluationContext; +import org.springframework.expression.Expression; +import org.springframework.security.access.expression.ExpressionUtils; +import org.springframework.security.access.expression.SecurityExpressionHandler; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContextHolder; +import org.springframework.security.web.FilterInvocation; +import org.springframework.stereotype.Component; + +@Component +public class WebSecurityExpressionEvaluator { + + private static final FilterChain EMPTY_CHAIN = (request, response) -> { + throw new UnsupportedOperationException(); + }; + + private final List securityExpressionHandlers; + + public WebSecurityExpressionEvaluator(List securityExpressionHandlers) { + this.securityExpressionHandlers = securityExpressionHandlers; + } + + public boolean evaluate(String securityExpression, HttpServletRequest request, HttpServletResponse response) { + SecurityExpressionHandler handler = getFilterSecurityHandler(); + + Expression expression = handler.getExpressionParser().parseExpression(securityExpression); + + EvaluationContext evaluationContext = createEvaluationContext(handler, request, response); + + return ExpressionUtils.evaluateAsBoolean(expression, evaluationContext); + } + + @SuppressWarnings("unchecked") + private EvaluationContext createEvaluationContext(SecurityExpressionHandler handler, HttpServletRequest request, HttpServletResponse response) { + Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); + FilterInvocation filterInvocation = new FilterInvocation(request, response, EMPTY_CHAIN); + + return handler.createEvaluationContext(authentication, filterInvocation); + } + + private SecurityExpressionHandler getFilterSecurityHandler() { + return securityExpressionHandlers.stream() + .filter(handler -> FilterInvocation.class.equals(GenericTypeResolver + .resolveTypeArgument(handler.getClass(), SecurityExpressionHandler.class))) + .findAny() + .orElseThrow(() -> new IllegalStateException("No filter invocation security expression handler has been found! Handlers: " + securityExpressionHandlers.size())); + } +} \ No newline at end of file diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java deleted file mode 100644 index 9fbe73f19f..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidationException.java +++ /dev/null @@ -1,7 +0,0 @@ -package org.dspace.app.rest.spel; - -public class ExpressionValidationException extends Exception { - public ExpressionValidationException(String message) { - super(message); - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java deleted file mode 100644 index 8cf1dc2a6c..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/spel/ExpressionValidator.java +++ /dev/null @@ -1,86 +0,0 @@ -package org.dspace.app.rest.spel; - -import org.springframework.expression.ExpressionParser; -import org.springframework.expression.ParseException; -import org.springframework.expression.spel.SpelNode; -import org.springframework.expression.spel.ast.BeanReference; -import org.springframework.expression.spel.ast.MethodReference; -import org.springframework.expression.spel.ast.Operator; -import org.springframework.expression.spel.ast.TypeReference; -import org.springframework.expression.spel.ast.VariableReference; -import org.springframework.expression.spel.standard.SpelExpression; -import org.springframework.expression.spel.standard.SpelExpressionParser; -import org.springframework.security.access.expression.SecurityExpressionRoot; -import org.springframework.security.core.Authentication; -import org.springframework.security.core.context.SecurityContextHolder; - -import java.lang.reflect.InvocationTargetException; -import java.lang.reflect.Method; - -public class ExpressionValidator { - private final ExpressionParser parser = new SpelExpressionParser(); - - public void validate(String expression, Class expressionRoot) throws ExpressionValidationException, ParseException { - SpelExpression exp = (SpelExpression) parser.parseExpression(expression); - if (expressionRoot != null) { - SpelNode node = exp.getAST(); - handle(node, expressionRoot); - } - } - - private void handle(SpelNode node, Class expressionRoot) throws ExpressionValidationException{ - if (node instanceof MethodReference) { - verify((MethodReference) node, expressionRoot); - } else if (node instanceof Operator) { - Operator operator = (Operator) node; - handle(operator.getLeftOperand(), expressionRoot); - handle(operator.getRightOperand(), expressionRoot); - } else if (node != null) { - for(int i=0; i expressionRoot) throws ExpressionValidationException { - String methodName = node.getName(); - int args = node.getChildCount(); - Method[] methods = expressionRoot.getDeclaredMethods(); - for(Method m : methods) { - if (m.getName().equals(methodName)) { - // exact match on the args - if (args == m.getParameterCount()) { - try { - SecurityExpressionRoot.class.getConstructor(Authentication.class).newInstance( - SecurityContextHolder.getContext().getAuthentication()); - } catch (InstantiationException | IllegalAccessException | InvocationTargetException | NoSuchMethodException e) { - e.printStackTrace(); - } - return; - } - Class[] parameterTypes = m.getParameterTypes(); - if (m.getName().equals(methodName) && - parameterTypes != null && - parameterTypes.length>=1 && - parameterTypes[parameterTypes.length-1].isArray()) { - // allow the number of params to be one less or >= the reported length - if(args == m.getParameterCount()-1 || args >= m.getParameterCount()) { - return; - } - } - } - } - // if we get here, then we were unable to match the method call - String pattern = "Unable to match method %s with %d params"; - throw new ExpressionValidationException(String.format(pattern, methodName, args)); - } - -} From 49d40598f0e2ca26f3683128852887c970ee8046 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Mon, 8 Jun 2020 13:04:59 +0200 Subject: [PATCH 10/24] [Task 71143] implemented the PreAuthorize check in ConverterService for the findOne of a BaseObjectRest when the toRest method is called --- .../app/rest/converter/ConverterService.java | 6 ++-- .../rest/model/ExternalSourceEntryRest.java | 2 +- .../rest/repository/BundleRestRepository.java | 6 ++-- .../rest/repository/ItemRestRepository.java | 2 +- .../WebSecurityExpressionEvaluator.java | 12 +++++-- .../rest/AuthorizationRestRepositoryIT.java | 6 ++++ .../app/rest/ProcessRestRepositoryIT.java | 7 ++++ .../rest/converter/ConverterServiceIT.java | 9 +++++ .../repository/MockObjectRestRepository.java | 36 +++++++++++++++++++ 9 files changed, 77 insertions(+), 9 deletions(-) create mode 100644 dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java index e1d282fc8b..bf959f719e 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java @@ -107,10 +107,12 @@ public class ConverterService { DSpaceConverter converter = requireConverter(modelObject.getClass()); R restObject = converter.convert(transformedModel, projection); if (restObject instanceof BaseObjectRest) { - String preAuthorizeValue = getPreAuthorizeAnnotationForBaseObject((BaseObjectRest) restObject); + BaseObjectRest baseObjectRest = (BaseObjectRest) restObject; + String preAuthorizeValue = getPreAuthorizeAnnotationForBaseObject(baseObjectRest); if (!webSecurityExpressionEvaluator .evaluate(preAuthorizeValue, requestService.getCurrentRequest().getHttpServletRequest(), - requestService.getCurrentRequest().getHttpServletResponse())) { + requestService.getCurrentRequest().getHttpServletResponse(), + String.valueOf(baseObjectRest.getId()))) { log.info("Access denied on " + restObject.getClass()); return null; } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ExternalSourceEntryRest.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ExternalSourceEntryRest.java index aa5dfa8cf2..06af7e2227 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ExternalSourceEntryRest.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/model/ExternalSourceEntryRest.java @@ -12,7 +12,7 @@ import org.dspace.app.rest.ExternalSourcesRestController; /** * This class serves as a REST representation for an entry of external data */ -public class ExternalSourceEntryRest extends BaseObjectRest { +public class ExternalSourceEntryRest extends RestAddressableModel { public static final String NAME = "externalSourceEntry"; public static final String PLURAL_NAME = "externalSourceEntries"; diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BundleRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BundleRestRepository.java index d26ceeb2bf..f750743db6 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BundleRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BundleRestRepository.java @@ -74,11 +74,11 @@ public class BundleRestRepository extends DSpaceObjectRestRepository { + try { + processService.delete(context, process); + } catch (SQLException e) { + throw new RuntimeException(e); + } + }); parameters.add(new DSpaceCommandLineParameter("-r", "test")); parameters.add(new DSpaceCommandLineParameter("-i", null)); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java index e1d2e2d089..ea1a15dc04 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java @@ -13,6 +13,7 @@ import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.assertTrue; import static org.junit.Assert.fail; +import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import java.util.HashMap; @@ -41,6 +42,9 @@ import org.springframework.hateoas.EntityModel; import org.springframework.hateoas.Link; import org.springframework.mock.web.MockHttpServletRequest; import org.springframework.mock.web.MockHttpServletResponse; +import org.springframework.security.core.Authentication; +import org.springframework.security.core.context.SecurityContext; +import org.springframework.security.core.context.SecurityContextHolder; /** * Tests functionality of {@link ConverterService}. @@ -76,6 +80,11 @@ public class ConverterServiceIT extends AbstractControllerIntegrationTest { mockHttpServletRequest.setAttribute("dspace.context", new Context()); MockHttpServletResponse mockHttpServletResponse = new MockHttpServletResponse(); requestService.startRequest(mockHttpServletRequest, mockHttpServletResponse); + Authentication authentication = mock(Authentication.class); + SecurityContext securityContext = mock(SecurityContext.class); + when(securityContext.getAuthentication()).thenReturn(authentication); + SecurityContextHolder.setContext(securityContext); + when(SecurityContextHolder.getContext().getAuthentication().getPrincipal()).thenReturn(eperson); } /** * When calling {@code toRest} with an object for which an appropriate {@link DSpaceConverter} can't be found, diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java new file mode 100644 index 0000000000..5c12bc1b14 --- /dev/null +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java @@ -0,0 +1,36 @@ +/** + * The contents of this file are subject to the license and copyright + * detailed in the LICENSE and NOTICE files at the root of the source + * tree and available online at + * + * http://www.dspace.org/license/ + */ +package org.dspace.app.rest.repository; + +import org.dspace.app.rest.model.MockObjectRest; +import org.dspace.core.Context; +import org.springframework.data.domain.Page; +import org.springframework.data.domain.Pageable; +import org.springframework.security.access.prepost.PreAuthorize; +import org.springframework.stereotype.Component; + +@Component(MockObjectRest.CATEGORY + "." + MockObjectRest.NAME) +public class MockObjectRestRepository extends DSpaceRestRepository { + + @Override + @PreAuthorize("permitAll()") + public MockObjectRest findOne(Context context, Long aLong) { + return null; + } + + @Override + @PreAuthorize("permitAll()") + public Page findAll(Context context, Pageable pageable) { + return null; + } + + @Override + public Class getDomainClass() { + return MockObjectRest.class; + } +} From 7be092813903c4e59f27109fd6702e5d281a1966 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Mon, 8 Jun 2020 15:12:40 +0200 Subject: [PATCH 11/24] [Task 71143] fixed checkstyle --- .../security/WebSecurityExpressionEvaluator.java | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java index 8d75c24b1c..0b1b69e6e1 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java @@ -47,7 +47,8 @@ public class WebSecurityExpressionEvaluator { } @SuppressWarnings("unchecked") - private EvaluationContext createEvaluationContext(SecurityExpressionHandler handler, HttpServletRequest request, HttpServletResponse response) { + private EvaluationContext createEvaluationContext(SecurityExpressionHandler handler, HttpServletRequest request, + HttpServletResponse response) { Authentication authentication = SecurityContextHolder.getContext().getAuthentication(); FilterInvocation filterInvocation = new FilterInvocation(request, response, EMPTY_CHAIN); @@ -56,9 +57,13 @@ public class WebSecurityExpressionEvaluator { private SecurityExpressionHandler getFilterSecurityHandler() { return securityExpressionHandlers.stream() - .filter(handler -> FilterInvocation.class.equals(GenericTypeResolver - .resolveTypeArgument(handler.getClass(), SecurityExpressionHandler.class))) + .filter(handler -> + FilterInvocation.class.equals( + GenericTypeResolver.resolveTypeArgument(handler.getClass(), + SecurityExpressionHandler.class))) .findAny() - .orElseThrow(() -> new IllegalStateException("No filter invocation security expression handler has been found! Handlers: " + securityExpressionHandlers.size())); + .orElseThrow(() -> new IllegalStateException("No filter invocation security" + + " expression handler has been found! Handlers: " + + securityExpressionHandlers.size())); } } \ No newline at end of file From 4a40b8d1028aaa37f712fd53243696650a99763b Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Tue, 9 Jun 2020 09:25:06 +0200 Subject: [PATCH 12/24] [Task 71308] applied feedback to the PreAuthorize parsing in converterService feature --- .../app/rest/converter/ConverterService.java | 6 ++++- .../WebSecurityExpressionEvaluator.java | 26 +++++++++++++++++++ .../rest/converter/ConverterServiceIT.java | 4 +++ .../repository/MockObjectRestRepository.java | 5 ++++ 4 files changed, 40 insertions(+), 1 deletion(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java index bf959f719e..fc786bfc85 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/converter/ConverterService.java @@ -108,12 +108,16 @@ public class ConverterService { R restObject = converter.convert(transformedModel, projection); if (restObject instanceof BaseObjectRest) { BaseObjectRest baseObjectRest = (BaseObjectRest) restObject; + // This section will verify whether the current user has permissions to retrieve the + // rest object. It'll only return the REST object if the permission is granted. + // If permission isn't granted, it'll return null String preAuthorizeValue = getPreAuthorizeAnnotationForBaseObject(baseObjectRest); if (!webSecurityExpressionEvaluator .evaluate(preAuthorizeValue, requestService.getCurrentRequest().getHttpServletRequest(), requestService.getCurrentRequest().getHttpServletResponse(), String.valueOf(baseObjectRest.getId()))) { - log.info("Access denied on " + restObject.getClass()); + log.debug("Access denied on " + restObject.getClass() + " with id: " + + ((BaseObjectRest) restObject).getId()); return null; } } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java index 0b1b69e6e1..364e93e297 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WebSecurityExpressionEvaluator.java @@ -22,6 +22,17 @@ import org.springframework.security.core.context.SecurityContextHolder; import org.springframework.security.web.FilterInvocation; import org.springframework.stereotype.Component; +/** + * This class will contain the logic to allow us to evaluate an expression given through a String. + * This will be used by the {@link org.dspace.app.rest.converter.ConverterService} for parsing + * the {@link org.springframework.security.access.prepost.PreAuthorize} annotations used on the findOne + * methods of RestRepositories. A String will be given to the evaluate method and that String will then + * be parsed and a boolean will be returned based on the condition in the String. + * For example: "hasPermission(#id, 'ITEM', 'READ')" is such a String + * This will be evaluated and if the current user has the permission to read an item with the given id, + * a true will be returned, if not it'll be false. + * This works on all the methods in {@link org.springframework.security.access.expression.SecurityExpressionRoot} + */ @Component public class WebSecurityExpressionEvaluator { @@ -31,10 +42,25 @@ public class WebSecurityExpressionEvaluator { private final List securityExpressionHandlers; + /** + * Constructor for this class that sets all the {@link SecurityExpressionHandler} objects in a list + * @param securityExpressionHandlers The {@link SecurityExpressionHandler} for this class + */ public WebSecurityExpressionEvaluator(List securityExpressionHandlers) { this.securityExpressionHandlers = securityExpressionHandlers; } + /** + * This method will have to be used to evaluate the String given. It'll parse the String and resolve + * it to a method in {@link org.springframework.security.access.expression.SecurityExpressionRoot} + * and evaluate it to then return a boolean + * @param securityExpression The String that resembles the expression that has to be parsed + * @param request The current request + * @param response The current response + * @param id The id for the Object that is the subject of the permission + * @return A boolean indicating whether the currentUser adheres to the + * permissions in the securityExpression String or not + */ public boolean evaluate(String securityExpression, HttpServletRequest request, HttpServletResponse response, String id) { SecurityExpressionHandler handler = getFilterSecurityHandler(); diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java index ea1a15dc04..685bd5dbfd 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/converter/ConverterServiceIT.java @@ -112,6 +112,10 @@ public class ConverterServiceIT extends AbstractControllerIntegrationTest { /** * When calling {@code toRest} with the default projection, the converter should run and no changes should be made. + * This converter.toRest will now also check permissions through the PreAuthorize annotation on the + * Repository's findOne method. Therefor a repository has been added for this MockObjectRest namely + * {@link org.dspace.app.rest.repository.MockObjectRestRepository} and added PreAuthorize annotations + * on the methods of this Repository */ @Test public void toRestWithDefaultProjection() { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java index 5c12bc1b14..0c54811249 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/repository/MockObjectRestRepository.java @@ -14,9 +14,14 @@ import org.springframework.data.domain.Pageable; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; +/** + * This class has been added to allow the MockObjectRest to act as an actual BaseObjectRest since they're + * expected to have a RestRepository + */ @Component(MockObjectRest.CATEGORY + "." + MockObjectRest.NAME) public class MockObjectRestRepository extends DSpaceRestRepository { + // Added a permitAll preAuthorize annotation to allow the object to be used in tests by every user @Override @PreAuthorize("permitAll()") public MockObjectRest findOne(Context context, Long aLong) { From faef05258a60e04bfcdaed9d8e81a0ec266b4a29 Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Wed, 10 Jun 2020 11:51:44 +0200 Subject: [PATCH 13/24] [Task 71335] removed unneeded PermissionEvaluatorPlugins and changed PreAuthorize annotations where necessary --- .../AuthorizationFeatureRestRepository.java | 6 +-- .../BitstreamFormatRestRepository.java | 1 + .../repository/EntityTypeRestRepository.java | 3 ++ .../TemplateItemRestRepository.java | 2 + ...onStatusRestPermissionEvaluatorPlugin.java | 31 -------------- .../AuthnRestPermissionEvaluatorPlugin.java | 31 -------------- ...nFeatureRestPermissionEvaluatorPlugin.java | 31 -------------- ...amFormatRestPermissionEvaluatorPlugin.java | 31 -------------- ...wseIndexRestPermissionEvaluatorPlugin.java | 31 -------------- ...ryResultRestPermissionEvaluatorPlugin.java | 31 -------------- ...tityTypeRestPermissionEvaluatorPlugin.java | 31 -------------- ...rceEntryRestPermissionEvaluatorPlugin.java | 31 -------------- ...alSourceRestPermissionEvaluatorPlugin.java | 31 -------------- ...gurationRestPermissionEvaluatorPlugin.java | 31 -------------- ...llectionRestPermissionEvaluatorPlugin.java | 31 -------------- ...MetadataRestPermissionEvaluatorPlugin.java | 31 -------------- ...ataFieldRestPermissionEvaluatorPlugin.java | 31 -------------- ...taSchemaRestPermissionEvaluatorPlugin.java | 31 -------------- ...tionshipRestPermissionEvaluatorPlugin.java | 31 -------------- ...shipTypeRestPermissionEvaluatorPlugin.java | 31 -------------- .../ScriptRestPermissionEvaluatorPlugin.java | 30 -------------- ...gurationRestPermissionEvaluatorPlugin.java | 31 -------------- ...rchEventRestPermissionEvaluatorPlugin.java | 31 -------------- ...hResultsRestPermissionEvaluatorPlugin.java | 31 -------------- ...hSupportRestPermissionEvaluatorPlugin.java | 31 -------------- .../SiteRestPermissionEvaluatorPlugin.java | 31 -------------- ...sSupportRestPermissionEvaluatorPlugin.java | 31 -------------- ...finitionRestPermissionEvaluatorPlugin.java | 31 -------------- ...sionFormRestPermissionEvaluatorPlugin.java | 31 -------------- ...onUploadRestPermissionEvaluatorPlugin.java | 31 -------------- ...nSectionRestPermissionEvaluatorPlugin.java | 31 -------------- ...lateItemRestPermissionEvaluatorPlugin.java | 31 -------------- ...iewEventRestPermissionEvaluatorPlugin.java | 31 -------------- ...owActionRestPermissionEvaluatorPlugin.java | 31 -------------- ...finitionRestPermissionEvaluatorPlugin.java | 31 -------------- ...flowStepRestPermissionEvaluatorPlugin.java | 31 -------------- .../AuthorizationFeatureRestRepositoryIT.java | 41 +++---------------- 37 files changed, 15 insertions(+), 1029 deletions(-) delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthenticationStatusRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthnRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthorizationFeatureRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BitstreamFormatRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BrowseIndexRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/DiscoveryResultRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EntityTypeRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceEntryRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/FacetConfigurationRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvestedCollectionRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvesterMetadataRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataFieldRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataSchemaRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipTypeRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ScriptRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchConfigurationRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchEventRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchResultsRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchSupportRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SiteRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatisticsSupportRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionDefinitionRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionFormRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionUploadRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissonSectionRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/TemplateItemRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ViewEventRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowActionRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowDefinitionRestPermissionEvaluatorPlugin.java delete mode 100644 dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowStepRestPermissionEvaluatorPlugin.java diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java index 0048898e22..b5d102215c 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java @@ -42,13 +42,13 @@ public class AuthorizationFeatureRestRepository extends DSpaceRestRepository findAll(Context context, Pageable pageable) { return converter.toRestPage(authorizationFeatureService.findAll(), pageable, utils.obtainProjection()); } - @PreAuthorize("hasAuthority('ADMIN')") + @PreAuthorize("permitAll()") @Override public AuthorizationFeatureRest findOne(Context context, String id) { AuthorizationFeature authzFeature = authorizationFeatureService.find(id); @@ -58,7 +58,7 @@ public class AuthorizationFeatureRestRepository extends DSpaceRestRepository findByResourceType(@Parameter(value = "type", required = true) String type, Pageable pageable) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java index 49585ee9db..e63069233e 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java @@ -57,6 +57,7 @@ public class BitstreamFormatRestRepository extends DSpaceRestRepository findAll(Context context, Pageable pageable) { try { List bit = bitstreamFormatService.findAll(context); diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EntityTypeRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EntityTypeRestRepository.java index e6c6fcea53..29db39d879 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EntityTypeRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EntityTypeRestRepository.java @@ -30,6 +30,7 @@ public class EntityTypeRestRepository extends DSpaceRestRepository findAll(Context context, Pageable pageable) { try { List entityTypes = entityTypeService.findAll(context); @@ -52,6 +54,7 @@ public class EntityTypeRestRepository extends DSpaceRestRepository getDomainClass() { return EntityTypeRest.class; } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/TemplateItemRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/TemplateItemRestRepository.java index 2e1436d4fb..ad80140481 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/TemplateItemRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/TemplateItemRestRepository.java @@ -28,6 +28,7 @@ import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; import org.springframework.data.domain.Pageable; import org.springframework.data.rest.webmvc.ResourceNotFoundException; +import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; /** @@ -49,6 +50,7 @@ public class TemplateItemRestRepository extends DSpaceRestRepository resourcePatch; @Override + @PreAuthorize("permitAll()") public TemplateItemRest findOne(Context context, UUID uuid) { Item item = null; try { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthenticationStatusRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthenticationStatusRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 220c75d893..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthenticationStatusRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.AuthenticationStatusRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to AuthenticationStatusRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class AuthenticationStatusRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(AuthenticationStatusRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthnRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthnRestPermissionEvaluatorPlugin.java deleted file mode 100644 index d310e781b4..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthnRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.AuthnRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to AuthnRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class AuthnRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(AuthnRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthorizationFeatureRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthorizationFeatureRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 2a2dec0655..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/AuthorizationFeatureRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.AuthorizationFeatureRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to AuthorizationRest endpoints. It will return true because access can be granted - * anytime it's linked from another resource. - */ -@Component -public class AuthorizationFeatureRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(AuthorizationFeatureRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BitstreamFormatRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BitstreamFormatRestPermissionEvaluatorPlugin.java deleted file mode 100644 index f5027a5f14..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BitstreamFormatRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.BitstreamFormatRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to BitstreamFormatRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class BitstreamFormatRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(BitstreamFormatRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BrowseIndexRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BrowseIndexRestPermissionEvaluatorPlugin.java deleted file mode 100644 index a4f57e1ea7..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/BrowseIndexRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.BrowseIndexRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to BrowseIndexRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class BrowseIndexRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(BrowseIndexRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/DiscoveryResultRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/DiscoveryResultRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 04207e4a93..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/DiscoveryResultRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.DiscoveryResultsRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to DiscoveryResultsRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class DiscoveryResultRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(DiscoveryResultsRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EntityTypeRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EntityTypeRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 017e0c2b56..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EntityTypeRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.EntityTypeRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to EntityTypeRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class EntityTypeRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(EntityTypeRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceEntryRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceEntryRestPermissionEvaluatorPlugin.java deleted file mode 100644 index fc050b4d3e..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceEntryRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.ExternalSourceEntryRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to ExternalSourceEntryRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class ExternalSourceEntryRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(ExternalSourceEntryRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 5340251f0d..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ExternalSourceRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.ExternalSourceRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to ExternalSourceRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class ExternalSourceRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(ExternalSourceRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/FacetConfigurationRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/FacetConfigurationRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 5a9432d466..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/FacetConfigurationRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.FacetConfigurationRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to FacetConfigurationRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class FacetConfigurationRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(FacetConfigurationRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvestedCollectionRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvestedCollectionRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 9f78f5a043..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvestedCollectionRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.HarvestedCollectionRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to HarvestedCollectionRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class HarvestedCollectionRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(HarvestedCollectionRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvesterMetadataRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvesterMetadataRestPermissionEvaluatorPlugin.java deleted file mode 100644 index ec0e3e202b..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/HarvesterMetadataRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.HarvesterMetadataRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to HarvesterMetadataRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class HarvesterMetadataRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(HarvesterMetadataRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataFieldRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataFieldRestPermissionEvaluatorPlugin.java deleted file mode 100644 index c53fffe5d0..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataFieldRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.MetadataFieldRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to MetadataFieldRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class MetadataFieldRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(MetadataFieldRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataSchemaRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataSchemaRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 07da54a170..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/MetadataSchemaRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.MetadataSchemaRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to MetadataSchemaRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class MetadataSchemaRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(MetadataSchemaRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 690a8bde9f..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.RelationshipRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to RelationshipRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class RelationshipRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(RelationshipRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipTypeRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipTypeRestPermissionEvaluatorPlugin.java deleted file mode 100644 index b371c8a516..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/RelationshipTypeRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.RelationshipTypeRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to RelationshipTypeRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class RelationshipTypeRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(RelationshipTypeRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ScriptRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ScriptRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 3672ad47c4..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ScriptRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,30 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.ScriptRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle Permissions for the {@link ScriptRest} object and its calls - */ -@Component -public class ScriptRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(ScriptRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchConfigurationRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchConfigurationRestPermissionEvaluatorPlugin.java deleted file mode 100644 index b540b4d855..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchConfigurationRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SearchConfigurationRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SearchConfigurationRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SearchConfigurationRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SearchConfigurationRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchEventRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchEventRestPermissionEvaluatorPlugin.java deleted file mode 100644 index e7124b834e..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchEventRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SearchEventRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SearchEventRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SearchEventRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SearchEventRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchResultsRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchResultsRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 8e1ef2ada7..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchResultsRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SearchResultsRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SearchResultsRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SearchResultsRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SearchResultsRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchSupportRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchSupportRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 6ad65351c6..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SearchSupportRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SearchSupportRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SearchSupportRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SearchSupportRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SearchSupportRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SiteRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SiteRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 6385b2c751..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SiteRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SiteRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SiteRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SiteRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SiteRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatisticsSupportRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatisticsSupportRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 190afafe14..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/StatisticsSupportRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.StatisticsSupportRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to StatisticsSupportRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class StatisticsSupportRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(StatisticsSupportRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionDefinitionRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionDefinitionRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 7efb24a6ee..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionDefinitionRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SubmissionDefinitionRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SubmissionDefinitionRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SubmissionDefinitionRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SubmissionDefinitionRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionFormRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionFormRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 19db77ca57..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionFormRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SubmissionFormRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SubmissionFormRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SubmissionFormRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SubmissionFormRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionUploadRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionUploadRestPermissionEvaluatorPlugin.java deleted file mode 100644 index aa15522a1c..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissionUploadRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SubmissionUploadRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SubmissionUploadRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SubmissionUploadRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SubmissionUploadRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissonSectionRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissonSectionRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 7c998d48d0..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/SubmissonSectionRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.SubmissionSectionRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to SubmissionSectionRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class SubmissonSectionRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(SubmissionSectionRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/TemplateItemRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/TemplateItemRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 192bdf040b..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/TemplateItemRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.TemplateItemRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to TemplateItemRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class TemplateItemRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(TemplateItemRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ViewEventRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ViewEventRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 097bfc1e3a..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/ViewEventRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.ViewEventRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to ViewEventRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class ViewEventRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(ViewEventRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowActionRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowActionRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 4f4180e960..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowActionRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.WorkflowActionRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to WorkflowActionRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class WorkflowActionRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(WorkflowActionRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowDefinitionRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowDefinitionRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 6168b3b370..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowDefinitionRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.WorkflowDefinitionRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to WorkflowDefinitionRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class WorkflowDefinitionRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(WorkflowDefinitionRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowStepRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowStepRestPermissionEvaluatorPlugin.java deleted file mode 100644 index 8c8216c412..0000000000 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/WorkflowStepRestPermissionEvaluatorPlugin.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * The contents of this file are subject to the license and copyright - * detailed in the LICENSE and NOTICE files at the root of the source - * tree and available online at - * - * http://www.dspace.org/license/ - */ -package org.dspace.app.rest.security; - -import java.io.Serializable; - -import org.apache.commons.lang3.StringUtils; -import org.dspace.app.rest.model.WorkflowStepRest; -import org.springframework.security.core.Authentication; -import org.springframework.stereotype.Component; - -/** - * This class will handle calls made to WorkflowStepRest endpoints. - * It will return true because access can be granted anytime it's linked from another resource - */ -@Component -public class WorkflowStepRestPermissionEvaluatorPlugin extends RestObjectPermissionEvaluatorPlugin { - @Override - public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, - DSpaceRestPermission restPermission) { - if (!StringUtils.equalsIgnoreCase(WorkflowStepRest.NAME, targetType)) { - return false; - } - return true; - } -} diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java index a3556ad503..bffcbcb54f 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java @@ -46,19 +46,12 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte public void findAllTest() throws Exception { int featuresNum = authzFeatureService.findAll().size(); int expReturn = featuresNum > 20 ? 20 : featuresNum; - String adminToken = getAuthToken(admin.getEmail(), password); - // verify that only the admin can access the endpoint (see subsequent call in the method) - getClient(adminToken).perform(get("/api/authz/features")).andExpect(status().isOk()) + getClient().perform(get("/api/authz/features")).andExpect(status().isOk()) .andExpect(jsonPath("$._embedded.features", Matchers.hasSize(is(expReturn)))) .andExpect(jsonPath("$._links.self.href", Matchers.containsString("/api/authz/features"))) .andExpect(jsonPath("$.page.size", is(20))) .andExpect(jsonPath("$.page.totalElements", is(featuresNum))); - // verify that anonymous user cannot access - getClient().perform(get("/api/authz/features")).andExpect(status().isUnauthorized()); - // verify that normal user cannot access - String epersonAuthToken = getAuthToken(eperson.getEmail(), password); - getClient(epersonAuthToken).perform(get("/api/authz/features")).andExpect(status().isForbidden()); } @@ -108,30 +101,17 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte * @throws Exception */ public void findOneTest() throws Exception { - String adminToken = getAuthToken(admin.getEmail(), password); - // verify that only the admin can access the endpoint (see subsequent call in the method) - getClient(adminToken).perform(get("/api/authz/features/withdrawItem")).andExpect(status().isOk()) + getClient().perform(get("/api/authz/features/withdrawItem")).andExpect(status().isOk()) .andExpect(jsonPath("$.id", is("withdrawItem"))) .andExpect(jsonPath("$.description", Matchers.any(String.class))) .andExpect(jsonPath("$.resourcetypes", Matchers.contains("core.item"))) .andExpect(jsonPath("$.type", is("feature"))); - // verify that anonymous user cannot access - getClient().perform(get("/api/authz/features/withdrawItem")).andExpect(status().isUnauthorized()); - // verify that normal user cannot access - String epersonAuthToken = getAuthToken(eperson.getEmail(), password); - getClient(epersonAuthToken).perform(get("/api/authz/features/withdrawItem")).andExpect(status().isForbidden()); } @Test public void findOneNotFoundTest() throws Exception { - String adminToken = getAuthToken(admin.getEmail(), password); - // verify that only the admin can access the endpoint and get the not found response code - // (see subsequent calls in the method for unauthorized and forbidden attempts) - getClient(adminToken).perform(get("/api/authz/features/not-existing-feature")).andExpect(status().isNotFound()); - // verify that anonymous user cannot access, without information disclosure - getClient().perform(get("/api/authz/features/not-existing-feature")).andExpect(status().isUnauthorized()); - // verify that normal user cannot access, without information disclosure - getClient(adminToken).perform(get("/api/authz/features/1")).andExpect(status().isNotFound()); + getClient().perform(get("/api/authz/features/not-existing-feature")).andExpect(status().isNotFound()); + getClient().perform(get("/api/authz/features/1")).andExpect(status().isNotFound()); } @Test @@ -142,10 +122,8 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte */ public void findByResourceTypeTest() throws Exception { AuthorizationFeature alwaysTrueFeature = authzFeatureService.find(AlwaysTrueFeature.NAME); - String adminToken = getAuthToken(admin.getEmail(), password); for (String type : alwaysTrueFeature.getSupportedTypes()) { - // verify that only the admin can access the endpoint (see subsequent call in the method) - getClient(adminToken).perform(get("/api/authz/features/search/resourcetype").param("type", type)) + getClient().perform(get("/api/authz/features/search/resourcetype").param("type", type)) .andExpect(status().isOk()) .andExpect(jsonPath("$", JsonPathMatchers.hasJsonPath("$._embedded.features", @@ -158,15 +136,8 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte Matchers.containsString("/api/authz/features/search/resourcetype"))); } // verify that the right response code is returned also for not existing types - getClient(adminToken).perform(get("/api/authz/features/search/resourcetype").param("type", "NOT-EXISTING")) + getClient().perform(get("/api/authz/features/search/resourcetype").param("type", "NOT-EXISTING")) .andExpect(status().isOk()).andExpect(jsonPath("$.page.totalElements", is(0))); - // verify that anonymous user cannot access, without information disclosure - getClient().perform(get("/api/authz/features/search/resourcetype").param("type", "core.item")) - .andExpect(status().isUnauthorized()); - // verify that normal user cannot access, without information disclosure - String epersonAuthToken = getAuthToken(eperson.getEmail(), password); - getClient(epersonAuthToken).perform(get("/api/authz/features/search/resourcetype").param("type", "core.item")) - .andExpect(status().isForbidden()); } From 56629fd34e82b9a29ada8b65a7b1a41557b99f7f Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Wed, 10 Jun 2020 11:53:24 +0200 Subject: [PATCH 14/24] [Task 71335] removed wrongly added preAuthorize on bitstreamFormatRestRepository#findAll --- .../app/rest/repository/BitstreamFormatRestRepository.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java index e63069233e..49585ee9db 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/BitstreamFormatRestRepository.java @@ -57,7 +57,6 @@ public class BitstreamFormatRestRepository extends DSpaceRestRepository findAll(Context context, Pageable pageable) { try { List bit = bitstreamFormatService.findAll(context); From 5f44fe11f7da5885f7f6f285fac23fd3a5c2605e Mon Sep 17 00:00:00 2001 From: Raf Ponsaerts Date: Wed, 10 Jun 2020 13:50:17 +0200 Subject: [PATCH 15/24] Undo findall preauthorize annotation change on AuthorizationFeatureRestRepository --- .../AuthorizationFeatureRestRepository.java | 4 +- .../AuthorizationFeatureRestRepositoryIT.java | 95 +++++++++++-------- 2 files changed, 58 insertions(+), 41 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java index b5d102215c..62781fe8e8 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/AuthorizationFeatureRestRepository.java @@ -42,7 +42,7 @@ public class AuthorizationFeatureRestRepository extends DSpaceRestRepository findAll(Context context, Pageable pageable) { return converter.toRestPage(authorizationFeatureService.findAll(), pageable, utils.obtainProjection()); @@ -58,7 +58,7 @@ public class AuthorizationFeatureRestRepository extends DSpaceRestRepository findByResourceType(@Parameter(value = "type", required = true) String type, Pageable pageable) { diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java index bffcbcb54f..0aadff7a99 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationFeatureRestRepositoryIT.java @@ -29,7 +29,7 @@ import org.springframework.beans.factory.annotation.Autowired; /** * Test suite for the Authorization Feature endpoint - * + * * @author Andrea Bollini (andrea.bollini at 4science.it) * */ @@ -46,12 +46,19 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte public void findAllTest() throws Exception { int featuresNum = authzFeatureService.findAll().size(); int expReturn = featuresNum > 20 ? 20 : featuresNum; + String adminToken = getAuthToken(admin.getEmail(), password); - getClient().perform(get("/api/authz/features")).andExpect(status().isOk()) - .andExpect(jsonPath("$._embedded.features", Matchers.hasSize(is(expReturn)))) - .andExpect(jsonPath("$._links.self.href", Matchers.containsString("/api/authz/features"))) - .andExpect(jsonPath("$.page.size", is(20))) - .andExpect(jsonPath("$.page.totalElements", is(featuresNum))); + // verify that only the admin can access the endpoint (see subsequent call in the method) + getClient(adminToken).perform(get("/api/authz/features")).andExpect(status().isOk()) + .andExpect(jsonPath("$._embedded.features", Matchers.hasSize(is(expReturn)))) + .andExpect(jsonPath("$._links.self.href", Matchers.containsString("/api/authz/features"))) + .andExpect(jsonPath("$.page.size", is(20))) + .andExpect(jsonPath("$.page.totalElements", is(featuresNum))); + // verify that anonymous user cannot access + getClient().perform(get("/api/authz/features")).andExpect(status().isUnauthorized()); + // verify that normal user cannot access + String epersonAuthToken = getAuthToken(eperson.getEmail(), password); + getClient(epersonAuthToken).perform(get("/api/authz/features")).andExpect(status().isForbidden()); } @@ -71,21 +78,21 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte AtomicReference idRef = new AtomicReference(); getClient(adminToken) - .perform(get("/api/authz/features").param("page", String.valueOf(page)).param("size", "1")) - .andExpect(status().isOk()).andExpect(jsonPath("$._embedded.features", Matchers.hasSize(is(1)))) - .andExpect(jsonPath("$._links.self.href", Matchers.containsString("/api/authz/features"))) - .andExpect( - (page == 0) ? jsonPath("$._links.prev.href").doesNotExist() - : jsonPath("$._links.prev.href", Matchers.containsString("/api/authz/features"))) - .andExpect((page == featuresNum - 1) - ? jsonPath("$._links.next.href").doesNotExist() - : jsonPath("$._links.next.href", Matchers.containsString("/api/authz/features"))) - .andExpect(jsonPath("$._links.first.href", Matchers.containsString("/api/authz/features"))) - .andExpect(jsonPath("$._links.last.href", Matchers.containsString("/api/authz/features"))) - .andExpect(jsonPath("$.page.size", is(1))) - .andExpect(jsonPath("$.page.totalElements", is(Integer.valueOf(featuresNum)))) - .andDo(result -> idRef - .set(read(result.getResponse().getContentAsString(), "$._embedded.features[0].id"))); + .perform(get("/api/authz/features").param("page", String.valueOf(page)).param("size", "1")) + .andExpect(status().isOk()).andExpect(jsonPath("$._embedded.features", Matchers.hasSize(is(1)))) + .andExpect(jsonPath("$._links.self.href", Matchers.containsString("/api/authz/features"))) + .andExpect( + (page == 0) ? jsonPath("$._links.prev.href").doesNotExist() + : jsonPath("$._links.prev.href", Matchers.containsString("/api/authz/features"))) + .andExpect((page == featuresNum - 1) + ? jsonPath("$._links.next.href").doesNotExist() + : jsonPath("$._links.next.href", Matchers.containsString("/api/authz/features"))) + .andExpect(jsonPath("$._links.first.href", Matchers.containsString("/api/authz/features"))) + .andExpect(jsonPath("$._links.last.href", Matchers.containsString("/api/authz/features"))) + .andExpect(jsonPath("$.page.size", is(1))) + .andExpect(jsonPath("$.page.totalElements", is(Integer.valueOf(featuresNum)))) + .andDo(result -> idRef + .set(read(result.getResponse().getContentAsString(), "$._embedded.features[0].id"))); if (idRef.get() == null || featureIDs.contains(idRef.get())) { fail("Duplicate feature " + idRef.get() + " returned at page " + page); @@ -102,16 +109,16 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte */ public void findOneTest() throws Exception { getClient().perform(get("/api/authz/features/withdrawItem")).andExpect(status().isOk()) - .andExpect(jsonPath("$.id", is("withdrawItem"))) - .andExpect(jsonPath("$.description", Matchers.any(String.class))) - .andExpect(jsonPath("$.resourcetypes", Matchers.contains("core.item"))) - .andExpect(jsonPath("$.type", is("feature"))); + .andExpect(jsonPath("$.id", is("withdrawItem"))) + .andExpect(jsonPath("$.description", Matchers.any(String.class))) + .andExpect(jsonPath("$.resourcetypes", Matchers.contains("core.item"))) + .andExpect(jsonPath("$.type", is("feature"))); } @Test public void findOneNotFoundTest() throws Exception { getClient().perform(get("/api/authz/features/not-existing-feature")).andExpect(status().isNotFound()); - getClient().perform(get("/api/authz/features/1")).andExpect(status().isNotFound()); + } @Test @@ -122,22 +129,32 @@ public class AuthorizationFeatureRestRepositoryIT extends AbstractControllerInte */ public void findByResourceTypeTest() throws Exception { AuthorizationFeature alwaysTrueFeature = authzFeatureService.find(AlwaysTrueFeature.NAME); + String adminToken = getAuthToken(admin.getEmail(), password); for (String type : alwaysTrueFeature.getSupportedTypes()) { - getClient().perform(get("/api/authz/features/search/resourcetype").param("type", type)) - .andExpect(status().isOk()) - .andExpect(jsonPath("$", - JsonPathMatchers.hasJsonPath("$._embedded.features", - Matchers.everyItem( - JsonPathMatchers.hasJsonPath("$.resourcetypes", - Matchers.hasItem(is(type)))) - ))) - .andExpect( - jsonPath("$._links.self.href", - Matchers.containsString("/api/authz/features/search/resourcetype"))); + // verify that only the admin can access the endpoint (see subsequent call in the method) + getClient(adminToken).perform(get("/api/authz/features/search/resourcetype").param("type", type)) + .andExpect(status().isOk()) + .andExpect(jsonPath("$", + JsonPathMatchers.hasJsonPath("$._embedded.features", + Matchers.everyItem( + JsonPathMatchers.hasJsonPath( + "$.resourcetypes", + Matchers.hasItem(is(type)))) + ))) + .andExpect( + jsonPath("$._links.self.href", + Matchers.containsString("/api/authz/features/search/resourcetype"))); } // verify that the right response code is returned also for not existing types - getClient().perform(get("/api/authz/features/search/resourcetype").param("type", "NOT-EXISTING")) - .andExpect(status().isOk()).andExpect(jsonPath("$.page.totalElements", is(0))); + getClient(adminToken).perform(get("/api/authz/features/search/resourcetype").param("type", "NOT-EXISTING")) + .andExpect(status().isOk()).andExpect(jsonPath("$.page.totalElements", is(0))); + // verify that anonymous user cannot access, without information disclosure + getClient().perform(get("/api/authz/features/search/resourcetype").param("type", "core.item")) + .andExpect(status().isUnauthorized()); + // verify that normal user cannot access, without information disclosure + String epersonAuthToken = getAuthToken(eperson.getEmail(), password); + getClient(epersonAuthToken).perform(get("/api/authz/features/search/resourcetype").param("type", "core.item")) + .andExpect(status().isForbidden()); } From f5b7d5854bbbfd7ca7af85632a9eb6e3114bdf9d Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Thu, 11 Jun 2020 11:20:48 +0200 Subject: [PATCH 16/24] Implement community feedbacks --- .../src/main/java/org/dspace/eperson/GroupServiceImpl.java | 4 +++- .../rest/security/EPersonRestAuthenticationProvider.java | 6 +++--- 2 files changed, 6 insertions(+), 4 deletions(-) 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 449ddca973..4437516315 100644 --- a/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/eperson/GroupServiceImpl.java @@ -659,7 +659,9 @@ public class GroupServiceImpl extends DSpaceObjectServiceImpl implements } } } else { - if (AuthorizeConfiguration.canCollectionAdminManagePolicies()) { + if (AuthorizeConfiguration.canCollectionAdminManagePolicies() + || AuthorizeConfiguration.canCommunityAdminManagePolicies() + || AuthorizeConfiguration.canCommunityAdminManageCollectionWorkflows()) { List groups = new ArrayList(); groups.add(group); List policies = resourcePolicyService.find(context, null, groups, diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java index 576c7e7e7d..9a5faf01ee 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java @@ -144,11 +144,11 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider if (eperson != null) { boolean isAdmin = false; boolean isCommunityAdmin = false; - boolean isColectionAdmin = false; + boolean isCollectionAdmin = false; try { isAdmin = authorizeService.isAdmin(context, eperson); isCommunityAdmin = authorizeService.isCommunityAdmin(context, eperson); - isColectionAdmin = authorizeService.isCollectionAdmin(context, eperson); + isCollectionAdmin = authorizeService.isCollectionAdmin(context, eperson); } catch (SQLException e) { log.error("SQL error while checking for admin rights", e); } @@ -156,7 +156,7 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider if (isAdmin) { authorities.add(new SimpleGrantedAuthority(ADMIN_GRANT)); } else if ((isCommunityAdmin && AuthorizeUtil.canCommunityAdminManageAccounts()) - || (isColectionAdmin && AuthorizeUtil.canCollectionAdminManageAccounts())) { + || (isCollectionAdmin && AuthorizeUtil.canCollectionAdminManageAccounts())) { authorities.add(new SimpleGrantedAuthority(ACCOUNT_ADMIN_GRANT)); } From c3d043f2a39710eeb3b0cbaaef75869bb1f6811d Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 12 Jun 2020 11:13:59 +0200 Subject: [PATCH 17/24] renamed variable --- .../org/dspace/app/rest/repository/EPersonRestRepository.java | 2 +- .../org/dspace/app/rest/repository/GroupRestRepository.java | 2 +- .../app/rest/security/EPersonRestAuthenticationProvider.java | 4 ++-- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java index 020f8b6af0..e044346c2b 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java @@ -155,7 +155,7 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository findByMetadata(@Parameter(value = "query", required = true) String query, Pageable pageable) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java index f150ec0f3c..b531c4fcb7 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/GroupRestRepository.java @@ -131,7 +131,7 @@ public class GroupRestRepository extends DSpaceObjectRestRepository findByMetadata(@Parameter(value = "query", required = true) String query, Pageable pageable) { diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java index 9a5faf01ee..a470515419 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestAuthenticationProvider.java @@ -48,7 +48,7 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider private static final Logger log = LoggerFactory.getLogger(EPersonRestAuthenticationProvider.class); - public static final String ACCOUNT_ADMIN_GRANT = "ACCOUNT_ADMIN"; + public static final String MANAGE_ACCESS_GROUP = "MANAGE_ACCESS_GROUP"; @Autowired private AuthenticationService authenticationService; @@ -157,7 +157,7 @@ public class EPersonRestAuthenticationProvider implements AuthenticationProvider authorities.add(new SimpleGrantedAuthority(ADMIN_GRANT)); } else if ((isCommunityAdmin && AuthorizeUtil.canCommunityAdminManageAccounts()) || (isCollectionAdmin && AuthorizeUtil.canCollectionAdminManageAccounts())) { - authorities.add(new SimpleGrantedAuthority(ACCOUNT_ADMIN_GRANT)); + authorities.add(new SimpleGrantedAuthority(MANAGE_ACCESS_GROUP)); } authorities.add(new SimpleGrantedAuthority(AUTHENTICATED_GRANT)); From 381fe08ad029322c84049c12affb2d287b47ecae Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 12 Jun 2020 11:15:56 +0200 Subject: [PATCH 18/24] added missing annotations --- .../main/java/org/dspace/authorize/AuthorizeServiceImpl.java | 2 ++ 1 file changed, 2 insertions(+) diff --git a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java index 2ebecf2005..07a7ac70d3 100644 --- a/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authorize/AuthorizeServiceImpl.java @@ -433,6 +433,7 @@ public class AuthorizeServiceImpl implements AuthorizeService { return isCommunityAdmin(c, e); } + @Override public boolean isCommunityAdmin(Context c, EPerson e) throws SQLException { if (e != null) { List policies = resourcePolicyService.find(c, e, @@ -452,6 +453,7 @@ public class AuthorizeServiceImpl implements AuthorizeService { return isCollectionAdmin(c, e); } + @Override public boolean isCollectionAdmin(Context c, EPerson e) throws SQLException { if (e != null) { List policies = resourcePolicyService.find(c, e, From 25adabe0a4ad2c3402c68b5f6cf930bfff92fdd3 Mon Sep 17 00:00:00 2001 From: Mykhaylo Date: Fri, 12 Jun 2020 11:18:52 +0200 Subject: [PATCH 19/24] cleanup tests --- .../CommunityAdminGroupRestControllerIT.java | 52 +------------------ .../app/rest/GroupRestRepositoryIT.java | 22 +------- 2 files changed, 3 insertions(+), 71 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java index bf606def8c..fb00219a4d 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/CommunityAdminGroupRestControllerIT.java @@ -483,11 +483,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.epersons", Matchers.not(Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) )))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -513,9 +508,8 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) ))); - context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.admin-group", false); - context.restoreAuthSystemState(); getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) .andExpect(status().isForbidden()); @@ -526,11 +520,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) ))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -559,11 +548,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( GroupMatcher.matchGroupWithName(group.getName()) )))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -590,9 +574,7 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg GroupMatcher.matchGroupWithName(group.getName()) ))); - context.turnOffAuthorisationSystem(); configurationService.setProperty("core.authorization.community-admin.admin-group", false); - context.restoreAuthSystemState(); getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) .andExpect(status().isForbidden()); @@ -603,10 +585,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( GroupMatcher.matchGroupWithName(group.getName()) ))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.admin-group", true); - context.restoreAuthSystemState(); } @Test @@ -697,12 +675,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.epersons", Matchers.not(Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) )))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -728,10 +700,9 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) ))); - context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); configurationService.setProperty("core.authorization.collection-admin.admin-group", false); - context.restoreAuthSystemState(); getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) .andExpect(status().isForbidden()); @@ -742,12 +713,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) ))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -777,12 +742,6 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( GroupMatcher.matchGroupWithName(group.getName()) )))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -809,10 +768,8 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg GroupMatcher.matchGroupWithName(group.getName()) ))); - context.turnOffAuthorisationSystem(); configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); configurationService.setProperty("core.authorization.collection-admin.admin-group", false); - context.restoreAuthSystemState(); getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) .andExpect(status().isForbidden()); @@ -823,10 +780,5 @@ public class CommunityAdminGroupRestControllerIT extends AbstractControllerInteg .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( GroupMatcher.matchGroupWithName(group.getName()) ))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); } } 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 ff5e69c396..868b5d271e 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 @@ -2819,10 +2819,9 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) ))); - context.turnOffAuthorisationSystem(); + configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); configurationService.setProperty("core.authorization.collection-admin.admin-group", false); - context.restoreAuthSystemState(); getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/epersons/" + ePerson.getID())) .andExpect(status().isForbidden()); @@ -2833,12 +2832,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(jsonPath("$._embedded.epersons", Matchers.hasItem( EPersonMatcher.matchEPersonOnEmail(ePerson.getEmail()) ))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -2868,12 +2861,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(jsonPath("$._embedded.subgroups", Matchers.not(Matchers.hasItem( GroupMatcher.matchGroupWithName(group.getName()) )))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); - } @Test @@ -2900,10 +2887,8 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { GroupMatcher.matchGroupWithName(group.getName()) ))); - context.turnOffAuthorisationSystem(); configurationService.setProperty("core.authorization.community-admin.collection.admin-group", false); configurationService.setProperty("core.authorization.collection-admin.admin-group", false); - context.restoreAuthSystemState(); getClient(token).perform(delete("/api/eperson/groups/" + adminGroup.getID() + "/subgroups/" + group.getID())) .andExpect(status().isForbidden()); @@ -2914,11 +2899,6 @@ public class GroupRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(jsonPath("$._embedded.subgroups", Matchers.hasItem( GroupMatcher.matchGroupWithName(group.getName()) ))); - - context.turnOffAuthorisationSystem(); - configurationService.setProperty("core.authorization.community-admin.collection.admin-group", true); - configurationService.setProperty("core.authorization.collection-admin.admin-group", true); - context.restoreAuthSystemState(); } } From 8172dd5fb0688ef2539af96b5a470b6187a4f937 Mon Sep 17 00:00:00 2001 From: benbosman Date: Thu, 18 Jun 2020 17:05:31 +0200 Subject: [PATCH 20/24] Re-enable findByObjectAndFeatureTest The findByObjectAndFeatureTest can be used again thanks to https://github.com/DSpace/DSpace/pull/2778/files#diff-311a7e07c257d9c869f4de1347f5f761R51 --- .../org/dspace/app/rest/AuthorizationRestRepositoryIT.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java index 22188a36af..7092bb62d7 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java @@ -805,11 +805,6 @@ public class AuthorizationRestRepositoryIT extends AbstractControllerIntegration * * @throws Exception */ - // This test currently doesn't work as expected since the AuthorizationFeatureRestRepository#findOne method - // is only exposed to admins. Currently we're performing checks on the individual REST objects with its findOne - // Permission constraints, which is ADMIN in this case. Seeing as we're trying to retrieve it with a normal - // EPerson token in the second test, this will fail. - @Ignore public void findByObjectAndFeatureTest() throws Exception { context.turnOffAuthorisationSystem(); Community com = CommunityBuilder.createCommunity(context).withName("A test community").build(); From 559001cf431a987ee03563c392003195f141baf8 Mon Sep 17 00:00:00 2001 From: benbosman Date: Thu, 18 Jun 2020 17:16:56 +0200 Subject: [PATCH 21/24] Removing unused import --- .../java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java | 1 - 1 file changed, 1 deletion(-) diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java index 7092bb62d7..05631790e3 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/AuthorizationRestRepositoryIT.java @@ -52,7 +52,6 @@ import org.dspace.eperson.Group; import org.dspace.services.ConfigurationService; import org.hamcrest.Matchers; import org.junit.Before; -import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; From 9b2451754676740535841001593b6be431e8c969 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Thu, 18 Jun 2020 23:49:25 +0200 Subject: [PATCH 22/24] Fix obviously wrong implementation --- .../SubmissionUploadRestRepository.java | 38 ++++++------------- 1 file changed, 11 insertions(+), 27 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java index 3ea5989f5a..a359a7ec4f 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java @@ -8,6 +8,7 @@ package org.dspace.app.rest.repository; import java.util.ArrayList; +import java.util.Collection; import java.util.List; import org.apache.commons.lang3.StringUtils; @@ -16,10 +17,6 @@ import org.dspace.app.rest.model.AccessConditionOptionRest; import org.dspace.app.rest.model.SubmissionUploadRest; import org.dspace.app.rest.projection.Projection; import org.dspace.app.rest.utils.DateMathParser; -import org.dspace.app.util.SubmissionConfig; -import org.dspace.app.util.SubmissionConfigReader; -import org.dspace.app.util.SubmissionConfigReaderException; -import org.dspace.app.util.SubmissionStepConfig; import org.dspace.core.Context; import org.dspace.eperson.Group; import org.dspace.eperson.service.GroupService; @@ -28,7 +25,6 @@ import org.dspace.submit.model.UploadConfiguration; import org.dspace.submit.model.UploadConfigurationService; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Page; -import org.springframework.data.domain.PageImpl; import org.springframework.data.domain.Pageable; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.stereotype.Component; @@ -45,8 +41,6 @@ public class SubmissionUploadRestRepository extends DSpaceRestRepository findAll(Context context, Pageable pageable) { - List subConfs = new ArrayList(); - subConfs = submissionConfigReader.getAllSubmissionConfigs(pageable.getPageSize(), - Math.toIntExact(pageable.getOffset())); + Collection uploadConfigs = uploadConfigurationService.getMap().values(); Projection projection = utils.obtainProjection(); List results = new ArrayList<>(); - for (SubmissionConfig config : subConfs) { - for (int i = 0; i < config.getNumberOfSteps(); i++) { - SubmissionStepConfig step = config.getStep(i); - if (SubmissionStepConfig.UPLOAD_STEP_NAME.equals(step.getType())) { - UploadConfiguration uploadConfig = uploadConfigurationService.getMap().get(step.getId()); - if (uploadConfig != null) { - try { - results.add(convert(context, uploadConfig, projection)); - } catch (Exception e) { - log.error(e.getMessage(), e); - } - } + List configNames = new ArrayList(); + for (UploadConfiguration uploadConfig : uploadConfigs) { + if (!configNames.contains(uploadConfig.getName())) { + configNames.add(uploadConfig.getName()); + try { + results.add(convert(context, uploadConfig, projection)); + } catch (Exception e) { + log.error(e.getMessage(), e); } } } - return new PageImpl(results, pageable, results.size()); + return utils.getPage(results, pageable); } @Override From e4d61191e61ddf7f994c6fea4c4b699ead688488 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Thu, 18 Jun 2020 23:52:49 +0200 Subject: [PATCH 23/24] Disable test failing due to bug in the Mock library --- .../ContentLanguageHeaderResponseFilter.java | 23 +++++++++---------- .../dspace/app/rest/LanguageSupportIT.java | 5 +++- 2 files changed, 15 insertions(+), 13 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/filter/ContentLanguageHeaderResponseFilter.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/filter/ContentLanguageHeaderResponseFilter.java index 6552ae4f7f..74ffd73ad4 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/filter/ContentLanguageHeaderResponseFilter.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/filter/ContentLanguageHeaderResponseFilter.java @@ -22,7 +22,9 @@ import org.springframework.stereotype.Component; /** * This filter assures that when the dspace instance supports multiple languages - * they are noted in the Content-Language Header of the response + * they are noted in the Content-Language Header of the response. Where + * appropriate the single endpoint can set the Content-Language header directly + * to note that the response is specific for a language * * @author Mykhaylo Boychuk (at 4science.it) */ @@ -36,20 +38,17 @@ public class ContentLanguageHeaderResponseFilter implements Filter { @Override public void doFilter(ServletRequest request, ServletResponse response, FilterChain chain) throws IOException, ServletException { - chain.doFilter(request, response); - HttpServletResponse httpServletResponse = (HttpServletResponse) response; - if (!httpServletResponse.containsHeader("Content-Language")) { - Locale[] locales = I18nUtil.getSupportedLocales(); - StringBuilder locsStr = new StringBuilder(); - for (Locale locale : locales) { - if (locsStr.length() > 0) { - locsStr.append(","); - } - locsStr.append(locale.getLanguage()); + Locale[] locales = I18nUtil.getSupportedLocales(); + StringBuilder locsStr = new StringBuilder(); + for (Locale locale : locales) { + if (locsStr.length() > 0) { + locsStr.append(","); } - httpServletResponse.setHeader("Content-Language", locsStr.toString()); + locsStr.append(locale.getLanguage()); } + httpServletResponse.setHeader("Content-Language", locsStr.toString()); + chain.doFilter(request, response); } @Override diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/LanguageSupportIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/LanguageSupportIT.java index 2dc06eca8b..cc8af92e2b 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/LanguageSupportIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/LanguageSupportIT.java @@ -18,6 +18,7 @@ import org.dspace.content.authority.ChoiceAuthorityServiceImpl; import org.dspace.core.LegacyPluginServiceImpl; import org.dspace.eperson.EPerson; import org.dspace.services.ConfigurationService; +import org.junit.Ignore; import org.junit.Test; import org.springframework.beans.factory.annotation.Autowired; @@ -42,6 +43,8 @@ public class LanguageSupportIT extends AbstractControllerIntegrationTest { } @Test + @Ignore("This test fails due to a bug in the MockHttpResponseServlet," + + " see https://github.com/spring-projects/spring-framework/issues/25281") public void checkEnabledMultipleLanguageSupportTest() throws Exception { context.turnOffAuthorisationSystem(); String[] supportedLanguage = {"uk","it"}; @@ -75,7 +78,7 @@ public class LanguageSupportIT extends AbstractControllerIntegrationTest { .andExpect(header().stringValues("Content-Language","uk, it")); getClient(tokenEPersonFR).perform(get("/api").locale(it)) - .andExpect(header().stringValues("Content-Language","en")); + .andExpect(header().stringValues("Content-Language","uk, it")); configurationService.setProperty("webui.supported.locales",null); legacyPluginService.clearNamedPluginClasses(); From c23a5b64de597dc78212522b775289e530470ea2 Mon Sep 17 00:00:00 2001 From: Andrea Bollini Date: Fri, 19 Jun 2020 09:35:40 +0200 Subject: [PATCH 24/24] Improve error handling and test env clenaup --- .../SubmissionUploadRestRepository.java | 35 +++++++++++++++---- .../app/rest/SubmissionFormsControllerIT.java | 11 +++--- 2 files changed, 35 insertions(+), 11 deletions(-) diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java index a359a7ec4f..25ac640d49 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/SubmissionUploadRestRepository.java @@ -7,6 +7,8 @@ */ package org.dspace.app.rest.repository; +import java.sql.SQLException; +import java.text.ParseException; import java.util.ArrayList; import java.util.Collection; import java.util.List; @@ -89,20 +91,31 @@ public class SubmissionUploadRestRepository extends DSpaceRestRepository