Files
tehran_taj/docs/parity/DECISIONS.md
T
s.esmaeiliandClaude Opus 5 004a3945c2 parity(docs): record user decisions D0-D12 and urgent investigation results
Phase 0 gate passed. All rulings recorded in docs/parity/DECISIONS.md and
applied to PROTECTED.md, PROGRESS.md and ISSUES.md. No source code changed.

Urgent investigations requested by the user, all evidence-cited:

- percent trace: the target's implementation is WRONG. `percent` is the
  customer's OWN column in the reference (customer_add.php:32), a per-customer
  making-charge offset used to shift catalogue filter bounds
  (ApiMobileController.php:391-396,76-77). The target reads the referring
  agent's wagePercent instead (admin-customers.service.ts:245) and never
  writes the field at all. It is NOT part of the price formula -- that extra
  percent comes from a global ConfigStatic row (ProductVariant.php:50,183).
  Two unrelated `percent` keys exist and must not be confused with it.

- @Public audit: 15 routes across 7 controllers, grouped by risk. Group A is
  5 ledger routes (4 writes) that contradict decision D1; group B is 3 order
  routes incl. a public status change accepting a client-supplied
  actorAdminUserId; group C is 8 reads, of which only the catalogue, pricing,
  slider and uploads are legitimately public.

- Login leak: it is in the TARGET. GET /customers is public and returns every
  active customer with national ID numbers, debts and ID-document URLs.
  passwordHash is correctly excluded. Not possible in the reference.

Also logged 5 customer-module defects found in the deep check the user asked
for (CUST-01..05), including the fullName collapse the user reported.

Skill updated per D12: Step C gains rule C0 (field-by-field matrix, never
page-by-page) and Step E gains rule E0 (mandatory 4-step round-trip test).

[ref: henza_server, henza_admin]

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
2026-09-21 23:26:35 +03:30

363 lines
17 KiB
Markdown
Raw Blame History

