[CST-6152] Renamed challenge to currentPassword

This commit is contained in:
Luca Giamminonni
2022-09-20 15:29:20 +02:00
parent 04c8e0d7c4
commit f2eb9c55b4
16 changed files with 66 additions and 56 deletions

View File

@@ -226,12 +226,13 @@ public interface AuthenticationMethod {
public boolean isUsed(Context context, HttpServletRequest request); public boolean isUsed(Context context, HttpServletRequest request);
/** /**
* Check if the given challenge is valid to change the password of the given * Check if the given current password is valid to change the password of the
* ePerson * given ePerson
* @param context The DSpace context * @param context The DSpace context
* @param ePerson the ePerson related to the password change * @param ePerson the ePerson related to the password change
* @param challenge The challenge to check * @param currentPassword The current password to check
* @return true if challenge matches with current password * @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);
} }

View File

@@ -209,11 +209,11 @@ public class AuthenticationServiceImpl implements AuthenticationService {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
for (AuthenticationMethod method : getAuthenticationMethodStack()) { for (AuthenticationMethod method : getAuthenticationMethodStack()) {
if (method.getName().equals(context.getAuthenticationMethod())) { if (method.getName().equals(context.getAuthenticationMethod())) {
return method.canChangePassword(context, ePerson, challenge); return method.canChangePassword(context, ePerson, currentPassword);
} }
} }

View File

@@ -280,7 +280,7 @@ public class IPAuthentication implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }
} }

View File

@@ -754,7 +754,7 @@ public class LDAPAuthentication
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }
} }

View File

@@ -87,7 +87,7 @@ public class OidcAuthentication implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }

View File

@@ -295,7 +295,7 @@ public class OidcAuthenticationBean implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }

View File

@@ -102,7 +102,7 @@ public class OrcidAuthentication implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }

View File

@@ -124,7 +124,7 @@ public class OrcidAuthenticationBean implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }

View File

@@ -269,10 +269,10 @@ public class PasswordAuthentication
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
if (context == null || ePerson == null) { if (context == null || ePerson == null) {
return false; return false;
} }
return ePersonService.checkPassword(context, ePerson, challenge); return ePersonService.checkPassword(context, ePerson, currentPassword);
} }
} }

View File

@@ -1278,7 +1278,7 @@ public class ShibAuthentication implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }
} }

View File

@@ -610,7 +610,7 @@ public class X509Authentication implements AuthenticationMethod {
} }
@Override @Override
public boolean canChangePassword(Context context, EPerson ePerson, String challenge) { public boolean canChangePassword(Context context, EPerson ePerson, String currentPassword) {
return false; return false;
} }
} }

View File

@@ -178,10 +178,15 @@ public interface AuthenticationService {
public String getAuthenticationMethod(Context context, HttpServletRequest request); public String getAuthenticationMethod(Context context, HttpServletRequest request);
/** /**
* Check if the given current password is valid to change the password of the
* given ePerson.
*
* @param context The DSpace context * @param context The DSpace context
* @param challenge The current password of the user * @param ePerson the ePerson related to the password change
* @return true if challenge matches with current password * @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);
} }

View File

@@ -190,7 +190,7 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH
HttpStatus.BAD_REQUEST.value()); HttpStatus.BAD_REQUEST.value());
} }
@ExceptionHandler({InvalidPasswordChallengeException.class}) @ExceptionHandler({WrongCurrentPasswordException.class})
protected void handleInvalidPasswordException(HttpServletRequest request, HttpServletResponse response, protected void handleInvalidPasswordException(HttpServletRequest request, HttpServletResponse response,
Exception ex) throws IOException { Exception ex) throws IOException {
sendErrorResponse(request, response, ex, ex.getMessage(), HttpServletResponse.SC_FORBIDDEN); sendErrorResponse(request, response, ex, ex.getMessage(), HttpServletResponse.SC_FORBIDDEN);

View File

@@ -8,15 +8,15 @@
package org.dspace.app.rest.exception; 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) * @author Mohamed Eskander (mohamed.eskander at 4science.it)
*/ */
public class InvalidPasswordChallengeException extends RuntimeException { public class WrongCurrentPasswordException extends RuntimeException {
private static final long serialVersionUID = 7774965236190392985L; private static final long serialVersionUID = 7774965236190392985L;
public InvalidPasswordChallengeException(String message) { public WrongCurrentPasswordException(String message) {
super(message); super(message);
} }

View File

@@ -10,11 +10,12 @@ package org.dspace.app.rest.repository.patch.operation;
import java.sql.SQLException; import java.sql.SQLException;
import java.util.Optional; import java.util.Optional;
import com.fasterxml.jackson.annotation.JsonProperty;
import org.apache.commons.lang3.StringUtils; import org.apache.commons.lang3.StringUtils;
import org.apache.logging.log4j.Logger; import org.apache.logging.log4j.Logger;
import org.dspace.app.rest.exception.DSpaceBadRequestException; 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.UnprocessableEntityException;
import org.dspace.app.rest.exception.WrongCurrentPasswordException;
import org.dspace.app.rest.model.patch.JsonValueEvaluator; import org.dspace.app.rest.model.patch.JsonValueEvaluator;
import org.dspace.app.rest.model.patch.Operation; import org.dspace.app.rest.model.patch.Operation;
import org.dspace.app.util.AuthorizeUtil; import org.dspace.app.util.AuthorizeUtil;
@@ -31,13 +32,14 @@ import org.springframework.security.access.AccessDeniedException;
import org.springframework.stereotype.Component; import org.springframework.stereotype.Component;
/** /**
* Implementation for EPerson password patches. This Add Operation will add a new password the an eperson if it had * Implementation for EPerson password patches. This Add Operation will add a
* no password before, or will replace the existing password with the new value. * new password the an eperson if it had no password before, or will replace the
* existing password with the new value.
* *
* Example: <code> * Example: <code>
* curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H " * curl -X PATCH http://${dspace.server.url}/api/epersons/eperson/<:id-eperson> -H "
* Content-Type: application/json" -d '[{ "op": "add", "path": " * Content-Type: application/json" -d '[{ "op": "add", "path": "
* /password", "value": {"password": "newpassword", "challenge": "currentpassword"}]' * /password", "value": {"new_password": "newpassword", "current_password": "currentpassword"}]'
* </code> * </code>
*/ */
@Component @Component
@@ -70,7 +72,7 @@ public class EPersonPasswordAddOperation<R> extends PatchOperation<R> {
PasswordVO passwordVO = parseOperationValue(operation); PasswordVO passwordVO = parseOperationValue(operation);
String newPassword = passwordVO.getPassword() String newPassword = passwordVO.getNewPassword()
.orElseThrow(() -> new DSpaceBadRequestException("No password provided")); .orElseThrow(() -> new DSpaceBadRequestException("No password provided"));
EPerson eperson = (EPerson) object; EPerson eperson = (EPerson) object;
@@ -83,7 +85,7 @@ public class EPersonPasswordAddOperation<R> extends PatchOperation<R> {
if (StringUtils.isNotBlank(token)) { if (StringUtils.isNotBlank(token)) {
verifyAndDeleteToken(context, eperson, token, operation); verifyAndDeleteToken(context, eperson, token, operation);
} else if (eperson.hasPasswordSet()) { } else if (eperson.hasPasswordSet()) {
verifyChallenge(context, eperson, passwordVO); verifyCurrentPassword(context, eperson, passwordVO);
} }
ePersonService.setPassword(eperson, newPassword); ePersonService.setPassword(eperson, newPassword);
@@ -123,14 +125,14 @@ public class EPersonPasswordAddOperation<R> extends PatchOperation<R> {
} }
} }
private void verifyChallenge(Context context, EPerson eperson, PasswordVO passwordVO) { private void verifyCurrentPassword(Context context, EPerson eperson, PasswordVO passwordVO) {
String challenge = passwordVO.getChallenge() String currentPassword = passwordVO.getCurrentPassword()
.orElseThrow(() -> new InvalidPasswordChallengeException("No challenge provided")); .orElseThrow(() -> new WrongCurrentPasswordException("No current password provided"));
boolean canChangePassword = authenticationService.canChangePassword(context, eperson, challenge); boolean canChangePassword = authenticationService.canChangePassword(context, eperson, currentPassword);
if (!canChangePassword) { 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<R> extends PatchOperation<R> {
} }
/** /**
* Value object that stores the new password to set and the challange to verify. * Value object that stores the new password to set and the current password to
* This object models the value of the operation. * verify. This object models the value of the operation.
* *
* @author Luca Giamminonni (luca.giamminonni at 4science.it) * @author Luca Giamminonni (luca.giamminonni at 4science.it)
* *
*/ */
public static class PasswordVO { public static class PasswordVO {
private String password; @JsonProperty("new_password")
private String newPassword;
private String challenge; @JsonProperty("current_password")
private String currentPassword;
public Optional<String> getPassword() { public Optional<String> getNewPassword() {
return Optional.ofNullable(password); return Optional.ofNullable(newPassword);
} }
public void setPassword(String password) { public void setNewPassword(String newPassword) {
this.password = password; this.newPassword = newPassword;
} }
public Optional<String> getChallenge() { public Optional<String> getCurrentPassword() {
return Optional.ofNullable(challenge); return Optional.ofNullable(currentPassword);
} }
public void setChallenge(String challenge) { public void setCurrentPassword(String currentPassword) {
this.challenge = challenge; this.currentPassword = currentPassword;
} }
} }

View File

@@ -3141,7 +3141,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
} }
@Test @Test
public void patchChangePasswordWithWrongChallenge() throws Exception { public void patchChangePasswordWithWrongCurrentPassword() throws Exception {
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
@@ -3166,7 +3166,7 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
} }
@Test @Test
public void patchChangePasswordWithNoChallenge() throws Exception { public void patchChangePasswordWithNoCurrentPassword() throws Exception {
context.turnOffAuthorisationSystem(); context.turnOffAuthorisationSystem();
@@ -3189,14 +3189,14 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
.andExpect(status().isForbidden()); .andExpect(status().isForbidden());
} }
private String buildPasswordAddOperationPatchBody(String password, String challenge) { private String buildPasswordAddOperationPatchBody(String password, String currentPassword) {
Map<String, String> value = new HashMap<>(); Map<String, String> value = new HashMap<>();
if (password != null) { if (password != null) {
value.put("password", password); value.put("new_password", password);
} }
if (challenge != null) { if (currentPassword != null) {
value.put("challenge", challenge); value.put("current_password", currentPassword);
} }
return getPatchContent(List.of(new AddOperation("/password", value))); return getPatchContent(List.of(new AddOperation("/password", value)));