diff --git a/src/main/java/edu/cmipt/gcs/constant/ApiPathConstant.java b/src/main/java/edu/cmipt/gcs/constant/ApiPathConstant.java index 8fec2d2..4e81599 100644 --- a/src/main/java/edu/cmipt/gcs/constant/ApiPathConstant.java +++ b/src/main/java/edu/cmipt/gcs/constant/ApiPathConstant.java @@ -20,9 +20,10 @@ public class ApiPathConstant { public static final String USER_API_PREFIX = ALL_API_PREFIX + "/user"; - public static final String USER_GET_USER_BY_NAME_API_PATH = USER_API_PREFIX + "/{username}"; + public static final String USER_GET_USER_API_PATH = USER_API_PREFIX + "/get"; public static final String USER_UPDATE_USER_API_PATH = USER_API_PREFIX + "/update"; public static final String USER_CHECK_EMAIL_VALIDITY_API_PATH = USER_API_PREFIX + "/email"; public static final String USER_CHECK_USERNAME_VALIDITY_API_PATH = USER_API_PREFIX + "/username"; + public static final String USER_DELETE_USER_API_PATH = USER_API_PREFIX + "/delete"; } diff --git a/src/main/java/edu/cmipt/gcs/controller/UserController.java b/src/main/java/edu/cmipt/gcs/controller/UserController.java index 4fed021..73154db 100644 --- a/src/main/java/edu/cmipt/gcs/controller/UserController.java +++ b/src/main/java/edu/cmipt/gcs/controller/UserController.java @@ -33,8 +33,8 @@ import org.springframework.http.HttpHeaders; import org.springframework.http.ResponseEntity; import org.springframework.validation.annotation.Validated; +import org.springframework.web.bind.annotation.DeleteMapping; import org.springframework.web.bind.annotation.GetMapping; -import org.springframework.web.bind.annotation.PathVariable; import org.springframework.web.bind.annotation.PostMapping; import org.springframework.web.bind.annotation.RequestBody; import org.springframework.web.bind.annotation.RequestHeader; @@ -47,7 +47,7 @@ public class UserController { @Autowired private UserService userService; - @GetMapping(ApiPathConstant.USER_GET_USER_BY_NAME_API_PATH) + @GetMapping(ApiPathConstant.USER_GET_USER_API_PATH) @Operation( summary = "Get user by name", description = "Get user information by user name", @@ -74,7 +74,7 @@ public class UserController { description = "User not found", content = @Content(schema = @Schema(implementation = ErrorVO.class))) }) - public UserVO getUserByName(@PathVariable("username") String username) { + public UserVO getUser(@RequestParam("username") String username) { QueryWrapper wrapper = new QueryWrapper(); wrapper.eq("username", username); if (!userService.exists(wrapper)) { @@ -123,7 +123,7 @@ public ResponseEntity updateUser( assert user.id() != null; boolean res = userService.updateById(new UserPO(user)); if (!res) { - throw new GenericException(ErrorCodeEnum.USER_UPDATE_FAILED, user.toString()); + throw new GenericException(ErrorCodeEnum.USER_UPDATE_FAILED, user); } UserVO userVO = new UserVO(userService.getById(Long.valueOf(user.id()))); HttpHeaders headers = null; @@ -134,6 +134,52 @@ public ResponseEntity updateUser( return ResponseEntity.ok().headers(headers).body(userVO); } + @DeleteMapping(ApiPathConstant.USER_DELETE_USER_API_PATH) + @Operation( + summary = "Delete user", + description = "Delete user by id", + tags = {"User", "Delete Method"}) + @Parameters({ + @Parameter( + name = HeaderParameter.ACCESS_TOKEN, + description = "Access token", + required = true, + in = ParameterIn.HEADER, + schema = @Schema(implementation = String.class)), + @Parameter( + name = HeaderParameter.REFRESH_TOKEN, + description = "Refresh token", + required = true, + in = ParameterIn.HEADER, + schema = @Schema(implementation = String.class)), + @Parameter( + name = "id", + description = "User id", + required = true, + in = ParameterIn.QUERY, + schema = @Schema(implementation = Long.class)) + }) + @ApiResponses({ + @ApiResponse(responseCode = "200", description = "User deleted successfully"), + @ApiResponse( + responseCode = "404", + description = "User not found", + content = @Content(schema = @Schema(implementation = ErrorVO.class))) + }) + public void deleteUser( + @RequestHeader(HeaderParameter.ACCESS_TOKEN) String accessToken, + @RequestHeader(HeaderParameter.REFRESH_TOKEN) String refreshToken, + @RequestParam("id") Long id) { + if (userService.getById(id) == null) { + throw new GenericException(ErrorCodeEnum.USER_NOT_FOUND, id); + } + boolean res = userService.removeById(id); + if (!res) { + throw new GenericException(ErrorCodeEnum.USER_DELETE_FAILED, id); + } + JwtUtil.blacklistToken(accessToken, refreshToken); + } + @GetMapping(ApiPathConstant.USER_CHECK_EMAIL_VALIDITY_API_PATH) @Operation( summary = "Check email validity", diff --git a/src/main/java/edu/cmipt/gcs/enumeration/ErrorCodeEnum.java b/src/main/java/edu/cmipt/gcs/enumeration/ErrorCodeEnum.java index 847b9f1..5bef6de 100644 --- a/src/main/java/edu/cmipt/gcs/enumeration/ErrorCodeEnum.java +++ b/src/main/java/edu/cmipt/gcs/enumeration/ErrorCodeEnum.java @@ -31,7 +31,8 @@ public enum ErrorCodeEnum { USER_NOT_FOUND("USER_NOT_FOUND"), - USER_UPDATE_FAILED("USER_UPDATE_FAILED"); + USER_UPDATE_FAILED("USER_UPDATE_FAILED"), + USER_DELETE_FAILED("USER_DELETE_FAILED"); // code means the error code in the message.properties private String code; diff --git a/src/main/java/edu/cmipt/gcs/exception/GlobalExceptionHandler.java b/src/main/java/edu/cmipt/gcs/exception/GlobalExceptionHandler.java index 1ffeaf9..3e39f18 100644 --- a/src/main/java/edu/cmipt/gcs/exception/GlobalExceptionHandler.java +++ b/src/main/java/edu/cmipt/gcs/exception/GlobalExceptionHandler.java @@ -82,12 +82,12 @@ public ResponseEntity handleGenericException( } } - @ResponseStatus(HttpStatus.BAD_REQUEST) @ExceptionHandler(JsonParseException.class) - public void handleJsonParseException(JsonParseException e, HttpServletRequest request) { + public ResponseEntity handleJsonParseException( + JsonParseException e, HttpServletRequest request) { GenericException exception = new GenericException(e.getMessage()); exception.setCode(ErrorCodeEnum.MESSAGE_CONVERSION_ERROR); - handleGenericException(exception, request); + return handleGenericException(exception, request); } @ResponseStatus(HttpStatus.INTERNAL_SERVER_ERROR) diff --git a/src/main/java/edu/cmipt/gcs/filter/JwtFilter.java b/src/main/java/edu/cmipt/gcs/filter/JwtFilter.java index e4d2459..6962c84 100644 --- a/src/main/java/edu/cmipt/gcs/filter/JwtFilter.java +++ b/src/main/java/edu/cmipt/gcs/filter/JwtFilter.java @@ -164,12 +164,8 @@ private void authorize(HttpServletRequest request, String accessToken, String re // User can not update other user's information String idInToken = JwtUtil.getID(accessToken); String idInBody = getFromRequestBody(request, "id"); - if (request.getRequestURI().startsWith(ApiPathConstant.USER_API_PREFIX) - && !idInToken.equals(idInBody)) { - logger.info( - "User[{}] tried to update user[{}]'s information", - idInToken, - idInBody); + if (!idInToken.equals(idInBody)) { + logger.info("User[{}] tried to update user[{}]", idInToken, idInBody); throw new GenericException(ErrorCodeEnum.ACCESS_DENIED); } } else if (request.getRequestURI() @@ -181,6 +177,26 @@ private void authorize(HttpServletRequest request, String accessToken, String re throw new GenericException(ErrorCodeEnum.ACCESS_DENIED); } break; + case "DELETE": + if (accessToken == null) { + throw new GenericException(ErrorCodeEnum.TOKEN_NOT_FOUND); + } + if (request.getRequestURI().equals(ApiPathConstant.USER_DELETE_USER_API_PATH)) { + // for delete user, both access token and refresh token are needed + if (refreshToken == null) { + throw new GenericException(ErrorCodeEnum.TOKEN_NOT_FOUND); + } + // User can not delete other user + String idInToken = JwtUtil.getID(accessToken); + String idInParam = request.getParameter("id"); + if (!idInToken.equals(idInParam)) { + logger.info("User[{}] tried to delete user[{}]", idInToken, idInParam); + throw new GenericException(ErrorCodeEnum.ACCESS_DENIED); + } + } else { + throw new GenericException(ErrorCodeEnum.ACCESS_DENIED); + } + break; default: throw new GenericException(ErrorCodeEnum.ACCESS_DENIED); } diff --git a/src/main/resources/message/message.properties b/src/main/resources/message/message.properties index 97ce25e..acbad52 100644 --- a/src/main/resources/message/message.properties +++ b/src/main/resources/message/message.properties @@ -28,3 +28,4 @@ MESSAGE_CONVERSION_ERROR=Error occurs while converting message USER_NOT_FOUND=User not found: {} USER_UPDATE_FAILED=User update failed: {} +USER_DELETE_FAILED=User delete failed: {} diff --git a/src/test/java/edu/cmipt/gcs/controller/UserControllerTest.java b/src/test/java/edu/cmipt/gcs/controller/UserControllerTest.java index ef63612..60d5513 100644 --- a/src/test/java/edu/cmipt/gcs/controller/UserControllerTest.java +++ b/src/test/java/edu/cmipt/gcs/controller/UserControllerTest.java @@ -1,6 +1,7 @@ package edu.cmipt.gcs.controller; import static org.hamcrest.Matchers.is; +import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.delete; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.get; import static org.springframework.test.web.servlet.request.MockMvcRequestBuilders.post; import static org.springframework.test.web.servlet.result.MockMvcResultMatchers.content; @@ -14,10 +15,14 @@ import edu.cmipt.gcs.enumeration.ErrorCodeEnum; import edu.cmipt.gcs.util.MessageSourceUtil; +import org.junit.jupiter.api.MethodOrderer; +import org.junit.jupiter.api.Order; import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestMethodOrder; import org.springframework.beans.factory.annotation.Autowired; import org.springframework.boot.test.autoconfigure.web.servlet.AutoConfigureMockMvc; import org.springframework.boot.test.context.SpringBootTest; +import org.springframework.core.Ordered; import org.springframework.http.MediaType; import org.springframework.test.web.servlet.MockMvc; @@ -30,14 +35,17 @@ */ @SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.RANDOM_PORT) @AutoConfigureMockMvc +@TestMethodOrder(MethodOrderer.OrderAnnotation.class) +@Order(Ordered.LOWEST_PRECEDENCE) public class UserControllerTest { @Autowired private MockMvc mvc; @Test - public void testGetUserByNameValid() throws Exception { + public void testGetUserValid() throws Exception { mvc.perform( - get(ApiPathConstant.USER_API_PREFIX + "/" + TestConstant.USERNAME) - .header(HeaderParameter.ACCESS_TOKEN, TestConstant.ACCESS_TOKEN)) + get(ApiPathConstant.USER_GET_USER_API_PATH) + .header(HeaderParameter.ACCESS_TOKEN, TestConstant.ACCESS_TOKEN) + .param("username", TestConstant.USERNAME)) .andExpectAll( status().isOk(), jsonPath("$.username", is(TestConstant.USERNAME)), @@ -46,11 +54,12 @@ public void testGetUserByNameValid() throws Exception { } @Test - public void testGetUserByNameInvalid() throws Exception { + public void testGetUserInvalid() throws Exception { String invalidUsername = TestConstant.USERNAME + "invalid"; mvc.perform( - get(ApiPathConstant.USER_API_PREFIX + "/" + invalidUsername) - .header(HeaderParameter.ACCESS_TOKEN, TestConstant.ACCESS_TOKEN)) + get(ApiPathConstant.USER_GET_USER_API_PATH) + .header(HeaderParameter.ACCESS_TOKEN, TestConstant.ACCESS_TOKEN) + .param("username", invalidUsername)) .andExpectAll( status().isNotFound(), content() @@ -211,4 +220,39 @@ public void testUpdateUserInvalid() throws Exception { MessageSourceUtil.getMessage( ErrorCodeEnum.ACCESS_DENIED)))); } + + @Test + public void testDeleteUserInvalid() throws Exception { + String otherID = "123"; + mvc.perform( + delete(ApiPathConstant.USER_DELETE_USER_API_PATH) + .header(HeaderParameter.ACCESS_TOKEN, TestConstant.ACCESS_TOKEN) + .header(HeaderParameter.REFRESH_TOKEN, TestConstant.REFRESH_TOKEN) + .param("id", otherID)) + .andExpectAll( + status().isForbidden(), + content() + .json( + """ + { + "code": %d, + "message": "%s" + } + """ + .formatted( + ErrorCodeEnum.ACCESS_DENIED.ordinal(), + MessageSourceUtil.getMessage( + ErrorCodeEnum.ACCESS_DENIED)))); + } + + @Test + @Order(Ordered.LOWEST_PRECEDENCE) + public void testDeleteUserValid() throws Exception { + mvc.perform( + delete(ApiPathConstant.USER_DELETE_USER_API_PATH) + .header(HeaderParameter.ACCESS_TOKEN, TestConstant.ACCESS_TOKEN) + .header(HeaderParameter.REFRESH_TOKEN, TestConstant.REFRESH_TOKEN) + .param("id", TestConstant.ID)) + .andExpectAll(status().isOk()); + } }