Conversation
|
Warning Rate limit exceeded@yooooonshine has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 22 minutes and 41 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. 📒 Files selected for processing (3)
Walkthrough로그아웃·회원탈퇴 엔드포인트와 토큰 블랙리스트 로직이 추가되고, 리프레시 쿠키 처리를 추상화한 CookieUtil 인터페이스와 Dev/Prod 구현체가 도입되며, 요청에서 Bearer 액세스 토큰을 추출하는 메서드가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AuthController
participant JwtTokenProvider
participant AuthService
participant CookieUtil
participant HttpResponse
User->>AuthController: POST /api/auth/logout (refreshToken cookie)
AuthController->>JwtTokenProvider: extractAccessToken(request)
JwtTokenProvider-->>AuthController: Optional<accessToken>
AuthController->>AuthService: logout(refreshToken, accessToken)
AuthService->>JwtTokenProvider: blacklist(accessToken)
AuthService->>JwtTokenProvider: blacklist(refreshToken)
AuthService->>AuthService: delete refresh-token DB entry
AuthService-->>AuthController: void
AuthController->>CookieUtil: deleteRefreshCookie()
CookieUtil-->>AuthController: ResponseCookie (Max-Age=0)
AuthController->>HttpResponse: set Header Set-Cookie, 200 OK
HttpResponse-->>User: response
sequenceDiagram
participant User
participant UserController
participant UserService
participant UserEntity
participant AuthService
participant CookieUtil
User->>UserController: DELETE /api/users (refreshToken cookie)
UserController->>JwtTokenProvider: extractAccessToken(request)
JwtTokenProvider-->>UserController: Optional<accessToken>
UserController->>UserService: deleteAndLogout(userId, refreshToken, accessToken)
UserService->>UserEntity: softWithdraw()
UserEntity-->>UserService: updated entity
UserService->>UserService: save updated user
UserService->>AuthService: logout(refreshToken, accessToken)
AuthService-->>UserService: void
UserService->>CookieUtil: deleteRefreshCookie()
CookieUtil-->>UserController: ResponseCookie (Max-Age=0)
UserController->>User: 200 OK (Set-Cookie header cleared)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
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: 9
🧹 Nitpick comments (3)
src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java (1)
61-69: 토큰 검증 순서 개선 필요로그아웃 메서드에서 토큰을 블랙리스트에 추가하기 전에 검증하는 순서를 개선하면 더 안전합니다.
다음과 같이 개선할 수 있습니다:
public void logout(final String refreshToken, final String accessToken) { + // 1. 리프레시 토큰이 이미 블랙리스트에 있는지 확인 + if (blackListRepository.existsById(refreshToken)) { + throw new AppException(ErrorCode.TOKEN_BLACKLISTED_EXCEPTION); + } + + // 2. 사용자 확인 및 저장된 리프레시 토큰 검증 + UserEntity user = jwtTokenProvider.getUser(refreshToken) + .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_FOUND_EXCEPTION)); + + RefreshToken savedRefreshToken = refreshTokenRepository.findById(user.getId()) + .orElseThrow(() -> new AppException(ErrorCode.REFRESH_TOKEN_NOT_FOUND_EXCEPTION)); + + if (!savedRefreshToken.getRefreshToken().equals(refreshToken)) { + throw new AppException(ErrorCode.REFRESH_TOKEN_MISMATCH_EXCEPTION); + } + + // 3. 검증 완료 후 블랙리스트 추가 및 삭제 jwtTokenProvider.setBlackList(refreshToken); jwtTokenProvider.setBlackList(accessToken); - - UserEntity user = jwtTokenProvider.getUser(refreshToken) - .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_FOUND_EXCEPTION)); - + refreshTokenRepository.deleteById(user.getId()); }이렇게 하면:
- 이미 블랙리스트에 있는 토큰으로 재로그아웃 시도를 방지
- 저장된 리프레시 토큰과 일치하는지 검증 후 블랙리스트 추가
- 무효한 토큰을 블랙리스트에 추가하는 것을 방지
src/main/java/hanium/modic/backend/web/user/controller/UserController.java (1)
74-74: HTTP 상태 코드 개선 권장바디가 없는 DELETE 요청의 경우
200 OK대신204 No Content를 반환하는 것이 RESTful API 모범 사례입니다.위의 diff를 참고하여
ResponseEntity.noContent().build()로 변경하세요.src/main/java/hanium/modic/backend/domain/user/entity/UserEntity.java (1)
100-104: 멱등성 검증 추가 필요 (이메일 길이 제약은 문제없음)검증 결과:
이메일 길이 제약: 문제 없음
@Column애노테이션에 명시적 길이 제약 없음 (JPA 기본값 255자)- 생성되는 이메일 최대 길이:
withdrawn_+ id(~19자) +_+ UUID(36자) +@modic.com= 약 76자- 안전한 범위 내
멱등성 부재: 실제 문제 확인
softWithdraw()호출 시마다 새로운 UUID 생성- 동일 사용자에 대한 중복 호출 시 다른 이메일 생성
다음과 같이 수정하세요:
public void softWithdraw() { + if (this.userRole == UserRole.WITHDRAWN) { + return; + } this.userRole = UserRole.WITHDRAWN; - this.email = "withdrawn_" + this.id + "_" + java.util.UUID.randomUUID().toString() + "@modic.com"; + this.email = "withdrawn_" + this.id + "@modic.com"; this.name = "탈퇴회원"; }UUID 사용은 불필요합니다. 사용자 ID만으로도 고유한 이메일을 생성할 수 있습니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
src/main/java/hanium/modic/backend/common/jwt/JwtTokenProvider.java(2 hunks)src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java(2 hunks)src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java(1 hunks)src/main/java/hanium/modic/backend/domain/auth/util/CookieUtil.java(1 hunks)src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java(1 hunks)src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java(1 hunks)src/main/java/hanium/modic/backend/domain/user/entity/UserEntity.java(1 hunks)src/main/java/hanium/modic/backend/domain/user/enums/UserRole.java(1 hunks)src/main/java/hanium/modic/backend/domain/user/service/UserService.java(1 hunks)src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java(6 hunks)src/main/java/hanium/modic/backend/web/user/controller/UserController.java(2 hunks)src/test/java/hanium/modic/backend/common/oauth/OAuthSuccessHandlerTest.java(6 hunks)src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerIntegrationTest.java(2 hunks)src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerTest.java(4 hunks)
🧰 Additional context used
🧬 Code graph analysis (4)
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java (2)
src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java (1)
Component(9-32)src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java (1)
Component(22-54)
src/test/java/hanium/modic/backend/common/oauth/OAuthSuccessHandlerTest.java (2)
src/main/java/hanium/modic/backend/domain/auth/constant/AuthConstant.java (1)
AuthConstant(3-10)src/test/java/hanium/modic/backend/domain/user/factory/UserFactory.java (1)
UserFactory(9-29)
src/main/java/hanium/modic/backend/common/jwt/JwtTokenProvider.java (1)
src/main/java/hanium/modic/backend/domain/auth/constant/AuthConstant.java (1)
AuthConstant(3-10)
src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java (2)
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java (1)
Component(9-34)src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java (1)
Component(22-54)
🪛 ast-grep (0.39.7)
src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java
[warning] 62-62: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(refreshTokenCookie);
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 84-84: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(deleteRefreshTokenCookie);
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 62-62: 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(refreshTokenCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 84-84: 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(deleteRefreshTokenCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 62-62: 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(refreshTokenCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
[warning] 84-84: 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(deleteRefreshTokenCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
[warning] 105-105: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(refreshTokenCookie);
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 105-105: 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(refreshTokenCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 105-105: 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(refreshTokenCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java
[warning] 48-48: A cookie was detected without setting the 'HttpOnly' flag. The 'HttpOnly' flag for cookies instructs the browser to forbid client-side scripts from reading the cookie. Set the 'HttpOnly' flag by calling 'cookie.setHttpOnly(true);
Context: response.addCookie(refreshTokenCookie);
Note: [CWE-1004] Sensitive Cookie Without 'HttpOnly' Flag. [REFERENCES]
- https://owasp.org/www-community/HttpOnly
(cookie-missing-httponly-java)
[warning] 48-48: 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(refreshTokenCookie);
Note: [CWE-352] Cross-Site Request Forgery (CSRF). [REFERENCES]
- https://stackoverflow.com/questions/42717210/samesite-cookie-in-java-application
(cookie-missing-samesite-java)
[warning] 48-48: 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(refreshTokenCookie);
Note: [CWE-614] Sensitive Cookie in HTTPS Session Without 'Secure' Attribute. [REFERENCES]
- https://owasp.org/www-community/controls/SecureCookieAttribute
(cookie-missing-secure-flag-java)
🔇 Additional comments (13)
src/test/java/hanium/modic/backend/common/oauth/OAuthSuccessHandlerTest.java (2)
19-19: 테스트 프로파일 설정 적절함
@ActiveProfiles("test")추가로 테스트 환경별 설정을 명확히 분리했습니다.
50-51: CookieUtil 모킹 적절함CookieUtil 인터페이스 리팩토링에 맞춰 테스트에서 적절히 모킹했습니다. 정적 유틸리티에서 주입 가능한 인터페이스로 변경되어 테스트 가능성이 향상되었습니다.
src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerTest.java (1)
48-49: LGTM!CookieUtil을 MockitoBean으로 적절히 주입하여 컨트롤러 테스트에서 쿠키 생성 로직을 모킹했습니다.
src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerIntegrationTest.java (1)
44-45: LGTM!통합 테스트에서 CookieUtil을 적절히 주입받아 사용하고 있습니다. 정적 메서드에서 의존성 주입 방식으로 변경되어 테스트 가능성과 환경별 설정 분리가 개선되었습니다.
src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java (2)
29-29: LGTM: 의존성 주입 방식으로 개선
CookieUtil을 생성자 주입 방식으로 변경하여 테스트 가능성과 유연성이 향상되었습니다.
48-48: 정적 분석 경고는 오탐입니다정적 분석 도구가 쿠키의
HttpOnly,Secure,SameSite플래그 누락을 경고하고 있지만, 실제 쿠키 생성은DevCookieUtil과ProdCookieUtil구현체에서 이루어집니다. 해당 구현체들을 확인한 결과:
DevCookieUtil:HttpOnly(true)설정됨ProdCookieUtil:HttpOnly(true)및Secure(true)설정됨다만 두 구현체 모두
SameSite속성이 누락되어 있어 CSRF 보호가 미흡합니다. 이는 별도 코멘트에서 다룹니다.src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java (1)
25-31: LGTM: 쿠키 삭제 구현이 올바릅니다생성 시와 동일한 속성(path, HttpOnly)을 설정하고 maxAge를 0으로 설정하여 브라우저에서 즉시 삭제되도록 구현되었습니다.
src/main/java/hanium/modic/backend/common/jwt/JwtTokenProvider.java (1)
100-104: LGTM: 토큰 추출 로직이 올바르게 구현되었습니다
Authorization헤더에서 Bearer 토큰을 안전하게 추출하는 메서드입니다:
Optional반환으로 null 안전성 확보filter로 Bearer 접두사 검증map으로 접두사 제거구현이 간결하고 명확합니다.
src/main/java/hanium/modic/backend/domain/auth/util/CookieUtil.java (1)
5-9: LGTM: 인터페이스 기반 설계로 개선
CookieUtil을 인터페이스로 변경하여 환경별 구현체(DevCookieUtil,ProdCookieUtil)를 분리한 것은 좋은 설계입니다:
- 개발 환경에서는 Secure 플래그 없이 테스트 가능
- 프로덕션 환경에서는 Secure 플래그 적용
- 환경별 프로파일을 통한 자동 빈 선택
src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java (4)
45-46: LGTM: 의존성 추가가 적절합니다로그아웃 기능 구현을 위해
CookieUtil과JwtTokenProvider를 추가한 것이 적절합니다.
62-62: 정적 분석 경고는 오탐입니다정적 분석 도구가 쿠키 보안 플래그 누락을 경고하고 있으나, 실제 플래그 설정은
CookieUtil구현체에서 처리됩니다. 다만SameSite속성 누락은 별도 코멘트에서 다룹니다.
84-85: 정적 분석 경고는 오탐입니다쿠키 보안 플래그 설정은
CookieUtil.deleteRefreshCookie()구현체에서 처리됩니다.
105-105: 정적 분석 경고는 오탐입니다쿠키 보안 플래그 설정은
CookieUtil.createRefreshCookie()구현체에서 처리됩니다.
src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java
Outdated
Show resolved
Hide resolved
src/main/java/hanium/modic/backend/web/user/controller/UserController.java
Show resolved
Hide resolved
…into feature/242-logout-and-user-withdraw
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java (1)
62-62: 입력 파라미터 null 체크 추가 권장토큰 파라미터들이 null이거나 빈 문자열인 경우에 대한 검증이 없습니다. 방어적 코딩을 위해 입력 검증을 추가하는 것이 좋습니다.
메서드 시작 부분에 null 체크를 추가할 수 있습니다:
public void logout(final String refreshToken, final String accessToken) { + if (refreshToken == null || refreshToken.isBlank() || accessToken == null || accessToken.isBlank()) { + throw new AppException(ErrorCode.INVALID_TOKEN_EXCEPTION); + } + jwtTokenProvider.setBlackList(refreshToken);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java(1 hunks)src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerTest.java(4 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerTest.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: build
src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java (1)
82-84: 여전히 null accessToken이 흘러갑니다Line 82에서
extractAccessToken(request).orElse(null)을 그대로 넘기면 Authorization 헤더가 없는 요청에서 nullaccessToken이authService.logout으로 전달됩니다. 기존 리뷰에서 지적된 것처럼jwtTokenProvider.setBlackList가 여전히 null을 허용하지 않는 한, DB에 null 키 저장이나 예외가 재발합니다. 헤더가 비어 있으면 401을 반환하거나 Service 호출 전에 명시적으로 차단해주세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java(3 hunks)src/main/java/hanium/modic/backend/domain/auth/util/CookieUtil.java(1 hunks)src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java(1 hunks)src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java(1 hunks)src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java(6 hunks)src/test/java/hanium/modic/backend/common/oauth/OAuthSuccessHandlerTest.java(6 hunks)src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerIntegrationTest.java(3 hunks)src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerTest.java(5 hunks)
🚧 Files skipped from review as they are similar to previous changes (5)
- src/test/java/hanium/modic/backend/common/oauth/OAuthSuccessHandlerTest.java
- src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java
- src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerTest.java
- src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java
- src/test/java/hanium/modic/backend/web/auth/controller/AuthControllerIntegrationTest.java
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java (2)
src/main/java/hanium/modic/backend/common/oauth/OAuthSuccessHandler.java (1)
Component(23-55)src/main/java/hanium/modic/backend/domain/auth/util/DevCookieUtil.java (1)
Component(8-35)
⏰ 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: build
🔇 Additional comments (2)
src/main/java/hanium/modic/backend/domain/auth/util/ProdCookieUtil.java (1)
18-35: 보안 속성 구성이 잘 반영되었습니다
ResponseCookie를 사용해 HttpOnly, Secure, SameSite 등 핵심 속성을 모두 지정하여 프로덕션 환경에서 요구되는 쿠키 보안 기준을 충족하고 있습니다.src/main/java/hanium/modic/backend/domain/auth/util/CookieUtil.java (1)
5-8: 인터페이스 추출 방향에 동의합니다환경별 구현을 주입형으로 갈아탈 수 있게 계약을 인터페이스로 명확히 정의한 점이 테스트와 확장성 측면에서 적절합니다.
src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java
Show resolved
Hide resolved
- User 조회를 먼저하게 함으로서 안정성 확보
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java (1)
70-79: 로그아웃 토큰 검증 순서/일관성 여전히 누락
refresh token → repository 존재/일치 검증 없이 곧바로 블랙리스트 처리하고 삭제하고 있습니다. 검증 실패 시 일부 단계만 실행되는 불일치 상태(블랙리스트만 추가, DB 미삭제 등)가 그대로 남고, 위변조된/만료된 토큰으로도 다른 사용자의 refresh token 레코드를 삭제할 수 있는 여지가 있습니다. 이전 리뷰 피드백과 동일하게, 먼저 refresh token 저장소에서 존재 여부·값 일치 여부를 확인하고, access token 역시 동일 사용자 소유인지 확인한 뒤에 블랙리스트/삭제를 수행하도록 수정해주세요.예시 수정안:
- public void logout(final String refreshToken, final String accessToken) { - jwtTokenProvider.setBlackList(refreshToken); - jwtTokenProvider.setBlackList(accessToken); - - UserEntity user = jwtTokenProvider.getUser(refreshToken) - .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_FOUND_EXCEPTION)); - - refreshTokenRepository.deleteById(user.getId()); + public void logout(final String refreshToken, final String accessToken) { + UserEntity user = jwtTokenProvider.getUser(refreshToken) + .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_FOUND_EXCEPTION)); + + RefreshToken savedRefreshToken = refreshTokenRepository.findById(user.getId()) + .orElseThrow(() -> new AppException(ErrorCode.REFRESH_TOKEN_NOT_FOUND_EXCEPTION)); + if (!savedRefreshToken.getRefreshToken().equals(refreshToken)) { + throw new AppException(ErrorCode.REFRESH_TOKEN_MISMATCH_EXCEPTION); + } + + UserEntity accessTokenUser = jwtTokenProvider.getUser(accessToken) + .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_AUTHENTICATED_EXCEPTION)); + if (!user.getId().equals(accessTokenUser.getId())) { + throw new AppException(ErrorCode.REFRESH_TOKEN_MISMATCH_EXCEPTION); + } + + jwtTokenProvider.setBlackList(refreshToken); + jwtTokenProvider.setBlackList(accessToken); + refreshTokenRepository.deleteById(user.getId()); }src/main/java/hanium/modic/backend/web/user/controller/UserController.java (2)
87-89: Bearer 토큰 누락 시 즉시 예외 처리 필요
extractAccessToken(request).orElse(null)로 null을 넘기면authService.logout내부에서jwtTokenProvider.setBlackList(null)이 호출되어 NullPointerException 혹은 잘못된 블랙리스트 레코드가 발생합니다. Authorization 헤더가 없거나 형식이 잘못된 경우 즉시 예외를 던져야 합니다.+import hanium.modic.backend.common.error.exception.AppException; ... - String accessToken = jwtTokenProvider.extractAccessToken(request).orElse(null); + String accessToken = jwtTokenProvider.extractAccessToken(request) + .orElseThrow(() -> new AppException(USER_NOT_AUTHENTICATED_EXCEPTION));
93-93: 반환 타입과 응답 본문이 불일치합니다
메서드 시그니처가ResponseEntity<AppResponse<Void>>인데ResponseEntity.ok().build()는ResponseEntity<Void>를 반환하여 컴파일조차 되지 않습니다. 다른 엔드포인트와 동일하게AppResponse로 감싼 응답을 리턴하거나, 시그니처를ResponseEntity<Void>로 수정해야 합니다.- return ResponseEntity.ok().build(); + return ResponseEntity.ok(AppResponse.ok(null));src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java (1)
91-91: 응답 타입 불일치로 컴파일 불가
이 메서드도 시그니처가ResponseEntity<AppResponse<Void>>인데ResponseEntity.ok().build()를 반환하여 제네릭 타입이 맞지 않습니다.AppResponse로 감싼 응답을 리턴하거나 시그니처를ResponseEntity<Void>로 바꿔 주세요.- return ResponseEntity.ok().build(); + return ResponseEntity.ok(AppResponse.ok(null));
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/hanium/modic/backend/common/error/ErrorCode.java(1 hunks)src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java(4 hunks)src/main/java/hanium/modic/backend/domain/user/service/UserService.java(1 hunks)src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java(6 hunks)src/main/java/hanium/modic/backend/web/user/controller/UserController.java(5 hunks)
⏰ 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: build
| String accessToken = jwtTokenProvider.extractAccessToken(request).orElse(null); | ||
| authService.logout(refreshToken, accessToken); | ||
|
|
There was a problem hiding this comment.
AccessToken Optional을 null로 넘기지 마세요
Authorization 헤더가 비어 있으면 accessToken이 null이 되어 authService.logout에서 jwtTokenProvider.setBlackList(null)을 호출하게 되고, 유효하지 않은 키로 블랙리스트에 저장하거나 NPE가 발생합니다. 헤더가 없으면 즉시 인증 예외를 던져 주세요.
+import hanium.modic.backend.common.error.exception.AppException;
...
- String accessToken = jwtTokenProvider.extractAccessToken(request).orElse(null);
+ String accessToken = jwtTokenProvider.extractAccessToken(request)
+ .orElseThrow(() -> new AppException(USER_NOT_AUTHENTICATED_EXCEPTION));📝 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.
| String accessToken = jwtTokenProvider.extractAccessToken(request).orElse(null); | |
| authService.logout(refreshToken, accessToken); | |
| String accessToken = jwtTokenProvider.extractAccessToken(request) | |
| .orElseThrow(() -> new AppException(USER_NOT_AUTHENTICATED_EXCEPTION)); | |
| authService.logout(refreshToken, accessToken); |
🤖 Prompt for AI Agents
In src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java
around lines 85 to 87, the code extracts the access token as an Optional and
passes null to authService.logout when the Authorization header is missing;
instead fail fast: if the access token Optional is empty, throw an
authentication/authorization exception (e.g., BadCredentialsException or a
custom AuthException) immediately rather than passing null, otherwise call
authService.logout with the non-null access token; ensure
jwtTokenProvider.setBlackList is never called with null by enforcing the
presence of the token before calling logout.
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (2)
src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java (1)
70-79: 이전 리뷰의 주요 검증 로직 문제가 여전히 존재합니다.
@Transactional추가는 좋지만, 이전 리뷰에서 지적된 다음 문제들이 해결되지 않았습니다:
토큰 검증 전 블랙리스트 처리: 75-76행에서 토큰을 먼저 블랙리스트에 추가하고, 그 후 72-73행에서 사용자를 추출합니다. 사용자 추출이 실패하면 토큰만 블랙리스트에 남아 불일치 상태가 됩니다.
Access token 사용자 검증 누락: Access token이 refresh token과 동일한 사용자의 것인지 확인하지 않습니다. 악의적인 사용자가 다른 사용자의 access token을 제공할 수 있습니다.
reissue메서드(82-105행)처럼 검증을 먼저 수행한 후 블랙리스트 처리를 권장합니다.@Transactional public void logout(final String refreshToken, final String accessToken) { - UserEntity user = jwtTokenProvider.getUser(refreshToken) - .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_FOUND_EXCEPTION)); - - jwtTokenProvider.setBlackList(refreshToken); - jwtTokenProvider.setBlackList(accessToken); - + // 1. Refresh token 검증 + UserEntity user = jwtTokenProvider.getUser(refreshToken) + .orElseThrow(() -> new AppException(ErrorCode.USER_NOT_FOUND_EXCEPTION)); + + RefreshToken savedRefreshToken = refreshTokenRepository.findById(user.getId()) + .orElseThrow(() -> new AppException(ErrorCode.REFRESH_TOKEN_NOT_FOUND_EXCEPTION)); + + if (!savedRefreshToken.getRefreshToken().equals(refreshToken)) { + throw new AppException(ErrorCode.REFRESH_TOKEN_MISMATCH_EXCEPTION); + } + + // 2. Access token 검증 (동일 사용자인지 확인) + if (accessToken != null) { + UserEntity accessTokenUser = jwtTokenProvider.getUser(accessToken) + .orElseThrow(() -> new AppException(ErrorCode.TOKEN_INVALID_EXCEPTION)); + + if (!user.getId().equals(accessTokenUser.getId())) { + throw new AppException(ErrorCode.REFRESH_TOKEN_MISMATCH_EXCEPTION); + } + jwtTokenProvider.setBlackList(accessToken); + } + + // 3. 검증 성공 후 블랙리스트 처리 + jwtTokenProvider.setBlackList(refreshToken); + refreshTokenRepository.deleteById(user.getId()); }src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java (1)
85-86: 이전 리뷰의 Critical 이슈가 해결되지 않았습니다: null access token 처리
orElse(null)을 사용하면 Authorization 헤더가 없을 때accessToken이 null이 되어authService.logout에 전달됩니다. 이는jwtTokenProvider.setBlackList(null)을 호출하게 되어 NPE 또는 유효하지 않은 블랙리스트 엔트리가 생성될 수 있습니다.로그아웃은 인증된 사용자만 수행할 수 있으므로, access token이 없으면 즉시 예외를 발생시켜야 합니다.
+import hanium.modic.backend.common.error.exception.AppException; ... - String accessToken = jwtTokenProvider.extractAccessToken(request).orElse(null); + String accessToken = jwtTokenProvider.extractAccessToken(request) + .orElseThrow(() -> new AppException(USER_NOT_AUTHENTICATED_EXCEPTION)); authService.logout(refreshToken, accessToken);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/hanium/modic/backend/common/security/SecurityConfig.java(1 hunks)src/main/java/hanium/modic/backend/common/security/principal/UserPrincipal.java(2 hunks)src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java(4 hunks)src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java(6 hunks)
🔇 Additional comments (6)
src/main/java/hanium/modic/backend/common/security/principal/UserPrincipal.java (1)
62-65: LGTM! 탈퇴 회원 처리가 적절합니다.
isEnabled()에서WITHDRAWN상태를 확인하여 탈퇴한 회원을 비활성화하는 로직이 Spring Security의 표준 패턴을 잘 따르고 있습니다.src/main/java/hanium/modic/backend/domain/auth/service/AuthService.java (3)
45-45: 트랜잭션 추가가 적절합니다.로그인 메서드에
@Transactional을 추가하여 토큰 생성과 저장이 원자적으로 처리됩니다.
50-53: 탈퇴 회원 로그인 차단이 적절합니다.탈퇴한 회원의 로그인 시도를 명시적으로 차단하고 적절한 예외를 발생시킵니다.
81-81: 트랜잭션 추가가 적절합니다.토큰 재발급 작업의 원자성을 보장합니다.
src/main/java/hanium/modic/backend/web/auth/controller/AuthController.java (2)
46-47: 쿠키 처리 추상화가 좋습니다.
CookieUtil을 도입하여 쿠키 생성 및 삭제 로직을 일관되게 처리합니다. Dev/Prod 환경별 쿠키 설정을 중앙화할 수 있어 유지보수성이 향상됩니다.Also applies to: 63-64
91-91: 반환 타입 문제가 해결되었습니다.
AppResponse.noContent()를 사용하여 선언된 반환 타입ResponseEntity<AppResponse<Void>>와 일치하도록 수정되었습니다.
| .authorizeHttpRequests(auth -> auth | ||
| .anyRequest().permitAll()) | ||
| .anyRequest().authenticated()) |
There was a problem hiding this comment.
모든 요청에 인증을 요구하면 로그인이 불가능합니다.
.anyRequest().authenticated()로 변경하면 로그인, 회원가입, 이메일 인증 등의 공개 엔드포인트도 인증을 요구하게 되어 사용자가 로그인할 수 없습니다.
다음과 같이 공개 엔드포인트를 명시적으로 허용해야 합니다:
.authorizeHttpRequests(auth -> auth
+ .requestMatchers("/api/auth/login", "/api/auth/email/**", "/api/auth/signup").permitAll()
+ .requestMatchers("/oauth2/**", "/login/oauth2/**").permitAll()
.anyRequest().authenticated())📝 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.
| .authorizeHttpRequests(auth -> auth | |
| .anyRequest().permitAll()) | |
| .anyRequest().authenticated()) | |
| .authorizeHttpRequests(auth -> auth | |
| .requestMatchers("/api/auth/login", "/api/auth/email/**", "/api/auth/signup").permitAll() | |
| .requestMatchers("/oauth2/**", "/login/oauth2/**").permitAll() | |
| .anyRequest().authenticated()) |
🤖 Prompt for AI Agents
In src/main/java/hanium/modic/backend/common/security/SecurityConfig.java around
lines 37-38, the current .anyRequest().authenticated() forces authentication for
all endpoints (breaking login/signup/email verification). Update the
authorization config to explicitly permit public endpoints first (e.g.
requestMatchers or ant patterns for /api/auth/**, /api/signup,
/api/verify-email, static/public resources, swagger/actuator if used) with
.permitAll(), and then leave .anyRequest().authenticated() last so only the
remaining endpoints require authentication.
1 participant
개요
작업사항
Summary by CodeRabbit
새로운 기능
개선 사항
테스트