ImanThiyanga
f02c89b3cb
- Updated purchase order descriptions for clarity on draft and submission processes. - Implemented submit and delete functionalities for draft purchase orders, allowing users to manage their orders more effectively. - Added discount and VAT fields to GRN lines, enabling better cost tracking and reporting. - Enhanced validation for GRN lines to ensure discount and VAT percentages are within acceptable ranges. - Updated API to support new functionalities, including submitting and deleting purchase orders. - Improved UI components for better user experience in managing purchase orders and GRNs. - Documented changes in security and backend phase documentation to reflect new processes and requirements.
134 lines
11 KiB
Markdown
134 lines
11 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. |
|
|
| **AR-07** | **`getUserDetails` / `LogoutUser` callable without a bearer token** — `GET /api/v1/auth/users/{userId}` and `POST /api/v1/auth/logout` resolve the target user from the URL/payload, not the caller's session, so any anonymous caller can fetch a profile or log out an arbitrary user's sessions by GUID. | Carried over verbatim from AuthHex's own dispatcher contract (API_REFERENCE.md §3) — ERPCore's `AuthController` proxies it as-is rather than silently tightening a contract it doesn't own. | GUIDs are not enumerable; every call is written to `AuthEventLogs` upstream in AuthHex. | Revisit once AuthHex exposes a token-scoped variant, or add ERPCore-side rate limiting / auth requirement ahead of AuthHex. |
|
|
| **AR-08** | **No rate limiting on `AuthController`'s anonymous endpoints** (login, register, refresh, recovery, OTP send/verify) — brute-force and account-enumeration exposure. | Not built in this pass (docs/11 §2.0, added 2026-07-16); AuthHex may rate-limit server-side but ERPCore does not add its own layer yet. | AuthHex's own lockout/backoff (per docs/10 NFR-03), immutable audit trail. | Add ASP.NET Core rate limiting middleware to `AuthController` before any non-local deployment. |
|
|
|
|
---
|
|
|
|
## 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)*
|
|
- [x] Token in an **httpOnly, Secure** cookie (never localStorage) — removes XSS token theft (`AuthCookieWriter.WriteSession`, `erp_at`/`erp_rt`, 2026-07-16)
|
|
- [x] `SameSite=Strict` on the auth cookie (`AuthCookieWriter`; assumes frontend + ERPCore share a registrable domain — revisit if deployed cross-domain)
|
|
- [x] **CSRF protection on every state-changing request** (double-submit `XSRF-TOKEN` cookie + `X-XSRF-TOKEN` header, `ValidateCsrfAttribute`, applied to every mutating `AuthController` action; Bearer-header callers exempt since they aren't cookie-driven)
|
|
- [ ] CORS locked to the known frontend origin(s); credentials mode aligned with the cookie — **not yet configured**; required before any browser frontend can call these endpoints cross-origin (tracked with the frontend-wiring follow-up)
|
|
- [x] Cookie scoped minimally (path/domain), Secure flag on (`erp_rt` scoped to `/api/v1/auth/refresh-token`; all three cookies `Secure=true`)
|
|
|
|
### 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` **defaults to the PO line price**; a per-line override **is now permitted** *(decision revised 2026-07-20 — was "locked, client cost ignored")*. When an override is entered it is used, and the PO price is snapshotted (`poUnitPrice`) so a **`priceVariance` is recorded** against it for review. Rationale: one PO legitimately spans batches received at different prices; the variance trail (plus the audit log) is the compensating control that replaces the old hard block.
|
|
- [ ] **Derived figures stay server-computed** — `netUnitCost`/`receivedValue`/`vatAmount`/`lineTotal` are never accepted from the client, so the client cannot inflate stock value except by an *auditable* unit-cost override. Discount reduces inventory cost; **VAT is recoverable and never enters stock value**.
|
|
- [ ] Direct GRN (no PO) remains the higher-scrutiny path where cost is entered with no PO to compare against → 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`.*
|