Files
chocoadmin/docs/plan/improvement-plan.md
T

201 lines
14 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# chocoadmin 전체 검토 및 개선 계획
작성일: 2026-07-04
검토 범위: Phase 0–12 전체 구현 (인증, 마에스트로 목록/상세/수정, 연장/업그레이드 신청·승인, 이메일 발송, 로깅)
---
## 1. 요약
코드·로직·비즈니스 측면을 전수 검토한 결과, 데이터 파괴적인 치명 결함은 없으나
**(1) 로그를 통한 마에스트로 비밀번호 노출 가능성, (2) 동시 승인 이중 처리 가능성,
(3) 타임존 미확정으로 인한 만료일 오차** 세 가지가 우선 수정 대상이다.
그 외 체험(TRIAL) 계정 관련 비즈니스 규칙 공백과 트랜잭션/로그 기록 비일관성이 있다.
| 등급 | 건수 | 내용 |
|---|---|---|
| P0 (즉시 수정) | 3 | 비밀번호 로그 노출, 동시 승인 레이스, 타임존 정책 |
| P1 (다음 작업) | 7 | TRIAL 규칙 공백, 승인 시점 재검증, 트랜잭션/로그 일관성, 이메일 실패 처리 |
| P2 (보완) | 8 | 매직 넘버, 에러 응답 일관성, 세션 정책, 목록 기본 필터 등 |
---
## 2. P0 — 즉시 수정
### P0-1. 연장 승인 시 maestro 전체 컬럼 조회 → Password 로그 노출 위험
- 위치: `lib/extension-requests.ts:235` (`getActionTarget``include: { maestro: true }`)
- 문제: `maestro.Password`(char 50)까지 조회된다. `lib/db.ts`의 debug 레벨 결과 테이블 로깅이
조회 행 전체를 콘솔에 출력하므로, local/stage(debug 기본)에서 승인 처리 시
**마에스트로 비밀번호가 로그에 그대로 남는다.** phase9 마스킹 정책 위반.
- 대조: `lib/upgrade-requests.ts:253`은 이미 `select: { Name, Email }`로 제한하고 있어 비일관.
- 개선:
1. extension 쪽 `getActionTarget`을 upgrade와 동일하게 `select` 방식으로 변경
(필요 컬럼: 요청 전체 + `maestro.Name`, `Email`, `AvailableActivateDateTime`).
2. 방어선 추가: `lib/db.ts` 결과 로깅에서 `Password` 컬럼명을 만나면 `***` 마스킹.
### P0-2. 동시 승인 레이스 컨디션 (이중 승인 → +2년 연장)
- 위치: `lib/extension-requests.ts:116-157`, `lib/upgrade-requests.ts:123-168`
- 문제: 트랜잭션 안에서 `findUnique``Status = 1` 확인 후 `update`하는 read-then-write 패턴.
MariaDB 기본 격리수준(REPEATABLE READ)에서는 두 관리자가 동시에 승인 버튼을 누르면
둘 다 `Status = 1`을 읽고 둘 다 진행 → 연장이 두 번 적용(만료일 +2년), 로그·메일 2회 발생 가능.
- 개선(택1):
- A안(권장): 상태 전이를 조건부 UPDATE로 원자화.
`updateMany({ where: { MaestroExtensionID, Status: 1 }, data: { Status: 3 } })`
`count === 0`이면 409 throw. maestro 갱신은 그 뒤에 수행.
- B안: `$queryRaw``SELECT ... FOR UPDATE`로 요청 행 잠금 후 진행.
- 적용 대상: 연장 승인/취소, 업그레이드 승인/거절 4개 함수 모두.
### P0-3. 타임존 정책 미확정 — 만료일·이메일 날짜 오차
- 위치:
- `lib/utils.ts:93-105` `calculateExtendedAvailableDate``setHours(0,0,0,0)` — 서버 로컬 자정
- `lib/utils.ts:63-75` `formatDate` — 서버 로컬 기준
- `lib/upgrade-requests.ts:360-366` `calculateUpgradeAvailableDate`
- `docker-compose.yml` / `docker-compose.stage.yml``TZ` 환경변수 없음 → 컨테이너는 UTC
- 문제: 기존 chocomae(PHP)는 DB 서버 `NOW()`(KST 추정) 기준으로 날짜를 기록해 왔다.
chocoadmin 컨테이너가 UTC로 돌면:
- 연장 승인 시 "자정"이 UTC 자정(KST 09:00)으로 저장돼 기존 데이터와 9시간 어긋남.
- `formatDate`로 만든 이메일 표기 날짜가 KST 기준과 하루 차이 날 수 있음 (KST 00:0008:59 구간).
- `updateMaestro`의 날짜 변경 로그 remark는 `toISOString().slice(0,10)`(UTC)인데
다른 곳은 로컬 기준 — 같은 값이 화면/로그/메일에서 다르게 보일 수 있음 (`lib/maestros.ts:216-220`).
- 개선:
1. docker-compose 두 파일에 `TZ: Asia/Seoul` 명시 (앱 레벨 최소 변경으로 기존 동작 재현).
2. 날짜→문자열 변환을 한 가지 방식으로 통일: remark의 `toISOString().slice(0,10)`
`formatDate()`로 교체.
3. stage에서 승인 → DB 저장값 → 이메일 표기 날짜를 실측 검증 (phase11 잔여 항목과 함께).
---
## 3. P1 — 비즈니스 규칙·정합성
### P1-1. 체험(TRIAL) 상태 계정의 연장 신청 허용 문제
- 위치: `lib/extension-requests.ts:160-195` `createExtensionRequest`
- 문제: 유료 `AccountType`(1~5)만 검사하고 `ActivateStatus`는 검사하지 않는다.
체험 계정(ActivateStatus=1)도 AccountType 1~5를 가지므로(`TRIAL_ACCOUNT_TYPE_LABELS` 존재)
체험 계정에 연장 신청→승인하면 **결제 없이 ActivateStatus=2(활성) + 1년**이 부여된다.
승인 로직(`approveExtensionRequest`)에도 동일 검증이 없어 CANCELED(100) 계정도 재활성화된다.
- 결정 필요(비즈니스): 연장 대상을 `ActivateStatus = 2(활성)`로 제한할지,
만료/취소 계정 연장을 관리자 재량으로 허용할지.
- 활성 상태에서만 승인하도록 제한: 신청 생성과 승인 양쪽에서 `ActivateStatus` 검증 추가, 위반 시 400/409.
### P1-2. 체험 → 동일 요금제 유료 전환 신청 불가
- 위치: `lib/upgrade-requests.ts:184-186`, `UpgradeRequestsSection.tsx:42-43,110`
- 문제: `requestedAccountType <= maestro.AccountType`이면 거부한다.
그러나 체험 계정(ActivateStatus=1, AccountType=3)이 같은 3번 요금제를 유료 전환하는 것은
정당한 흐름이며, 가격 계산(`calculateAdditionalPrice`)과 완료 메일도 이미 이 케이스를 지원한다.
현재는 관리자 대행 등록이 불가능하다(같은 tier가 select에서 disabled + 서버 400).
- 개선: `ActivateStatus === TRIAL`이면 `requestedAccountType >= AccountType`(동일 tier 포함) 허용,
ACTIVE면 기존대로 `>`만 허용. UI select disabled 조건도 동일하게 분기.
### P1-3. 업그레이드 승인 시점의 재검증 없음
- 위치: `lib/upgrade-requests.ts:113-168` `approveUpgradeRequest`
- 문제: 신청 생성 시엔 상향 여부를 검증하지만 승인 시엔 하지 않는다.
신청 후 관리자가 정보 수정(Phase 10)으로 `AccountType`을 올려두면,
대기 중인 신청을 승인할 때 **사실상 다운그레이드**가 적용될 수 있다.
- 개선: 승인 트랜잭션 안에서 maestro의 현재 `AccountType`을 읽어
`RequestedAccountType`이 여전히 상향(또는 TRIAL 전환)인지 확인, 아니면 409 + 안내 메시지.
### P1-4. 연장 신청 등록이 트랜잭션 미사용 (일관성 위반)
- 위치: `lib/extension-requests.ts:160-195`
- 문제: maestro 조회 → insert가 트랜잭션 밖에서 수행된다. AGENTS 규칙
("모든 변경은 트랜잭션") 위반이며, `createUpgradeRequest`(트랜잭션 사용)와 비일관.
- 개선: `db.$transaction`으로 감싸 upgrade 쪽과 동일 구조로 정리.
### P1-5. 신청 등록 시 maestro_log 미기록
- 위치: `createExtensionRequest`, `createUpgradeRequest`
- 문제: 관리자 대행 신청 등록은 DB 변경인데 로그를 남기지 않는다.
("성공한 DB 변경 후 항상 로그 기록" 규칙 위반.) 나중에 신청 출처(마에스트로 본인 vs 관리자 대행)를
구분할 수 없다.
- 개선: 신규 타입 `request_extension_maestro` / `request_upgrade_maestro`(가칭)를 정의해
트랜잭션 안에서 기록. 타입 확정 후 AGENTS.md 로그 표에 추가.
### P1-6. 이메일 발송 실패가 관리자에게 보이지 않음
- 위치: `app/api/extension-requests/[id]/route.ts:43-47`, `app/api/upgrade-requests/[id]/route.ts:43-50`
- 문제: `void sendXxxDoneEmail(...)` fire-and-forget. 실패해도 로그만 남고
승인 응답은 `ok: true` → 관리자는 발송 성공으로 인지. 재발송 수단도 없다.
- 개선(단계적):
1. `await`로 결과를 받아 응답에 `emailSent: boolean` 포함, 실패 시 목록 UI에 경고 표시.
(커밋 후 발송이므로 트랜잭션 규칙과 충돌 없음. 지연은 SMTP 1회 호출 수준.)
2. (후속) 실패 건 재발송 버튼 또는 관리자 알림.
- 참고: 현재 Docker 상주 프로세스라 void 실행이 잘리지는 않지만, 배포 방식이 바뀌면
응답 후 프로세스 중단으로 발송 누락 가능 — await 전환으로 함께 해소된다.
### P1-7. 관리자 대행 신청 등록 시 입금 안내 메일 부재 (정책 확인)
- 문제: 기존 서비스는 마에스트로가 신청하면 입금 안내 메일을 발송한다.
관리자 대행 등록(Phase 12)은 메일을 보내지 않는데, 의도된 생략인지 확인이 안 됨.
- 확인: 기존 서비스와 달리 관리자가 대행해서 등록했을 경우 메일을 보내지 않는 것이 의도가 맞다.
- 개선: 정책 결정 후 문서화(AGENTS.md). 발송하기로 하면 기존 안내 메일 템플릿 이식.
---
## 4. P2 — 코드 품질·보완
| # | 항목 | 위치 | 내용 |
|---|---|---|---|
| P2-1 | 매직 넘버 | `lib/extension-requests.ts:125` | `ActivateStatus: 2``ACTIVATE_STATUSES.ACTIVE` 상수 사용 |
| P2-2 | 날짜 입력 silent 무시 | `lib/maestros.ts:207-227` | `availableActivateDateTime`이 파싱 불가면 조용히 건너뜀 → 400 반환이 명확 |
| P2-3 | datetime-local 파싱 TZ | `lib/maestros.ts:208` | `new Date("YYYY-MM-DDTHH:mm")`은 서버 TZ로 해석. 관리자 브라우저 TZ ≠ 서버 TZ면 저장값 이동. P0-3에서 TZ 고정 후 재확인 |
| P2-4 | 이름 중복 검사 TOCTOU | `lib/maestros.ts:153-161` | 검사와 update 사이 레이스. unique 제약이 없으므로(스키마 변경 금지) 검사를 트랜잭션 안으로 이동하는 정도로 완화. 이메일 중복 검사는 아예 없음 — 기존 서비스 정책 확인 필요 |
| P2-5 | 미확인 예외 rethrow | `lib/api-handler.ts:47-51` | 예상 외 오류가 Next 기본 500(HTML)으로 반환. JSON `{ message }` 500 응답으로 통일 |
| P2-6 | 세션 만료 기본 30일 | `auth.ts` | 관리자 도구 치고 길다. `session.maxAge` 단축(예: 8~24h) 검토 |
| P2-7 | 로그인 브루트포스 방어 없음 | `auth.ts:63-96` | rate limit 없음. 공개망 노출 시 실패 횟수 제한/지연 도입 검토. `PASSWORD()` 해시 마이그레이션은 기존 계획대로 별도 과제 |
| P2-8 | 목록 기본 필터 | `lib/extension-requests.ts:23-27` 등 | `status` 파라미터가 없으면 `optional`이 catch보다 먼저 적용돼 전체 표시. 기존 관리자 기본은 미처리(Status<2)였음 — 기본값을 `REQUESTED`로 할지 결정 (`.default(REQUEST_STATUSES.REQUESTED)`) |
| P2-9 | SQL 파라미터 인라인 로깅 | `lib/db.ts:15-29` | debug 한정이지만 이름/이메일 등 PII가 로그에 남음. stage에서 실데이터 사용 시 phase9 마스킹 규칙과 대조해 마스킹 대상 확정 |
| P2-10 | 연장 승인 시 요청 AccountType 불일치 | `lib/extension-requests.ts:140-150` | 신청 시점 AccountType으로 로그/메일 remark 생성 — 신청 후 요금제가 바뀌면 현재값과 다름. 승인 시 maestro 현재값 기준으로 표기할지 결정 |
---
## 5. 이행 순서 제안
1. **Step 1 (P0, 코드 수정 소규모) ✅ 완료 (2026-07-04)**
- P0-1: extension `getActionTarget` select 전환 + db.ts Password 마스킹
- P0-2: 승인/취소/거절 4개 함수 조건부 UPDATE 가드
- P0-3: docker-compose `TZ: Asia/Seoul` + 날짜 문자열 변환 통일
2. **Step 2 (P1 비즈니스 결정 → 반영) ✅ 완료 (2026-07-04)**
- P1-1: 연장 신청·승인 모두 ActivateStatus=2(ACTIVE) 필수 검증 추가
- P1-2: TRIAL 계정의 동일 tier 업그레이드(체험→유료 전환) 허용
- P1-3: 업그레이드 승인 시점에 현재 AccountType 기준 방향 재검증
- P1-4: `createExtensionRequest` 트랜잭션 감싸기
- P1-5: `request_extension_maestro` / `request_upgrade_maestro` 로그 추가
- P1-7: 관리자 대행 등록 시 입금 메일 미발송 — 의도 확인, AGENTS.md에 명시
- P2-10: 신청 시점 스냅샷 유지 — 변경 없음
3. **Step 3 (P1-6 이메일 + 검증) ✅ 완료 (2026-07-04)**
- API route 2곳: `void sendXxxDoneEmail``await emailSent`, 응답에 `emailSent: boolean` 포함
- 테이블 UI 2곳: 응답 body를 단일 파싱으로 통일, 승인 성공 후 `emailSent === false`이면 인라인 경고 표시 (`router.refresh()` 후에도 client state 유지됨)
- stage 실측(체험→유료, 유료→유료 메일 검증)은 별도 수동 테스트 항목으로 남김
4. **Step 4 (P2 일괄) ✅ 완료 (2026-07-04)**
- P2-2: `updateMaestro`에서 파싱 불가 날짜 silent skip → 400 반환으로 교체 (`lib/maestros.ts`)
- P2-5: `withApiHandler` catch 블록 `throw error` → JSON 500 응답으로 교체 (`lib/api-handler.ts`)
- P2-6: NextAuth `session.maxAge: 8 * 60 * 60` 추가 (`auth.ts`)
- P2-7: 인메모리 브루트포스 방어 추가 (`auth.ts`) — 5회 실패 시 15분 잠금, 성공 시 카운터 초기화
- P2-8: 연장·업그레이드 신청 목록 `status` 기본값 `undefined`(전체) → `REQUEST_STATUSES.REQUESTED` 변경; 결과 타입 `status?: number | "all"``status: number | "all"` 업데이트; 초기화 링크 `?status=1` 포함으로 변경
- P2-4: 이메일 중복 검사 추가 안 함 (기존 서비스 정책 유지)
- P2-9: SQL 파라미터 PII 로깅 정책 — 이름·이메일 등이 debug 레벨에서 노출됨. stage에서 실데이터 사용 시 phase9 마스킹 규칙과 대조해 마스킹 대상 확정 (별도 작업)
- P2-10: 변경 없음 (신청 시점 스냅샷 유지)
- AGENTS.md 인증 섹션 및 구현 phase 목록 갱신
각 Step 완료 시 AGENTS.md의 규칙 표(로그 타입, 검증 규칙)를 함께 갱신한다.
---
## 6. 검토했으나 문제 없음으로 확인한 사항
- 승인/취소/거절의 트랜잭션 구성과 `Status=1` 사전 확인(단일 요청 기준)은 규칙대로 구현됨.
- 업그레이드 승인이 `PlayerCount`를 변경하지 않음 — 규칙 준수.
- 메일 발송은 커밋 이후 수행 — 규칙 준수.
- 모든 API 라우트가 `withApiHandler``auth()` 검증 — 인증 우회 경로 없음
(`proxy.ts` matcher가 `/api/`를 제외하지만 핸들러 레벨에서 401 처리됨).
- 로그인 쿼리는 태그드 템플릿 파라미터 바인딩 사용 — SQL 인젝션 없음.
`PASSWORD(` 포함 쿼리는 SQL 로깅에서 파라미터 인라인을 생략함.
- 검색 파라미터는 zod `.catch()`로 방어되어 잘못된 입력이 500을 유발하지 않음.
- NextAuth 쿠키 SameSite=Lax + same-origin fetch 구조로 CSRF 위험은 낮음.