This file contains ambiguous Unicode characters
This file contains Unicode characters that might be confused with other characters. If you think that this is intentional, you can safely ignore this warning. Use the Escape button to reveal them.
# DECISIONS — user rulings on the parity project
Binding decisions made by the user. They override defaults and survive context
loss. Referenced from `PROTECTED.md` and `PROGRESS.md`.
| Date | Phase | Decisions |
|---|---|---|
| 2026-09-21 | Phase 0 gate | D0–D5, D6–D8 (urgent), D9–D11, D12 (method change) |
---
## D0 — First name / last name: split the column (option **الف**)
**Ruling.** Add real `firstName` / `lastName` columns to `Customer` via an
**additive** migration.
Requirements:
1. Additive migration only — nothing dropped.
2. Backfill existing rows: **first token** of `fullName` → `firstName`, **the
rest** → `lastName`.
3. Rows whose `fullName` has **more than two tokens** must be written to a report
for manual review (they cannot be split safely).
4. **`fullName` is NOT removed.** It stays and is kept in sync as
`firstName + " " + lastName` so existing code keeps working.
5. `toRow()` must return the **real** `fname` / `lname`, not `fullName` / `''`.
6. Fix every consumer: `singleCustomer.vue`, `bulkCustomer.vue`,
`ReagentCustomer.vue`, and anywhere a name is displayed, searched or sorted.
**Remove the `split(' ')` fallback in `singleCustomer.vue`.**
7. Check the same defect on **`AdminUser`** (sales expert) and on
**registration / profile in `user-app` and `user-app-v2`**.
8. **Show the migration file and the command, and get approval, before running it.**
Evidence for the defect: reference has two columns
(`henza_server/database/migrations/2020_10_01_120430_customers_table.php:18-19`);
target collapses them (`apps/api/src/modules/admin-customers/admin-customers.service.ts:231-233`
returns `fname: c.fullName, lname: ''`).
## D1 — Admin auth: keep the target stricter
The reference leaves all ~215 admin routes unauthenticated
(`henza_server/routes/admin.php:12,272`). The target's JWT + 144-rid enforcement
**stands**, recorded as an accepted deviation.
**Additional requirement:** build a complete mapping matrix
**"every reference admin operation ↔ its permission key (rid) in the target"**,
so no operation is left without an equivalent permission. Explain the
**172 (reference) vs 174 (target) unique key** difference.
## D2 — Domain scope: audit only what the reference has
Confirmed. Do not manufacture rows for buy/sell gold, deposit, withdraw,
transfer, KYC levels, price sockets or payment callbacks — none exist in the
reference (`00-INVENTORY.md` §0).
Target-only capabilities (e.g. the dual-currency ledger, gram/rial debt) are
listed with status **➕** and **left untouched**. Ask when unsure.
## D3 — The 5 missing user-app screens: build them in Phase 2
`/report`, `/report/detail`, `/ChildOrder`, `/customerChoice`, `/complete_profile`
are built in **both** apps, each in its own design language, plus whatever backend
they need.
Also brought to reference behaviour in Phase 2:
- route analytics (`POST /log/create/{key}`, `GET /log/update/{id}`)
- the `isLoaded` guard
- the `sts_verify_step_two` guard on `completeProfile`
- guest behaviour: **toast «ابتدا وارد شوید» + redirect to `home`** (not a redirect to `/login`)
- **v2's missing cart stock ceiling (`capQty`/`maxQty`) must match v1 and the reference**
## D4 — Reference bugs: replicate behaviour, not breakage
- Broken or destructive reference behaviour is **not** ported — logged only.
- **Business behaviour, calculations, rounding and messages must match the
reference exactly.**
- Every deliberate non-replication gets a matrix note:
**«عمداً تکرار نشد — ISSUE #n»**.
- The price formula is the **live** version at
`henza_server/app/Models/ProductVariant.php:183-190` — *not* the commented-out
ones above it.
- Write a **unit test** for it with sample numbers and compare against the
reference formula computed by hand.
## D5 — Stray root folder: leave it for now
`gold-banking-monorepo/user-app-v2/` is logged in `ISSUES.md` (DEAD-03). At the
very end, after verifying nothing references it, delete it in **its own commit**.
---
## D6 — URGENT: OTP must be verified (exception to "log only", per `CLAUDE.md` §3)
The reference **does** verify the OTP, so the target accepting any code is a
**parity gap**, not just a security issue. Fix it to match the reference:
code matching, expiry, and attempt limits.
Reference implementation (`henza_server/app/Http/Controllers/Api/ApiMobileController.php:39-78`):
| Step | Reference behaviour | Line |
|---|---|---|
| Send | `userAuth` validates `username`, finds an **active** customer by mobile, generates `rand(1000,9999)`, stores it in `customer.send_password`, sends it by SMS | `:39-49` |
| Verify | `userVerfiy` validates `username` + `password`; if `faTOen(password) != customer.send_password` → **HTTP 422** | `:51-57` |
| Token | issues `api_token = Str::random(60)` only if the customer doesn't already have one | `:60-62` |
| Digits | the code is normalised through `Utility::faTOen()` (Persian → English digits) on **both** send and verify | `:42,56` |
⚠️ Note for the matrix: the reference has **no expiry and no attempt limit** —
`send_password` is a plain column that stays valid indefinitely. D6 asks for
expiry and attempt limits, which go **beyond** the reference. Recorded as a
deliberate, user-requested improvement; the parity row is the code match itself.
## D7 — URGENT: `@Public()` routes that write to the ledger
Show the exact list with file paths and what the reference did for each, then
protect them **after user approval**. See §"@Public audit" below.
## D8 — URGENT: the login customer-list leak
Identify whether it is reference or target, and what data leaks. See §"Login leak"
below.
---
## D9 — `percent`: trace before acting
Done — see `00-INVENTORY.md` and the trace summary delivered to the user.
**Key finding: the target's implementation is wrong.** `percent` is a real column
on the customer in the reference, not a derived value from the referring agent.
## D10 — Sales expert: the 5 missing fields
The move to `AdminUser` is **accepted**. For `ncode`, `phone`, `city_id`,
`gram_level`, `reagent_id`: find where the reference uses each for `type=1`
(list, filter, report, order, calculation). Fields that are genuinely used must
be added — **proposed shape: a `SalesExpertProfile` table related to `AdminUser`**.
Fields with no use are listed for the user to decide.
## D11 — `singleCustomer.vue`: migrate to the diff-send pattern
Bring it in line with `bulkCustomer.vue` and `ReagentCustomer.vue`. **Diff-send
is recorded as an accepted deviation** from the reference (which sends every
field on every save).
---
## D12 — METHOD CHANGE for Phase 1 (and Phase 2)
**1. The matrix is field-by-field, not page-by-page.** Every form field (name,
type, required, validation, message, default), every table column, every filter,
every button — and for each one, the **client field name ↔ API field name ↔ DB
column**, on **both** sides.
**2. A round-trip test is mandatory per entity:**
```
create with every field populated
→ open the edit form, assert every field is correctly populated
→ save with no changes, assert the DB row is byte-identical
→ change exactly one field, assert only that field changed
```
Run it for the sales expert too.
**3. Both rules are to be added to the `reference-parity-audit` skill**
(Step C and Step E) so Phase 2 inherits them.
---
# Urgent investigation results
## `percent` — full trace (D9)
### In the reference, `percent` is a real column on the customer
| Fact | Evidence |
|---|---|
| Column exists on `customers` | `database/migrations/2020_10_19_153228_customer_add.php:32` — `$table->double('percent')->default(0);` |
| Mass-assignable | `app/Models/Customer.php:31` |
| Defaulted to **3** for app-registered customers | `app/Http/Controllers/Adm/AdminController.php:338` — `$input['percent'] = 3;` |
| **Required** when an admin creates a wholesale customer | `AdminController.php:354` — `'percent'=>'required'` |
| Selected into the wholesale customer list | `Adm/MajorCustomerController.php:36` |
| Shown in a report column «درصد» | `Adm/ReportController.php:137` |
### What it actually does — it shifts the customer's making-charge filter
`app/Http/Controllers/Api/ApiMobileController.php:391-396`:
```php
if (sizeof($limit_percent) > 0 and auth('api-customer')->check()) {
$limit_percent[0] = $limit_percent[0] - auth('api-customer')->user()->percent;
$limit_percent[1] = $limit_percent[1] - auth('api-customer')->user()->percent;
$product->whereHas('variants', fn($q) => $q->whereBetween('percentage_pay', $limit_percent));
}
```
And it shifts the **filter bounds the app is told to display**, at login
(`ApiMobileController.php:76-77`):
```php
$customer['filterPercentPay'][] = strval(ProductVariant::min('percentage_pay') + $customer['percent']);
$customer['filterPercentPay'][] = strval(ProductVariant::max('percentage_pay') + $customer['percent']);
```
So `customer.percent` is a **per-customer making-charge offset**: the UI shows
`percentage_pay + percent`, and the filter subtracts it again before querying.
### What it does NOT do
**It is not part of the price formula.** The formula's extra percent comes from a
**global** config row, not from the customer:
- `app/Models/ProductVariant.php:50` — `ConfigStatic::where('key','product_price_extra_percent')->first()['val'] ?? 7`
- `app/Models/ProductVariant.php:183` — `($this->percentage_pay + static::$product_price_extra_percent)`
There is a **separate, unrelated** `ConfigStatic` key also called `percent`, used
for the cashback/wallet rule (`AdminController.php:903,994`; `Controller.php:203`) —
`value = total_weight * (percent/100)`. **Do not confuse the two.**
A **third** unrelated `percent` is a per-order-item field in wholesale orders
(`Adm/MajorOrderController.php:220`): `w = rash_weight + rash_weight * (percent/100)`.
That one comes from the request, not from the customer.
### The target is wrong
`apps/api/src/modules/admin-customers/admin-customers.service.ts:245`:
```ts
// legacy `percent` came from the referring agent's commission rate
percent: c.referredByAgent?.wagePercent ?? null,
```
That comment is **incorrect** — the reference reads `customer.percent`, its own
column. Consequences:
1. **Read is wrong** — it shows the agent's commission, not the customer's offset.
2. **Write is missing** — `update()` never handles `data.percent`
(`admin-customers.service.ts:600-658`), so editing the field in the admin form
does nothing.
3. **The create default is missing** — the reference sets `percent = 3` for
app-registered customers and **requires** it for admin-created wholesale
customers.
4. **The app-side effect is missing** — no `filterPercentPay` offset anywhere in
`user-app` / `user-app-v2`.
### Proposed reconstruction (needs user approval — it is a schema change)
1. Add `makingChargeOffsetPercent Decimal @default(0)` to `Customer` (additive
migration). Name it clearly so it is never confused with the two other
`percent`s; map it to the legacy key `percent` in `toRow()`.
2. `toRow()` returns that column, not `referredByAgent.wagePercent`. Remove the
incorrect comment.
3. `create()`: default **3** on the app-registration path; **require** it on the
admin wholesale path (reference `AdminController.php:354`).
4. `update()`: handle `data.percent`.
5. Catalogue filter: subtract the customer's offset from the `percentage_pay`
bounds before querying, and return shifted `filterPercentPay` bounds at login
— matching `ApiMobileController.php:391-396,76-77`.
6. The referring agent's `wagePercent` stays where it belongs (the agent), and is
**not** exposed through the customer form.
---
## `@Public()` audit (D7) — every unauthenticated route in the target
15 routes across 7 controllers. Verified by reading each controller.
### 🔴 Group A — ledger writes (the ones the user flagged)
`apps/api/src/modules/accounting/accounting.controller.ts` — `@Public()` on the
**whole controller** (`:7`), prefix `accounting` (`:8`):
| # | Route | Line | Effect | Reference equivalent |
|---|---|---|---|---|
| A1 | `GET accounting/customers/:customerId/balance` | `:12` | reads any customer's balance | `Adm/WalletController` — reference had auth **commented out** too (`routes/admin.php:12`) |
| A2 | `POST accounting/customers/:customerId/reconcile` | `:17` | **writes** — reconciles a balance | same: unauthenticated in reference |
| A3 | `POST accounting/repayments/gold` | `:22` | **writes** — records a gold repayment | same |
| A4 | `POST accounting/repayments/rial` | `:27` | **writes** — records a rial repayment | same |
| A5 | `POST accounting/entries/:entryId/confirm` | `:32` | **writes** — confirms a ledger entry | same |
**In the reference these were admin routes in `routes/admin.php`, and that whole
file's `auth:api` wrapper is commented out — so they were unauthenticated there
too.** The target faithfully reproduced that. Under **D1** the target is supposed
to be stricter, so these five are the clearest violation of D1 still standing.
**Recommendation:** A1–A5 are admin operations. Put them behind the admin JWT
guard with the matching rid (the wallet/document rids from `acl.constants.ts`).
**Awaiting your approval — this is the list you asked for.**
### 🟠 Group B — order writes
`apps/api/src/modules/orders/orders.controller.ts` — `@Public()` at `:7`, prefix `orders`:
| # | Route | Line | Note | Reference |
|---|---|---|---|---|
| B1 | `POST orders` | `:12` | customer places an order | `ApiMobileController::setOrder` — **was authenticated** via `auth('api-customer')`; it reads `auth()->id()` at `:533` |
| B2 | `GET orders/:id` | `:17` | reads any order by id | same guard in reference |
| B3 | `PATCH orders/:id/status` | `:22` | **changes order status**; DTO accepts a client-supplied `actorAdminUserId` | admin operation in reference |
**B1/B2 differ from the reference in kind:** the reference's customer endpoints
sat behind the `api-customer` guard and derived the customer from the token.
The target has **no customer session at all**, so it takes `customerId` from the
request body. That is why they are public. Fixing this properly means building
customer auth — which is exactly what **D6** starts.
**B3 is the worst of the three:** it lets an anonymous caller move an order's
status *and* attribute the action to any admin.
### 🟢 Group C — genuinely public reads (match the reference's public storefront)
| # | Route | File:line | Reference |
|---|---|---|---|
| C1 | `GET customers` | `customers/customers.controller.ts:10` | **see the leak below** |
| C2 | `GET customers/:id` | `:15` | — |
| C3 | `GET customers/:id/orders` | `:20` | — |
| C4 | `GET customers/:id/ledger-entries` | `:25` | — |
| C5 | `GET catalog/products` | `catalog/catalog.controller.ts:10` | reference storefront was public (`routes/site.php`) ✅ |
| C6 | `GET pricing/latest` | `pricing/pricing.controller.ts:10` | reference `/price` was public ✅ |
| C7 | `GET home/slider` | `admin-misc/public-slider.controller.ts:21` | reference `mobile_main_slider` config, public ✅ |
| C8 | `GET uploads/:filename` | `admin-uploads/public-uploads.controller.ts:17` | reference served uploads statically ✅ |
C5–C8 are legitimately public. **C1–C4 are not** — they expose customer records.
---
## Login leak (D8) — it is in the **target**, and it is worse than "a list"
**Where:** target only. `apps/user-app` and `apps/user-app-v2` both call
`GET /api/customers` at login and match the phone number **in the browser**.
**What leaks:** `GET /customers` is `@Public()`
(`customers/customers.controller.ts:10`) and returns **every active,
non-archived customer** with these fields
(`customers/customers.service.ts:6-23`, `:44-50`):
| Field | Sensitive? |
|---|---|
| `id`, `createdAt`, `updatedAt` | low |
| `fullName` | 🔴 **PII** |
| `mobile` | 🔴 **PII** |
| `nationalCode` | 🔴 **national ID number** |
| `cityId`, `referredByAgentId` | medium |
| `goldCreditLimitGrams`, `goldDebtGrams`, `fiatDebtRial` | 🔴 **financial position of every customer** |
| `verificationStep`, `isActive`, `isArchived` | medium |
| `nationalIdImageUrl`, `businessLicenseUrl`, `shopImageUrl` | 🔴 **URLs of ID-document scans** |
**`passwordHash` is correctly excluded** — the select list is explicit and the
source comment says so (`customers.service.ts:5`). So no password material leaks.
**Everything else does.** Any anonymous visitor can fetch the full customer book —
names, mobiles, national ID numbers, debts, and links to identity-document images
(and `GET uploads/:filename` is public too, so those links resolve).
**In the reference this was not possible.** Login was
`POST userAuth` → SMS code → `POST userVerfiy`
(`ApiMobileController.php:39-78`): the server looked the customer up by mobile
and returned **only that one customer** after the code matched. There was no
"download everyone and match on the client" step.
**Root cause:** the target has no customer authentication, so the client does the
lookup. **D6 (real OTP) is the fix for both problems at once** — once
`POST auth/otp/verify` returns the authenticated customer, `GET /customers` can
be taken off the public surface entirely.