From d426445b4f89d50d40f5669215048d1bc6c11dbb Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Mon, 17 Sep 2018 09:59:19 +0200 Subject: [PATCH] change password endpoint --- .../src/main/java/sonia/scm/user/User.java | 4 ++ .../main/java/sonia/scm/web/VndMediaType.java | 1 + scm-ui/src/users/modules/users.test.js | 4 +- .../ChangePasswordNotAllowedException.java | 9 +++ ...angePasswordNotAllowedExceptionMapper.java | 15 +++++ .../resources/IdResourceManagerAdapter.java | 10 +++ .../scm/api/v2/resources/MeResource.java | 19 ++++++ .../api/v2/resources/PasswordChangeDto.java | 25 ++++++++ .../scm/api/v2/resources/ResourceLinks.java | 4 ++ .../SingleResourceManagerAdapter.java | 6 ++ .../v2/resources/UserCollectionResource.java | 9 +-- .../api/v2/resources/UserDtoToUserMapper.java | 32 ++++------ .../scm/api/v2/resources/UserResource.java | 51 ++++++++++++++-- .../api/v2/resources/UserToUserDtoMapper.java | 19 ++++-- .../scm/api/v2/resources/DispatcherMock.java | 1 + .../scm/api/v2/resources/MeResourceTest.java | 2 +- .../v2/resources/UserDtoToUserMapperTest.java | 13 +--- .../v2/resources/UserRootResourceTest.java | 61 +++++++++++++++---- .../v2/resources/UserToUserDtoMapperTest.java | 50 ++++++++++----- 19 files changed, 263 insertions(+), 72 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedExceptionMapper.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordChangeDto.java 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 778e573c14..97c6bb16c7 100644 --- a/scm-core/src/main/java/sonia/scm/user/User.java +++ b/scm-core/src/main/java/sonia/scm/user/User.java @@ -274,6 +274,10 @@ User extends BasicPropertiesAware implements Principal, ModelObject, PermissionO //J+ } + public User changePassword(String password){ + setPassword(password); + return this; + } //~--- get methods ---------------------------------------------------------- /** 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 ddcce9f4e8..5d8dc2a84f 100644 --- a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java +++ b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java @@ -34,6 +34,7 @@ public class VndMediaType { public static final String REPOSITORY_TYPE = PREFIX + "repositoryType" + SUFFIX; public static final String UI_PLUGIN = PREFIX + "uiPlugin" + SUFFIX; public static final String UI_PLUGIN_COLLECTION = PREFIX + "uiPluginCollection" + SUFFIX; + public static final String PASSWORD_CHANGE = PREFIX + "passwordChange" + SUFFIX; public static final String ME = PREFIX + "me" + SUFFIX; public static final String SOURCE = PREFIX + "source" + SUFFIX; diff --git a/scm-ui/src/users/modules/users.test.js b/scm-ui/src/users/modules/users.test.js index 895e28a7b0..c8c56f2ef5 100644 --- a/scm-ui/src/users/modules/users.test.js +++ b/scm-ui/src/users/modules/users.test.js @@ -56,7 +56,7 @@ const userZaphod = { displayName: "Z. Beeblebrox", mail: "president@heartofgold.universe", name: "zaphod", - password: "__dummypassword__", + password: "", type: "xml", properties: {}, _links: { @@ -79,7 +79,7 @@ const userFord = { displayName: "F. Prefect", mail: "ford@prefect.universe", name: "ford", - password: "__dummypassword__", + password: "", type: "xml", properties: {}, _links: { diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java new file mode 100644 index 0000000000..755cd709cc --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java @@ -0,0 +1,9 @@ +package sonia.scm.api.v2.resources; + +public class ChangePasswordNotAllowedException extends RuntimeException { + + public ChangePasswordNotAllowedException(String message) { + super(message); + } + +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedExceptionMapper.java new file mode 100644 index 0000000000..e83be3e386 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedExceptionMapper.java @@ -0,0 +1,15 @@ +package sonia.scm.api.v2.resources; + +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +@Provider +public class ChangePasswordNotAllowedExceptionMapper implements ExceptionMapper { + @Override + public Response toResponse(ChangePasswordNotAllowedException exception) { + return Response.status(Response.Status.BAD_REQUEST) + .entity(exception.getMessage()) + .build(); + } +} 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 27abed637e..9b890eb174 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 @@ -10,6 +10,7 @@ import sonia.scm.PageResult; import javax.ws.rs.core.Response; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -37,6 +38,15 @@ class IdResourceManagerAdapter applyChanges, Consumer checker) throws NotFoundException, ConcurrentModificationException { + return singleAdapter.update( + loadBy(id), + applyChanges, + idStaysTheSame(id), + checker + ); + } + public Response update(String id, Function applyChanges) throws NotFoundException, 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 f016f4604c..2967c0be4e 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 @@ -10,7 +10,9 @@ import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; import javax.inject.Inject; +import javax.ws.rs.Consumes; import javax.ws.rs.GET; +import javax.ws.rs.PUT; import javax.ws.rs.Path; import javax.ws.rs.Produces; import javax.ws.rs.core.Context; @@ -52,4 +54,21 @@ public class MeResource { String id = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); return adapter.get(id, userToDtoMapper::map); } + + /** + * Change password of the current user + */ + @PUT + @Path("password") + @StatusCodes({ + @ResponseCode(code = 204, condition = "update success"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 500, condition = "internal server error") + }) + @TypeHint(TypeHint.NO_CONTENT.class) + @Consumes(VndMediaType.PASSWORD_CHANGE) + public Response changePassword(PasswordChangeDto passwordChange) throws NotFoundException { + String id = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); + return adapter.get(id, userToDtoMapper::map); + } } 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 new file mode 100644 index 0000000000..2db96ccdf1 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/PasswordChangeDto.java @@ -0,0 +1,25 @@ +package sonia.scm.api.v2.resources; + +import de.otto.edison.hal.HalRepresentation; +import de.otto.edison.hal.Links; +import lombok.Getter; +import lombok.Setter; +import lombok.ToString; +import org.hibernate.validator.constraints.NotEmpty; + +@Getter +@Setter +@ToString +public class PasswordChangeDto extends HalRepresentation { + + private String oldPassword; + + @NotEmpty + private String newPassword; + + @Override + @SuppressWarnings("squid:S1185") // We want to have this method available in this package + protected HalRepresentation add(Links links) { + return super.add(links); + } +} 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 3d61083033..9905fb8985 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 @@ -86,6 +86,10 @@ class ResourceLinks { String update(String name) { return userLinkBuilder.method("getUserResource").parameters(name).method("update").parameters().href(); } + + public String passwordChange(String name) { + return userLinkBuilder.method("getUserResource").parameters(name).method("changePassword").parameters().href(); + } } UserCollectionLinks userCollection() { 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 fa50cdcc87..f2bc93d47e 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 @@ -11,6 +11,7 @@ import javax.ws.rs.core.GenericEntity; import javax.ws.rs.core.Response; import java.util.Collection; import java.util.Optional; +import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; import java.util.function.Supplier; @@ -53,6 +54,11 @@ class SingleResourceManagerAdapter> reader, Function applyChanges, Predicate hasSameKey, Consumer checker) throws NotFoundException, ConcurrentModificationException { + MODEL_OBJECT existingModelObject = reader.get().orElseThrow(NotFoundException::new); + checker.accept(existingModelObject); + return update(reader,applyChanges,hasSameKey); + } /** * Update the model object for the given id according to the given function and returns a corresponding http response. diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java index 81c3a66c6d..9b39888104 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserCollectionResource.java @@ -5,6 +5,7 @@ import com.webcohesion.enunciate.metadata.rs.ResponseHeader; import com.webcohesion.enunciate.metadata.rs.ResponseHeaders; import com.webcohesion.enunciate.metadata.rs.StatusCodes; import com.webcohesion.enunciate.metadata.rs.TypeHint; +import org.apache.shiro.authc.credential.PasswordService; import sonia.scm.AlreadyExistsException; import sonia.scm.user.User; import sonia.scm.user.UserManager; @@ -29,14 +30,16 @@ public class UserCollectionResource { private final ResourceLinks resourceLinks; private final IdResourceManagerAdapter adapter; + private final PasswordService passwordService; @Inject public UserCollectionResource(UserManager manager, UserDtoToUserMapper dtoToUserMapper, - UserCollectionToDtoMapper userCollectionToDtoMapper, ResourceLinks resourceLinks) { + UserCollectionToDtoMapper userCollectionToDtoMapper, ResourceLinks resourceLinks, PasswordService passwordService) { this.dtoToUserMapper = dtoToUserMapper; this.userCollectionToDtoMapper = userCollectionToDtoMapper; this.adapter = new IdResourceManagerAdapter<>(manager, User.class); this.resourceLinks = resourceLinks; + this.passwordService = passwordService; } /** @@ -89,8 +92,6 @@ public class UserCollectionResource { @TypeHint(TypeHint.NO_CONTENT.class) @ResponseHeaders(@ResponseHeader(name = "Location", description = "uri to the created user")) public Response create(@Valid UserDto userDto) throws AlreadyExistsException { - return adapter.create(userDto, - () -> dtoToUserMapper.map(userDto, ""), - user -> resourceLinks.user().self(user.getName())); + return adapter.create(userDto, () -> dtoToUserMapper.map(userDto, passwordService.encryptPassword(userDto.getPassword())), user -> resourceLinks.user().self(user.getName())); } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDtoToUserMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDtoToUserMapper.java index 6d266bea5a..1ea07e4be7 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDtoToUserMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDtoToUserMapper.java @@ -1,37 +1,31 @@ package sonia.scm.api.v2.resources; -import org.apache.shiro.authc.credential.PasswordService; import org.mapstruct.Context; import org.mapstruct.Mapper; import org.mapstruct.Mapping; import org.mapstruct.Named; import sonia.scm.user.User; -import javax.inject.Inject; - -import java.time.Instant; - -import static sonia.scm.api.rest.resources.UserResource.DUMMY_PASSWORT; - // Mapstruct does not support parameterized (i.e. non-default) constructors. Thus, we need to use field injection. @SuppressWarnings("squid:S3306") @Mapper public abstract class UserDtoToUserMapper extends BaseDtoMapper { - @Inject - private PasswordService passwordService; - - @Mapping(source = "password", target = "password", qualifiedByName = "encrypt") + @Mapping(source = "password", target = "password", qualifiedByName = "getUsedPassword") @Mapping(target = "creationDate", ignore = true) - public abstract User map(UserDto userDto, @Context String originalPassword); + public abstract User map(UserDto userDto, @Context String usedPassword); - @Named("encrypt") - String encrypt(String password, @Context String originalPassword) { - if (DUMMY_PASSWORT.equals(password)) { - return originalPassword; - } else { - return passwordService.encryptPassword(password); - } + /** + * depends on the use case the right password will be mapped. + * eg. for update user action the password will be set to the original password + * for create user and change password actions the password is the user input + * + * @param usedPassword the password to be mapped + * @return the password to be mapped + */ + @Named("getUsedPassword") + String getUsedPassword(String password, @Context String usedPassword) { + return usedPassword; } } 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 151abfa20f..126d738e1e 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,6 +3,7 @@ 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.authc.credential.PasswordService; import sonia.scm.ConcurrentModificationException; import sonia.scm.NotFoundException; import sonia.scm.user.User; @@ -19,6 +20,8 @@ import javax.ws.rs.Path; import javax.ws.rs.PathParam; import javax.ws.rs.Produces; import javax.ws.rs.core.Response; +import java.text.MessageFormat; +import java.util.function.Consumer; public class UserResource { @@ -26,12 +29,16 @@ public class UserResource { private final UserToUserDtoMapper userToDtoMapper; private final IdResourceManagerAdapter adapter; + private final UserManager userManager; + private final PasswordService passwordService; @Inject - public UserResource(UserDtoToUserMapper dtoToUserMapper, UserToUserDtoMapper userToDtoMapper, UserManager manager) { + public UserResource(UserDtoToUserMapper dtoToUserMapper, UserToUserDtoMapper userToDtoMapper, UserManager manager, PasswordService passwordService) { this.dtoToUserMapper = dtoToUserMapper; this.userToDtoMapper = userToDtoMapper; this.adapter = new IdResourceManagerAdapter<>(manager, User.class); + this.userManager = manager; + this.passwordService = passwordService; } /** @@ -40,7 +47,6 @@ public class UserResource { * Note: This method requires "user" privilege. * * @param id the id/name of the user - * */ @GET @Path("") @@ -63,7 +69,6 @@ public class UserResource { * Note: This method requires "user" privilege. * * @param name the name of the user to delete. - * */ @DELETE @Path("") @@ -80,10 +85,11 @@ public class UserResource { /** * Modifies the given user. + * The given Password in the payload will be ignored. To Change Password use the changePassword endpoint * * Note: This method requires "user" privilege. * - * @param name name of the user to be modified + * @param name name of the user to be modified * @param userDto user object to modify */ @PUT @@ -101,4 +107,41 @@ public class UserResource { public Response update(@PathParam("id") String name, @Valid UserDto userDto) throws NotFoundException, ConcurrentModificationException { return adapter.update(name, existing -> dtoToUserMapper.map(userDto, existing.getPassword())); } + + /** + * This Endpoint is for Admin user to modify a user password. + * The oldPassword property of the DTO is not needed here. it will be ignored. + * The oldPassword property is needed in the MeResources when the actual user change the own password. + * + * Note: This method requires "user:modify" privilege. + * @param name name of the user to be modified + * @param passwordChangeDto change password object to modify password. the old password is here not required + */ + @PUT + @Path("password") + @Consumes(VndMediaType.PASSWORD_CHANGE) + @StatusCodes({ + @ResponseCode(code = 204, condition = "update success"), + @ResponseCode(code = 400, condition = "Invalid body, e.g. illegal change of id/user name"), + @ResponseCode(code = 401, condition = "not authenticated / invalid credentials"), + @ResponseCode(code = 403, condition = "not authorized, the current user does not have the \"user\" privilege"), + @ResponseCode(code = 404, condition = "not found, no user with the specified id/name available"), + @ResponseCode(code = 500, condition = "internal server error") + }) + @TypeHint(TypeHint.NO_CONTENT.class) + public Response changePassword(@PathParam("id") String name, @Valid PasswordChangeDto passwordChangeDto) throws NotFoundException, ConcurrentModificationException { + return adapter.update(name, user -> user.changePassword(passwordService.encryptPassword(passwordChangeDto.getNewPassword())), getUserTypeChecker()); + } + + + /** + * Only account of the default type "xml" can change their password + */ + private Consumer getUserTypeChecker() { + return user -> { + if (!userManager.getDefaultType().equals(user.getType())) { + throw new ChangePasswordNotAllowedException(MessageFormat.format("It is not possible to change password for User of type {0}", user.getType())); + } + }; + } } 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 00aba5a700..75566c1c3b 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 @@ -4,9 +4,10 @@ import com.google.common.annotations.VisibleForTesting; import de.otto.edison.hal.Links; import org.mapstruct.AfterMapping; import org.mapstruct.Mapper; +import org.mapstruct.Mapping; import org.mapstruct.MappingTarget; -import sonia.scm.api.rest.resources.UserResource; import sonia.scm.user.User; +import sonia.scm.user.UserManager; import sonia.scm.user.UserPermissions; import javax.inject.Inject; @@ -19,6 +20,14 @@ import static de.otto.edison.hal.Links.linkingTo; @Mapper public abstract class UserToUserDtoMapper extends BaseMapper { + @Inject + private UserManager userManager; + + @Override + @Mapping(target = "attributes", ignore = true) + @Mapping(target = "password", ignore = true) + public abstract UserDto map(User modelObject); + @Inject private ResourceLinks resourceLinks; @@ -27,11 +36,6 @@ public abstract class UserToUserDtoMapper extends BaseMapper { this.resourceLinks = resourceLinks; } - @AfterMapping - void removePassword(@MappingTarget UserDto target) { - target.setPassword(UserResource.DUMMY_PASSWORT); - } - @AfterMapping void appendLinks(User user, @MappingTarget UserDto target) { Links.Builder linksBuilder = linkingTo().self(resourceLinks.user().self(target.getName())); @@ -41,6 +45,9 @@ public abstract class UserToUserDtoMapper extends BaseMapper { if (UserPermissions.modify(user).isPermitted()) { linksBuilder.single(link("update", resourceLinks.user().update(target.getName()))); } + if (userManager.getDefaultType().equals(user.getType())) { + linksBuilder.single(link("password", resourceLinks.user().passwordChange(target.getName()))); + } target.add(linksBuilder.build()); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DispatcherMock.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DispatcherMock.java index 4d4ed435df..d0705e2b2d 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DispatcherMock.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/DispatcherMock.java @@ -14,6 +14,7 @@ public class DispatcherMock { dispatcher.getProviderFactory().registerProvider(AlreadyExistsExceptionMapper.class); dispatcher.getProviderFactory().registerProvider(AuthorizationExceptionMapper.class); dispatcher.getProviderFactory().registerProvider(ConcurrentModificationExceptionMapper.class); + dispatcher.getProviderFactory().registerProvider(ChangePasswordNotAllowedExceptionMapper.class); return dispatcher; } } 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 09dd545eb5..5bfaad66f8 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 @@ -77,7 +77,7 @@ public class MeResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"trillian\"")); - assertTrue(response.getContentAsString().contains("\"password\":\"__dummypassword__\"")); + assertTrue(response.getContentAsString().contains("\"password\":null")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/trillian\"}")); assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/trillian\"}")); } diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserDtoToUserMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserDtoToUserMapperTest.java index f97fd91c3b..19f247b3b2 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserDtoToUserMapperTest.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/UserDtoToUserMapperTest.java @@ -23,18 +23,9 @@ public class UserDtoToUserMapperTest { @Test public void shouldMapFields() { UserDto dto = createDefaultDto(); - User user = mapper.map(dto, "original password"); + User user = mapper.map(dto, "used password"); assertEquals("abc" , user.getName()); - } - - @Test - public void shouldEncodePassword() { - when(passwordService.encryptPassword("unencrypted")).thenReturn("encrypted"); - - UserDto dto = createDefaultDto(); - dto.setPassword("unencrypted"); - User user = mapper.map(dto, "original password"); - assertEquals("encrypted" , user.getPassword()); + assertEquals("used password" , user.getPassword()); } @Before 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 5004eb7665..d75fd606ec 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 @@ -22,6 +22,7 @@ import javax.servlet.http.HttpServletResponse; import java.net.URI; import java.net.URISyntaxException; import java.net.URL; +import java.text.MessageFormat; import static java.util.Collections.singletonList; import static org.junit.Assert.assertEquals; @@ -60,21 +61,23 @@ public class UserRootResourceTest { private UserToUserDtoMapperImpl userToDtoMapper; private ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); + private User originalUser; @Before public void prepareEnvironment() throws Exception { initMocks(this); - User dummyUser = createDummyUser("Neo"); + originalUser = createDummyUser("Neo"); when(userManager.create(userCaptor.capture())).thenAnswer(invocation -> invocation.getArguments()[0]); doNothing().when(userManager).modify(userCaptor.capture()); doNothing().when(userManager).delete(userCaptor.capture()); + when(userManager.getDefaultType()).thenReturn("xml"); UserCollectionToDtoMapper userCollectionToDtoMapper = new UserCollectionToDtoMapper(userToDtoMapper, resourceLinks); UserCollectionResource userCollectionResource = new UserCollectionResource(userManager, dtoToUserMapper, - userCollectionToDtoMapper, resourceLinks); - UserResource userResource = new UserResource(dtoToUserMapper, userToDtoMapper, userManager); + userCollectionToDtoMapper, resourceLinks, passwordService); + UserResource userResource = new UserResource(dtoToUserMapper, userToDtoMapper, userManager, passwordService); UserRootResource userRootResource = new UserRootResource(MockProvider.of(userCollectionResource), - MockProvider.of(userResource)); + MockProvider.of(userResource)); dispatcher = createDispatcher(userRootResource); } @@ -88,7 +91,7 @@ public class UserRootResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"Neo\"")); - assertTrue(response.getContentAsString().contains("\"password\":\"__dummypassword__\"")); + assertTrue(response.getContentAsString().contains("\"password\":null")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/Neo\"}")); assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/Neo\"}")); } @@ -103,13 +106,49 @@ public class UserRootResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"Neo\"")); - assertTrue(response.getContentAsString().contains("\"password\":\"__dummypassword__\"")); + assertTrue(response.getContentAsString().contains("\"password\":null")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/Neo\"}")); assertFalse(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/Neo\"}")); } @Test - public void shouldCreateNewUserWithEncryptedPassword() throws Exception { + public void shouldEncryptPasswordBeforeChanging() throws Exception { + String newPassword = "pwd123"; + String content = MessageFormat.format("'{'\"newPassword\": \"{0}\"'}'", newPassword); + MockHttpRequest request = MockHttpRequest + .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") + .contentType(VndMediaType.PASSWORD_CHANGE) + .content(content.getBytes()); + MockHttpResponse response = new MockHttpResponse(); + when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123"); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); + verify(userManager).modify(any(User.class)); + User updatedUser = userCaptor.getValue(); + assertEquals("encrypted123", updatedUser.getPassword()); + } + + @Test + public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception { + originalUser.setType("not an xml type"); + String newPassword = "pwd123"; + String content = MessageFormat.format("'{'\"newPassword\": \"{0}\"'}'", newPassword); + MockHttpRequest request = MockHttpRequest + .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") + .contentType(VndMediaType.PASSWORD_CHANGE) + .content(content.getBytes()); + MockHttpResponse response = new MockHttpResponse(); + when(passwordService.encryptPassword(newPassword)).thenReturn("encrypted123"); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus()); + } + + @Test + public void shouldEncryptPasswordBeforeCreatingUser() throws Exception { URL url = Resources.getResource("sonia/scm/api/v2/user-test-create.json"); byte[] userJson = Resources.toByteArray(url); @@ -129,7 +168,7 @@ public class UserRootResourceTest { } @Test - public void shouldUpdateChangedUserWithEncryptedPassword() throws Exception { + public void shouldIgnoreGivenPasswordOnUpdatingUser() throws Exception { URL url = Resources.getResource("sonia/scm/api/v2/user-test-update.json"); byte[] userJson = Resources.toByteArray(url); @@ -138,14 +177,13 @@ public class UserRootResourceTest { .contentType(VndMediaType.USER) .content(userJson); MockHttpResponse response = new MockHttpResponse(); - when(passwordService.encryptPassword("pwd123")).thenReturn("encrypted123"); dispatcher.invoke(request, response); assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); verify(userManager).modify(any(User.class)); User updatedUser = userCaptor.getValue(); - assertEquals("encrypted123", updatedUser.getPassword()); + assertEquals(originalUser.getPassword(), updatedUser.getPassword()); } @Test @@ -153,7 +191,7 @@ public class UserRootResourceTest { MockHttpRequest request = MockHttpRequest .post("/" + UserRootResource.USERS_PATH_V2) .contentType(VndMediaType.USER) - .content(new byte[] {}); + .content(new byte[]{}); MockHttpResponse response = new MockHttpResponse(); when(passwordService.encryptPassword("pwd123")).thenReturn("encrypted123"); @@ -264,6 +302,7 @@ public class UserRootResourceTest { private User createDummyUser(String name) { User user = new User(); user.setName(name); + user.setType("xml"); user.setPassword("redpill"); user.setCreationDate(System.currentTimeMillis()); when(userManager.get(name)).thenReturn(user); 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 330dd2a89e..4c9388cd18 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 @@ -8,12 +8,14 @@ import org.junit.After; import org.junit.Before; import org.junit.Test; import org.mockito.InjectMocks; -import sonia.scm.api.rest.resources.UserResource; +import org.mockito.Mock; import sonia.scm.user.User; +import sonia.scm.user.UserManager; import java.net.URI; import java.time.Instant; +import static org.assertj.core.api.Assertions.assertThat; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.mockito.Mockito.mock; @@ -26,6 +28,9 @@ public class UserToUserDtoMapperTest { @SuppressWarnings("unused") // Is injected private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri); + @Mock + private UserManager userManager; + @InjectMocks private UserToUserDtoMapperImpl mapper; @@ -37,6 +42,7 @@ public class UserToUserDtoMapperTest { @Before public void init() { initMocks(this); + when(userManager.getDefaultType()).thenReturn("xml"); expectedBaseUri = baseUri.resolve(UserRootResource.USERS_PATH_V2 + "/"); subjectThreadState.bind(); ThreadContext.bind(subject); @@ -53,11 +59,37 @@ public class UserToUserDtoMapperTest { when(subject.isPermitted("user:modify:abc")).thenReturn(true); UserDto userDto = mapper.map(user); - - assertEquals("expected self link", expectedBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("self").get().getHref()); + assertEquals("expected self link", expectedBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("self").get().getHref()); assertEquals("expected update link", expectedBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("update").get().getHref()); } + @Test + public void shouldGetPasswordLinkOnlyForDefaultUserType() { + User user = createDefaultUser(); + when(subject.isPermitted("user:modify:abc")).thenReturn(true); + user.setType("xml"); + + UserDto userDto = mapper.map(user); + + assertEquals("expected password link", expectedBaseUri.resolve("abc/password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); + user.setType("db"); + + userDto = mapper.map(user); + + assertFalse("expected no password link", userDto.getLinks().getLinkBy("password").isPresent()); + } + + @Test + public void shouldGetEmptyPasswordProperty() { + User user = createDefaultUser(); + user.setPassword("myHighSecurePassword"); + when(subject.isPermitted("user:modify:abc")).thenReturn(true); + + UserDto userDto = mapper.map(user); + + assertThat(userDto.getPassword()).isBlank(); + } + @Test public void shouldMapLinks_forDelete() { User user = createDefaultUser(); @@ -65,7 +97,7 @@ public class UserToUserDtoMapperTest { UserDto userDto = mapper.map(user); - assertEquals("expected self link", expectedBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("self").get().getHref()); + assertEquals("expected self link", expectedBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("self").get().getHref()); assertEquals("expected delete link", expectedBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("delete").get().getHref()); } @@ -97,16 +129,6 @@ public class UserToUserDtoMapperTest { assertEquals("abc", userDto.getName()); } - @Test - public void shouldRemovePassword() { - User user = createDefaultUser(); - user.setPassword("password"); - - UserDto userDto = mapper.map(user); - - assertEquals(UserResource.DUMMY_PASSWORT, userDto.getPassword()); - } - @Test public void shouldMapTimes() { User user = createDefaultUser();