mirror of
https://github.com/DSpace/DSpace.git
synced 2025-10-07 01:54:22 +00:00
[DS-4062] Added support for eperson updates by the currently logged in user.
Removed misplaced factory call. Removed misplaced factory call. Added exception and updated signature for the eperson repository patch method. Removed patch endpoint from the authentication controller and modifed eperson repository to use Spring PreAuthorize annotation and the eperson permission evaluator plugin. [DS-4062] Removed unused autowired bean from DSpaceRestRepository. No longer needed after the previous refactor to remove endpoint. [DS-4062] Removed unused import. [DS-4062] The EPersonRestRepository updated to hasPermission annontation only. Also limiting (experimentally) some eperson patch operations to administrators. Added adminstartor restriction for netid patch operation. Removed support for patching eperson profile metadata. Removed support for patching eperson profile metadata. Added eperson email patch operation. Updated permission plugins to support patch requests per suggestion by @tomdesair. Updated eperson authentication plugin and added unit test. Added the new PreAuthorize annotations for patch. Added the missing header reported by checkstyle. Changed order of static imports in unit test to pass checkstyle. Added integration test to verify that a non-admin user cannot update another eperson password.
This commit is contained in:
@@ -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();
|
||||
}
|
||||
}
|
||||
|
||||
}
|
||||
|
@@ -332,9 +332,11 @@ public abstract class DSpaceRestRepository<T extends RestAddressableModel, ID ex
|
||||
public T patch(HttpServletRequest request, String apiCategory, String model, ID id, Patch patch)
|
||||
throws HttpRequestMethodNotSupportedException, UnprocessableEntityException, PatchBadRequestException {
|
||||
Context context = obtainContext();
|
||||
|
||||
try {
|
||||
thisRepository.patch(context, request, apiCategory, model, id, patch);
|
||||
context.commit();
|
||||
|
||||
} catch (AuthorizeException ae) {
|
||||
throw new RESTAuthorizationException(ae);
|
||||
} catch (SQLException | DCInputsReaderException e) {
|
||||
|
@@ -186,7 +186,7 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
|
||||
}
|
||||
|
||||
@Override
|
||||
@PreAuthorize("hasAuthority('ADMIN')")
|
||||
@PreAuthorize("hasPermission(#uuid, 'EPERSON', #patch)")
|
||||
protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, UUID uuid,
|
||||
Patch patch) throws AuthorizeException, SQLException {
|
||||
patchDSpaceObject(apiCategory, model, uuid, patch);
|
||||
@@ -207,6 +207,9 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
|
||||
if (ePersonRest.isCanLogIn() != ePerson.canLogIn()) {
|
||||
ePerson.setCanLogIn(ePersonRest.isCanLogIn());
|
||||
}
|
||||
if (!Objects.equals(ePersonRest.getEmail(), ePerson.getEmail())) {
|
||||
ePerson.setEmail(ePersonRest.getEmail());
|
||||
}
|
||||
if (!Objects.equals(ePersonRest.getNetid(), ePerson.getNetid())) {
|
||||
ePerson.setNetid(ePersonRest.getNetid());
|
||||
}
|
||||
|
@@ -121,7 +121,7 @@ public class ItemRestRepository extends DSpaceObjectRestRepository<Item, ItemRes
|
||||
}
|
||||
|
||||
@Override
|
||||
@PreAuthorize("hasPermission(#id, 'ITEM', 'WRITE')")
|
||||
@PreAuthorize("hasPermission(#id, 'ITEM', #patch)")
|
||||
protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, UUID id,
|
||||
Patch patch) throws AuthorizeException, SQLException {
|
||||
patchDSpaceObject(apiCategory, model, id, patch);
|
||||
|
@@ -18,6 +18,8 @@ import org.springframework.stereotype.Component;
|
||||
|
||||
/**
|
||||
* Provides patch operations for eperson updates.
|
||||
*
|
||||
* @author Michael Spalti
|
||||
*/
|
||||
@Component
|
||||
public class EPersonPatch extends DSpaceObjectPatch<EPersonRest> {
|
||||
@@ -33,7 +35,6 @@ public class EPersonPatch extends DSpaceObjectPatch<EPersonRest> {
|
||||
* @throws PatchBadRequestException
|
||||
*/
|
||||
protected EPersonRest replace(EPersonRest eperson, Operation operation) {
|
||||
|
||||
ResourcePatchOperation<EPersonRest> patchOperation =
|
||||
patchFactory.getReplaceOperationForPath(operation.getPath());
|
||||
|
||||
|
@@ -18,6 +18,8 @@ import org.springframework.stereotype.Component;
|
||||
|
||||
/**
|
||||
* Provides PATCH operations for item updates.
|
||||
*
|
||||
* @author Michael Spalti
|
||||
*/
|
||||
@Component
|
||||
public class ItemPatch extends DSpaceObjectPatch<ItemRest> {
|
||||
@@ -26,7 +28,7 @@ public class ItemPatch extends DSpaceObjectPatch<ItemRest> {
|
||||
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
|
||||
|
@@ -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);
|
||||
}
|
||||
|
@@ -37,7 +37,7 @@ public class EPersonCertificateReplaceOperation extends ReplacePatchOperation<EP
|
||||
}
|
||||
|
||||
@Override
|
||||
void checkModelForExistingValue(EPersonRest resource) {
|
||||
void checkModelForExistingValue(EPersonRest resource, Operation operation) {
|
||||
// TODO: many (all?) boolean values on the rest model should never be null.
|
||||
// So perhaps the error to throw in this case is different...IllegalStateException?
|
||||
// Or perhaps do nothing (no check is required).
|
||||
|
@@ -0,0 +1,54 @@
|
||||
/**
|
||||
* 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.patch.factories.impl;
|
||||
|
||||
import org.dspace.app.rest.exception.PatchBadRequestException;
|
||||
import org.dspace.app.rest.model.EPersonRest;
|
||||
import org.dspace.app.rest.model.patch.Operation;
|
||||
import org.springframework.stereotype.Component;
|
||||
|
||||
/**
|
||||
* Implementation for EPerson password patches.
|
||||
*
|
||||
* Example: <code>
|
||||
* curl -X PATCH http://${dspace.url}/api/epersons/eperson/<:id-eperson> -H "
|
||||
* Content-Type: application/json" -d '[{ "op": "replace", "path": "
|
||||
* /email", "value": "new@email"]'
|
||||
* </code>
|
||||
*
|
||||
* @author Michael Spalti
|
||||
*/
|
||||
@Component
|
||||
public class EPersonEmailReplaceOperation extends ReplacePatchOperation<EPersonRest, String>
|
||||
implements ResourcePatchOperation<EPersonRest> {
|
||||
@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<String[]> getArrayClassForEvaluation() {
|
||||
|
||||
return String[].class;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Class<String> getClassForEvaluation() {
|
||||
|
||||
return String.class;
|
||||
}
|
||||
}
|
@@ -36,7 +36,7 @@ public class EPersonLoginReplaceOperation extends ReplacePatchOperation<EPersonR
|
||||
}
|
||||
|
||||
@Override
|
||||
void checkModelForExistingValue(EPersonRest resource) {
|
||||
void checkModelForExistingValue(EPersonRest resource, Operation operation) {
|
||||
if ((Object) resource.isCanLogIn() == null) {
|
||||
throw new PatchBadRequestException("Attempting to replace a non-existent value.");
|
||||
}
|
||||
|
@@ -36,7 +36,7 @@ public class EPersonNetidReplaceOperation extends ReplacePatchOperation<EPersonR
|
||||
|
||||
|
||||
@Override
|
||||
void checkModelForExistingValue(EPersonRest resource) {
|
||||
void checkModelForExistingValue(EPersonRest resource, Operation operation) {
|
||||
if (resource.getNetid() == null) {
|
||||
throw new PatchBadRequestException("Attempting to replace a non-existent value.");
|
||||
}
|
||||
|
@@ -33,7 +33,7 @@ public class EPersonPasswordReplaceOperation extends ReplacePatchOperation<EPers
|
||||
}
|
||||
|
||||
@Override
|
||||
void checkModelForExistingValue(EPersonRest resource) {
|
||||
void checkModelForExistingValue(EPersonRest resource, Operation operation) {
|
||||
/*
|
||||
* FIXME: the password field in eperson rest model is always null because
|
||||
* the value is not set in the rest converter.
|
||||
@@ -45,11 +45,13 @@ public class EPersonPasswordReplaceOperation extends ReplacePatchOperation<EPers
|
||||
|
||||
@Override
|
||||
protected Class<String[]> getArrayClassForEvaluation() {
|
||||
|
||||
return String[].class;
|
||||
}
|
||||
|
||||
@Override
|
||||
protected Class<String> getClassForEvaluation() {
|
||||
|
||||
return String.class;
|
||||
}
|
||||
}
|
||||
|
@@ -40,7 +40,7 @@ public class ItemDiscoverableReplaceOperation extends ReplacePatchOperation<Item
|
||||
}
|
||||
|
||||
@Override
|
||||
void checkModelForExistingValue(ItemRest resource) {
|
||||
void checkModelForExistingValue(ItemRest resource, Operation operation) {
|
||||
if ((Object) resource.getDiscoverable() == null) {
|
||||
throw new PatchBadRequestException("Attempting to replace a non-existent value.");
|
||||
}
|
||||
|
@@ -60,11 +60,10 @@ public class ItemWithdrawReplaceOperation extends ReplacePatchOperation<ItemRest
|
||||
return item;
|
||||
}
|
||||
|
||||
|
||||
}
|
||||
|
||||
@Override
|
||||
void checkModelForExistingValue(ItemRest resource) {
|
||||
void checkModelForExistingValue(ItemRest resource, Operation operation) {
|
||||
if ((Object) resource.getWithdrawn() == null) {
|
||||
throw new PatchBadRequestException("Attempting to replace a non-existent value.");
|
||||
}
|
||||
|
@@ -7,9 +7,6 @@
|
||||
*/
|
||||
package org.dspace.app.rest.repository.patch.factories.impl;
|
||||
|
||||
import java.util.ArrayList;
|
||||
import java.util.List;
|
||||
|
||||
import org.apache.commons.lang3.BooleanUtils;
|
||||
import org.dspace.app.rest.exception.PatchBadRequestException;
|
||||
import org.dspace.app.rest.model.RestModel;
|
||||
@@ -58,7 +55,6 @@ public abstract class PatchOperation<R extends RestModel, T>
|
||||
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 +63,6 @@ public abstract class PatchOperation<R extends RestModel, T>
|
||||
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<T> evaluateArrayObject(LateObjectEvaluator value) {
|
||||
List<T> results = new ArrayList<T>();
|
||||
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.
|
||||
|
@@ -25,6 +25,7 @@ public abstract class ReplacePatchOperation<R extends RestModel, T>
|
||||
* 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<R extends RestModel, T>
|
||||
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<R extends RestModel, T>
|
||||
/**
|
||||
* 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<R extends RestModel, T>
|
||||
* 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);
|
||||
|
||||
|
||||
}
|
||||
|
@@ -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
|
||||
|
||||
|
@@ -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;
|
||||
}
|
||||
|
||||
}
|
||||
|
@@ -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<Operation> 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;
|
||||
}
|
||||
|
||||
}
|
||||
|
@@ -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);
|
||||
|
@@ -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);
|
||||
|
||||
}
|
||||
|
@@ -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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
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 {
|
||||
|
||||
|
@@ -73,6 +73,24 @@ public class EPersonBuilder extends AbstractDSpaceObjectBuilder<EPerson> {
|
||||
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;
|
||||
|
@@ -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<Operation> ops = new ArrayList<Operation>();
|
||||
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<Operation> ops = new ArrayList<Operation>();
|
||||
ReplaceOperation passwordOperation = new ReplaceOperation("/password", "testpass");
|
||||
ops.add(passwordOperation);
|
||||
Patch patch = new Patch(ops);
|
||||
assertTrue(ePersonRestPermissionEvaluatorPlugin
|
||||
.hasPatchPermission(authentication, null, null, patch));
|
||||
|
||||
}
|
||||
|
||||
}
|
Reference in New Issue
Block a user