feat: JWT 인증 필터에서 검증 토큰 처리 로직 개선 및 쿠키 삭제 기능 추가 #401
Conversation
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ 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. 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: 1
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)
65-78: 심각한 버그: Verification 토큰 처리 로직에 여러 오류가 있습니다.
Line 67:
accessToken을 사용하고 있지만, 검증된 것은verificationToken입니다.accessToken이 null일 경우 NPE가 발생합니다.Lines 70-71:
clearAuthCookie의 쿠키 이름 파라미터에 실제 Java 코드가 문자열로 전달되고 있습니다. 이는 명백한 복사-붙여넣기 오류입니다.Lines 75-78: if-else 블록 이후에 무조건 실행되어, 토큰이 유효하지 않은 경우에도
extractEmailFromVerificationToken을 호출하여 예외가 발생합니다. 또한 토큰이 유효한 경우에는 line 68에서 설정한 인증 정보가 덮어씌워집니다.🔎 수정 제안
if (verificationToken != null) { - if (jwtTokenProvider.validateToken(verificationToken)) { - // 토큰이 유효한 경우, 인증 정보를 SecurityContext에 설정 - Authentication auth = jwtTokenProvider.getAuthenticationByToken(accessToken); - SecurityContextHolder.getContext().setAuthentication(auth); - } else { - clearAuthCookie(response, "Authentication auth = jwtTokenProvider.getAuthenticationByToken(accessToken);\n" + - " SecurityContextHolder.getContext().setAuthentication(auth);"); + 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 (3)
src/main/java/org/ject/support/common/security/jwt/JwtAuthenticationFilter.javasrc/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.javasrc/main/java/org/ject/support/domain/member/controller/MemberController.java
🧰 Additional context used
🪛 ast-grep (0.40.3)
src/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.java
[warning] 217-217: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 217-217: A cookie was detected without setting the 'secure' flag. The 'secure' flag for cookies prevents the client from transmitting the cookie over insecure channels such as HTTP. Set the 'secure' flag by calling '.setSecure(true);'.
Context: response.addCookie(cookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-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/domain/member/controller/MemberController.java (1)
53-55: LGTM!등록 성공 후 verification 쿠키를 삭제하는 것은 적절합니다. 불필요한 인증 토큰을 정리하여 보안을 강화합니다.
src/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.java
Show resolved
Hide resolved
📊테스트 커버리지
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
src/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.java (1)
210-220:Secure플래그 추가 확인 완료,SameSite속성 추가를 권장합니다.이전 리뷰에서 지적된
Secure플래그가 올바르게 추가되었습니다(Line 217). 다만, CSRF 보호를 강화하기 위해SameSite속성 추가를 권장합니다.Jakarta Servlet의
CookieAPI는SameSite를 직접 지원하지 않으므로, Spring의ResponseCookie를 사용하거나Set-Cookie헤더를 직접 설정하는 방식으로 추가할 수 있습니다.🔎 ResponseCookie를 사용한 SameSite 속성 추가 방법
+import org.springframework.http.ResponseCookie; + public void deleteVerificationCookie(HttpServletResponse response) { - Cookie cookie = new Cookie("verificationToken", null); - cookie.setPath("/"); - cookie.setHttpOnly(true); - cookie.setSecure(true); - cookie.setMaxAge(0); - response.addCookie(cookie); + ResponseCookie cookie = ResponseCookie.from("verificationToken", "") + .path("/") + .httpOnly(true) + .secure(true) + .maxAge(0) + .sameSite("Strict") + .build(); + response.addHeader("Set-Cookie", cookie.toString()); }
📜 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/JwtTokenProvider.java
🧰 Additional context used
🪛 ast-grep (0.40.3)
src/main/java/org/ject/support/common/security/jwt/JwtTokenProvider.java
[warning] 218-218: The application does not appear to verify inbound requests which can lead to a Cross-site request forgery (CSRF) vulnerability. If the application uses cookie-based authentication, an attacker can trick users into sending authenticated HTTP requests without their knowledge from any arbitrary domain they visit. To prevent this vulnerability start by identifying if the framework or library leveraged has built-in features or offers plugins for CSRF protection. CSRF tokens should be unique and securely random. The Synchronizer Token or Double Submit Cookie patterns with defense-in-depth mechanisms such as the sameSite cookie flag can help prevent CSRF. For more information, see: [Cross-site request forgery prevention](https://cheatsheetseries.owasp.org/cheatsheets/Cross-Site_Req\ uest_Forgery_Prevention_Cheat_Sheet.html).
Context: response.addCookie(cookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-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/JwtTokenProvider.java (1)
13-13: LGTM!새로운
deleteVerificationCookie메서드에 필요한 import가 올바르게 추가되었습니다.
📝 작업 내용
1. AccessToken 생성시 VerificationCookie 삭제
Summary by CodeRabbit
릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings.