diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/AuthenticationRestController.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/AuthenticationRestController.java index d0c8001433..281494c081 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/AuthenticationRestController.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/AuthenticationRestController.java @@ -117,7 +117,6 @@ public class AuthenticationRestController implements InitializingBean { context = ContextUtil.obtainContext(request); - if (context == null || context.getCurrentUser() == null) { // Note that the actual HTTP status in this case is set by // org.dspace.app.rest.security.StatelessLoginFilter.unsuccessfulAuthentication() @@ -128,4 +127,5 @@ public class AuthenticationRestController implements InitializingBean { return ResponseEntity.ok().build(); } } + } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java index b8b9585804..986aadb1b1 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/DSpaceRestRepository.java @@ -332,9 +332,11 @@ public abstract class DSpaceRestRepository { @@ -33,7 +35,6 @@ public class EPersonPatch extends DSpaceObjectPatch { * @throws PatchBadRequestException */ protected EPersonRest replace(EPersonRest eperson, Operation operation) { - ResourcePatchOperation patchOperation = patchFactory.getReplaceOperationForPath(operation.getPath()); diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/ItemPatch.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/ItemPatch.java index b34a62d1ea..cc8a331f62 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/ItemPatch.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/ItemPatch.java @@ -18,6 +18,8 @@ import org.springframework.stereotype.Component; /** * Provides PATCH operations for item updates. + * + * @author Michael Spalti */ @Component public class ItemPatch extends DSpaceObjectPatch { @@ -26,7 +28,7 @@ public class ItemPatch extends DSpaceObjectPatch { ItemOperationFactory patchFactory; /** - * Peforms the replace operation. + * Performs the replace operation. * @param item the rest representation of the item * @param operation the replace operation * @throws UnprocessableEntityException diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/EPersonOperationFactory.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/EPersonOperationFactory.java index 27cee32ee4..c979a4f953 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/EPersonOperationFactory.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/EPersonOperationFactory.java @@ -10,6 +10,7 @@ package org.dspace.app.rest.repository.patch.factories; import org.dspace.app.rest.exception.PatchBadRequestException; import org.dspace.app.rest.model.EPersonRest; import org.dspace.app.rest.repository.patch.factories.impl.EPersonCertificateReplaceOperation; +import org.dspace.app.rest.repository.patch.factories.impl.EPersonEmailReplaceOperation; import org.dspace.app.rest.repository.patch.factories.impl.EPersonLoginReplaceOperation; import org.dspace.app.rest.repository.patch.factories.impl.EPersonNetidReplaceOperation; import org.dspace.app.rest.repository.patch.factories.impl.EPersonPasswordReplaceOperation; @@ -35,13 +36,16 @@ public class EPersonOperationFactory { EPersonCertificateReplaceOperation certificateReplaceOperation; @Autowired - EPersonNetidReplaceOperation netidReplaceOperation; + EPersonNetidReplaceOperation netIdReplaceOperation; - private static final String OPERATION_PASSWORD_CHANGE = "/password"; - private static final String OPERATION_CAN_LOGIN = "/canLogin"; - private static final String OPERATION_REQUIRE_CERTIFICATE = "/certificate"; - private static final String OPERATION_SET_NETID = "/netid"; + @Autowired + EPersonEmailReplaceOperation emailReplaceOperation; + public static final String OPERATION_PASSWORD_CHANGE = "/password"; + public static final String OPERATION_CAN_LOGIN = "/canLogin"; + public static final String OPERATION_REQUIRE_CERTIFICATE = "/certificate"; + public static final String OPERATION_SET_NETID = "/netid"; + public static final String OPERATION_SET_EMAIL = "/email"; /** * Returns the patch instance for the replace operation (based on the operation path). * @@ -59,7 +63,9 @@ public class EPersonOperationFactory { case OPERATION_REQUIRE_CERTIFICATE: return certificateReplaceOperation; case OPERATION_SET_NETID: - return netidReplaceOperation; + return netIdReplaceOperation; + case OPERATION_SET_EMAIL: + return emailReplaceOperation; default: throw new PatchBadRequestException("Missing patch operation for: " + path); } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonCertificateReplaceOperation.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonCertificateReplaceOperation.java index 8f75a1fd8c..1ab27d57cc 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonCertificateReplaceOperation.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonCertificateReplaceOperation.java @@ -37,7 +37,7 @@ public class EPersonCertificateReplaceOperation extends ReplacePatchOperation + * curl -X PATCH http://${dspace.url}/api/epersons/eperson/<:id-eperson> -H " + * Content-Type: application/json" -d '[{ "op": "replace", "path": " + * /email", "value": "new@email"]' + * + * + * @author Michael Spalti + */ +@Component +public class EPersonEmailReplaceOperation extends ReplacePatchOperation + implements ResourcePatchOperation { + @Override + EPersonRest replace(EPersonRest eperson, Operation operation) { + + eperson.setEmail((String) operation.getValue()); + return eperson; + } + + @Override + void checkModelForExistingValue(EPersonRest resource, Operation operation) { + if (resource.getEmail() == null) { + throw new PatchBadRequestException("Attempting to replace a non-existent value."); + } + } + + @Override + protected Class getArrayClassForEvaluation() { + + return String[].class; + } + + @Override + protected Class getClassForEvaluation() { + + return String.class; + } +} diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonLoginReplaceOperation.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonLoginReplaceOperation.java index d8e5e90c02..533e2804a4 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonLoginReplaceOperation.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/EPersonLoginReplaceOperation.java @@ -36,7 +36,7 @@ public class EPersonLoginReplaceOperation extends ReplacePatchOperation getArrayClassForEvaluation() { + return String[].class; } @Override protected Class getClassForEvaluation() { + return String.class; } } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ItemDiscoverableReplaceOperation.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ItemDiscoverableReplaceOperation.java index fca3f31b36..b8e1a63faa 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ItemDiscoverableReplaceOperation.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ItemDiscoverableReplaceOperation.java @@ -40,7 +40,7 @@ public class ItemDiscoverableReplaceOperation extends ReplacePatchOperation if (value instanceof String) { bool = BooleanUtils.toBooleanObject((String) value); if (bool == null) { - // make sure the string was converted to boolean. throw new PatchBadRequestException("Boolean value not provided."); } } else { @@ -67,35 +62,6 @@ public abstract class PatchOperation return bool; } - // This is duplicated code (see org.dspace.app.rest.submit.factory.impl.PatchOperation) - // If it stays here, it should be DRY. Current patch resource patch operations do not - // use these methods since operation values are either strings or booleans. - // These methods handle JsonValueEvaluator instances for json objects and arrays, - // as returned by the JsonPatchConverter. A complete implementation of the PatchOperation - // class will need these methods. - public List evaluateArrayObject(LateObjectEvaluator value) { - List results = new ArrayList(); - T[] list = null; - if (value != null) { - LateObjectEvaluator object = (LateObjectEvaluator) value; - list = (T[]) object.evaluate(getArrayClassForEvaluation()); - } - - for (T t : list) { - results.add(t); - } - return results; - } - - public T evaluateSingleObject(LateObjectEvaluator value) { - T single = null; - if (value != null) { - LateObjectEvaluator object = (LateObjectEvaluator) value; - single = (T) object.evaluate(getClassForEvaluation()); - } - return single; - } - /** * This method should return the typed array to be used in the * LateObjectEvaluator evaluation of json arrays. diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ReplacePatchOperation.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ReplacePatchOperation.java index 64a1615437..f7c441918b 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ReplacePatchOperation.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/repository/patch/factories/impl/ReplacePatchOperation.java @@ -25,6 +25,7 @@ public abstract class ReplacePatchOperation * Before performing the replace operation this method checks * for a non-null operation value and a non-null value on the rest model * (replace operations should only be applied to an existing value). + * * @param resource the rest model. * @param operation the replace patch operation. * @return the updated rest model. @@ -35,7 +36,7 @@ public abstract class ReplacePatchOperation public R perform(R resource, Operation operation) { checkOperationValue(operation.getValue()); - checkModelForExistingValue(resource); + checkModelForExistingValue(resource, operation); return replace(resource, operation); } @@ -43,7 +44,7 @@ public abstract class ReplacePatchOperation /** * Executes the replace patch operation. * - * @param resource the rest model. + * @param resource the rest model. * @param operation the replace patch operation. * @return the updated rest model. * @throws PatchBadRequestException @@ -56,9 +57,11 @@ public abstract class ReplacePatchOperation * Null values may exist in the RestModel for certain fields * (usually non-boolean). This method should be implemented * to assure that the replace operation acts only on an existing value. + * * @param resource the rest model. * @throws PatchBadRequestException */ - abstract void checkModelForExistingValue(R resource); + abstract void checkModelForExistingValue(R resource, Operation operation); + } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AdminRestPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AdminRestPermissionEvaluatorPlugin.java index d51cbb80b4..dd04fa95ee 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AdminRestPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AdminRestPermissionEvaluatorPlugin.java @@ -43,8 +43,8 @@ public class AdminRestPermissionEvaluatorPlugin extends RestObjectPermissionEval private EPersonService ePersonService; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, - Object permission) { + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, + DSpaceRestPermission permission) { //We do not check the "permission" object here because administrators are allowed to do everything diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AuthorizeServicePermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AuthorizeServicePermissionEvaluatorPlugin.java index e915f74790..c5e9541976 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AuthorizeServicePermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/AuthorizeServicePermissionEvaluatorPlugin.java @@ -52,8 +52,8 @@ public class AuthorizeServicePermissionEvaluatorPlugin extends RestObjectPermiss private ContentServiceFactory contentServiceFactory; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, - Object permission) { + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, + DSpaceRestPermission permission) { DSpaceRestPermission restPermission = DSpaceRestPermission.convert(permission); if (restPermission == null) { @@ -94,4 +94,5 @@ public class AuthorizeServicePermissionEvaluatorPlugin extends RestObjectPermiss return false; } + } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/ClaimedTaskRestPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/ClaimedTaskRestPermissionEvaluatorPlugin.java index 6ec4b3ce97..8304277dff 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/ClaimedTaskRestPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/ClaimedTaskRestPermissionEvaluatorPlugin.java @@ -46,8 +46,8 @@ public class ClaimedTaskRestPermissionEvaluatorPlugin extends RestObjectPermissi private EPersonService ePersonService; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, - String targetType, Object permission) { + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, + String targetType, DSpaceRestPermission permission) { if (Constants.getTypeID(targetType) != Constants.CLAIMEDTASK) { return false; @@ -77,4 +77,5 @@ public class ClaimedTaskRestPermissionEvaluatorPlugin extends RestObjectPermissi } return false; } + } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java index a8c6564243..551dd545f2 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java @@ -9,9 +9,14 @@ package org.dspace.app.rest.security; import java.io.Serializable; import java.sql.SQLException; +import java.util.List; import java.util.UUID; +import org.dspace.app.rest.model.patch.Operation; +import org.dspace.app.rest.model.patch.Patch; +import org.dspace.app.rest.repository.patch.factories.EPersonOperationFactory; import org.dspace.app.rest.utils.ContextUtil; +import org.dspace.authorize.service.AuthorizeService; import org.dspace.core.Constants; import org.dspace.core.Context; import org.dspace.eperson.EPerson; @@ -33,6 +38,9 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv private static final Logger log = LoggerFactory.getLogger(EPersonRestPermissionEvaluatorPlugin.class); + @Autowired + AuthorizeService authorizeService; + @Autowired private RequestService requestService; @@ -40,9 +48,9 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv private EPersonService ePersonService; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, - String targetType, Object permission) { - //For now this plugin only evaluates READ access + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, + String targetType, DSpaceRestPermission permission) { + DSpaceRestPermission restPermission = DSpaceRestPermission.convert(permission); if (!DSpaceRestPermission.READ.equals(restPermission) && !DSpaceRestPermission.WRITE.equals(restPermission) @@ -55,11 +63,18 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv Request request = requestService.getCurrentRequest(); Context context = ContextUtil.obtainContext(request.getServletRequest()); + EPerson ePerson = null; + try { ePerson = ePersonService.findByEmail(context, (String) authentication.getPrincipal()); UUID dsoId = UUID.fromString(targetId.toString()); + // anonymous user + if (ePerson == null) { + return false; + } + if (dsoId.equals(ePerson.getID())) { return true; } @@ -70,4 +85,32 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv return false; } + + @Override + public boolean hasPatchPermission(Authentication authentication, Serializable targetId, String targetType, + Patch patch) { + + /** + * First verify that the user has write permission on the eperson. + */ + if (!hasPermission(authentication, targetId, targetType, "WRITE")) { + return false; + } + + List operations = patch.getOperations(); + + /** + * The entire Patch request should be denied if it contains operations that are + * restricted to Dspace administrators. The authenticated user is currently allowed to + * update their own password. + */ + for (Operation op: operations) { + if (!op.getPath().contentEquals(EPersonOperationFactory.OPERATION_PASSWORD_CHANGE)) { + return false; + } + } + + return true; + } + } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java index 800c9110ee..91cc302aa1 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/GroupRestPermissionEvaluatorPlugin.java @@ -45,8 +45,8 @@ public class GroupRestPermissionEvaluatorPlugin extends RestObjectPermissionEval private EPersonService ePersonService; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, - String targetType, Object permission) { + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, + String targetType, DSpaceRestPermission permission) { //This plugin only evaluates READ access DSpaceRestPermission restPermission = DSpaceRestPermission.convert(permission); diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/PoolTaskRestPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/PoolTaskRestPermissionEvaluatorPlugin.java index 00bec2ac87..34b1bbf2cb 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/PoolTaskRestPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/PoolTaskRestPermissionEvaluatorPlugin.java @@ -48,8 +48,8 @@ public class PoolTaskRestPermissionEvaluatorPlugin extends RestObjectPermissionE private EPersonService ePersonService; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, - String targetType, Object permission) { + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, + String targetType, DSpaceRestPermission permission) { if (Constants.getTypeID(targetType) != Constants.POOLTASK) { return false; diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/RestObjectPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/RestObjectPermissionEvaluatorPlugin.java index 2f467a8fd5..6ee17f8420 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/RestObjectPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/RestObjectPermissionEvaluatorPlugin.java @@ -7,7 +7,10 @@ */ package org.dspace.app.rest.security; +import java.io.Serializable; + import org.dspace.app.rest.model.BaseObjectRest; +import org.dspace.app.rest.model.patch.Patch; import org.springframework.security.core.Authentication; /** @@ -27,11 +30,50 @@ public abstract class RestObjectPermissionEvaluatorPlugin implements RestPermis * expression system. This corresponds to the DSpace action. Not null. * @return true if the permission is granted by one of the plugins, false otherwise */ + @Override public boolean hasPermission(Authentication authentication, Object targetDomainObject, Object permission) { BaseObjectRest restObject = (BaseObjectRest) targetDomainObject; return hasPermission(authentication, restObject.getId(), restObject.getType(), permission); } + @Override + public boolean hasPermission(Authentication authentication, Serializable targetId, String targetType, + Object permission) { + + if (permission instanceof Patch) { + return hasPatchPermission(authentication, targetId, targetType, (Patch) permission); + } else { + DSpaceRestPermission restPermission = DSpaceRestPermission.convert(permission); + return hasDSpacePermission(authentication, targetId, targetType, restPermission); + } + } + + /** + * Checks permissions for {@link Patch } requests. Override the default implementation in + * plugins that require this capability. + * @param authentication Authentication object providing user details of the authenticated user + * @param targetId Unique identifier of the target object the user wants to view or manipulate + * @param targetType Type of the target object the users wants to view or manipulate + * @param patch The {@link Patch } instance + * @return true if the user is allowed to perform the action described by the permission. False otherwise + */ + public boolean hasPatchPermission(Authentication authentication, Serializable targetId, String targetType, + Patch patch) { + + return hasPermission(authentication, targetId, targetType, "WRITE"); + } + + /** + * Plugins must implement this method to receive {@link RestPermissionEvaluatorPlugin} hasPermission + * requests. + * @param authentication Authentication object providing user details of the authenticated user + * @param targetId Unique identifier of the target object the user wants to view or manipulate + * @param targetType Type of the target object the users wants to view or manipulate + * @param restPermission Permission object that describes the action the user wants to perform on the target object + * @return true if the user is allowed to perform the action described by the permission. False otherwise. + */ + public abstract boolean hasDSpacePermission(Authentication authentication, Serializable targetId, String targetType, + DSpaceRestPermission restPermission); } diff --git a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java index 3673a4cada..54040fb5f0 100644 --- a/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java +++ b/dspace-spring-rest/src/main/java/org/dspace/app/rest/security/WorkflowRestPermissionEvaluatorPlugin.java @@ -56,8 +56,8 @@ public class WorkflowRestPermissionEvaluatorPlugin extends RestObjectPermissionE private EPersonService ePersonService; @Override - public boolean hasPermission(Authentication authentication, Serializable targetId, - String targetType, Object permission) { + public boolean hasDSpacePermission(Authentication authentication, Serializable targetId, + String targetType, DSpaceRestPermission permission) { //This plugin currently only evaluates READ access DSpaceRestPermission restPermission = DSpaceRestPermission.convert(permission); diff --git a/dspace-spring-rest/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java b/dspace-spring-rest/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java index 65377677ce..6cbd0168fe 100644 --- a/dspace-spring-rest/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java +++ b/dspace-spring-rest/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java @@ -48,6 +48,7 @@ import org.junit.Test; public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { + @Test public void createTest() throws Exception { context.turnOffAuthorisationSystem(); @@ -555,7 +556,6 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { ReplaceOperation replaceOperation = new ReplaceOperation("/netid", "newNetId"); ops.add(replaceOperation); String patchBody = getPatchContent(ops); - // should be unauthorized. getClient().perform(patch("/api/eperson/epersons/" + ePerson.getID()) .content(patchBody) @@ -879,7 +879,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { EPerson ePerson = EPersonBuilder.createEPerson(context) .withNameInMetadata("John", "Doe") .withEmail("Johndoe@fake-email.com") - .withPassword("7Testpass") + .withPassword(password) .build(); String newPassword = "newpassword"; @@ -904,6 +904,134 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { } + @Test + public void patchPasswordIsForbidden() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson1 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@fake-email.com") + .withPassword(password) + .build(); + + EPerson ePerson2 = EPersonBuilder.createEPerson(context) + .withNameInMetadata("Jane", "Doe") + .withEmail("Janedoe@fake-email.com") + .withPassword(password) + .build(); + + String newPassword = "newpassword"; + + List ops = new ArrayList(); + ReplaceOperation replaceOperation = new ReplaceOperation("/password", newPassword); + ops.add(replaceOperation); + String patchBody = getPatchContent(ops); + + // eperson one + String token = getAuthToken(ePerson1.getEmail(), password); + + // should not be able to update the password of eperson two + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson2.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + + // login with old password + token = getAuthToken(ePerson2.getEmail(), password); + getClient(token).perform(get("/api/")) + .andExpect(status().isOk()); + + } + @Test + public void patchPasswordForNonAdminUser() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@fake-email.com") + .withPassword(password) + .build(); + + String newPassword = "newpassword"; + + context.restoreAuthSystemState(); + + List ops = new ArrayList(); + ReplaceOperation replaceOperation = new ReplaceOperation("/password", newPassword); + ops.add(replaceOperation); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(ePerson.getEmail(), password); + + // updates password + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isOk()); + + // login with new password + token = getAuthToken(ePerson.getEmail(), newPassword); + getClient(token).perform(get("/api/")) + .andExpect(status().isOk()); + + } + + @Test + public void patchCanLoginNonAdminUser() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("CanLogin@fake-email.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + ReplaceOperation replaceOperation = new ReplaceOperation("/canLogin", true); + List ops = new ArrayList(); + ops.add(replaceOperation); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(ePerson.getEmail(), password); + + // should return + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + + } + + @Test + public void patchCertificateNonAdminUser() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("CanLogin@fake-email.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + ReplaceOperation replaceOperation = new ReplaceOperation("/certificate", true); + List ops = new ArrayList(); + ops.add(replaceOperation); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(ePerson.getEmail(), password); + + // should return + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + + } + @Test public void patchPasswordMissingValue() throws Exception { @@ -949,6 +1077,100 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { } + @Test + public void patchEmail() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@fake-email.com") + .withPassword(password) + .build(); + + String newEmail = "janedoe@real-email.com"; + + List ops = new ArrayList(); + ReplaceOperation replaceOperation = new ReplaceOperation("/email", newEmail); + ops.add(replaceOperation); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(admin.getEmail(), password); + + // updates email + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(jsonPath("$.email", Matchers.is(newEmail))) + .andExpect(status().isOk()); + + // login with new email address + token = getAuthToken(newEmail, password); + getClient(token).perform(get("/api/")) + .andExpect(status().isOk()); + + } + + @Test + public void patchEmailNonAdminUser() throws Exception { + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@fake-email.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + String newEmail = "new@email.com"; + + ReplaceOperation replaceOperation = new ReplaceOperation("/email", newEmail); + List ops = new ArrayList(); + ops.add(replaceOperation); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(ePerson.getEmail(), password); + + // returns 403 + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + + } + + @Test + public void patchEmailMissingValue() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@fake-email.com") + .withPassword(password) + .build(); + + String token = getAuthToken(admin.getEmail(), password); + + List ops = new ArrayList(); + ReplaceOperation replaceOperation2 = new ReplaceOperation("/email", null); + ops.add(replaceOperation2); + String patchBody = getPatchContent(ops); + + // should return bad request + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isBadRequest()); + + // login with original password + token = getAuthToken(ePerson.getEmail(), password); + getClient(token).perform(get("/api/")) + .andExpect(status().isOk()); + + } + @Test public void patchMultipleOperationsWithSuccess() throws Exception { diff --git a/dspace-spring-rest/src/test/java/org/dspace/app/rest/builder/EPersonBuilder.java b/dspace-spring-rest/src/test/java/org/dspace/app/rest/builder/EPersonBuilder.java index 4f4f740e21..0c058f8610 100644 --- a/dspace-spring-rest/src/test/java/org/dspace/app/rest/builder/EPersonBuilder.java +++ b/dspace-spring-rest/src/test/java/org/dspace/app/rest/builder/EPersonBuilder.java @@ -73,6 +73,24 @@ public class EPersonBuilder extends AbstractDSpaceObjectBuilder { return this; } + public EPersonBuilder withLanguage(String lang) throws SQLException { + ePerson.setLanguage(context, lang); + return this; + } + + public EPersonBuilder withPhone(String phone) throws SQLException { + ePersonService.setMetadataSingleValue( + context, + ePerson, + "eperson", + "phone", + null, + null, + phone + ); + return this; + } + public EPersonBuilder withGroupMembership(Group group) { groupService.addMember(context, group, ePerson); return this; diff --git a/dspace-spring-rest/src/test/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPluginTest.java b/dspace-spring-rest/src/test/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPluginTest.java new file mode 100644 index 0000000000..21fcd60b38 --- /dev/null +++ b/dspace-spring-rest/src/test/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPluginTest.java @@ -0,0 +1,71 @@ +/** + * 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 static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertTrue; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.spy; +import static org.mockito.Mockito.when; + +import java.util.ArrayList; +import java.util.List; + +import org.dspace.app.rest.model.patch.Operation; +import org.dspace.app.rest.model.patch.Patch; +import org.dspace.app.rest.model.patch.ReplaceOperation; +import org.junit.Before; +import org.junit.Test; +import org.springframework.security.core.Authentication; + +/** + * This class verifies that {@link EPersonRestPermissionEvaluatorPlugin} properly + * evaluates Patch requests. + */ +public class EPersonRestPermissionEvaluatorPluginTest { + + private EPersonRestPermissionEvaluatorPlugin ePersonRestPermissionEvaluatorPlugin; + + private Authentication authentication; + + @Before + public void setUp() throws Exception { + ePersonRestPermissionEvaluatorPlugin = spy(EPersonRestPermissionEvaluatorPlugin.class); + authentication = mock(Authentication.class); + DSpaceRestPermission restPermission = DSpaceRestPermission.convert("WRITE"); + when(ePersonRestPermissionEvaluatorPlugin + .hasDSpacePermission(authentication, null, null, restPermission)).thenReturn(true); + } + + @Test + public void testHasPatchPermissionAuthFails() throws Exception { + + List ops = new ArrayList(); + ReplaceOperation passwordOperation = new ReplaceOperation("/password", "testpass"); + ops.add(passwordOperation); + ReplaceOperation canLoginOperation = new ReplaceOperation("/canLogin", false); + ops.add(canLoginOperation); + Patch patch = new Patch(ops); + assertFalse(ePersonRestPermissionEvaluatorPlugin + .hasPatchPermission(authentication, null, null, patch)); + + } + + @Test + public void testHasPatchPermissionAuthOk() throws Exception { + + List ops = new ArrayList(); + ReplaceOperation passwordOperation = new ReplaceOperation("/password", "testpass"); + ops.add(passwordOperation); + Patch patch = new Patch(ops); + assertTrue(ePersonRestPermissionEvaluatorPlugin + .hasPatchPermission(authentication, null, null, patch)); + + } + +}