Skip to content

test: 핵심 도메인 서비스 단위 테스트 반영 + IDOR 3건 포함 버그 수정 (prod) - #220

Merged
unam98 merged 9 commits into
mainfrom
tests/service-coverage-and-bugfixes-prod
Aug 5, 2026
Merged

test: 핵심 도메인 서비스 단위 테스트 반영 + IDOR 3건 포함 버그 수정 (prod)#220
unam98 merged 9 commits into
mainfrom
tests/service-coverage-and-bugfixes-prod

Conversation

@unam98

@unam98 unam98 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

작업 배경

  • dev에서 검증 완료한 핵심 도메인 서비스(Course/Record/PublicCourse/User/Scrap/Health/JwtService/UserIdResolver) 단위 테스트와, 그 과정에서 발견한 버그 수정을 prod에 반영. dev 브랜치 전체를 머지하면 아직 미검증인 대규모 인프라 변경(Grafana/ELK 등)이 같이 나가므로, 관련 커밋만 별도로 cherry-pick해서 올림 (기존 feat/banner-api-prod와 동일한 방식).
  • dev PR: feat: 요청별 로그에 userId MDC 추가 #211~test: HealthService 단위 테스트 추가 #218

⚠️ 이번 PR에서 제외한 것

변경 사항

영역 내용
DepartureConverter, CourseService.updateCourse 출발지 주소 검증 NPE 수정, updateCourse IDOR 수정
RunnectUser id 기준 equals/hashCode 추가 (참조비교 버그 해결)
RecordService.updateRecord updateRecord IDOR 수정
PublicCourse, PublicCourseService equals/hashCode 추가, 삭제코스 체크 dead code 수정, updatePublicCourse IDOR 수정, 잘못된 정렬값 NPE 수정
UserService.updateUserNickname 닉네임 자기재저장 버그 수정
ScrapService.createAndDeleteScrap 스크랩 취소 NPE 수정
8개 서비스 테스트 파일 단위 테스트 148개 신규

영향 범위

  • 보안: updateCourse/updateRecord/updatePublicCourse 세 곳 모두, userId를 받으면서도 소유권 검증을 안 해서 다른 사람의 리소스를 수정할 수 있었던 IDOR 취약점이었음. 이번 반영으로 소유자 본인만 수정 가능하도록 막힘.
  • 나머지는 NPE/dead code 수정으로 전부 "에러가 나던 게 안 나는" 방향 — 기존 정상 동작에는 영향 없음.
  • 런타임 영향 없음 (스키마 변경 없음, API 요청/응답 포맷 변경 없음).

검증 매트릭스

영향 범위 테스트 코드
CourseService IDOR 수정 소유자가_아니면_수정_불가
RecordService IDOR 수정 소유자가_아니면_수정_불가
PublicCourseService IDOR 수정 (create/update 둘 다) 소유자가_아님
소유자가_아니면_수정_불가
RunnectUser equals/hashCode id가_같으면_인스턴스가_달라도_같다
UserService 닉네임 자기재저장 버그 본인_현재_닉네임으로_재저장
ScrapService 취소 NPE 버그 스크랩한_적_없는_코스_취소_요청은_무시된다
전체 서비스 정상/예외 케이스 dev PR #211~#218 참고 (동일 테스트 파일, 148개 전부 이 PR에도 포함됨)

