feat(F-154): completed feature

This commit is contained in:
chattie
2026-08-22 08:35:46 +02:00
parent 1415590106
commit 5098723284
19 changed files with 535 additions and 114 deletions

View File

@@ -0,0 +1,54 @@
# F-154 — Architect Evidence
## Feature
F-154 — Admin: separate customers from internal users (fix, high/high).
## Problem
Two admin list endpoints both returned **all** `identity_users`, so the Customers and Users
pages showed storefront customers and backoffice staff mixed together:
- `GET /users` (module `users`, `PgProfileRepository.listCustomers`) → all users, no role filter.
Consumed by the Customers page (`customersApi.list``/api/users`).
- `GET /admin/users` (module `security`) → all users unless `?role=` supplied.
Consumed by the Users page (`adminUsersApi.list``/api/admin/users`).
## Roles (DB `identity_users.role`, `citext NOT NULL DEFAULT 'customer'`)
- `customer` → storefront client.
- `admin` / `editor` / `pos_cashier` / `pos_manager` → internal / backoffice staff.
## Decision
Force the separation in the **backend** (single source of truth), not client-side:
1. `listCustomers` (→ `GET /users`) now always appends `AND iu.role = 'customer'` (code literal,
no user input → no injection). The `?q` search still applies on `email`. Single-user
`findCustomerById` (`/users/:id`) is untouched (owner-or-admin, role-agnostic).
2. `GET /admin/users`: base condition `role <> 'customer'` (literal) so the Users list NEVER
returns storefront customers; `?role=admin|editor` still narrows within internal staff.
`?role=customer` resolves to an empty intersection (still no leak).
3. Frontend: remove the `customer` option from the Users page role dropdown (UX polish — backend
already enforces the boundary). Customers page unchanged (already calls `/users`).
## Alternatives considered
- Client-side filtering only: rejected — backend is the trust boundary; the admin API must
not leak customers through `/admin/users`.
- New `/customers` endpoint: rejected — the frontend already binds Customers↔`/users` and
Users↔`/admin/users`; splitting now duplicates effort with no behavioral gain.
## Boundary / security
- `users` module references `identity_users` only as a SQL table name (existing pattern in
`search`); no TypeScript import crosses the identity↔users↔security boundary.
`lint:boundaries` unaffected.
- No migration (`identity_users.role` already exists, NOT NULL DEFAULT 'customer').
- User input (`q`, `role`) stays parameterized (`$N`); the role constants are code literals.
## Test plan (no DB required → runs in `npm test`)
- `users/infrastructure/pg-profile-repository.test.ts`: mock `pg.Pool`, assert `listCustomers`
emits `iu.role = 'customer'`, respects `q`, count === select filter, returns only customer rows.
- `security/api/security.routes.test.ts`: register routes on a mock Fastify with a mock pool +
mocked `authenticate` returning `admin`; assert `GET /admin/users` queries `role <> 'customer'`
and that `?role=admin` adds `AND role = $1`; response excludes `customer` rows.
- `app/tests/users.itest.ts` AC2/AC3: update assertion — `/users` returns the customer (ben),
not the promoted admin (ana).
## Risk
Medium-high: changes admin list semantics (`/users` now customer-only). Mitigations:
`/users/:id` (single) unchanged; only the LIST contract changes; existing itest updated;
no migration; boundary/lint verified.

View File

@@ -0,0 +1,21 @@
# F-154 — Documentation
## Summary
Separated storefront customers from internal/backoffice users in the admin panel.
## API behavior change
- `GET /users` (admin): now returns **storefront customers only** (`identity_users.role = 'customer'`).
Previously returned all users. Used by the Customers page (`/api/users`).
- `GET /admin/users` (admin): now returns **internal/backoffice users only** (`role <> 'customer'`),
narrowed by optional `?role=admin|editor`. Previously returned all users when no `?role=`.
`?role=customer` returns an empty list (never leaks customers). Used by the Users page (`/api/admin/users`).
- No change to `GET /users/:id`, `PATCH /users/:id`, `/users/:id/addresses*`, or `/admin/users/:id`.
Response shapes unchanged (`CustomerSummary` and admin user list both include `id, email, role, createdAt`).
## Frontend
- Users page: removed the "Customer" option from the role filter dropdown (backend already
enforces internal-only; the page lists `/admin/users` staff). Customers page unchanged.
## Notes
- No migration (`identity_users.role` already exists).
- The role filter is a code constant (SQL literal), not user input — no injection surface.

View File

