로그인 시 이메일 주소 인증 여부 확인하도록 수정 #113
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Summary by CodeRabbit
Walkthrough로그인(signIn) 로직에 이메일 인증 여부 검증이 추가되었습니다. 비밀번호 검증 이후 Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant A as AuthService
participant R as MemberRepository
participant J as JWT Generator
U->>A: signIn(credentials)
A->>R: findByEmail()
R-->>A: member
A->>A: validatePassword()
alt 이메일 미인증
A-->>U: throw BusinessException(EMAIL_VERIFICATION_REQUIRED)
else 이메일 인증됨
A->>J: generateJwtPair(member)
J-->>A: accessToken, refreshToken
A-->>U: tokens
end
%% Styling (subtle)
rect rgba(200,230,255,0.2)
note right of A: 새로 추가된 인증 여부 확인
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Pre-merge checks (1 passed, 2 warnings)❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ Finishing touches
🧪 Generate unit tests
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[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java (2)
66-74:readOnly트랜잭션에서 토큰 저장 발생 — 메서드에@Transactional로 오버라이드 필요클래스 레벨에
@Transactional(readOnly = true)가 선언되어 있는데,generateJwtPair내부에서refreshTokenRepository.save(refreshToken)로 쓰기 동작이 수행됩니다. JPA 환경에서 readOnly 트랜잭션은 플러시/더티체킹 최적화로 인해 쓰기 반영이 누락되거나 구현체별로 예외가 발생할 수 있습니다. 메서드 레벨에서@Transactional을 명시해 readOnly를 해제해 주세요.[sprecommendation]
적용 제안(diff):
@Service public class AuthService { @@ - public JwtPair generateJwtPair(Member member) { + @Transactional + public JwtPair generateJwtPair(Member member) { JwtClaims claims = JwtClaims.create(member); AccessTokenData accessToken = accessTokenProvider.createToken(claims); RefreshTokenData refreshToken = refreshTokenProvider.createToken(claims); refreshTokenRepository.save(refreshToken); return JwtPair.of(accessToken.token(), accessToken.expiredIn(), refreshToken.token(), refreshToken.expiredIn()); }
- 근거: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java, Line 21-24(클래스 선언부), 66-74(쓰기 발생 위치)
- 규칙: 트랜잭션 일관성 및 쓰기 시멘틱 보장
- 참고 문서:
https://docs.spring.io/spring-framework/reference/data-access/transaction/declarative/tx-decl.html
26-33: 이메일 변경 시 검증 상태 리셋 및 인증 메일 재발송 필요현재
changeEmail은 이메일만 변경하고 검증 상태를 유지합니다. 사용자가 이메일을 바꾸더라도 이전의 “검증됨” 상태가 지속되면, 새로운 이메일 소유권 검증 없이 로그인 가능해지는 보안/무결성 문제가 발생합니다. 이메일 변경 시 검증 상태를 미검증으로 전환하고 인증 메일을 재발송하는 것이 안전합니다. (장기 학습 메모에도 이메일 인증 예외 전파 흐름이 명시되어 있습니다.)적용 제안 1/2(diff, 필드 주입 추가):
public class AuthService { private final MemberRepository memberRepository; private final RefreshTokenRepository refreshTokenRepository; private final AccessTokenProvider accessTokenProvider; private final RefreshTokenProvider refreshTokenProvider; private final PasswordEncoder passwordEncoder; + private final kr.ac.kumoh.d138.JobForeigner.email.service.AuthEmailService authEmailService;적용 제안 2/2(diff, 이메일 변경 로직 보강):
@Transactional public void changeEmail(Long memberId, ChangeEmailRequest changeEmailRequest) { Member member = memberRepository.findById(memberId) .orElseThrow(() -> new BusinessException(ExceptionType.MEMBER_NOT_FOUND)); // 이미 등록된 이메일인지 확인 if (memberRepository.existsByEmail(changeEmailRequest.email())) { throw new BusinessException(ExceptionType.EMAIL_ALREADY_EXISTS); } // 이메일 주소 변경 - member.changeEmail(changeEmailRequest.email()); + member.changeEmail(changeEmailRequest.email()); + // 검증 상태 리셋 (Member에 대응 메서드가 없다면 추가 필요) + member.unverify(); + // 새 이메일로 인증 메일 발송 + authEmailService.sendMail(member.getEmail()); }
- 참고: 위 변경에는
Member엔티티에unverify()(또는 동등 기능 메서드) 추가가 필요합니다. 메서드명이 다를 경우 해당 명칭에 맞춰 적용 바랍니다.- 근거: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java, Line 76-88
- 규칙: 입력/상태 일관성, 계정 소유권 검증 강화
Also applies to: 76-88
🧹 Nitpick comments (1)
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java (1)
15-16: 로깅 프레임워크 사용 자제 — 사용하지 않는@Slf4j제거 권장본 파일에서는 로깅 호출이 없는데
@Slf4j와 관련 import가 남아 있습니다. 프로젝트 가이드라인(메인 코드에서는 로깅 프레임워크 대신System.out.println선호)에 맞춰 불필요한 의존을 제거해 주세요.적용 제안(diff):
-import lombok.extern.slf4j.Slf4j; @@ -@Slf4j @Transactional(readOnly = true) @RequiredArgsConstructor @Service
- 근거: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java, Line 15-16, 21
- 규칙: 프로젝트 코딩 규칙(메인 코드의 로깅 정책)
- 참고:
https://naver.github.io/hackday-conventions-java/Also applies to: 21-21
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java(1 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
**/*
⚙️ CodeRabbit configuration file
**/*: - For every finding, include evidence: file path(s), line number(s), rule names (e.g., Checkstyle), and links to credible docs when relevant.
- Wrap any tokens starting with '@' (e.g.,
@JoinColumn) in backticks in PR comments.
Files:
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java
**/*.java
⚙️ CodeRabbit configuration file
**/*.java: - Review against Java 21.
- Indentation: use spaces (no tabs), tab width = 4 spaces; files must end with Unix LF newline. (Team adaptation)
- Maximum line length: 120 characters.
- Imports: single-class imports only; allow wildcard for static imports; group imports with blank lines between sections.
- Assignment operators (
=,+=,-=, etc.): always one space before and after; on line breaks, place the operator at the start of the next line.- Lambda expressions: omit parentheses for a single parameter; surround
->with spaces (param -> expression); use braces and explicitreturnfor multi-statement bodies; choose short, clear parameter names.- In multi-line expressions, place operators and ternary separators at end-of-line.
- Prefer Java 21 standard APIs over Guava.
- Do not annotate immutable local variables with
finalunless required for an inner class.- Allow the
varkeyword when the value is a castnull.- For the complete NAVER Campus Hackday conventions, see: https://naver.github.io/hackday-conventions-java/
Files:
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java
**/main/java/**/*.java
⚙️ CodeRabbit configuration file
**/main/java/**/*.java: - This project is mature and must preserve a stable, backward-compatible public Java API.
- In the "Changes" section, list up to 25 changes to the public Java API that could affect end users; if none, explicitly state "No public Java API changes in this PR."
- Define the public Java API as public/protected methods on public classes plus
module-info.java.- Derive the list by deeply analyzing code flow, including through private methods and calls to Java 21 and Guava.
- Report:
- New or removed public API methods
- Changes to return types or parameter types
- Behavioral changes that may impact consumers
- Use
System.out.printlninstead of logging frameworks.- For performance reasons, the project prefers for-loops; do not suggest converting between loops and streams.
Files:
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java
🧠 Learnings (2)
📓 Common learnings
Learnt from: patulus
PR: th-D138/Backend-JobForeigner#32
File: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/CompanyMemberService.java:64-67
Timestamp: 2025-05-12T01:00:17.875Z
Learning: AuthEmailService는 회원 검증과 이메일 인증 관련 비즈니스 예외(MEMBER_NOT_FOUND, EMAIL_ALREADY_VERIFIED, AUTH_CODE_INVALID)를 발생시키며, 이 예외들은 호출 서비스로 전파된다.
📚 Learning: 2025-05-12T01:00:17.875Z
Learnt from: patulus
PR: th-D138/Backend-JobForeigner#32
File: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/CompanyMemberService.java:64-67
Timestamp: 2025-05-12T01:00:17.875Z
Learning: AuthEmailService는 회원 검증과 이메일 인증 관련 비즈니스 예외(MEMBER_NOT_FOUND, EMAIL_ALREADY_VERIFIED, AUTH_CODE_INVALID)를 발생시키며, 이 예외들은 호출 서비스로 전파된다.
Applied to files:
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java
🧬 Code graph analysis (1)
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java (4)
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/TestMemberService.java (1)
Transactional(66-88)src/main/java/kr/ac/kumoh/d138/JobForeigner/email/service/AuthEmailService.java (1)
Slf4j(26-109)src/main/java/kr/ac/kumoh/d138/JobForeigner/member/controller/AuthController.java (1)
AuthController(26-79)src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/ForeignerMemberService.java (2)
ForeignerMemberService(18-52)signUp(29-51)
🔇 Additional comments (3)
src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java (3)
43-46: 이메일 인증 게이트 추가, 로직 타당합니다비밀번호 검증 이후
member.isVerified()로 인증 여부를 확인하고, 미인증 시BusinessException(ExceptionType.EMAIL_VERIFICATION_REQUIRED)를 던지는 흐름이 요구사항과 일치합니다. 토큰 발급 전에 차단되는 점도 적절합니다.
- 근거: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java, Line 43-46
43-46: PR “Changes” 섹션에 공개 API 변경 내역(행동 변화) 명시 바랍니다외부에서 호출 가능한
AuthService#signIn의 동작이 “미인증 계정에 대해 예외 발생”으로 변경되었습니다. 공개 메서드의 동작 변화는 소비자에게 영향이 있으므로 PR의 “Changes” 섹션에 아래와 같이 명시해 주세요.제안 템플릿:
Public Java API changes in this PR:
- Behavior change:
AuthService#signIn(String, String)now throwsBusinessException(EMAIL_VERIFICATION_REQUIRED)when the account is not email-verified (previously issued tokens regardless of verification status).근거: src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java, Line 34-50
규칙:
**/main/java/**/*.javaPR 문서화 가이드(공개 API 변화 최대 25개까지 열거, 없으면 명시적으로 “No public Java API changes in this PR.”)
43-46: 미인증 로그인 차단 흐름 — 예외 매핑·테스트 확인 필요
- 확인: ExceptionType에 EMAIL_VERIFICATION_REQUIRED가 HttpStatus.UNAUTHORIZED로 정의되어 있음 — src/main/java/kr/ac/kumoh/d138/JobForeigner/global/exception/ExceptionType.java (라인 25).
- 확인: AuthService에서 미인증 시 BusinessException(ExceptionType.EMAIL_VERIFICATION_REQUIRED)를 던짐 — src/main/java/kr/ac/kumoh/d138/JobForeigner/member/service/AuthService.java (라인 45).
- 미확인: 글로벌 예외 핸들러(@ControllerAdvice/@RestControllerAdvice)에서 BusinessException 또는 ExceptionType을 받아 HTTP 상태(401/403)로 매핑하는 구현을 확인하지 못함.
- 미확인: sign-in 관련 테스트(미인증 시 실패, 인증 후 정상 로그인)가 리포지토리에서 발견되지 않음.
조치 요청:
- 글로벌 예외 핸들러의 파일/라인을 제공하거나 BusinessException 처리로 적절한 HTTP 상태가 반환되도록 구현/검증할 것.
- sign-in의 미인증 차단 시나리오와 인증 완료 후 정상 로그인 시나리오에 대한 테스트를 추가하거나 해당 테스트 경로를 알려줄 것.
Juhye0k
approved these changes
Sep 11, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What is this PR?🔍