From 9bfb2cdadbd1240edfc594fd680f7350b14efdde Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 17 Oct 2018 11:58:37 +0200 Subject: [PATCH] Move password logic to manager --- .../ChangePasswordNotAllowedException.java | 8 +-- .../scm/user/InvalidPasswordException.java | 6 +- .../src/main/java/sonia/scm/user/User.java | 26 ++----- .../main/java/sonia/scm/user/UserManager.java | 16 +++-- .../sonia/scm/user/UserManagerDecorator.java | 11 ++- .../main/java/sonia/scm/web/VndMediaType.java | 2 + .../test/java/sonia/scm/it/UserITCase.java | 24 ------- .../java/sonia/scm/it/utils/ScmRequests.java | 17 +++-- .../resources/AbstractManagerResource.java | 30 ++++---- .../resources/IdResourceManagerAdapter.java | 48 ------------- .../scm/api/v2/resources/MeResource.java | 17 ++--- .../api/v2/resources/PasswordChangeDto.java | 1 + .../v2/resources/PasswordOverwriteDto.java | 14 ++++ .../scm/api/v2/resources/ResourceLinks.java | 2 +- .../SingleResourceManagerAdapter.java | 19 ++--- .../scm/api/v2/resources/UserResource.java | 16 ++--- .../api/v2/resources/UserToUserDtoMapper.java | 6 +- .../DefaultAuthorizationCollector.java | 6 +- .../sonia/scm/user/DefaultUserManager.java | 44 ++++++++---- .../scm/api/v2/resources/MeResourceTest.java | 49 +++++++++---- .../v2/resources/UserRootResourceTest.java | 52 ++++++++++++-- .../v2/resources/UserToUserDtoMapperTest.java | 13 ++-- .../DefaultAuthorizationCollectorTest.java | 6 +- .../scm/user/DefaultUserManagerTest.java | 70 ++++++++++++++++++- 24 files changed, 285 insertions(+), 218 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordOverwriteDto.java diff --git a/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java b/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java index a942943881..fda5e69323 100644 --- a/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java +++ b/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java @@ -2,12 +2,10 @@ package sonia.scm.user; public class ChangePasswordNotAllowedException extends RuntimeException { - public static final String WRONG_USER_TYPE = "User of type {0} are not allowed to change password"; - @SuppressWarnings("squid:S2068") - public static final String OLD_PASSWORD_REQUIRED = "the old password is required."; + public static final String WRONG_USER_TYPE = "User of type %s are not allowed to change password"; - public ChangePasswordNotAllowedException(String message) { - super(message); + public ChangePasswordNotAllowedException(String type) { + super(String.format(WRONG_USER_TYPE, type)); } } diff --git a/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java index e06191a8f2..870430a1bb 100644 --- a/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java +++ b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java @@ -2,9 +2,7 @@ package sonia.scm.user; public class InvalidPasswordException extends RuntimeException { - public static final String INVALID_MATCHING = "The given Password does not match with the stored one."; - - public InvalidPasswordException(String message) { - super(message); + public InvalidPasswordException() { + super("The given Password does not match with the stored one."); } } diff --git a/scm-core/src/main/java/sonia/scm/user/User.java b/scm-core/src/main/java/sonia/scm/user/User.java index b125561043..cae383a402 100644 --- a/scm-core/src/main/java/sonia/scm/user/User.java +++ b/scm-core/src/main/java/sonia/scm/user/User.java @@ -50,15 +50,16 @@ import javax.xml.bind.annotation.XmlAccessorType; import javax.xml.bind.annotation.XmlRootElement; import java.security.Principal; -import static sonia.scm.user.InvalidPasswordException.INVALID_MATCHING; - //~--- JDK imports ------------------------------------------------------------ /** * * @author Sebastian Sdorra */ -@StaticPermissions(value = "user", globalPermissions = {"create", "list", "autocomplete", "changeOwnPassword"}) +@StaticPermissions( + value = "user", + globalPermissions = {"create", "list", "autocomplete"}, + permissions = {"read", "modify", "delete", "changePassword"}) @XmlRootElement(name = "users") @XmlAccessorType(XmlAccessType.FIELD) public class User extends BasicPropertiesAware implements Principal, ModelObject, PermissionObject, ReducedModelObject @@ -276,25 +277,6 @@ public class User extends BasicPropertiesAware implements Principal, ModelObject //J+ } - public User changePassword(String password){ - setPassword(password); - return this; - } - - /** - * Match given old password from the dto with the stored password before updating - * - * @param newPassword the new password - * @param oldPassword the old password - * @return this - */ - public User changePassword(String newPassword, String oldPassword){ - if (!getPassword().equals(oldPassword)) { - throw new InvalidPasswordException(INVALID_MATCHING); - } - setPassword(newPassword); - return this; - } //~--- get methods ---------------------------------------------------------- /** diff --git a/scm-core/src/main/java/sonia/scm/user/UserManager.java b/scm-core/src/main/java/sonia/scm/user/UserManager.java index 55f6a17d3f..f301c1f2b1 100644 --- a/scm-core/src/main/java/sonia/scm/user/UserManager.java +++ b/scm-core/src/main/java/sonia/scm/user/UserManager.java @@ -38,11 +38,7 @@ package sonia.scm.user; import sonia.scm.Manager; import sonia.scm.search.Searchable; -import java.text.MessageFormat; import java.util.Collection; -import java.util.function.Consumer; - -import static sonia.scm.user.ChangePasswordNotAllowedException.WRONG_USER_TYPE; /** * The central class for managing {@link User} objects. @@ -87,5 +83,17 @@ public interface UserManager */ Collection autocomplete(String filter); + /** + * Changes the password of the logged in user. + * @param oldPassword The current encrypted password of the user. + * @param newPassword The new encrypted password of the user. + */ + void changePasswordForLoggedInUser(String oldPassword, String newPassword); + /** + * Overwrites the password for the given user id. This needs user write privileges. + * @param userId The id of the user to change the password for. + * @param newPassword The new encrypted password. + */ + void overwritePassword(String userId, String newPassword); } diff --git a/scm-core/src/main/java/sonia/scm/user/UserManagerDecorator.java b/scm-core/src/main/java/sonia/scm/user/UserManagerDecorator.java index f291e325c1..0384fe1b52 100644 --- a/scm-core/src/main/java/sonia/scm/user/UserManagerDecorator.java +++ b/scm-core/src/main/java/sonia/scm/user/UserManagerDecorator.java @@ -126,7 +126,16 @@ public class UserManagerDecorator extends ManagerDecorator return decorated.autocomplete(filter); } - //~--- fields --------------------------------------------------------------- + @Override + public void changePasswordForLoggedInUser(String oldPassword, String newPassword) { + decorated.changePasswordForLoggedInUser(oldPassword, newPassword); + } + + @Override + public void overwritePassword(String userId, String newPassword) { + decorated.overwritePassword(userId, newPassword); + } +//~--- fields --------------------------------------------------------------- /** Field description */ private final UserManager decorated; diff --git a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java index b6f2210d80..7b6d5cb039 100644 --- a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java +++ b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java @@ -39,6 +39,8 @@ public class VndMediaType { public static final String UI_PLUGIN_COLLECTION = PREFIX + "uiPluginCollection" + SUFFIX; @SuppressWarnings("squid:S2068") public static final String PASSWORD_CHANGE = PREFIX + "passwordChange" + SUFFIX; + @SuppressWarnings("squid:S2068") + public static final String PASSWORD_OVERWRITE = PREFIX + "passwordOverwrite" + SUFFIX; public static final String ME = PREFIX + "me" + SUFFIX; public static final String SOURCE = PREFIX + "source" + SUFFIX; diff --git a/scm-it/src/test/java/sonia/scm/it/UserITCase.java b/scm-it/src/test/java/sonia/scm/it/UserITCase.java index b646fea0b1..f517123dcc 100644 --- a/scm-it/src/test/java/sonia/scm/it/UserITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/UserITCase.java @@ -85,30 +85,6 @@ public class UserITCase { .assertStatusCode(403); } - @Test - public void nonAdminUserShouldChangeOwnPassword() { - String newUser = "user1"; - String password = "pass"; - TestData.createUser(newUser, password, false, "xml", "em@l.de"); - String newPassword = "new_password"; - ScmRequests.start() - .requestIndexResource(newUser, password) - .assertUsersLinkDoesNotExists(); - // use the users/password endpoint bypassed the index resource - ScmRequests.start() - .requestUser(newUser, password, newUser) - .assertStatusCode(200) - .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.FALSE)) - .requestChangePassword(password, newPassword) // the oldPassword is needed when the own password should be changed - .assertStatusCode(204); -// // assert password is changed -> login with the new Password - ScmRequests.start() - .requestUser(newUser, newPassword, newUser) - .assertStatusCode(200) - .assertAdmin(isAdmin -> assertThat(isAdmin).isEqualTo(Boolean.FALSE)) - .assertPassword(Assert::assertNull); - } - @Test public void shouldHidePasswordLinkIfUserTypeIsNotXML() { String newUser = "user"; diff --git a/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java index 6fc8c85140..d15cdc9b7f 100644 --- a/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java @@ -2,6 +2,8 @@ package sonia.scm.it.utils; import io.restassured.RestAssured; import io.restassured.response.Response; +import org.slf4j.Logger; +import org.slf4j.LoggerFactory; import sonia.scm.web.VndMediaType; import java.util.List; @@ -24,7 +26,8 @@ import static sonia.scm.it.utils.TestData.createPasswordChangeJson; */ public class ScmRequests { - private String url; + private static final Logger LOG = LoggerFactory.getLogger(ScmRequests.class); + private String username; private String password; @@ -47,7 +50,7 @@ public class ScmRequests { public ChangePasswordResponse requestUserChangePassword(String username, String password, String userPathParam, String newPassword) { setUsername(username); setPassword(password); - return new ChangePasswordResponse<>(applyPUTRequest(RestUtil.REST_BASE_URL.resolve("users/"+userPathParam+"/password").toString(), VndMediaType.PASSWORD_CHANGE, TestData.createPasswordChangeJson(password,newPassword)), null); + return new ChangePasswordResponse<>(applyPUTRequest(RestUtil.REST_BASE_URL.resolve("users/"+userPathParam+"/password").toString(), VndMediaType.PASSWORD_OVERWRITE, TestData.createPasswordChangeJson(password,newPassword)), null); } @@ -85,6 +88,7 @@ public class ScmRequests { * @return the response of the GET request using the given url */ private Response applyGETRequestWithQueryParams(String url, String params) { + LOG.info("GET {}", url); return RestAssured.given() .auth().preemptive().basic(username, password) .when() @@ -127,6 +131,7 @@ public class ScmRequests { * @return the response of the PUT request using the given url */ private Response applyPUTRequest(String url, String mediaType, String body) { + LOG.info("PUT {}", url); return RestAssured.given() .auth().preemptive().basic(username, password) .when() @@ -144,8 +149,6 @@ public class ScmRequests { this.password = password; } - - public class IndexResponse extends ModelResponse { public static final String LINK_AUTOCOMPLETE_USERS = "_links.autocomplete.find{it.name=='users'}.href"; public static final String LINK_AUTOCOMPLETE_GROUPS = "_links.autocomplete.find{it.name=='groups'}.href"; @@ -292,7 +295,9 @@ public class ScmRequests { super(response, previousResponse); } - + public ChangePasswordResponse requestChangePassword(String oldPassword, String newPassword) { + return new ChangePasswordResponse<>(applyPUTRequestFromLink(super.response, LINKS_PASSWORD_HREF, VndMediaType.PASSWORD_CHANGE, createPasswordChangeJson(oldPassword, newPassword)), this); + } } public class UserResponse extends ModelResponse, PREV> { @@ -328,7 +333,7 @@ public class ScmRequests { } public ChangePasswordResponse requestChangePassword(String oldPassword, String newPassword) { - return new ChangePasswordResponse<>(applyPUTRequestFromLink(super.response, LINKS_PASSWORD_HREF, VndMediaType.PASSWORD_CHANGE, createPasswordChangeJson(oldPassword, newPassword)), this); + return new ChangePasswordResponse<>(applyPUTRequestFromLink(super.response, LINKS_PASSWORD_HREF, VndMediaType.PASSWORD_OVERWRITE, createPasswordChangeJson(oldPassword, newPassword)), this); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java index 794f11893b..f00072cdcb 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/rest/resources/AbstractManagerResource.java @@ -35,7 +35,6 @@ package sonia.scm.api.rest.resources; //~--- non-JDK imports -------------------------------------------------------- -import com.github.sdorra.ssp.PermissionCheck; import org.apache.commons.beanutils.BeanComparator; import org.apache.shiro.authz.AuthorizationException; import org.slf4j.Logger; @@ -64,8 +63,6 @@ import java.util.Arrays; import java.util.Collection; import java.util.Comparator; import java.util.Date; -import java.util.function.Consumer; -import java.util.function.Function; //~--- JDK imports ------------------------------------------------------------ @@ -199,24 +196,27 @@ public abstract class AbstractManagerResource { return response; } - public Response update(T item, Function permissionChecker ){ - Consumer updateAction = mng -> mng.modify(item, permissionChecker); - return applyUpdate(item, updateAction); - } - - public Response update(String name, T item) { - Consumer updateAction = mng -> mng.modify(item); - return applyUpdate(item, updateAction); - } - - public Response applyUpdate(T item, Consumer updateAction) { + /** + * Method description + * + * + * + * + * @param name + * @param item + * + * + * @return + */ + public Response update(String name, T item) + { Response response = null; preUpdate(item); try { - updateAction.accept(manager); + manager.modify(item); response = Response.noContent().build(); } catch (AuthorizationException ex) diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IdResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IdResourceManagerAdapter.java index 5abc209463..af2b4217e9 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IdResourceManagerAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/IdResourceManagerAdapter.java @@ -47,54 +47,6 @@ class IdResourceManagerAdapter getChangePasswordPermission(String usernameToChangePassword) { - AssertUtil.assertIsNotEmpty(usernameToChangePassword); - return model -> { - User user = (User) model; - if (usernameToChangePassword.equals(AuthenticationUtil.getAuthenticatedUsername())) { - return UserPermissions.changeOwnPassword(); - } - return UserPermissions.modify(user); - }; - } - - - /** - * Check if a user can modify the password - * - * 1 - the permission changeOwnPassword should be checked - * 2 - Only account of the default type "xml" can change their password - * - */ - private Consumer getChangePasswordChecker() { - return model -> { - User user = (User) model; - UserPermissions.changeOwnPassword().check(); - UserManager userManager = (UserManager) manager; - if (!userManager.isTypeDefault(user)) { - throw new ChangePasswordNotAllowedException(MessageFormat.format(WRONG_USER_TYPE, user.getType())); - } - }; - } - - - public Response changePassword(String id, Function applyChanges ) throws ConcurrentModificationException { - return singleAdapter.changePassword( - loadBy(id), - applyChanges, - idStaysTheSame(id), - getChangePasswordChecker(), - getChangePasswordPermission(id)); - } - public Response update(String id, Function applyChanges) throws ConcurrentModificationException { return singleAdapter.update( loadBy(id), diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java index e30b589c6f..f616aff852 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeResource.java @@ -5,14 +5,12 @@ import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; import org.apache.shiro.SecurityUtils; import org.apache.shiro.authc.credential.PasswordService; -import sonia.scm.ConcurrentModificationException; -import sonia.scm.user.ChangePasswordNotAllowedException; -import sonia.scm.user.InvalidPasswordException; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; import javax.inject.Inject; +import javax.validation.Valid; import javax.ws.rs.Consumes; import javax.ws.rs.GET; import javax.ws.rs.PUT; @@ -22,9 +20,6 @@ import javax.ws.rs.core.Context; import javax.ws.rs.core.Request; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; -import java.util.function.Consumer; - -import static sonia.scm.user.InvalidPasswordException.INVALID_MATCHING; /** @@ -78,12 +73,8 @@ public class MeResource { }) @TypeHint(TypeHint.NO_CONTENT.class) @Consumes(VndMediaType.PASSWORD_CHANGE) - public Response changePassword(PasswordChangeDto passwordChangeDto) throws ConcurrentModificationException { - String name = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); - if (passwordChangeDto.getOldPassword() == null){ - throw new ChangePasswordNotAllowedException(ChangePasswordNotAllowedException.OLD_PASSWORD_REQUIRED); - } - return adapter.changePassword(name, user -> user.clone().changePassword(passwordService.encryptPassword(passwordChangeDto.getNewPassword()), passwordService.encryptPassword(passwordChangeDto.getOldPassword()))); + public Response changePassword(@Valid PasswordChangeDto passwordChangeDto) { + userManager.changePasswordForLoggedInUser(passwordService.encryptPassword(passwordChangeDto.getOldPassword()), passwordService.encryptPassword(passwordChangeDto.getNewPassword())); + return Response.noContent().build(); } - } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordChangeDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordChangeDto.java index 47ad6cd147..8a69c58e86 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordChangeDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordChangeDto.java @@ -10,6 +10,7 @@ import org.hibernate.validator.constraints.NotEmpty; @ToString public class PasswordChangeDto { + @NotEmpty private String oldPassword; @NotEmpty diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordOverwriteDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordOverwriteDto.java new file mode 100644 index 0000000000..0570ed81e1 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordOverwriteDto.java @@ -0,0 +1,14 @@ +package sonia.scm.api.v2.resources; + +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; +import org.hibernate.validator.constraints.NotEmpty; + +@Getter +@Setter +@ToString +public class PasswordOverwriteDto { + @NotEmpty + private String newPassword; +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java index 820e722814..2b37eda9b7 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ResourceLinks.java @@ -87,7 +87,7 @@ class ResourceLinks { } public String passwordChange(String name) { - return userLinkBuilder.method("getUserResource").parameters(name).method("changePassword").parameters().href(); + return userLinkBuilder.method("getUserResource").parameters(name).method("overwritePassword").parameters().href(); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java index b5763c4ff0..d484567bf8 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/SingleResourceManagerAdapter.java @@ -1,6 +1,5 @@ package sonia.scm.api.v2.resources; -import com.github.sdorra.ssp.PermissionCheck; import de.otto.edison.hal.HalRepresentation; import sonia.scm.ConcurrentModificationException; import sonia.scm.Manager; @@ -17,6 +16,8 @@ import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; + /** * Adapter from resource http endpoints to managers, for Single resources (e.g. {@code /user/name}). * @@ -53,32 +54,26 @@ class SingleResourceManagerAdapter> reader, Function applyChanges, Predicate hasSameKey, Consumer checker, Function permissionCheck) throws ConcurrentModificationException { + public Response update(Supplier> reader, Function applyChanges, Predicate hasSameKey, Consumer checker) throws NotFoundException, ConcurrentModificationException { MODEL_OBJECT existingModelObject = reader.get().orElseThrow(NotFoundException::new); - MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject); - checkForUpdate(hasSameKey, existingModelObject, changedModelObject); checker.accept(existingModelObject); - return update(changedModelObject, permissionCheck); + return update(reader,applyChanges,hasSameKey); } /** * Update the model object for the given id according to the given function and returns a corresponding http response. * This handles all corner cases, eg. no matching object for the id or missing privileges. */ - public Response update(Supplier> reader, Function applyChanges, Predicate hasSameKey) throws ConcurrentModificationException { + public Response update(Supplier> reader, Function applyChanges, Predicate hasSameKey) throws NotFoundException, ConcurrentModificationException { MODEL_OBJECT existingModelObject = reader.get().orElseThrow(NotFoundException::new); MODEL_OBJECT changedModelObject = applyChanges.apply(existingModelObject); - checkForUpdate(hasSameKey, existingModelObject, changedModelObject); - return update(getId(existingModelObject), changedModelObject); - } - - public void checkForUpdate(Predicate hasSameKey, MODEL_OBJECT existingModelObject, MODEL_OBJECT changedModelObject) throws ConcurrentModificationException { if (!hasSameKey.test(changedModelObject)) { - throw new IllegalArgumentException("illegal change of id"); + return Response.status(BAD_REQUEST).entity("illegal change of id").build(); } else if (modelObjectWasModifiedConcurrently(existingModelObject, changedModelObject)) { throw new ConcurrentModificationException(); } + return update(getId(existingModelObject), changedModelObject); } private boolean modelObjectWasModifiedConcurrently(MODEL_OBJECT existing, MODEL_OBJECT updated) { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java index 5912184f4e..ef4b8a6eb4 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserResource.java @@ -3,10 +3,8 @@ package sonia.scm.api.v2.resources; import com.webcohesion.enunciate.metadata.rs.ResponseCode; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; -import org.apache.shiro.SecurityUtils; import org.apache.shiro.authc.credential.PasswordService; import sonia.scm.ConcurrentModificationException; -import sonia.scm.user.ChangePasswordNotAllowedException; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; @@ -116,11 +114,11 @@ public class UserResource { * Note: This method requires "user:changeOwnPassword" privilege to modify the own password. * * @param name name of the user to be modified - * @param passwordChangeDto change password object to modify password. the old password is here not required + * @param passwordOverwriteDto change password object to modify password. the old password is here not required */ @PUT @Path("password") - @Consumes(VndMediaType.PASSWORD_CHANGE) + @Consumes(VndMediaType.PASSWORD_OVERWRITE) @StatusCodes({ @ResponseCode(code = 204, condition = "update success"), @ResponseCode(code = 400, condition = "Invalid body, e.g. the user type is not xml or the given oldPassword do not match the stored one"), @@ -130,12 +128,8 @@ public class UserResource { @ResponseCode(code = 500, condition = "internal server error") }) @TypeHint(TypeHint.NO_CONTENT.class) - public Response changePassword(@PathParam("id") String name, @Valid PasswordChangeDto passwordChangeDto) throws ConcurrentModificationException { - String currentUserName = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); - if (currentUserName.equals(name) && passwordChangeDto.getOldPassword() == null){ - throw new ChangePasswordNotAllowedException(ChangePasswordNotAllowedException.OLD_PASSWORD_REQUIRED); - } - return adapter.changePassword(name, user -> user.changePassword(passwordService.encryptPassword(passwordChangeDto.getNewPassword()))); + public Response overwritePassword(@PathParam("id") String name, @Valid PasswordOverwriteDto passwordOverwriteDto) { + userManager.overwritePassword(name, passwordService.encryptPassword(passwordOverwriteDto.getNewPassword())); + return Response.noContent().build(); } - } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java index 832829883b..eb49ff5f3d 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserToUserDtoMapper.java @@ -39,9 +39,9 @@ public abstract class UserToUserDtoMapper extends BaseMapper { } if (UserPermissions.modify(user).isPermitted()) { linksBuilder.single(link("update", resourceLinks.user().update(target.getName()))); - } - if (userManager.isTypeDefault(user)) { - linksBuilder.single(link("password", resourceLinks.user().passwordChange(target.getName()))); + if (userManager.isTypeDefault(user)) { + linksBuilder.single(link("password", resourceLinks.user().passwordChange(target.getName()))); + } } target.add(linksBuilder.build()); } diff --git a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java index 265e163c23..faa7208c66 100644 --- a/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java +++ b/scm-webapp/src/main/java/sonia/scm/security/DefaultAuthorizationCollector.java @@ -260,7 +260,7 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector builder.add(canReadOwnUser(user)); builder.add(getUserAutocompletePermission()); builder.add(getGroupAutocompletePermission()); - builder.add(getChangeOwnPasswordPermission()); + builder.add(getChangeOwnPasswordPermission(user)); permissions = builder.build(); } @@ -273,8 +273,8 @@ public class DefaultAuthorizationCollector implements AuthorizationCollector return GroupPermissions.autocomplete().asShiroString(); } - private String getChangeOwnPasswordPermission() { - return UserPermissions.changeOwnPassword().asShiroString(); + private String getChangeOwnPasswordPermission(User user) { + return UserPermissions.changePassword(user).asShiroString(); } private String getUserAutocompletePermission() { diff --git a/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java b/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java index 2e3981b2cf..4efaaa1d1c 100644 --- a/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java +++ b/scm-webapp/src/main/java/sonia/scm/user/DefaultUserManager.java @@ -33,12 +33,10 @@ package sonia.scm.user; -//~--- non-JDK imports -------------------------------------------------------- - import com.github.sdorra.ssp.PermissionActionCheck; -import com.github.sdorra.ssp.PermissionCheck; import com.google.inject.Inject; import com.google.inject.Singleton; +import org.apache.shiro.SecurityUtils; import org.slf4j.Logger; import org.slf4j.LoggerFactory; import sonia.scm.AlreadyExistsException; @@ -64,9 +62,6 @@ import java.util.Collection; import java.util.Collections; import java.util.Comparator; import java.util.List; -import java.util.function.Function; - -//~--- JDK imports ------------------------------------------------------------ /** * @@ -196,15 +191,10 @@ public class DefaultUserManager extends AbstractUserManager */ @Override public void modify(User user) { - modify(user,UserPermissions::modify); - } - - @Override - public void modify(User user, Function permissionChecker) { logger.info("modify user {} of type {}", user.getName(), user.getType()); managerDaoAdapter.modify( user, - permissionChecker, + UserPermissions::modify, notModified -> fireEvent(HandlerEventType.BEFORE_MODIFY, user, notModified), notModified -> fireEvent(HandlerEventType.MODIFY, user, notModified)); } @@ -408,6 +398,36 @@ public class DefaultUserManager extends AbstractUserManager //~--- methods -------------------------------------------------------------- + @Override + public void changePasswordForLoggedInUser(String oldPassword, String newPassword) { + User user = get((String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal()); + + if (!user.getPassword().equals(oldPassword)) { + throw new InvalidPasswordException(); + } + + user.setPassword(newPassword); + + managerDaoAdapter.modify( + user, + UserPermissions::changePassword, + notModified -> fireEvent(HandlerEventType.BEFORE_MODIFY, user, notModified), + notModified -> fireEvent(HandlerEventType.MODIFY, user, notModified)); + } + + @Override + public void overwritePassword(String userId, String newPassword) { + User user = get(userId); + if (user == null) { + throw new NotFoundException(); + } + if (!isTypeDefault(user)) { + throw new ChangePasswordNotAllowedException(user.getType()); + } + user.setPassword(newPassword); + this.modify(user); + } + /** * Method description * diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java index 9da9b1dd3d..143b469650 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java @@ -13,6 +13,7 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import sonia.scm.user.InvalidPasswordException; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; @@ -27,6 +28,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.ArgumentMatchers.any; import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; @@ -97,6 +99,7 @@ public class MeResourceTest { public void shouldEncryptPasswordBeforeChanging() throws Exception { String newPassword = "pwd123"; String encryptedNewPassword = "encrypted123"; + String encryptedOldPassword = "encryptedOld"; String oldPassword = "secret"; String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword); MockHttpRequest request = MockHttpRequest @@ -104,33 +107,32 @@ public class MeResourceTest { .contentType(VndMediaType.PASSWORD_CHANGE) .content(content.getBytes()); MockHttpResponse response = new MockHttpResponse(); + when(passwordService.encryptPassword(newPassword)).thenReturn(encryptedNewPassword); - when(passwordService.encryptPassword(oldPassword)).thenReturn("secret"); - ArgumentCaptor modifyUserCaptor = ArgumentCaptor.forClass(User.class); - doNothing().when(userManager).modify(modifyUserCaptor.capture(), any()); + when(passwordService.encryptPassword(oldPassword)).thenReturn(encryptedOldPassword); + + ArgumentCaptor encryptedOldPasswordCaptor = ArgumentCaptor.forClass(String.class); + ArgumentCaptor encryptedNewPasswordCaptor = ArgumentCaptor.forClass(String.class); + doNothing().when(userManager).changePasswordForLoggedInUser(encryptedOldPasswordCaptor.capture(), encryptedNewPasswordCaptor.capture()); dispatcher.invoke(request, response); assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); - verify(userManager).modify(any(), any()); - User updatedUser = modifyUserCaptor.getValue(); - assertEquals(encryptedNewPassword, updatedUser.getPassword()); + assertEquals(encryptedNewPassword, encryptedNewPasswordCaptor.getValue()); + assertEquals(encryptedOldPassword, encryptedOldPasswordCaptor.getValue()); } @Test @SubjectAware(username = "trillian", password = "secret") - public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception { + public void shouldGet400OnMissingOldPassword() throws Exception { originalUser.setType("not an xml type"); String newPassword = "pwd123"; - String oldPassword = "secret"; - String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword); + String content = String.format("{ \"newPassword\": \"%s\" }", newPassword); MockHttpRequest request = MockHttpRequest .put("/" + MeResource.ME_PATH_V2 + "password") .contentType(VndMediaType.PASSWORD_CHANGE) .content(content.getBytes()); MockHttpResponse response = new MockHttpResponse(); - when(passwordService.encryptPassword(eq(newPassword))).thenReturn("encrypted123"); - when(passwordService.encryptPassword(eq(oldPassword))).thenReturn("secret"); dispatcher.invoke(request, response); @@ -139,17 +141,34 @@ public class MeResourceTest { @Test @SubjectAware(username = "trillian", password = "secret") - public void shouldGet400OnChangePasswordIfOldPasswordDoesNotMatchOriginalPassword() throws Exception { + public void shouldGet400OnMissingEmptyPassword() throws Exception { String newPassword = "pwd123"; - String oldPassword = "notEncriptedSecret"; + String oldPassword = ""; String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword); MockHttpRequest request = MockHttpRequest .put("/" + MeResource.ME_PATH_V2 + "password") .contentType(VndMediaType.PASSWORD_CHANGE) .content(content.getBytes()); MockHttpResponse response = new MockHttpResponse(); - when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123"); - when(passwordService.encryptPassword(eq(oldPassword))).thenReturn("differentThanSecret"); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus()); + } + + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldMapExceptionFromManager() throws Exception { + String newPassword = "pwd123"; + String oldPassword = "secret"; + String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword); + MockHttpRequest request = MockHttpRequest + .put("/" + MeResource.ME_PATH_V2 + "password") + .contentType(VndMediaType.PASSWORD_CHANGE) + .content(content.getBytes()); + MockHttpResponse response = new MockHttpResponse(); + + doThrow(InvalidPasswordException.class).when(userManager).changePasswordForLoggedInUser(any(), any()); dispatcher.invoke(request, response); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java index bced4e6545..b363b66bfb 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserRootResourceTest.java @@ -14,7 +14,9 @@ import org.junit.Test; import org.mockito.ArgumentCaptor; import org.mockito.InjectMocks; import org.mockito.Mock; +import sonia.scm.NotFoundException; import sonia.scm.PageResult; +import sonia.scm.user.ChangePasswordNotAllowedException; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; @@ -31,6 +33,7 @@ import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; import static org.mockito.Matchers.eq; import static org.mockito.Mockito.doNothing; +import static org.mockito.Mockito.doThrow; import static org.mockito.Mockito.never; import static org.mockito.Mockito.verify; import static org.mockito.Mockito.when; @@ -142,7 +145,7 @@ public class UserRootResourceTest { String content = String.format("{\"newPassword\": \"%s\"}", newPassword); MockHttpRequest request = MockHttpRequest .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") - .contentType(VndMediaType.PASSWORD_CHANGE) + .contentType(VndMediaType.PASSWORD_OVERWRITE) .content(content.getBytes()); MockHttpResponse response = new MockHttpResponse(); when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123"); @@ -150,26 +153,61 @@ public class UserRootResourceTest { dispatcher.invoke(request, response); assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); - verify(userManager).modify(any(), any()); - User updatedUser = userCaptor.getValue(); - assertEquals("encrypted123", updatedUser.getPassword()); + verify(userManager).overwritePassword("Neo", "encrypted123"); } @Test - public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception { + public void shouldGet400OnOverwritePasswordWhenManagerThrowsNotAllowed() throws Exception { originalUser.setType("not an xml type"); String newPassword = "pwd123"; String content = String.format("{\"newPassword\": \"%s\"}", newPassword); MockHttpRequest request = MockHttpRequest .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") - .contentType(VndMediaType.PASSWORD_CHANGE) + .contentType(VndMediaType.PASSWORD_OVERWRITE) + .content(content.getBytes()); + MockHttpResponse response = new MockHttpResponse(); + + doThrow(ChangePasswordNotAllowedException.class).when(userManager).overwritePassword(any(), any()); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus()); + } + + @Test + public void shouldGet404OnOverwritePasswordWhenNotFound() throws Exception { + originalUser.setType("not an xml type"); + String newPassword = "pwd123"; + String content = String.format("{\"newPassword\": \"%s\"}", newPassword); + MockHttpRequest request = MockHttpRequest + .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") + .contentType(VndMediaType.PASSWORD_OVERWRITE) + .content(content.getBytes()); + MockHttpResponse response = new MockHttpResponse(); + + doThrow(NotFoundException.class).when(userManager).overwritePassword(any(), any()); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_NOT_FOUND, response.getStatus()); + } + + @Test + public void shouldEncryptPasswordOnOverwritePassword() throws Exception { + originalUser.setType("not an xml type"); + String newPassword = "pwd123"; + String content = String.format("{\"newPassword\": \"%s\"}", newPassword); + MockHttpRequest request = MockHttpRequest + .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") + .contentType(VndMediaType.PASSWORD_OVERWRITE) .content(content.getBytes()); MockHttpResponse response = new MockHttpResponse(); when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123"); dispatcher.invoke(request, response); - assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus()); + assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); + verify(userManager).overwritePassword("Neo", "encrypted123"); } @Test diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserToUserDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserToUserDtoMapperTest.java index 7570a3f162..dfff933f19 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserToUserDtoMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserToUserDtoMapperTest.java @@ -65,7 +65,7 @@ public class UserToUserDtoMapperTest { } @Test - public void shouldGetPasswordLinkOnlyForDefaultUserType() { + public void shouldGetPasswordLinkForAdmin() { User user = createDefaultUser(); when(subject.isPermitted("user:modify:abc")).thenReturn(true); when(userManager.isTypeDefault(eq(user))).thenReturn(true); @@ -73,14 +73,15 @@ public class UserToUserDtoMapperTest { UserDto userDto = mapper.map(user); assertEquals("expected password link with modify permission", expectedBaseUri.resolve("abc/password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); + } - when(subject.isPermitted("user:modify:abc")).thenReturn(false); - userDto = mapper.map(user); - assertEquals("expected password link on mission modify permission", expectedBaseUri.resolve("abc/password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); - + @Test + public void shouldGetPasswordLinkOnlyForDefaultUserType() { + User user = createDefaultUser(); + when(subject.isPermitted("user:modify:abc")).thenReturn(true); when(userManager.isTypeDefault(eq(user))).thenReturn(false); - userDto = mapper.map(user); + UserDto userDto = mapper.map(user); assertFalse("expected no password link", userDto.getLinks().getLinkBy("password").isPresent()); } diff --git a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java index a85d03991d..2f455469dd 100644 --- a/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java +++ b/scm-webapp/src/test/java/sonia/scm/security/DefaultAuthorizationCollectorTest.java @@ -159,7 +159,7 @@ public class DefaultAuthorizationCollectorTest { AuthorizationInfo authInfo = collector.collect(); assertThat(authInfo.getRoles(), Matchers.contains(Role.USER)); assertThat(authInfo.getStringPermissions(), hasSize(4)); - assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changeOwnPassword", "user:read:trillian")); + assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changePassword:trillian", "user:read:trillian")); assertThat(authInfo.getObjectPermissions(), nullValue()); } @@ -204,7 +204,7 @@ public class DefaultAuthorizationCollectorTest { AuthorizationInfo authInfo = collector.collect(); assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER)); assertThat(authInfo.getObjectPermissions(), nullValue()); - assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changeOwnPassword", "repository:read,pull:one", "repository:read,pull,push:two", "user:read:trillian")); + assertThat(authInfo.getStringPermissions(), containsInAnyOrder("user:autocomplete", "group:autocomplete", "user:changePassword:trillian", "repository:read,pull:one", "repository:read,pull,push:two", "user:read:trillian")); } /** @@ -225,7 +225,7 @@ public class DefaultAuthorizationCollectorTest { AuthorizationInfo authInfo = collector.collect(); assertThat(authInfo.getRoles(), Matchers.containsInAnyOrder(Role.USER)); assertThat(authInfo.getObjectPermissions(), nullValue()); - assertThat(authInfo.getStringPermissions(), containsInAnyOrder("one:one", "two:two", "user:read:trillian", "user:autocomplete", "group:autocomplete", "user:changeOwnPassword")); + assertThat(authInfo.getStringPermissions(), containsInAnyOrder("one:one", "two:two", "user:read:trillian", "user:autocomplete", "group:autocomplete", "user:changePassword:trillian")); } private void authenticate(User user, String group, String... groups) { diff --git a/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java b/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java index eb892c7e89..df32528e5e 100644 --- a/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java +++ b/scm-webapp/src/test/java/sonia/scm/user/DefaultUserManagerTest.java @@ -39,9 +39,13 @@ import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; import com.google.common.collect.Lists; +import org.assertj.core.api.Assertions; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; +import org.mockito.ArgumentCaptor; +import sonia.scm.NotFoundException; import sonia.scm.store.JAXBConfigurationStoreFactory; import sonia.scm.user.xml.XmlUserDAO; import sonia.scm.util.MockUtil; @@ -70,6 +74,10 @@ public class DefaultUserManagerTest extends UserManagerTestBase @Rule public ShiroRule shiro = new ShiroRule(); + + private UserDAO userDAO = mock(UserDAO.class); + private User trillian; + /** * Method description * @@ -82,6 +90,16 @@ public class DefaultUserManagerTest extends UserManagerTestBase return new DefaultUserManager(createXmlUserDAO()); } + @Before + public void initDao() { + trillian = UserTestData.createTrillian(); + trillian.setPassword("oldEncrypted"); + + userDAO = mock(UserDAO.class); + when(userDAO.getType()).thenReturn("xml"); + when(userDAO.get("trillian")).thenReturn(trillian); + } + /** * Method description * @@ -89,7 +107,6 @@ public class DefaultUserManagerTest extends UserManagerTestBase @Test public void testDefaultAccountAfterFristStart() { - UserDAO userDAO = mock(UserDAO.class); List users = Lists.newArrayList(new User("tuser")); when(userDAO.getAll()).thenReturn(users); @@ -108,8 +125,6 @@ public class DefaultUserManagerTest extends UserManagerTestBase @SuppressWarnings("unchecked") public void testDefaultAccountCreation() { - UserDAO userDAO = mock(UserDAO.class); - when(userDAO.getAll()).thenReturn(Collections.EMPTY_LIST); UserManager userManager = new DefaultUserManager(userDAO); @@ -118,6 +133,55 @@ public class DefaultUserManagerTest extends UserManagerTestBase verify(userDAO, times(2)).add(any(User.class)); } + @Test(expected = InvalidPasswordException.class) + public void shouldFailChangePasswordForWrongOldPassword() { + UserManager userManager = new DefaultUserManager(userDAO); + + userManager.changePasswordForLoggedInUser("wrongPassword", "---"); + } + + @Test + public void shouldSucceedChangePassword() { + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); + + doNothing().when(userDAO).modify(userCaptor.capture()); + + UserManager userManager = new DefaultUserManager(userDAO); + + userManager.changePasswordForLoggedInUser("oldEncrypted", "newEncrypted"); + + Assertions.assertThat(userCaptor.getValue().getPassword()).isEqualTo("newEncrypted"); + } + + @Test(expected = ChangePasswordNotAllowedException.class) + public void shouldFailOverwritePasswordForWrongType() { + trillian.setType("wrongType"); + + UserManager userManager = new DefaultUserManager(userDAO); + + userManager.overwritePassword("trillian", "---"); + } + + @Test(expected = NotFoundException.class) + public void shouldFailOverwritePasswordForMissingUser() { + UserManager userManager = new DefaultUserManager(userDAO); + + userManager.overwritePassword("notExisting", "---"); + } + + @Test + public void shouldSucceedOverwritePassword() { + ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); + + doNothing().when(userDAO).modify(userCaptor.capture()); + + UserManager userManager = new DefaultUserManager(userDAO); + + userManager.overwritePassword("trillian", "newEncrypted"); + + Assertions.assertThat(userCaptor.getValue().getPassword()).isEqualTo("newEncrypted"); + } + //~--- methods -------------------------------------------------------------- private XmlUserDAO createXmlUserDAO() {