Conversation
Walkthrough보안 설정에서 "/test" 경로를 공개로 추가하고, 로그인 성공 시 쿠키의 Secure 속성을 활성화했습니다. 애플리케이션 설정에 세션 쿠키의 SameSite=None 및 secure=true를 추가했습니다. 사용자 등록 컨트롤러에는 사용되지 않는 @Autowired import가 추가되었습니다. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor C as Client
participant SF as SecurityFilterChain
participant TC as TestController (/test)
C->>SF: GET /test (Unauthenticated)
rect rgba(200,230,255,0.3)
note right of SF: "/test"가 permitAll
SF-->>C: Allow without auth
end
SF->>TC: Forward request
TC-->>C: 200 OK (response)
sequenceDiagram
autonumber
actor U as User
participant OAuth as OAuth2 Login
participant SH as CustomSuccessHandler
participant R as Response
U->>OAuth: Complete OAuth2 login
OAuth-->>SH: onAuthenticationSuccess
rect rgba(220,255,220,0.3)
SH->>R: Set cookie (HttpOnly, Secure=true, MaxAge=7d, Path="/")
note right of R: Session cookie: SameSite=None, Secure=true (application.yaml)
end
R-->>U: Redirect / success response
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings, 1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests
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. 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: 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/hello/cluebackend/global/config/CustomSuccessHandler.java (1)
70-78: refresh_token 쿠키에 SameSite=None 누락 — 크로스사이트 전송 실패 가능
server.servlet.session.cookie.same-site: none은 JSESSIONID에만 적용됩니다. 여기서 직접 생성하는refresh_token쿠키는jakarta.servlet.http.Cookie로는 SameSite를 설정할 수 없어, 크로스 도메인 환경에서 브라우저가 쿠키를 보내지 않을 수 있습니다.Secure활성화는 잘 추가되었지만 SameSite=None도 함께 포함해야 합니다.ResponseCookie로 전환해 Set-Cookie 헤더를 직접 추가하는 방식을 권장합니다.아래 변경으로 SameSite=None을 명시하세요(호출부도 함께 수정 필요).
- private Cookie createCookie(String key, String value) { - Cookie cookie = new Cookie(key, value); - cookie.setMaxAge(7 * 24 * 60 * 60); - cookie.setSecure(true); - cookie.setPath("/"); - cookie.setHttpOnly(true); - - return cookie; - } + private String createCookie(String key, String value) { + ResponseCookie cookie = ResponseCookie.from(key, value) + .httpOnly(true) + .secure(true) + .sameSite("None") + .path("/") + .maxAge(Duration.ofDays(7)) + .build(); + return cookie.toString(); + }호출부(Line 64) 예시 수정:
// imports: org.springframework.http.ResponseCookie; java.time.Duration; response.addHeader("Set-Cookie", createCookie("refresh_token", refresh));참고: 개발 환경이 HTTPS가 아니면(예: http://localhost)
Secure쿠키는 세팅/전송되지 않습니다.
🧹 Nitpick comments (2)
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java (1)
10-10: 불필요한 import 제거
@Autowired를 사용하지 않습니다. 정리해 주세요.- import org.springframework.beans.factory.annotation.Autowired;src/main/java/hello/cluebackend/global/config/SecurityConfig.java (1)
84-88: 로그아웃 시 삭제 쿠키 속성 일치 권장원쿠키가
Secure(그리고 SameSite=None)라면 삭제 쿠키도 동일 속성으로 설정하는 편이 안전합니다.ResponseCookie로 삭제 헤더를 내려 일관성을 맞추는 것을 권장합니다.예시:
response.addHeader("Set-Cookie", ResponseCookie.from("refresh_token", "") .path("/") .httpOnly(true) .secure(true) .sameSite("None") .maxAge(0) .build() .toString() );
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
src/main/java/hello/cluebackend/domain/user/presentation/RegisterController.java(1 hunks)src/main/java/hello/cluebackend/global/config/CustomSuccessHandler.java(1 hunks)src/main/java/hello/cluebackend/global/config/SecurityConfig.java(1 hunks)src/main/resources/application.yaml(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/domain/user/presentation/RegisterController.java
🔇 Additional comments (2)
src/main/java/hello/cluebackend/global/config/SecurityConfig.java (1)
98-100: /test 공개 허용 범위 확인의도가 단일 엔드포인트 공개라면 현재(
/test)로 충분합니다. 하위 경로까지 공개하려면"/test/**"로 명시해야 합니다. 의도 확인 부탁드립니다.src/main/resources/application.yaml (1)
76-81: SameSite=None + Secure 설정 적절 — Spring Boot 3.4.4 확인; dev 환경 HTTPS 적용 필요build.gradle에서 'org.springframework.boot' 플러그인 버전 '3.4.4'로 확인되어 SameSite=None 지원(2.6+)은 충족됩니다.
src/main/resources/application.yaml (lines 76–81): JSESSIONID에 same-site: none 및 secure: true 설정이 적용되어 있으나, 개발 환경에서 HTTPS 미사용 시 세션 쿠키가 전송되지 않아 인증 흐름이 깨질 수 있으니 dev에서 HTTPS 사용(예: 로컬 리버스 프록시) 또는 프로파일별 쿠키 설정 분리 적용.
커스텀 refresh_token 쿠키는 별도 처리 필요.
2 participants
📌 PR 제목 규칙
[#81 ] 인가 제외
✅ 작업 내용
🔗 관련 이슈
Close #81
—
📌 체크리스트
Summary by CodeRabbit