Refactor : 이메일 인증 및 비밀번호 재설정 링크 구조 개선 및 redirectUrl 적용 #96
Conversation
|
""" Walkthrough이 변경은 인증 및 비밀번호 재설정 플로우를 리팩터링하고 확장합니다. 이메일 전송 및 비밀번호 재설정 요청에 리디렉션 URL을 추가하고, DTO 및 서비스 메서드를 POST 요청 기반으로 통합하며, 메일 서비스와 시큐리티 설정을 업데이트합니다. 불필요한 import도 정리되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant AuthController
participant AuthService
participant MailService
Client->>AuthController: POST /email/send (SendEmailRequest)
AuthController->>AuthService: sendEmailCode(userId, email, redirectUrl)
AuthService->>MailService: sendButtonMail(userId, email, redirectUrl)
MailService-->>AuthService: (메일 전송)
AuthService-->>AuthController: SendEmailResponse
AuthController-->>Client: SendEmailResponse
Client->>AuthController: POST /auth/reset-password (ResetPasswordRequest)
AuthController->>AuthService: resetPassword(ResetPasswordRequest)
AuthService->>MailService: verifyPasswordCode(userId, token)
MailService-->>AuthService: 검증 결과
AuthService->>AuthService: 비밀번호 변경
AuthService-->>AuthController: FindPasswordResponse
AuthController-->>Client: FindPasswordResponse
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms (1)
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🔭 Outside diff range comments (2)
src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java (1)
235-248: 비밀번호 재설정 로직 개선 필요메소드 구현에서 몇 가지 개선이 필요합니다:
@Transactional어노테이션이 누락되었습니다- 사용자 존재 여부 및 삭제 상태 검증이 필요합니다
- 에러 처리 로직을 일관성 있게 개선할 수 있습니다
+@Transactional public FindPasswordResponse resetPassword(ResetPasswordRequest request) { User user = userDomainService.getUserByEmail(request.getEmail()); + + if (user == null || user.isDeleted()) { + throw new AuthException(AuthExceptionCode.USER_NOT_FOUND); + } boolean isMatch = mailService.verifyPasswordCode(user.getId(), request.getToken()); if (isMatch) { String encodedPassword = userDomainService.encodePassword(request.getNewPassword()); user.modifyPassword(encodedPassword); return FindPasswordResponse.from("비밀번호가 변경되었습니다."); } else { throw new UserException(UserExceptionCode.NOT_MATCH_CODE); } }src/main/java/org/ezcode/codetest/domain/user/service/MailService.java (1)
64-92: 비밀번호 재설정 이메일 링크와 엔드포인트 불일치 문제
CreatePasswordMail메서드에서 생성되는 이메일 링크가 GET 방식의 쿼리 파라미터를 사용하고 있지만 (/api/auth/reset-password?email=...&key=...),AuthController의 해당 엔드포인트는 POST 방식으로ResetPasswordRequest바디를 받도록 구현되어 있습니다. 또한 URL을 통해 민감한 토큰을 전달하는 것은 보안상 좋지 않습니다.다음 중 하나의 해결책을 고려해보세요:
- 이메일 링크를 프론트엔드 페이지로 연결하고, 해당 페이지에서 토큰을 추출해 POST 요청으로 변환
- 별도의 GET 엔드포인트를 추가해 토큰 검증 후 비밀번호 재설정 페이지로 리다이렉트
-body += "<a href='"+redirectUrl+"/api/auth/reset-password?email="+ email + "&key=" + verificationCode + "' target='_blenk'>비밀번호 변경하기</a>"; +body += "<a href='"+redirectUrl+"/reset-password?token=" + verificationCode + "' target='_blenk'>비밀번호 변경하기</a>";
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
src/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/request/FindPasswordRequest.java(1 hunks)src/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/request/ResetPasswordRequest.java(1 hunks)src/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/request/SendEmailRequest.java(1 hunks)src/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/response/SendEmailResponse.java(1 hunks)src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java(3 hunks)src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java(0 hunks)src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java(2 hunks)src/main/java/org/ezcode/codetest/domain/user/service/MailService.java(5 hunks)src/main/java/org/ezcode/codetest/presentation/usermanagement/AuthController.java(3 hunks)src/main/java/org/ezcode/codetest/presentation/usermanagement/UserController.java(0 hunks)
💤 Files with no reviewable changes (2)
- src/main/java/org/ezcode/codetest/presentation/usermanagement/UserController.java
- src/main/java/org/ezcode/codetest/application/usermanagement/user/service/UserService.java
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: build
🔇 Additional comments (13)
src/main/java/org/ezcode/codetest/common/security/config/SecurityConfig.java (2)
15-15: HttpMethod import 추가 승인새로운 request matchers를 위해 필요한 import가 올바르게 추가되었습니다.
75-78: PR 목표와 실제 변경사항 간의 불일치 확인 필요PR 목표에서는 "인증 관련 GET 요청 허용"을 위한 SecurityConfig 업데이트라고 명시되어 있으나, 실제 변경사항은 토론/답글 엔드포인트에 대한 공개 접근 허용입니다. 이는 두 가지 가능성을 시사합니다:
- PR 설명이 부정확하거나 불완전할 수 있음
- 인증 관련 엔드포인트에 대한 추가 변경사항이 누락되었을 수 있음
토론/답글 엔드포인트를 공개적으로 접근 가능하게 하는 것 자체는 적절해 보이지만, PR의 주 목적인 이메일 인증 및 비밀번호 재설정과는 직접적인 관련이 없어 보입니다.
다음 스크립트를 실행하여 인증 관련 엔드포인트가 다른 곳에서 처리되는지 확인해주세요:
#!/bin/bash # 인증 관련 엔드포인트 설정이 다른 곳에 있는지 확인 rg -A 5 -B 5 "auth.*verify|reset.*password|email.*verification" --type javasrc/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/response/SendEmailResponse.java (1)
6-12: 클래스명 변경이 적절합니다
SendEmailCodeResponse에서SendEmailResponse로의 이름 변경이 더 일반적이고 명확합니다. 리팩토링 목표에 잘 부합합니다.src/main/java/org/ezcode/codetest/application/usermanagement/auth/service/AuthService.java (4)
7-7: 새로운 DTO import가 적절합니다
ResetPasswordRequestimport 추가가 올바르게 되었습니다.
11-11: 응답 DTO 이름 변경이 일관성 있게 적용되었습니다
SendEmailCodeResponse에서SendEmailResponse로의 변경이 적절히 반영되었습니다.
229-229: redirectUrl 전달이 올바르게 구현되었습니다
FindPasswordRequest에서redirectUrl을mailService.sendPasswordMail로 전달하는 로직이 적절합니다.
103-106: 메소드 시그니처 변경 확인 필요
sendEmailCode메소드에redirectUrl매개변수가 추가되었습니다. 이는 기존 호출 코드에 영향을 줄 수 있습니다.다음 스크립트를 실행하여 기존 메소드 호출 코드가 업데이트되었는지 확인하세요:
#!/bin/bash # Description: sendEmailCode 메소드 호출부 확인 # Expected: 모든 호출부가 새로운 시그니처에 맞게 업데이트되어야 함 rg -A 3 "sendEmailCode\(" --type javasrc/main/java/org/ezcode/codetest/presentation/usermanagement/AuthController.java (3)
6-7: 새로운 DTO 클래스 import가 적절히 추가됨
ResetPasswordRequest,SendEmailRequest,SendEmailResponse등 새로운 DTO 클래스들이 올바르게 import되어 리팩터링된 구조를 지원합니다.Also applies to: 11-11
88-93: redirectUrl 지원으로 개선된 이메일 전송 로직이메일 전송 메서드가
SendEmailRequest를 통해redirectUrl을 받도록 개선되어 하드코딩된 URL 대신 동적 URL 구성이 가능해졌습니다. 이는 배포 환경에 따른 유연성을 제공합니다.
113-118: 비밀번호 재설정 엔드포인트 구현 검토 필요POST 방식의 비밀번호 재설정 엔드포인트가 추가되었으나, 이메일에서 생성되는 링크와의 일관성을 확인해야 합니다.
MailService.java의CreatePasswordMail메서드에서 생성되는 이메일 링크가 GET 방식의 쿼리 파라미터를 사용하는 반면, 여기서는 POST 방식으로ResetPasswordRequest바디를 받고 있습니다.다음 스크립트로 이메일 링크 생성 로직과 컨트롤러 엔드포인트 간의 일관성을 확인해보겠습니다:
#!/bin/bash # 비밀번호 재설정 관련 URL 패턴과 엔드포인트 매핑 확인 echo "=== 이메일에서 생성되는 비밀번호 재설정 링크 패턴 확인 ===" rg -A 3 -B 3 "reset-password.*email.*key" --type java echo "=== 컨트롤러의 비밀번호 재설정 엔드포인트 확인 ===" rg -A 5 -B 2 "@.*Mapping.*reset-password" --type java echo "=== ResetPasswordRequest DTO 구조 확인 ===" rg -A 10 "class.*ResetPasswordRequest" --type javasrc/main/java/org/ezcode/codetest/domain/user/service/MailService.java (3)
33-36: redirectUrl 지원으로 개선된 메일 전송 메서드
sendButtonMail과sendPasswordMail메서드가redirectUrl파라미터를 받도록 업데이트되어 하드코딩된 URL 의존성이 제거되었습니다. 이는 다양한 환경에서의 배포 유연성을 크게 향상시킵니다.Also applies to: 38-41
43-62: 이메일 인증 링크 생성 로직 개선
CreateButtonMail메서드가 동적redirectUrl을 사용하도록 개선되어 환경별 URL 구성이 가능해졌습니다. 생성되는 GET 링크 형식이 컨트롤러의/auth/verify엔드포인트와 일치하여 일관성이 유지됩니다.
158-174: 비밀번호 검증 메서드 추가
verifyPasswordCode메서드가 적절히 구현되어 비밀번호 재설정 토큰 검증 기능을 제공합니다. Redis 키 패턴과 만료 시간 관리, 검증 후 토큰 삭제 로직이 기존verifyCode메서드와 일관성 있게 구현되었습니다.
| @Schema(description = "비밀번호 찾기 요청") | ||
| public class FindPasswordRequest { | ||
| private String email; | ||
| private String redirectUrl; |
There was a problem hiding this comment.
보안 검증 필요: redirectUrl 필드에 대한 입력 검증 추가
redirectUrl 필드는 오픈 리다이렉트 공격에 취약할 수 있습니다. 허용된 도메인 목록이나 URL 패턴 검증을 추가하는 것을 권장합니다.
+ @Pattern(regexp = "^https?://(localhost|yourdomain\\.com)(:[0-9]+)?(/.*)?$",
+ message = "허용되지 않은 리다이렉트 URL입니다")
private String redirectUrl;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private String redirectUrl; | |
| @Pattern(regexp = "^https?://(localhost|yourdomain\\.com)(:[0-9]+)?(/.*)?$", | |
| message = "허용되지 않은 리다이렉트 URL입니다") | |
| private String redirectUrl; |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/request/FindPasswordRequest.java
at line 12, the redirectUrl field lacks input validation, making it vulnerable
to open redirect attacks. Implement validation logic to check that redirectUrl
matches a whitelist of allowed domains or conforms to specific URL patterns
before processing it. This can be done by adding validation annotations or
custom validation methods to ensure only safe URLs are accepted.
There was a problem hiding this comment.
@minjee2758 네, 보안 검증에 대해 이해해 주셔서 감사합니다!
redirectUrl 필드에 대한 입력 검증을 추가하시면 오픈 리다이렉트 공격을 방지할 수 있습니다. 구현하시면서 추가적인 도움이 필요하시면 언제든 말씀해 주세요.
...n/java/org/ezcode/codetest/application/usermanagement/auth/dto/request/SendEmailRequest.java
Show resolved
Hide resolved
| @Getter | ||
| @AllArgsConstructor | ||
| public class ResetPasswordRequest { | ||
| private String email; | ||
| private String newPassword; | ||
| private String token; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
필수 입력 검증 및 문서화 추가 필요
비밀번호 재설정 요청에는 보안상 중요한 검증이 필요합니다:
- 이메일 형식 검증
- 새 비밀번호 강도 검증
- 토큰 형식 검증
- API 문서화
+import io.swagger.v3.oas.annotations.media.Schema;
+import jakarta.validation.constraints.Email;
+import jakarta.validation.constraints.NotBlank;
+import jakarta.validation.constraints.Pattern;
+@Schema(description = "비밀번호 재설정 요청")
@Getter
@AllArgsConstructor
public class ResetPasswordRequest {
+ @Schema(description = "사용자 이메일")
+ @Email(message = "올바른 이메일 형식이 아닙니다")
+ @NotBlank(message = "이메일은 필수입니다")
private String email;
+
+ @Schema(description = "새 비밀번호")
+ @Pattern(regexp = "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)(?=.*[@$!%*?&])[A-Za-z\\d@$!%*?&]{8,}$",
+ message = "비밀번호는 8자 이상이며, 대소문자, 숫자, 특수문자를 포함해야 합니다")
+ @NotBlank(message = "새 비밀번호는 필수입니다")
private String newPassword;
+
+ @Schema(description = "인증 토큰")
+ @NotBlank(message = "토큰은 필수입니다")
private String token;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| @Getter | |
| @AllArgsConstructor | |
| public class ResetPasswordRequest { | |
| private String email; | |
| private String newPassword; | |
| private String token; | |
| } | |
| import io.swagger.v3.oas.annotations.media.Schema; | |
| import jakarta.validation.constraints.Email; | |
| import jakarta.validation.constraints.NotBlank; | |
| import jakarta.validation.constraints.Pattern; | |
| @Schema(description = "비밀번호 재설정 요청") | |
| @Getter | |
| @AllArgsConstructor | |
| public class ResetPasswordRequest { | |
| @Schema(description = "사용자 이메일") | |
| @Email(message = "올바른 이메일 형식이 아닙니다") | |
| @NotBlank(message = "이메일은 필수입니다") | |
| private String email; | |
| @Schema(description = "새 비밀번호") | |
| @Pattern( | |
| regexp = "^(?=.*[a-z])(?=.*[A-Z])(?=.*\\d)(?=.*[@$!%*?&])[A-Za-z\\d@$!%*?&]{8,}$", | |
| message = "비밀번호는 8자 이상이며, 대소문자, 숫자, 특수문자를 포함해야 합니다" | |
| ) | |
| @NotBlank(message = "새 비밀번호는 필수입니다") | |
| private String newPassword; | |
| @Schema(description = "인증 토큰") | |
| @NotBlank(message = "토큰은 필수입니다") | |
| private String token; | |
| } |
🤖 Prompt for AI Agents
In
src/main/java/org/ezcode/codetest/application/usermanagement/auth/dto/request/ResetPasswordRequest.java
around lines 6 to 12, the ResetPasswordRequest class lacks validation and
documentation. Add validation annotations to ensure the email field has a proper
email format, the newPassword field meets password strength requirements, and
the token field has the correct format. Additionally, include API documentation
annotations to describe these fields and their validation constraints.
There was a problem hiding this comment.
웅 나중에 비번 수정 다시 리팩토링할때 반영할게
There was a problem hiding this comment.
@minjee2758 네, 알겠습니다! 나중에 비밀번호 관련 기능을 리팩토링하실 때 함께 적용하시면 되겠네요. 👍
작업 내용
변경 사항
기존 하드코딩된 인증 URL(localhost:8080) 제거 → 프론트에서 전달한 redirectUrl 사용
이메일 본문 링크 생성 방식 변경
http://localhost:8080/api/auth/verify?email=...&key=...→
redirectUrl + "?token=" + key인증 플로우 개선
SecurityConfig에 인증 관련 GET 요청 허용 경로 추가
트러블 슈팅
/auth/reset-passwordap 준비되어 있긴하지만 UI와 실제 연동은 미구현코드 리뷰 전 확인 체크리스트
type :)Summary by CodeRabbit
신규 기능
기능 개선
버그 수정
보안