♻️ refactor: Auth 도메인 디테일/일관성 보완 (#142)#143
Conversation
- SmsSendResponse.expiredAt 주석 오류 정정 (3분 -> 5분) - 데드코드 generateSmsToken 제거 (호출처 없음, 인증 우회 소지) - resetPassword 트랜잭션 애노테이션 스프링으로 통일 - resetPassword에서 sms_required 플래그 삭제로 잠금 해제 일관성 확보 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
📝 WalkthroughWalkthroughSMS 만료 시간 주석을 실제 5분 기준으로 수정하고, 비밀번호 재설정 성공 시 SMS 재인증 플래그를 삭제하도록 변경했습니다. 트랜잭션 애노테이션을 통일했으며 미사용 SMS 토큰 발급 메서드를 제거했습니다. Changes인증 도메인 정합성 정리
Estimated code review effort: 2 (Simple) | ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/main/java/com/example/scoi/domain/auth/service/AuthService.java (1)
233-238: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚖️ Poor tradeoffRedis 플래그 삭제는 DB 커밋과 원자적이지 않음
member.resetLoginFailCount()는 더티체킹으로 트랜잭션 커밋 시점에 flush되지만,redisUtil.delete(...)는 즉시 실행됩니다. 커밋이 실패해도 이미 삭제된 Redis 플래그는 롤백되지 않아, 비밀번호가 실제로는 변경되지 않았는데 SMS 재인증 잠금이 해제된 상태로 남을 수 있습니다. 다만login()(L330-341),reissue()(L433-434, L445-446)에도 동일한 패턴이 이미 존재하므로 이번 PR에서 새로 생긴 문제는 아니며, 기능적으로는 PR 목표에 부합합니다.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/main/java/com/example/scoi/domain/auth/service/AuthService.java` around lines 233 - 238, AuthService의 RT 만료 SMS 재인증 처리에서 Redis 플래그 삭제가 DB 커밋 전에 실행되지 않도록 조정하세요. member.resetLoginFailCount() 변경이 트랜잭션으로 성공적으로 커밋된 이후에 SMS_REQUIRED_PREFIX + phoneNumber 삭제가 수행되도록 기존 트랜잭션 동기화 또는 커밋 후 콜백 방식을 활용하고, login() 및 reissue()의 기존 패턴은 이번 변경 범위에 포함하지 마세요.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@src/main/java/com/example/scoi/domain/auth/service/AuthService.java`:
- Around line 233-238: AuthService의 RT 만료 SMS 재인증 처리에서 Redis 플래그 삭제가 DB 커밋 전에
실행되지 않도록 조정하세요. member.resetLoginFailCount() 변경이 트랜잭션으로 성공적으로 커밋된 이후에
SMS_REQUIRED_PREFIX + phoneNumber 삭제가 수행되도록 기존 트랜잭션 동기화 또는 커밋 후 콜백 방식을 활용하고,
login() 및 reissue()의 기존 패턴은 이번 변경 범위에 포함하지 마세요.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 50f7ebb5-d02e-47c4-aead-43db00e3fef6
📒 Files selected for processing (2)
src/main/java/com/example/scoi/domain/auth/dto/AuthResDTO.javasrc/main/java/com/example/scoi/domain/auth/service/AuthService.java
📍 PR 타입 (하나 이상 선택)
❗️ 관련 이슈 링크
Closed #142
📌 개요
✨ [Feat]이슈로 분리했습니다.🔁 변경 사항
SmsSendResponse.expiredAt주석발송 시간 + 3분→+ 5분(실제 값SMS_EXPIRATION_MINUTES = 5와 불일치 정정)generateSmsToken메서드 삭제 (전 코드베이스 호출처 0건, SMS 인증 없이 verificationToken을 발급해 노출 시 인증 우회 소지)resetPassword의@jakarta.transaction.Transactional→ 스프링@Transactional(나머지 메서드와 일관성, 동작 변화 없음)resetPassword에서 실패 카운트 초기화와 함께sms_required:{phone}Redis 플래그도 삭제 → SMS 인증을 이미 통과했으므로 RT 만료 잠금까지 일관되게 해제📸 스크린샷
👀 기타 더 이야기해볼 점
✅ 체크 리스트
Summary by CodeRabbit
버그 수정
문서