-
Notifications
You must be signed in to change notification settings - Fork 4
feat: JWT 인증 필터에서 검증 토큰 처리 로직 개선 #402
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Walkthrough검증 토큰 처리 흐름이 재구성되었습니다: accessToken에서 인증을 직접 얻던 방식 대신 verificationToken에서 이메일을 추출해 새로운 헬퍼 메서드( Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 축하 시
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (2)
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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.java (1)
64-78: 심각한 보안 취약점: 토큰 검증이 우회됩니다.Lines 74-77이 if-else 블록 바깥에 위치하여 토큰 검증 실패 시에도 무조건 실행됩니다:
- 검증 실패 시: Line 71에서
SecurityContextHolder.clearContext()호출 후, Lines 74-77에서 다시 인증을 설정하여 검증을 완전히 우회합니다.- 검증 성공 시: 동일한 로직이 두 번 실행됩니다 (Lines 66-68, Lines 74-77).
- 무효한 토큰에서 이메일 추출 시도: Line 74에서 이미 무효화된 토큰에서 이메일을 추출하면 예외가 발생하거나 잘못된 데이터가 반환될 수 있습니다.
🔎 수정 제안: 중복 코드 제거
if (verificationToken != null) { if (jwtTokenProvider.validateToken(verificationToken)) { String email = jwtTokenProvider.extractEmailFromVerificationToken(verificationToken); Authentication auth = createVerificationAuthentication(email); SecurityContextHolder.getContext().setAuthentication(auth); } else { clearAuthCookie(response, "verificationToken"); SecurityContextHolder.clearContext(); } - - String email = jwtTokenProvider.extractEmailFromVerificationToken(verificationToken); - - Authentication auth = createVerificationAuthentication(email); - SecurityContextHolder.getContext().setAuthentication(auth); }
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check-application
🔇 Additional comments (1)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.java (1)
89-99: LGTM!헬퍼 메서드가 잘 구현되었습니다.
Collections.singletonList를 사용한 불변 권한 리스트 생성과ROLE_VERIFICATION을 통한 권한 제한이 적절합니다.
Test Results250 tests 250 ✅ 10s ⏱️ Results for commit a436a08. ♻️ This comment has been updated with latest results. |
📊테스트 커버리지
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
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 (1)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.java (1)
84-94: 권한 설정 중복 제거 필요
CustomUserDetails.getAuthorities()가 이미 Role에서 권한을 반환하므로, Line 86-87의 명시적 권한 생성은 불필요합니다.
CustomUserDetails의getAuthorities()메서드는Role.VERIFICATION에 대해"ROLE_VERIFICATION"을 반환하는데, 코드에서 동일한 권한을 중복으로 생성하고 있습니다. 다음과 같이 수정하세요:private Authentication createVerificationAuthentication(String email) { CustomUserDetails userDetails = new CustomUserDetails(email, null, Role.VERIFICATION); return new UsernamePasswordAuthenticationToken( userDetails, "", userDetails.getAuthorities()); }명시적으로
SimpleGrantedAuthority를 생성하는 대신userDetails.getAuthorities()를 사용하여 권한이 한 곳에서만 정의되도록 하십시오.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.java
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: check-application
🔇 Additional comments (2)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.java (2)
70-70: LGTM!검증 실패 시 verificationToken 쿠키를 명시적으로 제거하는 로직이 적절하게 구현되었습니다. accessToken 처리와 일관성 있는 패턴을 따르고 있습니다.
66-67: extractEmailFromVerificationToken 메서드는 존재하며 정상 작동합니다
jwtTokenProvider.extractEmailFromVerificationToken()메서드는JwtTokenProvider클래스에 구현되어 있으며, null을 반환하지 않습니다. 메서드는 토큰을 파싱하고 type이 "verification"인지 검증한 후 subject(이메일)를 반환하거나 JwtException을 던집니다. 따라서 NPE 발생 위험은 없습니다.다만,
validateToken()은 토큰의 서명과 만료 시간만 검증하고 토큰 type은 검증하지 않으므로,extractEmailFromVerificationToken()에서 발생하는 JwtException(유효하지 않은 토큰 타입)이 필터 레벨에서 처리되지 않을 수 있습니다. 만약 다른 종류의 유효한 JWT(예: 만료되지 않은 access token)가 verification token 쿠키에 저장되면,validateToken()통과 후extractEmailFromVerificationToken()에서 예외가 발생합니다. 이 경우를 대비해 try-catch로 감싸거나validateToken()에서 token type도 함께 검증하는 것을 고려하세요.
SUMMARY
verificationToken 에서 정보 추출시 email로 추출
Summary by CodeRabbit
릴리스 노트
리팩토링
✏️ Tip: You can customize this high-level summary in your review settings.