refactor: refreshToken 쿠키에서 환경별로 Domain 설정하도록 #450
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
nayonsoso
requested review from
Gyuhyeok99,
lsy1307,
whqtker and
wibaek
as code owners
Walkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the "Integrations" page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
nayonsoso
commented
Aug 13, 2025
Collaborator
Author
nayonsoso
left a comment
nayonsoso
left a comment
There was a problem hiding this 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
🧹 Nitpick comments (5)
src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java (1)
7-16: 1) 입력 값 유효성 검증 추가를 권장합니다. 2) 오입력(예: 'localhost', 공백) 방어 코드를 추가하면 운영 안전성이 올라갑니다.
- @validated + @pattern 으로 cookieDomain 형식을 사전에 검증할 수 있습니다.
- 운영 환경에서 'localhost' 나 IP, 빈 문자열 설정을 방지하면 예기치 못한 쿠키 동작을 줄일 수 있습니다.
다음과 같이 보완할 수 있습니다.
package com.example.solidconnection.auth.controller.config; +import jakarta.validation.constraints.Pattern; +import org.springframework.validation.annotation.Validated; import org.springframework.boot.context.properties.ConfigurationProperties; import org.springframework.boot.web.server.Cookie.SameSite; @ConfigurationProperties(prefix = "token.refresh") +@Validated public record RefreshTokenCookieProperties( - String cookieDomain + // 선행 점(.) 허용, 다중 레이블 도메인 허용, TLD 2자 이상 + @Pattern(regexp = "^\\.?([a-zA-Z0-9-]+\\.)+[a-zA-Z]{2,}$", message = "유효한 도메인 형식이어야 합니다.") + String cookieDomain ) {참고: dev에서는 속성 미지정(null)로 운용하므로, 위 @pattern은 prod/stage 등 실제 도메인이 지정되는 프로필에서만 적용되도록 프로필 분리 또는 값 존재 시에만 검사하는 커스텀 Validator 적용을 고려할 수 있습니다.
src/test/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookiePropertiesTest.java (1)
24-35: 1) 공백 문자열 케이스도 테스트해주세요. 2) isDomainSet()의 공백 처리 로직을 검증하면 안정성이 올라갑니다.
- 현재 null 케이스만 검증 중입니다.
- " " 같은 공백 입력도 SameSite=None 을 반환하는지 확인해 주세요.
아래 테스트를 추가하면 좋습니다.
@Test void Domain이_공백이면_SameSite가_None() { // given RefreshTokenCookieProperties properties = new RefreshTokenCookieProperties(" "); // when String sameSite = properties.sameSite(); // then assertThat(sameSite).isEqualTo(SameSite.NONE.attributeValue()); }src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
25-27: 1) 파라미터 네이밍을 milliseconds로 통일하면 읽기 쉽습니다. 2) 주석과 용어 일관성을 챙기면 유지보수성이 올라갑니다.
- milliSeconds → milliseconds 로 소문자 s 일관화만 해도 가독성 개선에 도움이 됩니다.
- private long convertExpireTimeToCookieMaxAge(long milliSeconds) { - // jwt의 expireTime 단위인 millisecond를 cookie의 maxAge 단위인 second로 변환 - return milliSeconds / 1000; + private long convertExpireTimeToCookieMaxAge(long milliseconds) { + // JWT의 expireTime 단위(ms)를 Cookie의 maxAge 단위(s)로 변환 + return milliseconds / 1000; }src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java (2)
76-79: 1) SameSite 검증이 중복입니다. 2) 하드코딩된 "Strict" 검증은 제거해도 충분합니다.
- sameSite 변수가 이미 "Strict"를 가리키므로, "SameSite=Strict"와 "SameSite=" + sameSite 동시 검증은 중복입니다.
- 상수 하드코딩을 줄이면 변경에도 견고해집니다.
assertAll( () -> assertThat(header).isNotNull(), () -> assertThat(header).contains("refreshToken="), () -> assertThat(header).contains("HttpOnly"), () -> assertThat(header).contains("Secure"), () -> assertThat(header).contains("Path=/"), () -> assertThat(header).contains("Max-Age=0"), - () -> assertThat(header).contains("SameSite=Strict"), () -> assertThat(header).contains("Domain=" + domain), () -> assertThat(header).contains("SameSite=" + sameSite) );
21-34: 1) 도메인 미설정(dev) 시 동작을 보강 테스트로 확인해 주세요. 2) Domain 속성 미포함과 SameSite=None을 함께 검증하면 좋습니다.
- dev 시나리오에서는 cookieDomain이 null/blank 입니다.
- 이 경우 Domain 속성이 없어야 하고 SameSite=None 이어야 하므로, 명시적 테스트가 있으면 회귀를 막을 수 있습니다.
아래 테스트 추가를 제안합니다.
@Test void 도메인을_지정하지_않으면_Domain_속성이_없고_SameSite가_None이다() { // given MockHttpServletResponse response = new MockHttpServletResponse(); given(refreshTokenCookieProperties.cookieDomain()).willReturn(null); given(refreshTokenCookieProperties.sameSite()).willReturn("None"); // when cookieManager.setCookie(response, "test-refresh-token"); // then String header = response.getHeader("Set-Cookie"); assertAll( () -> assertThat(header).isNotNull(), () -> assertThat(header).doesNotContain("Domain="), () -> assertThat(header).contains("SameSite=None") ); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(2 hunks)src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookiePropertiesTest.java(1 hunks)src/test/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
RequiredArgsConstructor(35-144)
🔇 Additional comments (2)
src/test/resources/application.yml (1)
88-90: 1) 테스트 전용 cookie-domain 추가 OK. 2) 릴랙스드 바인딩으로 속성 매핑 안정적.
- token.refresh.cookie-domain → RefreshTokenCookieProperties.cookieDomain으로 무리 없이 바인딩됩니다.
- 테스트에서 Domain과 SameSite 검증이 가능해져 목적에 부합합니다.
src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java (1)
6-9: 확인: @ConfigurationPropertiesScan이 이미 등록되어 있어 빈 등록 누락 우려 없음.SolidConnectionApplication 클래스에 @ConfigurationPropertiesScan이 선언되어 token.refresh 프로퍼티 바인딩이 스캔/등록됩니다.
- 등록 위치 확인 — src/main/java/com/example/solidconnection/SolidConnectionApplication.java에 @ConfigurationPropertiesScan 존재.
- 프로퍼티 정의 — src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java에 @ConfigurationProperties(prefix = "token.refresh")로 선언됨.
- 사용 및 테스트 — src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java에서 주입 사용되며 테스트(src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java, src/test/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookiePropertiesTest.java)에서도 확인됨.
- 결론 — 원래 코멘트의 우려(스캔/등록 누락)는 해소되었으므로 추가 조치 불필요.
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (1)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
37-45: 도메인 미지정(dev) 시 Domain 속성은 생략되어야 합니다. 조건부 적용으로 안전하게 처리해 주세요.
- 현재는 domain(properties.cookieDomain())을 무조건 호출합니다. 대부분의 스프링 버전에서 null을 무시하지만, 명시적으로 조건을 두면 의도가 분명하고 버전 차이 리스크도 줄어듭니다.
- dev 시나리오에서는 Domain 생략이 요구사항입니다.
아래처럼 빌더를 분리하고 도메인만 조건부로 적용해 주세요.
- ResponseCookie cookie = ResponseCookie.from(COOKIE_NAME, refreshToken) - .httpOnly(true) - .secure(true) - .path(PATH) - .maxAge(maxAge) - .domain(properties.cookieDomain()) - .sameSite(properties.sameSite()) - .build(); + ResponseCookie.ResponseCookieBuilder builder = ResponseCookie.from(COOKIE_NAME, refreshToken) + .httpOnly(true) + .secure(true) + .path(PATH) + .maxAge(maxAge); + if (properties.cookieDomain() != null && !properties.cookieDomain().isBlank()) { + builder.domain(properties.cookieDomain()); + } + ResponseCookie cookie = builder + .sameSite(properties.sameSite()) + .build();
🧹 Nitpick comments (4)
src/test/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookiePropertiesTest.java (1)
24-35: 공백 문자열 케이스 테스트를 추가하면 회귀에 더 강해집니다.
- isDomainSet()는 공백도 “미지정”으로 분기하므로, " " 입력 시 SameSite=None을 보장하는 테스트를 하나 더 두면 안전합니다.
- 현재 null 케이스만 검증되므로 보완을 제안합니다.
아래 테스트를 추가해 주세요.
@@ void Domain을_지정하지_않았으면_SameSite가_None() { // given RefreshTokenCookieProperties properties = new RefreshTokenCookieProperties(null); // when String sameSite = properties.sameSite(); // then assertThat(sameSite).isEqualTo(SameSite.NONE.attributeValue()); } + + @Test + void Domain이_공백이면_SameSite가_None() { + // given + RefreshTokenCookieProperties properties = new RefreshTokenCookieProperties(" "); + + // when + String sameSite = properties.sameSite(); + + // then + assertThat(sameSite).isEqualTo(SameSite.NONE.attributeValue()); + }src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java (2)
76-79: SameSite 검증이 중복되었습니다. 한 줄만 남기면 충분합니다.
- “SameSite=Strict”와 “SameSite=" + sameSite”는 현재 동일한 검증입니다.
- 상수화된 sameSite를 사용하는 한 줄만 유지하면 테스트 의도가 더 명확합니다.
아래처럼 중복 한 줄을 제거해 주세요.
@@ - () -> assertThat(header).contains("SameSite=Strict"), () -> assertThat(header).contains("Domain=" + domain), () -> assertThat(header).contains("SameSite=" + sameSite)
36-57: 도메인 미설정(dev) 시 동작을 검증하는 테스트를 하나 더 추가하면 완결됩니다.
- cookieDomain()이 null 또는 공백일 때 Domain 속성이 생략되고, SameSite=None 이 적용되는지 통합 테스트로도 확인하면 좋습니다.
- 현재는 prod 시나리오(도메인/Strict)만 커버하고 있습니다.
아래 테스트 추가를 제안드립니다.
@@ class RefreshTokenCookieManagerTest { @@ void 리프레시_토큰을_쿠키로_설정한다() { @@ } + @Test + void 리프레시_토큰_도메인_미지정_시_Domain_생략_및_SameSite_None() { + // given + MockHttpServletResponse response = new MockHttpServletResponse(); + String refreshToken = "test-refresh-token"; + given(refreshTokenCookieProperties.cookieDomain()).willReturn(null); + given(refreshTokenCookieProperties.sameSite()).willReturn("None"); + + // when + cookieManager.setCookie(response, refreshToken); + + // then + String header = response.getHeader("Set-Cookie"); + assertAll( + () -> assertThat(header).isNotNull(), + () -> assertThat(header).contains("refreshToken=" + refreshToken), + () -> assertThat(header).contains("HttpOnly"), + () -> assertThat(header).contains("Secure"), + () -> assertThat(header).contains("Path=/"), + () -> assertThat(header).contains("SameSite=None"), + () -> assertThat(header).doesNotContain("Domain=") // 미지정 시 생략 + ); + }src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
25-28: 시간 단위 변환은 TimeUnit을 쓰면 가독성이 더 좋아집니다.
- 의미를 더 분명히 전달하고 실수를 줄일 수 있습니다.
- 동작은 동일합니다.
아래처럼 변경을 고려해 주세요.
- private long convertExpireTimeToCookieMaxAge(long milliSeconds) { - // jwt의 expireTime 단위인 millisecond를 cookie의 maxAge 단위인 second로 변환 - return milliSeconds / 1000; - } + private long convertExpireTimeToCookieMaxAge(long milliSeconds) { + // jwt의 expireTime(ms) → cookie의 maxAge(s) + return java.util.concurrent.TimeUnit.MILLISECONDS.toSeconds(milliSeconds); + }필요 시 파일 상단에 별도 import 없이 정적 호출로도 충분합니다.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java(2 hunks)src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java(1 hunks)src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java(3 hunks)src/test/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookiePropertiesTest.java(1 hunks)src/test/resources/application.yml(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java (1)
src/main/java/com/example/solidconnection/auth/controller/AuthController.java (1)
RequiredArgsConstructor(35-144)
🔇 Additional comments (3)
src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java (2)
11-16: SameSite 동적 결정 로직이 요구사항과 정확히 일치합니다.
- 도메인 지정 시 Strict, 미지정 시 None 반환이 prod/dev 전략과 정확히 매칭됩니다.
- Secure 처리는 매니저에서 일괄 적용하므로 조합도 안전합니다.
6-9: @ConfigurationProperties 스캔 활성화 확인 — 조치 불필요SolidConnectionApplication에 @ConfigurationPropertiesScan가 선언되어 있어 런타임에 @ConfigurationProperties 레코드가 바인딩됩니다.
스캔 등록 확인
- src/main/java/com/example/solidconnection/SolidConnectionApplication.java:10에 @ConfigurationPropertiesScan가 있습니다.
프로퍼티 레코드 참조
- src/main/java/com/example/solidconnection/auth/controller/config/RefreshTokenCookieProperties.java는 @ConfigurationProperties(prefix = "token.refresh") 레코드입니다.
- src/main/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManager.java에서 생성자 주입으로 사용됩니다.
테스트 관련 주의
- src/test/java/com/example/solidconnection/auth/controller/RefreshTokenCookieManagerTest.java는 @MockBean으로 RefreshTokenCookieProperties를 주입하므로, 테스트만으로는 스캔 누락을 발견하기 어렵습니다.
- 하지만 현재 애플리케이션 레벨에서 스캔이 활성화되어 있어 런타임 바인딩에는 문제가 없습니다.
따라서 별도 수정은 필요하지 않습니다.
src/test/resources/application.yml (1)
88-90: 테스트용 cookie-domain 바인딩이 올바르게 연결됩니다.
- token.refresh.cookie-domain → RefreshTokenCookieProperties.cookieDomain 매핑이 정상입니다.
- 운영/스테이지 환경에서는 실제 도메인(.solid-connection.com 등)으로 오버라이드되면 됩니다.
nayonsoso
changed the title
Gyuhyeok99
approved these changes
Aug 14, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
관련 이슈
작업 내용
🔽 프론트 만옥님이 말씀하신 문제 상황입니다
api.stage.solid-connection.comwww.stage.solid-connection.com쿠키를 발급하는 서버의 도메인 (api.stage.xxx) 로 자동 설정된다고 합니다 😓
환경별로 나누어 문제를 해결합니다.
Domain=.solid-connection.com; Path=/; HttpOnly; Secure; SameSite=Strict;Domain=api.stage.solid-connection.com;(생략 시, 자동으로 서버의 도메인 할당됨) Path=/; HttpOnly; Secure; SameSite=Node;추가 요구사항으로 https로 localhost에서 테스트할 수 있도록 cors 허용 리스트에
https://localhost:3000을 추가합니다특이 사항
시크릿 레포에도 PR 올렸습니다 🏃♀️
https://github.com/solid-connection/solid-connect-secret/pull/18/files