@@ -0,0 +1,44 @@
# F-154 — Implementer Evidence
## Feature
F-154 — Admin: separate customers from internal users (fix, high/high).
## Changes (all under project/)
### Backend
1. `users/infrastructure/pg-profile-repository.ts` (`listCustomers``GET /users`):
Added literal `iu.role = 'customer' AND` to both the COUNT and SELECT WHERE clauses.
Parameter indices unchanged (`$1` = email search, `$2` = limit, `$3` = offset) since the
role filter is a code constant, not a bound parameter (no injection). `findCustomerById`
(`/users/:id`) is untouched (owner-or-admin, role-agnostic).
2. `security/api/security.routes.ts` (`GET /admin/users`):
Base condition `["role <> 'customer'"]` so the Users list NEVER returns storefront
customers. `?role=admin|editor` still narrows within internal staff via `role = $N`.
`?role=customer` resolves to an empty intersection (no leak). `where` is always present.
### Frontend (apps/admin)
- `(dashboard)/users/page.tsx`: removed the `customer` option from the Users role filter
dropdown. The Customers page is unchanged (already calls `/users`, now customer-only).
### Tests
- `users/infrastructure/pg-profile-repository.test.ts` (NEW, 3 tests): mock pool —
`listCustomers` emits `iu.role = 'customer'` on SELECT+COUNT; `q` stays parametrized as `$1`.
- `security/api/security.routes.test.ts` (NEW, 3 tests): mock Fastify + mock deps —
`/admin/users` default → `role <> 'customer'`; `?role=admin` → adds `role = $1` with param;
`?role=customer` → empty (intersection), never returns customers.
- `app/tests/users.itest.ts` AC2/AC3: flipped — `/users` returns the customer (ben), not the
promoted admin (ana). (DB itest, skipped without TEST_DATABASE_URL.)
## Verification
- `npx vitest run <2 new files>` → 6/6 pass (re-run fresh for this session).
- `npm test` → 206 passed / 56 skipped (itests), 0 failures, no regressions.
- `npm run typecheck` → 0 errors.
- `cd project/apps/admin && npx tsc --noEmit` → 0 errors.
- `npm run lint` → no errors/warnings in files touched.
- `npm run lint:boundaries` → no NEW violation (R1 on security.routes.ts → log-broadcaster
is pre-existing: git diff shows the import is untouched by F-154).
- `./scripts/verify.sh` → exit 0.
## Risk
Medium-high: list semantics change. Mitigated by: single-user `/users/:id` unchanged,
no migration (`identity_users.role` already exists), tsc + lint + tests green, itest updated.

View File

@@ -0,0 +1,13 @@
{
"feature_id": "F-154",
"agent": "leader",
"stage": "close",
"verdict": "APPROVED",
"summary": "F-154 completed: Customers (/users) now customer-only and Users (/admin/users) internal-only — backend-enforced role separation, frontend dropdown cleaned. 6 new unit tests + itest flip; full suite green; tsc/lint/verifysh clean.",
"checks": [
{"item": "Gates approved", "ok": true, "evidence": "reviewer.json, security.json, qa.json -> APPROVED"},
{"item": "verify.sh", "ok": true, "evidence": "exit 0"},
{"item": "Artifacts present", "ok": true, "evidence": "architect.md, implementer.md, reviewer.json, security.json, qa.json, documenter.md, leader-close.json"}
],
"issues": []
}

View File

@@ -0,0 +1,15 @@
{
"feature_id": "F-154",
"agent": "qa",
"stage": "qa_gate",
"verdict": "APPROVED",
"summary": "6 new unit tests (mock, no DB) + itest AC2/AC3 updated. Full suite green, 0 regressions.",
"checks": [
{"item": "AC1 /users customer-only", "ok": true, "evidence": "pg-profile-repository.test.ts: listCustomers SELECT+COUNT contain role = 'customer'; returns customer row"},
{"item": "AC2 /admin/users internal-only", "ok": true, "evidence": "security.routes.test.ts: default excludes customers; ?role=admin narrows; ?role=customer empty"},
{"item": "No regression (full suite)", "ok": true, "evidence": "npm test = 206 passed / 56 skipped (DB itests), 0 failures"},
{"item": "Types/lint green", "ok": true, "evidence": "tsc --noEmit 0 errors (API + apps/admin); eslint/prettier clean on touched files; lint:boundaries no new violations"},
{"item": "verify.sh", "ok": true, "evidence": "exit 0"}
],
"issues": []
}

View File

@@ -0,0 +1,15 @@
{
"feature_id": "F-154",
"agent": "reviewer",
"stage": "review_gate",
"verdict": "APPROVED",
"summary": "Separa customers de internos en backend (listCustomers role=customer, /admin/users role<>customer) + frontend dropdown. AC cubiertas por tests unitarios mock-pool/mock-Fastify y itest AC2/AC3 actualizado.",
"checks": [
{"item": "AC1 /users customer-only", "ok": true, "evidence": "pg-profile-repository.ts listCustomers: WHERE iu.role = 'customer' (COUNT+SELECT); pg-profile-repository.test.ts assertion"},
{"item": "AC2 /admin/users internal-only", "ok": true, "evidence": "security.routes.ts base condition role <> customer; security.routes.test.ts default+narrowing+empty-customer assertions"},
{"item": "AC3 single user / addresses unchanged", "ok": true, "evidence": "findCustomerById (/users/:id) untouched; route keeps owner-or-admin; git diff scope"},
{"item": "No new boundary violation", "ok": true, "evidence": "git diff: touched files are users/infra + security/api + apps/admin/(dashboard)/users + 2 tests + itest; only SQL table-name ref identity_users (preexistent pattern)"},
{"item": "Spec coverage", "ok": true, "evidence": "spec/{product,tech,acceptance}.md + work/artifacts/F-154/architect.md"}
],
"issues": []
}

View File

@@ -0,0 +1,14 @@
{
"feature_id": "F-154",
"agent": "security",
"stage": "security_gate",
"verdict": "APPROVED",
"summary": "Backend-only role filtering with code-constant literals (role = 'customer' / role <> 'customer'); user input (q, role) stays parameterized. No new routes, RBAC, or auth changes. Single-user endpoints unchanged.",
"checks": [
{"item": "No new routes/RBAC/auth", "ok": true, "evidence": "GET /users and GET /admin/users signatures unchanged; requireRole(authenticate) intact; only WHERE clause literal added"},
{"item": "Injection safety", "ok": true, "evidence": "role compared to code constant (not user input); q and ?role parameterized as $N; param indices unchanged"},
{"item": "IDOR", "ok": true, "evidence": "single-user GET /users/:id unchanged (owner-or-admin)"},
{"item": "Pre-existing boundary note", "ok": true, "evidence": "git diff excludes log-broadcaster import in security.routes.ts; R1 is pre-existing, not introduced by F-154"}
],
"issues": []
}