Skip to content

[#281]; feat: 메일예약조회관련 권한 변경#282

Merged
seulnan merged 1 commit into
developfrom
feat/281
Mar 10, 2026
Merged

[#281]; feat: 메일예약조회관련 권한 변경#282
seulnan merged 1 commit into
developfrom
feat/281

Conversation

@seulnan

@seulnan seulnan commented Mar 10, 2026

Copy link
Copy Markdown
Collaborator

개요

  • 메일 예약 조회 API에서 같은 팀원특권 유저(화이트리스트+HR) 의 조회 범위를 확장했습니다.
  • 관리자/운영 관점에서 예약 메일 모니터링을 더 쉽게 하기 위한 변경입니다.

주요 변경 사항

  • 권한/팀 로직 강화

    • MemberPrivacyServicegetActiveTeamMemberEmails(userId) 추가
      • 로그인 사용자의 파트 ID를 기준으로, 같은 파트에 속한 Active 멤버들의 이메일 목록을 계산
      • 본인 이메일도 항상 포함되도록 처리
  • 메일 예약 조회 범위 확장

    • MailService.getPendingReservationStatuses
      • 특권 유저(isPrivilegedUser == true) → 전체 예약(readAllBefore)
      • 일반 유저 → getActiveTeamMemberEmails 기반 senderEmail IN (...) 조회
    • MailService.getUserMailReservations
      • 특권 유저 → 전체 예약(readAll)
      • 일반 유저 → 팀 이메일 목록 기반 readAllBySenderEmails
    • MailService.getUserMailReservation
      • 특권 유저이거나, 예약 메일의 senderEmailAddress가 팀 이메일 목록에 포함되면 접근 허용
      • 그 외에는 기존처럼 MailReservationAccessDeniedException 발생
  • 저장소 계층 확장

    • JpaMailReservationRepository / MailReservationRepository / MailReservationReader
      • senderEmails IN (...) 기반 조회 메서드 추가
      • 시간 조건(<= time, BETWEEN from AND to)이 있는 버전까지 지원
  • 테스트

    • MailServiceTest에서 다음 케이스 추가/보완
      • 일반 유저: getActiveTeamMemberEmails 기반으로 자기 메일이 조회되는지 검증
      • 같은 팀 유저: 다른 팀원의 예약 단건을 조회할 수 있는지 검증
      • 특권 유저: 예약 목록 조회 시 전체 예약이 반환되는지 검증
    • 전체 테스트(./gradlew test) 통과 확인

변경 의도

  • 운영자가 팀 단위로 예약 메일을 쉽게 확인할 수 있도록 하고,
  • HR/화이트리스트 계정은 조직 전체 예약 현황을 빠르게 파악할 수 있게 하기 위함입니다.

📎 Issue 번호

closed #281

@seulnan seulnan self-assigned this Mar 10, 2026
@seulnan

seulnan commented Mar 10, 2026

Copy link
Copy Markdown
Collaborator Author

@claude 코드변경사항이 pr설명과 맞는지 추적하고, 크리티컬한 이슈가 있는지 말해줘

@github-actions

github-actions Bot commented Mar 10, 2026

Copy link
Copy Markdown

Claude finished @seulnan's task in 3m 11s —— View job


코드 리뷰: PR #282 메일예약조회 권한 변경

  • 작업 목록 생성
  • 변경된 파일 읽기
  • PR 설명과 코드 변경사항 비교
  • 크리티컬 이슈 분석
  • 리뷰 결과 정리 및 게시

✅ PR 설명 vs 코드 변경 추적

PR 설명 항목 코드 반영 여부 위치
MemberPrivacyService.getActiveTeamMemberEmails 추가 ✅ 일치 MemberPrivacyService.kt:42-60
getPendingReservationStatuses 권한 분기 ✅ 일치 MailService.kt:67-102
getUserMailReservations 권한 분기 ✅ 일치 MailService.kt:204-220
getUserMailReservation 팀원/특권 접근 허용 ✅ 일치 MailService.kt:234-239
저장소 계층 senderEmails IN (...) 메서드 추가 ✅ 일치 JpaMailReservationRepository, MailReservationRepositoryImpl, MailReservationReader
테스트 케이스 추가 (일반 유저, 팀원, 특권 유저) ✅ 일치 MailServiceTest.kt:595-672

⚠️ PR 설명에 언급되지 않은 변경사항

  1. FormResponseToApplicantProcessor.ktavailableTimeParser.parse() 호출 방식 변경 (기존 getAll() → named parameter 방식). PR 설명에 없음.
  2. MailReservationDetailResponse.kt, MailReservationListResponse.kt — 응답에 senderEmailAddress 필드 추가. PR 설명에 명시 없음. (팀원 예약 조회 기능과 연계되어 합리적인 변경이나 설명 누락)
  3. scripts/sql/mail-reservation-queries.sql — 범용 쿼리 5개를 특정 이메일 하드코딩 쿼리 2개로 전면 교체. PR 설명에 전혀 언급 없음.

🔴 크리티컬 이슈

1. getPendingReservationStatuses에서 특권 유저의 OAuth 상태 체크 로직 버그

// MailService.kt:67-101
fun getPendingReservationStatuses(userId: Long): List<PendingMailReservationStatus> {
    val user = userReader.readById(userId)  // 특권 유저 (본인)
    ...
    val reservations = mailReservationReader.readAllBefore(now)  // 전체 팀원 예약 포함
    return reservations.map { reservation ->
        ...
        val (failureErrorCode, failedAt) = try {
            // ⚠️ 항상 현재 로그인한 특권 유저(user.id!!)의 토큰만 체크
            oauth2Service.refreshOAuth2TokenBeforeExpiry(user.id!!, OAuth2Type.GOOGLE, 10L)

특권 유저가 타 팀원의 예약 목록을 조회하면, 각 예약의 failureErrorCode예약 발신자의 OAuth 상태가 아닌 특권 유저 본인의 토큰 상태를 기준으로 반환됩니다. 즉, 팀원 A의 토큰이 만료되어도 특권 유저의 토큰이 유효하면 failureErrorCode = null로 반환되어 실제 장애 상황을 숨기게 됩니다.

Fix this →

2. getActiveTeamMemberEmails의 전체 멤버 풀스캔 성능 이슈

// MemberPrivacyService.kt:50-57
val activeMembers = memberReader.readAllActive()  // 전체 활성 멤버 메모리 로드
val teamEmails =
    activeMembers
        .asSequence()
        .filter { member -> member.parts.any { part -> part.id != null && myPartIds.contains(part.id) } }

일반 유저 요청마다 전체 활성 멤버를 메모리에 올려 Kotlin 레벨에서 필터링합니다. 멤버 수가 증가할수록 선형적으로 성능이 저하됩니다. memberReaderreadAllActiveByPartIds(partIds: Set<Long>) 같은 파트 필터링 쿼리를 추가하는 것이 바람직합니다.

Fix this →

3. getUserMailReservation에서 중복 DB 조회

// MailService.kt:234-236
val canAccess =
    memberPrivacyService.isPrivilegedUser(userId) ||          // userReader.readById + memberReader.readByEmailOrNull
        memberPrivacyService.getActiveTeamMemberEmails(userId) // userReader.readById + getMemberPartIds + readAllActive

비특권 유저의 경우 isPrivilegedUsergetActiveTeamMemberEmails 모두 내부적으로 userReader.readById(userId)를 호출하여 동일한 유저 정보를 최소 2회 이상 DB 조회합니다. isPrivilegedUsergetMemberPartIds를 간접 호출하여 memberReader.readByEmailOrNull이 2회 이상 호출될 수 있습니다.


🟡 일반 이슈

4. SQL 파일에 하드코딩된 실제 이메일 주소

-- scripts/sql/mail-reservation-queries.sql:33
WHERE m.sender_email_address = 'glen.urssu@gmail.com'

glen.urssu@gmail.com이 실제 계정 이메일로 추정되며, 소스 코드에 하드코딩된 채 공개 레포지토리에 커밋되어 있습니다. 실제 사용 예제라면 'your_email@example.com'과 같은 플레이스홀더로 대체를 권장합니다.

또한 기존 SQL 파일에는 범용 쿼리(전체 조회, 건수 확인, 미발송/스케줄러 누락 케이스 등) 5개가 있었는데, 이번 PR에서 특정 이메일 하드코딩 쿼리 2개로 완전히 교체되었습니다. 기존 범용 쿼리들이 삭제된 의도가 PR 설명에 없어 의도적인 변경인지 확인이 필요합니다.

5. updateMailReservation/cancelMailReservation 권한 정책 불일치 (의도적이면 OK)

getUserMailReservation은 팀원도 조회 가능하게 확장되었지만, updateMailReservationcancelMailReservation은 여전히 소유자만 가능합니다. 의도적인 설계라면 PR 설명이나 주석에 명시하는 것을 권장합니다.

6. retryReservation의 권한 로직은 변경되지 않음

retryReservation은 소유자 이메일만 체크하는 기존 방식 유지 (MailService.kt:136-138). 팀원 재전송 여부의 정책적 판단이 필요합니다.


📋 테스트 케이스 검토

테스트 케이스 PR 설명 기재 여부 구현 여부
일반 유저: 자기 메일 예약 조회 ✅ (line:86)
같은 팀 유저: 타 팀원 단건 조회 ✅ (line:596)
특권 유저: 전체 예약 목록 반환 ✅ (line:635)
특권 유저의 단건 조회 테스트 ❌ 미기재 ❌ 미구현
getPendingReservationStatuses 권한 분기 테스트 ❌ 미기재 ❌ 미구현

요약

PR 설명에 기재된 핵심 기능 변경사항은 코드와 전반적으로 일치합니다. 다만:

  • 크리티컬: getPendingReservationStatuses에서 특권 유저 조회 시 OAuth 상태 체크 대상이 잘못됨 (실제 버그)
  • 성능: getActiveTeamMemberEmails의 전체 멤버 풀스캔
  • 보안: SQL 파일에 실제 이메일 하드코딩
  • 누락: FormResponseToApplicantProcessor, SQL 파일, 응답 DTO 변경이 PR 설명에 미언급

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.

feat: 메일예약조회관련 권한 변경

1 participant