✨ feat: 비밀번호 변경 기능 추가 #458
Conversation
| import { ApiProperty } from '@nestjs/swagger'; | ||
| import { IsNotEmpty, Matches } from 'class-validator'; | ||
|
|
||
| export class PasswordResetRequestDto { |
There was a problem hiding this comment.
P2) PasswordReset 보다는 ResetPassword가 기존 DTO 명칭과 맞는 것 같습니다! 변경 부탁드립니다.
| ); | ||
| } | ||
|
|
||
| @ApiRequestPasswordReset() |
There was a problem hiding this comment.
P2) ApiForgotPassword() 가 더 괜찮지 않을까 싶습니다!
|
리뷰 확인했습니다! 메서드나 변수명은 저도 어떻게 해야할지 고민이 있었는데,
이 질문에 대해서는 결론부터 말하자면 악의적인 사용자가 이메일을 입력해보면서, 위의 예시대로 이메일 목록을 추출해서 데나무 이용자들에게 피싱 메일을 작성한다고 가정해볼게요. |
| await this.redisService.set( | ||
| `${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`, | ||
| JSON.stringify(user), | ||
| 'EX', | ||
| 600, | ||
| ); |
There was a problem hiding this comment.
P2: 단순히 유저를 구분하는데에만 필요할텐데, user 객체 전체를 직렬화해 저장하신 이유가 궁금합니다.
별도 이유가 없다면, 유저 식별에 필요한 고유값만 저장해 인증에 활용 하는 방법이 더 좋다고 생각해요!
| const userData = await this.redisService.get( | ||
| `${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`, | ||
| ); | ||
|
|
||
| if (!userData) { | ||
| throw new NotFoundException('인증에 실패했습니다.'); | ||
| } | ||
|
|
||
| const user = JSON.parse(userData); | ||
| user.password = await this.createHashedPassword(password); | ||
|
|
||
| this.redisService.del(`${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`); | ||
| await this.userRepository.save(user); |
There was a problem hiding this comment.
| const userData = await this.redisService.get( | |
| `${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`, | |
| ); | |
| if (!userData) { | |
| throw new NotFoundException('인증에 실패했습니다.'); | |
| } | |
| const user = JSON.parse(userData); | |
| user.password = await this.createHashedPassword(password); | |
| this.redisService.del(`${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`); | |
| await this.userRepository.save(user); | |
| const userData = await this.redisService.get( | |
| `${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`, | |
| ); | |
| if (!userData) { | |
| throw new NotFoundException('인증에 실패했습니다.'); | |
| } | |
| const user = // 고유값으로 유저 DB에서 조회 | |
| user.password = await this.createHashedPassword(password); | |
| this.redisService.del(`${REDIS_KEYS.USER_RESET_PASSWORD_KEY}:${uuid}`); | |
| await this.userRepository.save(user); |
P2: 현재 API의 목적은 비밀번호 변경이기에, stale한 데이터를 들고와서 직렬화 후 DB에 넣기보다는 새롭게 DB에서 fresh하게 조회한 후 유저 객체를 저장하는 것이 좋을 것 같아요.
그럴 가능성은 적겠지만, 만약에 사용자가 비밀번호 변경을 요청하고 인증을 수행하는 사이에 유저 정보가 뭔가 변경된다면, 이 API로 인해서 그 변경이 이전 상태로 돌아갈 수 있어요.
There was a problem hiding this comment.
윗 부분과 연관성이 있는 부분이네요.
먼저 제가 코드를 저렇게 작성한 이유는 레디스를 인증을 위한 세션에 더해서 캐싱 기능까지 같이 생각을 했어요.
미리 유저 정보를 직렬화해서 캐싱한다면 굳이 DB를 한 번 더 체크할 필요가 없지 않을까?라는 질문이 떠올랐고,
코드로 옮긴 게 성윤님이 짚어준 두 개의 코드 블럭이고요.
그럴 가능성은 적겠지만, 만약에 사용자가 비밀번호 변경을 요청하고 인증을 수행하는 사이에 유저 정보가 뭔가 변경된다면, 이 API로 인해서 그 변경이 이전 상태로 돌아갈 수 있어요.
이 부분은 미처 생각을 못했네요.
안전하게 DB를 한 번 더 체크해보는 게 맞는 것 같습니다.
좋은 예시 감사합니다.👍
There was a problem hiding this comment.
Pull Request Overview
This PR adds a password reset functionality for users that implements a secure two-step process: requesting a password reset via email and then resetting the password using a time-limited token. The implementation prevents user enumeration attacks by always returning success responses regardless of email existence.
- Password reset request API that generates UUID tokens stored in Redis with 10-minute expiration
- Password reset API that validates tokens and updates user passwords
- Comprehensive test coverage including DTO validation and E2E tests
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| server/src/user/service/user.service.ts | Implements core password reset logic with Redis token storage |
| server/src/user/controller/user.controller.ts | Adds REST endpoints for password reset request and execution |
| server/src/user/dto/request/*.dto.ts | DTOs for password reset requests with validation rules |
| server/src/common/email/email.service.ts | Email service methods for sending password reset emails |
| server/src/common/email/mailContent.ts | HTML email template for password reset notifications |
| server/src/common/redis/redis.constant.ts | Redis key constant for password reset tokens |
| server/src/user/api-docs/*.api-docs.ts | Swagger API documentation for new endpoints |
| server/test/**/*.spec.ts | Comprehensive unit and E2E tests for password reset functionality |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
# Conflicts: # server/src/common/redis/redis.constant.ts
🔨 테스크
Issue
잘못된 이메일을 입력했을 때, 사용자에게 예외를 응답해야 하는가?
먼저 배경에 대해 설명하자면, 요즘 해킹 관련해서 많은 뉴스들이 나오고 있습니다.
예전처럼 낭만으로 OS나 DB, 프레임워크 등의 취약점을 찾아서 해킹을 하는 것보다 보이스피싱, 이메일피싱 등을 통해서 쉽게 해킹을 한다고 해요.
제가 걱정했던 부분은
User Enumeration Attack이라고 부르는 공격입니다.비밀번호 변경 요청을 통해서 어떤 이메일이 데이터베이스에 있는지 확인하고 그 목록을 뽑는다면, 데나무 이름을 도용한 이메일피싱이 더 쉬워질 수 있을 것이라 생각했습니다.
그래서 존재하지 않는 이메일을 대상으로 시도했을 경우, 응답은
성공적으로 진행했다고 보내되 실제로는 이메일을 보내지 않도록 구현했습니다. 다시 정리하면 일관된 답변으로 DB 상태를 추측할 수 없도록 했습니다.📋 작업 내용
이해하기 쉽도록 머메이드로 시퀀스 다이어그램을 그렸습니다.
sequenceDiagram participant User as 사용자 participant Frontend as 프론트엔드 participant Backend as 백엔드 서버 participant Redis as Redis participant EmailService as 이메일 서비스 Note over User, EmailService: 비밀번호 변경 요청 프로세스 User->>Frontend: `비밀번호를 잊어버렸습니다.`와 같은 문구 클릭 후 email 입력 폼을 작성 Frontend->>Backend: POST /api/user/password-reset<br/>{email: "[email protected]"} Backend->>Backend: UUID 토큰 생성 Backend->>Redis: 토큰 및 사용자 데이터 저장<br/>SET token:uuid "user" EX 600 Backend->>EmailService: 이메일 전송 요청<br/>(토큰이 포함된 링크) EmailService->>User: 비밀번호 재설정 이메일 전송 Backend->>Frontend: 200 OK<br/>{"message": "비밀번호 재설정 링크를 이메일로 발송했습니다."} Frontend->>User: "이메일을 확인해주세요" 메시지 표시 Note over User, EmailService: 이메일 확인 및 비밀번호 변경 User->>User: 이메일 확인 User->>Frontend: 이메일 링크 클릭<br/>GET user/password?token=506c975a-6d2b-458d-8a89-a79363806b22 Frontend->>User: 비밀번호 변경 페이지 출력<br/>(토큰이 포함된 폼) User->>Frontend: 새 비밀번호 입력 및 제출 Frontend->>Backend: PATCH /api/user/password<br/>{uuid: "uuid", password: "new_password"} Backend->>Redis: 토큰 검증<br/>GET token:uuid Redis->>Backend: 사용자 데이터 반환 Backend->>Backend: 비밀번호 업데이트 Backend->>Redis: 토큰 삭제<br/>DEL token:uuid Backend->>Frontend: 200 OK<br/>{"message": "비밀번호가 성공적으로 수정되었습니다."} Frontend->>User: 성공 메시지 표시📷 스크린 샷(선택 사항)
이메일 생성을 위한 요청
이메일 확인
잘못된 토큰을 입력
정상적인 토큰 입력
