From d426445b4f89d50d40f5669215048d1bc6c11dbb Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Mon, 17 Sep 2018 09:59:19 +0200 Subject: [PATCH 01/12] 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(); From cea4f99d02406ede306c7713e2ca0e5751d0f321 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Mon, 17 Sep 2018 10:39:07 +0200 Subject: [PATCH 02/12] fix unit test --- .../src/test/java/sonia/scm/api/v2/resources/MeResourceTest.java | 1 + 1 file changed, 1 insertion(+) 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 5bfaad66f8..340533b5d2 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 @@ -59,6 +59,7 @@ public class MeResourceTest { 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"); userToDtoMapper.setResourceLinks(resourceLinks); MeResource meResource = new MeResource(userToDtoMapper, userManager); dispatcher.getRegistry().addSingletonResource(meResource); From 2eb685c5d0da7e8e392366748fa32204ed3bbacf Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Wed, 19 Sep 2018 10:47:26 +0200 Subject: [PATCH 03/12] Delete EscapeUtil and dependencies --- .../java/sonia/scm/repository/EscapeUtil.java | 207 ------------------ .../scm/repository/PreProcessorUtil.java | 87 +------- .../repository/api/BlameCommandBuilder.java | 23 +- .../repository/api/BrowseCommandBuilder.java | 23 +- .../repository/api/HookChangesetBuilder.java | 24 +- .../scm/repository/api/LogCommandBuilder.java | 23 +- .../api/ModificationsCommandBuilder.java | 5 +- .../sonia/scm/web/GitRepositoryViewer.java | 1 - 8 files changed, 7 insertions(+), 386 deletions(-) delete mode 100644 scm-core/src/main/java/sonia/scm/repository/EscapeUtil.java diff --git a/scm-core/src/main/java/sonia/scm/repository/EscapeUtil.java b/scm-core/src/main/java/sonia/scm/repository/EscapeUtil.java deleted file mode 100644 index 979ea0056d..0000000000 --- a/scm-core/src/main/java/sonia/scm/repository/EscapeUtil.java +++ /dev/null @@ -1,207 +0,0 @@ -/** - * Copyright (c) 2010, Sebastian Sdorra - * All rights reserved. - * - * Redistribution and use in source and binary forms, with or without - * modification, are permitted provided that the following conditions are met: - * - * 1. Redistributions of source code must retain the above copyright notice, - * this list of conditions and the following disclaimer. - * 2. Redistributions in binary form must reproduce the above copyright notice, - * this list of conditions and the following disclaimer in the documentation - * and/or other materials provided with the distribution. - * 3. Neither the name of SCM-Manager; nor the names of its - * contributors may be used to endorse or promote products derived from this - * software without specific prior written permission. - * - * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" - * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE - * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE - * DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR ANY - * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES - * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; - * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON - * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT - * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS - * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. - * - * http://bitbucket.org/sdorra/scm-manager - * - */ - - - -package sonia.scm.repository; - -//~--- non-JDK imports -------------------------------------------------------- - -import com.google.common.collect.Lists; -import org.apache.commons.lang.StringEscapeUtils; -import sonia.scm.util.Util; - -import java.util.List; - -//~--- JDK imports ------------------------------------------------------------ - -/** - * - * @author Sebastian Sdorra - * @since 1.15 - */ -public final class EscapeUtil -{ - - /** - * Constructs ... - * - */ - private EscapeUtil() {} - - //~--- methods -------------------------------------------------------------- - - /** - * Method description - * - * - * @param result - */ - public static void escape(BrowserResult result) - { - result.setBranch(escape(result.getBranch())); - result.setTag(escape(result.getTag())); - - for (FileObject fo : result) - { - escape(fo); - } - } - - /** - * Method description - * - * - * @param result - * @since 1.17 - */ - public static void escape(BlameResult result) - { - for (BlameLine line : result.getBlameLines()) - { - escape(line); - } - } - - /** - * Method description - * - * - * @param line - * @since 1.17 - */ - public static void escape(BlameLine line) - { - line.setDescription(escape(line.getDescription())); - escape(line.getAuthor()); - } - - /** - * Method description - * - * - * @param fo - */ - public static void escape(FileObject fo) - { - fo.setDescription(escape(fo.getDescription())); - fo.setName(fo.getName()); - fo.setPath(fo.getPath()); - } - - /** - * Method description - * - * - * @param changeset - */ - public static void escape(Changeset changeset) - { - changeset.setDescription(escape(changeset.getDescription())); - escape(changeset.getAuthor()); - changeset.setBranches(escapeList(changeset.getBranches())); - changeset.setTags(escapeList(changeset.getTags())); - } - - /** - * Method description - * - * - * @param person - * @since 1.17 - */ - public static void escape(Person person) - { - if (person != null) - { - person.setName(escape(person.getName())); - person.setMail(escape(person.getMail())); - } - } - - /** - * Method description - * - * - * @param result - */ - public static void escape(ChangesetPagingResult result) - { - for (Changeset c : result) - { - escape(c); - } - } - - /** - * Method description - * - * - * @param value - * - * @return - */ - public static String escape(String value) - { - return StringEscapeUtils.escapeHtml(value); - } - - /** - * Method description - * - * - * @param values - * - * @return - */ - public static List escapeList(List values) - { - if (Util.isNotEmpty(values)) - { - List newList = Lists.newArrayList(); - - for (String v : values) - { - newList.add(StringEscapeUtils.escapeHtml(v)); - } - - values = newList; - } - - return values; - } - - public static void escape(Modifications modifications) { - modifications.setAdded(escapeList(modifications.getAdded())); - modifications.setModified(escapeList(modifications.getModified())); - modifications.setRemoved(escapeList(modifications.getRemoved())); - } -} diff --git a/scm-core/src/main/java/sonia/scm/repository/PreProcessorUtil.java b/scm-core/src/main/java/sonia/scm/repository/PreProcessorUtil.java index 98730c2039..2a1d9c0340 100644 --- a/scm-core/src/main/java/sonia/scm/repository/PreProcessorUtil.java +++ b/scm-core/src/main/java/sonia/scm/repository/PreProcessorUtil.java @@ -111,7 +111,6 @@ public class PreProcessorUtil blameLine.getLineNumber(), repository.getName()); } - EscapeUtil.escape(blameLine); handlePreProcess(repository,blameLine,blameLinePreProcessorFactorySet, blameLinePreProcessorSet); } @@ -123,22 +122,6 @@ public class PreProcessorUtil * @param blameResult */ public void prepareForReturn(Repository repository, BlameResult blameResult) - { - prepareForReturn(repository, blameResult, true); - } - - /** - * Method description - * - * - * @param repository - * @param blameResult - * @param escape - * - * @since 1.35 - */ - public void prepareForReturn(Repository repository, BlameResult blameResult, - boolean escape) { if (logger.isTraceEnabled()) { @@ -146,10 +129,6 @@ public class PreProcessorUtil repository.getName()); } - if (escape) - { - EscapeUtil.escape(blameResult); - } handlePreProcessForIterable(repository, blameResult.getBlameLines(),blameLinePreProcessorFactorySet, blameLinePreProcessorSet); } @@ -162,32 +141,12 @@ public class PreProcessorUtil */ public void prepareForReturn(Repository repository, Changeset changeset) { - prepareForReturn(repository, changeset, true); - } - - /** - * Method description - * - * - * @param repository - * @param changeset - * @param escape - * - * @since 1.35 - */ - public void prepareForReturn(Repository repository, Changeset changeset, boolean escape){ logger.trace("prepare changeset {} of repository {} for return", changeset.getId(), repository.getName()); - if (escape) { - EscapeUtil.escape(changeset); - } handlePreProcess(repository, changeset, changesetPreProcessorFactorySet, changesetPreProcessorSet); } - public void prepareForReturn(Repository repository, Modifications modifications, boolean escape) { + public void prepareForReturn(Repository repository, Modifications modifications) { logger.trace("prepare modifications {} of repository {} for return", modifications, repository.getName()); - if (escape) { - EscapeUtil.escape(modifications); - } handlePreProcess(repository, modifications, modificationsPreProcessorFactorySet, modificationsPreProcessorSet); } @@ -199,22 +158,6 @@ public class PreProcessorUtil * @param result */ public void prepareForReturn(Repository repository, BrowserResult result) - { - prepareForReturn(repository, result, true); - } - - /** - * Method description - * - * - * @param repository - * @param result - * @param escape - * - * @since 1.35 - */ - public void prepareForReturn(Repository repository, BrowserResult result, - boolean escape) { if (logger.isTraceEnabled()) { @@ -222,10 +165,6 @@ public class PreProcessorUtil repository.getName()); } - if (escape) - { - EscapeUtil.escape(result); - } handlePreProcessForIterable(repository, result,fileObjectPreProcessorFactorySet, fileObjectPreProcessorSet); } @@ -235,12 +174,8 @@ public class PreProcessorUtil * * @param repository * @param result - * @param escape - * - * @since 1.35 */ - public void prepareForReturn(Repository repository, - ChangesetPagingResult result, boolean escape) + public void prepareForReturn(Repository repository, ChangesetPagingResult result) { if (logger.isTraceEnabled()) { @@ -248,27 +183,9 @@ public class PreProcessorUtil repository.getName()); } - if (escape) - { - EscapeUtil.escape(result); - } handlePreProcessForIterable(repository,result,changesetPreProcessorFactorySet, changesetPreProcessorSet); } - /** - * Method description - * - * - * @param repository - * @param result - */ - public void prepareForReturn(Repository repository, - ChangesetPagingResult result) - { - prepareForReturn(repository, result, true); - } - - private , P extends PreProcessor> void handlePreProcess(Repository repository, T processedObject, Collection factories, Collection

preProcessors) { diff --git a/scm-core/src/main/java/sonia/scm/repository/api/BlameCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/BlameCommandBuilder.java index 5a34345dce..f55abd4598 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/BlameCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/BlameCommandBuilder.java @@ -185,7 +185,7 @@ public final class BlameCommandBuilder if (!disablePreProcessors && (result != null)) { - preProcessorUtil.prepareForReturn(repository, result, !disableEscaping); + preProcessorUtil.prepareForReturn(repository, result); } return result; @@ -210,24 +210,6 @@ public final class BlameCommandBuilder return this; } - /** - * Disable html escaping for the returned blame lines. By default all - * blame lines are html escaped. - * - * - * @param disableEscaping true to disable the html escaping - * - * @return {@code this} - * - * @since 1.35 - */ - public BlameCommandBuilder setDisableEscaping(boolean disableEscaping) - { - this.disableEscaping = disableEscaping; - - return this; - } - /** * Disable the execution of pre processors. * @@ -362,9 +344,6 @@ public final class BlameCommandBuilder /** the cache */ private final Cache cache; - /** disable escaping */ - private boolean disableEscaping = false; - /** disable change */ private boolean disableCache = false; diff --git a/scm-core/src/main/java/sonia/scm/repository/api/BrowseCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/BrowseCommandBuilder.java index eeae45b893..fe39aa0a05 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/BrowseCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/BrowseCommandBuilder.java @@ -179,7 +179,7 @@ public final class BrowseCommandBuilder if (!disablePreProcessors && (result != null)) { - preProcessorUtil.prepareForReturn(repository, result, !disableEscaping); + preProcessorUtil.prepareForReturn(repository, result); List fileObjects = result.getFiles(); @@ -212,24 +212,6 @@ public final class BrowseCommandBuilder return this; } - /** - * Disable html escaping for the returned file objects. By default all - * file objects are html escaped. - * - * - * @param disableEscaping true to disable the html escaping - * - * @return {@code this} - * - * @since 1.35 - */ - public BrowseCommandBuilder setDisableEscaping(boolean disableEscaping) - { - this.disableEscaping = disableEscaping; - - return this; - } - /** * Disabling the last commit means that every call to * {@link FileObject#getDescription()} and @@ -433,9 +415,6 @@ public final class BrowseCommandBuilder /** cache */ private final Cache cache; - /** disable escaping */ - private boolean disableEscaping = false; - /** disables the cache */ private boolean disableCache = false; diff --git a/scm-core/src/main/java/sonia/scm/repository/api/HookChangesetBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/HookChangesetBuilder.java index c06e227d5f..fe43fee038 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/HookChangesetBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/HookChangesetBuilder.java @@ -132,8 +132,7 @@ public final class HookChangesetBuilder try { copy = DeepCopy.copy(c); - preProcessorUtil.prepareForReturn(repository, copy, - !disableEscaping); + preProcessorUtil.prepareForReturn(repository, copy); } catch (IOException ex) { @@ -156,24 +155,6 @@ public final class HookChangesetBuilder //~--- set methods ---------------------------------------------------------- - /** - * Disable html escaping for the returned changesets. By default all - * changesets are html escaped. - * - * - * @param disableEscaping true to disable the html escaping - * - * @return {@code this} - * - * @since 1.35 - */ - public HookChangesetBuilder setDisableEscaping(boolean disableEscaping) - { - this.disableEscaping = disableEscaping; - - return this; - } - /** * Disable the execution of pre processors. * @@ -192,9 +173,6 @@ public final class HookChangesetBuilder //~--- fields --------------------------------------------------------------- - /** disable escaping */ - private boolean disableEscaping = false; - /** disable pre processors marker */ private boolean disablePreProcessors = false; diff --git a/scm-core/src/main/java/sonia/scm/repository/api/LogCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/LogCommandBuilder.java index 1450cf687a..9c782a781b 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/LogCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/LogCommandBuilder.java @@ -264,7 +264,7 @@ public final class LogCommandBuilder if (!disablePreProcessors && (cpr != null)) { - preProcessorUtil.prepareForReturn(repository, cpr, !disableEscaping); + preProcessorUtil.prepareForReturn(repository, cpr); } return cpr; @@ -306,24 +306,6 @@ public final class LogCommandBuilder return this; } - /** - * Disable html escaping for the returned changesets. By default all - * changesets are html escaped. - * - * - * @param disableEscaping true to disable the html escaping - * - * @return {@code this} - * - * @since 1.35 - */ - public LogCommandBuilder setDisableEscaping(boolean disableEscaping) - { - this.disableEscaping = disableEscaping; - - return this; - } - /** * Disable the execution of pre processors. * @@ -545,9 +527,6 @@ public final class LogCommandBuilder /** repository to query */ private final Repository repository; - /** disable escaping */ - private boolean disableEscaping = false; - /** disable cache */ private boolean disableCache = false; diff --git a/scm-core/src/main/java/sonia/scm/repository/api/ModificationsCommandBuilder.java b/scm-core/src/main/java/sonia/scm/repository/api/ModificationsCommandBuilder.java index d81088bd33..6459b47cd5 100644 --- a/scm-core/src/main/java/sonia/scm/repository/api/ModificationsCommandBuilder.java +++ b/scm-core/src/main/java/sonia/scm/repository/api/ModificationsCommandBuilder.java @@ -43,9 +43,6 @@ public final class ModificationsCommandBuilder { private final PreProcessorUtil preProcessorUtil; - @Setter - private boolean disableEscaping = false; - @Setter private boolean disableCache = false; @@ -90,7 +87,7 @@ public final class ModificationsCommandBuilder { } } if (!disablePreProcessors && (modifications != null)) { - preProcessorUtil.prepareForReturn(repository, modifications, !disableEscaping); + preProcessorUtil.prepareForReturn(repository, modifications); } return modifications; } diff --git a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitRepositoryViewer.java b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitRepositoryViewer.java index 162e3db489..773e09aada 100644 --- a/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitRepositoryViewer.java +++ b/scm-plugins/scm-git-plugin/src/main/java/sonia/scm/web/GitRepositoryViewer.java @@ -278,7 +278,6 @@ public class GitRepositoryViewer { //J- ChangesetPagingResult cpr = service.getLogCommand() - .setDisableEscaping(true) .setBranch(name) .setPagingLimit(CHANGESET_PER_BRANCH) .getChangesets(); From 5c64c52a31f1e89fa2f1af5e83d594f6ca6a6af8 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Wed, 19 Sep 2018 15:54:24 +0200 Subject: [PATCH 04/12] Add tests for the Change Password Endpoints --- .../ChangePasswordNotAllowedException.java | 2 +- .../scm/user/InvalidPasswordException.java | 8 + .../main/java/sonia/scm/user/UserManager.java | 21 + .../src/test/java/sonia/scm/it/MeITCase.java | 79 +++ .../java/sonia/scm/it/PermissionsITCase.java | 12 +- .../java/sonia/scm/it/RepositoriesITCase.java | 12 +- .../sonia/scm/it/RepositoryAccessITCase.java | 22 +- .../java/sonia/scm/it/RepositoryRequests.java | 293 ----------- .../test/java/sonia/scm/it/UserITCase.java | 112 +++++ .../it/utils/NullAwareJsonObjectBuilder.java | 95 ++++ .../scm/it/{ => utils}/RegExMatcher.java | 4 +- .../scm/it/{ => utils}/RepositoryUtil.java | 16 +- .../sonia/scm/it/{ => utils}/RestUtil.java | 2 +- .../java/sonia/scm/it/utils/ScmRequests.java | 465 ++++++++++++++++++ .../sonia/scm/it/{ => utils}/ScmTypes.java | 6 +- .../sonia/scm/it/{ => utils}/TestData.java | 66 ++- ...angePasswordNotAllowedExceptionMapper.java | 2 + .../InvalidPasswordExceptionMapper.java | 17 + .../scm/api/v2/resources/MapperModule.java | 1 + .../scm/api/v2/resources/MeResource.java | 36 +- .../api/v2/resources/MeToUserDtoMapper.java | 41 ++ .../scm/api/v2/resources/ResourceLinks.java | 31 ++ .../scm/api/v2/resources/UserResource.java | 15 +- .../api/v2/resources/UserToUserDtoMapper.java | 2 +- .../scm/api/v2/resources/DispatcherMock.java | 1 + .../scm/api/v2/resources/MeResourceTest.java | 91 +++- .../v2/resources/MeToUserDtoMapperTest.java | 135 +++++ .../api/v2/resources/ResourceLinksMock.java | 4 +- .../v2/resources/UserRootResourceTest.java | 6 +- .../v2/resources/UserToUserDtoMapperTest.java | 12 +- 30 files changed, 1229 insertions(+), 380 deletions(-) rename {scm-webapp/src/main/java/sonia/scm/api/v2/resources => scm-core/src/main/java/sonia/scm/user}/ChangePasswordNotAllowedException.java (82%) create mode 100644 scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java create mode 100644 scm-it/src/test/java/sonia/scm/it/MeITCase.java delete mode 100644 scm-it/src/test/java/sonia/scm/it/RepositoryRequests.java create mode 100644 scm-it/src/test/java/sonia/scm/it/UserITCase.java create mode 100644 scm-it/src/test/java/sonia/scm/it/utils/NullAwareJsonObjectBuilder.java rename scm-it/src/test/java/sonia/scm/it/{ => utils}/RegExMatcher.java (87%) rename scm-it/src/test/java/sonia/scm/it/{ => utils}/RepositoryUtil.java (81%) rename scm-it/src/test/java/sonia/scm/it/{ => utils}/RestUtil.java (97%) create mode 100644 scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java rename scm-it/src/test/java/sonia/scm/it/{ => utils}/ScmTypes.java (72%) rename scm-it/src/test/java/sonia/scm/it/{ => utils}/TestData.java (77%) create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java create mode 100644 scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java create mode 100644 scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java b/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java similarity index 82% rename from scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java rename to scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java index 755cd709cc..b2925cf65c 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/ChangePasswordNotAllowedException.java +++ b/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java @@ -1,4 +1,4 @@ -package sonia.scm.api.v2.resources; +package sonia.scm.user; public class ChangePasswordNotAllowedException extends RuntimeException { diff --git a/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java new file mode 100644 index 0000000000..beda2c83fb --- /dev/null +++ b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java @@ -0,0 +1,8 @@ +package sonia.scm.user; + +public class InvalidPasswordException extends RuntimeException { + + public InvalidPasswordException(String message) { + super(message); + } +} 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 0a705c8d60..90dbcd74b3 100644 --- a/scm-core/src/main/java/sonia/scm/user/UserManager.java +++ b/scm-core/src/main/java/sonia/scm/user/UserManager.java @@ -38,6 +38,9 @@ package sonia.scm.user; import sonia.scm.Manager; import sonia.scm.search.Searchable; +import java.text.MessageFormat; +import java.util.function.Consumer; + /** * The central class for managing {@link User} objects. * This class is a singleton and is available via injection. @@ -68,4 +71,22 @@ public interface UserManager * @since 1.14 */ public String getDefaultType(); + + + /** + * Only account of the default type "xml" can change their password + */ + default Consumer getUserTypeChecker() { + return user -> { + if (!isTypeDefault(user)) { + throw new ChangePasswordNotAllowedException(MessageFormat.format("It is not possible to change password for User of type {0}", user.getType())); + } + }; + } + + default boolean isTypeDefault(User user) { + return getDefaultType().equals(user.getType()); + } + + } diff --git a/scm-it/src/test/java/sonia/scm/it/MeITCase.java b/scm-it/src/test/java/sonia/scm/it/MeITCase.java new file mode 100644 index 0000000000..1c7b5c9e03 --- /dev/null +++ b/scm-it/src/test/java/sonia/scm/it/MeITCase.java @@ -0,0 +1,79 @@ +package sonia.scm.it; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import sonia.scm.it.utils.ScmRequests; +import sonia.scm.it.utils.TestData; + +import static org.assertj.core.api.Assertions.assertThat; + +public class MeITCase { + + @Before + public void init() { + TestData.cleanup(); + } + + @Test + public void adminShouldChangeOwnPassword() { + String newPassword = TestData.USER_SCM_ADMIN + "1"; + // admin change the own password + ScmRequests.start() + .given() + .url(TestData.getMeUrl()) + .usernameAndPassword(TestData.USER_SCM_ADMIN, TestData.USER_SCM_ADMIN) + .getMeResource() + .assertStatusCode(200) + .usingMeResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) + .assertPassword(Assert::assertNull) + .assertType(s -> assertThat(s).isEqualTo("xml")) + .requestChangePassword(TestData.USER_SCM_ADMIN, newPassword) + .assertStatusCode(204); + // assert password is changed -> login with the new Password than undo changes + ScmRequests.start() + .given() + .url(TestData.getUserUrl(TestData.USER_SCM_ADMIN)) + .usernameAndPassword(TestData.USER_SCM_ADMIN, newPassword) + .getMeResource() + .assertStatusCode(200) + .usingMeResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE))// still admin + .requestChangePassword(newPassword, TestData.USER_SCM_ADMIN) + .assertStatusCode(204); + } + + @Test + public void shouldHidePasswordLinkIfUserTypeIsNotXML() { + String newUser = "user"; + String password = "pass"; + String type = "not XML Type"; + TestData.createUser(newUser, password, true, type); + ScmRequests.start() + .given() + .url(TestData.getMeUrl()) + .usernameAndPassword(newUser, password) + .getMeResource() + .assertStatusCode(200) + .usingMeResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) + .assertPassword(Assert::assertNull) + .assertType(s -> assertThat(s).isEqualTo(type)) + .assertPasswordLinkDoesNotExists(); + } + + @Test + public void shouldGet403IfUserIsNotAdmin() { + String newUser = "user"; + String password = "pass"; + String type = "xml"; + TestData.createUser(newUser, password, false, type); + ScmRequests.start() + .given() + .url(TestData.getMeUrl()) + .usernameAndPassword(newUser, password) + .getMeResource() + .assertStatusCode(403); + } +} diff --git a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java index 63d8b46d47..f288d4891c 100644 --- a/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/PermissionsITCase.java @@ -39,6 +39,8 @@ import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import sonia.scm.it.utils.RepositoryUtil; +import sonia.scm.it.utils.TestData; import sonia.scm.repository.PermissionType; import sonia.scm.repository.client.api.RepositoryClient; import sonia.scm.repository.client.api.RepositoryClientException; @@ -51,11 +53,11 @@ import java.util.Objects; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNull; -import static sonia.scm.it.RepositoryUtil.addAndCommitRandomFile; -import static sonia.scm.it.RestUtil.given; -import static sonia.scm.it.ScmTypes.availableScmTypes; -import static sonia.scm.it.TestData.USER_SCM_ADMIN; -import static sonia.scm.it.TestData.callRepository; +import static sonia.scm.it.utils.RepositoryUtil.addAndCommitRandomFile; +import static sonia.scm.it.utils.RestUtil.given; +import static sonia.scm.it.utils.ScmTypes.availableScmTypes; +import static sonia.scm.it.utils.TestData.USER_SCM_ADMIN; +import static sonia.scm.it.utils.TestData.callRepository; @RunWith(Parameterized.class) public class PermissionsITCase { diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java index 21d4d97b1b..c49a65bea2 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoriesITCase.java @@ -42,6 +42,8 @@ import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; import org.junit.runners.Parameterized.Parameters; +import sonia.scm.it.utils.RepositoryUtil; +import sonia.scm.it.utils.TestData; import sonia.scm.repository.client.api.RepositoryClient; import sonia.scm.web.VndMediaType; @@ -53,11 +55,11 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.nullValue; import static org.junit.Assert.assertEquals; -import static sonia.scm.it.RegExMatcher.matchesPattern; -import static sonia.scm.it.RestUtil.createResourceUrl; -import static sonia.scm.it.RestUtil.given; -import static sonia.scm.it.ScmTypes.availableScmTypes; -import static sonia.scm.it.TestData.repositoryJson; +import static sonia.scm.it.utils.RegExMatcher.matchesPattern; +import static sonia.scm.it.utils.RestUtil.createResourceUrl; +import static sonia.scm.it.utils.RestUtil.given; +import static sonia.scm.it.utils.ScmTypes.availableScmTypes; +import static sonia.scm.it.utils.TestData.repositoryJson; @RunWith(Parameterized.class) public class RepositoriesITCase { diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java index 7bf04b7b81..4934094083 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/RepositoryAccessITCase.java @@ -11,6 +11,9 @@ import org.junit.Test; import org.junit.rules.TemporaryFolder; import org.junit.runner.RunWith; import org.junit.runners.Parameterized; +import sonia.scm.it.utils.RepositoryUtil; +import sonia.scm.it.utils.ScmRequests; +import sonia.scm.it.utils.TestData; import sonia.scm.repository.Changeset; import sonia.scm.repository.client.api.ClientCommand; import sonia.scm.repository.client.api.RepositoryClient; @@ -28,10 +31,10 @@ import static java.lang.Thread.sleep; import static org.assertj.core.api.Assertions.assertThat; import static org.hamcrest.Matchers.equalTo; import static org.junit.Assert.assertNotNull; -import static sonia.scm.it.RestUtil.ADMIN_PASSWORD; -import static sonia.scm.it.RestUtil.ADMIN_USERNAME; -import static sonia.scm.it.RestUtil.given; -import static sonia.scm.it.ScmTypes.availableScmTypes; +import static sonia.scm.it.utils.RestUtil.ADMIN_PASSWORD; +import static sonia.scm.it.utils.RestUtil.ADMIN_USERNAME; +import static sonia.scm.it.utils.RestUtil.given; +import static sonia.scm.it.utils.ScmTypes.availableScmTypes; @RunWith(Parameterized.class) public class RepositoryAccessITCase { @@ -41,7 +44,7 @@ public class RepositoryAccessITCase { private final String repositoryType; private File folder; - private RepositoryRequests.AppliedRepositoryGetRequest repositoryGetRequest; + private ScmRequests.AppliedRepositoryRequest repositoryGetRequest; public RepositoryAccessITCase(String repositoryType) { this.repositoryType = repositoryType; @@ -56,12 +59,17 @@ public class RepositoryAccessITCase { public void init() { TestData.createDefault(); folder = tempFolder.getRoot(); - repositoryGetRequest = RepositoryRequests.start() + repositoryGetRequest = ScmRequests.start() .given() .url(TestData.getDefaultRepositoryUrl(repositoryType)) .usernameAndPassword(ADMIN_USERNAME, ADMIN_PASSWORD) - .get() + .getRepositoryResource() .assertStatusCode(HttpStatus.SC_OK); + ScmRequests.AppliedMeRequest meGetRequest = ScmRequests.start() + .given() + .url(TestData.getMeUrl()) + .usernameAndPassword(ADMIN_USERNAME, ADMIN_PASSWORD) + .getMeResource(); } @Test diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryRequests.java b/scm-it/src/test/java/sonia/scm/it/RepositoryRequests.java deleted file mode 100644 index 79300d7b45..0000000000 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryRequests.java +++ /dev/null @@ -1,293 +0,0 @@ -package sonia.scm.it; - -import io.restassured.RestAssured; -import io.restassured.response.Response; - -import java.util.List; -import java.util.Map; -import java.util.function.Consumer; - - -/** - * Encapsulate rest requests of a repository in builder pattern - *

- * A Get Request can be applied with the methods request*() - * These methods return a AppliedGet*Request object - * This object can be used to apply general assertions over the rest Assured response - * In the AppliedGet*Request classes there is a using*Response() method - * that return the *Response class containing specific operations related to the specific response - * the *Response class contains also the request*() method to apply the next GET request from a link in the response. - */ -public class RepositoryRequests { - - private String url; - private String username; - private String password; - - static RepositoryRequests start() { - return new RepositoryRequests(); - } - - Given given() { - return new Given(); - } - - - /** - * Apply a GET Request to the extracted url from the given link - * - * @param linkPropertyName the property name of link - * @param response the response containing the link - * @return the response of the GET request using the given link - */ - private Response getResponseFromLink(Response response, String linkPropertyName) { - return getResponse(response - .then() - .extract() - .path(linkPropertyName)); - } - - - /** - * Apply a GET Request to the given url and return the response. - * - * @param url the url of the GET request - * @return the response of the GET request using the given url - */ - private Response getResponse(String url) { - return RestAssured.given() - .auth().preemptive().basic(username, password) - .when() - .get(url); - } - - private void setUrl(String url) { - this.url = url; - } - - private void setUsername(String username) { - this.username = username; - } - - private void setPassword(String password) { - this.password = password; - } - - private String getUrl() { - return url; - } - - private String getUsername() { - return username; - } - - private String getPassword() { - return password; - } - - class Given { - - GivenUrl url(String url) { - setUrl(url); - return new GivenUrl(); - } - - } - - class GivenWithUrlAndAuth { - AppliedRepositoryGetRequest get() { - return new AppliedRepositoryGetRequest( - getResponse(url) - ); - } - } - - class AppliedGetRequest { - private Response response; - - public AppliedGetRequest(Response response) { - this.response = response; - } - - /** - * apply custom assertions to the actual response - * - * @param consumer consume the response in order to assert the content. the header, the payload etc.. - * @return the self object - */ - SELF assertResponse(Consumer consumer) { - consumer.accept(response); - return (SELF) this; - } - - /** - * special assertion of the status code - * - * @param expectedStatusCode the expected status code - * @return the self object - */ - SELF assertStatusCode(int expectedStatusCode) { - this.response.then().assertThat().statusCode(expectedStatusCode); - return (SELF) this; - } - - } - - class AppliedRepositoryGetRequest extends AppliedGetRequest { - - AppliedRepositoryGetRequest(Response response) { - super(response); - } - - RepositoryResponse usingRepositoryResponse() { - return new RepositoryResponse(super.response); - } - } - - class RepositoryResponse { - - private Response repositoryResponse; - - public RepositoryResponse(Response repositoryResponse) { - this.repositoryResponse = repositoryResponse; - } - - AppliedGetSourcesRequest requestSources() { - return new AppliedGetSourcesRequest(getResponseFromLink(repositoryResponse, "_links.sources.href")); - } - - AppliedGetChangesetsRequest requestChangesets() { - return new AppliedGetChangesetsRequest(getResponseFromLink(repositoryResponse, "_links.changesets.href")); - } - } - - class AppliedGetChangesetsRequest extends AppliedGetRequest { - - AppliedGetChangesetsRequest(Response response) { - super(response); - } - - ChangesetsResponse usingChangesetsResponse() { - return new ChangesetsResponse(super.response); - } - } - - class ChangesetsResponse { - private Response changesetsResponse; - - public ChangesetsResponse(Response changesetsResponse) { - this.changesetsResponse = changesetsResponse; - } - - ChangesetsResponse assertChangesets(Consumer> changesetsConsumer) { - List changesets = changesetsResponse.then().extract().path("_embedded.changesets"); - changesetsConsumer.accept(changesets); - return this; - } - - AppliedGetDiffRequest requestDiff(String revision) { - return new AppliedGetDiffRequest(getResponseFromLink(changesetsResponse, "_embedded.changesets.find{it.id=='" + revision + "'}._links.diff.href")); - } - - public AppliedGetModificationsRequest requestModifications(String revision) { - return new AppliedGetModificationsRequest(getResponseFromLink(changesetsResponse, "_embedded.changesets.find{it.id=='" + revision + "'}._links.modifications.href")); - } - } - - class AppliedGetSourcesRequest extends AppliedGetRequest { - - public AppliedGetSourcesRequest(Response sourcesResponse) { - super(sourcesResponse); - } - - SourcesResponse usingSourcesResponse() { - return new SourcesResponse(super.response); - } - } - - class SourcesResponse { - - private Response sourcesResponse; - - SourcesResponse(Response sourcesResponse) { - this.sourcesResponse = sourcesResponse; - } - - SourcesResponse assertRevision(Consumer assertRevision) { - String revision = sourcesResponse.then().extract().path("revision"); - assertRevision.accept(revision); - return this; - } - - SourcesResponse assertFiles(Consumer assertFiles) { - List files = sourcesResponse.then().extract().path("files"); - assertFiles.accept(files); - return this; - } - - AppliedGetChangesetsRequest requestFileHistory(String fileName) { - return new AppliedGetChangesetsRequest(getResponseFromLink(sourcesResponse, "_embedded.files.find{it.name=='" + fileName + "'}._links.history.href")); - } - - AppliedGetSourcesRequest requestSelf(String fileName) { - return new AppliedGetSourcesRequest(getResponseFromLink(sourcesResponse, "_embedded.files.find{it.name=='" + fileName + "'}._links.self.href")); - } - } - - class AppliedGetDiffRequest extends AppliedGetRequest { - - AppliedGetDiffRequest(Response response) { - super(response); - } - } - - class GivenUrl { - - GivenWithUrlAndAuth usernameAndPassword(String username, String password) { - setUsername(username); - setPassword(password); - return new GivenWithUrlAndAuth(); - } - } - - class AppliedGetModificationsRequest extends AppliedGetRequest { - public AppliedGetModificationsRequest(Response response) { super(response); } - ModificationsResponse usingModificationsResponse() { - return new ModificationsResponse(super.response); - } - - } - - class ModificationsResponse { - private Response resource; - - public ModificationsResponse(Response resource) { - this.resource = resource; - } - - ModificationsResponse assertRevision(Consumer assertRevision) { - String revision = resource.then().extract().path("revision"); - assertRevision.accept(revision); - return this; - } - - ModificationsResponse assertAdded(Consumer> assertAdded) { - List added = resource.then().extract().path("added"); - assertAdded.accept(added); - return this; - } - - ModificationsResponse assertRemoved(Consumer> assertRemoved) { - List removed = resource.then().extract().path("removed"); - assertRemoved.accept(removed); - return this; - } - - ModificationsResponse assertModified(Consumer> assertModified) { - List modified = resource.then().extract().path("modified"); - assertModified.accept(modified); - return this; - } - - } -} diff --git a/scm-it/src/test/java/sonia/scm/it/UserITCase.java b/scm-it/src/test/java/sonia/scm/it/UserITCase.java new file mode 100644 index 0000000000..65ed6b71bb --- /dev/null +++ b/scm-it/src/test/java/sonia/scm/it/UserITCase.java @@ -0,0 +1,112 @@ +package sonia.scm.it; + +import org.junit.Assert; +import org.junit.Before; +import org.junit.Test; +import sonia.scm.it.utils.ScmRequests; +import sonia.scm.it.utils.TestData; + +import static org.assertj.core.api.Assertions.assertThat; + +public class UserITCase { + + @Before + public void init(){ + TestData.cleanup(); + } + + @Test + public void adminShouldChangeOwnPassword() { + String newPassword = TestData.USER_SCM_ADMIN + "1"; + // admin change the own password + ScmRequests.start() + .given() + .url(TestData.getUserUrl(TestData.USER_SCM_ADMIN)) + .usernameAndPassword(TestData.USER_SCM_ADMIN, TestData.USER_SCM_ADMIN) + .getUserResource() + .assertStatusCode(200) + .usingUserResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) + .assertPassword(Assert::assertNull) + .requestChangePassword(newPassword) // the oldPassword is not needed in the user resource + .assertStatusCode(204); + // assert password is changed -> login with the new Password than undo changes + ScmRequests.start() + .given() + .url(TestData.getUserUrl(TestData.USER_SCM_ADMIN)) + .usernameAndPassword(TestData.USER_SCM_ADMIN, newPassword) + .getUserResource() + .assertStatusCode(200) + .usingUserResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) + .assertPassword(Assert::assertNull) + .requestChangePassword(TestData.USER_SCM_ADMIN) + .assertStatusCode(204); + + } + + @Test + public void adminShouldChangePasswordOfOtherUser() { + String newUser = "user"; + String password = "pass"; + TestData.createUser(newUser, password, true, "xml"); + String newPassword = "new_password"; + // admin change the password of the user + ScmRequests.start() + .given() + .url(TestData.getUserUrl(newUser))// the admin get the user object + .usernameAndPassword(TestData.USER_SCM_ADMIN, TestData.USER_SCM_ADMIN) + .getUserResource() + .assertStatusCode(200) + .usingUserResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) // the user anonymous is not an admin + .assertPassword(Assert::assertNull) + .requestChangePassword(newPassword) // the oldPassword is not needed in the user resource + .assertStatusCode(204); + // assert password is changed + ScmRequests.start() + .given() + .url(TestData.getUserUrl(newUser)) + .usernameAndPassword(newUser, newPassword) + .getUserResource() + .assertStatusCode(200); + + } + + + @Test + public void shouldHidePasswordLinkIfUserTypeIsNotXML() { + String newUser = "user"; + String password = "pass"; + String type = "not XML Type"; + TestData.createUser(newUser, password, true, type); + ScmRequests.start() + .given() + .url(TestData.getMeUrl()) + .usernameAndPassword(newUser, password) + .getUserResource() + .assertStatusCode(200) + .usingUserResponse() + .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) + .assertPassword(Assert::assertNull) + .assertType(s -> assertThat(s).isEqualTo(type)) + .assertPasswordLinkDoesNotExists(); + } + + @Test + public void shouldGet403IfUserIsNotAdmin() { + String newUser = "user"; + String password = "pass"; + String type = "xml"; + TestData.createUser(newUser, password, false, type); + ScmRequests.start() + .given() + .url(TestData.getMeUrl()) + .usernameAndPassword(newUser, password) + .getUserResource() + .assertStatusCode(403); + } + + + +} diff --git a/scm-it/src/test/java/sonia/scm/it/utils/NullAwareJsonObjectBuilder.java b/scm-it/src/test/java/sonia/scm/it/utils/NullAwareJsonObjectBuilder.java new file mode 100644 index 0000000000..31a12f1969 --- /dev/null +++ b/scm-it/src/test/java/sonia/scm/it/utils/NullAwareJsonObjectBuilder.java @@ -0,0 +1,95 @@ +package sonia.scm.it.utils; + +import javax.json.JsonArrayBuilder; +import javax.json.JsonObject; +import javax.json.JsonObjectBuilder; +import javax.json.JsonValue; +import java.math.BigDecimal; +import java.math.BigInteger; + +public class NullAwareJsonObjectBuilder implements JsonObjectBuilder { + public static JsonObjectBuilder wrap(JsonObjectBuilder builder) { + if (builder == null) { + throw new IllegalArgumentException("Can't wrap nothing."); + } + return new NullAwareJsonObjectBuilder(builder); + } + + private final JsonObjectBuilder builder; + + private NullAwareJsonObjectBuilder(JsonObjectBuilder builder) { + this.builder = builder; + } + + public JsonObjectBuilder add(String name, JsonValue value) { + return builder.add(name, (value == null) ? JsonValue.NULL : value); + } + + @Override + public JsonObjectBuilder add(String name, String value) { + if (value != null){ + return builder.add(name, value ); + }else{ + return builder.addNull(name); + } + } + + @Override + public JsonObjectBuilder add(String name, BigInteger value) { + if (value != null){ + return builder.add(name, value ); + }else{ + return builder.addNull(name); + } + } + + @Override + public JsonObjectBuilder add(String name, BigDecimal value) { + if (value != null){ + return builder.add(name, value ); + }else{ + return builder.addNull(name); + } + } + + @Override + public JsonObjectBuilder add(String s, int i) { + return builder.add(s, i); + } + + @Override + public JsonObjectBuilder add(String s, long l) { + return builder.add(s, l); + } + + @Override + public JsonObjectBuilder add(String s, double v) { + return builder.add(s, v); + } + + @Override + public JsonObjectBuilder add(String s, boolean b) { + return builder.add(s, b); + } + + @Override + public JsonObjectBuilder addNull(String s) { + return builder.addNull(s); + } + + @Override + public JsonObjectBuilder add(String s, JsonObjectBuilder jsonObjectBuilder) { + return builder.add(s, jsonObjectBuilder); + } + + @Override + public JsonObjectBuilder add(String s, JsonArrayBuilder jsonArrayBuilder) { + return builder.add(s, jsonArrayBuilder); + } + + @Override + public JsonObject build() { + return builder.build(); + } + +} diff --git a/scm-it/src/test/java/sonia/scm/it/RegExMatcher.java b/scm-it/src/test/java/sonia/scm/it/utils/RegExMatcher.java similarity index 87% rename from scm-it/src/test/java/sonia/scm/it/RegExMatcher.java rename to scm-it/src/test/java/sonia/scm/it/utils/RegExMatcher.java index e5dc7931d3..10386a682f 100644 --- a/scm-it/src/test/java/sonia/scm/it/RegExMatcher.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/RegExMatcher.java @@ -1,4 +1,4 @@ -package sonia.scm.it; +package sonia.scm.it.utils; import org.hamcrest.BaseMatcher; import org.hamcrest.Description; @@ -6,7 +6,7 @@ import org.hamcrest.Matcher; import java.util.regex.Pattern; -class RegExMatcher extends BaseMatcher { +public class RegExMatcher extends BaseMatcher { public static Matcher matchesPattern(String pattern) { return new RegExMatcher(pattern); } diff --git a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java b/scm-it/src/test/java/sonia/scm/it/utils/RepositoryUtil.java similarity index 81% rename from scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java rename to scm-it/src/test/java/sonia/scm/it/utils/RepositoryUtil.java index 96b92a0bed..2987780bcb 100644 --- a/scm-it/src/test/java/sonia/scm/it/RepositoryUtil.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/RepositoryUtil.java @@ -1,4 +1,4 @@ -package sonia.scm.it; +package sonia.scm.it.utils; import com.google.common.base.Charsets; import com.google.common.io.Files; @@ -24,11 +24,11 @@ public class RepositoryUtil { private static final RepositoryClientFactory REPOSITORY_CLIENT_FACTORY = new RepositoryClientFactory(); - static RepositoryClient createRepositoryClient(String repositoryType, File folder) throws IOException { + public static RepositoryClient createRepositoryClient(String repositoryType, File folder) throws IOException { return createRepositoryClient(repositoryType, folder, "scmadmin", "scmadmin"); } - static RepositoryClient createRepositoryClient(String repositoryType, File folder, String username, String password) throws IOException { + public static RepositoryClient createRepositoryClient(String repositoryType, File folder, String username, String password) throws IOException { String httpProtocolUrl = TestData.callRepository(username, password, repositoryType, HttpStatus.SC_OK) .extract() .path("_links.httpProtocol.href"); @@ -36,14 +36,14 @@ public class RepositoryUtil { return REPOSITORY_CLIENT_FACTORY.create(repositoryType, httpProtocolUrl, username, password, folder); } - static String addAndCommitRandomFile(RepositoryClient client, String username) throws IOException { + public static String addAndCommitRandomFile(RepositoryClient client, String username) throws IOException { String uuid = UUID.randomUUID().toString(); String name = "file-" + uuid + ".uuid"; createAndCommitFile(client, username, name, uuid); return name; } - static Changeset createAndCommitFile(RepositoryClient repositoryClient, String username, String fileName, String content) throws IOException { + public static Changeset createAndCommitFile(RepositoryClient repositoryClient, String username, String fileName, String content) throws IOException { writeAndAddFile(repositoryClient, fileName, content); return commit(repositoryClient, username, "added " + fileName); } @@ -59,7 +59,7 @@ public class RepositoryUtil { * @return the changeset with all modifications * @throws IOException */ - static Changeset commitMultipleFileModifications(RepositoryClient repositoryClient, String username, Map addedFiles, Map modifiedFiles, List removedFiles) throws IOException { + public static Changeset commitMultipleFileModifications(RepositoryClient repositoryClient, String username, Map addedFiles, Map modifiedFiles, List removedFiles) throws IOException { for (String fileName : addedFiles.keySet()) { writeAndAddFile(repositoryClient, fileName, addedFiles.get(fileName)); } @@ -80,7 +80,7 @@ public class RepositoryUtil { return file; } - static Changeset removeAndCommitFile(RepositoryClient repositoryClient, String username, String fileName) throws IOException { + public static Changeset removeAndCommitFile(RepositoryClient repositoryClient, String username, String fileName) throws IOException { deleteFileAndApplyRemoveCommand(repositoryClient, fileName); return commit(repositoryClient, username, "removed " + fileName); } @@ -115,7 +115,7 @@ public class RepositoryUtil { return changeset; } - static Tag addTag(RepositoryClient repositoryClient, String revision, String tagName) throws IOException { + public static Tag addTag(RepositoryClient repositoryClient, String revision, String tagName) throws IOException { if (repositoryClient.isCommandSupported(ClientCommand.TAG)) { Tag tag = repositoryClient.getTagCommand().setRevision(revision).tag(tagName, TestData.USER_SCM_ADMIN); if (repositoryClient.isCommandSupported(ClientCommand.PUSH)) { diff --git a/scm-it/src/test/java/sonia/scm/it/RestUtil.java b/scm-it/src/test/java/sonia/scm/it/utils/RestUtil.java similarity index 97% rename from scm-it/src/test/java/sonia/scm/it/RestUtil.java rename to scm-it/src/test/java/sonia/scm/it/utils/RestUtil.java index a7409e1995..c8b01a6d72 100644 --- a/scm-it/src/test/java/sonia/scm/it/RestUtil.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/RestUtil.java @@ -1,4 +1,4 @@ -package sonia.scm.it; +package sonia.scm.it.utils; import io.restassured.RestAssured; import io.restassured.specification.RequestSpecification; 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 new file mode 100644 index 0000000000..41fd9a1290 --- /dev/null +++ b/scm-it/src/test/java/sonia/scm/it/utils/ScmRequests.java @@ -0,0 +1,465 @@ +package sonia.scm.it.utils; + +import io.restassured.RestAssured; +import io.restassured.response.Response; +import sonia.scm.web.VndMediaType; + +import java.net.URI; +import java.util.List; +import java.util.Map; +import java.util.function.Consumer; + +import static org.hamcrest.Matchers.is; +import static sonia.scm.it.utils.TestData.createPasswordChangeJson; + + +/** + * Encapsulate rest requests of a repository in builder pattern + *

+ * A Get Request can be applied with the methods request*() + * These methods return a AppliedGet*Request object + * This object can be used to apply general assertions over the rest Assured response + * In the AppliedGet*Request classes there is a using*Response() method + * that return the *Response class containing specific operations related to the specific response + * the *Response class contains also the request*() method to apply the next GET request from a link in the response. + */ +public class ScmRequests { + + private String url; + private String username; + private String password; + + public static ScmRequests start() { + return new ScmRequests(); + } + + public Given given() { + return new Given(); + } + + + /** + * Apply a GET Request to the extracted url from the given link + * + * @param linkPropertyName the property name of link + * @param response the response containing the link + * @return the response of the GET request using the given link + */ + private Response applyGETRequestFromLink(Response response, String linkPropertyName) { + return applyGETRequest(response + .then() + .extract() + .path(linkPropertyName)); + } + + + /** + * Apply a GET Request to the given url and return the response. + * + * @param url the url of the GET request + * @return the response of the GET request using the given url + */ + private Response applyGETRequest(String url) { + return RestAssured.given() + .auth().preemptive().basic(username, password) + .when() + .get(url); + } + + + /** + * Apply a PUT Request to the extracted url from the given link + * + * @param response the response containing the link + * @param linkPropertyName the property name of link + * @param body + * @return the response of the PUT request using the given link + */ + private Response applyPUTRequestFromLink(Response response, String linkPropertyName, String content, String body) { + return applyPUTRequest(response + .then() + .extract() + .path(linkPropertyName), content, body); + } + + + /** + * Apply a PUT Request to the given url and return the response. + * + * @param url the url of the PUT request + * @param mediaType + * @param body + * @return the response of the PUT request using the given url + */ + private Response applyPUTRequest(String url, String mediaType, String body) { + return RestAssured.given() + .auth().preemptive().basic(username, password) + .when() + .contentType(mediaType) + .accept(mediaType) + .body(body) + .put(url); + } + + + private void setUrl(String url) { + this.url = url; + } + + private void setUsername(String username) { + this.username = username; + } + + private void setPassword(String password) { + this.password = password; + } + + private String getUrl() { + return url; + } + + private String getUsername() { + return username; + } + + private String getPassword() { + return password; + } + + public class Given { + + public GivenUrl url(String url) { + setUrl(url); + return new GivenUrl(); + } + + public GivenUrl url(URI url) { + setUrl(url.toString()); + return new GivenUrl(); + } + + } + + public class GivenWithUrlAndAuth { + public AppliedMeRequest getMeResource() { + return new AppliedMeRequest(applyGETRequest(url)); + } + + public AppliedUserRequest getUserResource() { + return new AppliedUserRequest(applyGETRequest(url)); + } + + public AppliedRepositoryRequest getRepositoryResource() { + return new AppliedRepositoryRequest( + applyGETRequest(url) + ); + } + } + + public class AppliedRequest { + private Response response; + + public AppliedRequest(Response response) { + this.response = response; + } + + /** + * apply custom assertions to the actual response + * + * @param consumer consume the response in order to assert the content. the header, the payload etc.. + * @return the self object + */ + public SELF assertResponse(Consumer consumer) { + consumer.accept(response); + return (SELF) this; + } + + /** + * special assertion of the status code + * + * @param expectedStatusCode the expected status code + * @return the self object + */ + public SELF assertStatusCode(int expectedStatusCode) { + this.response.then().assertThat().statusCode(expectedStatusCode); + return (SELF) this; + } + + } + + public class AppliedRepositoryRequest extends AppliedRequest { + + public AppliedRepositoryRequest(Response response) { + super(response); + } + + public RepositoryResponse usingRepositoryResponse() { + return new RepositoryResponse(super.response); + } + } + + public class RepositoryResponse { + + private Response repositoryResponse; + + public RepositoryResponse(Response repositoryResponse) { + this.repositoryResponse = repositoryResponse; + } + + public AppliedSourcesRequest requestSources() { + return new AppliedSourcesRequest(applyGETRequestFromLink(repositoryResponse, "_links.sources.href")); + } + + public AppliedChangesetsRequest requestChangesets() { + return new AppliedChangesetsRequest(applyGETRequestFromLink(repositoryResponse, "_links.changesets.href")); + } + } + + public class AppliedChangesetsRequest extends AppliedRequest { + + public AppliedChangesetsRequest(Response response) { + super(response); + } + + public ChangesetsResponse usingChangesetsResponse() { + return new ChangesetsResponse(super.response); + } + } + + public class ChangesetsResponse { + private Response changesetsResponse; + + public ChangesetsResponse(Response changesetsResponse) { + this.changesetsResponse = changesetsResponse; + } + + public ChangesetsResponse assertChangesets(Consumer> changesetsConsumer) { + List changesets = changesetsResponse.then().extract().path("_embedded.changesets"); + changesetsConsumer.accept(changesets); + return this; + } + + public AppliedDiffRequest requestDiff(String revision) { + return new AppliedDiffRequest(applyGETRequestFromLink(changesetsResponse, "_embedded.changesets.find{it.id=='" + revision + "'}._links.diff.href")); + } + + public AppliedModificationsRequest requestModifications(String revision) { + return new AppliedModificationsRequest(applyGETRequestFromLink(changesetsResponse, "_embedded.changesets.find{it.id=='" + revision + "'}._links.modifications.href")); + } + } + + public class AppliedSourcesRequest extends AppliedRequest { + + public AppliedSourcesRequest(Response sourcesResponse) { + super(sourcesResponse); + } + + public SourcesResponse usingSourcesResponse() { + return new SourcesResponse(super.response); + } + } + + public class SourcesResponse { + + private Response sourcesResponse; + + public SourcesResponse(Response sourcesResponse) { + this.sourcesResponse = sourcesResponse; + } + + public SourcesResponse assertRevision(Consumer assertRevision) { + String revision = sourcesResponse.then().extract().path("revision"); + assertRevision.accept(revision); + return this; + } + + public SourcesResponse assertFiles(Consumer assertFiles) { + List files = sourcesResponse.then().extract().path("files"); + assertFiles.accept(files); + return this; + } + + public AppliedChangesetsRequest requestFileHistory(String fileName) { + return new AppliedChangesetsRequest(applyGETRequestFromLink(sourcesResponse, "_embedded.files.find{it.name=='" + fileName + "'}._links.history.href")); + } + + public AppliedSourcesRequest requestSelf(String fileName) { + return new AppliedSourcesRequest(applyGETRequestFromLink(sourcesResponse, "_embedded.files.find{it.name=='" + fileName + "'}._links.self.href")); + } + } + + public class AppliedDiffRequest extends AppliedRequest { + + public AppliedDiffRequest(Response response) { + super(response); + } + } + + public class GivenUrl { + + public GivenWithUrlAndAuth usernameAndPassword(String username, String password) { + setUsername(username); + setPassword(password); + return new GivenWithUrlAndAuth(); + } + } + + public class AppliedModificationsRequest extends AppliedRequest { + public AppliedModificationsRequest(Response response) { + super(response); + } + + public ModificationsResponse usingModificationsResponse() { + return new ModificationsResponse(super.response); + } + + } + + public class ModificationsResponse { + private Response resource; + + public ModificationsResponse(Response resource) { + this.resource = resource; + } + + public ModificationsResponse assertRevision(Consumer assertRevision) { + String revision = resource.then().extract().path("revision"); + assertRevision.accept(revision); + return this; + } + + public ModificationsResponse assertAdded(Consumer> assertAdded) { + List added = resource.then().extract().path("added"); + assertAdded.accept(added); + return this; + } + + public ModificationsResponse assertRemoved(Consumer> assertRemoved) { + List removed = resource.then().extract().path("removed"); + assertRemoved.accept(removed); + return this; + } + + public ModificationsResponse assertModified(Consumer> assertModified) { + List modified = resource.then().extract().path("modified"); + assertModified.accept(modified); + return this; + } + + } + + public class AppliedMeRequest extends AppliedRequest { + + public AppliedMeRequest(Response response) { + super(response); + } + + public MeResponse usingMeResponse() { + return new MeResponse(super.response); + } + + } + + public class MeResponse extends UserResponse { + + + public MeResponse(Response response) { + super(response); + } + + public AppliedChangePasswordRequest requestChangePassword(String oldPassword, String newPassword) { + return new AppliedChangePasswordRequest(applyPUTRequestFromLink(super.response, "_links.password.href", VndMediaType.PASSWORD_CHANGE, createPasswordChangeJson(oldPassword, newPassword))); + } + + + } + + public class UserResponse extends ModelResponse { + + public static final String LINKS_PASSWORD_HREF = "_links.password.href"; + + public UserResponse(Response response) { + super(response); + } + + public SELF assertPassword(Consumer assertPassword) { + return super.assertSingleProperty(assertPassword, "password"); + } + + public SELF assertType(Consumer assertType) { + return assertSingleProperty(assertType, "type"); + } + + public SELF assertAdmin(Consumer assertAdmin) { + return assertSingleProperty(assertAdmin, "admin"); + } + + public SELF assertPasswordLinkDoesNotExists() { + return assertPropertyPathDoesNotExists(LINKS_PASSWORD_HREF); + } + + public SELF assertPasswordLinkExists() { + return assertPropertyPathExists(LINKS_PASSWORD_HREF); + } + + public AppliedChangePasswordRequest requestChangePassword(String newPassword) { + return new AppliedChangePasswordRequest(applyPUTRequestFromLink(super.response, LINKS_PASSWORD_HREF, VndMediaType.PASSWORD_CHANGE, createPasswordChangeJson(null, newPassword))); + } + + } + + + /** + * encapsulate standard assertions over model properties + */ + public class ModelResponse { + + protected Response response; + + public ModelResponse(Response response) { + this.response = response; + } + + public SELF assertSingleProperty(Consumer assertSingleProperty, String propertyJsonPath) { + T propertyValue = response.then().extract().path(propertyJsonPath); + assertSingleProperty.accept(propertyValue); + return (SELF) this; + } + + public SELF assertPropertyPathExists(String propertyJsonPath) { + response.then().assertThat().body("any { it.containsKey('" + propertyJsonPath + "')}", is(true)); + return (SELF) this; + } + + public SELF assertPropertyPathDoesNotExists(String propertyJsonPath) { + response.then().assertThat().body("this.any { it.containsKey('" + propertyJsonPath + "')}", is(false)); + return (SELF) this; + } + + public SELF assertArrayProperty(Consumer assertProperties, String propertyJsonPath) { + List properties = response.then().extract().path(propertyJsonPath); + assertProperties.accept(properties); + return (SELF) this; + } + } + + public class AppliedChangePasswordRequest extends AppliedRequest { + + public AppliedChangePasswordRequest(Response response) { + super(response); + } + + } + + public class AppliedUserRequest extends AppliedRequest { + + public AppliedUserRequest(Response response) { + super(response); + } + + public UserResponse usingUserResponse() { + return new UserResponse(super.response); + } + + } +} diff --git a/scm-it/src/test/java/sonia/scm/it/ScmTypes.java b/scm-it/src/test/java/sonia/scm/it/utils/ScmTypes.java similarity index 72% rename from scm-it/src/test/java/sonia/scm/it/ScmTypes.java rename to scm-it/src/test/java/sonia/scm/it/utils/ScmTypes.java index e8ba67e561..4c9ac0ea44 100644 --- a/scm-it/src/test/java/sonia/scm/it/ScmTypes.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/ScmTypes.java @@ -1,12 +1,12 @@ -package sonia.scm.it; +package sonia.scm.it.utils; import sonia.scm.util.IOUtil; import java.util.ArrayList; import java.util.Collection; -class ScmTypes { - static Collection availableScmTypes() { +public class ScmTypes { + public static Collection availableScmTypes() { Collection params = new ArrayList<>(); params.add("git"); diff --git a/scm-it/src/test/java/sonia/scm/it/TestData.java b/scm-it/src/test/java/sonia/scm/it/utils/TestData.java similarity index 77% rename from scm-it/src/test/java/sonia/scm/it/TestData.java rename to scm-it/src/test/java/sonia/scm/it/utils/TestData.java index ae0e35004d..03da80ea3b 100644 --- a/scm-it/src/test/java/sonia/scm/it/TestData.java +++ b/scm-it/src/test/java/sonia/scm/it/utils/TestData.java @@ -1,4 +1,4 @@ -package sonia.scm.it; +package sonia.scm.it.utils; import io.restassured.response.ValidatableResponse; import org.apache.http.HttpStatus; @@ -8,14 +8,16 @@ import sonia.scm.repository.PermissionType; import sonia.scm.web.VndMediaType; import javax.json.Json; +import javax.json.JsonObjectBuilder; +import java.net.URI; import java.util.HashMap; import java.util.List; import java.util.Map; import static java.util.Arrays.asList; -import static sonia.scm.it.RestUtil.createResourceUrl; -import static sonia.scm.it.RestUtil.given; -import static sonia.scm.it.ScmTypes.availableScmTypes; +import static sonia.scm.it.utils.RestUtil.createResourceUrl; +import static sonia.scm.it.utils.RestUtil.given; +import static sonia.scm.it.utils.ScmTypes.availableScmTypes; public class TestData { @@ -26,6 +28,7 @@ public class TestData { private static final List PROTECTED_USERS = asList(USER_SCM_ADMIN, USER_ANONYMOUS); private static Map DEFAULT_REPOSITORIES = new HashMap<>(); + public static final JsonObjectBuilder JSON_BUILDER = NullAwareJsonObjectBuilder.wrap(Json.createObjectBuilder()); public static void createDefault() { cleanup(); @@ -44,27 +47,31 @@ public class TestData { } public static void createUser(String username, String password) { + createUser(username, password, false, "xml"); + } + + public static void createUser(String username, String password, boolean isAdmin, String type) { LOG.info("create user with username: {}", username); + String admin = isAdmin ? "true" : "false"; given(VndMediaType.USER) .when() - .content(" {\n" + - " \"active\": true,\n" + - " \"admin\": false,\n" + - " \"creationDate\": \"2018-08-21T12:26:46.084Z\",\n" + - " \"displayName\": \"" + username + "\",\n" + - " \"mail\": \"user1@scm-manager.org\",\n" + - " \"name\": \"" + username + "\",\n" + - " \"password\": \"" + password + "\",\n" + - " \"type\": \"xml\"\n" + - " \n" + - " }") - .post(createResourceUrl("users")) + .content(new StringBuilder() + .append(" {\n") + .append(" \"active\": true,\n") + .append(" \"admin\": ").append(admin).append(",\n") + .append(" \"creationDate\": \"2018-08-21T12:26:46.084Z\",\n") + .append(" \"displayName\": \"").append(username).append("\",\n") + .append(" \"mail\": \"user1@scm-manager.org\",\n") + .append(" \"name\": \"").append(username).append("\",\n") + .append(" \"password\": \"").append(password).append("\",\n") + .append(" \"type\": \"").append(type).append("\"\n") + .append(" }").toString()) + .post(getUsersUrl()) .then() .statusCode(HttpStatus.SC_CREATED) ; } - public static void createUserPermission(String name, PermissionType permissionType, String repositoryType) { String defaultPermissionUrl = TestData.getDefaultPermissionUrl(USER_SCM_ADMIN, USER_SCM_ADMIN, repositoryType); LOG.info("create permission with name {} and type: {} using the endpoint: {}", name, permissionType, defaultPermissionUrl); @@ -183,7 +190,7 @@ public class TestData { } public static String repositoryJson(String repositoryType) { - return Json.createObjectBuilder() + return JSON_BUILDER .add("contact", "zaphod.beeblebrox@hitchhiker.com") .add("description", "Heart of Gold") .add("name", "HeartOfGold-" + repositoryType) @@ -192,6 +199,29 @@ public class TestData { .build().toString(); } + public static URI getMeUrl() { + return RestUtil.createResourceUrl("me/"); + + } + + public static URI getUsersUrl() { + return RestUtil.createResourceUrl("users/"); + + } + + public static URI getUserUrl(String username) { + return getUsersUrl().resolve(username); + + } + + + public static String createPasswordChangeJson(String oldPassword, String newPassword) { + return JSON_BUILDER + .add("oldPassword", oldPassword) + .add("newPassword", newPassword) + .build().toString(); + } + public static void main(String[] args) { cleanup(); } 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 index e83be3e386..e9bb5304a5 100644 --- 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 @@ -1,5 +1,7 @@ package sonia.scm.api.v2.resources; +import sonia.scm.user.ChangePasswordNotAllowedException; + import javax.ws.rs.core.Response; import javax.ws.rs.ext.ExceptionMapper; import javax.ws.rs.ext.Provider; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java new file mode 100644 index 0000000000..eda850d6ce --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java @@ -0,0 +1,17 @@ +package sonia.scm.api.v2.resources; + +import sonia.scm.user.InvalidPasswordException; + +import javax.ws.rs.core.Response; +import javax.ws.rs.ext.ExceptionMapper; +import javax.ws.rs.ext.Provider; + +@Provider +public class InvalidPasswordExceptionMapper implements ExceptionMapper { + @Override + public Response toResponse(InvalidPasswordException exception) { + return Response.status(Response.Status.UNAUTHORIZED) + .entity(exception.getMessage()) + .build(); + } +} diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java index bebd7def23..a600d1f45a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MapperModule.java @@ -8,6 +8,7 @@ public class MapperModule extends AbstractModule { @Override protected void configure() { bind(UserDtoToUserMapper.class).to(Mappers.getMapper(UserDtoToUserMapper.class).getClass()); + bind(MeToUserDtoMapper.class).to(Mappers.getMapper(MeToUserDtoMapper.class).getClass()); bind(UserToUserDtoMapper.class).to(Mappers.getMapper(UserToUserDtoMapper.class).getClass()); bind(UserCollectionToDtoMapper.class); 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 2967c0be4e..da9489a4bb 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 @@ -4,7 +4,10 @@ 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.NotFoundException; +import sonia.scm.user.InvalidPasswordException; import sonia.scm.user.User; import sonia.scm.user.UserManager; import sonia.scm.web.VndMediaType; @@ -19,6 +22,7 @@ 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; /** @@ -26,15 +30,20 @@ import javax.ws.rs.core.UriInfo; */ @Path(MeResource.ME_PATH_V2) public class MeResource { - static final String ME_PATH_V2 = "v2/me/"; + public static final String ME_PATH_V2 = "v2/me/"; - private final UserToUserDtoMapper userToDtoMapper; + private final MeToUserDtoMapper meToUserDtoMapper; private final IdResourceManagerAdapter adapter; + private final PasswordService passwordService; + private final UserManager userManager; + @Inject - public MeResource(UserToUserDtoMapper userToDtoMapper, UserManager manager) { - this.userToDtoMapper = userToDtoMapper; + public MeResource(MeToUserDtoMapper meToUserDtoMapper, UserManager manager, PasswordService passwordService) { + this.meToUserDtoMapper = meToUserDtoMapper; this.adapter = new IdResourceManagerAdapter<>(manager, User.class); + this.passwordService = passwordService; + this.userManager = manager; } /** @@ -52,7 +61,7 @@ public class MeResource { public Response get(@Context Request request, @Context UriInfo uriInfo) throws NotFoundException { String id = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); - return adapter.get(id, userToDtoMapper::map); + return adapter.get(id, meToUserDtoMapper::map); } /** @@ -67,8 +76,19 @@ public class MeResource { }) @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); + public Response changePassword(PasswordChangeDto passwordChangeDto) throws NotFoundException, ConcurrentModificationException { + String name = (String) SecurityUtils.getSubject().getPrincipals().getPrimaryPrincipal(); + return adapter.update(name, user -> user.changePassword(passwordService.encryptPassword(passwordChangeDto.getNewPassword())), userManager.getUserTypeChecker().andThen(getOldOriginalPasswordChecker(passwordChangeDto.getOldPassword()))); + } + + /** + * Match given old password from the dto with the stored password before updating + */ + private Consumer getOldOriginalPasswordChecker(String oldPassword) { + return user -> { + if (!user.getPassword().equals(passwordService.encryptPassword(oldPassword))) { + throw new InvalidPasswordException("The password is invalid"); + } + }; } } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java new file mode 100644 index 0000000000..b79dd9a766 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java @@ -0,0 +1,41 @@ +package sonia.scm.api.v2.resources; + +import de.otto.edison.hal.Links; +import org.mapstruct.AfterMapping; +import org.mapstruct.Mapper; +import org.mapstruct.MappingTarget; +import sonia.scm.user.User; +import sonia.scm.user.UserManager; +import sonia.scm.user.UserPermissions; + +import javax.inject.Inject; + +import static de.otto.edison.hal.Link.link; +import static de.otto.edison.hal.Links.linkingTo; + +@Mapper +public abstract class MeToUserDtoMapper extends UserToUserDtoMapper{ + + @Inject + private UserManager userManager; + + @Inject + private ResourceLinks resourceLinks; + + + @AfterMapping + void appendLinks(User user, @MappingTarget UserDto target) { + Links.Builder linksBuilder = linkingTo().self(resourceLinks.me().self()); + if (UserPermissions.delete(user).isPermitted()) { + linksBuilder.single(link("delete", resourceLinks.me().delete(target.getName()))); + } + if (UserPermissions.modify(user).isPermitted()) { + linksBuilder.single(link("update", resourceLinks.me().update(target.getName()))); + } + if (userManager.isTypeDefault(user)) { + linksBuilder.single(link("password", resourceLinks.me().passwordChange())); + } + target.add(linksBuilder.build()); + } + +} 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 1faa38a10c..f1f7b80ba8 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 @@ -92,6 +92,37 @@ class ResourceLinks { } } + MeLinks me() { + return new MeLinks(uriInfoStore.get(), this.user()); + } + + static class MeLinks { + private final LinkBuilder meLinkBuilder; + private UserLinks userLinks; + + MeLinks(UriInfo uriInfo, UserLinks user) { + meLinkBuilder = new LinkBuilder(uriInfo, MeResource.class); + userLinks = user; + } + + String self() { + return meLinkBuilder.method("get").parameters().href(); + } + + String delete(String name) { + return userLinks.delete(name); + } + + String update(String name) { + return userLinks.update(name); + } + + public String passwordChange() { + return meLinkBuilder.method("changePassword").parameters().href(); + } + } + + UserCollectionLinks userCollection() { return new UserCollectionLinks(uriInfoStore.get()); } 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 126d738e1e..714ca432ee 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 @@ -20,8 +20,6 @@ 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 { @@ -130,18 +128,7 @@ public class UserResource { }) @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()); + return adapter.update(name, user -> user.changePassword(passwordService.encryptPassword(passwordChangeDto.getNewPassword())), userManager.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 75566c1c3b..d23f8de744 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 @@ -45,7 +45,7 @@ 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())) { + if (userManager.isTypeDefault(user)) { 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 dc6a4095df..a1abdb6ff4 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 @@ -16,6 +16,7 @@ public class DispatcherMock { dispatcher.getProviderFactory().registerProvider(ConcurrentModificationExceptionMapper.class); dispatcher.getProviderFactory().registerProvider(InternalRepositoryExceptionMapper.class); dispatcher.getProviderFactory().registerProvider(ChangePasswordNotAllowedExceptionMapper.class); + dispatcher.getProviderFactory().registerProvider(InvalidPasswordExceptionMapper.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 340533b5d2..b0109d86b7 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 @@ -2,8 +2,8 @@ package sonia.scm.api.v2.resources; import com.github.sdorra.shiro.ShiroRule; import com.github.sdorra.shiro.SubjectAware; +import org.apache.shiro.authc.credential.PasswordService; import org.jboss.resteasy.core.Dispatcher; -import org.jboss.resteasy.mock.MockDispatcherFactory; import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; import org.junit.Before; @@ -23,11 +23,17 @@ import java.net.URISyntaxException; import static org.junit.Assert.assertEquals; 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.verify; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; +import static sonia.scm.api.v2.resources.DispatcherMock.createDispatcher; @SubjectAware( + username = "trillian", + password = "secret", configuration = "classpath:sonia/scm/repository/shiro.ini" ) public class MeResourceTest { @@ -35,8 +41,7 @@ public class MeResourceTest { @Rule public ShiroRule shiro = new ShiroRule(); - private Dispatcher dispatcher = MockDispatcherFactory.createDispatcher(); - + private Dispatcher dispatcher; private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(URI.create("/")); @Mock @@ -48,21 +53,27 @@ public class MeResourceTest { private UserManager userManager; @InjectMocks - private UserToUserDtoMapperImpl userToDtoMapper; + private MeToUserDtoMapperImpl userToDtoMapper; private ArgumentCaptor userCaptor = ArgumentCaptor.forClass(User.class); + @Mock + private PasswordService passwordService; + private User originalUser; + @Before public void prepareEnvironment() throws Exception { initMocks(this); - createDummyUser("trillian"); + originalUser = createDummyUser("trillian"); when(userManager.create(userCaptor.capture())).thenAnswer(invocation -> invocation.getArguments()[0]); doNothing().when(userManager).modify(userCaptor.capture()); doNothing().when(userManager).delete(userCaptor.capture()); + when(userManager.isTypeDefault(userCaptor.capture())).thenCallRealMethod(); + when(userManager.getUserTypeChecker()).thenCallRealMethod(); when(userManager.getDefaultType()).thenReturn("xml"); userToDtoMapper.setResourceLinks(resourceLinks); - MeResource meResource = new MeResource(userToDtoMapper, userManager); - dispatcher.getRegistry().addSingletonResource(meResource); + MeResource meResource = new MeResource(userToDtoMapper, userManager, passwordService); + dispatcher = createDispatcher(meResource); when(uriInfo.getBaseUri()).thenReturn(URI.create("/")); when(uriInfoStore.get()).thenReturn(uriInfo); } @@ -79,13 +90,77 @@ public class MeResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"trillian\"")); assertTrue(response.getContentAsString().contains("\"password\":null")); - assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/trillian\"}")); + assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/me/\"}")); assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/trillian\"}")); } + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldEncryptPasswordBeforeChanging() throws Exception { + String newPassword = "pwd123"; + String encryptedNewPassword = "encrypted123"; + String oldPassword = "notEncriptedSecret"; + 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(eq(newPassword))).thenReturn(encryptedNewPassword); + when(passwordService.encryptPassword(eq(oldPassword))).thenReturn("secret"); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_NO_CONTENT, response.getStatus()); + verify(userManager).modify(any(User.class)); + User updatedUser = userCaptor.getValue(); + assertEquals(encryptedNewPassword, updatedUser.getPassword()); + } + + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception { + originalUser.setType("not an xml type"); + String newPassword = "pwd123"; + String oldPassword = "notEncriptedSecret"; + 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("secret"); + + dispatcher.invoke(request, response); + + assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus()); + } + + @Test + @SubjectAware(username = "trillian", password = "secret") + public void shouldGet401OnChangePasswordIfOldPasswordDoesNotMatchOriginalPassword() throws Exception { + String newPassword = "pwd123"; + String oldPassword = "notEncriptedSecret"; + 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_UNAUTHORIZED, response.getStatus()); + } + + private User createDummyUser(String name) { User user = new User(); user.setName(name); + user.setType("xml"); user.setPassword("secret"); user.setCreationDate(System.currentTimeMillis()); when(userManager.get(name)).thenReturn(user); diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java new file mode 100644 index 0000000000..4f40098da5 --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/MeToUserDtoMapperTest.java @@ -0,0 +1,135 @@ +package sonia.scm.api.v2.resources; + +import org.apache.shiro.subject.Subject; +import org.apache.shiro.subject.support.SubjectThreadState; +import org.apache.shiro.util.ThreadContext; +import org.apache.shiro.util.ThreadState; +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.mockito.InjectMocks; +import org.mockito.Mock; +import sonia.scm.user.User; +import sonia.scm.user.UserManager; + +import java.net.URI; + +import static org.assertj.core.api.Assertions.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.mockito.ArgumentMatchers.eq; +import static org.mockito.Mockito.mock; +import static org.mockito.Mockito.when; +import static org.mockito.MockitoAnnotations.initMocks; + +public class MeToUserDtoMapperTest { + + private final URI baseUri = URI.create("http://example.com/base/"); + @SuppressWarnings("unused") // Is injected + private final ResourceLinks resourceLinks = ResourceLinksMock.createMock(baseUri); + + @Mock + private UserManager userManager; + + @InjectMocks + private MeToUserDtoMapperImpl mapper; + + private final Subject subject = mock(Subject.class); + private final ThreadState subjectThreadState = new SubjectThreadState(subject); + + private URI expectedBaseUri; + private URI expectedUserBaseUri; + + @Before + public void init() { + initMocks(this); + when(userManager.getDefaultType()).thenReturn("xml"); + expectedBaseUri = baseUri.resolve(MeResource.ME_PATH_V2 + "/"); + expectedUserBaseUri = baseUri.resolve(UserRootResource.USERS_PATH_V2 + "/"); + subjectThreadState.bind(); + ThreadContext.bind(subject); + } + + @After + public void unbindSubject() { + ThreadContext.unbindSubject(); + } + + @Test + public void shouldMapTheUpdateLink() { + User user = createDefaultUser(); + when(subject.isPermitted("user:modify:abc")).thenReturn(true); + + UserDto userDto = mapper.map(user); + assertEquals("expected update link", expectedUserBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("update").get().getHref()); + + when(subject.isPermitted("user:modify:abc")).thenReturn(false); + userDto = mapper.map(user); + assertFalse("expected no update link", userDto.getLinks().getLinkBy("update").isPresent()); + } + + @Test + public void shouldMapTheSelfLink() { + User user = createDefaultUser(); + when(subject.isPermitted("user:modify:abc")).thenReturn(true); + + UserDto userDto = mapper.map(user); + assertEquals("expected self link", expectedBaseUri.toString(), userDto.getLinks().getLinkBy("self").get().getHref()); + + } + + @Test + public void shouldMapTheDeleteLink() { + User user = createDefaultUser(); + when(subject.isPermitted("user:delete:abc")).thenReturn(true); + + UserDto userDto = mapper.map(user); + assertEquals("expected update link", expectedUserBaseUri.resolve("abc").toString(), userDto.getLinks().getLinkBy("delete").get().getHref()); + + when(subject.isPermitted("user:delete:abc")).thenReturn(false); + userDto = mapper.map(user); + assertFalse("expected no delete link", userDto.getLinks().getLinkBy("delete").isPresent()); + } + + @Test + public void shouldGetPasswordLinkOnlyForDefaultUserType() { + User user = createDefaultUser(); + when(subject.isPermitted("user:modify:abc")).thenReturn(true); + when(userManager.isTypeDefault(eq(user))).thenReturn(true); + + UserDto userDto = mapper.map(user); + + assertEquals("expected password link with modify permission", expectedBaseUri.resolve("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("password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); + + when(userManager.isTypeDefault(eq(user))).thenReturn(false); + + 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()).as("hide password for the me resource").isBlank(); + } + + private User createDefaultUser() { + User user = new User(); + user.setName("abc"); + user.setCreationDate(1L); + return user; + } + + +} diff --git a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java index 1d0fac68e3..5e5897eb2a 100644 --- a/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java +++ b/scm-webapp/src/test/java/sonia/scm/api/v2/resources/ResourceLinksMock.java @@ -13,7 +13,9 @@ public class ResourceLinksMock { UriInfo uriInfo = mock(UriInfo.class); when(uriInfo.getBaseUri()).thenReturn(baseUri); - when(resourceLinks.user()).thenReturn(new ResourceLinks.UserLinks(uriInfo)); + ResourceLinks.UserLinks userLinks = new ResourceLinks.UserLinks(uriInfo); + when(resourceLinks.user()).thenReturn(userLinks); + when(resourceLinks.me()).thenReturn(new ResourceLinks.MeLinks(uriInfo,userLinks)); when(resourceLinks.userCollection()).thenReturn(new ResourceLinks.UserCollectionLinks(uriInfo)); when(resourceLinks.group()).thenReturn(new ResourceLinks.GroupLinks(uriInfo)); when(resourceLinks.groupCollection()).thenReturn(new ResourceLinks.GroupCollectionLinks(uriInfo)); 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 d75fd606ec..35cd755a5d 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 @@ -68,6 +68,8 @@ public class UserRootResourceTest { initMocks(this); originalUser = createDummyUser("Neo"); when(userManager.create(userCaptor.capture())).thenAnswer(invocation -> invocation.getArguments()[0]); + when(userManager.isTypeDefault(userCaptor.capture())).thenCallRealMethod(); + when(userManager.getUserTypeChecker()).thenCallRealMethod(); doNothing().when(userManager).modify(userCaptor.capture()); doNothing().when(userManager).delete(userCaptor.capture()); when(userManager.getDefaultType()).thenReturn("xml"); @@ -114,7 +116,7 @@ public class UserRootResourceTest { @Test public void shouldEncryptPasswordBeforeChanging() throws Exception { String newPassword = "pwd123"; - String content = MessageFormat.format("'{'\"newPassword\": \"{0}\"'}'", newPassword); + String content = String.format("{\"newPassword\": \"%s\"}", newPassword); MockHttpRequest request = MockHttpRequest .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") .contentType(VndMediaType.PASSWORD_CHANGE) @@ -134,7 +136,7 @@ public class UserRootResourceTest { public void shouldGet400OnChangePasswordOfUserWithNonDefaultType() throws Exception { originalUser.setType("not an xml type"); String newPassword = "pwd123"; - String content = MessageFormat.format("'{'\"newPassword\": \"{0}\"'}'", newPassword); + String content = String.format("{\"newPassword\": \"%s\"}", newPassword); MockHttpRequest request = MockHttpRequest .put("/" + UserRootResource.USERS_PATH_V2 + "Neo/password") .contentType(VndMediaType.PASSWORD_CHANGE) 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 4c9388cd18..7570a3f162 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 @@ -18,6 +18,7 @@ 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.ArgumentMatchers.eq; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; import static org.mockito.MockitoAnnotations.initMocks; @@ -67,12 +68,17 @@ public class UserToUserDtoMapperTest { public void shouldGetPasswordLinkOnlyForDefaultUserType() { User user = createDefaultUser(); when(subject.isPermitted("user:modify:abc")).thenReturn(true); - user.setType("xml"); + when(userManager.isTypeDefault(eq(user))).thenReturn(true); UserDto userDto = mapper.map(user); - assertEquals("expected password link", expectedBaseUri.resolve("abc/password").toString(), userDto.getLinks().getLinkBy("password").get().getHref()); - user.setType("db"); + 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()); + + when(userManager.isTypeDefault(eq(user))).thenReturn(false); userDto = mapper.map(user); From dabce994d9d69df77595448b7c83a2d21683345b Mon Sep 17 00:00:00 2001 From: Johannes Schnatterer Date: Wed, 19 Sep 2018 15:25:22 +0000 Subject: [PATCH 05/12] Close branch feature/remove_escape_util From b80e9666aa4f4b451e7bb8f4c7eaabffbd26ed02 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 20 Sep 2018 11:51:10 +0200 Subject: [PATCH 06/12] review --- .../ChangePasswordNotAllowedException.java | 2 ++ .../scm/user/InvalidPasswordException.java | 2 ++ .../main/java/sonia/scm/user/UserManager.java | 4 +++- .../test/java/sonia/scm/it/UserITCase.java | 21 ++++++++++--------- .../InvalidPasswordExceptionMapper.java | 2 +- .../scm/api/v2/resources/MeResource.java | 4 +++- .../api/v2/resources/MeToUserDtoMapper.java | 1 + .../api/v2/resources/PasswordChangeDto.java | 10 +-------- .../scm/api/v2/resources/ResourceLinks.java | 4 ++-- .../api/v2/resources/UserDtoToUserMapper.java | 18 +++++++++------- .../scm/api/v2/resources/UserResource.java | 2 +- .../api/v2/resources/UserToUserDtoMapper.java | 5 ----- .../scm/api/v2/resources/MeResourceTest.java | 5 ++--- 13 files changed, 40 insertions(+), 40 deletions(-) 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 b2925cf65c..49e4714e80 100644 --- a/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java +++ b/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java @@ -2,6 +2,8 @@ package sonia.scm.user; public class ChangePasswordNotAllowedException extends RuntimeException { + public static final String WRON_USER_TYPE = "User of type {0} are not allowed to change password"; + public ChangePasswordNotAllowedException(String message) { super(message); } 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 beda2c83fb..8d25e01a1e 100644 --- a/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java +++ b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java @@ -2,6 +2,8 @@ package sonia.scm.user; public class InvalidPasswordException extends RuntimeException { + public static final String PASSWORD_NOT_MATCHED = "The given Password does not match with the stored one."; + public InvalidPasswordException(String message) { super(message); } 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 90dbcd74b3..8b46fcc893 100644 --- a/scm-core/src/main/java/sonia/scm/user/UserManager.java +++ b/scm-core/src/main/java/sonia/scm/user/UserManager.java @@ -41,6 +41,8 @@ import sonia.scm.search.Searchable; import java.text.MessageFormat; import java.util.function.Consumer; +import static sonia.scm.user.ChangePasswordNotAllowedException.WRON_USER_TYPE; + /** * The central class for managing {@link User} objects. * This class is a singleton and is available via injection. @@ -79,7 +81,7 @@ public interface UserManager default Consumer getUserTypeChecker() { return user -> { if (!isTypeDefault(user)) { - throw new ChangePasswordNotAllowedException(MessageFormat.format("It is not possible to change password for User of type {0}", user.getType())); + throw new ChangePasswordNotAllowedException(MessageFormat.format(WRON_USER_TYPE, user.getType())); } }; } 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 65ed6b71bb..67fe23dcbc 100644 --- a/scm-it/src/test/java/sonia/scm/it/UserITCase.java +++ b/scm-it/src/test/java/sonia/scm/it/UserITCase.java @@ -17,12 +17,15 @@ public class UserITCase { @Test public void adminShouldChangeOwnPassword() { - String newPassword = TestData.USER_SCM_ADMIN + "1"; + String newUser = "user"; + String password = "pass"; + TestData.createUser(newUser, password, true, "xml"); + String newPassword = "new_password"; // admin change the own password ScmRequests.start() .given() - .url(TestData.getUserUrl(TestData.USER_SCM_ADMIN)) - .usernameAndPassword(TestData.USER_SCM_ADMIN, TestData.USER_SCM_ADMIN) + .url(TestData.getUserUrl(newUser)) + .usernameAndPassword(newUser, password) .getUserResource() .assertStatusCode(200) .usingUserResponse() @@ -30,18 +33,16 @@ public class UserITCase { .assertPassword(Assert::assertNull) .requestChangePassword(newPassword) // the oldPassword is not needed in the user resource .assertStatusCode(204); - // assert password is changed -> login with the new Password than undo changes + // assert password is changed -> login with the new Password ScmRequests.start() .given() - .url(TestData.getUserUrl(TestData.USER_SCM_ADMIN)) - .usernameAndPassword(TestData.USER_SCM_ADMIN, newPassword) + .url(TestData.getUserUrl(newUser)) + .usernameAndPassword(newUser, newPassword) .getUserResource() .assertStatusCode(200) .usingUserResponse() - .assertAdmin(aBoolean -> assertThat(aBoolean).isEqualTo(Boolean.TRUE)) - .assertPassword(Assert::assertNull) - .requestChangePassword(TestData.USER_SCM_ADMIN) - .assertStatusCode(204); + .assertAdmin(isAdmin -> assertThat(isAdmin).isEqualTo(Boolean.TRUE)) + .assertPassword(Assert::assertNull); } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java index eda850d6ce..7c5364ba03 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/InvalidPasswordExceptionMapper.java @@ -10,7 +10,7 @@ import javax.ws.rs.ext.Provider; public class InvalidPasswordExceptionMapper implements ExceptionMapper { @Override public Response toResponse(InvalidPasswordException exception) { - return Response.status(Response.Status.UNAUTHORIZED) + return Response.status(Response.Status.BAD_REQUEST) .entity(exception.getMessage()) .build(); } 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 da9489a4bb..0100728220 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 @@ -24,6 +24,8 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.util.function.Consumer; +import static sonia.scm.user.InvalidPasswordException.PASSWORD_NOT_MATCHED; + /** * RESTful Web Service Resource to get currently logged in users. @@ -87,7 +89,7 @@ public class MeResource { private Consumer getOldOriginalPasswordChecker(String oldPassword) { return user -> { if (!user.getPassword().equals(passwordService.encryptPassword(oldPassword))) { - throw new InvalidPasswordException("The password is invalid"); + throw new InvalidPasswordException(PASSWORD_NOT_MATCHED); } }; } diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java index a4586fab36..2a872eadd9 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/MeToUserDtoMapper.java @@ -23,6 +23,7 @@ public abstract class MeToUserDtoMapper extends UserToUserDtoMapper{ private ResourceLinks resourceLinks; + @Override @AfterMapping protected void appendLinks(User user, @MappingTarget UserDto target) { Links.Builder linksBuilder = linkingTo().self(resourceLinks.me().self()); 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 2db96ccdf1..47ad6cd147 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 @@ -1,7 +1,5 @@ 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; @@ -10,16 +8,10 @@ import org.hibernate.validator.constraints.NotEmpty; @Getter @Setter @ToString -public class PasswordChangeDto extends HalRepresentation { +public class PasswordChangeDto { 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 1301a5bb94..e978de443a 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 @@ -100,8 +100,8 @@ class ResourceLinks { private final LinkBuilder meLinkBuilder; private UserLinks userLinks; - MeLinks(ScmPathInfo uriInfo, UserLinks user) { - meLinkBuilder = new LinkBuilder(uriInfo, MeResource.class); + MeLinks(ScmPathInfo pathInfo, UserLinks user) { + meLinkBuilder = new LinkBuilder(pathInfo, MeResource.class); userLinks = user; } 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 1ea07e4be7..e6608f0804 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,9 +1,10 @@ package sonia.scm.api.v2.resources; +import org.mapstruct.AfterMapping; import org.mapstruct.Context; import org.mapstruct.Mapper; import org.mapstruct.Mapping; -import org.mapstruct.Named; +import org.mapstruct.MappingTarget; import sonia.scm.user.User; // Mapstruct does not support parameterized (i.e. non-default) constructors. Thus, we need to use field injection. @@ -11,21 +12,24 @@ import sonia.scm.user.User; @Mapper public abstract class UserDtoToUserMapper extends BaseDtoMapper { - @Mapping(source = "password", target = "password", qualifiedByName = "getUsedPassword") @Mapping(target = "creationDate", ignore = true) public abstract User map(UserDto userDto, @Context String usedPassword); /** * depends on the use case the right password will be mapped. + * The given Password in the context parameter will be set. + * The mapper consumer have the control of what password should be set. + *

* 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 + * @param usedPassword the password to be set + * @param user the target */ - @Named("getUsedPassword") - String getUsedPassword(String password, @Context String usedPassword) { - return usedPassword; + @AfterMapping + void overridePassword(@MappingTarget User user, @Context String usedPassword) { + user.setPassword(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 714ca432ee..c0bb1e65ad 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 @@ -120,7 +120,7 @@ public class UserResource { @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 = 400, condition = "Invalid body, e.g. the user type is not xml or the given oldPassword do not match the stored one"), @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"), 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 e2e79e2902..832829883b 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 @@ -31,11 +31,6 @@ public abstract class UserToUserDtoMapper extends BaseMapper { @Inject private ResourceLinks resourceLinks; - @VisibleForTesting - void setResourceLinks(ResourceLinks resourceLinks) { - this.resourceLinks = resourceLinks; - } - @AfterMapping protected void appendLinks(User user, @MappingTarget UserDto target) { Links.Builder linksBuilder = linkingTo().self(resourceLinks.user().self(target.getName())); 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 1dc5049fd9..3a5caa63eb 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 @@ -70,7 +70,6 @@ public class MeResourceTest { when(userManager.isTypeDefault(userCaptor.capture())).thenCallRealMethod(); when(userManager.getUserTypeChecker()).thenCallRealMethod(); when(userManager.getDefaultType()).thenReturn("xml"); - userToDtoMapper.setResourceLinks(resourceLinks); MeResource meResource = new MeResource(userToDtoMapper, userManager, passwordService); when(uriInfo.getApiRestUri()).thenReturn(URI.create("/")); when(scmPathInfoStore.get()).thenReturn(uriInfo); @@ -138,7 +137,7 @@ public class MeResourceTest { @Test @SubjectAware(username = "trillian", password = "secret") - public void shouldGet401OnChangePasswordIfOldPasswordDoesNotMatchOriginalPassword() throws Exception { + public void shouldGet400OnChangePasswordIfOldPasswordDoesNotMatchOriginalPassword() throws Exception { String newPassword = "pwd123"; String oldPassword = "notEncriptedSecret"; String content = String.format("{ \"oldPassword\": \"%s\" , \"newPassword\": \"%s\" }", oldPassword, newPassword); @@ -152,7 +151,7 @@ public class MeResourceTest { dispatcher.invoke(request, response); - assertEquals(HttpServletResponse.SC_UNAUTHORIZED, response.getStatus()); + assertEquals(HttpServletResponse.SC_BAD_REQUEST, response.getStatus()); } From 2683dd651bd692ad118c160e200762472f8b02b8 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 20 Sep 2018 12:13:21 +0200 Subject: [PATCH 07/12] review --- .../sonia/scm/user/ChangePasswordNotAllowedException.java | 2 +- .../main/java/sonia/scm/user/InvalidPasswordException.java | 2 +- scm-core/src/main/java/sonia/scm/user/UserManager.java | 4 ++-- .../src/main/java/sonia/scm/api/v2/resources/MeResource.java | 4 ++-- 4 files changed, 6 insertions(+), 6 deletions(-) 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 49e4714e80..19c609ba30 100644 --- a/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java +++ b/scm-core/src/main/java/sonia/scm/user/ChangePasswordNotAllowedException.java @@ -2,7 +2,7 @@ package sonia.scm.user; public class ChangePasswordNotAllowedException extends RuntimeException { - public static final String WRON_USER_TYPE = "User of type {0} are not allowed to change password"; + public static final String WRONG_USER_TYPE = "User of type {0} are not allowed to change password"; public ChangePasswordNotAllowedException(String message) { super(message); 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 8d25e01a1e..3f4d1477f9 100644 --- a/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java +++ b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java @@ -2,7 +2,7 @@ package sonia.scm.user; public class InvalidPasswordException extends RuntimeException { - public static final String PASSWORD_NOT_MATCHED = "The given Password does not match with the stored one."; + public static final String PWD_NOT_MATCHED = "The given Password does not match with the stored one."; public InvalidPasswordException(String message) { super(message); 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 8b46fcc893..1f5aee1f19 100644 --- a/scm-core/src/main/java/sonia/scm/user/UserManager.java +++ b/scm-core/src/main/java/sonia/scm/user/UserManager.java @@ -41,7 +41,7 @@ import sonia.scm.search.Searchable; import java.text.MessageFormat; import java.util.function.Consumer; -import static sonia.scm.user.ChangePasswordNotAllowedException.WRON_USER_TYPE; +import static sonia.scm.user.ChangePasswordNotAllowedException.WRONG_USER_TYPE; /** * The central class for managing {@link User} objects. @@ -81,7 +81,7 @@ public interface UserManager default Consumer getUserTypeChecker() { return user -> { if (!isTypeDefault(user)) { - throw new ChangePasswordNotAllowedException(MessageFormat.format(WRON_USER_TYPE, user.getType())); + throw new ChangePasswordNotAllowedException(MessageFormat.format(WRONG_USER_TYPE, user.getType())); } }; } 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 0100728220..bcf63733aa 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 @@ -24,7 +24,7 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.util.function.Consumer; -import static sonia.scm.user.InvalidPasswordException.PASSWORD_NOT_MATCHED; +import static sonia.scm.user.InvalidPasswordException.PWD_NOT_MATCHED; /** @@ -89,7 +89,7 @@ public class MeResource { private Consumer getOldOriginalPasswordChecker(String oldPassword) { return user -> { if (!user.getPassword().equals(passwordService.encryptPassword(oldPassword))) { - throw new InvalidPasswordException(PASSWORD_NOT_MATCHED); + throw new InvalidPasswordException(PWD_NOT_MATCHED); } }; } From 484d5c68c8ef530e68297045c123d309e9d2aaf3 Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 20 Sep 2018 12:57:01 +0200 Subject: [PATCH 08/12] review --- .../main/java/sonia/scm/user/InvalidPasswordException.java | 2 +- .../src/main/java/sonia/scm/api/v2/resources/MeResource.java | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) 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 3f4d1477f9..e06191a8f2 100644 --- a/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java +++ b/scm-core/src/main/java/sonia/scm/user/InvalidPasswordException.java @@ -2,7 +2,7 @@ package sonia.scm.user; public class InvalidPasswordException extends RuntimeException { - public static final String PWD_NOT_MATCHED = "The given Password does not match with the stored one."; + public static final String INVALID_MATCHING = "The given Password does not match with the stored one."; public InvalidPasswordException(String message) { super(message); 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 bcf63733aa..e684bc25db 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 @@ -24,7 +24,7 @@ import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; import java.util.function.Consumer; -import static sonia.scm.user.InvalidPasswordException.PWD_NOT_MATCHED; +import static sonia.scm.user.InvalidPasswordException.INVALID_MATCHING; /** @@ -89,7 +89,7 @@ public class MeResource { private Consumer getOldOriginalPasswordChecker(String oldPassword) { return user -> { if (!user.getPassword().equals(passwordService.encryptPassword(oldPassword))) { - throw new InvalidPasswordException(PWD_NOT_MATCHED); + throw new InvalidPasswordException(INVALID_MATCHING); } }; } From a8aa69097c702ea4f6c72971d79189bf979db296 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 20 Sep 2018 11:17:32 +0000 Subject: [PATCH 09/12] Close branch feature/password_modification_v2 From 8b11263d478cac7f7fc384b3d0ceb54514e75f4d Mon Sep 17 00:00:00 2001 From: Mohamed Karray Date: Thu, 20 Sep 2018 17:06:34 +0200 Subject: [PATCH 10/12] SuppressWarnings --- scm-core/src/main/java/sonia/scm/web/VndMediaType.java | 3 ++- .../src/main/java/sonia/scm/api/v2/resources/UserDto.java | 1 + 2 files changed, 3 insertions(+), 1 deletion(-) 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 6d9958e0c7..3c6455ff0e 100644 --- a/scm-core/src/main/java/sonia/scm/web/VndMediaType.java +++ b/scm-core/src/main/java/sonia/scm/web/VndMediaType.java @@ -21,7 +21,7 @@ public class VndMediaType { public static final String PERMISSION = PREFIX + "permission" + SUFFIX; public static final String CHANGESET = PREFIX + "changeset" + SUFFIX; public static final String CHANGESET_COLLECTION = PREFIX + "changesetCollection" + SUFFIX; - public static final String MODIFICATIONS = PREFIX + "modifications" + SUFFIX;; + public static final String MODIFICATIONS = PREFIX + "modifications" + SUFFIX; public static final String TAG = PREFIX + "tag" + SUFFIX; public static final String TAG_COLLECTION = PREFIX + "tagCollection" + SUFFIX; public static final String BRANCH = PREFIX + "branch" + SUFFIX; @@ -35,6 +35,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; + @SuppressWarnings("all") public static final String PASSWORD_CHANGE = PREFIX + "passwordChange" + SUFFIX; public static final String ME = PREFIX + "me" + SUFFIX; diff --git a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java index 74f43c61c2..4e4345445a 100644 --- a/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java +++ b/scm-webapp/src/main/java/sonia/scm/api/v2/resources/UserDto.java @@ -26,6 +26,7 @@ public class UserDto extends HalRepresentation { private String mail; @Pattern(regexp = "^[A-z0-9\\.\\-_@]|[^ ]([A-z0-9\\.\\-_@ ]*[A-z0-9\\.\\-_@]|[^ ])?$") private String name; + @JsonInclude(JsonInclude.Include.NON_NULL) private String password; private String type; private Map properties; From ee10fbee42f11e2010825834d45aa2d835d2b6d0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 20 Sep 2018 21:55:39 +0200 Subject: [PATCH 11/12] Remove empty passwords from unit test asserts --- .../java/sonia/scm/api/v2/resources/UserRootResourceTest.java | 2 -- 1 file changed, 2 deletions(-) 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 2cd1e01989..065a33313f 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 @@ -94,7 +94,6 @@ public class UserRootResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"Neo\"")); - assertTrue(response.getContentAsString().contains("\"password\":null")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/Neo\"}")); assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/Neo\"}")); } @@ -109,7 +108,6 @@ public class UserRootResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"Neo\"")); - assertTrue(response.getContentAsString().contains("\"password\":null")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/users/Neo\"}")); assertFalse(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/Neo\"}")); } From cd621ded812de2541b7c1a05ef9202c24187edc1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ren=C3=A9=20Pfeuffer?= Date: Thu, 20 Sep 2018 21:58:17 +0200 Subject: [PATCH 12/12] Remove empty passwords from unit test asserts --- .../test/java/sonia/scm/api/v2/resources/MeResourceTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 3a5caa63eb..c7b040172e 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 @@ -7,6 +7,7 @@ import org.jboss.resteasy.core.Dispatcher; import org.jboss.resteasy.mock.MockHttpRequest; import org.jboss.resteasy.mock.MockHttpResponse; import org.junit.Before; +import org.junit.Ignore; import org.junit.Rule; import org.junit.Test; import org.mockito.ArgumentCaptor; @@ -87,7 +88,6 @@ public class MeResourceTest { assertEquals(HttpServletResponse.SC_OK, response.getStatus()); assertTrue(response.getContentAsString().contains("\"name\":\"trillian\"")); - assertTrue(response.getContentAsString().contains("\"password\":null")); assertTrue(response.getContentAsString().contains("\"self\":{\"href\":\"/v2/me/\"}")); assertTrue(response.getContentAsString().contains("\"delete\":{\"href\":\"/v2/users/trillian\"}")); }