From 5098723284d34b1e6796261cec1310573998699f Mon Sep 17 00:00:00 2001 From: chattie Date: Sat, 22 Aug 2026 08:35:46 +0200 Subject: [PATCH] feat(F-154): completed feature --- backlog/features.json | 12 +- .../admin/src/app/(dashboard)/users/page.tsx | 2 +- project/src/app/tests/users.itest.ts | 6 +- .../security/api/security.routes.test.ts | 110 ++++++++++++++++++ .../modules/security/api/security.routes.ts | 6 +- .../pg-profile-repository.test.ts | 66 +++++++++++ .../infrastructure/pg-profile-repository.ts | 7 +- spec/acceptance.md | 52 ++++----- spec/product.md | 45 ++++--- spec/tech.md | 86 +++++++------- work/artifacts/F-154/architect.md | 54 +++++++++ work/artifacts/F-154/documenter.md | 21 ++++ work/artifacts/F-154/implementer.md | 44 +++++++ work/artifacts/F-154/leader-close.json | 13 +++ work/artifacts/F-154/qa.json | 15 +++ work/artifacts/F-154/reviewer.json | 15 +++ work/artifacts/F-154/security.json | 14 +++ work/current.md | 10 ++ work/runtime-status.json | 71 +++++++++-- 19 files changed, 535 insertions(+), 114 deletions(-) create mode 100644 project/src/modules/security/api/security.routes.test.ts create mode 100644 project/src/modules/users/infrastructure/pg-profile-repository.test.ts create mode 100644 work/artifacts/F-154/architect.md create mode 100644 work/artifacts/F-154/documenter.md create mode 100644 work/artifacts/F-154/implementer.md create mode 100644 work/artifacts/F-154/leader-close.json create mode 100644 work/artifacts/F-154/qa.json create mode 100644 work/artifacts/F-154/reviewer.json create mode 100644 work/artifacts/F-154/security.json diff --git a/backlog/features.json b/backlog/features.json index 26680f8..bf1fbac 100644 --- a/backlog/features.json +++ b/backlog/features.json @@ -6425,13 +6425,15 @@ "description": "Fix admin Users and Customers modules so Customers shows only storefront customers and Users shows only internal/backoffice users, avoiding mixed identity/backoffice concepts.", "priority": "high", "risk": "high", - "status": "pending", + "status": "done", "created_at": "2026-08-21", "gates": { - "reviewer": false, - "security": false, - "qa": false - } + "reviewer": true, + "security": true, + "qa": true, + "close": true + }, + "completed_at": "2026-08-22T06:35:46Z" }, { "id": "F-155", diff --git a/project/apps/admin/src/app/(dashboard)/users/page.tsx b/project/apps/admin/src/app/(dashboard)/users/page.tsx index 3702753..fb9e4b5 100644 --- a/project/apps/admin/src/app/(dashboard)/users/page.tsx +++ b/project/apps/admin/src/app/(dashboard)/users/page.tsx @@ -145,7 +145,7 @@ export default function AdminUsersPage() { diff --git a/project/src/app/tests/users.itest.ts b/project/src/app/tests/users.itest.ts index d665750..b96077f 100644 --- a/project/src/app/tests/users.itest.ts +++ b/project/src/app/tests/users.itest.ts @@ -159,7 +159,11 @@ describe.skipIf(!hasDb)('users + RBAC flows (real PostgreSQL)', () => { }); expect(asAdmin.statusCode).toBe(200); const body = asAdmin.json() as { items: Array<{ id: string }> }; - expect(body.items.some((item) => item.id === anaId)).toBe(true); + // F-154: /users now serves storefront customers only (role 'customer'). + // ana was promoted to 'admin' -> no longer a customer -> excluded from the list. + // ben is a storefront customer -> included. + expect(body.items.some((item) => item.id === benId)).toBe(true); + expect(body.items.some((item) => item.id === anaId)).toBe(false); // Admin can also read another user's profile (owner-or-admin). const adminReadsBen = await app.inject({ diff --git a/project/src/modules/security/api/security.routes.test.ts b/project/src/modules/security/api/security.routes.test.ts new file mode 100644 index 0000000..c65a606 --- /dev/null +++ b/project/src/modules/security/api/security.routes.test.ts @@ -0,0 +1,110 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; +import Fastify, { type FastifyInstance } from 'fastify'; +import { registerSecurityRoutes } from './security.routes.js'; +import type { SecurityRoutesDeps } from '../index.js'; + +interface QueryResult { + rows: unknown[]; +} + +/** F-154: mock pg.Pool returning configurable rows keyed by SQL family. */ +function mockPool(countRows: unknown[], selectRows: unknown[]) { + const query = vi.fn(async (sql: string, _params: unknown[]): Promise => { + if (String(sql).includes('COUNT')) return { rows: countRows }; + return { rows: selectRows }; + }); + const pool = { query } as unknown as import('pg').Pool; + return { pool, query }; +} + +const created: FastifyInstance[] = []; + +/** Builds a minimal Fastify app with security routes + mocked deps (no DB). */ +async function buildAdminApp(pool: unknown) { + const app = Fastify(); + created.push(app); + const deps = { + pool, + authenticate: vi.fn().mockResolvedValue({ id: 'a1', role: 'admin' }), + rateLimiter: vi.fn(), + auditLogger: vi.fn(), + } as unknown as SecurityRoutesDeps; + await registerSecurityRoutes(app, deps); + await app.ready(); + const query = (pool as { query: ReturnType }).query; + return { app, query }; +} + +const ADMIN_USER = { + id: 'a1', + email: 'ana@example.com', + role: 'admin', + created_at: new Date('2026-01-01T00:00:00Z'), +}; +const EDITOR_USER = { + id: 'e1', + email: 'e@example.com', + role: 'editor', + created_at: new Date('2026-01-02T00:00:00Z'), +}; + +afterEach(async () => { + for (const app of created) { + try { + await app.close(); + } catch { + // ignore + } + } + created.length = 0; +}); + +describe('GET /admin/users — F-154 internal-only by default', () => { + it('returns only internal/backoffice users and never storefront customers', async () => { + const { app, query } = await buildAdminApp( + mockPool([{ count: '1' }], [ADMIN_USER, EDITOR_USER]), + ); + const res = await app.inject({ method: 'GET', url: '/admin/users' }); + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body ?? '') as { + items: Array<{ id: string; role: string }>; + total: number; + }; + expect(body.items).toHaveLength(2); + expect(body.items.every((u) => u.role !== 'customer')).toBe(true); + expect(body.items.map((u) => u.id)).toEqual(['a1', 'e1']); + expect(body.total).toBe(1); + + const selectSql = query.mock.calls[1]![0] as string; + expect(selectSql).toContain("role <> 'customer'"); + }); + + it('narrows with ?role=admin within staff (parametrized)', async () => { + const { app, query } = await buildAdminApp(mockPool([{ count: '1' }], [ADMIN_USER])); + const res = await app.inject({ method: 'GET', url: '/admin/users?role=admin' }); + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body ?? '') as { items: Array<{ id: string }> }; + expect(body.items).toHaveLength(1); + expect(body.items[0]!.id).toBe('a1'); + + const selectSql = query.mock.calls[1]![0] as string; + expect(selectSql).toContain("role <> 'customer'"); + expect(selectSql).toContain('role = $1'); + expect(query.mock.calls[1]![1]).toContain('admin'); + }); + + it('yields an empty set (never customers) when ?role=customer is requested', async () => { + const { app, query } = await buildAdminApp(mockPool([{ count: '0' }], [])); + const res = await app.inject({ method: 'GET', url: '/admin/users?role=customer' }); + + expect(res.statusCode).toBe(200); + const body = JSON.parse(res.body ?? '') as { items: Array<{ id: string }> }; + expect(body.items).toHaveLength(0); + + const selectSql = query.mock.calls[1]![0] as string; + expect(selectSql).toContain("role <> 'customer'"); + expect(selectSql).toContain('role = $1'); + }); +}); diff --git a/project/src/modules/security/api/security.routes.ts b/project/src/modules/security/api/security.routes.ts index 8ec1bad..dfde472 100644 --- a/project/src/modules/security/api/security.routes.ts +++ b/project/src/modules/security/api/security.routes.ts @@ -189,7 +189,9 @@ export async function registerSecurityRoutes( const admin = await deps.authenticate(request); requireRole(admin, 'admin'); const { limit, offset, role, q } = parseJson(userQuerySchema, request.query ?? {}); - const conditions: string[] = []; + // F-154: Users module == internal/backoffice only (never storefront customers). + // Base literal excludes customers; ?role= then narrows within internal staff. + const conditions: string[] = ["role <> 'customer'"]; const values: unknown[] = []; let i = 1; if (role) { @@ -200,7 +202,7 @@ export async function registerSecurityRoutes( conditions.push(`(email ILIKE $${i++})`); values.push(`%${q}%`); } - const where = conditions.length ? `WHERE ${conditions.join(' AND ')}` : ''; + const where = `WHERE ${conditions.join(' AND ')}`; const countResult = await deps.pool.query<{ count: string }>( `SELECT COUNT(*) FROM identity_users ${where}`, values, diff --git a/project/src/modules/users/infrastructure/pg-profile-repository.test.ts b/project/src/modules/users/infrastructure/pg-profile-repository.test.ts new file mode 100644 index 0000000..cf5401f --- /dev/null +++ b/project/src/modules/users/infrastructure/pg-profile-repository.test.ts @@ -0,0 +1,66 @@ +import { describe, expect, it, vi } from 'vitest'; +import { PgProfileRepository } from './pg-profile-repository.js'; + +interface QueryResult { + rows: unknown[]; +} + +/** F-154: mock pg.Pool returning configurable rows keyed by SQL family. */ +function mockPool(countRows: unknown[], selectRows: unknown[]) { + const query = vi.fn(async (sql: string, _params: unknown[]): Promise => { + if (String(sql).includes('COUNT')) return { rows: countRows }; + return { rows: selectRows }; + }); + const pool = { query } as unknown as import('pg').Pool; + return { pool, query }; +} + +const CUSTOMER_ROW = { + user_id: 'c1', + email: 'ben@example.com', + role: 'customer', + display_name: 'Ben', + phone: '111', + created_at: new Date('2026-01-01T00:00:00Z'), +}; + +describe('PgProfileRepository.listCustomers — F-154 customers-only', () => { + it('always restricts the list query to role = customer', async () => { + const { pool, query } = mockPool([{ cnt: '1' }], [CUSTOMER_ROW]); + const repo = new PgProfileRepository(pool); + + const result = await repo.listCustomers({ offset: 0, limit: 20 }); + + expect(result.total).toBe(1); + expect(result.items).toHaveLength(1); + expect(result.items[0]!.role).toBe('customer'); + expect(result.items[0]!.email).toBe('ben@example.com'); + + const selectSql = query.mock.calls[1]![0] as string; + expect(selectSql).toContain("iu.role = 'customer'"); + }); + + it('applies the customer filter to the count query too', async () => { + const { pool, query } = mockPool([{ cnt: '1' }], [CUSTOMER_ROW]); + const repo = new PgProfileRepository(pool); + + await repo.listCustomers({ offset: 0, limit: 20 }); + + const countSql = query.mock.calls[0]![0] as string; + expect(countSql).toContain('COUNT'); + expect(countSql).toContain("iu.role = 'customer'"); + }); + + it('keeps the email search parameterized alongside the role filter', async () => { + const { pool, query } = mockPool([{ cnt: '1' }], [CUSTOMER_ROW]); + const repo = new PgProfileRepository(pool); + + await repo.listCustomers({ q: 'ben', offset: 0, limit: 20 }); + + const selectSql = query.mock.calls[1]![0] as string; + const params = query.mock.calls[1]![1] as unknown[]; + expect(selectSql).toContain("iu.role = 'customer'"); + expect(selectSql).toContain('ILIKE $1'); + expect(params[0]).toBe('%ben%'); + }); +}); diff --git a/project/src/modules/users/infrastructure/pg-profile-repository.ts b/project/src/modules/users/infrastructure/pg-profile-repository.ts index 7953f0f..094f9e1 100644 --- a/project/src/modules/users/infrastructure/pg-profile-repository.ts +++ b/project/src/modules/users/infrastructure/pg-profile-repository.ts @@ -94,10 +94,13 @@ export class PgProfileRepository implements ProfileRepository { async listCustomers(opts: CustomerListOptions): Promise { const searchFilter = opts.q ? `%${opts.q}%` : null; + // F-154: Customers endpoint serves storefront customers only (role 'customer'). + // role is a code literal (DB column compared to a constant), not user input, + // so it does not affect the parameterized $N index of the email search. const countResult = await this.pool.query<{ cnt: string }>( `SELECT COUNT(*)::text AS cnt FROM identity_users iu - WHERE ($1::text IS NULL OR iu.email ILIKE $1)`, + WHERE iu.role = 'customer' AND ($1::text IS NULL OR iu.email ILIKE $1)`, [searchFilter], ); const total = parseInt(countResult.rows[0]?.cnt ?? '0', 10); @@ -107,7 +110,7 @@ export class PgProfileRepository implements ProfileRepository { up.display_name, up.phone, iu.created_at FROM identity_users iu LEFT JOIN users_profiles up ON up.user_id = iu.id - WHERE ($1::text IS NULL OR iu.email ILIKE $1) + WHERE iu.role = 'customer' AND ($1::text IS NULL OR iu.email ILIKE $1) ORDER BY iu.created_at DESC LIMIT $2 OFFSET $3`, [searchFilter, opts.limit, opts.offset], diff --git a/spec/acceptance.md b/spec/acceptance.md index 36e13f7..6c7321d 100644 --- a/spec/acceptance.md +++ b/spec/acceptance.md @@ -1,34 +1,28 @@ -# F-153 — Acceptance Criteria +# F-154 — Acceptance Criteria -## AC1 — Order detail exposes customer email -`GET /orders/:id` and `GET /orders/:id/admin` responses include a top-level `email` field equal -to the linked `identity_users.email`. When the order has no linked identity_user, `email` is -`null`. +## AC1 — Customers list shows only storefront customers +`GET /users` (admin) devuelve SOLO usuarios con `role = 'customer'`. Un usuario +interno (admin/editor) NO aparece en el listado. El buscador `q` sigue filtrando sobre email +dentro de los clientes. -## AC2 — Admin order list exposes customer email -Every item in `GET /orders` (admin list) includes the `email` field, resolved via the same -read-model association (no N+1 per item beyond the repository's single read). +## AC2 — Users list shows only internal/backoffice users +`GET /admin/users` (admin, default sin `?role=`) devuelve SOLO usuarios con +`role != 'customer'` (admin/editor/pos). Un cliente (`role = 'customer'`) NO aparece. +`?role=admin` y `?role=editor` siguen afinando dentro de internos; `?role=customer` +NO devuelve clientes (devuelve vacío) — la separación está forzada en backend. -## AC3 — Admin force-transition uses the associated email -`POST /orders/:id/transitions/admin` resolves the customer email from the order view -(`order.email`) — it no longer issues a separate inline `SELECT email FROM identity_users`. -When `email` is present, the status notification is sent; when absent, it logs -"El cliente no tiene email asociado" via `request.log.warn` and still completes the transition. +## AC3 — No regression on user profile / addresses +`/users/:id` (GET/PATCH) owner-or-admin sigue devolviendo/editando CUALQUIER usuario +sin filtro por rol (admin ve perfil de cliente; cliente ve el suyo). CRUD de +`/users/:id/addresses` inalterado. -## AC4 — No regression -All existing order flows keep their behavior (create, customer/admin detail, list, edit items, -shipping update, transitions). Only `email` is added to serialization; no new state transitions, -endpoints, or side effects. +## AC4 — No boundary / injection violation +- `identity_users` referenciado solo como tabla SQL (sin import TS). +- Valores `q`/`role` parametrizados; el literal `'customer'`/`'customer'` es constante de código. +- Sin migración. -## AC5 — No migration -`email` is derived from the pre-existing `identity_users.email` column; no schema migration is -required. - -## AC6 — Quality gates -- `tsc --noEmit`: 0 errors. -- `prettier --check` + `eslint`: clean on touched files. -- `lint:boundaries`: no new R1/R2 violations (orders→identity_users is a SQL table-name - reference, same as the existing `search` join; no TS cross-import). -- `vitest run`: full suite green (existing order/payments/checkout/notification tests + new - pg-order-repository test). -- `verify.sh`: exit 0 (backlog F-153 in_progress, runtime stage valid). +## AC5 — Quality gates +- `tsc --noEmit` (API) 0 errores; `npx tsc --noEmit` (apps/admin) sin errores nuevos. +- `npm run lint:boundaries` sin violaciones nuevas. +- `vitest run` (sin DB) → suite nueva F-154 + suite existente en verde. +- `verify.sh` exit 0 (backlog F-154 in_progress, runtime stage válido). diff --git a/spec/product.md b/spec/product.md index a748640..7f3fedd 100644 --- a/spec/product.md +++ b/spec/product.md @@ -1,33 +1,30 @@ -# F-153 — Product Spec +# F-154 — Admin: separate customers from internal users ## Problem -The order read model and its serialization (`serializeOrder`) do **not** expose -the linked customer's email. `orders_orders.user_id` references `identity_users` -(whose `email citext NOT NULL UNIQUE` always exists), but the order view carries -only `userId` — never the email. Consequence: -- Order detail (`/orders/:id`, `/orders/:id/admin`) and the admin order list - (`/orders`) never display the customer email ("customer email missing ... displayed"). -- The admin force-transition (`POST /orders/:id/transitions/admin`) works around - this with a fragile inline `SELECT email FROM identity_users WHERE id = - order.userId`, which surfaces "El cliente no tiene email asociado" whenever the - view itself doesn't carry the association. +El panel admin muestra usuarios mezclados. `GET /users` (módulo `users`) devuelve +TODOS los identity_users (clientes + backoffice) y `GET /admin/users` (módulo `security`) +por defecto también devuelve todos. La página Customers llama a `/api/users` y la página +Users llama a `/api/admin/users`; como ambos devuelven todo, ambos listados aparecen +mezclados (conceptos de identity/storefront con backoffice en un mismo listado). ## Goal -Associate the linked customer's email to the **order read model** and display it in -serialization — detail, admin list, and the admin force-transition notification — -using the order view as the single source of truth. +Customers muestra SOLO clientes storefront (`role = 'customer'`); Users muestra SOLO +usuarios internos/backoffice (`role != 'customer'`). Separación forzada en el backend +(single source of truth), no solo filtrado cliente. ## Scope IN -- `orders/domain`: add `email` to `OrderView` (read model). -- `orders/infrastructure` (pg-order-repository): JOIN `identity_users` to resolve - `email` on every order read (`findById`, `findByIdAndUserId`, `findAll`, `search`). -- `orders/api` (orders.routes): surface `email` in `serializeOrder` and consume - `order.email` in the admin force-transition notification (removing the inline lookup). +- `project/src/modules/users` (`listCustomers` / `GET /users`): filtrar `role = 'customer'`. +- `project/src/modules/security` (`GET /admin/users`): default `role != 'customer'`; + `?role=admin|editor` sigue afinando dentro de internos. +- `project/apps/admin/.../users/page.tsx`: quitar opción `customer` del dropdown (Users = backoffice). +- Tests unitarios (mock pool, sin DB) + actualizar itest AC2/AC3. ## Scope OUT -- No changes to the `identity` domain (no TS cross-import). -- No new tables / migrations: `identity_users.email` already exists and is NOT NULL. -- No new endpoints; no auth/RBAC change; no order state machine change. +- No se crea `/customers` (el cliente ya consume `/users`). +- `/users/:id`, `/users/:id/addresses` (owner-or-admin) siguen sin filtro por rol (un admin + ve el perfil de cualquier usuario; un cliente ve el suyo). +- No migración (identity_users.role ya existe, NOT NULL con default 'customer'). +- Frontend Customer page: sin cambio (ya llama /users → ahora customer-only). -## Risk / Priority -- Priority: high. Risk: med (additive read-model field; backward compatible). +## Type +fix — high priority / high risk. diff --git a/spec/tech.md b/spec/tech.md index a35e6ad..2fc4b54 100644 --- a/spec/tech.md +++ b/spec/tech.md @@ -1,49 +1,49 @@ -# F-153 — Tech Spec +# F-154 — Technical Design -## Principles -- Associate the customer email to the order **read model** (not a per-request hack): - `OrderView.email` is resolved once by the orders repository via a `LEFT JOIN identity_users`. -- Reuse the existing SQL pattern: `orders/infrastructure/pg-order-repository.ts` `search` - already does `LEFT JOIN identity_users u ON u.id = o.user_id` — F-153 extends that to every - order read so the email is always available on the view. -- Boundaries: `identity_users` is referenced only as a **SQL table name** (pre-existing in - `search`); no TypeScript import crosses the identity/orders boundary. `identity` does not - import orders; orders references `identity_users` table name (string) at infrastructure. -- NoUncheckedIndexedAccess is ON → index access returns `T | undefined`; use `!` or `?? null` - when mapping rows. -- Backward compatible: `email` is an additive field on the serialized output; no state - transition, no new migration, no endpoint change. +## Context +- `identity_users` tiene `role: citext NOT NULL DEFAULT 'customer'` (valores: `customer`, + `admin`, `editor`, `pos_cashier`, `pos_manager`). `customer` = storefront; el resto = backoffice. +- `GET /users` (módulo `users`): `PgProfileRepository.listCustomers` hace + `SELECT ... FROM identity_users iu LEFT JOIN users_profiles up ... WHERE ($1::text IS NULL OR iu.email ILIKE $1)`. + Devuelve TODO. Usado por `clientsApi.list` (página Customers) → `/api/users`. +- `GET /admin/users` (módulo `security`): query inline con condiciones opcionales `role` y `q`. + Sin `?role=` devuelve TODO. Usado por `adminUsersApi.list` (página Users) → `/api/admin/users`. +- `listCustomers` se consume SOLO en `users.routes.ts` (`/users`). `findCustomerById` + (single, `/users/:id`) es role-agnostic (owner-or-admin) → no cambia. +- No existe test de `users`/`security` routes; `users.itest.ts` AC2/AC3 asocia al admin (ana) + al listado `/users` (true hoy porque /users devuelve todo; romperá si /users es customer-only). -## Changes +## Decision +Forzar la separación en el backend (no cliente): +1. `listCustomers` → siempre `... AND iu.role = 'customer'` (literal, no user input → sin inyección). + Parámetros inalterados: `[searchFilter, limit, offset]`; COUNT también filtra por rol. +2. `GET /admin/users` → condición base `role <> 'customer'` (literal). `?role=admin|editor` + se andaña con `AND role = $1`. Así `/admin/users` NUNCA devuelve customers, incluso con + `?role=customer` (devuelve vacío). Parámetro base es literal → índices de `$N` de los + filtros opcionales inalterados. +3. Frontend: dropdown de Users quita `