-
Notifications
You must be signed in to change notification settings - Fork 2
fix: cookie max age 변경 - #193 #194
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
📝 WalkthroughSummary by CodeRabbit릴리스 노트
✏️ Tip: You can customize this high-level summary in your review settings. 요약 및 분석Walkthrough쿠키 max-age 설정을 일관되게 관리하기 위해 AuthController와 CookieCreatorUtil을 수정했습니다. 토큰별 expiration 파라미터를 제거하고 COOKIE_MAX_AGE 상수(90)를 도입하여 접근 및 갱신 토큰 쿠키에 고정 max-age를 적용했습니다. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 분
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
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/com/permitseoul/permitserver/domain/auth/core/jwt/CookieCreatorUtil.java (1)
23-41: Cookie max-age values must be synchronized with JWT token expiration times from JwtProperties.Both
createAccessTokenCookie()andcreateRefreshTokenCookie()use a hardcodedCOOKIE_MAX_AGEof 90 seconds (marked "테스트용" for testing), whileJwtGeneratorcorrectly assigns different expiration times to access and refresh tokens viaJwtProperties.accessTokenExpirationTime()andJwtProperties.refreshTokenExpirationTime().The unused helper method
toCookieMaxAgeSeconds()indicates this implementation is incomplete. Update both methods to:
- Use the appropriate JWT expiration time from
JwtPropertiesfor each token type- Convert milliseconds to seconds using
toCookieMaxAgeSeconds()- Ensure refresh token cookie max-age exceeds access token cookie max-age to match JWT design
🧹 Nitpick comments (1)
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/CookieCreatorUtil.java (1)
63-65: 사용되지 않는 메서드를 제거하세요.
toCookieMaxAgeSeconds메서드는 더 이상 호출되지 않아 데드 코드입니다. 코드 정리를 위해 제거하는 것을 권장합니다.다음 diff를 적용하여 사용되지 않는 메서드를 제거하세요:
- private static long toCookieMaxAgeSeconds(final long jwtExpirationMillis) { - return jwtExpirationMillis / 1000; - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/main/java/com/permitseoul/permitserver/domain/auth/api/controller/AuthController.java(1 hunks)src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/CookieCreatorUtil.java(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: sjk4618
Repo: PERMIT-SEOUL/permit-server PR: 48
File: src/main/java/com/permitseoul/permitserver/domain/guest/core/domain/entity/GuestEntity.java:31-37
Timestamp: 2025-07-15T09:37:32.765Z
Learning: sjk4618 prefers to implement factory methods or public constructors for entities when they are actually needed, rather than creating them proactively.
🧬 Code graph analysis (1)
src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/CookieCreatorUtil.java (1)
src/main/java/com/permitseoul/permitserver/global/Constants.java (1)
Constants(3-24)
🔇 Additional comments (1)
src/main/java/com/permitseoul/permitserver/domain/auth/api/controller/AuthController.java (1)
83-84: 메서드 호출이 올바르게 단순화되었습니다.
CookieCreatorUtil의 메서드 시그니처와 일치하게 호출되고 있습니다. 코드베이스 전체에서 이 메서드들은 토큰 값만 인자로 받아 호출되고 있으며, 다중 인자로 호출하는 레거시 코드는 없습니다.
| private static JwtProperties jwtProperties; | ||
| private static final long ACCESS_COOKIE_EXTRA_SECONDS = 5L * 60; // 5분 | ||
| private static final long REFRESH_COOKIE_EXTRA_SECONDS = 15L * 60; // 15분 | ||
| private static final long COOKIE_MAX_AGE = 90L; // 90초 테스트용 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🧩 Analysis chain
🏁 Script executed:
# First, let's read the full CookieCreatorUtil.java file to understand context
cat -n src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/CookieCreatorUtil.javaRepository: PERMIT-SEOUL/permit-server
Length of output: 2809
🏁 Script executed:
# Search for application configuration files
find . -type f \( -name "application*.properties" -o -name "application*.yml" -o -name "application*.yaml" \) | head -20Repository: PERMIT-SEOUL/permit-server
Length of output: 52
🏁 Script executed:
# Check if there are any configuration classes or beans related to cookie settings
rg -i "cookie.*max.*age|COOKIE_MAX_AGE" --type javaRepository: PERMIT-SEOUL/permit-server
Length of output: 643
🏁 Script executed:
# Check for environment-specific settings
fd -e properties -e yml -e yaml | grep -E "(prod|dev|test|config)" | head -20Repository: PERMIT-SEOUL/permit-server
Length of output: 52
🏁 Script executed:
# Check git history for CookieCreatorUtil.java
git log --oneline src/main/java/com/permitseoul/permitserver/domain/auth/core/jwt/CookieCreatorUtil.java | head -10Repository: PERMIT-SEOUL/permit-server
Length of output: 107
🏁 Script executed:
# Check for CI/CD or deployment files
find . -type f \( -name "*.yml" -o -name "*.yaml" -o -name "Dockerfile" -o -name ".github" -o -name "pom.xml" \) | head -20Repository: PERMIT-SEOUL/permit-server
Length of output: 222
🏁 Script executed:
# Check if this is a production project by looking at pom.xml or build files
ls -la | grep -E "(pom|build|gradle|docker)"Repository: PERMIT-SEOUL/permit-server
Length of output: 370
🏁 Script executed:
# Let's check the recent commit to see what "fix" was made
git show 91dc9fe --statRepository: PERMIT-SEOUL/permit-server
Length of output: 31912
Test configuration in production cookie settings.
The constant COOKIE_MAX_AGE = 90L with comment "90초 테스트용" is being used in both createAccessTokenCookie() and createRefreshTokenCookie(). A 90-second max-age is far too short for production use, particularly for refresh tokens, and will cause frequent user logouts.
This needs to be replaced with appropriate production values:
- Access token: 15-30 minutes
- Refresh token: 7-30 days
Also remove the unused toCookieMaxAgeSeconds() method (lines 63-65) and consider externalizing cookie lifetimes to configuration properties to allow environment-specific values.
🔥Pull requests
⛳️ 작업한 브랜치
👷 작업한 내용
🚨 참고 사항