Merge pull request #2257 from mspalti/eperson_update

[DS-4062] Endpoint to allow logged in EPerson to change password or other profile information.
This commit is contained in:
Tim Donohue
2019-04-15 16:08:21 -05:00
committed by GitHub
27 changed files with 508 additions and 72 deletions

View File

@@ -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();
}
}
}

View File

@@ -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) {

View File

@@ -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());
}

View File

@@ -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);

View File

@@ -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());

View File

@@ -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

View File

@@ -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);
}

View File

@@ -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).

View File

@@ -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;
}
}

View File

@@ -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.");
}

View File

@@ -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.");
}

View File

@@ -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;
}
}

View File

@@ -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.");
}

View File

@@ -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.");
}

View File

@@ -7,13 +7,9 @@
*/
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;
import org.dspace.app.rest.model.patch.LateObjectEvaluator;
import org.dspace.app.rest.model.patch.Operation;
/**
@@ -58,7 +54,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 +62,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.

View File

@@ -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);
}
@@ -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);
}

View File

@@ -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

View File

@@ -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;
}
}

View File

@@ -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;
}
}

View File

@@ -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;
}
}

View File

@@ -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);

View File

@@ -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;

View File

@@ -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);
}

View File

@@ -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);

View File

@@ -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 {

View File

@@ -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;

View File

@@ -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));
}
}