The user declined to defer this to Phase 2, correctly: a public POST /orders with customerId in the body let anyone place an order on any customer's account, and an order lands on that customer's gold debt and ledger. POST /orders is now behind CustomerAuth. OrdersService.createOrder takes the customer id as a separate argument that the controller fills from the token; the body's customerId is ignored and a mismatch is logged. The field stays optional in the DTO and in shared-types only so both apps keep validating while they still send it -- they drop it in Phase 2. The admin builder is unaffected: it legitimately places orders for a chosen customer and passes that id explicitly, behind the admin guard. GET /orders/:id is scoped to the owner. A foreign or nonexistent id both return 404, never 403, so ids stay unenumerable. Operators read through the new GET /orders/admin/:id behind rid order.list, declared before the :id route because Nest matches in declaration order. Both apps' CartView now gates on session.isLoggedIn rather than a remembered customerId, which is no longer a credential. OTP audit (requested): no master code, no bypass, no hardcoded code anywhere; one code path issues a token and only after bcrypt.compare. No SMS provider is wired up -- in development the code is logged once, guarded by NODE_ENV !== 'production'. Fixed while auditing (SEC-11): deliver() persisted the real message text, code included, into SmsMessage.body, which the admin SMS grid lists and full-text searches. Any operator with the sms rid could read live codes, defeating the hashing of codeHash. The stored row is now a redacted audit line. Also logged MON-08: wagePercent/discountPercent/flatRialCharge are still taken from the client on order creation. The reference is worse -- it takes rash_weight from the client too -- so this is already a partial improvement; logged rather than changed, per D4. Tests: auth-smoke.sh now 53 passed, 0 failed, including a spoofed body customerId being ignored in favour of the token. Typecheck clean on api, user-app, user-app-v2 and shared-types. [ref: henza_server ApiMobileController.php:533,536-545,615-632] Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
238 lines
12 KiB
Markdown
238 lines
12 KiB
Markdown
# CHANGELOG — parity changes
|
||
|
||
Every change made to `gold-banking-monorepo` during the parity project, with the
|
||
reason and the reference path that justifies it.
|
||
|
||
Format:
|
||
|
||
```
|
||
## <date> — <phase> — <group title>
|
||
- **What:** <the change>
|
||
- **Why:** <the parity gap it closes>
|
||
- **Reference:** <henza_* path:line>
|
||
- **Target:** <gold-banking-monorepo path:line>
|
||
- **Matrix row:** <id in 01-ADMIN-MATRIX.md / 02-APPS-MATRIX.md>
|
||
- **Commit:** <sha>
|
||
```
|
||
|
||
---
|
||
|
||
## 2026-09-21 — Phase 0 — Setup
|
||
|
||
- **What:** Created branch `parity/phase-0` and `docs/parity/` scaffolding
|
||
(`PROGRESS.md`, `ISSUES.md`, `CHANGELOG.md`).
|
||
- **Why:** Persistent state required by `CLAUDE.md` §6.
|
||
- **Reference:** n/a
|
||
- **Target:** `docs/parity/`
|
||
- **Matrix row:** n/a
|
||
|
||
_No source code changed in Phase 0 (read-only phase)._
|
||
|
||
---
|
||
|
||
## 2026-09-22 — Security hotfix, stage 1 — close the unauthenticated write paths
|
||
|
||
Branch `parity/security-hotfix`. Authorised by the user's rulings D7, T1–T3.
|
||
|
||
### 1. Ledger routes are admin-only (SEC-09)
|
||
|
||
- **What:** removed the controller-level `@Public()` from `AccountingController`
|
||
and gave each route a rid: `wallet.list` (balance), `wallet.charge`
|
||
(reconcile), `document.create` (both repayments), `document.list.confirm`
|
||
(confirm entry).
|
||
- **Why:** four of the five routes WRITE to the ledger and were reachable with no
|
||
token at all. Faithful to the reference, whose entire admin route file is
|
||
unauthenticated, but contrary to confirmed decision D1.
|
||
- **Reference:** `henza_server/routes/admin.php:12,272` (the `auth:api` wrapper is
|
||
commented out at both ends)
|
||
- **Target:** `apps/api/src/modules/accounting/accounting.controller.ts`
|
||
- **Breakage check:** `api.getBalance()` is the only app-side consumer and it has
|
||
**zero call sites** in either user app (dead code, already logged as DEAD-01/02).
|
||
|
||
### 2. Order status change is admin-only (SEC-10)
|
||
|
||
- **What:** moved `@Public()` from the class to the two customer-facing handlers,
|
||
leaving `PATCH orders/:id/status` behind `order.list.sts.submit`.
|
||
- **Why:** it let an anonymous caller move any order's status **and** attribute
|
||
the change to any admin through the DTO's `actorAdminUserId`.
|
||
- **Reference:** an admin operation; the admin panel posts to `admin/orders/status`
|
||
(`apps/admin-panel/src/api/endpoints.ts:42,48`), so this route had no caller.
|
||
- **Target:** `apps/api/src/modules/orders/orders.controller.ts`
|
||
- **Still public:** `POST /orders` and `GET /orders/:id`, because the target has
|
||
no customer session yet — closed in stage 2.
|
||
|
||
### 3. `GET /customers` no longer leaks the customer book (SEC-08)
|
||
|
||
- **What:** added `LOGIN_LOOKUP_SELECT` (`id`, `mobile`, `isActive`,
|
||
`isArchived`) and pointed `list()` at it. Removed `fullName`, `nationalCode`,
|
||
`goldDebtGrams`, `fiatDebtRial`, `goldCreditLimitGrams`, `verificationStep`,
|
||
`cityId`, `referredByAgentId`, `nationalIdImageUrl`, `businessLicenseUrl`,
|
||
`shopImageUrl`, `createdAt`, `updatedAt` from that response.
|
||
- **Why:** the route is unauthenticated because both apps match the phone number
|
||
in the browser, so it shipped every customer's national ID number, debts,
|
||
credit limit and identity-document URLs to any visitor.
|
||
- **Reference:** login was `POST userAuth` → SMS → `POST userVerfiy`, returning
|
||
exactly one customer after the code matched
|
||
(`henza_server/app/Http/Controllers/Api/ApiMobileController.php:39-78`).
|
||
- **Target:** `apps/api/src/modules/customers/customers.service.ts`,
|
||
plus `CustomerSummary` narrowed in **both** apps' `src/api/client.ts`
|
||
(parity rule: the same logic change in both).
|
||
- **Route survives** until stage 2 replaces the client-side phone match with OTP.
|
||
|
||
### 4. Test script
|
||
|
||
- **What:** added `apps/api/scripts/auth-smoke.sh` — asserts 401 without a token,
|
||
403 with a low-privilege token, and pass-through for a super admin, on every
|
||
closed route; plus that the public routes still answer 200 and that
|
||
`GET /customers` exposes only the four login fields.
|
||
- **Result:** **23 passed, 0 failed** against a live server and the dev seed.
|
||
|
||
### Not changed
|
||
|
||
`/uploads` — per ruling T2, uploaded files including identity documents stay
|
||
public. The filename check requested alongside it passed with no changes needed
|
||
(md5 of time + name + UUID; no directory listing).
|
||
|
||
---
|
||
|
||
## 2026-09-22 — Security hotfix, stage 2 — real customer sessions
|
||
|
||
Branch `parity/security-hotfix`. Authorised by D6, D7, T1 and T3.
|
||
|
||
Stage 1 narrowed `GET /customers` but, as the user pointed out, did not close
|
||
the leak: the response still carried every customer **id**, and
|
||
`/customers/:id`, `/:id/orders` and `/:id/ledger-entries` were still public — so
|
||
two requests still reconstructed the whole book. Stage 2 removes the id-bearing
|
||
surface entirely.
|
||
|
||
### 1. Customer JWT, separate from the admin one (T1)
|
||
|
||
- **New:** `src/modules/customer-auth/` — `CustomerJwtStrategy` (passport
|
||
strategy `'customer-jwt'`), `CustomerJwtGuard`, `@CustomerAuth()`,
|
||
`@CurrentCustomer()`.
|
||
- Signed with **`JWT_CUSTOMER_SECRET`**, separate from `JWT_SECRET`, and every
|
||
token carries **`typ: 'customer'`** which the strategy verifies. Either check
|
||
alone would be enough; both are present deliberately.
|
||
- Customer state is re-read from the DB on every request, so deactivating or
|
||
archiving a customer takes effect at once rather than at token expiry.
|
||
- **Reference:** the same split existed there — `auth:api` for admins,
|
||
`auth('api-customer')` for the app.
|
||
|
||
### 2. Real OTP (D6)
|
||
|
||
- **New table `CustomerOtp`** (migration `20260922000000_customer_otp`,
|
||
additive: one new table, nothing existing touched).
|
||
- `POST /api/customer/auth/request-otp` ← `ApiMobileController::userAuth` (:39-49)
|
||
- `POST /api/customer/auth/verify-otp` ← `ApiMobileController::userVerfiy` (:51-64)
|
||
- **Parity behaviour:** the 4-digit `rand(1000,9999)` range, the
|
||
active-customer filter, `Utility::faTOen()` digit normalisation on both the
|
||
number and the code, and rejection on mismatch.
|
||
- **Deliberate improvements over the reference** (user-approved, recorded in
|
||
DECISIONS.md so the matrix does not flag them later):
|
||
- the code is stored as a **bcrypt hash**, never plain text;
|
||
- it **expires** after 2 minutes (the reference's never did);
|
||
- **5 attempts** per challenge, then lockout (the reference had no limit);
|
||
- **single use** — a consumed code cannot be replayed;
|
||
- **send throttle**: 60s cooldown per number, 5 per number and 20 per IP per
|
||
10 minutes, counted off the table so it survives restarts;
|
||
- **no enumeration** — an unregistered number returns the same `{sent:true}`.
|
||
The reference's `firstOrFail()` leaked a 404 for unknown numbers.
|
||
- No SMS gateway exists in this project (STUB-02), so the code is logged in
|
||
non-production only and the message is persisted as an `SmsMessage` row. The
|
||
reference's message text is kept verbatim for when a gateway lands.
|
||
|
||
### 3. The id-bearing customer routes are gone
|
||
|
||
- **Deleted** `customers.controller.ts` — with it `GET /customers`,
|
||
`/customers/:id`, `/:id/orders`, `/:id/ledger-entries`.
|
||
- **Replaced by** `/api/customer/me`, `/me/orders`, `/me/ledger-entries`, which
|
||
take the id **from the token**. There is no longer any route that accepts a
|
||
customer id, so the IDOR is closed structurally rather than by a check.
|
||
- `CustomersService` stays and is reached only through those routes.
|
||
- The admin panel was unaffected — it uses `admin/customers-omde/*`.
|
||
|
||
### 4. Both apps migrated together (parity rule)
|
||
|
||
- `stores/session.ts`: `token` is now the credential and `isLoggedIn` is
|
||
token-based. `customerId` is kept for display only and is no longer an
|
||
identity claim.
|
||
- `api/client.ts`: sends `Authorization: Bearer`, throws `UnauthorizedError` on
|
||
401 and calls the handler registered in `main.ts`, which clears the session
|
||
and routes to login so stale personal data cannot stay on screen.
|
||
- `LoginView.vue`: real two-step OTP. Each app keeps its own markup and styling;
|
||
only the logic changed, and it is identical in both.
|
||
- The four account screens now call `getMe()` / `getMyOrders()` /
|
||
`getMyLedgerEntries()`; router guards test `isLoggedIn`.
|
||
|
||
### Tests
|
||
|
||
- `apps/api/scripts/auth-smoke.sh` extended with the stage-2 assertions:
|
||
**46 passed, 0 failed** against a live server — old routes 404, customer
|
||
routes 401 without a token, wrong/reused code 401, correct code 200, throttle
|
||
429, no enumeration, customer token rejected on admin routes and admin token
|
||
rejected on customer routes, no id-bearing surface, and `/customer/me`
|
||
returning the token's own customer.
|
||
- **jest installed** (dev-only, user-approved) with `jest.config.cjs`, so
|
||
`pnpm test` works for the first time: **7 passed** covering `faToEn`.
|
||
`faToEn` moved to its own `digits.ts` so it can be tested without booting Nest.
|
||
- Typecheck clean on api, user-app and user-app-v2; both apps build.
|
||
|
||
---
|
||
|
||
## 2026-09-22 — Security hotfix, stage 3 — SEC-02 (orders) + OTP audit
|
||
|
||
Branch `parity/security-hotfix`. The user declined to defer SEC-02 to Phase 2:
|
||
a public `POST /orders` taking `customerId` from the body let anyone place an
|
||
order on any customer's account, and an order lands on that customer's **gold
|
||
debt and ledger**. That is a money hole, so it is closed here.
|
||
|
||
### 1. `POST /orders` is customer-authenticated
|
||
|
||
- **What:** the route is behind `@CustomerAuth()`. `OrdersService.createOrder`
|
||
now takes `customerId` as a **separate argument**, supplied by the controller
|
||
from the token. The body's `customerId` is ignored entirely; a mismatch is
|
||
logged (`OrdersService` warn) and the token always wins.
|
||
- `CreateOrderDto.customerId` and the shared `CreateOrderDto` are **optional**,
|
||
so both apps keep validating while they still send it. Removed in Phase 2.
|
||
- **Reference:** `ApiMobileController::setOrder` read `auth()->id()`
|
||
(henza_server:533) behind the `api-customer` guard — this restores that.
|
||
- **Admin path unaffected:** `AdminOrdersService` places orders *for* a selected
|
||
customer and passes that id explicitly; its route is admin-guarded.
|
||
|
||
### 2. `GET /orders/:id` is scoped to the owner
|
||
|
||
- `getOwnedById(orderId, customerId)` matches on both, and a miss is **404, not
|
||
403** — a 403 would confirm the id exists, the same enumeration leak the old
|
||
`/customers/:id` routes had. A nonexistent id and someone else's id are
|
||
indistinguishable.
|
||
- Operators use the new `GET /orders/admin/:id` behind rid `order.list`.
|
||
Declared **before** `:id` because Nest matches in declaration order.
|
||
|
||
### 3. Both apps
|
||
|
||
- `CartView.vue` in **both** apps now gates on `session.isLoggedIn` (the token)
|
||
rather than on a remembered `customerId`, which is no longer a credential.
|
||
|
||
### 4. OTP audit (user request 3)
|
||
|
||
- **No master code, no bypass, no hardcoded code** anywhere in the API or either
|
||
app — verified by grep for `master.?code|bypass|backdoor|1111|0000|1234|skipOtp|DEV_OTP|TEST_OTP`.
|
||
Exactly one code path issues a token, and only after `bcrypt.compare`.
|
||
- **SMS provider: none.** No gateway is wired up (STUB-02). In development the
|
||
code is logged once via `Logger`, guarded by `NODE_ENV !== 'production'`.
|
||
- **Fixed while auditing:** `deliver()` persisted the real message text — with
|
||
the code in it — into `SmsMessage.body`, which the admin SMS grid both lists
|
||
and full-text searches (`admin-misc.service.ts:135-136`). Any operator with
|
||
the `sms` rid could read live OTP codes, defeating the point of hashing
|
||
`codeHash`. The persisted row is now a redacted audit line; the real text is
|
||
built for the future gateway and not stored. The 9 test rows created during
|
||
this session were deleted.
|
||
|
||
### Tests
|
||
|
||
`apps/api/scripts/auth-smoke.sh` extended again — **53 passed, 0 failed**:
|
||
order routes 401 without a token, a spoofed body `customerId` ignored in favour
|
||
of the token, own order readable, unknown/foreign order id 404, and the admin
|
||
order route rejecting a customer token while admitting an admin one.
|
||
Typecheck clean on api, user-app, user-app-v2 and shared-types.
|