Code Review

# 認可（Authorization）コードレビュー観点（一般編）
クライアント - サーバー構成での「認可」の実装をレビューするための、再現性の高いチェックリスト。

---

## 1. このガイドのスコープ
- 対象：Web/モバイル等の **クライアント** と **サーバー（API/BFF）** で構成される一般的なアプリ
- 目的：**IDOR（他人のデータ参照/更新）**、権限昇格、情報漏えい、CSRF/キャッシュ漏れ等の典型事故をレビュー段階で検出する
- 非対象：DB側でのRLSの詳細（→別ファイル「RLS編」で扱う）

---

## 2. コア原則（レビューでブレないための共通言語）
1. **クライアントは信用しない**  
   認可の最終判断は必ずサーバーで行う（クライアントはUXのための出し分けのみ）。
2. **ユーザー文脈（userId/tenantId/role）はサーバーで確定する**  
   body/query/ヘッダ等のユーザー入力に依存してはいけない。
3. **入口（API）で必ず認可し、データ取得/更新の直前でも再確認する**  
   “入口でチェックしたから後はOK” の設計は漏れやすい。
4. **認可ロジックは分散させず、共通化・宣言化する**  
   ばらばらな `if (isAdmin)` が増えると漏れが起きる。
5. **デフォルト拒否（Default Deny）**  
   “許可条件を満たすときだけ通す” を基本にする。

---

## 3. 用語の整理（レビュー時の誤解を防ぐ）
- 認証（Authentication）：あなたは誰か（ログイン、セッション検証）
- 認可（Authorization）：あなたはそれをして良いか（操作/参照の可否）
- 認可境界：**サーバー**（およびDB等のバックエンド）  
  クライアントのUI制御は境界ではない。

---

## 4. 入口（API/Server Action/Route）レビュー観点
> すべての「外部から呼べる入口」（HTTP、RPC、Server Action 等）が対象。

### ✅ チェックリスト（入口共通）
- [ ] **認証が必須の入口**で、必ずセッション/トークン検証をしている
- [ ] 認証の結果（`userId`、`tenantId`、`scopes/roles`）が **サーバー側で確定**している
- [ ] リクエストの `userId` / `role` / `isAdmin` を **信用していない**
- [ ] **入力バリデーション**（スキーマ検証）があり、型だけに依存していない
- [ ] エラー時に **適切なステータス**（401/403/404）で返している（後述）
- [ ] “公開API” と “内部API（管理者/バッチ用）” が **同じ入口/同じ権限で混ざっていない**

### ✅ チェックリスト（権限判定）
- [ ] 認可判定が **共通関数/ミドルレイヤ**に集約されている  
  例：`requireAuth()` / `requireRole()` / `requirePermission()` / `authorize(action, resource)`
- [ ] 認可判定が **リソース単位**で行われている（単なるロール判定だけで終わっていない）
- [ ] “参照可能” と “更新可能” が混同されていない（閲覧できても更新できない等）
- [ ] **マルチテナント**の場合、`tenantId` 境界が必ずチェックされている
- [ ] 認可判定に必要なデータ（所有者、メンバーシップ等）の取得が正しい
    - [ ] “クライアントが送ったownerId” を根拠にしていない
    - [ ] “DBにある真実” から判断している

---

## 5. データアクセス層レビュー観点（IDOR・漏えい対策の要）
> 認可が弱いとき、最後に壊れるのはここ。

### ✅ チェックリスト（取得・更新クエリ）
- [ ] **スコープ/テナント/所有者**の条件が必ず付く
    - 例：`WHERE tenant_id = ctx.tenantId AND ...`
- [ ] “IDだけで取得” がある場合、**必ず境界条件**が追加されている
    - NG例：`getById(id)` が tenant/owner を見ていない
- [ ] 更新・削除は **対象行が許可範囲か** を保証する
    - 例：`UPDATE ... WHERE id = $id AND tenant_id = $tenantId`
