132 lines
8.5 KiB
Markdown
132 lines
8.5 KiB
Markdown
# 02 · SECURITY — Phase 1 Review Aid (Accepted Risks + Per-Feature Checklist)
|
||
|
||
> **What this is:** a review aid, not a policy manual. Tick items during development and review. Before marking any feature `[x]` in `Backend/PROGRESS.md`, run its checklist here (Part C) plus the foundational controls (Part B).
|
||
> **Navigation:** you arrived from `00-CORE.md`. Business rules are in `10-BACKEND-PHASE1.md`, the API contract in `11-BACKEND-PHASE1.md`, the frontend validation posture in `20-FRONTEND.md`.
|
||
> **Context:** Phase 1 is single-tenant with **RBAC deferred** and **approvals auto-on**. Several exposures below are *deliberate, accepted* decisions — they are recorded in Part A so they are explicit, not implicit.
|
||
|
||
---
|
||
|
||
## Part A — Accepted Risks Register
|
||
|
||
These are known, deliberately-accepted Phase-1 exposures. Each has a compensating control and a trigger to revisit. Do not "fix" them ad-hoc during Phase 1 — they are tracked here and closed in Part D order.
|
||
|
||
| ID | Risk | Why accepted | Compensating control | Revisit trigger |
|
||
|---|---|---|---|---|
|
||
| **AR-01** | **No authorization** — any authenticated user can call any endpoint (post adjustments, confirm GRN, cancel PO, write off stock). | RBAC deferred (FR-X-01), Phase-1 scope. | Authentication required; **immutable audit trail** attributes every action. | First post-Phase-1 hardening → enable RBAC. |
|
||
| **AR-02** | **Approvals auto-on** — no value gate on PO or adjustment. | Phase-1 decision (FR-PROC-04, FR-STK-07). | Audit + mandatory reason codes. | Enable adjustment approval first, then PO approval. |
|
||
| **AR-03** | **IDOR** — sequential IDs + no RBAC → a user can access another's document by guessing the ID. | Consequence of AR-01. | Audit trail. | Closed with RBAC. |
|
||
| **AR-04** | **GRN-without-PO** — receive (and value) goods with no order; a fraud vector. | Emergency/direct receipt is useful (FR-GRN-02). | Permission-gated (when RBAC on); flagged for review; audit; cost entered here gets extra scrutiny (see C.3). | With RBAC / policy on direct receipts. |
|
||
| **AR-05** | **In-transit loss window** — dispatched-but-not-received stock is untracked shrinkage risk. | Inherent to in-transit transfers (FR-STK-05). | In-transit aging monitoring report. | Add stuck-transfer alert (Part D). |
|
||
| **AR-06** | **Localhost dev secrets** in `appsettings.Development.json`. | Local-dev convenience, current phase. | `.gitignore` + localhost only. | Before any shared/staging/prod → User Secrets / env vars; rotate. |
|
||
|
||
---
|
||
|
||
## Part B — Foundational Controls (cross-cutting — apply everywhere)
|
||
|
||
### B.1 Authentication & JWT
|
||
- [ ] Strong signing key (≥ 256-bit) from env/secrets; the dev `CHANGE_ME` key never reaches non-local environments
|
||
- [ ] Algorithm pinned (HS256 or RS256); reject `alg: none` and algorithm-confusion
|
||
- [ ] Validate issuer, audience, lifetime, and signature on every request
|
||
- [ ] Short access-token lifetime; define refresh strategy if needed
|
||
- [ ] Passwords hashed with a strong KDF (ASP.NET Identity / PBKDF2 / bcrypt / argon2) — never plaintext/reversible
|
||
- [ ] `/auth/login` rate-limited + backoff/lockout (brute-force)
|
||
- [ ] Generic auth-failure messages (no account-enumeration signal)
|
||
|
||
### B.2 Token storage & CSRF *(httpOnly-cookie decision)*
|
||
- [ ] Token in an **httpOnly, Secure** cookie (never localStorage) — removes XSS token theft
|
||
- [ ] `SameSite=Strict` (or `Lax`) on the auth cookie
|
||
- [ ] **CSRF protection on every state-changing request** (anti-forgery / double-submit token) — cookies are sent automatically, so CSRF is now in scope
|
||
- [ ] CORS locked to the known frontend origin(s); credentials mode aligned with the cookie
|
||
- [ ] Cookie scoped minimally (path/domain), Secure flag on
|
||
|
||
### B.3 Audit integrity *(this is the compensating control for AR-01 — it must hold)*
|
||
- [ ] Audit log **and** stock ledger are append-only **at the DB level** (the app's DB role has no `UPDATE`/`DELETE` on those tables)
|
||
- [ ] Every mutation records who / when / old→new / reason (FR-X-02)
|
||
- [ ] No API path edits or deletes a posted transaction (FR-X-05); corrections are reversing entries
|
||
- [ ] Actor derived from token `sub`, never from the request body
|
||
|
||
### B.4 Transport & headers
|
||
- [ ] HTTPS only; HSTS in production
|
||
- [ ] Security headers: CSP, `X-Content-Type-Options: nosniff`, frame-ancestors/`X-Frame-Options`, `Referrer-Policy`
|
||
|
||
### B.5 Error & logging hygiene
|
||
- [ ] `ProblemDetails.detail` leaks no stack traces / connection strings / internals in production
|
||
- [ ] Serilog never logs tokens, passwords, cookies, or secret-bearing request bodies
|
||
- [ ] Correlation/trace id present for support without exposing sensitive data
|
||
|
||
### B.6 Input & injection
|
||
- [ ] No raw or string-concatenated SQL; EF Core parameterizes — keep it that way
|
||
- [ ] **DTOs are narrow** — no mass-assignment/over-posting; client cannot set `status`, ids, `createdBy`, timestamps, or computed fields
|
||
- [ ] Server-side validation is authoritative (mirrors `20-FRONTEND §3`); client validation is UX only
|
||
- [ ] Pagination bounds enforced (`pageSize` max) to prevent resource exhaustion
|
||
|
||
### B.7 Concurrency as integrity
|
||
- [ ] FIFO layer consumption is **row-locked** inside the UoW transaction (no double-spend)
|
||
- [ ] `ETag`/`If-Match` on mutable resources; `412` on stale writes
|
||
|
||
---
|
||
|
||
## Part C — Per-Feature Checklists
|
||
|
||
### C.1 Master Data (Item / UOM / Category / Vendor / Warehouse / Bin)
|
||
- [ ] Create/update DTOs exclude server-controlled fields (`status`, ids, timestamps)
|
||
- [ ] Deactivate — not delete — referenced masters (FR-MD-08); hard delete blocked → `MASTER_IN_USE`
|
||
- [ ] Nested/reference writes validate the target exists and is active
|
||
|
||
### C.2 Procurement (Requisition / RFQ / PO / Purchase Return)
|
||
- [ ] PO totals computed **server-side** from lines (never trust client totals)
|
||
- [ ] PO **edit-while-open**: every change audit-logged old→new (this is the tampering surface)
|
||
- [ ] PO cancel blocked if any receipt exists
|
||
- [ ] Purchase-return quantity validated against received/available (no over-return)
|
||
- [ ] Note in review: **AR-01/AR-02/AR-03** apply to these endpoints
|
||
|
||
### C.3 GRN
|
||
- [ ] `unitCost` **derived from the PO line server-side**; any client-supplied cost is ignored *(decision locked)*
|
||
- [ ] `receivedValue` computed server-side (qty × PO-line cost), not accepted from client
|
||
- [ ] Direct GRN (no PO) is the exception where cost is entered → extra scrutiny + review flag + audit (**AR-04**)
|
||
- [ ] Over-receipt tolerance enforced server-side → `OVER_RECEIPT_TOLERANCE`
|
||
- [ ] On-hold stock is not issuable (FR-WH-07); expired batch blocked
|
||
|
||
### C.4 Stock Core (FIFO / Ledger)
|
||
- [ ] FIFO consumption row-locked (B.7) — integrity + no double-spend
|
||
- [ ] Negative-stock block enforced server-side → `STOCK_NEGATIVE_BLOCKED`
|
||
- [ ] Ledger append-only (B.3)
|
||
- [ ] Valuation computed from layers server-side; never from client input
|
||
|
||
### C.5 Adjustments — **HIGHEST-RISK FEATURE IN THE PHASE**
|
||
Auto-post + no approval + direct write-off = the primary theft/fraud surface. Reason code + user stamp are the *only* live controls.
|
||
- [ ] Reason code mandatory, server-enforced → `REASON_CODE_REQUIRED`
|
||
- [ ] User stamp mandatory (from token)
|
||
- [ ] Decrease cannot drive available negative → `STOCK_NEGATIVE_BLOCKED`
|
||
- [ ] `qtyDelta` sanity bounds; large write-offs surface on a review report
|
||
- [ ] Review note: **AR-02** — re-enabling adjustment approval is the **first** control to turn on post-Phase-1 (Part D)
|
||
|
||
### C.6 Transfers
|
||
- [ ] Dispatch validates available ≥ qty → block otherwise
|
||
- [ ] Cost-preserving (dest inherits source cost) — no revaluation via transfer
|
||
- [ ] `destWarehouseId != srcWarehouseId`
|
||
- [ ] In-transit aging monitored (**AR-05**: stuck/never-received transfers)
|
||
|
||
### C.7 Counts
|
||
- [ ] Variance posting is an adjustment in disguise → apply C.5 controls
|
||
- [ ] System-qty snapshot immutable once the count is opened
|
||
- [ ] Large variances flagged for review
|
||
|
||
---
|
||
|
||
## Part D — Post-Phase-1 controls to enable (in order)
|
||
1. **Adjustment approval** (config flag already reserved) — closes the top fraud surface (AR-02, C.5).
|
||
2. **RBAC enforcement** (role→permission) — closes AR-01, AR-03, AR-04.
|
||
3. **PO approval** (value thresholds) — closes remaining AR-02.
|
||
4. **Monitoring reports** — stuck-transfer aging (AR-05) and large-variance/write-off review.
|
||
|
||
---
|
||
|
||
## Part E — Locked security-relevant decisions
|
||
- **GRN `unitCost` from the PO line** (server-derived; not user-entered). Direct GRN is the audited exception.
|
||
- **Frontend token in an httpOnly Secure cookie + CSRF protection** (not localStorage).
|
||
|
||
---
|
||
|
||
*End of 02-SECURITY.md. Run the relevant checklist before ticking a feature in `Backend/PROGRESS.md`.*
|