Test Plan

  • 로컬에서 148개 전부 통과 확인
  • 로컬 postgres/redis 컨테이너 띄우고 ./gradlew build 전체(ServerApplicationTests 포함) 통과 확인
  • main에는 없는 docker-compose.yml 대신 docker run으로 임시 DB 띄워서 검증
  • 이 PR 자체의 prod-ci(방금 PR #219로 고친 버전)로 최종 재확인

🤖 Generated with Claude Code

alh0409 added 9 commits August 5, 2026 20:00
- JwtServiceTest: 토큰 발급/검증/만료/클레임 추출 검증
- UserIdResolverTest: 토큰 누락/만료/무효, 방문자 모드, userId 파싱 및 MDC 반영 검증
- dev-ci.yml: `-x test` 제거 — 지금까지 테스트가 아예 실행 안 되고 있었음
createCourse/getCourseByUser/getPrivateCourseByUser/getCourseDetail/updateCourse/
deleteCourses 전체 메서드에 대해 정상 케이스 + 예외 케이스 + 경계값을 검증.

테스트 작성 중 실제 프로덕션 코드에서 3가지 의심되는 부분을 발견해 별도로 표시해둠:
- createCourse: 출발지 주소가 3토큰 미만이면 DepartureConverter가 null을 반환하고
  이후 NPE로 이어짐 (요청값 검증 부재)
- updateCourse: courseId로만 조회하고 userId로 소유자 검증을 하지 않아,
  다른 사람의 코스 제목도 수정 가능 (IDOR 의심)
- getCourseDetail: RunnectUser가 equals/hashCode를 오버라이드하지 않아 isNowUser 판정이
  참조 동일성에 의존함 (같은 id라도 인스턴스가 다르면 다른 사람으로 판정될 수 있음)
1. DepartureConverter: 출발지 주소가 3토큰 미만이면 null 대신
   BadRequestException(VALIDATION_DEPARTURE_ADDRESS_EXCEPTION)을 던지도록 변경
   (기존엔 CourseService에서 바로 NPE로 이어짐)
2. CourseService.updateCourse: findById → findByCourseIdAndUserId로 변경해
   본인 소유 코스만 수정 가능하도록 수정 (IDOR 방지, deleteCourses와 동일 패턴)
3. RunnectUser: equals/hashCode를 id 기준으로 구현. 기존엔 참조 동일성에 의존해
   같은 유저라도 인스턴스가 다르면(Course.isMatchedUser, RecordService,
   PublicCourseService 등에서) 다른 사람으로 오판정될 수 있었음

CourseServiceTest의 관련 3개 테스트를 수정된 동작에 맞게 갱신하고,
RunnectUserTest를 새로 추가해 equals/hashCode 자체를 검증.
createRecord/getRecordByUser/updateRecord/deleteRecords 전체 메서드에 대해
정상 케이스 + 예외 케이스 + 경계값 검증 (18개).

테스트 작성 중 CourseService.updateCourse와 동일한 패턴의 버그 발견해 수정:
- updateRecord: userId 파라미터를 받지만 소유권 검증을 하지 않아 다른 사람의
  기록 제목도 수정 가능했음 (IDOR). deleteRecords는 이미 소유권을 검증하고
  있어서(PermissionDeniedException), 동일 패턴으로 맞춰서 수정.
  ErrorStatus.PERMISSION_DENIED_RECORD_UPDATE_EXCEPTION 추가.

getRecordByUser의 건강 데이터 조회 실패 시 전체 요청은 실패하지 않고
healthData만 null로 우아하게 처리되는 방어 로직도 별도로 검증함.
getPublicCourseTotalPageCount/getMarathonPublicCourse/searchPublicCourse/
recommendPublicCourse/getPublicCourseByUser/getPublicCourseDetail/
createPublicCourse/deletePublicCourses/updatePublicCourse 전체 메서드에
대해 정상 케이스 + 예외 케이스 + 경계값 검증 (38개).

테스트 작성 중 발견해서 함께 수정한 버그 4건:
1. PublicCourse에 equals/hashCode 부재 — RunnectUser와 동일한 참조비교 문제.
   scrap 목록과 publicCourse 목록을 서로 다른 쿼리로 가져와 비교하는 곳이
   5곳(getMarathonPublicCourse, searchPublicCourse, recommendPublicCourse,
   getPublicCourseByUser, getPublicCourseDetail)이라 isScrap이 잘못 표시될
   수 있었음. id 기준 equals/hashCode 추가로 일괄 해결.
2. getPublicCourseDetail: 삭제된 코스 체크 조건이 반대(`== null`)였고,
   심지어 예외를 생성만 하고 throw를 안 해서 완전히 죽은 코드였음. 조건
   반전 + throw 추가.
3. updatePublicCourse: userId를 받으면서 소유권 검증을 안 해 다른 사람의
   공개 코스 제목/설명도 수정 가능했음 (IDOR). deletePublicCourses와
   동일한 관리자 예외 패턴으로 소유권 검증 추가.
   ErrorStatus.PERMISSION_DENIED_PUBLIC_COURSE_UPDATE_EXCEPTION 추가.
4. recommendPublicCourse: sort 파라미터가 "scrap"/"date" 둘 다 아니면
   Page 변수가 null로 남아 NPE. 이미 정의돼 있던
   INVALID_SORT_PARAMETER_EXCEPTION을 실제로 사용하도록 수정.
getMyPage/updateUserNickname/getUserProfile/deleteUser 전체 메서드에
대해 정상 케이스 + 예외 케이스 + 경계값 검증 (17개).

테스트 작성 중 발견해서 수정한 버그:
- updateUserNickname: 중복 닉네임 체크를 유저 조회보다 먼저, 그리고
  본인의 현재 닉네임과 비교 없이 수행하고 있어서, 본인의 기존 닉네임을
  그대로 다시 저장하려고 해도 "이미 존재하는 닉네임"으로 거부됐음.
  유저 조회를 먼저 하고, 요청 닉네임이 현재 닉네임과 다를 때만 중복
  체크를 하도록 순서/조건 수정.
createAndDeleteScrap/getScrapCourseByUser 전체 메서드에 대해
정상 케이스 + 예외 케이스 + 경계값 검증 (9개).

테스트 작성 중 발견해서 수정한 버그:
- createAndDeleteScrap: 스크랩한 적 없는 코스를 "취소"(scrapTF=false)
  요청하면 scrap 변수가 null이라 scrap.updateScrapTF(false) 호출 시
  바로 NPE(500)가 났음. 클라이언트가 중복 취소 요청을 보내거나
  race condition만 있어도 쉽게 재현 가능한 케이스라 조건 분기 추가로
  null이면 조용히 무시하도록 수정 (idempotent하게).
createHealthData/getHealthData/getHealthSummary/deleteHealthData 전체
메서드에 대해 정상 케이스 + 예외 케이스 + 경계값 검증 (22개).

버그는 발견되지 않음 — 소유권 검증(record.getRunnectUser().getId().equals(userId))을
일관되게 사용하고 있고, 동시성 경쟁으로 인한 유니크 제약 위반도
DataIntegrityViolationException을 잡아 409로 변환하는 등 이번에 테스트한
서비스 중 가장 방어적으로 잘 짜여있었음.
MDC userId 로깅 기능(dev의 f1a7044, 2914ae5)은 이 브랜치가 아직 갖추지
못한 로깅 기반 인프라(MdcLoggingFilter, logback-spring.xml 등, dev 전용
모니터링 인프라 구축 작업에 딸려있음)에 의존하고 있어 이번엔 함께
가져오지 않았다. UserIdResolver 자체의 토큰 검증/파싱 로직은 동일하게
유효하므로 테스트는 유지하되, MDC 관련 assertion만 제거.
@unam98 unam98 self-assigned this Aug 5, 2026
@unam98
unam98 merged commit f78f3e9 into main Aug 5, 2026
2 checks passed
@unam98
unam98 deleted the tests/service-coverage-and-bugfixes-prod branch August 5, 2026 11:05
@coderabbitai

coderabbitai Bot commented Aug 5, 2026

Copy link
Copy Markdown

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 758d838c-1d85-43f6-a937-8ffb7fdedfdb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review

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.

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.

2 participants