Conversation
|
Warning Rate limit exceeded@vivivim has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 2 minutes and 19 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 (1)
WalkthroughJWT 기반 인증·인가와 Spring Security 통합이 추가되었습니다. 공개키 기반 JWT 파싱/검증, JwtAuthFilter, 인증/인가 오류 핸들러, 역할·워크스페이스 enum 및 역할 계층, ErrorStatus에 코드 추가와 ApiResponse 확장, 관련 예외 생성자 및 의존성 추가가 포함됩니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant JwtAuthFilter
participant JwtProvider
participant SecurityContext
participant CustomAuthEntryPoint
participant Resource
Client->>JwtAuthFilter: 요청 (쿠키/헤더에 토큰)
JwtAuthFilter->>JwtProvider: resolveAccessToken(request)
JwtProvider-->>JwtAuthFilter: token or null
alt token 없음/빈값
JwtAuthFilter->>Resource: 필터 체인 계속 (인증 없음)
else 토큰 존재
JwtAuthFilter->>JwtProvider: parse(token)
JwtProvider-->>JwtAuthFilter: Claims
alt token.type == "service"
JwtAuthFilter->>SecurityContext: SVC_* 권한으로 인증 설정
JwtAuthFilter->>Resource: 필터 체인 계속
else token.type == "refresh"
JwtAuthFilter->>CustomAuthEntryPoint: NOT_ACCESS_TOKEN 위임
CustomAuthEntryPoint-->>Client: 401 JSON 오류 응답
else 일반 토큰
JwtAuthFilter->>SecurityContext: ROLE_<ROLE>, ROLE_<WORKSPACE> 설정
JwtAuthFilter->>Resource: 필터 체인 계속
end
alt 파싱/검증 실패
JwtAuthFilter->>CustomAuthEntryPoint: INVALID/EXPIRED 예외 위임
CustomAuthEntryPoint-->>Client: 401 JSON 오류 응답
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 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 |
33Auto-Bot
requested review from
CHOOSLA,
Lee-Jong-Jin,
Sangyoon98,
taemin3 and
yangjiseonn
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| public SecurityFilterChain filterChain(HttpSecurity http, JwtAuthFilter jwtAuthFilter, CustomAuthEntryPoint customAuthEntryPoint, CustomAccessDeniedHandler customAccessDeniedHandler) throws Exception { | ||
| http | ||
| // CodeQL [java/spring-disabled-csrf-protection]: suppress - Stateless JWT API라 CSRF 불필요 | ||
| .csrf(csrf -> csrf.disable()) |
Check failure
Code scanning / CodeQL
Disabled Spring CSRF protection High
Show autofix suggestion
Hide autofix suggestion
Copilot Autofix
AI 4 months ago
To address the CSRF vulnerability detected by CodeQL, remove the explicit disabling of CSRF protection so that Spring Security's default CSRF protection remains enabled. This is done by removing (or commenting out) .csrf(csrf -> csrf.disable()). This change leaves CSRF protection on by default, which protects against cross-site request forgery when using browser clients. No further code changes are necessary in the bean unless special exceptions for non-browser clients are needed (not in scope of this fix).
Region to edit: In file src/main/java/com/sampoom/backend/common/config/security/SecurityConfig.java, remove or comment out line 27 which disables CSRF, and adjust the method chain so the builder is syntactically correct.
No imports, new methods, or definitions are needed.
| @@ -23,8 +23,7 @@ | ||
| @Bean | ||
| public SecurityFilterChain filterChain(HttpSecurity http, JwtAuthFilter jwtAuthFilter, CustomAuthEntryPoint customAuthEntryPoint, CustomAccessDeniedHandler customAccessDeniedHandler) throws Exception { | ||
| http | ||
| // CodeQL [java/spring-disabled-csrf-protection]: suppress - Stateless JWT API라 CSRF 불필요 | ||
| .csrf(csrf -> csrf.disable()) | ||
| // CSRF protection enabled (default). Remove explicit .disable(). | ||
| .authorizeHttpRequests(auth -> auth | ||
| // Feign용 POST만 허용 | ||
| .requestMatchers(HttpMethod.POST, "/internal/**").hasAuthority("SVC_AUTH") |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
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/com/sampoom/backend/common/config/swagger/SwaggerConfig.java (2)
18-22: 주석 처리된 JWT 설정 정리 필요토큰 검증 기능을 추가하는 PR이지만, JWT 헤더 설정이 주석 처리되어 있습니다. 이 설정이 필요하다면 활성화하고, 더 이상 필요하지 않다면 삭제해주세요. 주석 처리된 코드를 남겨두면 코드베이스의 유지보수성이 저하됩니다.
42-75: Swagger JWT 보안 설정 활성화 또는 삭제 필요토큰 검증 기능이 추가되었지만, JWT 보안 스키마가 포함된 대체 OpenAPI 빈 설정(33줄)이 주석 처리되어 있습니다. 이 설정은 Swagger UI에서 JWT 토큰을 테스트할 수 있도록 하는 것으로 보입니다.
다음 중 하나를 선택해주세요:
- 옵션 1 (권장): 현재 활성화된
openAPI()메서드에 JWT 보안 스키마를 추가하여 Swagger UI에서 토큰 기반 인증을 테스트할 수 있도록 합니다- 옵션 2: 이 주석 처리된 코드가 더 이상 필요하지 않다면 완전히 삭제합니다
주석 처리된 대체 구현을 남겨두는 것은 코드의 명확성을 저하시킵니다.
옵션 1을 선택하신다면, JWT 보안 설정을 활성화된 빈에 통합하는 코드를 생성해드릴까요?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (17)
build.gradle(1 hunks)src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java(1 hunks)src/main/java/com/sampoom/backend/common/config/security/CustomAccessDeniedHandler.java(1 hunks)src/main/java/com/sampoom/backend/common/config/security/CustomAuthEntryPoint.java(1 hunks)src/main/java/com/sampoom/backend/common/config/security/RoleHierarchyConfig.java(1 hunks)src/main/java/com/sampoom/backend/common/config/security/SecurityConfig.java(1 hunks)src/main/java/com/sampoom/backend/common/config/swagger/SwaggerConfig.java(1 hunks)src/main/java/com/sampoom/backend/common/entity/Role.java(1 hunks)src/main/java/com/sampoom/backend/common/entity/Workspace.java(1 hunks)src/main/java/com/sampoom/backend/common/exception/BadRequestException.java(2 hunks)src/main/java/com/sampoom/backend/common/exception/BaseException.java(2 hunks)src/main/java/com/sampoom/backend/common/exception/CustomAuthenticationException.java(1 hunks)src/main/java/com/sampoom/backend/common/exception/UnauthorizedException.java(2 hunks)src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java(1 hunks)src/main/java/com/sampoom/backend/common/jwt/JwtProvider.java(1 hunks)src/main/java/com/sampoom/backend/common/response/ApiResponse.java(2 hunks)src/main/java/com/sampoom/backend/common/response/ErrorStatus.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (6)
src/main/java/com/sampoom/backend/common/jwt/JwtProvider.java (3)
src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
BadRequestException(6-18)src/main/java/com/sampoom/backend/common/exception/CustomAuthenticationException.java (1)
CustomAuthenticationException(6-17)src/main/java/com/sampoom/backend/common/exception/UnauthorizedException.java (1)
UnauthorizedException(6-18)
src/main/java/com/sampoom/backend/common/config/security/RoleHierarchyConfig.java (1)
src/main/java/com/sampoom/backend/common/config/security/SecurityConfig.java (1)
Configuration(19-83)
src/main/java/com/sampoom/backend/common/config/security/CustomAuthEntryPoint.java (4)
src/main/java/com/sampoom/backend/common/exception/CustomAuthenticationException.java (1)
CustomAuthenticationException(6-17)src/main/java/com/sampoom/backend/common/config/security/CustomAccessDeniedHandler.java (1)
Slf4j(16-41)src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java (1)
Slf4j(26-137)src/main/java/com/sampoom/backend/common/jwt/JwtProvider.java (1)
Component(19-89)
src/main/java/com/sampoom/backend/common/config/security/CustomAccessDeniedHandler.java (2)
src/main/java/com/sampoom/backend/common/config/security/CustomAuthEntryPoint.java (1)
Slf4j(16-47)src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java (1)
Slf4j(26-137)
src/main/java/com/sampoom/backend/common/config/security/SecurityConfig.java (1)
src/main/java/com/sampoom/backend/common/config/swagger/SwaggerConfig.java (1)
Configuration(15-76)
src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java (3)
src/main/java/com/sampoom/backend/common/exception/CustomAuthenticationException.java (1)
CustomAuthenticationException(6-17)src/main/java/com/sampoom/backend/common/config/security/CustomAuthEntryPoint.java (1)
Slf4j(16-47)src/main/java/com/sampoom/backend/common/jwt/JwtProvider.java (1)
Component(19-89)
🪛 GitHub Check: CodeQL
src/main/java/com/sampoom/backend/common/config/security/SecurityConfig.java
[failure] 27-27: Disabled Spring CSRF protection
CSRF vulnerability due to protection being disabled.
⏰ 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: Analyze (java-kotlin)
🔇 Additional comments (13)
src/main/java/com/sampoom/backend/common/config/swagger/SwaggerConfig.java (1)
26-32: 원본 리뷰 의견 재검토 결과: URL 경로 불일치 우려는 타당하지 않음코드베이스 검증을 통해 로컬과 프로덕션 서버 URL의 경로 차이는 의도된 설정으로 보입니다:
- API 컨트롤러 경로:
/api/v1,/rop,/po,/branch등 다양하며 애플리케이션 내부에서/api/warehouse통일 경로를 사용하지 않음- 프로덕션 환경:
https://sampoom.store/api/warehouse는 API 게이트웨이나 리버스 프록시에서 추가되는 라우팅 경로로 추정- 로컬 개발: 애플리케이션에 직접 접근하므로 게이트웨이 경로 불필요
현재 Swagger 설정은 각 환경의 실제 접근 URL을 반영한 올바른 구성입니다. 다만 배포 인프라 구성 문서를 통해 이를 최종 확인해주시기 바랍니다.
src/main/java/com/sampoom/backend/common/exception/BaseException.java (1)
26-31: 새 에러 코드 생성자 동작 확인새 생성자가 상태/메시지와 함께 errorCode까지 초기화해 하위 예외들이 일관된 정보를 전달할 수 있게 됩니다. 추가 변경 없이도 정상 동작합니다.
src/main/java/com/sampoom/backend/common/entity/Workspace.java (1)
3-12: 워크스페이스 열거형 추가 확인조직별 워크스페이스 식별자가 명시적으로 정의되어 이후 토큰 클레임 파싱 시 안전하게 사용할 수 있습니다.
src/main/java/com/sampoom/backend/common/exception/CustomAuthenticationException.java (1)
6-16: 커스텀 인증 예외 도입 확인
ErrorStatus를 직접 보관해 인증 실패 시 표준화된 코드/메시지를 전달할 수 있어 보안 흐름에 필요한 확장성이 확보되었습니다.src/main/java/com/sampoom/backend/api/inventory/service/InventoryService.java (1)
295-296: BRANCH 오류 매핑 조정 확인 완료
BranchRepository.existsById검사와ErrorStatus.BRANCH_NOT_FOUND조합이 자연스러워졌습니다. 👍src/main/java/com/sampoom/backend/common/exception/UnauthorizedException.java (1)
15-17: ErrorStatus 생성자 추가 환영표준화된
ErrorStatus흐름과 잘 맞습니다. 👍src/main/java/com/sampoom/backend/common/exception/BadRequestException.java (1)
15-17: BadRequest에도 표준 오류 생성자 도입 👍
ErrorStatus기반 생성자로 응답 일관성이 높아졌습니다.src/main/java/com/sampoom/backend/common/entity/Role.java (1)
1-6: 기본 Role 정의 확인JWT 권한 파싱에 필요한 최소 롤이 정리되었습니다. 👌
src/main/java/com/sampoom/backend/common/config/security/RoleHierarchyConfig.java (1)
1-28: Role 계층 구성 잘 반영되었습니다권한 상위 롤이 하위 업무 롤과 USER 권한을 자연스럽게 포괄하도록 선언돼 있어 보안 규칙 관리가 수월해집니다. 💯
src/main/java/com/sampoom/backend/common/config/security/CustomAuthEntryPoint.java (1)
27-46: 일관된 인증 실패 응답 처리커스텀 예외에서 에러 상태를 추출해 표준화된 JSON 응답으로 직렬화하는 흐름이 명확하고 재사용성이 좋습니다.
src/main/java/com/sampoom/backend/common/jwt/JwtProvider.java (1)
24-53: RSA 공개키 검증 로직이 탄탄합니다Base64 디코딩과 2048비트 미만 키 차단으로 잘못된 키 주입을 초기에 방어하는 구성이 좋습니다.
src/main/java/com/sampoom/backend/common/config/security/SecurityConfig.java (1)
24-68: JWT 중심의 무상태 보안 구성이 명확합니다필수 엔드포인트만 허용하고 커스텀 인증/인가 핸들러를 연결한 설정이 일관된 오류 응답과 서비스 토큰 검증 흐름을 잘 뒷받침합니다.
src/main/java/com/sampoom/backend/common/config/security/CustomAccessDeniedHandler.java (1)
28-40: 인가 실패 응답 처리 흐름이 명확합니다공통 에러 포맷을 재사용해 403 응답을 즉시 직렬화하고 경고 로그를 남기는 구현이 일관성을 높입니다.
src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java
Outdated
Show resolved
Hide resolved
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java
Outdated
Show resolved
Hide resolved
에러 코드 오타 jwt 디버깅 주석 삭제
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (1)
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java (1)
62-62: 이전 리뷰 코멘트 반영 완료
ROP_NOT_FOUND의 에러 코드가 5자리50601로 올바르게 수정되었습니다. 이전 리뷰에서 지적된 6자리(506601) 오탈자 문제가 해결되었습니다.
🧹 Nitpick comments (4)
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java (1)
59-59: 에러 코드 자릿수 일관성 권장
NOT_FOUND(line 59)와CONFLICT(line 79)는 3자리 HTTP 코드(404, 409)를 그대로 사용하는 반면, 다른 모든 에러들은 5자리 커스텀 코드를 사용하고 있습니다. 코드 체계의 일관성을 위해 이들도 5자리 형식으로 통일하는 것을 고려해보세요.예시:
- NOT_FOUND(HttpStatus.NOT_FOUND, "요청한 리소스를 찾을 수 없습니다.", 404), + NOT_FOUND(HttpStatus.NOT_FOUND, "요청한 리소스를 찾을 수 없습니다.", 10404),- CONFLICT(HttpStatus.CONFLICT, "충돌이 발생했습니다.", 409), + CONFLICT(HttpStatus.CONFLICT, "충돌이 발생했습니다.", 10409),Also applies to: 79-79
src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java (3)
26-26: 사용하지 않는 @slf4j 애노테이션을 제거하세요.현재 코드에는 로그 출력문이 없어
@Slf4j애노테이션이 사용되지 않습니다. 불필요한 의존성을 제거하세요.-@Slf4j @Component @RequiredArgsConstructor public class JwtAuthFilter extends OncePerRequestFilter {
60-68: 역할 검증 로직을 간결하게 리팩터링할 수 있습니다.역할이 null인지, blank인지, "SVC_"로 시작하는지를 별도로 검증하고 있어 코드가 장황합니다. 조건을 결합하거나 helper 메서드로 추출하면 가독성이 향상됩니다.
- if (role == null) { - throw new CustomAuthenticationException(ErrorStatus.NULL_TOKEN_ROLE); - } - if (role.isBlank()) { - throw new CustomAuthenticationException(ErrorStatus.BLANK_TOKEN_ROLE); - } - if (!role.startsWith("SVC_")) { - throw new CustomAuthenticationException(ErrorStatus.NOT_SERVICE_TOKEN); - } + if (role == null || role.isBlank()) { + throw new CustomAuthenticationException( + role == null ? ErrorStatus.NULL_TOKEN_ROLE : ErrorStatus.BLANK_TOKEN_ROLE + ); + } + if (!role.startsWith("SVC_")) { + throw new CustomAuthenticationException(ErrorStatus.NOT_SERVICE_TOKEN); + }
94-102: 검증 로직을 더 간결하게 작성할 수 있습니다.여러 필드의 null/blank 검증을 하나의 복잡한 조건문으로 처리하고 있어 가독성이 떨어집니다. 각 필드를 순차적으로 검증하거나 helper 메서드를 사용하는 것을 고려하세요.
- if (userId == null - || userId.isBlank() - || roleStr == null - || roleStr.isBlank() - || workspaceStr == null - || workspaceStr.isBlank() - ) { - throw new CustomAuthenticationException(ErrorStatus.INVALID_TOKEN); - } + if (userId == null || userId.isBlank() || + roleStr == null || roleStr.isBlank() || + workspaceStr == null || workspaceStr.isBlank()) { + throw new CustomAuthenticationException(ErrorStatus.INVALID_TOKEN); + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java(1 hunks)src/main/java/com/sampoom/backend/common/response/ErrorStatus.java(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java (3)
src/main/java/com/sampoom/backend/common/exception/CustomAuthenticationException.java (1)
CustomAuthenticationException(6-17)src/main/java/com/sampoom/backend/common/config/security/CustomAuthEntryPoint.java (1)
Slf4j(16-47)src/main/java/com/sampoom/backend/common/jwt/JwtProvider.java (1)
Component(19-89)
⏰ 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: Analyze (java-kotlin)
🔇 Additional comments (5)
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java (1)
35-44: 토큰 검증 관련 에러 코드 추가 확인PR 목표에 부합하는 JWT 토큰 검증 및 인증/인가 관련 에러 코드들이 적절하게 추가되었습니다. 코드 범위도 논리적으로 그룹화되어 있습니다:
- 124xx: 토큰 검증 관련 (공개키, 토큰 형식, 만료 등)
- 114xx: 권한 및 입력값 검증 관련
Also applies to: 47-52
src/main/java/com/sampoom/backend/common/jwt/JwtAuthFilter.java (4)
84-87: 리프레시 토큰 거부 로직이 적절합니다.리프레시 토큰으로 API에 접근하는 것을 명시적으로 차단하고, 보안 컨텍스트를 정리한 후 적절한 에러를 반환합니다.
113-126: 권한 매핑과 인증 설정이 올바릅니다.Spring Security 규칙에 따라 "ROLE_" 접두사를 사용하고, userId를 principal로 설정하며, 역할과 워크스페이스를 권한으로 매핑하는 로직이 적절합니다. 이전 리뷰에서 지적된 민감한 정보 로깅도 제거되었습니다.
127-134: 예외 처리가 견고합니다.커스텀 인증 예외와 일반 예외를 모두 처리하고, 보안 컨텍스트를 정리한 후 진입점으로 위임하는 구조가 적절합니다. 예외 발생 시 부분적으로 인증된 상태가 남지 않도록 보장합니다.
45-49: 보안 설정이 올바르게 구현되어 있습니다.리뷰 코멘트의 우려는 해결되었습니다. SecurityConfig의
.anyRequest().authenticated()가 보호된 엔드포인트를 명시적으로 인증하도록 요구합니다. 토큰이 없거나 비어있을 때 JwtAuthFilter에서 필터 체인을 계속 진행하면, CustomAuthEntryPoint가 미인증 요청을 적절히 거부합니다. 이는 표준 Spring Security 패턴으로, 공개 엔드포인트(/swagger-ui, /invitations)와 보호된 엔드포인트를 효과적으로 구분합니다.Likely an incorrect or invalid review comment.
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java
Outdated
Show resolved
Hide resolved
어쓰에서 넘어온 건 그냥 살림
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/main/java/com/sampoom/backend/common/response/ErrorStatus.java(1 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: Analyze (java-kotlin)
2 participants
📝 Summary
토큰 검증 부분 추가
에러 반환 시 커스텀 에러 코드 추가
🙏 Question & PR point
📬 Reference
Summary by CodeRabbit
새로운 기능
버그 수정
문서