Skip to content

feat: OPTION 메서드 쿠키 요청 제외#122

Merged
wlgns12370 merged 3 commits intomainfrom
feat/#121
Mar 17, 2026
Merged

feat: OPTION 메서드 쿠키 요청 제외#122
wlgns12370 merged 3 commits intomainfrom
feat/#121

Conversation

@wlgns12370
Copy link
Contributor

@wlgns12370 wlgns12370 commented Mar 17, 2026

✨ 구현한 기능

Summary by CodeRabbit

주요 변경 사항

  • 버그 수정
    • 기기 ID 쿠키 처리 방식을 개선하고 보안 속성(HttpOnly, SameSite=Lax, Secure, Path, Max-Age)을 헤더 기반으로 강화했습니다.
    • 브라우저 프리플라이트(OPTIONS) 요청에 대해 필터가 적용되지 않도록 예외 처리했습니다.
  • 개선
    • 좋아요(rate limit) 제한 계산에서 만료 윈도우를 구성요소 설정값에 맞춰 동적으로 적용하도록 조정했습니다.

@coderabbitai
Copy link

coderabbitai bot commented Mar 17, 2026

Warning

Rate limit exceeded

@wlgns12370 has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 11 minutes and 49 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0471feed-039a-4bd3-a06c-2a8c84852901

📥 Commits

Reviewing files that changed from the base of the PR and between d718a53 and 5fedd94.

📒 Files selected for processing (3)
  • src/main/java/kr/co/knuserver/global/config/FilterConfig.java
  • src/main/java/kr/co/knuserver/global/filter/DeviceIdCookieFilter.java
  • src/main/resources/application.yml
📝 Walkthrough

Walkthrough

DeviceIdCookieFilter에서 쿠키 생성/추가 방식을 Cookie 객체 사용에서 Set-Cookie 응답 헤더 문자열 추가로 변경했고, OPTIONS preflight 요청에 대해 필터를 우회하도록 isLikesPath에 guard를 추가했습니다. LikeRateLimiter는 고정 윈도우 상수를 제거하고 TTL 기반 동적 윈도우를 사용하도록 변경했습니다.

Changes

Cohort / File(s) Summary
쿠키 처리 필터 변경
src/main/java/kr/co/knuserver/global/filter/DeviceIdCookieFilter.java
쿠키를 Cookie 객체로 생성해 addCookie 호출하던 방식에서 Set-Cookie 헤더 문자열(Name, Value, Path=/, Max-Age, HttpOnly, SameSite=Lax, Secure)을 직접 추가하도록 변경. isLikesPathOPTIONS 요청에 대해 false를 반환하는 guard 추가(프리플라이트 우회).
레이트리미터 TTL 적용
src/main/java/kr/co/knuserver/global/ratelimit/LikeRateLimiter.java
고정 WINDOW 상수를 제거하고 likeProperties.rateLimit().ttlSeconds() 기반으로 Duration window를 계산하여 Redis 키 만료에 사용하도록 변경(동일한 증감/검사 로직 유지).

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related issues

Poem

🐰 헤더로 싹 바꿨지, 쿠키는 반짝반짝,
OPTIONS는 살짝 건너뛰고 바람처럼,
보안 속성 모여 안전한 길만 닦아요,
바삭한 코드, 당근처럼 상쾌해! 🥕✨

🚥 Pre-merge checks | ✅ 3 | ❌ 2

❌ Failed checks (1 warning, 1 inconclusive)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Description check ❓ Inconclusive The PR description is minimal but includes the key information: reference to issue #122 and a brief statement of the implemented feature. However, it lacks details on the actual changes, implementation approach, and rationale. 확장된 설명을 추가하여 변경 사항의 구체적인 내용, 구현 방식, 테스트 방법 등을 포함하는 것이 권장됩니다.
✅ Passed checks (3 passed)
Check name Status Explanation
Title check ✅ Passed The PR title clearly describes the main change: excluding cookies from OPTIONS method requests, which aligns with the primary objective and code changes made to the filter.
Linked Issues check ✅ Passed The code changes successfully implement the objectives: OPTIONS 요청에서 쿠키 제외, 보안 속성 강화(HttpOnly, SameSite, Secure), CORS 프리플라이트 요청 최적화 등이 모두 구현되었습니다.
Out of Scope Changes check ✅ Passed All changes are within scope: DeviceIdCookieFilter는 쿠키 처리와 OPTIONS 요청 필터링, LikeRateLimiter는 동적 ttl 설정으로 관련 기능 개선이며, 모두 linked issue와 연관됩니다.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch feat/#121
📝 Coding Plan
  • Generate coding plan for human review comments

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link

@coderabbitai coderabbitai bot left a 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

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/main/java/kr/co/knuserver/global/filter/DeviceIdCookieFilter.java`:
- Around line 44-48: The Set-Cookie attributes are hardcoded causing policy
drift and cookie loss; update DeviceIdCookieFilter to accept a sameSitePolicy
(and optionally secure flag) via constructor (e.g., add a sameSitePolicy field
and constructor parameter alongside cookieMaxAge and deviceIdGenerator) and use
that field when building the cookie string instead of the literal "SameSite=Lax;
Secure"; ensure the value is populated from configuration
(application.yml/device-id.cookie.same-site or env var) so COOKIE_NAME, deviceId
and cookieMaxAge remain used but SameSite (and Secure behavior based on
environment) comes from the injected sameSitePolicy/config.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 0f2c48f5-d4e7-4b8f-b1ec-97d7ba615ff7

📥 Commits

Reviewing files that changed from the base of the PR and between 44c6d97 and 722f472.

📒 Files selected for processing (1)
  • src/main/java/kr/co/knuserver/global/filter/DeviceIdCookieFilter.java

@wlgns12370 wlgns12370 merged commit 62a6e2f into main Mar 17, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant