Skip to content

Refctor/299 event domain security#300

Merged
YEJIRYOO merged 4 commits into
mainfrom
refctor/299-event-domain-security
May 17, 2026
Merged

Refctor/299 event domain security#300
YEJIRYOO merged 4 commits into
mainfrom
refctor/299-event-domain-security

Conversation

@YEJIRYOO

@YEJIRYOO YEJIRYOO commented May 17, 2026

Copy link
Copy Markdown
Member

🔎 Resolved Issue

✅ Title

  • refactor: Event 도메인 PIN/QR 전송 방식 및 분산락 패턴 개선

📄 Content

  • 문제 상황
    • PIN 전송 보안 취약: @RequestParam storePin, @RequestParam qrToken 으로 받아 URL 쿼리스트링에 평문 노출 → ALB/Nginx access log, 브라우저 히스토리, Referer 헤더 등 다수 경로로 민감 정보 누출 위험. 전송 단계 보안 취약.
    • Redisson watchdog 비활성화로 인한 분산락 조기 만료 위험: tryLock(3, 3, TimeUnit.SECONDS) 호출 시 leaseTime 이 명시 → Redisson watchdog 메커니즘이 비활성화됨 → 3초 후 락이 자동 갱신되지 않고 만료. Redis 일시 지연, MySQL 느린 응답 등으로 처리 시간이 3초를 초과할 경우 동일 applicationId 에 대한 동시 요청이 통과하여 중복 발급 가능
  • EventVerifyQrRequest, EventCouponIssueRequest DTO 추가
    • @NotBlank 적용된 record 타입
    • verify-qr 요청: {"qrToken": "..."}
    • coupons 요청: {"qrToken": "...", "storePin": "..."}
  • EventController 변경
    • POST /api/v2/events/verify-qr@RequestParam qrToken@Valid @RequestBody EventVerifyQrRequest
    • POST /api/v2/events/coupons@RequestParam qrToken, storePin@Valid @RequestBody EventCouponIssueRequest
    • 빈 바디 / 누락된 필드는 Bean Validation 에서 400 응답으로 처리
    • 기존 ReportController 등의 @Valid @RequestBody 컨벤션 유지
  • CouponService.verifyPinAndIssueCoupon 변경
    • tryLock(3, 3, TimeUnit.SECONDS)tryLock(3, TimeUnit.SECONDS)
      • leaseTime 미지정으로 Redisson watchdog 활성화 → 락 점유 중 자동으로 TTL 갱신 (기본 30초 TTL을 10초마다 갱신)
      • 작업 진행 중 GC pause 나 외부 호출 지연이 발생해도 락 조기 만료 없이 안전하게 critical section 보호
      • 클라이언트 (서버) 장애 시에는 watchdog 갱신이 멈추므로 TTL 만료 후 자동 해제되어 deadlock 방어 유지

Summary by CodeRabbit

버그 수정 및 개선

  • 버그 수정

    • 쿠폰 대상자 정보의 불필요한 공백이 제거되어 데이터 처리의 일관성이 개선됨
  • 개선

    • 특정 API 엔드포인트의 요청 방식이 최적화되어 데이터 전송의 안정성이 향상됨
    • 동시성 처리 로직이 강화되어 시스템 안정성이 개선됨

Review Change Stack

@coderabbitai

coderabbitai Bot commented May 17, 2026

Copy link
Copy Markdown

Warning

Rate limit exceeded

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

You’ve run out of usage credits. Purchase more in the billing tab.

⌛ 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: 615d0107-d893-406f-9ab5-1696e1b1f9a6

📥 Commits

Reviewing files that changed from the base of the PR and between 95e909b and d776510.

📒 Files selected for processing (1)
  • src/main/java/com/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java

Walkthrough

Event 쿠폰 API의 엔드포인트 입력 방식을 쿼리 파라미터에서 요청 바디 DTO로 전환하고, 동시성 오류 처리를 GlobalException으로 통일하며, 대상자 이름의 공백을 정규화합니다.

Changes

Event 쿠폰 API 요청/응답 구조 및 동시성 개선

Layer / File(s) Summary
Event API 요청 DTO 계약
src/main/java/com/soongsil/CoffeeChat/domain/event/dto/EventVerifyQrRequest.java, src/main/java/com/soongsil/CoffeeChat/domain/event/dto/EventCouponIssueRequest.java
QR 토큰 검증(EventVerifyQrRequest)과 쿠폰 발급(EventCouponIssueRequest) 요청을 위한 새로운 Java record DTO가 추가되며, 모든 필드에 @NotBlank 검증 제약이 적용됩니다.
EventController 엔드포인트 요청 바디 전환
src/main/java/com/soongsil/CoffeeChat/domain/event/controller/EventController.java
verifyQrissueCoupon 엔드포인트가 @RequestParam 기반 입력에서 @Valid @RequestBody`` DTO 기반 입력으로 변경되며, DTO 임포트와 메서드 시그니처가 함께 업데이트됩니다.
CouponService 동시성 오류 처리
src/main/java/com/soongsil/CoffeeChat/domain/event/service/CouponService.java
쿠폰 발급 시 Redisson 분산 락의 tryLock 호출이 명시적 타임아웃 형태로 변경되고, InterruptedException 처리에서 IllegalStateException 대신 GlobalException(EVENT_CONCURRENCY_ERROR)를 던지도록 통일됩니다.
AssignedCouponService 대상자 이름 공백 정규화
src/main/java/com/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java
대상자 등록 시 이름을 trim()으로 전처리하여 Redis 해시에 저장되는 이름의 선행/후행 공백을 제거함으로써 이후 적격 검증 및 쿠폰 발급 로직에서 일관된 이름 비교를 보장합니다.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~12 minutes

