member: 이메일 인증 로직 책임 재배치 — MemberService에서 jwtUtil, frontendUrl 의존 제거#165
Conversation
- EmailVerificationTokenProvider 신설: 토큰 생성/검증 책임 분리 - EmailVerificationClaim 레코드 신설: 토큰 파싱 결과 - EmailChangeMailBuilder: 토큰 생성 + URL 조합 로직 흡수 - MemberService: 비즈니스 검증 + 오케스트레이션만 수행
🧪 테스트 결과145 tests 143 ✅ 3s ⏱️ Results for commit 02ecf4f. ♻️ This comment has been updated with latest results. |
|
Note
|
| 응집도 / 파일(들) | 요약 |
|---|---|
토큰 인증 책임 분리 src/main/java/com/ryu/studyhelper/member/MemberService.java |
JwtUtil과 frontendUrl 의존성을 제거하고, EmailVerificationTokenProvider를 통해 토큰 파싱 로직을 위임. 이메일 검증 흐름이 emailVerificationTokenProvider.parseToken()으로 단순화됨. |
토큰 제공자 및 클레임 신설 src/main/java/com/ryu/studyhelper/member/mail/EmailVerificationTokenProvider.java, src/main/java/com/ryu/studyhelper/member/mail/EmailVerificationClaim.java |
EmailVerificationTokenProvider는 JwtUtil을 래핑하여 토큰 생성/검증을 담당하고, EmailVerificationClaim 레코드는 파싱된 memberId와 newEmail을 보유. |
메일 빌더 로직 통합 src/main/java/com/ryu/studyhelper/member/mail/EmailChangeMailBuilder.java |
메소드 서명 변경: build(String newEmail, String verificationUrl) → build(Long memberId, String newEmail). 토큰 생성과 URL 조합 로직을 내부에서 처리. |
🎯 코드 리뷰 소요 시간
🎯 3 (중간) | ⏱️ ~25분
🐰 기념 시
🐇 토큰을 나누어 담고,
책임은 패키지에 정렬하니,
MemberService 한숨 놓고,
관심사가 맑게 분리되는군요! ✨
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | ✅ Passed | 제목이 PR의 주요 변경사항을 정확히 반영하고 있습니다: MemberService에서 jwtUtil과 frontendUrl 의존성을 제거하고 이메일 인증 로직 책임을 재배치한 것을 명확히 설명합니다. |
| Linked Issues check | ✅ Passed | PR의 모든 변경사항이 이슈 #162의 목표를 충족합니다: EmailVerificationTokenProvider와 EmailVerificationClaim 신설, EmailChangeMailBuilder에 토큰/URL 로직 이동, MemberService에서 jwtUtil/frontendUrl 의존 제거를 모두 구현했습니다. |
| Out of Scope Changes check | ✅ Passed | 모든 변경사항이 이메일 인증 로직의 책임 재배치라는 명확한 목표 범위 내에 있으며, 범위를 벗어난 변경사항은 없습니다. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
- 📝 Generate docstrings
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Post copyable unit tests in a comment
- Commit unit tests in branch
162-refactor-memberservice-이메일-인증-로직-책임-재배치
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.
Comment @coderabbitai help to get the list of available commands and usage tips.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/main/java/com/ryu/studyhelper/member/mail/EmailVerificationTokenProvider.java (1)
22-34:parseToken내 JWT 중복 파싱 최소화 고려
validateTokenOrThrow,getTokenType,getIdFromToken,getEmailFromToken호출마다 내부적으로 JWT를 파싱합니다. 저빈도 작업이라 실질적 성능 영향은 미미하지만,JwtUtil에 Claims를 한 번만 추출하는 메서드가 있거나 추가 가능하다면 단일 파싱으로 리팩토링할 수 있습니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ryu/studyhelper/member/mail/EmailVerificationTokenProvider.java` around lines 22 - 34, The parseToken method repeatedly parses the JWT via jwtUtil.validateTokenOrThrow, getTokenType, getIdFromToken and getEmailFromToken; refactor to parse the token once and reuse the Claims: add or use a JwtUtil method like parseClaims(token) or getClaims(token) that returns the Claims object, call jwtUtil.validateTokenOrThrow using that Claims (or validate then parse once), then extract tokenType, id and email from the single Claims instance and construct new EmailVerificationClaim(memberId, newEmail) to eliminate duplicate parsing in parseToken.src/main/java/com/ryu/studyhelper/member/MemberService.java (1)
133-147: LGTM — 이중 중복 확인 로직 적절토큰 발급과 인증 완료 사이의 TOCTOU 간격을 고려한 이메일 재확인(Line 138)이 적절합니다. 다만, 완전한 중복 방지를 위해서는 DB 레벨의
UNIQUE제약 조건이 최종 보루가 됩니다.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/main/java/com/ryu/studyhelper/member/MemberService.java` around lines 133 - 147, Add DB-level uniqueness enforcement and handle uniqueness violations in verifyAndChangeEmail: ensure the email column has a UNIQUE constraint (DB/migration) and wrap the update path (getById -> member.changeEmail -> save/update persistence) to catch unique-constraint/database-duplicate exceptions, translating them into the existing CustomException(CustomResponseStatus.EMAIL_ALREADY_EXISTS); reference verifyAndChangeEmail, isEmailAvailable, member.changeEmail and the persistence/save method used for Member to locate and implement the change.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/main/java/com/ryu/studyhelper/member/mail/EmailChangeMailBuilder.java`:
- Around line 20-21: EmailChangeMailBuilder의 `@Value` 기본값(frontendUrl)이 localhost로
설정되어 있어 InvitationMailBuilder의 기본값(https://codemate.kr)과 불일치하므로,
EmailChangeMailBuilder 클래스의 frontendUrl 필드(`@Value`("${FRONTEND_URL:...}")) 기본값을
InvitationMailBuilder와 동일한 https://codemate.kr로 변경하거나 둘 다 공용 설정(예:
application.properties 또는 공용 상수)으로 통일해 환경변수 누락 시 프로덕션 링크가 올바르게 사용되도록 수정하세요.
---
Nitpick comments:
In
`@src/main/java/com/ryu/studyhelper/member/mail/EmailVerificationTokenProvider.java`:
- Around line 22-34: The parseToken method repeatedly parses the JWT via
jwtUtil.validateTokenOrThrow, getTokenType, getIdFromToken and
getEmailFromToken; refactor to parse the token once and reuse the Claims: add or
use a JwtUtil method like parseClaims(token) or getClaims(token) that returns
the Claims object, call jwtUtil.validateTokenOrThrow using that Claims (or
validate then parse once), then extract tokenType, id and email from the single
Claims instance and construct new EmailVerificationClaim(memberId, newEmail) to
eliminate duplicate parsing in parseToken.
In `@src/main/java/com/ryu/studyhelper/member/MemberService.java`:
- Around line 133-147: Add DB-level uniqueness enforcement and handle uniqueness
violations in verifyAndChangeEmail: ensure the email column has a UNIQUE
constraint (DB/migration) and wrap the update path (getById ->
member.changeEmail -> save/update persistence) to catch
unique-constraint/database-duplicate exceptions, translating them into the
existing CustomException(CustomResponseStatus.EMAIL_ALREADY_EXISTS); reference
verifyAndChangeEmail, isEmailAvailable, member.changeEmail and the
persistence/save method used for Member to locate and implement the change.
| @Value("${FRONTEND_URL:http://localhost:5173}") | ||
| private String frontendUrl; |
There was a problem hiding this comment.
FRONTEND_URL 기본값이 InvitationMailBuilder와 불일치 — 프로덕션 배포 위험
EmailChangeMailBuilder의 기본값은 http://localhost:5173인 반면, 동일 프로젝트의 InvitationMailBuilder (Line 10)는 https://codemate.kr을 기본값으로 사용합니다.
FRONTEND_URL 환경변수가 누락된 프로덕션 환경에서 이메일 인증 링크가 localhost:5173을 가리켜 사용자가 이메일 인증을 완료할 수 없게 됩니다.
🔧 수정 제안
- `@Value`("${FRONTEND_URL:http://localhost:5173}")
+ `@Value`("${FRONTEND_URL:https://codemate.kr}")
private String frontendUrl;📝 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.
| @Value("${FRONTEND_URL:http://localhost:5173}") | |
| private String frontendUrl; | |
| `@Value`("${FRONTEND_URL:https://codemate.kr}") | |
| private String frontendUrl; |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/main/java/com/ryu/studyhelper/member/mail/EmailChangeMailBuilder.java`
around lines 20 - 21, EmailChangeMailBuilder의 `@Value` 기본값(frontendUrl)이
localhost로 설정되어 있어 InvitationMailBuilder의 기본값(https://codemate.kr)과 불일치하므로,
EmailChangeMailBuilder 클래스의 frontendUrl 필드(`@Value`("${FRONTEND_URL:...}")) 기본값을
InvitationMailBuilder와 동일한 https://codemate.kr로 변경하거나 둘 다 공용 설정(예:
application.properties 또는 공용 상수)으로 통일해 환경변수 누락 시 프로덕션 링크가 올바르게 사용되도록 수정하세요.
관련 이슈
변경 내용
변경 유형
테스트
스크린샷 (UI 변경 시)
참고사항
Summary by CodeRabbit
릴리스 노트