[CST-6152] allow password change only if a challenge is resolved.

This commit is contained in:
eskander
2022-07-01 14:04:23 +02:00
parent c532d410a2
commit aafa2c7d25
18 changed files with 321 additions and 1 deletions

View File

@@ -224,4 +224,11 @@ public interface AuthenticationMethod {
* @return whether the authentication method is being used. * @return whether the authentication method is being used.
*/ */
public boolean isUsed(Context context, HttpServletRequest request); 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);
} }

View File

@@ -207,4 +207,14 @@ public class AuthenticationServiceImpl implements AuthenticationService {
return null; return null;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
for (AuthenticationMethod method : getAuthenticationMethodStack()) {
if (method.canChangePassword(context, challenge)) {
return true;
}
}
return false;
}
} }

View File

@@ -278,4 +278,9 @@ public class IPAuthentication implements AuthenticationMethod {
public boolean isUsed(final Context context, final HttpServletRequest request) { public boolean isUsed(final Context context, final HttpServletRequest request) {
return false; return false;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -752,4 +752,9 @@ public class LDAPAuthentication
} }
return false; return false;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -86,4 +86,9 @@ public class OidcAuthentication implements AuthenticationMethod {
return false; return false;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -294,4 +294,9 @@ public class OidcAuthenticationBean implements AuthenticationMethod {
return false; return false;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -101,4 +101,9 @@ public class OrcidAuthentication implements AuthenticationMethod {
return getOrcidAuthentication().isUsed(context, request); return getOrcidAuthentication().isUsed(context, request);
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -123,6 +123,11 @@ public class OrcidAuthenticationBean implements AuthenticationMethod {
return request.getAttribute(ORCID_AUTH_ATTRIBUTE) != null; return request.getAttribute(ORCID_AUTH_ATTRIBUTE) != null;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
@Override @Override
public boolean canSelfRegister(Context context, HttpServletRequest request, String username) throws SQLException { public boolean canSelfRegister(Context context, HttpServletRequest request, String username) throws SQLException {
return canSelfRegister(); return canSelfRegister();

View File

@@ -22,6 +22,7 @@ import org.dspace.core.LogHelper;
import org.dspace.eperson.EPerson; import org.dspace.eperson.EPerson;
import org.dspace.eperson.Group; import org.dspace.eperson.Group;
import org.dspace.eperson.factory.EPersonServiceFactory; import org.dspace.eperson.factory.EPersonServiceFactory;
import org.dspace.eperson.service.EPersonService;
import org.dspace.services.factory.DSpaceServicesFactory; import org.dspace.services.factory.DSpaceServicesFactory;
/** /**
@@ -53,6 +54,8 @@ public class PasswordAuthentication
private static final String PASSWORD_AUTHENTICATED = "password.authenticated"; private static final String PASSWORD_AUTHENTICATED = "password.authenticated";
private EPersonService ePersonService = EPersonServiceFactory.getInstance().getEPersonService();
/** /**
@@ -264,4 +267,13 @@ public class PasswordAuthentication
} }
return false; 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;
}
} }

View File

@@ -1276,5 +1276,10 @@ public class ShibAuthentication implements AuthenticationMethod {
} }
return false; return false;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -608,4 +608,9 @@ public class X509Authentication implements AuthenticationMethod {
} }
return false; return false;
} }
@Override
public boolean canChangePassword(Context context, String challenge) {
return false;
}
} }

View File

@@ -177,4 +177,11 @@ public interface AuthenticationService {
*/ */
public String getAuthenticationMethod(Context context, HttpServletRequest request); 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);
} }

View File

@@ -190,6 +190,12 @@ public class DSpaceApiExceptionControllerAdvice extends ResponseEntityExceptionH
HttpStatus.BAD_REQUEST.value()); 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 @Override
protected ResponseEntity<Object> handleMissingServletRequestParameter(MissingServletRequestParameterException ex, protected ResponseEntity<Object> handleMissingServletRequestParameter(MissingServletRequestParameterException ex,
HttpHeaders headers, HttpStatus status, HttpHeaders headers, HttpStatus status,

View File

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

View File

@@ -24,6 +24,7 @@ import org.dspace.app.rest.SearchRestMethod;
import org.dspace.app.rest.authorization.AuthorizationFeatureService; import org.dspace.app.rest.authorization.AuthorizationFeatureService;
import org.dspace.app.rest.exception.DSpaceBadRequestException; import org.dspace.app.rest.exception.DSpaceBadRequestException;
import org.dspace.app.rest.exception.EPersonNameNotProvidedException; 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.RESTEmptyWorkflowGroupException;
import org.dspace.app.rest.exception.UnprocessableEntityException; import org.dspace.app.rest.exception.UnprocessableEntityException;
import org.dspace.app.rest.model.EPersonRest; 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.Operation;
import org.dspace.app.rest.model.patch.Patch; import org.dspace.app.rest.model.patch.Patch;
import org.dspace.app.util.AuthorizeUtil; import org.dspace.app.util.AuthorizeUtil;
import org.dspace.authenticate.service.AuthenticationService;
import org.dspace.authorize.AuthorizeException; import org.dspace.authorize.AuthorizeException;
import org.dspace.authorize.service.AuthorizeService; import org.dspace.authorize.service.AuthorizeService;
import org.dspace.content.service.SiteService; import org.dspace.content.service.SiteService;
@@ -82,6 +84,9 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
@Autowired @Autowired
private RegistrationDataService registrationDataService; private RegistrationDataService registrationDataService;
@Autowired
private AuthenticationService authenticationService;
private final EPersonService es; private final EPersonService es;
@@ -294,6 +299,8 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, UUID uuid, protected void patch(Context context, HttpServletRequest request, String apiCategory, String model, UUID uuid,
Patch patch) throws AuthorizeException, SQLException { Patch patch) throws AuthorizeException, SQLException {
boolean passwordChangeFound = false; boolean passwordChangeFound = false;
boolean challengeFound = false;
boolean canChangePassword;
for (Operation operation : patch.getOperations()) { for (Operation operation : patch.getOperations()) {
if (StringUtils.equalsIgnoreCase(operation.getPath(), "/password")) { if (StringUtils.equalsIgnoreCase(operation.getPath(), "/password")) {
passwordChangeFound = true; passwordChangeFound = true;
@@ -308,6 +315,21 @@ public class EPersonRestRepository extends DSpaceObjectRestRepository<EPerson, E
if (passwordChangeFound && !StringUtils.equals(context.getAuthenticationMethod(), "password")) { if (passwordChangeFound && !StringUtils.equals(context.getAuthenticationMethod(), "password")) {
throw new AccessDeniedException("Refused to perform the EPerson patch based to change the password " + throw new AccessDeniedException("Refused to perform the EPerson patch based to change the password " +
"for non \"password\" authentication"); "for non \"password\" authentication");
} else if (passwordChangeFound) {
for (Operation operation : patch.getOperations()) {
if (StringUtils.equalsIgnoreCase(operation.getPath(), "/challenge")) {
challengeFound = true;
canChangePassword =
authenticationService.canChangePassword(context, String.valueOf(operation.getValue()));
if (!canChangePassword) {
throw new InvalidPasswordException("Current password is not valid");
}
}
}
if (!challengeFound) {
throw new InvalidPasswordException("no challenge found");
}
} }
} }
patchDSpaceObject(apiCategory, model, uuid, patch); patchDSpaceObject(apiCategory, model, uuid, patch);

View File

@@ -0,0 +1,42 @@
/**
* 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.operation;
import org.dspace.app.rest.model.patch.Operation;
import org.dspace.core.Context;
import org.dspace.eperson.EPerson;
import org.springframework.stereotype.Component;
/**
* Implementation for EPerson challenge patches.
*
* Example: <code>
* 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"]'
* </code>
*/
@Component
public class EPersonChallengeAddOperation<R> extends PatchOperation<R> {
/**
* 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));
}
}

View File

@@ -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.Operation;
import org.dspace.app.rest.model.patch.Patch; 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.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.EPersonPasswordAddOperation;
import org.dspace.app.rest.repository.patch.operation.PatchOperation; import org.dspace.app.rest.repository.patch.operation.PatchOperation;
import org.dspace.app.rest.utils.ContextUtil; import org.dspace.app.rest.utils.ContextUtil;
@@ -123,7 +124,8 @@ public class EPersonRestPermissionEvaluatorPlugin extends RestObjectPermissionEv
*/ */
for (Operation op: operations) { for (Operation op: operations) {
if (!(op.getPath().contentEquals(EPersonPasswordAddOperation.OPERATION_PASSWORD_CHANGE) 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; return false;
} }
} }

View File

@@ -1304,7 +1304,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
List<Operation> ops = new ArrayList<Operation>(); List<Operation> ops = new ArrayList<Operation>();
AddOperation addOperation = new AddOperation("/password", newPassword); AddOperation addOperation = new AddOperation("/password", newPassword);
AddOperation addOperation2 = new AddOperation("/challenge", password);
ops.add(addOperation); ops.add(addOperation);
ops.add(addOperation2);
String patchBody = getPatchContent(ops); String patchBody = getPatchContent(ops);
String token = getAuthToken(admin.getEmail(), password); String token = getAuthToken(admin.getEmail(), password);
@@ -1401,7 +1403,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
List<Operation> ops = new ArrayList<Operation>(); List<Operation> ops = new ArrayList<Operation>();
AddOperation addOperation = new AddOperation("/password", newPassword); AddOperation addOperation = new AddOperation("/password", newPassword);
AddOperation addOperation2 = new AddOperation("/challenge", password);
ops.add(addOperation); ops.add(addOperation);
ops.add(addOperation2);
String patchBody = getPatchContent(ops); String patchBody = getPatchContent(ops);
String token = getAuthToken(ePerson.getEmail(), password); String token = getAuthToken(ePerson.getEmail(), password);
@@ -1444,7 +1448,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
List<Operation> ops = new ArrayList<Operation>(); List<Operation> ops = new ArrayList<Operation>();
AddOperation addOperation = new AddOperation("/password", newPassword); AddOperation addOperation = new AddOperation("/password", newPassword);
AddOperation addOperation2 = new AddOperation("/challenge", password);
ops.add(addOperation); ops.add(addOperation);
ops.add(addOperation2);
String patchBody = getPatchContent(ops); String patchBody = getPatchContent(ops);
String token = getAuthToken(admin.getEmail(), password); String token = getAuthToken(admin.getEmail(), password);
@@ -1536,7 +1542,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
List<Operation> ops = new ArrayList<>(); List<Operation> ops = new ArrayList<>();
AddOperation addOperation = new AddOperation("/password", null); AddOperation addOperation = new AddOperation("/password", null);
AddOperation addOperation2 = new AddOperation("/challenge", password);
ops.add(addOperation); ops.add(addOperation);
ops.add(addOperation2);
String patchBody = getPatchContent(ops); String patchBody = getPatchContent(ops);
// adding null pw should return bad request // adding null pw should return bad request
@@ -1581,7 +1589,9 @@ public class EPersonRestRepositoryIT extends AbstractControllerIntegrationTest {
List<Operation> ops = new ArrayList<Operation>(); List<Operation> ops = new ArrayList<Operation>();
AddOperation addOperation = new AddOperation("/password", newPassword); AddOperation addOperation = new AddOperation("/password", newPassword);
AddOperation addOperation2 = new AddOperation("/challenge", password);
ops.add(addOperation); ops.add(addOperation);
ops.add(addOperation2);
String patchBody = getPatchContent(ops); String patchBody = getPatchContent(ops);
// initialize password with add operation, not set during creation // 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<Operation> ops = new ArrayList<Operation>();
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<Operation> ops = new ArrayList<Operation>();
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<Operation> ops = new ArrayList<Operation>();
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<Operation> ops = new ArrayList<Operation>();
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());
}
} }