refactor: Auth DTO 검증 + 회원탈퇴 API 추가#16
Conversation
Summary of ChangesHello @coldmans, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! 이 PR은 인증 관련 로직을 리팩토링하여 기존의 Map 기반 요청 처리를 DTO 기반으로 변경하고, 이에 따른 유효성 검증을 도입합니다. 또한, 사용자가 안전하게 계정을 삭제할 수 있도록 회원 탈퇴 기능을 추가하고, 관련 데이터를 트랜잭션으로 처리하여 데이터 무결성을 보장합니다. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
이 PR은 인증 관련 API가 DTO를 사용하여 요청을 검증하도록 리팩토링하고, 회원 탈퇴 API를 추가하는 변경 사항을 담고 있습니다. DTO 기반 검증으로 전환한 것은 코드 품질과 안정성 측면에서 매우 좋은 개선입니다.
리뷰에서는 몇 가지 심각한(critical) 문제를 중점적으로 다루었습니다.
- 새로 추가된 회원 탈퇴 API의 보안 취약점으로, 악의적인 사용자가 다른 사용자의 계정을 삭제할 수 있는 가능성이 있습니다.
- 안전하지 않은 비밀번호 해싱 알고리즘을 사용하고 있습니다.
- 레포지토리 메서드가 존재하지 않아 발생하는 컴파일 오류가 있습니다.
이 외에도 예외 처리 방식과 JPA 데이터 일관성을 개선하기 위한 제안도 포함했습니다. PR 설명에 컴파일을 확인하지 못했다고 언급되었는데, 리뷰 결과 실제로 컴파일 오류가 발견되었습니다. 병합 전에 이러한 문제들, 특히 보안 취약점을 반드시 해결해야 합니다.
| } | ||
|
|
||
| @Transactional | ||
| public void withdraw(Long userId, String password) { |
There was a problem hiding this comment.
회원 탈퇴 기능에서 userId를 요청 본문(request body)으로 직접 받는 것은 심각한 보안 취약점입니다. 악의적인 사용자가 다른 사용자의 userId를 추측하여 계정 삭제를 시도할 수 있습니다. userId는 현재 인증된 사용자(principal)의 보안 컨텍스트에서 가져와야 합니다.
WithdrawRequest DTO에서 userId 필드를 제거하고, 컨트롤러에서 인증된 사용자의 식별자를 AuthService.withdraw 메서드로 전달하도록 수정하는 것을 강력히 권장합니다.
예를 들어, WithdrawRequest는 비밀번호만 포함하도록 변경하고, 컨트롤러에서는 @AuthenticationPrincipal 등을 사용하여 인증된 사용자의 ID를 가져와야 합니다.
| User user = userRepository.findById(userId) | ||
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); | ||
|
|
||
| String hashedPassword = hashPassword(password); |
There was a problem hiding this comment.
hashPassword 메서드에서 SHA-256을 솔트(salt) 없이 사용하는 것은 안전하지 않습니다. 이 방식은 레인보우 테이블 공격에 취약합니다. 비밀번호 해싱에는 BCrypt, SCrypt, 또는 Argon2와 같은 적응형 해시 함수를 사용해야 합니다. Spring Security는 PasswordEncoder 인터페이스와 BCryptPasswordEncoder 같은 구현체를 제공하므로 이를 활용하는 것이 좋습니다.
이 PR에서 새로 추가된 withdraw 메서드에서 취약한 hashPassword를 사용하고 있으므로, 이번 기회에 애플리케이션 전체의 비밀번호 저장 방식을 개선하는 것을 강력히 권장합니다.
| throw new RuntimeException("비밀번호가 일치하지 않습니다."); | ||
| } | ||
|
|
||
| wishlistRepository.deleteByUserId(userId); |
There was a problem hiding this comment.
WishlistRepository 인터페이스에는 deleteByUserId 메서드가 정의되어 있지 않아 컴파일 오류가 발생합니다. UserTimetableRepository에 추가한 것과 유사하게 WishlistRepository에도 deleteByUserId 메서드를 추가해야 합니다.
WishlistRepository.java가 이 PR에 포함되어 있지 않으므로, 해당 파일을 수정하여 아래 코드를 추가해주세요:
@Modifying
@Query("DELETE FROM WishlistItem w WHERE w.user.id = :userId")
void deleteByUserId(@Param("userId") Long userId);| Optional<UserTimetable> findByUserIdAndSubjectId(Long userId, Long subjectId); | ||
|
|
||
|
|
||
| @Modifying |
There was a problem hiding this comment.
@Modifying을 사용한 벌크 연산은 영속성 컨텍스트를 무시하고 직접 데이터베이스에 쿼리를 실행합니다. 이로 인해 영속성 컨텍스트에 있는 엔티티와 데이터베이스 상태가 불일치할 수 있습니다. @Modifying(clearAutomatically = true) 옵션을 사용하면 쿼리 실행 후 영속성 컨텍스트를 자동으로 비워주어 이러한 문제를 방지할 수 있습니다.
| @Modifying | |
| @Modifying(clearAutomatically = true) |
| User user = userRepository.findById(userId) | ||
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); |
There was a problem hiding this comment.
예외 처리를 위해 일반 RuntimeException을 사용하는 것보다, 특정 상황에 맞는 커스텀 예외(예: UserNotFoundException)를 정의하고 사용하는 것이 좋습니다. 이렇게 하면 전역 예외 핸들러(@ControllerAdvice)에서 예외 유형에 따라 다른 HTTP 상태 코드와 오류 메시지를 반환하는 등 더 세분화된 오류 처리가 가능해집니다. jakarta.persistence.EntityNotFoundException과 같은 표준 예외를 사용하는 것도 좋은 방법입니다.
| User user = userRepository.findById(userId) | |
| .orElseThrow(() -> new RuntimeException("사용자를 찾을 수 없습니다.")); | |
| User user = userRepository.findById(userId) | |
| .orElseThrow(() -> new jakarta.persistence.EntityNotFoundException("사용자를 찾을 수 없습니다.")); |
There was a problem hiding this comment.
Pull request overview
Auth 도메인에서 요청 바디를 Map 기반에서 DTO(+Bean Validation) 기반으로 전환하고, 회원탈퇴(Withdraw) API/트랜잭션을 추가하는 PR입니다.
Changes:
- AuthController의 register/login 요청을 DTO(
RegisterRequest,LoginRequest)로 변경하고@Valid검증을 적용 - 회원탈퇴 요청 DTO(
WithdrawRequest) 및/api/auth/withdraw엔드포인트 추가 - AuthService에 회원탈퇴 트랜잭션 추가 및 UserTimetableRepository에 사용자별 bulk delete 메서드 추가
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
src/main/java/inu/timetable/service/AuthService.java |
회원탈퇴 트랜잭션 추가 및 관련 repository 의존성 주입 |
src/main/java/inu/timetable/repository/UserTimetableRepository.java |
사용자별 시간표 bulk delete 쿼리 메서드 추가 |
src/main/java/inu/timetable/dto/auth/WithdrawRequest.java |
회원탈퇴 요청 DTO + validation 추가 |
src/main/java/inu/timetable/dto/auth/RegisterRequest.java |
회원가입 요청 DTO + validation 추가 |
src/main/java/inu/timetable/dto/auth/LoginRequest.java |
로그인 요청 DTO + validation 추가 |
src/main/java/inu/timetable/controller/AuthController.java |
register/login DTO 전환 및 withdraw 엔드포인트 추가 |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| @DeleteMapping("/withdraw") | ||
| public ResponseEntity<?> withdraw(@Valid @RequestBody WithdrawRequest request) { | ||
| authService.withdraw(request.userId(), request.password()); | ||
| return ResponseEntity.ok(Map.of("message", "회원탈퇴가 완료되었습니다.")); |
There was a problem hiding this comment.
DELETE /withdraw에 request body(@RequestBody WithdrawRequest)를 요구하면 일부 클라이언트/프록시에서 본문 전달이 제한될 수 있고, 이 코드베이스의 다른 삭제 API들도 대부분 @RequestParam 기반입니다. DELETE /withdraw?userId=...처럼 쿼리 파라미터로 받거나, body가 필요하다면 POST /withdraw로 변경하는 방향을 검토해주세요.
| wishlistRepository.deleteByUserId(userId); | ||
| userTimetableRepository.deleteByUserId(userId); | ||
| userRepository.delete(user); |
There was a problem hiding this comment.
User 엔티티가 timetables에 cascade = CascadeType.ALL, orphanRemoval = true를 사용하고 있어(User.java:46) userRepository.delete(user)만으로도 UserTimetable이 함께 삭제됩니다. 현재 userTimetableRepository.deleteByUserId(userId)는 중복 삭제/불필요한 쿼리를 유발할 수 있으니, 필요성이 없다면 제거하거나(의도적으로 bulk delete가 필요하면) 해당 이유를 주석/문서로 남겨주세요.
|
|
||
| wishlistRepository.deleteByUserId(userId); | ||
| userTimetableRepository.deleteByUserId(userId); | ||
| userRepository.delete(user); |
There was a problem hiding this comment.
wishlistRepository.deleteByUserId(userId) 호출에 대응하는 메서드가 WishlistRepository에 현재 존재하지 않아 컴파일 에러가 발생합니다. WishlistRepository에 deleteByUserId(Long userId)를 추가(derived query 또는 @Modifying/@query)하거나, 현재 존재하는 API로 사용자 위시리스트 전체 삭제를 수행하도록 수정해주세요.
| @PostMapping("/register") | ||
| public ResponseEntity<?> register(@RequestBody Map<String, Object> request) { | ||
| try { | ||
| String username = (String) request.get("username"); | ||
| String password = (String) request.get("password"); | ||
| Integer grade = (Integer) request.get("grade"); | ||
| String major = (String) request.get("major"); | ||
|
|
||
| User user = authService.register(username, password, grade, major); | ||
| return ResponseEntity.ok(UserResponse.from(user)); | ||
|
|
||
| } catch (RuntimeException e) { | ||
| return ResponseEntity.badRequest().body(Map.of("error", e.getMessage())); | ||
| } | ||
| public ResponseEntity<?> register(@Valid @RequestBody RegisterRequest request) { | ||
| User user = authService.register(request.username(), request.password(), request.grade(), request.major()); | ||
| return ResponseEntity.ok(UserResponse.from(user)); | ||
| } | ||
|
|
||
| @PostMapping("/login") | ||
| public ResponseEntity<?> login(@RequestBody Map<String, String> request) { | ||
| try { | ||
| String username = request.get("username"); | ||
| String password = request.get("password"); | ||
|
|
||
| User user = authService.login(username, password); | ||
| return ResponseEntity.ok(UserResponse.from(user)); | ||
|
|
||
| } catch (RuntimeException e) { | ||
| return ResponseEntity.badRequest().body(Map.of("error", e.getMessage())); | ||
| } | ||
| public ResponseEntity<?> login(@Valid @RequestBody LoginRequest request) { | ||
| User user = authService.login(request.username(), request.password()); | ||
| return ResponseEntity.ok(UserResponse.from(user)); | ||
| } |
There was a problem hiding this comment.
현재 프로젝트에는 @ControllerAdvice/@ExceptionHandler가 없어(검색 기준) AuthService에서 던지는 RuntimeException이 이 컨트롤러에서는 400이 아니라 500으로 반환됩니다. 다른 컨트롤러들은 동일한 패턴에서 badRequest()로 매핑하고 있으니, 기존처럼 try/catch로 400을 내려주거나 전역 예외 처리기를 도입해 응답 형식을 일관되게 맞춰주세요.
|
리뷰 반영 완료했습니다. 핵심 변경사항은 아래와 같습니다.
추가 확인 부탁드립니다. |
변경 사항
@Valid) 기반으로 전환DELETE /api/auth/withdraw추가분리 목적
참고