diff --git a/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationMethod.java b/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationMethod.java index 25d31776cc..6567c5031a 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationMethod.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationMethod.java @@ -224,4 +224,11 @@ public interface AuthenticationMethod { * @return whether the authentication method is being used. */ public boolean isUsed(Context context, HttpServletRequest request); + + /** + * @param context The DSpace context + * @param challenge The current password of the user + * @return true if challenge matches with current password + */ + public boolean canChangePassword(Context context, String challenge); } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java b/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java index 1270c1cb2c..177f2730f3 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java @@ -207,4 +207,14 @@ public class AuthenticationServiceImpl implements AuthenticationService { return null; } + + @Override + public boolean canChangePassword(Context context, String challenge) { + for (AuthenticationMethod method : getAuthenticationMethodStack()) { + if (method.canChangePassword(context, challenge)) { + return true; + } + } + return false; + } } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/IPAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/IPAuthentication.java index 67405b5c1c..ccf96e18c4 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/IPAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/IPAuthentication.java @@ -278,4 +278,9 @@ public class IPAuthentication implements AuthenticationMethod { public boolean isUsed(final Context context, final HttpServletRequest request) { return false; } + + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java index 520a5f62a6..a2b21a5ceb 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java @@ -752,4 +752,9 @@ public class LDAPAuthentication } return false; } + + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthentication.java index edaa87dd13..6d3b6962dd 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthentication.java @@ -86,4 +86,9 @@ public class OidcAuthentication implements AuthenticationMethod { return false; } + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } + } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java index 41b40066b3..4a727a2ebf 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java @@ -294,4 +294,9 @@ public class OidcAuthenticationBean implements AuthenticationMethod { return false; } + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } + } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthentication.java index 3bd8f8bce8..9f1165b1ec 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthentication.java @@ -101,4 +101,9 @@ public class OrcidAuthentication implements AuthenticationMethod { return getOrcidAuthentication().isUsed(context, request); } + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } + } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java index 07f64f3e8e..e2455383b9 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java @@ -123,6 +123,11 @@ public class OrcidAuthenticationBean implements AuthenticationMethod { return request.getAttribute(ORCID_AUTH_ATTRIBUTE) != null; } + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } + @Override public boolean canSelfRegister(Context context, HttpServletRequest request, String username) throws SQLException { return canSelfRegister(); diff --git a/dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java index 50a685872a..e086a4aea8 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java @@ -22,6 +22,7 @@ import org.dspace.core.LogHelper; import org.dspace.eperson.EPerson; import org.dspace.eperson.Group; import org.dspace.eperson.factory.EPersonServiceFactory; +import org.dspace.eperson.service.EPersonService; import org.dspace.services.factory.DSpaceServicesFactory; /** @@ -53,6 +54,8 @@ public class PasswordAuthentication private static final String PASSWORD_AUTHENTICATED = "password.authenticated"; + private EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService(); + /** @@ -264,4 +267,13 @@ public class PasswordAuthentication } return false; } + + @Override + public boolean canChangePassword(Context context, String challenge) { + EPerson ePerson = context.getCurrentUser(); + if (context != null && ePerson != null) { + return ePersonService.checkPassword(context, context.getCurrentUser(), challenge); + } + return false; + } } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java index dba5de90f3..ce6a111bbe 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -1276,5 +1276,10 @@ public class ShibAuthentication implements AuthenticationMethod { } return false; } + + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java b/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java index 503d90d0ec..6c4fafa350 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java @@ -608,4 +608,9 @@ public class X509Authentication implements AuthenticationMethod { } return false; } + + @Override + public boolean canChangePassword(Context context, String challenge) { + return false; + } } diff --git a/dspace-api/src/main/java/org/dspace/authenticate/service/AuthenticationService.java b/dspace-api/src/main/java/org/dspace/authenticate/service/AuthenticationService.java index fba2f00323..5a9866f103 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/service/AuthenticationService.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/service/AuthenticationService.java @@ -177,4 +177,11 @@ public interface AuthenticationService { */ public String getAuthenticationMethod(Context context, HttpServletRequest request); + /** + * @param context The DSpace context + * @param challenge The current password of the user + * @return true if challenge matches with current password + */ + public boolean canChangePassword(Context context, String challenge); + } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java index 6ded477813..5d66a81bb2 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/DSpaceApiExceptionControllerAdvice.java @@ -190,6 +190,12 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH HttpStatus.BAD_REQUEST.value()); } + @ExceptionHandler({InvalidPasswordException.class}) + protected void handleInvalidPasswordException(HttpServletRequest request, HttpServletResponse response, + Exception ex) throws IOException { + sendErrorResponse(request, response, ex, "Current password is not valid", HttpServletResponse.SC_FORBIDDEN); + } + @Override protected ResponseEntity handleMissingServletRequestParameter(MissingServletRequestParameterException ex, HttpHeaders headers, HttpStatus status, diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/InvalidPasswordException.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/InvalidPasswordException.java new file mode 100644 index 0000000000..7fb634eeff --- /dev/null +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/InvalidPasswordException.java @@ -0,0 +1,24 @@ +/** + * 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.exception; + +/** + * This exception is thrown when the current password of user is invalid + * + * @author Mohamed Eskander (mohamed.eskander at 4science.it) + */ +public class InvalidPasswordException extends RuntimeException { + + public InvalidPasswordException(String message) { + super(message); + } + + public InvalidPasswordException(String message, Exception exception) { + super(message, exception); + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java index cfae12584f..a0aa1cca88 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/EPersonRestRepository.java @@ -24,6 +24,7 @@ import org.dspace.app.rest.SearchRestMethod; import org.dspace.app.rest.authorization.AuthorizationFeatureService; import org.dspace.app.rest.exception.DSpaceBadRequestException; import org.dspace.app.rest.exception.EPersonNameNotProvidedException; +import org.dspace.app.rest.exception.InvalidPasswordException; import org.dspace.app.rest.exception.RESTEmptyWorkflowGroupException; import org.dspace.app.rest.exception.UnprocessableEntityException; import org.dspace.app.rest.model.EPersonRest; @@ -32,6 +33,7 @@ import org.dspace.app.rest.model.MetadataValueRest; import org.dspace.app.rest.model.patch.Operation; import org.dspace.app.rest.model.patch.Patch; import org.dspace.app.util.AuthorizeUtil; +import org.dspace.authenticate.service.AuthenticationService; import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.service.AuthorizeService; import org.dspace.content.service.SiteService; @@ -82,6 +84,9 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository + * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H " + * Content-Type: application/json" -d '[{"op":"add","path":"/password","value":"newpassword"}, + * { "op": "add", "path": /challenge", "value": "oldPassword"]' + * + */ +@Component +public class EPersonChallengeAddOperation extends PatchOperation { + + /** + * Path in json body of patch that uses this operation + */ + public static final String OPERATION_PASSWORD_CHANGE = "/challenge"; + + @Override + public R perform(Context context, R object, Operation operation) { + return object; + } + + @Override + public boolean supports(Object objectToMatch, Operation operation) { + return (objectToMatch instanceof EPerson && operation.getOp().trim().equalsIgnoreCase(OPERATION_ADD) + && operation.getPath().trim().equalsIgnoreCase(OPERATION_PASSWORD_CHANGE)); + } +} diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java index 0e208a68a2..7f6ff43915 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/security/EPersonRestPermissionEvaluatorPlugin.java @@ -17,6 +17,7 @@ import org.apache.commons.lang3.StringUtils; import org.dspace.app.rest.model.patch.Operation; import org.dspace.app.rest.model.patch.Patch; import org.dspace.app.rest.repository.patch.operation.DSpaceObjectMetadataPatchUtils; +import org.dspace.app.rest.repository.patch.operation.EPersonChallengeAddOperation; import org.dspace.app.rest.repository.patch.operation.EPersonPasswordAddOperation; import org.dspace.app.rest.repository.patch.operation.PatchOperation; import org.dspace.app.rest.utils.ContextUtil; @@ -123,7 +124,8 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv */ for (Operation op: operations) { if (!(op.getPath().contentEquals(EPersonPasswordAddOperation.OPERATION_PASSWORD_CHANGE) - || (op.getPath().startsWith(DSpaceObjectMetadataPatchUtils.OPERATION_METADATA_PATH)))) { + || (op.getPath().startsWith(DSpaceObjectMetadataPatchUtils.OPERATION_METADATA_PATH) + || op.getPath().contentEquals(EPersonChallengeAddOperation.OPERATION_PASSWORD_CHANGE)))) { return false; } } diff --git a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java index e6774ebb39..0c7ba62952 100644 --- a/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java +++ b/dspace-server-webapp/src/test/java/org/dspace/app/rest/EPersonRestRepositoryIT.java @@ -1304,7 +1304,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { List ops = new ArrayList(); AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", password); ops.add(addOperation); + ops.add(addOperation2); String patchBody = getPatchContent(ops); String token = getAuthToken(admin.getEmail(), password); @@ -1401,7 +1403,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { List ops = new ArrayList(); AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", password); ops.add(addOperation); + ops.add(addOperation2); String patchBody = getPatchContent(ops); String token = getAuthToken(ePerson.getEmail(), password); @@ -1444,7 +1448,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { List ops = new ArrayList(); AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", password); ops.add(addOperation); + ops.add(addOperation2); String patchBody = getPatchContent(ops); String token = getAuthToken(admin.getEmail(), password); @@ -1536,7 +1542,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { List ops = new ArrayList<>(); AddOperation addOperation = new AddOperation("/password", null); + AddOperation addOperation2 = new AddOperation("/challenge", password); ops.add(addOperation); + ops.add(addOperation2); String patchBody = getPatchContent(ops); // adding null pw should return bad request @@ -1581,7 +1589,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { List ops = new ArrayList(); AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", password); ops.add(addOperation); + ops.add(addOperation2); String patchBody = getPatchContent(ops); // initialize password with add operation, not set during creation @@ -3129,4 +3139,142 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { } + @Test + public void patchChangePassword() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@example.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + String newPassword = "newpassword"; + + List ops = new ArrayList(); + AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", password); + ops.add(addOperation); + ops.add(addOperation2); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(admin.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/authn/status")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.okay", is(true))) + .andExpect(jsonPath("$.authenticated", is(true))) + .andExpect(jsonPath("$.type", is("status"))); + + // can't login with old password + token = getAuthToken(ePerson.getEmail(), password); + getClient(token).perform(get("/api/authn/status")) + .andExpect(status().isOk()) + .andExpect(jsonPath("$.okay", is(true))) + .andExpect(jsonPath("$.authenticated", is(false))) + .andExpect(jsonPath("$.type", is("status"))); + } + + @Test + public void patchChangePasswordWithWrongChallenge() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@example.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + String newPassword = "newpassword"; + String wrongPassword = "wrong_password"; + + List ops = new ArrayList(); + AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", wrongPassword); + ops.add(addOperation); + ops.add(addOperation2); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(admin.getEmail(), password); + + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + } + + @Test + public void patchChangePasswordWithNoChallenge() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@example.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + String newPassword = "newpassword"; + + List ops = new ArrayList(); + AddOperation addOperation = new AddOperation("/password", newPassword); + ops.add(addOperation); + String patchBody = getPatchContent(ops); + + String token = getAuthToken(admin.getEmail(), password); + + getClient(token).perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON)) + .andExpect(status().isForbidden()); + } + + @Test + public void patchChangePasswordWithIPAuthenticationMethod() throws Exception { + + context.turnOffAuthorisationSystem(); + + EPerson ePerson = EPersonBuilder.createEPerson(context) + .withNameInMetadata("John", "Doe") + .withEmail("Johndoe@example.com") + .withPassword(password) + .build(); + + context.restoreAuthSystemState(); + + String newPassword = "newpassword"; + + List ops = new ArrayList(); + AddOperation addOperation = new AddOperation("/password", newPassword); + AddOperation addOperation2 = new AddOperation("/challenge", password); + ops.add(addOperation); + ops.add(addOperation2); + String patchBody = getPatchContent(ops); + + context.setAuthenticationMethod("ip"); + + getClient().perform(patch("/api/eperson/epersons/" + ePerson.getID()) + .content(patchBody) + .contentType(MediaType.APPLICATION_JSON_PATCH_JSON) + .with(ip("5.5.5.5"))) + .andExpect(status().isUnauthorized()); + + } + }