Possibly related issues

  • Issue #299: 엔드포인트 요청 바디 전환, Redisson 락 처리, InterruptedException 오류 처리, 쿠폰 발급 개선 등 PR의 핵심 변경 사항들이 이슈의 요구 사항과 직접 일치합니다.

Possibly related PRs

  • Soongsil-CoffeeChat/COGO-DEV-server#296: 본 PR의 AssignedCouponService.registerTargets에서 대상자 nametrim() 후 Redis 해시에 저장하는 로직이 해당 PR의 새로운 대상자 등록 및 Redis 저장 흐름과 직접적으로 연관됩니다.

Suggested labels

♻️ refactor

Suggested reviewers

  • jinuklee777

Poem

🐰 요청을 본체에 담아서,
쿠폰을 안전하게 나눠주고,
공백을 다듬어 정결하게,
Event의 API는 더욱 단정해지네!
🎟️✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Title check ⚠️ Warning 제목이 'Refctor/299 event domain security'로 철자 오류가 있고('Refctor' → 'Refactor'), 전반적으로 모호하며 주요 변경사항(보안 개선, 요청 바디화, 동시성 패턴)을 명확하게 전달하지 못합니다. 제목을 'Fix typo and refactor: event domain security (request body, watchdog, trim)'과 같이 수정하여 철자 오류를 바로잡고 주요 변경사항을 명확하게 표현하세요.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ 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 refctor/299-event-domain-security

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

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/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java (1)

132-133: ⚠️ Potential issue | 🟠 Major | ⚡ Quick win

저장된 대상자 이름과 사용자 이름 비교 시 일관성 문제 - 수정 필요

대상자 등록 시 이름을 trim하여 저장하지만(74번 줄), 비교 시에는 user.getName()을 trim하지 않고 직접 비교합니다(133번 줄, 185번 줄). User 엔티티의 name 필드에 트림 로직이 없으므로, 사용자 이름에 앞뒤 공백이 있으면 논리적으로 동일한 이름이어도 불일치로 판정되어 정상 사용자가 쿠폰 발급을 거부당합니다.

133번 줄 수정:

-        if (!user.getName().equals(targetName)) {
+        if (!user.getName().trim().equals(targetName)) {

185번 줄 수정:

-        if (!user.getName().equals(target.get("name"))) {
+        if (!user.getName().trim().equals(target.get("name"))) {
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@src/main/java/com/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java`
around lines 132 - 133, AssignedCouponService currently compares stored
targetName (which was trimmed on save) to user.getName() without trimming the
user value; update both comparisons that use user.getName().equals(targetName)
(the occurrences where targetName is read from target.get("name")) to trim the
user side before comparing (e.g., compare user.getName().trim() to targetName or
normalize both sides), ensuring you modify both places in AssignedCouponService
where targetName vs user.getName() is checked so leading/trailing whitespace
does not cause false mismatches.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@src/main/java/com/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java`:
- Line 74: In AssignedCouponService, avoid calling target.name().trim() directly
because target.name() may be null; move the null-check before trimming or use a
null-safe expression such as String name = target.name() == null ? "" :
target.name().trim() (or Objects.toString(target.name(), "").trim()) so that the
subsequent logic that currently checks for null at line 79 runs against a safe,
trimmed string instead of causing an NPE.

---

Outside diff comments:
In
`@src/main/java/com/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java`:
- Around line 132-133: AssignedCouponService currently compares stored
targetName (which was trimmed on save) to user.getName() without trimming the
user value; update both comparisons that use user.getName().equals(targetName)
(the occurrences where targetName is read from target.get("name")) to trim the
user side before comparing (e.g., compare user.getName().trim() to targetName or
normalize both sides), ensuring you modify both places in AssignedCouponService
where targetName vs user.getName() is checked so leading/trailing whitespace
does not cause false mismatches.
🪄 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: Repository UI

Review profile: CHILL

Plan: Pro

Run ID: 06b21dbe-fc08-4860-8020-0db4a6f11031

📥 Commits

Reviewing files that changed from the base of the PR and between bab41a5 and 95e909b.

📒 Files selected for processing (5)
  • src/main/java/com/soongsil/CoffeeChat/domain/assignedcoupon/service/AssignedCouponService.java
  • src/main/java/com/soongsil/CoffeeChat/domain/event/controller/EventController.java
  • src/main/java/com/soongsil/CoffeeChat/domain/event/dto/EventCouponIssueRequest.java
  • src/main/java/com/soongsil/CoffeeChat/domain/event/dto/EventVerifyQrRequest.java
  • src/main/java/com/soongsil/CoffeeChat/domain/event/service/CouponService.java

@YEJIRYOO YEJIRYOO merged commit bf493bf into main May 17, 2026
2 checks 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