Conversation
Walkthrough보안 설정을 단일 체인에서 두 개의 SecurityFilterChain(loginChain, apiChain)으로 분리했다. 생성자에 RefreshTokenService를 주입하고 로그아웃에서 이를 사용한다. JWTFilter는 apiChain에 배치되며, CORS/CSRF/세션 정책과 공개/인증 경로가 각각의 체인에서 재구성되었다. application-dev.yml은 실질 변경 없음. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant C as Client
participant A as App (Spring)
participant L as Security (loginChain)
participant O as OAuth2 Provider
note over L: Order 0 - OAuth2/회원가입 경로 전용
C->>A: /oauth2/authorization/... 또는 /login/** 등
A->>L: Security match (loginChain)
L-->>C: PermitAll for 공개 경로
C->>A: OAuth2 로그인 요청
A->>L: OAuth2Login 처리
L->>O: Authorization Code Flow
O-->>L: 사용자 정보
L->>A: CustomUserService + SuccessHandler
A-->>C: 로그인 성공 응답 (세션 IF_REQUIRED)
sequenceDiagram
autonumber
participant C as Client
participant A as App (Spring)
participant S as Security (apiChain)
participant J as JWTFilter
participant R as RefreshTokenService
note over S: Order 1 - API 경로, STATELESS
C->>A: API 요청
A->>S: Security match (apiChain)
S->>J: JWTFilter (pre OAuth2LoginAuthenticationFilter)
J->>S: 인증 컨텍스트 설정/미설정
alt 공개 경로
S-->>C: PermitAll 응답
else 인증 필요
S->>A: 컨트롤러 호출(인증됨) 또는 401
A-->>C: 응답
end
rect rgba(230,255,230,0.5)
note right of C: 로그아웃 흐름
C->>A: POST /logout (refresh_token 쿠키 포함)
A->>S: Logout 처리
S->>R: refresh_token 무효화
S->>C: Authorization: Bearer "" 헤더로 정리
S->>C: refresh_token 쿠키 삭제 (MaxAge=0, Path="/")
S-->>C: 세션 무효화 후 성공 응답
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Pre-merge checks (3 passed, 1 warning, 1 inconclusive)❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (3 passed)
Poem
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: 2
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/hello/cluebackend/global/config/SecurityConfig.java (1)
64-93: 로그아웃 핸들러의 예외 처리 및 응답 헤더 처리 수정 필요.
- refresh_token 쿠키 없음을 예외로 던지면 로그아웃이 500/401로 종료될 수 있어 비(idempotent)합니다. 존재하지 않아도 성공으로 처리하세요.
- 응답의
Authorization헤더를"Bearer "로 설정하는 것은 표준이 아니며 무의미합니다. 제거하세요.수정 예:
- if (refreshToken == null) { - throw new AuthenticationCredentialsNotFoundException("refresh_token 쿠키가 존재하지 않습니다."); - } - refreshTokenService.deleteByRefresh(refreshToken); + if (refreshToken != null) { + refreshTokenService.deleteByRefresh(refreshToken); + } ... - response.setHeader("Authorization", "Bearer "); + // 응답에 Authorization 헤더 설정 불필요. 제거.추가로, 삭제 쿠키에 보안 속성 부여 권장:
Cookie refreshTokenCookie = new Cookie("refresh_token", null); refreshTokenCookie.setMaxAge(0); refreshTokenCookie.setPath("/"); + refreshTokenCookie.setHttpOnly(true); + refreshTokenCookie.setSecure(true); response.addCookie(refreshTokenCookie);
🧹 Nitpick comments (3)
src/main/resources/application-dev.yml (1)
13-13: EOF 개행 추가 권장.YAMLlint가 말미 개행 누락을 지적합니다. 한 줄 개행을 추가해 린트 노이즈를 제거하세요.
src/main/java/hello/cluebackend/global/config/SecurityConfig.java (2)
61-64: (선택) api 체인에도 명시적 matcher를 지정해 충돌 여지를 줄이세요.
apiChain이 나머지 모든 요청을 받는 구조보다/api/**,/logout,/refresh-token등으로 한정하면 체인 경계가 명확해집니다.예시:
- http + http + .securityMatcher("/api/**", "/logout", "/refresh-token") .cors(Customizer.withDefaults()) .csrf(csrf -> csrf.disable())
96-99: (선택) H2 콘솔 사용 시 frame 옵션 비활성 필요.
/h2-console/**접근만 허용하면 UI가 frame 정책에 막힐 수 있습니다. 개발 프로파일 한정으로headers().frameOptions().disable()추가를 고려하세요.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/hello/cluebackend/global/config/SecurityConfig.java(3 hunks)src/main/resources/application-dev.yml(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-07-08T23:38:29.680Z
Learnt from: fixgramwork
PR: EscapeFrame/clueBackend#23
File: src/main/java/hello/cluebackend/domain/user/service/RegisterUserService.java:17-23
Timestamp: 2025-07-08T23:38:29.680Z
Learning: This project uses OAuth2 authentication and does not require traditional password storage or validation. The RegisterUserService in src/main/java/hello/cluebackend/domain/user/service/RegisterUserService.java should not store passwords since OAuth2 handles authentication externally.
Applied to files:
src/main/java/hello/cluebackend/global/config/SecurityConfig.java
🧬 Code graph analysis (1)
src/main/java/hello/cluebackend/global/config/SecurityConfig.java (1)
src/main/java/hello/cluebackend/global/config/JWTFilter.java (2)
JWTFilter(19-71)Override(27-70)
🪛 YAMLlint (1.37.1)
src/main/resources/application-dev.yml
[error] 13-13: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
src/main/resources/application-dev.yml (1)
11-13: 의미 변경 없음(LGTM).
logging.level.root: debug위치만 이동되어 동작 영향 없습니다.src/main/java/hello/cluebackend/global/config/SecurityConfig.java (5)
9-9: 필요한 import 추가 OK.
@Order,Customizer추가는 현재 설정에 적합합니다.Also applies to: 11-11
25-35: 생성자 주입 +final필드 유지, 적절합니다.
RefreshTokenService를 필드로 보관해 로그아웃에서 사용하는 변경 방향은 타당합니다.
95-101: 공개 경로 재확인.현재 공개 경로에 루트, 문서, H2 콘솔,
/refresh-token만 포함됩니다. 상기 코멘트대로/api/timetable/**가 공개 API라면 여기서만 permitAll 하도록 정렬하세요. 아니라면 보호 대상으로 남겨두세요.
103-103: 세션 정책 설정 적절(LGTM).API 체인을
STATELESS로 둔 점은 의도와 부합합니다.
62-64: 전역 CORS가 이미 설정되어 있어 우려 해소됨.src/main/java/hello/cluebackend/global/config/CorsMvcConfig.java — WebMvcConfigurer 구현부의 addCorsMappings에서 registry.addMapping("/**").allowedOriginPatterns("*")로 전역 CORS를 등록하고 있습니다.
| @Order(0) | ||
| public SecurityFilterChain loginChain(HttpSecurity http) throws Exception { | ||
| http | ||
| .securityMatcher("/oauth2/**", "/login/oauth2/**", "/first-register", "/register", "/api/timetable/**") | ||
| .cors(Customizer.withDefaults()) | ||
| .csrf(csrf -> csrf.disable()) | ||
| .authorizeHttpRequests(a -> a | ||
| .requestMatchers("/oauth2/**", "/login/oauth2/**", "/first-register", "/register", "/api/timetable/**") | ||
| .permitAll() | ||
| .anyRequest().permitAll() | ||
| ) | ||
| .oauth2Login(o -> o | ||
| .userInfoEndpoint(u -> u.userService(customOAuth2UserService)) | ||
| .successHandler(customSuccessHandler) | ||
| ) | ||
| .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)); | ||
|
|
||
| return http.build(); | ||
| } |
There was a problem hiding this comment.
login 체인에 API 경로 포함 — 인증 우회 위험.
loginChain.securityMatcher와 authorizeHttpRequests에 /api/timetable/**가 포함되어 Order(0) 체인이 먼저 매치되면 JWT가 적용되는 apiChain까지 도달하지 못하고 무조건 permitAll이 됩니다. 의도치 않은 익명 접근/권한 우회가 발생할 수 있습니다.
다음과 같이 /api/timetable/**를 login 체인에서 제거하고, 공개가 필요하다면 api 체인에서 명시적으로 permitAll 하십시오.
- .securityMatcher("/oauth2/**", "/login/oauth2/**", "/first-register", "/register", "/api/timetable/**")
+ .securityMatcher("/oauth2/**", "/login/oauth2/**", "/first-register", "/register")
...
- .requestMatchers("/oauth2/**", "/login/oauth2/**", "/first-register", "/register", "/api/timetable/**")
+ .requestMatchers("/oauth2/**", "/login/oauth2/**", "/first-register", "/register")
.permitAll()그리고 api 체인 쪽에 공개가 맞다면 추가:
- "/swagger-ui/**", "/swagger-resources/**", "/v3/api-docs/**"
+ "/swagger-ui/**", "/swagger-resources/**", "/v3/api-docs/**",
+ "/api/timetable/**"📝 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.
| @Order(0) | |
| public SecurityFilterChain loginChain(HttpSecurity http) throws Exception { | |
| http | |
| .securityMatcher("/oauth2/**", "/login/oauth2/**", "/first-register", "/register", "/api/timetable/**") | |
| .cors(Customizer.withDefaults()) | |
| .csrf(csrf -> csrf.disable()) | |
| .authorizeHttpRequests(a -> a | |
| .requestMatchers("/oauth2/**", "/login/oauth2/**", "/first-register", "/register", "/api/timetable/**") | |
| .permitAll() | |
| .anyRequest().permitAll() | |
| ) | |
| .oauth2Login(o -> o | |
| .userInfoEndpoint(u -> u.userService(customOAuth2UserService)) | |
| .successHandler(customSuccessHandler) | |
| ) | |
| .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)); | |
| return http.build(); | |
| } | |
| @Order(0) | |
| public SecurityFilterChain loginChain(HttpSecurity http) throws Exception { | |
| http | |
| .securityMatcher("/oauth2/**", "/login/oauth2/**", "/first-register", "/register") | |
| .cors(Customizer.withDefaults()) | |
| .csrf(csrf -> csrf.disable()) | |
| .authorizeHttpRequests(a -> a | |
| .requestMatchers("/oauth2/**", "/login/oauth2/**", "/first-register", "/register") | |
| .permitAll() | |
| .anyRequest().permitAll() | |
| ) | |
| .oauth2Login(o -> o | |
| .userInfoEndpoint(u -> u.userService(customOAuth2UserService)) | |
| .successHandler(customSuccessHandler) | |
| ) | |
| .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.IF_REQUIRED)); | |
| return http.build(); | |
| } |
🤖 Prompt for AI Agents
In src/main/java/hello/cluebackend/global/config/SecurityConfig.java around
lines 38 to 56, the loginChain currently includes "/api/timetable/**" in its
securityMatcher and permitAll requestMatchers which causes the Order(0) chain to
match API requests and bypass the JWT-protected apiChain; remove
"/api/timetable/**" from both the securityMatcher and authorizeHttpRequests
permit list in loginChain so that API requests reach the apiChain, and if
"/api/timetable/**" is intended to be public, explicitly add a permitAll matcher
for that path inside the apiChain (or adjust its authorization rules) so JWT
protection is applied correctly to other API endpoints.
| .addFilterBefore(new JWTFilter(jwtUtil), OAuth2LoginAuthenticationFilter.class) | ||
| .sessionManagement(s -> s.sessionCreationPolicy(SessionCreationPolicy.STATELESS)); |
There was a problem hiding this comment.
🛠️ Refactor suggestion
JWT 필터 삽입 위치를 UsernamePasswordAuthenticationFilter 앞로 조정하세요.
apiChain에는 OAuth2LoginAuthenticationFilter가 존재하지 않을 수 있어 상대 위치 기준이 불안정합니다. 일반적으로 JWT는 UsernamePasswordAuthenticationFilter보다 앞에 두는 것이 안전합니다.
아래와 같이 수정하고 import를 추가하세요.
+import org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
...
- .addFilterBefore(new JWTFilter(jwtUtil), OAuth2LoginAuthenticationFilter.class)
+ .addFilterBefore(new JWTFilter(jwtUtil), UsernamePasswordAuthenticationFilter.class)🤖 Prompt for AI Agents
In src/main/java/hello/cluebackend/global/config/SecurityConfig.java around
lines 102-103, move the JWT filter insertion to be before
UsernamePasswordAuthenticationFilter instead of OAuth2LoginAuthenticationFilter
and add the necessary import for
org.springframework.security.web.authentication.UsernamePasswordAuthenticationFilter;
update the DSL call to .addFilterBefore(new JWTFilter(jwtUtil),
UsernamePasswordAuthenticationFilter.class) so the JWTFilter runs prior to the
username/password authentication processing.
2 participants
📌 PR 제목 규칙
[#80] session 오류 해결
✅ 작업 내용
🔗 관련 이슈
Close #80
📌 체크리스트
Summary by CodeRabbit