- [ ] “一覧取得” は **フィルタ漏れ**しやすいので特に注意
    - 例：管理者/一般ユーザーで条件が分岐する場合の漏れ
- [ ] **返却フィールド**が権限に応じて適切（Field-level authorization）
    - 例：`email`, `billing`, `internalNotes` 等を全員に返していない

### ✅ チェックリスト（共通化）
- [ ] “安全なクエリの型” を共通化できている
    - 例：`scopedToTenant(ctx).projects.findMany(...)`
- [ ] 直書きクエリが散在していない（レビュー漏れを起こす）

---

## 6. クライアント側（UI/UXの認可）レビュー観点
> セキュリティ境界ではないが、事故の予兆を見つける重要ポイント。

### ✅ チェックリスト（Client）
- [ ] UIの出し分けはしているが、**サーバー側の認可に依存**している  
  （UIで隠してもサーバーが許可しない）
- [ ] クライアントで `isAdmin` 等を **ローカルストレージ/URL/任意入力**から作っていない
- [ ] “権限情報” はサーバーが確定したデータから来ている（`/me` 等）
- [ ] 401/403 の扱いが適切
    - 401：ログイン誘導
    - 403：権限不足表示（必要なら 404 相当の扱い）
- [ ] ユーザー固有データが **共有キャッシュ**で混ざらない（特にSSR/キャッシュ層）

---

## 7. セキュリティ周辺（認可とセットで事故りやすい）
### ✅ CSRF（Cookieベースのセッションを使う場合は要注意）
- [ ] 状態変更（POST/PUT/PATCH/DELETE）でCSRF対策がある  
  例：Origin/Referer検証、CSRFトークン、SameSite運用の前提が明文化

### ✅ CORS / Credentials
- [ ] `Access-Control-Allow-Origin: *` と `credentials: true` の組み合わせをしていない
- [ ] 許可オリジンが明示され、環境ごとに正しく分離されている

### ✅ キャッシュ
- [ ] ユーザー固有レスポンスはキャッシュされない、またはユーザーごとに分離される  
  例：`Vary: Cookie/Authorization`、キャッシュキーに user を含める、`no-store`
- [ ] “認可結果” 自体を共有キャッシュしていない

### ✅ 監査ログ
- [ ] 重要操作（権限変更、支払い、公開/非公開など）に監査ログがある
- [ ] ログに機密情報（トークン、パスワード、個人情報の過剰）が入っていない

---

## 8. エラーハンドリング方針（403 vs 404）
レビューで揺れやすいので、チーム方針を決める。

- 401：未認証（ログインしていない/セッション無効）
- 403：認証済みだが権限なし（存在を明示してよい）
- 404：**存在秘匿**したいリソース（“見つからない” と返す）

### ✅ チェックリスト
- [ ] リソース種別ごとに 403/404 の方針が明文化されている
- [ ] 実装が方針に沿って一貫している（入口ごとにブレていない）

---

## 9. よくあるアンチパターン（見つけたら指摘）
- [ ] クライアント由来の `userId` を信用してクエリする（典型IDOR）
- [ ] “role=admin” をクライアント入力で受け取り、サーバーが信じる
- [ ] エンドポイント追加時に `requireAuth/authorize` を呼び忘れる
- [ ] 一部のAPIだけ “内部用” と言って認可を省略する（後で外部から叩かれる）
- [ ] 権限不足でも詳細なエラーメッセージで存在や内部情報が漏れる

---

## 10. 最低限のテスト要件（PRのDoD）
- [ ] 代表的なロール/権限で “できる/できない” のテストがある
- [ ] **否定テスト**（他人のIDでアクセスして拒否される）がある
- [ ] マルチテナントの場合、tenant跨ぎのアクセス拒否テストがある
- [ ] 401/403/404 の期待値テストがある（存在秘匿の方針を含む）

---

## 11. レビュー質問テンプレ（迷ったらこれを問う）
- その `userId/tenantId/role` はどこで確定した？（入力じゃない