[BE] SICS1-212 [FIX] cookie 저장 실패 수정#117
Conversation
WalkthroughOAuth2 인증 구성에 환경 기반 설정이 추가됩니다. 쿠키에 도메인 속성이 추가되고, 실패 리다이렉트 및 CORS 설정이 프로덕션/개발 환경에 따라 동적으로 결정되도록 변경됩니다. 기존 쿠키의 다른 속성은 유지됩니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10분
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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 (2)
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java (2)
104-108: localhost를 domain 속성으로 설정하면 쿠키가 제대로 저장되지 않을 수 있습니다.RFC 6265에 따르면 localhost에는 domain 속성을 설정하지 않는 것이 권장됩니다. 많은 브라우저가
domain=localhost를 거부하거나 일관성 없이 처리할 수 있습니다. 로컬 환경에서는 domain 속성을 아예 생략해야 합니다.다음과 같이 수정하세요:
- boolean isProd = Arrays.asList(env.getActiveProfiles()).contains("prod"); - - String sameSite = isProd ? "None" : "Lax"; - boolean secure = isProd; - String domain = isProd ? "sisc-web.duckdns.org" : "localhost"; + boolean isProd = Arrays.asList(env.getActiveProfiles()).contains("prod"); + + String sameSite = isProd ? "None" : "Lax"; + boolean secure = isProd;그리고 쿠키 생성 시 조건부로 domain을 설정하세요:
ResponseCookie accessCookie = ResponseCookie.from("access", accessToken) .httpOnly(true) .secure(secure) // 로컬=false, 배포=true .sameSite(sameSite) // 로컬= "Lax", 배포="None" - .domain(domain) + .domain(isProd ? "sisc-web.duckdns.org" : null) // 로컬에서는 domain 속성 생략 .path("/") .maxAge(60L * 60) // 1 hour .build(); ResponseCookie refreshCookie = ResponseCookie.from("refresh", refreshToken) .httpOnly(true) .secure(secure) .sameSite(sameSite) - .domain(domain) + .domain(isProd ? "sisc-web.duckdns.org" : null) // 로컬에서는 domain 속성 생략 .path("/") .maxAge(60L * 60 * 24 * 14) // 2 weeks .build();
104-104: 환경 판별 로직이 SecurityConfig와 중복됩니다.
isProd로직이 SecurityConfig의isProd()메서드와 중복됩니다. 또한 구현 방식도 약간 다릅니다 (여기서는Arrays.asList, SecurityConfig에서는List.of). 공통 유틸리티 클래스나 설정 빈으로 추출하는 것을 고려하세요.예시:
@Component public class EnvironmentHelper { private final Environment env; public EnvironmentHelper(Environment env) { this.env = env; } public boolean isProd() { return Arrays.asList(env.getActiveProfiles()).contains("prod"); } }그리고 두 클래스 모두에서 주입받아 사용:
+ private final EnvironmentHelper environmentHelper; ... - boolean isProd = Arrays.asList(env.getActiveProfiles()).contains("prod"); + boolean isProd = environmentHelper.isProd();
🧹 Nitpick comments (2)
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java (1)
111-111: 불필요한 빈 줄입니다.이 빈 줄은 불필요해 보입니다. 제거를 고려하세요.
backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java (1)
73-79: 환경별 실패 리다이렉트 로직이 올바르게 구현되었습니다.프로덕션과 개발 환경에 맞는 리다이렉트 URL이 적절하게 설정되었습니다. 다만 URL을 application.yml에 외부화하면 유지보수성이 향상됩니다.
예시:
# application-prod.yml app: oauth2: redirect-failure: https://sisc-web.duckdns.org/oauth/fail # application-local.yml app: oauth2: redirect-failure: http://localhost:5173/oauth/fail@Value("${app.oauth2.redirect-failure}") private String redirectFailure; // ... .failureHandler((req, res, ex) -> { res.sendRedirect(redirectFailure); })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
backend/src/main/java/org/sejongisc/backend/common/auth/config/OAuth2SuccessHandler.java(2 hunks)backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java(4 hunks)
🔇 Additional comments (1)
backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java (1)
9-9: LGTM!환경 기반 설정을 위한 Environment 의존성 추가가 적절합니다.
Also applies to: 42-42
| private boolean isProd() { | ||
| return List.of(env.getActiveProfiles()).contains("prod"); | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
isProd() 메서드가 깔끔하게 구현되었습니다.
환경 판별 로직이 잘 캡슐화되어 있습니다. 하지만 OAuth2SuccessHandler에서 중복 구현되고 있으니, 공통 유틸리티로 추출하는 것을 고려하세요.
🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java
around lines 44 to 46, the isProd() method is implemented but duplicated in
OAuth2SuccessHandler; extract this environment-check into a shared utility
(e.g.,
backend/src/main/java/org/sejongisc/backend/common/util/EnvironmentUtils.java
with a static isProd(String[] activeProfiles) or isProd(Environment env)
method), replace both SecurityConfig.isProd() and the duplicate in
OAuth2SuccessHandler to call the new utility, update imports and unit tests if
any, and remove the duplicated method from OAuth2SuccessHandler to centralize
the logic.
| config.setAllowedOriginPatterns(List.of( | ||
| "http://localhost:5173", | ||
| "https://sisc-web.duckdns.org" | ||
| )); |
There was a problem hiding this comment.
🧩 Analysis chain
setAllowedOriginPatterns 사용을 확인하세요.
setAllowedOrigins에서 setAllowedOriginPatterns로 변경되었습니다. allowCredentials(true)와 함께 사용할 때 와일드카드 패턴이 필요한 경우 setAllowedOriginPatterns가 필요하지만, 여기서는 구체적인 출처만 나열하고 있습니다.
현재 구현은 정상 작동하지만, 와일드카드를 사용하지 않는다면 setAllowedOrigins를 계속 사용해도 됩니다. setAllowedOriginPatterns는 패턴 매칭 오버헤드가 약간 있을 수 있습니다.
다음 스크립트로 CORS 설정이 올바르게 작동하는지 확인하세요:
🏁 Script executed:
#!/bin/bash
# Description: 두 출처에서 CORS preflight 요청 테스트
echo "Testing localhost origin..."
curl -X OPTIONS http://localhost:8080/api/auth/login \
-H "Origin: http://localhost:5173" \
-H "Access-Control-Request-Method: POST" \
-v 2>&1 | grep -i "access-control"
echo -e "\n\nTesting production origin..."
curl -X OPTIONS https://sisc-web.duckdns.org/api/auth/login \
-H "Origin: https://sisc-web.duckdns.org" \
-H "Access-Control-Request-Method: POST" \
-v 2>&1 | grep -i "access-control"Length of output: 425
로컬호스트 CORS 요청이 실패하고 있습니다. 추가 정보가 필요합니다.
🏁 Script executed:
#!/bin/bash
# SecurityConfig 파일의 전체 CORS 설정 확인
echo "=== SecurityConfig.java 전체 내용 확인 ==="
find . -name "SecurityConfig.java" -type f | head -5
echo -e "\n=== CORS 관련 설정 확인 ==="
find . -name "SecurityConfig.java" -exec grep -A 20 -B 5 "setAllowedOriginPatterns\|setAllowedOrigins\|CORS\|cors" {} +
echo -e "\n=== 프로젝트에서 CorsConfigurationSource 검색 ==="
rg -n "CorsConfigurationSource|cors" --type=java -A 3 -B 3Length of output: 7691
🏁 Script executed:
#!/bin/bash
# 전체 SecurityConfig.java 파일 확인 및 CORS 설정 검증
echo "=== SecurityConfig.java 파일 위치 찾기 ==="
find . -name "SecurityConfig.java" -type f
echo -e "\n=== SecurityConfig.java 전체 내용 ==="
cat ./backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java
echo -e "\n\n=== 프로젝트 전체에서 CORS 관련 설정 검색 ==="
rg -n "CorsConfigurationSource|setAllowedOrigin|allowCredentials" --type=java -B 2 -A 2Length of output: 9264
localhost CORS 프리플라이트 요청이 실패하고 있습니다. 즉시 수정 필요.
setAllowedOriginPatterns은 와일드카드 패턴을 위한 것("*" 및 포트 범위)이며 정확한 문자열 매칭을 위한 것이 아닙니다. 현재 코드는 문자열 리터럴로 이 메서드를 사용하고 있어서 localhost 요청이 CORS 필터링을 통과하지 못합니다.
setAllowedOrigins로 되돌리세요 (라인 124-127). setAllowedOriginPatterns은 allowCredentials(true)와 함께 사용할 수 있지만, 와일드카드 패턴을 위한 것입니다. 와일드카드가 필요 없으면 setAllowedOrigins이 올바른 선택입니다:
config.setAllowedOrigins(List.of(
"http://localhost:5173",
"https://sisc-web.duckdns.org"
));🤖 Prompt for AI Agents
In
backend/src/main/java/org/sejongisc/backend/common/auth/config/SecurityConfig.java
around lines 124-127, replace the call to config.setAllowedOriginPatterns(...)
with config.setAllowedOrigins(...) using the same list of exact origin strings
("http://localhost:5173" and "https://sisc-web.duckdns.org"); update the code so
it uses setAllowedOrigins(List.of(...)) instead of setAllowedOriginPatterns to
ensure exact-origin matching for CORS preflight requests.
2 participants
Summary by CodeRabbit
릴리스 노트
Bug Fixes
Chores