greeneclipse
8580c996c3
Drop one-shot tmp/axe scripts and agent i18n scratch so the Gitea tree is deployable.
96 lines
4.6 KiB
Markdown
96 lines
4.6 KiB
Markdown
# 17 — Security pass: staff roles, plan features, support tickets
|
|
|
|
**Agent:** 17/20 · **Scope:** API handlers / middleware security fixes (no git).
|
|
|
|
## Goal
|
|
|
|
Close IDOR, missing authz, mass-assignment, and CSRF gaps around platform staff, plan-feature admin APIs, and support tickets. Enforce **least privilege** for `support_staff`. Keep secrets out of logs. Add forbidden-access tests.
|
|
|
|
## Threat model (in scope)
|
|
|
|
| Actor | May access | Must not |
|
|
|-------|------------|----------|
|
|
| Customer (company member) | Own tickets in active company | Other users' tickets; admin APIs; internal notes |
|
|
| `support_staff` | Support desk queue (unassigned + own) | Plan features, feature gates, billing, settings, users list, credits |
|
|
| `admin` / `developer` / legacy `is_platform_admin` | Full `/api/admin/*` | N/A (full staff) |
|
|
| Unauthenticated | Public / CSRF cookie seed on GET | Mutating `/api/*` without CSRF |
|
|
|
|
## Controls implemented
|
|
|
|
### 1. Staff roles (`users.staff_role`)
|
|
|
|
- Migration: `apps/api/sql/schema/029_staff_roles.sql`
|
|
- Values: `admin` \| `developer` \| `support_staff` (NULL allowed)
|
|
- Resolver: `auth.ResolveStaffAccess` / `auth.GetStaffAccess`
|
|
- `support_staff` → **support desk only** (even if `is_platform_admin=true`)
|
|
- `admin` / `developer` → full admin + support desk
|
|
- NULL + `is_platform_admin` → **legacy full admin** (backward compatible)
|
|
- Missing column (pre-migration) → boolean-only fallback
|
|
|
|
### 2. Middleware least privilege
|
|
|
|
| Middleware | Allows |
|
|
|------------|--------|
|
|
| `RequirePlatformAdmin` | Full admin only (`FullAdmin`) |
|
|
| `RequireSupportDesk` | Full admin **or** `support_staff` |
|
|
|
|
Route split in `Server.Router` (`server.go`):
|
|
|
|
- `/api/admin/support/*` → `RequireSession` + `RequireSupportDesk`
|
|
- All other `/api/admin/*` (plans, features, gates, billing, settings, users, …) → `RequireSession` + `RequirePlatformAdmin`
|
|
|
|
CSRF remains on the outer session group (double-submit cookie + `X-CSRF-Token`). Admin mutations are not CSRF-exempt.
|
|
|
|
### 3. Support ticket IDOR / visibility
|
|
|
|
- Customer paths always bind `company_id` + `user_id` from **session context** (not JSON body). Handlers fail closed if context missing.
|
|
- `GetForUser` / `ReplyAsUser` already filter by company + creator; internal notes excluded at SQL.
|
|
- Customer reply uses `UserReplyInput` (`body` only) — **mass assignment** of `is_internal_note` / `status` rejected via `DisallowUnknownFields`.
|
|
- `support_staff` list forced to `UnassignedOrSelf` (client `assignee_id` ignored).
|
|
- `support_staff` get/reply/update of another agent's ticket → **404** (anti-enumeration).
|
|
- Assignee updates: zero UUID rejected; assignee must be support-capable; `support_staff` may only assign self (or clear).
|
|
|
|
### 4. Plan feature admin APIs
|
|
|
|
- Remain behind `RequirePlatformAdmin` (support_staff → 403).
|
|
- Feature keys / sections already allowlisted in billing (`validateFeatureOverrides` / `validateGatesUpdate`).
|
|
- Unknown JSON fields rejected by `DecodeJSON`.
|
|
|
|
### 5. Secrets in logs
|
|
|
|
- `LogAndError` redacts password/secret/api_key/token/`sk_live`/`whsec_` patterns via `redactForLog`.
|
|
- Request logger continues to log method/path/status only (no bodies).
|
|
|
|
## Tests
|
|
|
|
| Test | Asserts |
|
|
|------|---------|
|
|
| `auth.TestResolveStaffAccess` | Capability matrix |
|
|
| `TestRequireSupportDeskForbiddenAndAllow` | 401 / member 403 / support_staff 204 |
|
|
| `TestRequirePlatformAdminExcludesSupportStaff` | support_staff blocked from full admin |
|
|
| `TestSupportStaffForbiddenOnPlanFeatures` | plan/gate routes 403 for support_staff |
|
|
| `TestMemberForbiddenOnAdminSupportAndPlanRoutes` | plain users 403 |
|
|
| `TestUserReplyMassAssignmentRejected` | unknown fields on customer reply |
|
|
| `TestStaffMayAccessTicket` | visibility rules |
|
|
| `TestRedactForLog` | secrets scrubbed |
|
|
| Existing `TestTicketCRUDAuthOwnership` | customer IDOR ownership |
|
|
| Existing CSRF tests | mutating requests need token |
|
|
|
|
Run:
|
|
|
|
```bash
|
|
cd apps/api
|
|
go test ./internal/auth ./internal/httpapi -count=1 -run "Staff|SupportDesk|PlatformAdmin|Redact|UserReply|MemberForbidden"
|
|
```
|
|
|
|
## Residual / handoff
|
|
|
|
- Staff **assignment APIs** (grant/revoke `staff_role`) owned by agent 6 — must call `NormalizeStaffRole` and remain `RequirePlatformAdmin`.
|
|
- Claim race / CSAT / email stubs owned by support backend agents; keep visibility checks on every new staff handler.
|
|
- Web UI gates should mirror `SupportDesk` vs `FullAdmin` (do not trust client-only hides).
|
|
- Apply migration `029_staff_roles` before relying on `staff_role` in production.
|
|
|
|
## Rollback
|
|
|
|
Revert middleware/route split + `029_staff_roles` down migration; restore prior `RequirePlatformAdmin` on all `/api/admin/*`.
|