From 89c559ae774ed4ac844d8534c63b03a205467d67 Mon Sep 17 00:00:00 2001 From: kolya-t Date: Tue, 20 Jun 2017 15:26:32 +0300 Subject: [PATCH 1/2] Add security tests for UserController --- pom.xml | 10 +- .../backend/controller/UserController.java | 5 +- .../teapot/backend/model/UserAuthority.java | 2 +- .../test/controller/AbstractControllerIT.java | 80 +++++++- .../test/controller/UserControllerIT.java | 186 +++++++++++++++--- 5 files changed, 246 insertions(+), 37 deletions(-) diff --git a/pom.xml b/pom.xml index a6c897b..afd087c 100644 --- a/pom.xml +++ b/pom.xml @@ -35,9 +35,6 @@ 5.2.10.Final - - 2.8.5 - 22.0 @@ -136,7 +133,6 @@ com.fasterxml.jackson.datatype jackson-datatype-jsr310 - ${jackson-datatype-jsr310.version} @@ -146,6 +142,12 @@ ${guava.version} + + org.springframework.security + spring-security-test + test + + diff --git a/src/main/java/org/teapot/backend/controller/UserController.java b/src/main/java/org/teapot/backend/controller/UserController.java index d744c1c..0067591 100644 --- a/src/main/java/org/teapot/backend/controller/UserController.java +++ b/src/main/java/org/teapot/backend/controller/UserController.java @@ -3,6 +3,7 @@ import com.google.common.primitives.Longs; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.data.domain.Pageable; +import org.springframework.http.HttpHeaders; import org.springframework.http.HttpStatus; import org.springframework.security.access.prepost.PreAuthorize; import org.springframework.security.core.Authentication; @@ -181,7 +182,7 @@ public User registerUser(@RequestBody User user, userRepository.save(user); } - response.setHeader("Location", "/users/" + user.getId()); + response.setHeader(HttpHeaders.LOCATION, "/users/" + user.getId()); return user; } @@ -245,7 +246,7 @@ public void patchUser(@PathVariable Long id, } else if (auth.getName().equals(user.getEmail())) { - if (username != null) user.setUsername(firstName); + if (username != null) user.setUsername(username); if ((available != null) && (!available)) user.setAvailable(false); if (firstName != null) user.setFirstName(firstName); if (lastName != null) user.setLastName(lastName); diff --git a/src/main/java/org/teapot/backend/model/UserAuthority.java b/src/main/java/org/teapot/backend/model/UserAuthority.java index 2a54665..36ffcd9 100644 --- a/src/main/java/org/teapot/backend/model/UserAuthority.java +++ b/src/main/java/org/teapot/backend/model/UserAuthority.java @@ -9,6 +9,6 @@ public enum UserAuthority implements GrantedAuthority { @Override public String getAuthority() { - return name(); + return "ROLE_" + name(); } } diff --git a/src/test/java/org/teapot/backend/test/controller/AbstractControllerIT.java b/src/test/java/org/teapot/backend/test/controller/AbstractControllerIT.java index 52997ca..fd0a976 100644 --- a/src/test/java/org/teapot/backend/test/controller/AbstractControllerIT.java +++ b/src/test/java/org/teapot/backend/test/controller/AbstractControllerIT.java @@ -2,36 +2,63 @@ import org.junit.Assert; import org.junit.Before; -import org.junit.runner.RunWith; import org.springframework.beans.factory.annotation.Autowired; +import org.springframework.boot.json.JacksonJsonParser; import org.springframework.boot.test.context.SpringBootTest; import org.springframework.http.MediaType; import org.springframework.http.converter.HttpMessageConverter; import org.springframework.http.converter.json.MappingJackson2HttpMessageConverter; import org.springframework.mock.http.MockHttpOutputMessage; -import org.springframework.test.context.junit4.SpringJUnit4ClassRunner; -import org.springframework.test.context.web.WebAppConfiguration; +import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.security.web.FilterChainProxy; import org.springframework.test.web.servlet.MockMvc; import org.springframework.test.web.servlet.setup.MockMvcBuilders; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.springframework.web.context.WebApplicationContext; +import org.teapot.backend.model.User; +import org.teapot.backend.model.UserAuthority; +import org.teapot.backend.repository.UserRepository; import org.teapot.backend.test.AbstractIT; -import javax.transaction.Transactional; import java.io.IOException; import java.nio.charset.Charset; import java.util.Arrays; +import static org.springframework.security.test.web.servlet.request.SecurityMockMvcRequestPostProcessors.httpBasic; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; +import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.status; + +@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) public abstract class AbstractControllerIT extends AbstractIT { + private static final String CLIENT_ID = "client"; + private static final String CLIENT_SECRET = "secret"; + protected MediaType contentType = new MediaType(MediaType.APPLICATION_JSON.getType(), MediaType.APPLICATION_JSON.getSubtype(), Charset.forName("utf8")); protected MockMvc mockMvc; + protected JacksonJsonParser parser = new JacksonJsonParser(); + private HttpMessageConverter mappingJackson2HttpMessageConverter; + @Autowired + protected UserRepository userRepository; + + @Autowired + protected PasswordEncoder passwordEncoder; + + protected User userWithAdminRole = new User(); + protected User userWithUserRole = new User(); + + protected String adminAccessToken; + protected String userAccessToken; + @Autowired private void setConverters(HttpMessageConverter[] converters) { mappingJackson2HttpMessageConverter = Arrays.stream(converters) @@ -41,16 +68,57 @@ private void setConverters(HttpMessageConverter[] converters) { } @Autowired - public void setMockMvc(WebApplicationContext webApplicationContext) throws Exception { + public void setMockMvc(WebApplicationContext wac, FilterChainProxy filter) throws Exception { mockMvc = MockMvcBuilders - .webAppContextSetup(webApplicationContext) + .webAppContextSetup(wac) + .addFilters(filter) .build(); } + @SuppressWarnings("unchecked") protected String json(Object object) throws IOException { MockHttpOutputMessage mockHttpOutputMessage = new MockHttpOutputMessage(); mappingJackson2HttpMessageConverter.write( object, MediaType.APPLICATION_JSON, mockHttpOutputMessage); return mockHttpOutputMessage.getBodyAsString(); } + + private String obtainAccessToken(String email, String password) throws Exception { + MultiValueMap params = new LinkedMultiValueMap<>(); + params.add("grant_type", "password"); + params.add("username", email); + params.add("password", password); + + String resultString = mockMvc.perform(post("/oauth/token") + .params(params) + .with(httpBasic(CLIENT_ID, CLIENT_SECRET)) + .accept(MediaType.APPLICATION_JSON_UTF8)) + .andExpect(status().isOk()) + .andExpect(content().contentType(MediaType.APPLICATION_JSON_UTF8)) + .andReturn().getResponse().getContentAsString(); + + + return parser.parseMap(resultString).get("access_token").toString(); + } + + @Before + public void obtainAccessTokens() throws Exception { + userRepository.deleteAllInBatch(); + + userWithAdminRole.setEmail("admin@auth.com"); + userWithAdminRole.setUsername("admin"); + userWithAdminRole.setPassword(passwordEncoder.encode("pass")); + userWithAdminRole.setAuthority(UserAuthority.ADMIN); + userWithAdminRole.setActivated(true); + userRepository.save(userWithAdminRole); + adminAccessToken = obtainAccessToken(userWithAdminRole.getEmail(), "pass"); + + userWithUserRole.setEmail("user@auth.com"); + userWithUserRole.setUsername("user"); + userWithUserRole.setPassword(passwordEncoder.encode("pass")); + userWithUserRole.setAuthority(UserAuthority.USER); + userWithUserRole.setActivated(true); + userRepository.save(userWithUserRole); + userAccessToken = obtainAccessToken(userWithUserRole.getEmail(), "pass"); + } } diff --git a/src/test/java/org/teapot/backend/test/controller/UserControllerIT.java b/src/test/java/org/teapot/backend/test/controller/UserControllerIT.java index 24c4cf4..06fffd6 100644 --- a/src/test/java/org/teapot/backend/test/controller/UserControllerIT.java +++ b/src/test/java/org/teapot/backend/test/controller/UserControllerIT.java @@ -1,16 +1,19 @@ package org.teapot.backend.test.controller; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; -import org.springframework.beans.factory.annotation.Autowired; -import org.springframework.security.crypto.password.PasswordEncoder; +import org.springframework.util.LinkedMultiValueMap; +import org.springframework.util.MultiValueMap; import org.teapot.backend.model.User; -import org.teapot.backend.repository.UserRepository; +import org.teapot.backend.model.UserAuthority; import java.time.LocalDate; import java.util.List; import static org.hamcrest.Matchers.*; +import static org.springframework.http.HttpHeaders.AUTHORIZATION; +import static org.springframework.security.oauth2.common.OAuth2AccessToken.BEARER_TYPE; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.*; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.*; @@ -19,23 +22,17 @@ public class UserControllerIT extends AbstractControllerIT { private static final String API_URL = "/users"; - @Autowired - private UserRepository userRepository; - - @Autowired - private PasswordEncoder passwordEncoder; - private User getUserOne = new User(); private User getUserTwo = new User(); - private User postUser = new User(); + private User postUserOne = new User(); + private User postUserTwo = new User(); private User repeatedPostUser = new User(); private User updateUser = new User(); private User deleteUser = new User(); + private User patchUser = new User(); @Before - public void init() { - userRepository.deleteAllInBatch(); - + public void addTestUsers() { getUserOne.setUsername("getUserOne"); getUserOne.setEmail("getUserOne@mail.com"); getUserOne.setPassword(passwordEncoder.encode("pass")); @@ -46,9 +43,13 @@ public void init() { getUserTwo.setPassword(passwordEncoder.encode("pass")); userRepository.save(getUserTwo); - postUser.setUsername("postUser"); - postUser.setEmail("postUser@mail.com"); - postUser.setPassword(passwordEncoder.encode("pass")); + postUserOne.setUsername("postUser"); + postUserOne.setEmail("postUser@mail.com"); + postUserOne.setPassword(passwordEncoder.encode("pass")); + + postUserTwo.setUsername("postUser"); + postUserTwo.setEmail("postUser@mail.com"); + postUserTwo.setPassword(passwordEncoder.encode("pass")); repeatedPostUser.setUsername("repeatedPostUser"); repeatedPostUser.setEmail("repeatedPostUser@mail.com"); @@ -64,8 +65,15 @@ public void init() { deleteUser.setEmail("deleteUser@mail.com"); deleteUser.setPassword(passwordEncoder.encode("pass")); userRepository.save(deleteUser); + + patchUser.setUsername("patchUser"); + patchUser.setEmail("patchUser@mail.com"); + patchUser.setPassword(passwordEncoder.encode("pass")); + userRepository.save(patchUser); } + // GET + @Test public void getUsersTest() throws Exception { List all = userRepository.findAll(); @@ -100,50 +108,180 @@ public void getNotExistsUserTest() throws Exception { .andExpect(status().isNotFound()); } + // POST + + @Test + public void registerUserTestByAnonymous() throws Exception { + mockMvc.perform(post(API_URL) + .content(json(postUserOne)) + .contentType(contentType)) + .andExpect(status().isCreated()) + .andExpect(header().string("Location", containsString(API_URL))); + } + + @Test + public void registerUserTestByUser() throws Exception { + mockMvc.perform(post(API_URL) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, userAccessToken)) + .content(json(postUserTwo)) + .contentType(contentType)) + .andExpect(status().isForbidden()); + } + @Test - public void registerUserTest() throws Exception { + public void registerUserTestByAdmin() throws Exception { mockMvc.perform(post(API_URL) - .content(json(postUser)) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken)) + .content(json(postUserTwo)) .contentType(contentType)) .andExpect(status().isCreated()) .andExpect(header().string("Location", containsString(API_URL))); } @Test - public void repeatRegisterUserTest() throws Exception { + public void repeatRegisterUserTestByAdmin() throws Exception { mockMvc.perform(post(API_URL) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken)) .content(json(repeatedPostUser)) .contentType(contentType)) .andExpect(status().isBadRequest()) .andExpect(header().doesNotExist("Location")); } + // PUT + + @Test + public void updateUserTestByAnonymous() throws Exception { + updateUser.setBirthday(LocalDate.now()); + mockMvc.perform(put(String.format("%s/%d", API_URL, updateUser.getId())) + .content(json(updateUser)) + .contentType(contentType)) + .andExpect(status().isUnauthorized()); + } + + @Test + public void updateUserTestByUser() throws Exception { + updateUser.setBirthday(LocalDate.now()); + mockMvc.perform(put(String.format("%s/%d", API_URL, updateUser.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, userAccessToken)) + .content(json(updateUser)) + .contentType(contentType)) + .andExpect(status().isForbidden()); + } + @Test - public void updateUserTest() throws Exception { + public void updateUserTestByAdmin() throws Exception { updateUser.setBirthday(LocalDate.now()); mockMvc.perform(put(String.format("%s/%d", API_URL, updateUser.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken)) .content(json(updateUser)) .contentType(contentType)) .andExpect(status().isNoContent()); } @Test - public void updateNotExistsUserTest() throws Exception { + public void updateNotExistsUserTestByAdmin() throws Exception { mockMvc.perform(put(String.format("%s/-1", API_URL)) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken)) .content(json(updateUser)) .contentType(contentType)) .andExpect(status().isNotFound()); } + // DELETE + @Test - public void deleteUserTest() throws Exception { + public void deleteUserTestByAnonymous() throws Exception { mockMvc.perform(delete(String.format("%s/%d", API_URL, deleteUser.getId()))) + .andExpect(status().isUnauthorized()); + } + + @Test + public void deleteUserTestByUser() throws Exception { + mockMvc.perform(delete(String.format("%s/%d", API_URL, deleteUser.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, userAccessToken))) + .andExpect(status().isForbidden()); + } + + @Test + public void deleteUserTestByAdmin() throws Exception { + mockMvc.perform(delete(String.format("%s/%d", API_URL, deleteUser.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken))) + .andExpect(status().isNoContent()); + } + + @Test + public void deleteNotExistsUserTestByAdmin() throws Exception { + mockMvc.perform(delete(String.format("%s/-1", API_URL)) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken))) + .andExpect(status().isNotFound()); + } + + // PATCH + + @Test + public void patchUserTestByAnonymous() throws Exception { + mockMvc.perform(patch(String.format("%s/%d", API_URL, patchUser.getId()))) + .andExpect(status().isUnauthorized()); + } + + @Test + public void patchUserTestByUserSameId() throws Exception { + MultiValueMap params = new LinkedMultiValueMap<>(); + params.add("username", "newUserWithUserRole"); + params.add("firstName", "Bob"); + params.add("lastName", "Brown"); + + mockMvc.perform(patch(String.format("%s/%d", API_URL, userWithUserRole.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, userAccessToken)) + .params(params)) + .andExpect(status().isNoContent()); + + User newUserWithUserRole = userRepository.findOne(userWithUserRole.getId()); + Assert.assertEquals("newUserWithUserRole", newUserWithUserRole.getUsername()); + Assert.assertEquals("Bob", newUserWithUserRole.getFirstName()); + Assert.assertEquals("Brown", newUserWithUserRole.getLastName()); + } + + @Test + public void patchUserTestByUserNotSameId() throws Exception { + mockMvc.perform(patch(String.format("%s/%d", API_URL, patchUser.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, userAccessToken))) + .andExpect(status().isForbidden()); + } + + @Test + public void patchUserTestByAdmin() throws Exception { + MultiValueMap params = new LinkedMultiValueMap<>(); + params.add("username", "newPatchUser"); + params.add("email", "newPatchUser@test.org"); + params.add("password", "123456"); + params.add("available", "false"); + params.add("firstName", "Bob"); + params.add("lastName", "Brown"); + params.add("authority", "ADMIN"); + params.add("description", "newPatchUser"); + + mockMvc.perform(patch(String.format("%s/%d", API_URL, patchUser.getId())) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken)) + .params(params)) .andExpect(status().isNoContent()); + + User newPatchUser = userRepository.findOne(patchUser.getId()); + Assert.assertEquals("newPatchUser", newPatchUser.getUsername()); + Assert.assertEquals("newPatchUser@test.org", newPatchUser.getEmail()); + Assert.assertTrue(passwordEncoder.matches("123456", newPatchUser.getPassword())); + Assert.assertFalse(newPatchUser.isAvailable()); + Assert.assertEquals("Bob", newPatchUser.getFirstName()); + Assert.assertEquals("Brown", newPatchUser.getLastName()); + Assert.assertEquals(UserAuthority.ADMIN, newPatchUser.getAuthority()); + Assert.assertEquals("newPatchUser", newPatchUser.getDescription()); } @Test - public void deleteNotExistsUserTest() throws Exception { - mockMvc.perform(delete(String.format("%s/-1", API_URL))) + public void patchNotExistsUserTestByAdmin() throws Exception { + mockMvc.perform(patch(String.format("%s/-1", API_URL)) + .header(AUTHORIZATION, String.format("%s %s", BEARER_TYPE, adminAccessToken))) .andExpect(status().isNotFound()); } } From f267087f810e3d6f689f96e09e3c78abd97461ca Mon Sep 17 00:00:00 2001 From: serjihsklovski Date: Tue, 20 Jun 2017 16:14:23 +0300 Subject: [PATCH 2/2] encoding fix --- src/main/resources/messages_ru.properties | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/resources/messages_ru.properties b/src/main/resources/messages_ru.properties index c74e6ae..93632a0 100644 --- a/src/main/resources/messages_ru.properties +++ b/src/main/resources/messages_ru.properties @@ -1,2 +1,2 @@ -mail.confirm.text=Нажмите на ссылку, чтобы подтвердить вам e-mail адрес: {0} -mail.confirm.subject=[Teapot.org] Подтверждение регистрации \ No newline at end of file +mail.confirm.text=Нажмите РЅР° ссылку, чтобы подтвердить Ваш e-mail адрес: {0} +mail.confirm.subject=[Teapot.org] Подтверждение регистрации