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 0684339a22..274779e928 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationMethod.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationMethod.java @@ -226,12 +226,13 @@ public interface AuthenticationMethod { public boolean isUsed(Context context, HttpServletRequest request); /** - * Check if the given challenge is valid to change the password of the given - * ePerson - * @param context The DSpace context - * @param ePerson the ePerson related to the password change - * @param challenge The challenge to check - * @return true if challenge matches with current password + * Check if the given current password is valid to change the password of the + * given ePerson + * @param context The DSpace context + * @param ePerson the ePerson related to the password change + * @param currentPassword The current password to check + * @return true if the provided password matches with current + * password */ - public boolean canChangePassword(Context context, EPerson ePerson, String challenge); + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword); } 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 9ad11bd073..a9449b87d4 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/AuthenticationServiceImpl.java @@ -209,11 +209,11 @@ public class AuthenticationServiceImpl implements AuthenticationService { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { for (AuthenticationMethod method : getAuthenticationMethodStack()) { if (method.getName().equals(context.getAuthenticationMethod())) { - return method.canChangePassword(context, ePerson, challenge); + return method.canChangePassword(context, ePerson, currentPassword); } } 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 ce5e34ac3c..9c37fcee47 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/IPAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/IPAuthentication.java @@ -280,7 +280,7 @@ public class IPAuthentication implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 0a5777ec8b..f3c6022e02 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/LDAPAuthentication.java @@ -754,7 +754,7 @@ public class LDAPAuthentication } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 98564f28cc..5d4635d48e 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthentication.java @@ -87,7 +87,7 @@ public class OidcAuthentication implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 eee19d6d27..8a4ac190c8 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OidcAuthenticationBean.java @@ -295,7 +295,7 @@ public class OidcAuthenticationBean implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 951859533d..3e9ff6638a 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthentication.java @@ -102,7 +102,7 @@ public class OrcidAuthentication implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 3386738dc2..a11bbfc867 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/OrcidAuthenticationBean.java @@ -124,7 +124,7 @@ public class OrcidAuthenticationBean implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { return false; } 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 03ebb763b4..6d1ca862d3 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/PasswordAuthentication.java @@ -269,10 +269,10 @@ public class PasswordAuthentication } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { if (context == null || ePerson == null) { return false; } - return ePersonService.checkPassword(context, ePerson, challenge); + return ePersonService.checkPassword(context, ePerson, currentPassword); } } 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 62e06c94bc..791634a7dc 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/ShibAuthentication.java @@ -1278,7 +1278,7 @@ public class ShibAuthentication implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 ae6f16191f..12dc5feda5 100644 --- a/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java +++ b/dspace-api/src/main/java/org/dspace/authenticate/X509Authentication.java @@ -610,7 +610,7 @@ public class X509Authentication implements AuthenticationMethod { } @Override - public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) { 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 024af47db2..e955302ec3 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 @@ -178,10 +178,15 @@ 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 + * Check if the given current password is valid to change the password of the + * given ePerson. + * + * @param context The DSpace context + * @param ePerson the ePerson related to the password change + * @param currentPassword The current password to check + * @return true if the provided password matches with current + * password */ - public boolean canChangePassword(Context context, EPerson ePerson, String challenge); + public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword); } 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 5a65ea2ed5..3ebf668226 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,7 +190,7 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH HttpStatus.BAD_REQUEST.value()); } - @ExceptionHandler({InvalidPasswordChallengeException.class}) + @ExceptionHandler({WrongCurrentPasswordException.class}) protected void handleInvalidPasswordException(HttpServletRequest request, HttpServletResponse response, Exception ex) throws IOException { sendErrorResponse(request, response, ex, ex.getMessage(), HttpServletResponse.SC_FORBIDDEN); diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/InvalidPasswordChallengeException.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/WrongCurrentPasswordException.java similarity index 67% rename from dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/InvalidPasswordChallengeException.java rename to dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/WrongCurrentPasswordException.java index 15250a6273..95c29fa592 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/InvalidPasswordChallengeException.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/exception/WrongCurrentPasswordException.java @@ -8,15 +8,15 @@ package org.dspace.app.rest.exception; /** - * This exception is thrown when the password challenge of user is invalid. + * This exception is thrown when the provided current password is wrong. * * @author Mohamed Eskander (mohamed.eskander at 4science.it) */ -public class InvalidPasswordChallengeException extends RuntimeException { +public class WrongCurrentPasswordException extends RuntimeException { private static final long serialVersionUID = 7774965236190392985L; - public InvalidPasswordChallengeException(String message) { + public WrongCurrentPasswordException(String message) { super(message); } diff --git a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/operation/EPersonPasswordAddOperation.java b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/operation/EPersonPasswordAddOperation.java index d43b965cda..3c9da1f472 100644 --- a/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/operation/EPersonPasswordAddOperation.java +++ b/dspace-server-webapp/src/main/java/org/dspace/app/rest/repository/patch/operation/EPersonPasswordAddOperation.java @@ -10,11 +10,12 @@ package org.dspace.app.rest.repository.patch.operation; import java.sql.SQLException; import java.util.Optional; +import com.fasterxml.jackson.annotation.JsonProperty; import org.apache.commons.lang3.StringUtils; import org.apache.logging.log4j.Logger; import org.dspace.app.rest.exception.DSpaceBadRequestException; -import org.dspace.app.rest.exception.InvalidPasswordChallengeException; import org.dspace.app.rest.exception.UnprocessableEntityException; +import org.dspace.app.rest.exception.WrongCurrentPasswordException; import org.dspace.app.rest.model.patch.JsonValueEvaluator; import org.dspace.app.rest.model.patch.Operation; import org.dspace.app.util.AuthorizeUtil; @@ -31,13 +32,14 @@ import org.springframework.security.access.AccessDeniedException; import org.springframework.stereotype.Component; /** - * Implementation for EPerson password patches. This Add Operation will add a new password the an eperson if it had - * no password before, or will replace the existing password with the new value. + * Implementation for EPerson password patches. This Add Operation will add a + * new password the an eperson if it had no password before, or will replace the + * existing password with the new value. * * Example: * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H " * Content-Type: application/json" -d '[{ "op": "add", "path": " - * /password", "value": {"password": "newpassword", "challenge": "currentpassword"}]' + * /password", "value": {"new_password": "newpassword", "current_password": "currentpassword"}]' * */ @Component @@ -70,7 +72,7 @@ public class EPersonPasswordAddOperation extends PatchOperation { PasswordVO passwordVO = parseOperationValue(operation); - String newPassword = passwordVO.getPassword() + String newPassword = passwordVO.getNewPassword() .orElseThrow(() -> new DSpaceBadRequestException("No password provided")); EPerson eperson = (EPerson) object; @@ -83,7 +85,7 @@ public class EPersonPasswordAddOperation extends PatchOperation { if (StringUtils.isNotBlank(token)) { verifyAndDeleteToken(context, eperson, token, operation); } else if (eperson.hasPasswordSet()) { - verifyChallenge(context, eperson, passwordVO); + verifyCurrentPassword(context, eperson, passwordVO); } ePersonService.setPassword(eperson, newPassword); @@ -123,14 +125,14 @@ public class EPersonPasswordAddOperation extends PatchOperation { } } - private void verifyChallenge(Context context, EPerson eperson, PasswordVO passwordVO) { + private void verifyCurrentPassword(Context context, EPerson eperson, PasswordVO passwordVO) { - String challenge = passwordVO.getChallenge() - .orElseThrow(() -> new InvalidPasswordChallengeException("No challenge provided")); + String currentPassword = passwordVO.getCurrentPassword() + .orElseThrow(() -> new WrongCurrentPasswordException("No current password provided")); - boolean canChangePassword = authenticationService.canChangePassword(context, eperson, challenge); + boolean canChangePassword = authenticationService.canChangePassword(context, eperson, currentPassword); if (!canChangePassword) { - throw new InvalidPasswordChallengeException("The provided challenge is not valid"); + throw new WrongCurrentPasswordException("The provided password is wrong"); } } @@ -142,32 +144,34 @@ public class EPersonPasswordAddOperation extends PatchOperation { } /** - * Value object that stores the new password to set and the challange to verify. - * This object models the value of the operation. + * Value object that stores the new password to set and the current password to + * verify. This object models the value of the operation. * * @author Luca Giamminonni (luca.giamminonni at 4science.it) * */ public static class PasswordVO { - private String password; + @JsonProperty("new_password") + private String newPassword; - private String challenge; + @JsonProperty("current_password") + private String currentPassword; - public Optional getPassword() { - return Optional.ofNullable(password); + public Optional getNewPassword() { + return Optional.ofNullable(newPassword); } - public void setPassword(String password) { - this.password = password; + public void setNewPassword(String newPassword) { + this.newPassword = newPassword; } - public Optional getChallenge() { - return Optional.ofNullable(challenge); + public Optional getCurrentPassword() { + return Optional.ofNullable(currentPassword); } - public void setChallenge(String challenge) { - this.challenge = challenge; + public void setCurrentPassword(String currentPassword) { + this.currentPassword = currentPassword; } } 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 7fddc8d027..aee068857d 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 @@ -3141,7 +3141,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { } @Test - public void patchChangePasswordWithWrongChallenge() throws Exception { + public void patchChangePasswordWithWrongCurrentPassword() throws Exception { context.turnOffAuthorisationSystem(); @@ -3166,7 +3166,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { } @Test - public void patchChangePasswordWithNoChallenge() throws Exception { + public void patchChangePasswordWithNoCurrentPassword() throws Exception { context.turnOffAuthorisationSystem(); @@ -3189,14 +3189,14 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest { .andExpect(status().isForbidden()); } - private String buildPasswordAddOperationPatchBody(String password, String challenge) { + private String buildPasswordAddOperationPatchBody(String password, String currentPassword) { Map value = new HashMap<>(); if (password != null) { - value.put("password", password); + value.put("new_password", password); } - if (challenge != null) { - value.put("challenge", challenge); + if (currentPassword != null) { + value.put("current_password", currentPassword); } return getPatchContent(List.of(new AddOperation("/password", value)));