[feature] IP 기반 Rate Limiting 및 임시 차단 기능 추가#1491
Conversation
- 10초 윈도우 내 20회 초과 요청 시 30초 임시 차단
- game:ratelimit:{ip} 키로 요청 카운트, game:banned:{ip} 키로 차단 관리
- CLICK_RATE_LIMITED 에러코드 추가 (600-14)
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
|
Warning
|
| Cohort / File(s) | Summary |
|---|---|
클릭 서비스 변경 backend/src/main/java/moadong/club/service/ClubClickService.java |
recordClick(...)에 클라이언트 IP별 Redis 키 검사(차단 여부), Lua 기반 원자적 카운터 증가 및 TTL 설정, 요청 한도 초과 시 임시 차단 키 설정 및 CLICK_RATE_LIMITED 예외 발생 로직이 추가됨. 쿨다운 및 화이트리스트 검사 순서 일부 조정. |
오류 코드 추가 backend/src/main/java/moadong/global/exception/ErrorCode.java |
클릭 속도 제한/차단 상황을 표현하는 CLICK_RATE_LIMITED(HTTP 429) enum 상수 추가. |
Sequence Diagram(s)
sequenceDiagram
participant Client as Client
participant Service as ClubClickService
participant Redis as Redis
Client->>Service: click request (clientIp, clubId)
Service->>Redis: GET banned:{clubId}:{clientIp}
alt banned exists
Redis-->>Service: key exists
Service-->>Client: ErrorCode.CLICK_RATE_LIMITED (429)
else not banned
Service->>Redis: EVAL(increment-with-ttl.lua, counter:{clubId}:{clientIp})
Redis-->>Service: count
alt count > RATE_LIMIT_MAX_REQUESTS
Service->>Redis: SET banned:{clubId}:{clientIp} (BAN_DURATION)
Service-->>Client: ErrorCode.CLICK_RATE_LIMITED (429)
else under limit
Service->>Redis: SETNX cooldown:{clubId}:{clientIp} (if applicable)
Service-->>Client: success / continue processing
end
end
Estimated code review effort
🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
- [feature] 동아리 클릭 게임 API 추가 #1470: 동일한
ClubClickService.recordClick()관련 구현에서 클라이언트별 Redis 속도 제한 및 차단 로직을 다룬 PR.
Suggested reviewers
- Zepelown
- suhyun113
- SeongHoonC
개요
클럽 클릭 기록 서비스에 클라이언트별 Redis 기반 속도 제한 및 임시 차단 기능을 추가하였으며, 이에 따른 오류 코드를 정의하였습니다.
변경 사항
| Cohort / File(s) | Summary |
|---|---|
속도 제한 및 차단 로직 backend/src/main/java/moadong/club/service/ClubClickService.java |
recordClick() 메서드에 클라이언트 IP별 Redis 차단 플래그 확인, 고정 시간 창 내 요청 카운터 증가, 한도 초과 시 임시 차단 기능을 추가하였습니다. 기존 화이트리스트 및 쿨다운 로직은 유지되나 검사 순서가 일부 조정되었습니다. |
오류 코드 정의 backend/src/main/java/moadong/global/exception/ErrorCode.java |
HTTP 429 상태 코드와 함께 CLICK_RATE_LIMITED 오류 코드를 추가하여 속도 제한 시나리오를 표현합니다. |
코드 리뷰 소요 시간
🎯 3 (Moderate) | ⏱️ ~20 minutes
관련 가능성 있는 PR
- [feature] 동아리 클릭 게임 API 추가 #1470: 동일한
ClubClickService.recordClick()구현을 확장하여 클라이언트별 Redis 속도 제한 및 차단 확인을 추가한 PR입니다.
제안된 검토자
- Zepelown
- suhyun113
- SeongHoonC
🚥 Pre-merge checks | ✅ 2 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | 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. |
✅ Passed checks (2 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | PR 제목은 IP 기반 Rate Limiting 및 임시 차단 기능 추가라는 주요 변경사항을 명확하고 간결하게 요약하고 있으며, 변경사항 전체를 잘 대표합니다. |
| Description Check | ✅ Passed | Check skipped - CodeRabbit’s high-level summary is enabled. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/#1474-click-game-rate-limit-MOA-819
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 help to get the list of available commands and usage tips.
Test Results123 tests 123 ✅ 21s ⏱️ Results for commit 1b330e8. ♻️ This comment has been updated with latest results. |
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 (1)
backend/src/main/java/moadong/club/service/ClubClickService.java (1)
69-92: 🛠️ Refactor suggestion | 🟠 Major검사 순서 재배치 권장 — ban 검사는 화이트리스트 검사보다 앞에 두는 것이 PR 의도와 일치합니다.
현재 흐름은
count 검증 → 화이트리스트 검증 → ban 검사 → rate limit인데, PR 설명의 동작 흐름은 “1. 차단된 IP인지 먼저 확인 → 2. 카운트 증가 → 3. 초과 시 ban”입니다. 현 구현은 두 가지 부작용이 있습니다.
- 이미 차단된 IP가 존재하지 않는
clubName으로 폭주하면 ban 검사에 도달하지 못해 매 요청마다WHITELIST_KEYSISMEMBER조회 비용이 발생하고, rate limit 카운터도 올라가지 않아 추가적인 억제 효과가 없습니다.- 의도적인 스팸 공격자가 잘못된 clubName으로 우회하여 차단 메커니즘을 무력화할 수 있습니다.
count 범위 검증 → ban 검사 → rate limit → 화이트리스트 검사 → cooldown → 영속화순서로 바꾸면 값싼 RedisEXISTS호출로 악성 IP를 가장 먼저 걸러낼 수 있습니다.위 코딩 가이드라인 "예외 처리와 검증은 기존 프로젝트 방식과 일관되게 맞춘다" 관점에서도, PR 설명이 명시한 동작 흐름과 코드 구현이 일치하도록 맞추는 것을 권장합니다.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/moadong/club/service/ClubClickService.java` around lines 69 - 92, In recordClick, keep the count validation first, then move the ban check (use BAN_KEY_PREFIX + clientIp and stringRedisTemplate.hasKey) immediately after it, then perform the rate-limit increment/expire/ban logic (RATE_LIMIT_KEY_PREFIX + clientIp, increment, expire when count==1, set ban when > RATE_LIMIT_MAX_REQUESTS) before doing the whitelist lookup (WHITELIST_KEY and opsForSet().isMember); this ensures banned IPs are filtered cheaply and the rate counter increments even for invalid clubName while preserving the existing exceptions (ErrorCode.CLICK_COUNT_INVALID, CLICK_RATE_LIMITED, CLUB_NOT_FOUND) and using the same keys/constants (BAN_KEY_PREFIX, RATE_LIMIT_KEY_PREFIX, WHITELIST_KEY, RATE_LIMIT_WINDOW_SECONDS, BAN_DURATION_SECONDS).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@backend/src/main/java/moadong/club/service/ClubClickService.java`:
- Around line 84-92: The current increment + expire sequence using
stringRedisTemplate.opsForValue().increment(rateLimitKey) then
expire(rateLimitKey, ...) is non-atomic and can leave the rateLimitKey without
TTL; replace it with an atomic approach: either run a Redis Lua script that
performs INCR and, if the returned value is 1, sets the TTL in one EVAL call (so
the INCR+conditional EXPIRE is atomic), or pre-create the key with TTL and then
increment by calling stringRedisTemplate.opsForValue().setIfAbsent(rateLimitKey,
"0", Duration.ofSeconds(RATE_LIMIT_WINDOW_SECONDS)) before calling increment to
ensure TTL is present; update the logic around RATE_LIMIT_KEY_PREFIX,
requestCount checks and banKey handling accordingly so the counter cannot
persist without TTL.
- Around line 79-92: The rate-limiting in ClubClickService relies on an
untrusted clientIp produced by ClubClickController.resolveClientIp which
currently takes the first token of X-Forwarded-For; fix by restoring client IP
using a trusted-proxy strategy instead of blindly trusting X-Forwarded-For:
enable and configure Spring's ForwardedHeaderFilter (and set
server.forward-headers-strategy=native) or implement a trusted proxy CIDR
whitelist and validate the X-Forwarded-For chain in resolveClientIp to pick the
last untrusted hop, rejecting/spoofed headers when the immediate peer is not a
trusted proxy; update ClubClickController.resolveClientIp and document config so
ClubClickService uses a reliable clientIp for
RATE_LIMIT_KEY_PREFIX/BAN_KEY_PREFIX logic.
---
Outside diff comments:
In `@backend/src/main/java/moadong/club/service/ClubClickService.java`:
- Around line 69-92: In recordClick, keep the count validation first, then move
the ban check (use BAN_KEY_PREFIX + clientIp and stringRedisTemplate.hasKey)
immediately after it, then perform the rate-limit increment/expire/ban logic
(RATE_LIMIT_KEY_PREFIX + clientIp, increment, expire when count==1, set ban when
> RATE_LIMIT_MAX_REQUESTS) before doing the whitelist lookup (WHITELIST_KEY and
opsForSet().isMember); this ensures banned IPs are filtered cheaply and the rate
counter increments even for invalid clubName while preserving the existing
exceptions (ErrorCode.CLICK_COUNT_INVALID, CLICK_RATE_LIMITED, CLUB_NOT_FOUND)
and using the same keys/constants (BAN_KEY_PREFIX, RATE_LIMIT_KEY_PREFIX,
WHITELIST_KEY, RATE_LIMIT_WINDOW_SECONDS, BAN_DURATION_SECONDS).
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d566ac58-1a76-4785-8e80-79d35e63fa8f
📒 Files selected for processing (2)
backend/src/main/java/moadong/club/service/ClubClickService.javabackend/src/main/java/moadong/global/exception/ErrorCode.java
lepitaaar
left a comment
There was a problem hiding this comment.
요구사항 범위(10초 윈도우 20회 초과 시 30초 차단, CLICK_RATE_LIMITED 추가)는 기능적으로 충족되어 APPROVE합니다.
거절 사유(비차단 아님)
- [Major] 검증 순서 이슈: 현재 화이트리스트 미통과 요청은 rate-limit 카운팅 전 조기 예외로 빠져 우회 트래픽이 카운팅되지 않습니다. PR 설명의 동작흐름과도 불일치합니다.
- [Major] INCR + EXPIRE 비원자성: INCR 성공 후 EXPIRE 실패 시 rate-limit key TTL 유실 가능성이 있어 윈도우 보장이 깨질 수 있습니다.
위 2건은 엄격 기준에서는 Request Changes 급이지만, 이번 정책에 따라 APPROVE로 제출하며 인라인에 근거와 수정 방향을 남겼습니다.
잘못된 clubName으로 반복 요청 시 IP 카운팅이 누락되는 문제 수정. 모든 요청에 대해 ban/rate-limit이 먼저 적용되도록 순서 재정렬.
increment() 후 별도 expire() 호출 시 TTL 누락 가능성을 Lua 스크립트 단일 연산으로 제거
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/src/main/java/moadong/club/service/ClubClickService.java (1)
78-96:⚠️ Potential issue | 🟠 Major
count검증 전에 ban/rate-limit을 먼저 적용해주세요.Line 79에서
count검증이 먼저 실행되어count=0또는count=6같은 요청은game:ratelimit:{ip}카운터를 증가시키지 않습니다. 잘못된 payload 반복 호출로 IP ban을 우회할 수 있으니, ban/rate-limit 블록을 메서드 진입 직후로 올리고count검증은 그 뒤로 이동하는 편이 PR 목표와 일치합니다.🐛 제안 수정
public ClubClickResponse recordClick(String clubName, int count, String clientIp) { - if (count < 1 || count > 5) { - throw new RestApiException(ErrorCode.CLICK_COUNT_INVALID); - } - String banKey = BAN_KEY_PREFIX + clientIp; if (Boolean.TRUE.equals(stringRedisTemplate.hasKey(banKey))) { throw new RestApiException(ErrorCode.CLICK_RATE_LIMITED); @@ if (requestCount != null && requestCount > RATE_LIMIT_MAX_REQUESTS) { stringRedisTemplate.opsForValue().set(banKey, "1", Duration.ofSeconds(BAN_DURATION_SECONDS)); throw new RestApiException(ErrorCode.CLICK_RATE_LIMITED); } + + if (count < 1 || count > 5) { + throw new RestApiException(ErrorCode.CLICK_COUNT_INVALID); + } String cooldownKey = COOLDOWN_KEY_PREFIX + clientIp;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@backend/src/main/java/moadong/club/service/ClubClickService.java` around lines 78 - 96, Move the ban/rate-limit checks to the start of recordClick so malformed payloads cannot bypass throttling: perform the BAN_KEY_PREFIX + clientIp hasKey check via stringRedisTemplate and then execute RATE_LIMIT_SCRIPT with RATE_LIMIT_KEY_PREFIX + clientIp, RATE_LIMIT_WINDOW_SECONDS and compare against RATE_LIMIT_MAX_REQUESTS (setting banKey with BAN_DURATION_SECONDS and throwing ErrorCode.CLICK_RATE_LIMITED as currently done) before validating the count; after those Redis checks, validate count and throw ErrorCode.CLICK_COUNT_INVALID if out of range so the rate-limit counters always increment even for invalid count values.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Outside diff comments:
In `@backend/src/main/java/moadong/club/service/ClubClickService.java`:
- Around line 78-96: Move the ban/rate-limit checks to the start of recordClick
so malformed payloads cannot bypass throttling: perform the BAN_KEY_PREFIX +
clientIp hasKey check via stringRedisTemplate and then execute RATE_LIMIT_SCRIPT
with RATE_LIMIT_KEY_PREFIX + clientIp, RATE_LIMIT_WINDOW_SECONDS and compare
against RATE_LIMIT_MAX_REQUESTS (setting banKey with BAN_DURATION_SECONDS and
throwing ErrorCode.CLICK_RATE_LIMITED as currently done) before validating the
count; after those Redis checks, validate count and throw
ErrorCode.CLICK_COUNT_INVALID if out of range so the rate-limit counters always
increment even for invalid count values.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 55f9e3fe-793b-45c3-b614-6fc88468298f
📒 Files selected for processing (1)
backend/src/main/java/moadong/club/service/ClubClickService.java
#️⃣연관된 이슈
📝작업 내용
동작흐름
30초로 설정한 것은 사용자 다수가 공인 ip (학교 와이파이)를 사용할 가능성이 높다고 생각했습니다.
5분도 생각해봤는데 사용자경험이 극히 떨어질 것 같아서 일단 30초로 설정해 두었습니다.
중점적으로 리뷰받고 싶은 부분(선택)
논의하고 싶은 부분(선택)
🫡 참고사항
Summary by CodeRabbit