From 546971280f43e48facdc419b102e8003afd72593 Mon Sep 17 00:00:00 2001 From: rikrdo Date: Sat, 15 Aug 2026 09:27:38 +0200 Subject: [PATCH] feat(F-006): users profile, addresses and RBAC - users module: profile + address CRUD behind use cases (users_profiles, users_addresses) - roles customer/admin on identity_users; role resolved from DB per request - shared auth contract (Authenticate, requireRole, requireOwnerOrAdmin) injected from composition root; users never imports identity - authorization runs before existence checks; address SQL scoped by user_id - @fastify/cookie registered once at app root (cross-module) - migrations 003_identity_roles + 004_users (reversible) - no new npm dependencies; tests: unit 52, integration 22 Gates: reviewer/security/qa APPROVED; verify.sh green --- backlog/features.json | 8 +- project/README.md | 45 ++- project/migrations/003_identity_roles.js | 19 + project/migrations/004_users.js | 40 +++ project/src/app/build-app.ts | 17 +- project/src/app/tests/users.itest.ts | 334 ++++++++++++++++++ .../db/tests/migrations.itest.ts | 6 +- .../modules/identity/api/identity.routes.ts | 11 +- project/src/modules/identity/domain/user.ts | 3 + project/src/modules/identity/index.ts | 1 + .../infrastructure/pg-user-repository.ts | 9 +- .../infrastructure/session-authenticator.ts | 44 +++ project/src/modules/users/api/users.routes.ts | 163 +++++++++ .../users/application/address-use-cases.ts | 37 ++ .../users/application/profile-use-cases.ts | 27 ++ project/src/modules/users/domain/address.ts | 30 ++ project/src/modules/users/domain/ports.ts | 22 ++ project/src/modules/users/domain/profile.ts | 17 + project/src/modules/users/index.ts | 6 + .../infrastructure/pg-address-repository.ts | 131 +++++++ .../infrastructure/pg-profile-repository.ts | 80 +++++ project/src/shared/auth.ts | 35 ++ project/src/shared/tests/auth.test.ts | 40 +++ specs/F-006-users-rbac/DESIGN.md | 99 ++++++ specs/F-006-users-rbac/SPEC.md | 31 ++ specs/F-006-users-rbac/TASKS.md | 14 + specs/F-006-users-rbac/TESTS.md | 20 ++ work/artifacts/F-006/architect.md | 32 ++ work/artifacts/F-006/documenter.md | 29 ++ work/artifacts/F-006/implementer.md | 105 ++++++ work/artifacts/F-006/leader-close.json | 33 ++ work/artifacts/F-006/qa.json | 46 +++ work/artifacts/F-006/reviewer.json | 35 ++ work/artifacts/F-006/security.json | 40 +++ work/current.md | 20 +- work/history.md | 6 + work/runtime-status.json | 258 +++++++------- 37 files changed, 1732 insertions(+), 161 deletions(-) create mode 100644 project/migrations/003_identity_roles.js create mode 100644 project/migrations/004_users.js create mode 100644 project/src/app/tests/users.itest.ts create mode 100644 project/src/modules/identity/infrastructure/session-authenticator.ts create mode 100644 project/src/modules/users/api/users.routes.ts create mode 100644 project/src/modules/users/application/address-use-cases.ts create mode 100644 project/src/modules/users/application/profile-use-cases.ts create mode 100644 project/src/modules/users/domain/address.ts create mode 100644 project/src/modules/users/domain/ports.ts create mode 100644 project/src/modules/users/domain/profile.ts create mode 100644 project/src/modules/users/index.ts create mode 100644 project/src/modules/users/infrastructure/pg-address-repository.ts create mode 100644 project/src/modules/users/infrastructure/pg-profile-repository.ts create mode 100644 project/src/shared/auth.ts create mode 100644 project/src/shared/tests/auth.test.ts create mode 100644 specs/F-006-users-rbac/DESIGN.md create mode 100644 specs/F-006-users-rbac/SPEC.md create mode 100644 specs/F-006-users-rbac/TASKS.md create mode 100644 specs/F-006-users-rbac/TESTS.md create mode 100644 work/artifacts/F-006/architect.md create mode 100644 work/artifacts/F-006/documenter.md create mode 100644 work/artifacts/F-006/implementer.md create mode 100644 work/artifacts/F-006/leader-close.json create mode 100644 work/artifacts/F-006/qa.json create mode 100644 work/artifacts/F-006/reviewer.json create mode 100644 work/artifacts/F-006/security.json diff --git a/backlog/features.json b/backlog/features.json index 5ed1d28..cd195e2 100644 --- a/backlog/features.json +++ b/backlog/features.json @@ -237,12 +237,12 @@ "Address CRUD works end to end for own addresses", "verify.sh green" ], - "status": "pending", + "status": "done", "created_at": "2026-08-14", "gates": { - "review": false, - "security": false, - "qa": false + "review": true, + "security": true, + "qa": true } }, { diff --git a/project/README.md b/project/README.md index 7821bee..ea13201 100644 --- a/project/README.md +++ b/project/README.md @@ -40,8 +40,9 @@ is separate from deployment (no redeploy). Copy `.env.example` to `.env` to star Codes: `NOT_FOUND`, `VALIDATION_ERROR`, `BAD_REQUEST`/Fastify 4xx codes, `INTERNAL_ERROR`. 5xx messages are always generic; stack traces stay in server logs only. - Input validation is explicit per route: `parseJson(schema, body)` (zod) in the handler. -- Auth codes: `INVALID_CREDENTIALS` (401), `EMAIL_ALREADY_REGISTERED` (409), - `TOO_MANY_ATTEMPTS` (429, with `Retry-After` header). +- Auth codes: `UNAUTHORIZED` (401, missing/invalid/revoked session), + `FORBIDDEN` (403, role or ownership check failed), `INVALID_CREDENTIALS` (401), + `EMAIL_ALREADY_REGISTERED` (409), `TOO_MANY_ATTEMPTS` (429, with `Retry-After` header). - Log level via `LOG_LEVEL` env var (default `info`); logs are JSON only. ## Authentication (identity module) @@ -49,11 +50,11 @@ is separate from deployment (no redeploy). Copy `.env.example` to `.env` to star The server is the only authority for identity; the frontend is never trusted with session or credential state. -| Route | Result | -| ------------------- | --------------------------------------------------- | -| POST /auth/register | `201` + `{ id, email, createdAt }` | -| POST /auth/login | `200` + `{ id, email }` + `Set-Cookie: mdv_session` | -| POST /auth/logout | `204`, cookie cleared, session revoked (idempotent) | +| Route | Result | +| ------------------- | --------------------------------------------------------- | +| POST /auth/register | `201` + `{ id, email, role, createdAt }` | +| POST /auth/login | `200` + `{ id, email, role }` + `Set-Cookie: mdv_session` | +| POST /auth/logout | `204`, cookie cleared, session revoked (idempotent) | - Passwords: argon2id (OWASP parameters). Only the PHC hash is stored, never plaintext or anything reversible. @@ -68,6 +69,33 @@ session or credential state. swap later without touching use cases). - Identity routes are wired only when the app is built with a DB pool. +## Users and RBAC (users module) + +Every route resolves the session cookie against the DB first (expired/revoked +sessions and missing cookies get `401 UNAUTHORIZED`). Roles are `customer` +(default) and `admin`; the role is read from `identity_users` on every request, +so promotions/demotions apply immediately. Role changes are an out-of-band DB +operation in this slice (no admin API yet). + +| Route | Access | Result | +| -------------------------------------- | -------------- | ---------------------------------- | +| GET /users | admin only | `200` + `{ items: [profile] }` | +| GET /users/:id | owner or admin | `200` profile, `404` if none yet | +| PATCH /users/:id | owner or admin | `200` upserted profile | +| GET /users/:id/addresses | owner or admin | `200` + `{ items: [address] }` | +| POST /users/:id/addresses | owner or admin | `201` address | +| PATCH /users/:id/addresses/:addressId | owner or admin | `200` address, `404` if not theirs | +| DELETE /users/:id/addresses/:addressId | owner or admin | `204`, `404` if not theirs | + +- Authorization is checked before existence: a non-owner gets `403 FORBIDDEN` + regardless of whether the target resource exists (no enumeration). +- Address queries are scoped by `user_id` in SQL, so a valid foreign address id + is unreachable. +- `GET /users` lists users that have a profile row (users who have patched their + profile at least once). +- The users module never imports identity: session resolution arrives as an + injected `Authenticate` function from the composition root. + ## Database (local dev) ```bash @@ -100,7 +128,8 @@ src/ ├── modules/ # business modules, one folder each │ ├── health/ # exemplar module: public API only via index.ts │ ├── flags/ # feature flags (unknown default OFF, runtime flip) -│ └── identity/ # register/login/logout, argon2, sessions, rate limit +│ ├── identity/ # register/login/logout, argon2, sessions, rate limit +│ └── users/ # profile + address CRUD, owner-or-admin RBAC └── shared/ # cross-cutting helpers (error envelope, input parsing) ``` diff --git a/project/migrations/003_identity_roles.js b/project/migrations/003_identity_roles.js new file mode 100644 index 0000000..7493250 --- /dev/null +++ b/project/migrations/003_identity_roles.js @@ -0,0 +1,19 @@ +/** + * Identity schema evolution: roles for RBAC (introduced by F-006). + * Role is authorization truth and belongs with the account row. + * Shipped as its own migration to keep ownership explicit. + */ + +/** @param {import('node-pg-migrate').MigrationBuilder} pgm */ +export const up = (pgm) => { + pgm.sql(` + ALTER TABLE identity_users + ADD COLUMN role text NOT NULL DEFAULT 'customer' + CHECK (role IN ('customer', 'admin')) + `); +}; + +/** @param {import('node-pg-migrate').MigrationBuilder} pgm */ +export const down = (pgm) => { + pgm.sql('ALTER TABLE identity_users DROP COLUMN role'); +}; diff --git a/project/migrations/004_users.js b/project/migrations/004_users.js new file mode 100644 index 0000000..629a17a --- /dev/null +++ b/project/migrations/004_users.js @@ -0,0 +1,40 @@ +/** + * Users module tables. Module-owned naming: users_. + * FKs to identity_users are schema-level integrity only; runtime queries in + * the users module touch users_* tables exclusively. + */ + +/** @param {import('node-pg-migrate').MigrationBuilder} pgm */ +export const up = (pgm) => { + pgm.sql(` + CREATE TABLE users_profiles ( + user_id uuid PRIMARY KEY REFERENCES identity_users(id) ON DELETE CASCADE, + display_name text, + phone text, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() + ) + `); + pgm.sql(` + CREATE TABLE users_addresses ( + id uuid PRIMARY KEY DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES identity_users(id) ON DELETE CASCADE, + label text, + recipient_name text NOT NULL, + street text NOT NULL, + city text NOT NULL, + postal_code text NOT NULL, + country text NOT NULL, + is_default boolean NOT NULL DEFAULT false, + created_at timestamptz NOT NULL DEFAULT now(), + updated_at timestamptz NOT NULL DEFAULT now() + ) + `); + pgm.sql('CREATE INDEX users_addresses_user_id_idx ON users_addresses (user_id)'); +}; + +/** @param {import('node-pg-migrate').MigrationBuilder} pgm */ +export const down = (pgm) => { + pgm.sql('DROP TABLE IF EXISTS users_addresses'); + pgm.sql('DROP TABLE IF EXISTS users_profiles'); +}; diff --git a/project/src/app/build-app.ts b/project/src/app/build-app.ts index 90b89c2..2f55337 100644 --- a/project/src/app/build-app.ts +++ b/project/src/app/build-app.ts @@ -5,11 +5,13 @@ import { randomUUID } from 'node:crypto'; import { performance } from 'node:perf_hooks'; import Fastify, { type FastifyInstance } from 'fastify'; +import fastifyCookie from '@fastify/cookie'; import type { FastifyError, FastifyReply, FastifyRequest } from 'fastify'; import type { IncomingMessage } from 'node:http'; import type pg from 'pg'; import { registerHealthRoutes } from '../modules/health/index.js'; -import { registerIdentityRoutes } from '../modules/identity/index.js'; +import { registerIdentityRoutes, createSessionAuthenticator } from '../modules/identity/index.js'; +import { registerUsersRoutes } from '../modules/users/index.js'; import { createFlagStore, type FeatureFlagProvider } from '../modules/flags/index.js'; import { AppError, errorEnvelope } from '../shared/errors.js'; import { createLogger, type Logger } from '../infrastructure/logging/logger.js'; @@ -113,6 +115,9 @@ export async function buildApp(deps: BuildAppDeps = {}): Promise { await registerIdentityRoutes(instance, { @@ -120,6 +125,16 @@ export async function buildApp(deps: BuildAppDeps = {}): Promise { + await registerUsersRoutes(instance, { + pool: deps.pool as pg.Pool, + authenticate, + }); + }); } return app; diff --git a/project/src/app/tests/users.itest.ts b/project/src/app/tests/users.itest.ts new file mode 100644 index 0000000..84cec47 --- /dev/null +++ b/project/src/app/tests/users.itest.ts @@ -0,0 +1,334 @@ +import type { DestinationStream } from 'pino'; +import type pg from 'pg'; +import { afterAll, beforeAll, describe, expect, it } from 'vitest'; +import { buildApp } from '../build-app.js'; +import { createPool } from '../../infrastructure/db/pool.js'; +import { createLogger } from '../../infrastructure/logging/logger.js'; +import { + getTestDbUrl, + recreateDatabase, + runMigrations, +} from '../../infrastructure/db/tests/db-test-support.js'; +import { SESSION_COOKIE_NAME } from '../../modules/identity/index.js'; + +const hasDb = Boolean(process.env.TEST_DATABASE_URL); + +function silentLogger() { + const destination: DestinationStream = { write: () => undefined }; + return createLogger({ level: 'info', destination }); +} + +function cookieValue(setCookieHeader: string | string[] | undefined): string { + const raw = Array.isArray(setCookieHeader) ? setCookieHeader[0] : setCookieHeader; + expect(raw).toBeDefined(); + const pair = (raw as string).split(';')[0] as string; + return pair.slice(pair.indexOf('=') + 1); +} + +describe.skipIf(!hasDb)('users + RBAC flows (real PostgreSQL)', () => { + const url = hasDb ? getTestDbUrl() : ''; + let pool: pg.Pool; + let app: Awaited>; + + const ana = { email: 'ana@example.com', password: 'correct horse battery staple' }; + const ben = { email: 'ben@example.com', password: 'another valid password' }; + let anaId = ''; + let anaCookie = ''; + let benId = ''; + let benCookie = ''; + + async function registerAndLogin(user: { email: string; password: string }) { + const registered = await app.inject({ + method: 'POST', + url: '/auth/register', + headers: { 'content-type': 'application/json' }, + payload: user, + }); + expect(registered.statusCode).toBe(201); + const login = await app.inject({ + method: 'POST', + url: '/auth/login', + headers: { 'content-type': 'application/json' }, + payload: user, + }); + expect(login.statusCode).toBe(200); + const body = login.json() as { id: string; role: string }; + expect(body.role).toBe('customer'); + return { id: body.id, cookie: cookieValue(login.headers['set-cookie']) }; + } + + beforeAll(async () => { + await recreateDatabase(url); + await runMigrations(url, 'up'); + pool = createPool(url); + app = await buildApp({ logger: silentLogger(), pool, cookieSecure: true }); + + const a = await registerAndLogin(ana); + anaId = a.id; + anaCookie = a.cookie; + const b = await registerAndLogin(ben); + benId = b.id; + benCookie = b.cookie; + }); + + afterAll(async () => { + await app.close(); + await pool.end(); + }); + + it('unauthenticated and bad-cookie requests get 401', async () => { + const missing = await app.inject({ method: 'GET', url: `/users/${anaId}` }); + expect(missing.statusCode).toBe(401); + expect(missing.json().error.code).toBe('UNAUTHORIZED'); + + const forged = await app.inject({ + method: 'GET', + url: `/users/${anaId}`, + cookies: { [SESSION_COOKIE_NAME]: 'forged-token' }, + }); + expect(forged.statusCode).toBe(401); + }); + + it('user A requesting user B profile gets 403, own flow works (AC1)', async () => { + const cross = await app.inject({ + method: 'GET', + url: `/users/${benId}`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(cross.statusCode).toBe(403); + expect(cross.json().error.code).toBe('FORBIDDEN'); + + // Own profile does not exist yet -> 404 (authz passed, resource missing). + const ownMissing = await app.inject({ + method: 'GET', + url: `/users/${anaId}`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(ownMissing.statusCode).toBe(404); + }); + + it('PATCH own profile upserts; PATCH is validated', async () => { + const emptyBody = await app.inject({ + method: 'PATCH', + url: `/users/${anaId}`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + payload: {}, + }); + expect(emptyBody.statusCode).toBe(400); + + const created = await app.inject({ + method: 'PATCH', + url: `/users/${anaId}`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + payload: { displayName: 'Ana', phone: '+54 11 5555 0001' }, + }); + expect(created.statusCode).toBe(200); + expect(created.json().displayName).toBe('Ana'); + + const partial = await app.inject({ + method: 'PATCH', + url: `/users/${anaId}`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + payload: { phone: '+54 11 5555 0002' }, + }); + expect(partial.statusCode).toBe(200); + const body = partial.json(); + expect(body.phone).toBe('+54 11 5555 0002'); + expect(body.displayName).toBe('Ana'); // untouched field preserved + }); + + it('GET /users is admin-only: customer 403, admin 200 (AC2, AC3)', async () => { + const asCustomer = await app.inject({ + method: 'GET', + url: '/users', + cookies: { [SESSION_COOKIE_NAME]: benCookie }, + }); + expect(asCustomer.statusCode).toBe(403); + + // Promotion happens out-of-band (DB); session cookie picks up the new role. + await pool.query('UPDATE identity_users SET role = $1 WHERE id = $2', ['admin', anaId]); + + const asAdmin = await app.inject({ + method: 'GET', + url: '/users', + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(asAdmin.statusCode).toBe(200); + const body = asAdmin.json() as { items: Array<{ userId: string }> }; + expect(body.items.some((item) => item.userId === anaId)).toBe(true); + + // Admin can also read another user's profile (owner-or-admin). + const adminReadsBen = await app.inject({ + method: 'GET', + url: `/users/${benId}`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + // Ben has no profile yet: 404, not 403 (authz passed). + expect(adminReadsBen.statusCode).toBe(404); + }); + + it('address CRUD works end to end for own addresses (AC4)', async () => { + const addressInput = { + label: 'Casa', + recipientName: 'Ana Torres', + street: 'Av. Siempre Viva 742', + city: 'Springfield', + postalCode: '1000', + country: 'AR', + isDefault: true, + }; + + const created = await app.inject({ + method: 'POST', + url: `/users/${anaId}/addresses`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + payload: addressInput, + }); + expect(created.statusCode).toBe(201); + const address = created.json(); + expect(address.id).toMatch(/^[0-9a-f-]{36}$/); + expect(address.isDefault).toBe(true); + + const invalid = await app.inject({ + method: 'POST', + url: `/users/${anaId}/addresses`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + payload: { label: 'sin destinatario' }, + }); + expect(invalid.statusCode).toBe(400); + + const list = await app.inject({ + method: 'GET', + url: `/users/${anaId}/addresses`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(list.statusCode).toBe(200); + expect(list.json().items).toHaveLength(1); + + const updated = await app.inject({ + method: 'PATCH', + url: `/users/${anaId}/addresses/${address.id}`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + payload: { city: 'Shelbyville', isDefault: false }, + }); + expect(updated.statusCode).toBe(200); + expect(updated.json().city).toBe('Shelbyville'); + expect(updated.json().isDefault).toBe(false); + + const deleted = await app.inject({ + method: 'DELETE', + url: `/users/${anaId}/addresses/${address.id}`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(deleted.statusCode).toBe(204); + + const empty = await app.inject({ + method: 'GET', + url: `/users/${anaId}/addresses`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(empty.json().items).toHaveLength(0); + }); + + it("users cannot mutate addresses on another user's resource -> 403", async () => { + const created = await app.inject({ + method: 'POST', + url: `/users/${benId}/addresses`, + headers: { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: benCookie }, + payload: { + recipientName: 'Ben', + street: 'Calle 1', + city: 'CABA', + postalCode: '1400', + country: 'AR', + }, + }); + expect(created.statusCode).toBe(201); + const addressId = created.json().id as string; + + // Fresh regular customer (Ana was promoted to admin earlier, so she no + // longer represents a plain outsider). + const outsider = await registerAndLogin({ + email: 'dani@example.com', + password: 'dani valid password', + }); + + for (const attempt of [ + { + method: 'POST', + url: `/users/${benId}/addresses`, + payload: { + recipientName: 'Intruso', + street: 'x', + city: 'x', + postalCode: 'x', + country: 'x', + }, + }, + { method: 'PATCH', url: `/users/${benId}/addresses/${addressId}`, payload: { city: 'x' } }, + { method: 'DELETE', url: `/users/${benId}/addresses/${addressId}`, payload: undefined }, + ]) { + const response = await app.inject({ + method: attempt.method as 'POST' | 'PATCH' | 'DELETE', + url: attempt.url, + headers: attempt.payload === undefined ? {} : { 'content-type': 'application/json' }, + cookies: { [SESSION_COOKIE_NAME]: outsider.cookie }, + payload: attempt.payload, + }); + expect(response.statusCode).toBe(403); + } + + // Ana is admin now (previous test promoted her): admin bypasses ownership. + const adminDelete = await app.inject({ + method: 'DELETE', + url: `/users/${benId}/addresses/${addressId}`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(adminDelete.statusCode).toBe(204); + }); + + it('unknown address on own resource -> 404; malformed uuid -> 400', async () => { + const ghostId = '00000000-0000-0000-0000-000000000000'; + const missing = await app.inject({ + method: 'DELETE', + url: `/users/${anaId}/addresses/${ghostId}`, + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(missing.statusCode).toBe(404); + + const malformed = await app.inject({ + method: 'GET', + url: '/users/not-a-uuid', + cookies: { [SESSION_COOKIE_NAME]: anaCookie }, + }); + expect(malformed.statusCode).toBe(400); + expect(malformed.json().error.code).toBe('VALIDATION_ERROR'); + }); + + it('logout kills access to protected routes (F-005 regression)', async () => { + const session = await registerAndLogin({ + email: 'carla@example.com', + password: 'carla pass 123', + }); + const logout = await app.inject({ + method: 'POST', + url: '/auth/logout', + cookies: { [SESSION_COOKIE_NAME]: session.cookie }, + }); + expect(logout.statusCode).toBe(204); + + const after = await app.inject({ + method: 'GET', + url: `/users/${session.id}`, + cookies: { [SESSION_COOKIE_NAME]: session.cookie }, + }); + expect(after.statusCode).toBe(401); + }); +}); diff --git a/project/src/infrastructure/db/tests/migrations.itest.ts b/project/src/infrastructure/db/tests/migrations.itest.ts index 4427e0a..002b27d 100644 --- a/project/src/infrastructure/db/tests/migrations.itest.ts +++ b/project/src/infrastructure/db/tests/migrations.itest.ts @@ -17,11 +17,13 @@ describe.skipIf(!hasDb)('migrations', () => { await pool.end(); }); - it('fresh up creates the full schema (baseline + identity)', async () => { + it('fresh up creates the full schema (baseline + identity + users)', async () => { await runMigrations(url, 'up'); expect(await tableExists(pool, 'app_meta')).toBe(true); expect(await tableExists(pool, 'identity_users')).toBe(true); expect(await tableExists(pool, 'identity_sessions')).toBe(true); + expect(await tableExists(pool, 'users_profiles')).toBe(true); + expect(await tableExists(pool, 'users_addresses')).toBe(true); }); it('second up is a no-op', async () => { @@ -35,6 +37,8 @@ describe.skipIf(!hasDb)('migrations', () => { it('down rolls back the full schema cleanly', async () => { // count 0 reverts every applied migration in reverse order. await runMigrations(url, 'down', 0); + expect(await tableExists(pool, 'users_addresses')).toBe(false); + expect(await tableExists(pool, 'users_profiles')).toBe(false); expect(await tableExists(pool, 'identity_sessions')).toBe(false); expect(await tableExists(pool, 'identity_users')).toBe(false); expect(await tableExists(pool, 'app_meta')).toBe(false); diff --git a/project/src/modules/identity/api/identity.routes.ts b/project/src/modules/identity/api/identity.routes.ts index 7cba785..67e1729 100644 --- a/project/src/modules/identity/api/identity.routes.ts +++ b/project/src/modules/identity/api/identity.routes.ts @@ -3,7 +3,6 @@ * calls use cases, maps domain errors to the shared error envelope. */ import type { FastifyInstance, FastifyReply } from 'fastify'; -import fastifyCookie from '@fastify/cookie'; import { z } from 'zod'; import { parseJson } from '../../../shared/http-input.js'; import { AppError } from '../../../shared/errors.js'; @@ -47,8 +46,6 @@ export async function registerIdentityRoutes( app: FastifyInstance, deps: IdentityRoutesDeps, ): Promise { - await app.register(fastifyCookie); - const cookieSecure = deps.cookieSecure ?? true; const hasher = deps.hasher ?? new Argon2PasswordHasher(); const users = new PgUserRepository(deps.pool); @@ -70,7 +67,9 @@ export async function registerIdentityRoutes( const input = parseJson(credentialsSchema, request.body); try { const user = await registerUser.execute(input); - return reply.code(201).send({ id: user.id, email: user.email, createdAt: user.createdAt }); + return reply + .code(201) + .send({ id: user.id, email: user.email, role: user.role, createdAt: user.createdAt }); } catch (error) { if (error instanceof EmailAlreadyRegisteredError) { throw new AppError(409, 'EMAIL_ALREADY_REGISTERED', 'Email already registered'); @@ -84,7 +83,9 @@ export async function registerIdentityRoutes( try { const result = await login.execute(input); setSessionCookie(reply, result.token, cookieSecure); - return reply.code(200).send({ id: result.user.id, email: result.user.email }); + return reply + .code(200) + .send({ id: result.user.id, email: result.user.email, role: result.user.role }); } catch (error) { if (error instanceof RateLimitedError) { void reply.header('Retry-After', String(Math.ceil(error.retryAfterMs / 1000))); diff --git a/project/src/modules/identity/domain/user.ts b/project/src/modules/identity/domain/user.ts index 5c3abe7..b0b5516 100644 --- a/project/src/modules/identity/domain/user.ts +++ b/project/src/modules/identity/domain/user.ts @@ -2,9 +2,12 @@ * Identity domain. Pure types and rules: no framework, no infrastructure. */ +import type { Role } from '../../../shared/auth.js'; + export interface User { id: string; email: string; + role: Role; createdAt: Date; } diff --git a/project/src/modules/identity/index.ts b/project/src/modules/identity/index.ts index 4f1ab71..82a0e64 100644 --- a/project/src/modules/identity/index.ts +++ b/project/src/modules/identity/index.ts @@ -7,3 +7,4 @@ export { SESSION_COOKIE_NAME, type IdentityRoutesDeps, } from './api/identity.routes.js'; +export { createSessionAuthenticator } from './infrastructure/session-authenticator.js'; diff --git a/project/src/modules/identity/infrastructure/pg-user-repository.ts b/project/src/modules/identity/infrastructure/pg-user-repository.ts index e7d3d65..8b6e89d 100644 --- a/project/src/modules/identity/infrastructure/pg-user-repository.ts +++ b/project/src/modules/identity/infrastructure/pg-user-repository.ts @@ -5,12 +5,14 @@ import type pg from 'pg'; import type { UserRepository } from '../domain/ports.js'; import type { NewUser, User } from '../domain/user.js'; +import type { Role } from '../../../shared/auth.js'; import { EmailAlreadyRegisteredError } from '../domain/errors.js'; interface UserRow { id: string; email: string; password_hash: string; + role: Role; created_at: Date; } @@ -24,14 +26,14 @@ export class PgUserRepository implements UserRepository { const result = await this.pool.query( `INSERT INTO identity_users (email, password_hash) VALUES ($1, $2) - RETURNING id, email, created_at`, + RETURNING id, email, role, created_at`, [user.email, user.passwordHash], ); const row = result.rows[0]; if (!row) { throw new Error('identity_users INSERT returned no row'); } - return { id: row.id, email: row.email, createdAt: row.created_at }; + return { id: row.id, email: row.email, role: row.role, createdAt: row.created_at }; } catch (error) { if (isPgError(error) && error.code === UNIQUE_VIOLATION) { throw new EmailAlreadyRegisteredError(); @@ -42,7 +44,7 @@ export class PgUserRepository implements UserRepository { async findByEmail(email: string): Promise<(User & { passwordHash: string }) | undefined> { const result = await this.pool.query( - `SELECT id, email, password_hash, created_at + `SELECT id, email, password_hash, role, created_at FROM identity_users WHERE email = $1`, [email], @@ -54,6 +56,7 @@ export class PgUserRepository implements UserRepository { return { id: row.id, email: row.email, + role: row.role, createdAt: row.created_at, passwordHash: row.password_hash, }; diff --git a/project/src/modules/identity/infrastructure/session-authenticator.ts b/project/src/modules/identity/infrastructure/session-authenticator.ts new file mode 100644 index 0000000..5ea9442 --- /dev/null +++ b/project/src/modules/identity/infrastructure/session-authenticator.ts @@ -0,0 +1,44 @@ +/** + * Resolves the session cookie into the current user. + * Server-side truth: validity (expiry + revocation) and role come from the DB, + * never from the client. Exported through the module index so the composition + * root can inject it into other modules without cross-module imports. + */ +import type { FastifyRequest } from 'fastify'; +import type pg from 'pg'; +import type { Authenticate, CurrentUser } from '../../../shared/auth.js'; +import { AppError } from '../../../shared/errors.js'; +import { hashSessionToken } from './session-token.js'; +import { SESSION_COOKIE_NAME } from '../api/identity.routes.js'; + +interface ResolvedRow { + id: string; + email: string; + role: string; +} + +const RESOLVE_SQL = ` + SELECT u.id, u.email, u.role + FROM identity_sessions s + JOIN identity_users u ON u.id = s.user_id + WHERE s.token_hash = $1 + AND s.revoked_at IS NULL + AND s.expires_at > now() +`; + +export function createSessionAuthenticator(pool: pg.Pool): Authenticate { + return async (request: FastifyRequest): Promise => { + const token = request.cookies[SESSION_COOKIE_NAME]; + if (!token) { + throw new AppError(401, 'UNAUTHORIZED', 'Authentication required'); + } + + const result = await pool.query(RESOLVE_SQL, [hashSessionToken(token)]); + const row = result.rows[0]; + if (!row) { + throw new AppError(401, 'UNAUTHORIZED', 'Authentication required'); + } + + return { id: row.id, email: row.email, role: row.role as CurrentUser['role'] }; + }; +} diff --git a/project/src/modules/users/api/users.routes.ts b/project/src/modules/users/api/users.routes.ts new file mode 100644 index 0000000..912332b --- /dev/null +++ b/project/src/modules/users/api/users.routes.ts @@ -0,0 +1,163 @@ +/** + * Users API adapters. Authorization (role + ownership) runs BEFORE existence + * checks, so a non-owner always gets 403 regardless of resource existence. + */ +import type { FastifyInstance } from 'fastify'; +import { z } from 'zod'; +import type pg from 'pg'; +import { parseJson } from '../../../shared/http-input.js'; +import { AppError } from '../../../shared/errors.js'; +import { requireOwnerOrAdmin, requireRole, type Authenticate } from '../../../shared/auth.js'; +import { GetProfile, ListProfiles, UpdateProfile } from '../application/profile-use-cases.js'; +import { + CreateAddress, + DeleteAddress, + ListAddresses, + UpdateAddress, +} from '../application/address-use-cases.js'; +import { PgProfileRepository } from '../infrastructure/pg-profile-repository.js'; +import { PgAddressRepository } from '../infrastructure/pg-address-repository.js'; +import type { Address } from '../domain/address.js'; +import type { Profile } from '../domain/profile.js'; + +export interface UsersRoutesDeps { + pool: pg.Pool; + /** Injected by the composition root (identity owns session resolution). */ + authenticate: Authenticate; +} + +const uuidParamSchema = z.object({ id: z.uuid() }); +const addressIdParamSchema = z.object({ id: z.uuid(), addressId: z.uuid() }); + +const profilePatchSchema = z + .object({ + displayName: z.string().min(1).max(200).optional(), + phone: z.string().min(1).max(50).optional(), + }) + .refine((value) => value.displayName !== undefined || value.phone !== undefined, { + message: 'At least one of displayName or phone is required', + }); + +const newAddressSchema = z.object({ + label: z.string().max(100).optional().nullable(), + recipientName: z.string().min(1).max(200), + street: z.string().min(1).max(300), + city: z.string().min(1).max(100), + postalCode: z.string().min(1).max(20), + country: z.string().min(1).max(100), + isDefault: z.boolean().optional(), +}); + +const addressPatchSchema = newAddressSchema + .partial() + .refine((value) => Object.values(value).some((field) => field !== undefined), { + message: 'At least one address field is required', + }); + +export async function registerUsersRoutes( + app: FastifyInstance, + deps: UsersRoutesDeps, +): Promise { + const profiles = new PgProfileRepository(deps.pool); + const addresses = new PgAddressRepository(deps.pool); + const getProfile = new GetProfile(profiles); + const updateProfile = new UpdateProfile(profiles); + const listProfiles = new ListProfiles(profiles); + const listAddresses = new ListAddresses(addresses); + const createAddress = new CreateAddress(addresses); + const updateAddress = new UpdateAddress(addresses); + const deleteAddress = new DeleteAddress(addresses); + + app.get('/users', async (request, reply) => { + const user = await deps.authenticate(request); + requireRole(user, 'admin'); + const items = await listProfiles.execute(); + return reply.send({ items: items.map(serializeProfile) }); + }); + + app.get('/users/:id', async (request, reply) => { + const user = await deps.authenticate(request); + const { id } = parseJson(uuidParamSchema, request.params); + requireOwnerOrAdmin(user, id); + const profile = await getProfile.execute(id); + if (!profile) { + throw new AppError(404, 'NOT_FOUND', 'Profile not found'); + } + return reply.send(serializeProfile(profile)); + }); + + app.patch('/users/:id', async (request, reply) => { + const user = await deps.authenticate(request); + const { id } = parseJson(uuidParamSchema, request.params); + requireOwnerOrAdmin(user, id); + const patch = parseJson(profilePatchSchema, request.body); + const profile = await updateProfile.execute(id, patch); + return reply.send(serializeProfile(profile)); + }); + + app.get('/users/:id/addresses', async (request, reply) => { + const user = await deps.authenticate(request); + const { id } = parseJson(uuidParamSchema, request.params); + requireOwnerOrAdmin(user, id); + const items = await listAddresses.execute(id); + return reply.send({ items: items.map(serializeAddress) }); + }); + + app.post('/users/:id/addresses', async (request, reply) => { + const user = await deps.authenticate(request); + const { id } = parseJson(uuidParamSchema, request.params); + requireOwnerOrAdmin(user, id); + const input = parseJson(newAddressSchema, request.body); + const address = await createAddress.execute(id, input); + return reply.code(201).send(serializeAddress(address)); + }); + + app.patch('/users/:id/addresses/:addressId', async (request, reply) => { + const user = await deps.authenticate(request); + const { id, addressId } = parseJson(addressIdParamSchema, request.params); + requireOwnerOrAdmin(user, id); + const patch = parseJson(addressPatchSchema, request.body); + const address = await updateAddress.execute(id, addressId, patch); + if (!address) { + throw new AppError(404, 'NOT_FOUND', 'Address not found'); + } + return reply.send(serializeAddress(address)); + }); + + app.delete('/users/:id/addresses/:addressId', async (request, reply) => { + const user = await deps.authenticate(request); + const { id, addressId } = parseJson(addressIdParamSchema, request.params); + requireOwnerOrAdmin(user, id); + const deleted = await deleteAddress.execute(id, addressId); + if (!deleted) { + throw new AppError(404, 'NOT_FOUND', 'Address not found'); + } + return reply.code(204).send(); + }); +} + +function serializeProfile(profile: Profile) { + return { + userId: profile.userId, + displayName: profile.displayName, + phone: profile.phone, + createdAt: profile.createdAt.toISOString(), + updatedAt: profile.updatedAt.toISOString(), + }; +} + +function serializeAddress(address: Address) { + return { + id: address.id, + userId: address.userId, + label: address.label, + recipientName: address.recipientName, + street: address.street, + city: address.city, + postalCode: address.postalCode, + country: address.country, + isDefault: address.isDefault, + createdAt: address.createdAt.toISOString(), + updatedAt: address.updatedAt.toISOString(), + }; +} diff --git a/project/src/modules/users/application/address-use-cases.ts b/project/src/modules/users/application/address-use-cases.ts new file mode 100644 index 0000000..5313cc0 --- /dev/null +++ b/project/src/modules/users/application/address-use-cases.ts @@ -0,0 +1,37 @@ +/** + * Address use cases. Every operation is scoped to the owner's userId. + */ +import type { AddressRepository } from '../domain/ports.js'; +import type { Address, AddressPatch, NewAddress } from '../domain/address.js'; + +export class ListAddresses { + constructor(private readonly addresses: AddressRepository) {} + async execute(userId: string): Promise { + return this.addresses.listByUserId(userId); + } +} + +export class CreateAddress { + constructor(private readonly addresses: AddressRepository) {} + async execute(userId: string, input: NewAddress): Promise
{ + return this.addresses.create(userId, input); + } +} + +export class UpdateAddress { + constructor(private readonly addresses: AddressRepository) {} + async execute( + userId: string, + addressId: string, + patch: AddressPatch, + ): Promise
{ + return this.addresses.update(userId, addressId, patch); + } +} + +export class DeleteAddress { + constructor(private readonly addresses: AddressRepository) {} + async execute(userId: string, addressId: string): Promise { + return this.addresses.delete(userId, addressId); + } +} diff --git a/project/src/modules/users/application/profile-use-cases.ts b/project/src/modules/users/application/profile-use-cases.ts new file mode 100644 index 0000000..e157ed8 --- /dev/null +++ b/project/src/modules/users/application/profile-use-cases.ts @@ -0,0 +1,27 @@ +/** + * Profile use cases. Thin orchestration over ports; ownership/role checks + * happen in the API layer before these run. + */ +import type { ProfileRepository } from '../domain/ports.js'; +import type { Profile, ProfilePatch } from '../domain/profile.js'; + +export class GetProfile { + constructor(private readonly profiles: ProfileRepository) {} + async execute(userId: string): Promise { + return this.profiles.findByUserId(userId); + } +} + +export class UpdateProfile { + constructor(private readonly profiles: ProfileRepository) {} + async execute(userId: string, patch: ProfilePatch): Promise { + return this.profiles.upsert(userId, patch); + } +} + +export class ListProfiles { + constructor(private readonly profiles: ProfileRepository) {} + async execute(): Promise { + return this.profiles.list(); + } +} diff --git a/project/src/modules/users/domain/address.ts b/project/src/modules/users/domain/address.ts new file mode 100644 index 0000000..374d4bd --- /dev/null +++ b/project/src/modules/users/domain/address.ts @@ -0,0 +1,30 @@ +/** + * Address domain model. + */ + +export interface Address { + id: string; + userId: string; + label: string | null; + recipientName: string; + street: string; + city: string; + postalCode: string; + country: string; + isDefault: boolean; + createdAt: Date; + updatedAt: Date; +} + +export interface NewAddress { + label?: string | null; + recipientName: string; + street: string; + city: string; + postalCode: string; + country: string; + isDefault?: boolean; +} + +/** Fields an address update may set. Undefined = leave unchanged. */ +export type AddressPatch = Partial; diff --git a/project/src/modules/users/domain/ports.ts b/project/src/modules/users/domain/ports.ts new file mode 100644 index 0000000..d38a325 --- /dev/null +++ b/project/src/modules/users/domain/ports.ts @@ -0,0 +1,22 @@ +/** + * Ports (driven interfaces). Domain owns them; infrastructure implements them. + * All operations are scoped by userId so ownership is enforced in the query. + */ +import type { Profile, ProfilePatch } from './profile.js'; +import type { Address, AddressPatch, NewAddress } from './address.js'; + +export interface ProfileRepository { + findByUserId(userId: string): Promise; + /** Idempotent upsert; only provided fields change. */ + upsert(userId: string, patch: ProfilePatch): Promise; + list(): Promise; +} + +export interface AddressRepository { + listByUserId(userId: string): Promise; + create(userId: string, input: NewAddress): Promise
; + /** Returns undefined when the address does not belong to userId. */ + update(userId: string, addressId: string, patch: AddressPatch): Promise
; + /** Returns false when the address does not belong to userId. */ + delete(userId: string, addressId: string): Promise; +} diff --git a/project/src/modules/users/domain/profile.ts b/project/src/modules/users/domain/profile.ts new file mode 100644 index 0000000..46a45b6 --- /dev/null +++ b/project/src/modules/users/domain/profile.ts @@ -0,0 +1,17 @@ +/** + * Users domain. Pure types: no framework, no infrastructure. + */ + +export interface Profile { + userId: string; + displayName: string | null; + phone: string | null; + createdAt: Date; + updatedAt: Date; +} + +/** Fields a profile update may set. Undefined = leave unchanged. */ +export interface ProfilePatch { + displayName?: string | null; + phone?: string | null; +} diff --git a/project/src/modules/users/index.ts b/project/src/modules/users/index.ts new file mode 100644 index 0000000..6de0db9 --- /dev/null +++ b/project/src/modules/users/index.ts @@ -0,0 +1,6 @@ +/** + * Public API of the users module. Everything the module exposes to the outside + * world goes through this file. Auth arrives by injection (shared contract), + * never by importing identity. + */ +export { registerUsersRoutes, type UsersRoutesDeps } from './api/users.routes.js'; diff --git a/project/src/modules/users/infrastructure/pg-address-repository.ts b/project/src/modules/users/infrastructure/pg-address-repository.ts new file mode 100644 index 0000000..0f4d612 --- /dev/null +++ b/project/src/modules/users/infrastructure/pg-address-repository.ts @@ -0,0 +1,131 @@ +/** + * PostgreSQL AddressRepository. All operations are scoped by user_id, so a + * caller can never read or mutate another user's address even with a valid id. + */ +import type pg from 'pg'; +import type { AddressRepository } from '../domain/ports.js'; +import type { Address, AddressPatch, NewAddress } from '../domain/address.js'; + +interface AddressRow { + id: string; + user_id: string; + label: string | null; + recipient_name: string; + street: string; + city: string; + postal_code: string; + country: string; + is_default: boolean; + created_at: Date; + updated_at: Date; +} + +/** Whitelist of updatable columns -> input key. Prevents SQL building from input. */ +const UPDATABLE: ReadonlyArray<[keyof AddressPatch, string]> = [ + ['label', 'label'], + ['recipientName', 'recipient_name'], + ['street', 'street'], + ['city', 'city'], + ['postalCode', 'postal_code'], + ['country', 'country'], + ['isDefault', 'is_default'], +]; + +export class PgAddressRepository implements AddressRepository { + constructor(private readonly pool: pg.Pool) {} + + async listByUserId(userId: string): Promise { + const result = await this.pool.query( + `SELECT * FROM users_addresses WHERE user_id = $1 + ORDER BY is_default DESC, created_at`, + [userId], + ); + return result.rows.map(toAddress); + } + + async create(userId: string, input: NewAddress): Promise
{ + const result = await this.pool.query( + `INSERT INTO users_addresses + (user_id, label, recipient_name, street, city, postal_code, country, is_default) + VALUES ($1, $2, $3, $4, $5, $6, $7, $8) + RETURNING *`, + [ + userId, + input.label ?? null, + input.recipientName, + input.street, + input.city, + input.postalCode, + input.country, + input.isDefault ?? false, + ], + ); + const row = result.rows[0]; + if (!row) { + throw new Error('users_addresses INSERT returned no row'); + } + return toAddress(row); + } + + async update( + userId: string, + addressId: string, + patch: AddressPatch, + ): Promise
{ + const setClauses: string[] = []; + const values: unknown[] = []; + for (const [key, column] of UPDATABLE) { + const value = patch[key]; + if (value !== undefined) { + values.push(value); + setClauses.push(`${column} = $${values.length}`); + } + } + if (setClauses.length === 0) { + return this.findByIdScoped(userId, addressId); + } + + values.push(userId, addressId); + const result = await this.pool.query( + `UPDATE users_addresses SET ${setClauses.join(', ')}, updated_at = now() + WHERE user_id = $${values.length - 1} AND id = $${values.length} + RETURNING *`, + values, + ); + const row = result.rows[0]; + return row ? toAddress(row) : undefined; + } + + async delete(userId: string, addressId: string): Promise { + const result = await this.pool.query( + `DELETE FROM users_addresses WHERE user_id = $1 AND id = $2`, + [userId, addressId], + ); + return (result.rowCount ?? 0) > 0; + } + + private async findByIdScoped(userId: string, addressId: string): Promise
{ + const result = await this.pool.query( + `SELECT * FROM users_addresses WHERE user_id = $1 AND id = $2`, + [userId, addressId], + ); + const row = result.rows[0]; + return row ? toAddress(row) : undefined; + } +} + +function toAddress(row: AddressRow): Address { + return { + id: row.id, + userId: row.user_id, + label: row.label, + recipientName: row.recipient_name, + street: row.street, + city: row.city, + postalCode: row.postal_code, + country: row.country, + isDefault: row.is_default, + createdAt: row.created_at, + updatedAt: row.updated_at, + }; +} diff --git a/project/src/modules/users/infrastructure/pg-profile-repository.ts b/project/src/modules/users/infrastructure/pg-profile-repository.ts new file mode 100644 index 0000000..236e229 --- /dev/null +++ b/project/src/modules/users/infrastructure/pg-profile-repository.ts @@ -0,0 +1,80 @@ +/** + * PostgreSQL ProfileRepository. Parameterized queries only; update columns are + * whitelisted, never built from user input. + */ +import type pg from 'pg'; +import type { ProfileRepository } from '../domain/ports.js'; +import type { Profile, ProfilePatch } from '../domain/profile.js'; + +interface ProfileRow { + user_id: string; + display_name: string | null; + phone: string | null; + created_at: Date; + updated_at: Date; +} + +export class PgProfileRepository implements ProfileRepository { + constructor(private readonly pool: pg.Pool) {} + + async findByUserId(userId: string): Promise { + const result = await this.pool.query( + `SELECT user_id, display_name, phone, created_at, updated_at + FROM users_profiles WHERE user_id = $1`, + [userId], + ); + const row = result.rows[0]; + return row ? toProfile(row) : undefined; + } + + async upsert(userId: string, patch: ProfilePatch): Promise { + await this.pool.query( + `INSERT INTO users_profiles (user_id) VALUES ($1) + ON CONFLICT (user_id) DO NOTHING`, + [userId], + ); + + const setClauses: string[] = []; + const values: unknown[] = []; + if (patch.displayName !== undefined) { + values.push(patch.displayName); + setClauses.push(`display_name = $${values.length}`); + } + if (patch.phone !== undefined) { + values.push(patch.phone); + setClauses.push(`phone = $${values.length}`); + } + if (setClauses.length > 0) { + values.push(userId); + await this.pool.query( + `UPDATE users_profiles SET ${setClauses.join(', ')}, updated_at = now() + WHERE user_id = $${values.length}`, + values, + ); + } + + const profile = await this.findByUserId(userId); + if (!profile) { + throw new Error('users_profiles upsert did not return a row'); + } + return profile; + } + + async list(): Promise { + const result = await this.pool.query( + `SELECT user_id, display_name, phone, created_at, updated_at + FROM users_profiles ORDER BY created_at`, + ); + return result.rows.map(toProfile); + } +} + +function toProfile(row: ProfileRow): Profile { + return { + userId: row.user_id, + displayName: row.display_name, + phone: row.phone, + createdAt: row.created_at, + updatedAt: row.updated_at, + }; +} diff --git a/project/src/shared/auth.ts b/project/src/shared/auth.ts new file mode 100644 index 0000000..bec31aa --- /dev/null +++ b/project/src/shared/auth.ts @@ -0,0 +1,35 @@ +/** + * Shared auth contracts. Identity implements the authenticator; consumers + * (users, future modules) receive it by injection from the composition root. + * No module ever imports another module for auth. + */ +import type { FastifyRequest } from 'fastify'; +import { AppError } from './errors.js'; + +export type Role = 'customer' | 'admin'; + +export interface CurrentUser { + id: string; + email: string; + role: Role; +} + +/** + * Resolves the session on the request into the current user. + * Throws AppError(401) when the request is not authenticated. + */ +export type Authenticate = (request: FastifyRequest) => Promise; + +/** Throws AppError(403) unless the user holds the required role. */ +export function requireRole(user: CurrentUser, role: Role): void { + if (user.role !== role) { + throw new AppError(403, 'FORBIDDEN', 'Access denied'); + } +} + +/** Throws AppError(403) unless the user is the resource owner or an admin. */ +export function requireOwnerOrAdmin(user: CurrentUser, ownerId: string): void { + if (user.role !== 'admin' && user.id !== ownerId) { + throw new AppError(403, 'FORBIDDEN', 'Access denied'); + } +} diff --git a/project/src/shared/tests/auth.test.ts b/project/src/shared/tests/auth.test.ts new file mode 100644 index 0000000..c167973 --- /dev/null +++ b/project/src/shared/tests/auth.test.ts @@ -0,0 +1,40 @@ +import { describe, expect, it } from 'vitest'; +import { AppError } from '../errors.js'; +import { requireOwnerOrAdmin, requireRole, type CurrentUser } from '../auth.js'; + +const customer: CurrentUser = { id: 'user-a', email: 'a@example.com', role: 'customer' }; +const admin: CurrentUser = { id: 'user-admin', email: 'admin@example.com', role: 'admin' }; + +function codeOf(fn: () => void): string | undefined { + try { + fn(); + } catch (error) { + return error instanceof AppError ? error.code : undefined; + } + return undefined; +} + +describe('requireRole', () => { + it('allows a matching role', () => { + expect(() => requireRole(admin, 'admin')).not.toThrow(); + expect(() => requireRole(customer, 'customer')).not.toThrow(); + }); + + it('throws 403 FORBIDDEN for a missing role', () => { + expect(codeOf(() => requireRole(customer, 'admin'))).toBe('FORBIDDEN'); + }); +}); + +describe('requireOwnerOrAdmin', () => { + it('allows the owner regardless of role', () => { + expect(() => requireOwnerOrAdmin(customer, 'user-a')).not.toThrow(); + }); + + it('allows an admin on any user', () => { + expect(() => requireOwnerOrAdmin(admin, 'user-b')).not.toThrow(); + }); + + it('throws 403 FORBIDDEN for a non-owner customer', () => { + expect(codeOf(() => requireOwnerOrAdmin(customer, 'user-b'))).toBe('FORBIDDEN'); + }); +}); diff --git a/specs/F-006-users-rbac/DESIGN.md b/specs/F-006-users-rbac/DESIGN.md new file mode 100644 index 0000000..0f4b920 --- /dev/null +++ b/specs/F-006-users-rbac/DESIGN.md @@ -0,0 +1,99 @@ +# DESIGN — F-006 Users: profile, addresses, RBAC + +## Boundary constraint that drives everything +Rule R1: a module may import only its own subtree, `src/shared`, Node builtins, and +npm packages. Therefore `users` MUST NOT import `identity`. The session authenticator +(which reads identity tables) is exported by identity's public API and INJECTED into +users by the composition root (`src/app/build-app.ts`). Shared contract types live in +`src/shared/auth.ts` so both modules agree without importing each other. + +## Shared contract — src/shared/auth.ts +```ts +Role = 'customer' | 'admin' +CurrentUser = { id, email, role } +Authenticate = (request: FastifyRequest) => Promise // throws AppError 401 +requireRole(user, role) -> void | throws AppError 403 +``` + +## Role +Migration 003 (identity namespace): `ALTER TABLE identity_users ADD COLUMN role text +NOT NULL DEFAULT 'customer' CHECK (role IN ('customer','admin'))`. Role is the single +source of truth for "who may act as what" and travels with the authenticated user. + +## identity additions +- `createSessionAuthenticator(pool): Authenticate` exported from identity index. + Reads `mdv_session` cookie, hashes token, resolves via + `identity_sessions JOIN identity_users` (valid, not revoked, not expired). Returns + CurrentUser or throws AppError(401, 'UNAUTHORIZED'). +- Login response now includes `role` (additive). + +## users module layout (hexagonal) +``` +src/modules/users/ + index.ts # registerUsersRoutes(app, { pool, authenticate }) + domain/ + profile.ts # Profile type + normalize + address.ts # Address type + errors.ts # ForbiddenError, NotFoundError + ports.ts # ProfileRepository, AddressRepository + application/ + get-profile.ts update-profile.ts list-profiles.ts + list-addresses.ts create-address.ts update-address.ts delete-address.ts + infrastructure/ + pg-profile-repository.ts pg-address-repository.ts + api/users.routes.ts + tests/ +``` + +## Data model — migration 004_users +```sql +users_profiles( + user_id uuid PK REFERENCES identity_users(id) ON DELETE CASCADE, + display_name text, phone text, + created_at timestamptz DEFAULT now(), updated_at timestamptz DEFAULT now() +) +users_addresses( + id uuid PK DEFAULT gen_random_uuid(), + user_id uuid NOT NULL REFERENCES identity_users(id) ON DELETE CASCADE, + label text, recipient_name text NOT NULL, street text NOT NULL, + city text NOT NULL, postal_code text NOT NULL, country text NOT NULL, + is_default boolean NOT NULL DEFAULT false, + created_at timestamptz DEFAULT now(), updated_at timestamptz DEFAULT now() +) + index on user_id +``` +FK to identity_users is schema-level integrity only; users runtime queries touch only +`users_*` tables (prefix rule upheld). + +## API (all guarded; ownership = param id === auth id OR role admin) +| Route | Authz | Notes | +|---|---|---| +| GET /users | admin only | list profiles (AC2/AC3) | +| GET /users/:id | owner-or-admin | profile; non-owner -> 403 (AC1) | +| PATCH /users/:id | owner-or-admin | upsert display_name/phone | +| GET /users/:id/addresses | owner-or-admin | list | +| POST /users/:id/addresses | owner-or-admin | create | +| PATCH /users/:id/addresses/:addressId | owner-or-admin | update | +| DELETE /users/:id/addresses/:addressId | owner-or-admin | delete | + +Authorization runs BEFORE existence checks so a non-owner gets 403 regardless of +whether the target exists (no enumeration). Bodies validated with parseJson (zod). + +## Wiring (build-app) +``` +if (deps.pool) { + registerIdentityRoutes(...) + const authenticate = createSessionAuthenticator(deps.pool) + registerUsersRoutes(app, { pool, authenticate }) +} +``` + +## Test strategy +- Unit (module-scoped): requireRole, ownership decision, address/profile normalization. +- Integration (app-scoped itest, real PostgreSQL): A->B 403; customer->admin-only 403; + admin->admin-only 200; address CRUD end-to-end; unauthenticated -> 401; admin role + granted by direct SQL UPDATE in the test. + +## Risks / mitigations +- Cross-module sharing solved by injection + shared contract, not imports. +- Role escalation is impossible via API (role only set by DB/migration; register always + creates 'customer'). diff --git a/specs/F-006-users-rbac/SPEC.md b/specs/F-006-users-rbac/SPEC.md new file mode 100644 index 0000000..7d7c366 --- /dev/null +++ b/specs/F-006-users-rbac/SPEC.md @@ -0,0 +1,31 @@ +# SPEC — F-006 Users: profile, addresses, RBAC + +## Problem +Authenticated users need profile data, addresses, and clear roles. + +## Goal +Users module with profile + address CRUD behind use cases, and customer/admin roles +enforced by an RBAC guard on the API layer. The server decides ownership and role — +the frontend is never trusted. + +## Scope IN +- `src/modules/users` (hexagonal) owning `users_profiles` + `users_addresses` +- Profile + address CRUD behind use cases +- Roles customer/admin; `role` column on `identity_users` +- Session authenticator exported by identity, injected into users by the composition root +- Owner-or-admin authorization; admin-only list endpoint + +## Scope OUT +- No fine-grained permissions (only role + ownership) +- No admin UI +- No profile auto-creation at register (profiles are created on demand) + +## Acceptance criteria +1. Given user A When A requests user B profile Then HTTP 403. +2. Given customer role When admin-only endpoint called Then HTTP 403. +3. Given admin role When admin-only endpoint called Then HTTP 200. +4. Address CRUD works end to end for own addresses. +5. `./scripts/verify.sh` green. + +## Dependencies added +- None. Reuses pg, zod, @fastify/cookie. diff --git a/specs/F-006-users-rbac/TASKS.md b/specs/F-006-users-rbac/TASKS.md new file mode 100644 index 0000000..d10a415 --- /dev/null +++ b/specs/F-006-users-rbac/TASKS.md @@ -0,0 +1,14 @@ +# TASKS — F-006 Users: profile, addresses, RBAC + +- [ ] TASK-001 migrations: 003_identity_roles.js (role column) + 004_users.js (profiles + addresses), reversible +- [ ] TASK-002 shared/auth.ts: Role, CurrentUser, Authenticate, requireRole +- [ ] TASK-003 identity: session-authenticator (cookie -> CurrentUser) + export from index + role in login response +- [ ] TASK-004 users domain: profile.ts, address.ts, errors.ts, ports.ts +- [ ] TASK-005 users application: profile get/update/list + address list/create/update/delete (owner-or-admin checks) +- [ ] TASK-006 users infrastructure: pg repos (parameterized) +- [ ] TASK-007 users api: users.routes.ts (parseJson, error mapping) +- [ ] TASK-008 users index.ts public API +- [ ] TASK-009 build-app wiring: authenticate injection +- [ ] TASK-010 unit tests (module-scoped) +- [ ] TASK-011 integration tests (app-scoped itest, AC1-AC4) +- [ ] TASK-012 lint + boundaries + typecheck + build + test + test:integration + verify.sh diff --git a/specs/F-006-users-rbac/TESTS.md b/specs/F-006-users-rbac/TESTS.md new file mode 100644 index 0000000..7b6f21c --- /dev/null +++ b/specs/F-006-users-rbac/TESTS.md @@ -0,0 +1,20 @@ +# TESTS — F-006 Users: profile, addresses, RBAC + +## Unit — src/shared/tests/auth.test.ts +1. requireRole allows matching role, throws 403 AppError otherwise + +## Unit — src/modules/users/tests/ownership.test.ts +1. canAccessUser: owner true, admin true, other customer false +2. same decisions regardless of target existence (authz before lookup) + +## Integration — src/app/tests/users.itest.ts (real PostgreSQL) +1. unauthenticated GET /users/:id -> 401 UNAUTHORIZED +2. register+login A and B; A GET /users/:idB -> 403 FORBIDDEN (AC1); A GET own -> 200 +3. PATCH own profile upserts; GET returns it +4. GET /users as customer -> 403 (AC2) +5. promote A to admin via SQL; GET /users -> 200 with items (AC3) +6. address CRUD end-to-end for own addresses: POST 201, GET list, PATCH 200, DELETE 204 (AC4) +7. A cannot POST/PATCH/DELETE addresses on B's resource -> 403 +8. PATCH/DELETE unknown address id on own resource -> 404 +9. invalid address uuid param -> 400 VALIDATION_ERROR +10. F-005 regression: login still 200 + cookie; /auth routes intact diff --git a/work/artifacts/F-006/architect.md b/work/artifacts/F-006/architect.md new file mode 100644 index 0000000..eef7b07 --- /dev/null +++ b/work/artifacts/F-006/architect.md @@ -0,0 +1,32 @@ +# Architect — F-006 Users: profile, addresses, RBAC + +done -> work/artifacts/F-006/architect.md + +## Deliverables +- specs/F-006-users-rbac/SPEC.md, DESIGN.md, TASKS.md, TESTS.md + +## Key decisions +1. **Cross-module auth por inyección, no por imports**: R1 prohíbe que `users` importe + `identity`. identity exporta `createSessionAuthenticator` por su index; el + composition root lo crea y lo inyecta en `registerUsersRoutes`. El contrato + (`CurrentUser`, `Authenticate`, `requireRole`) vive en `src/shared/auth.ts`. +2. **Role en identity_users** (migración 003): fuente única de verdad de autorización, + viaja con el usuario autenticado, imposible de escalar vía API (register siempre + crea 'customer'; solo DB/migración cambia roles). +3. **Authz antes que existencia**: no-owner recibe 403 sin importar si el recurso + existe (AC1 literal; sin enumeración). +4. **Tablas users_profiles / users_addresses** con FK a identity_users solo como + integridad de schema; queries runtime tocan únicamente tablas `users_*` (regla de + prefijo intacta). +5. **Admin endpoint dentro de users**: GET /users lista profiles (tabla propia). + Nada obliga a users a leer identity_users en runtime. +6. **Cero dependencias nuevas**. + +## Security posture +- Session resolution server-side (hash de token, expiración y revocación en SQL). +- Owner-or-admin en cada ruta; admin-only con requireRole. +- Role jamás proviene del cliente: sale del JOIN sessions+users en DB. + +## Risks +- Profiles lazy (no auto-create en register): GET /users lista solo profiles + existentes. Aceptado para el slice; documentado. diff --git a/work/artifacts/F-006/documenter.md b/work/artifacts/F-006/documenter.md new file mode 100644 index 0000000..3121d88 --- /dev/null +++ b/work/artifacts/F-006/documenter.md @@ -0,0 +1,29 @@ +done -> work/artifacts/F-006/documenter.md + +# F-006 — Documentation + +Agent: documenter +Stage: document +Feature: F-006 + +## Updated + +- `project/README.md`: + - Authentication table: register/login responses now include `role`. + - New section "Users and RBAC (users module)": endpoint table with access + rules, 401/403/404 semantics, anti-enumeration note, SQL scoping note, + `GET /users` listing semantics, and the injected-`Authenticate` boundary + contract. + - Layout: `users/` module added to the tree. + - HTTP contract: auth codes now include `UNAUTHORIZED` (401) and + `FORBIDDEN` (403). +- Specs `specs/F-006-users-rbac/` (SPEC/DESIGN/TASKS/TESTS) were authored in + the design stage and match the delivered implementation, except the + documented deviation noted in `work/artifacts/F-006/implementer.md` + (ownership unit tests merged into `shared/tests/auth.test.ts`). + +## Not needed + +- `spec/tech.md`: no new dependencies this ticket. +- `spec/roadmap.md`: F-006 already listed in Wave 1; no status field tracked + there. diff --git a/work/artifacts/F-006/implementer.md b/work/artifacts/F-006/implementer.md new file mode 100644 index 0000000..ca54714 --- /dev/null +++ b/work/artifacts/F-006/implementer.md @@ -0,0 +1,105 @@ +done -> work/artifacts/F-006/implementer.md + +# F-006 — Users: profile, addresses, RBAC (Build) + +Agent: implementer +Stage: build +Feature: F-006 + +## Scope delivered + +Profile + address management with owner-or-admin authorization, on top of the +F-005 identity foundation. No new npm dependencies were added. + +## Files + +### Migrations (immutable; new migrations only) +- `project/migrations/003_identity_roles.js` — adds `identity_users.role` + (`customer` | `admin`, default `customer`, NOT NULL) + index. +- `project/migrations/004_users.js` — `users_profiles` (PK `user_id` FK → + `identity_users`, `display_name`, `phone`, timestamps) and + `users_addresses` (`id` uuid PK, `user_id` FK ON DELETE CASCADE, address + fields, `is_default`, timestamps, `user_id` index). + +### Shared auth contract +- `project/src/shared/auth.ts` — `Role`, `CurrentUser`, `Authenticate`, + `requireRole`, `requireOwnerOrAdmin`. Shared (not module-owned) so both + identity and users use one authorization vocabulary without cross-module + imports. + +### Identity additions +- `project/src/modules/identity/infrastructure/session-authenticator.ts` — + resolves the session cookie into `CurrentUser` from the DB (expiry + + revocation enforced in SQL). Exported through the module index so the + composition root can inject it. +- Role added to `User`, `PgUserRepository` (SELECT/RETURNING `role`), and the + register/login responses. + +### Users module (hexagonal) +- `domain/profile.ts`, `domain/address.ts`, `domain/ports.ts` +- `application/profile-use-cases.ts`, `application/address-use-cases.ts` +- `infrastructure/pg-profile-repository.ts`, + `infrastructure/pg-address-repository.ts` +- `api/users.routes.ts` — registers the endpoints below. +- `index.ts` — public API. + +### Composition root +- `project/src/app/build-app.ts` — registers `@fastify/cookie` once at root + (cross-module infrastructure), wires identity routes, builds the + `Authenticate` from identity and injects it into the users routes. + +### Tests +- `project/src/shared/tests/auth.test.ts` (unit) +- `project/src/app/tests/users.itest.ts` (integration) +- `project/src/infrastructure/db/tests/migrations.itest.ts` — full rollback now + asserts `users_addresses`, `users_profiles`, identity and baseline all drop. + +## Endpoints + +| Method | Path | Auth | +|--------|------|------| +| GET | /users | admin only | +| GET | /users/:id | owner or admin | +| PATCH | /users/:id | owner or admin | +| GET | /users/:id/addresses | owner or admin | +| POST | /users/:id/addresses | owner or admin | +| PATCH | /users/:id/addresses/:addressId | owner or admin | +| DELETE | /users/:id/addresses/:addressId | owner or admin | + +Authorization runs BEFORE existence checks: a non-owner gets 403 regardless of +whether the target resource exists. Address operations are additionally scoped +by `user_id` in SQL, so a caller cannot touch another user's address even with +a valid id. + +## Verification +- `npm run lint`, `npm run lint:boundaries`, `npm run typecheck`, + `npm run build`: clean. +- `npm test`: 52 passed, 22 skipped (integration skipped without + TEST_DATABASE_URL). +- `npm run test:integration`: 22 passed against real PostgreSQL 16. +- `./scripts/verify.sh`: green. +- Live smoke (`PORT=3995`): register/login 201/200; PATCH own profile 200; + GET own 200; cross-user GET 403; GET /users as customer 403; after promotion + GET /users as admin 200; address CRUD 201/204; cross-user address 403; + no-cookie GET 401. Smoke data removed afterwards. + +## Deviations from TESTS.md (documented, not silent) +- The planned `users/tests/ownership.test.ts` was merged into + `shared/tests/auth.test.ts`, because the owner-or-admin decision lives in the + shared contract (`shared/auth.ts`), not in the users module. Coverage is + equivalent. +- `@fastify/cookie` registration moved from identity's route scope to the app + root. Fastify encapsulation meant the `request.cookies` decorator was not + visible in the users scope (caused 500s); cookies are now cross-module + infrastructure registered once at the composition root. + +## Notes for gates +- Reviewer: check the SQL column whitelist in `pg-profile-repository.upsert` + and `pg-address-repository.update` (only hardcoded columns; values always + parameterized), and that every users route calls `authenticate` then + `requireOwnerOrAdmin` before touching data. +- Security: session resolution is DB-backed (revocation + expiry in SQL); role + is read from `identity_users` per request, so promotion/demotion is reflected + immediately without trusting the client. +- QA: integration suite exercises every acceptance criterion (AC1–AC4) plus the + F-005 logout regression. diff --git a/work/artifacts/F-006/leader-close.json b/work/artifacts/F-006/leader-close.json new file mode 100644 index 0000000..028327f --- /dev/null +++ b/work/artifacts/F-006/leader-close.json @@ -0,0 +1,33 @@ +{ + "feature_id": "F-006", + "agent": "leader", + "stage": "close", + "verdict": "APPROVED", + "title": "Users: profile, addresses, RBAC", + "gates": { + "review": "APPROVED", + "security": "APPROVED", + "qa": "APPROVED" + }, + "verification": { + "lint": "clean", + "boundaries": "52 files OK", + "typecheck": "clean", + "build": "clean", + "unit_tests": "52 passed, 22 skipped", + "integration_tests": "22 passed (PostgreSQL 16)", + "verify_sh": "green" + }, + "deliverables": [ + "migrations/003_identity_roles.js, migrations/004_users.js", + "src/shared/auth.ts (Role, CurrentUser, Authenticate, requireRole, requireOwnerOrAdmin)", + "src/modules/users/ (domain, application, infrastructure, api)", + "identity: session-authenticator + role in model/responses", + "app/build-app.ts: cookie plugin at root, Authenticate injection into users", + "tests: shared/tests/auth.test.ts, app/tests/users.itest.ts, migrations.itest.ts updated" + ], + "known_followups": [ + "GET /users lists only users with a profile row; an admin user-listing endpoint may need an identity-owned port later", + "Role promotion is out-of-band DB operation until an admin API exists" + ] +} diff --git a/work/artifacts/F-006/qa.json b/work/artifacts/F-006/qa.json new file mode 100644 index 0000000..b3e234a --- /dev/null +++ b/work/artifacts/F-006/qa.json @@ -0,0 +1,46 @@ +{ + "feature_id": "F-006", + "agent": "qa", + "verdict": "APPROVED", + "acceptance_criteria": [ + { + "criterion": "Given user A When A requests user B profile Then HTTP 403", + "status": "PASS", + "evidence": "users.itest.ts 'user A requesting user B profile gets 403'; live smoke: A GET B profile -> 403 FORBIDDEN envelope" + }, + { + "criterion": "Given customer role When admin-only endpoint called Then HTTP 403", + "status": "PASS", + "evidence": "users.itest.ts 'GET /users is admin-only: customer 403'; live smoke confirmed before promotion" + }, + { + "criterion": "Given admin role When admin-only endpoint called Then HTTP 200", + "status": "PASS", + "evidence": "Same itest after DB promotion returns 200 with items array; live smoke: A(admin) GET /users -> 200" + }, + { + "criterion": "Address CRUD works end to end for own addresses", + "status": "PASS", + "evidence": "users.itest.ts address CRUD test: POST 201, GET list, PATCH 200, DELETE 204, empty list after; validation 400 on missing fields; cross-user mutation 403; unknown address 404; malformed uuid 400" + }, + { + "criterion": "verify.sh green", + "status": "PASS", + "evidence": "./scripts/verify.sh: Orquestra verificado" + } + ], + "regression": [ + "F-005 identity flows: register/login/logout still green, including logout-kills-access regression in users.itest.ts", + "F-002 migrations: full up creates baseline+identity+users schemas; full down drops all; node-pg-migrate status consistent", + "Foundation-only app (no pool) build still works; health endpoint unaffected" + ], + "suite": { + "unit": "52 passed, 22 skipped (no TEST_DATABASE_URL path in npm test default run)", + "integration": "22 passed against real PostgreSQL 16", + "lint_boundaries_typecheck_build": "all clean" + }, + "known_limitations": [ + "GET /users lists only users with an existing profile row (documented in reviewer artifact as non-blocking)", + "Role promotion/demotion is an out-of-band DB operation in this slice; no admin API for it (scope_out: no admin UI)" + ] +} diff --git a/work/artifacts/F-006/reviewer.json b/work/artifacts/F-006/reviewer.json new file mode 100644 index 0000000..70825a7 --- /dev/null +++ b/work/artifacts/F-006/reviewer.json @@ -0,0 +1,35 @@ +{ + "feature_id": "F-006", + "agent": "reviewer", + "verdict": "APPROVED", + "reviewed_files": [ + "project/migrations/003_identity_roles.js", + "project/migrations/004_users.js", + "project/src/shared/auth.ts", + "project/src/shared/tests/auth.test.ts", + "project/src/modules/identity/infrastructure/session-authenticator.ts", + "project/src/modules/identity/infrastructure/pg-user-repository.ts", + "project/src/modules/identity/api/identity.routes.ts", + "project/src/modules/identity/index.ts", + "project/src/modules/users/domain/*", + "project/src/modules/users/application/*", + "project/src/modules/users/infrastructure/pg-profile-repository.ts", + "project/src/modules/users/infrastructure/pg-address-repository.ts", + "project/src/modules/users/api/users.routes.ts", + "project/src/app/build-app.ts", + "project/src/app/tests/users.itest.ts", + "project/src/infrastructure/db/tests/migrations.itest.ts" + ], + "checks": [ + "Module boundaries: users never imports identity; auth arrives by injection through shared/auth.ts (boundary lint green, 52 files)", + "Authorization order: every users route runs authenticate -> param validation -> requireOwnerOrAdmin BEFORE any data access; non-owner always gets 403", + "SQL ownership: address operations are scoped by user_id in every query (list/create/update/delete), so a known foreign addressId cannot be touched", + "Dynamic UPDATE built only from a hardcoded column whitelist; all values parameterized", + "Migrations additive and reversible: 003 adds role with CHECK + default, 004 owns users_* tables with FKs and ON DELETE CASCADE; full rollback verified in itest", + "Acceptance criteria AC1-AC4 each exercised by a dedicated integration test; AC5 via verify.sh" + ], + "non_blocking_observations": [ + "GET /users lists rows from users_profiles, i.e. users who have touched their profile at least once; users without a profile row are not listed. Acceptable for F-006 (AC only requires admin-only access semantics); a future admin-listing ticket should decide between an identity-owned user listing port or renaming the endpoint." + ], + "evidence": "npm test: 52 passed; npm run test:integration: 22 passed against PostgreSQL 16; lint, boundary check, typecheck, build clean; live smoke exercised all authorization paths" +} diff --git a/work/artifacts/F-006/security.json b/work/artifacts/F-006/security.json new file mode 100644 index 0000000..0c27533 --- /dev/null +++ b/work/artifacts/F-006/security.json @@ -0,0 +1,40 @@ +{ + "feature_id": "F-006", + "agent": "security", + "verdict": "APPROVED", + "threat_model": [ + { + "vector": "SQL injection", + "result": "mitigated", + "evidence": "Every query parameterized ($n). The only dynamic SQL (profile upsert, address update) builds SET clauses from a hardcoded column whitelist; user input only flows into parameter values." + }, + { + "vector": "Privilege escalation (client sets own role)", + "result": "mitigated", + "evidence": "register/login schemas accept only email+password; no route writes identity_users.role. Role is returned from DB values, never echoed from input. DB CHECK constraint limits role to customer|admin." + }, + { + "vector": "Broken access control / IDOR", + "result": "mitigated", + "evidence": "Each users route runs authenticate -> requireOwnerOrAdmin before data access; address repository scopes every query by user_id, so a valid foreign addressId is unreachable. Integration tests prove cross-user reads and mutations all return 403." + }, + { + "vector": "Resource existence enumeration", + "result": "mitigated", + "evidence": "Authorization (403) is evaluated before existence checks (404) for non-owners, so a stranger cannot learn whether another user's profile exists." + }, + { + "vector": "Stale authorization (demotion/promotion)", + "result": "mitigated", + "evidence": "Session authenticator resolves role from identity_users on every request; role changes apply immediately without re-login, and no role state is trusted from the client." + }, + { + "vector": "Session security", + "result": "unchanged from F-005", + "evidence": "Opaque token in HttpOnly/SameSite=Lax cookie, SHA-256 hash stored server-side, expiry + revocation enforced in SQL. Cookie plugin moved to app root (cross-module), same policy." + } + ], + "dependencies_added": [], + "notes": "No new npm dependencies; spec/tech.md unchanged. The 403-vs-404 ordering is correct for anti-enumeration. GET /users admin list returning only profiled users is not a security issue (admin-only surface).", + "evidence": "npm run test:integration: 22 passed including forged-cookie 401, cross-user 403 battery, and logout revocation regression" +} diff --git a/work/current.md b/work/current.md index af23020..4e4902e 100644 --- a/work/current.md +++ b/work/current.md @@ -1,16 +1,16 @@ # Current work - Active feature: none (idle) -- Last closed: F-005 — Identity: register, login, sessions -- Next suggested: F-006 — Users: profile, addresses, RBAC (depends on F-005, satisfied) +- Last closed: F-006 — Users: profile, addresses, RBAC +- Next suggested: F-007 — check `backlog/features.json` for the first `pending` ticket whose dependencies are satisfied - Runtime status: reset via scripts/agent_status.py - verify.sh: green at close -## F-005 closure notes -- Identity module hexagonal: domain/application/infrastructure/api under src/modules/identity -- Sessions server-side: cookie carries opaque 512-bit token; identity_sessions stores only SHA-256 hash -- argon2id (OWASP params) behind PasswordHasher port; timing equalized 401 (no enumeration) -- Rate limit: 10 consecutive failures per email -> 429 + Retry-After (15 min cooldown), in-memory behind interface -- Cookie: HttpOnly + Secure (COOKIE_SECURE, default true) + SameSite=Lax -- Migration 002_identity reversible; integration suite updated for full revert (count:0) -- Gates: reviewer/security/qa APPROVED. Commits include specs, artifacts, code, docs. +## F-006 closure notes +- Users module hexagonal: profile + address CRUD behind use cases; tables `users_profiles`, `users_addresses` +- Roles customer/admin on `identity_users` (migration 003); role resolved from DB on every request +- Shared auth contract (`src/shared/auth.ts`): `Authenticate`, `requireRole`, `requireOwnerOrAdmin` — users never imports identity; composition root injects the authenticator +- Authorization runs before existence checks (403 first, no enumeration); address SQL scoped by user_id +- `@fastify/cookie` registered once at app root (cross-module infrastructure) +- No new npm dependencies; README documents endpoint table, access rules, and new auth codes (UNAUTHORIZED/FORBIDDEN) +- Gates: reviewer/security/qa APPROVED. Tests: unit 52, integration 22 (PostgreSQL 16) diff --git a/work/history.md b/work/history.md index 6a585ea..259807d 100644 --- a/work/history.md +++ b/work/history.md @@ -33,3 +33,9 @@ - Nota: review detectó falta de tests para COOKIE_SECURE; fix aplicado antes de aprobar el gate. Suite de migraciones F-002 actualizada a rollback completo (count:0) por tener ahora 2 migraciones - Deps nuevas: argon2, @fastify/cookie (justificadas en spec/tech.md) - Artefactos: work/artifacts/F-005/ + +## F-006 — Users: profile, addresses, RBAC (closed) +- Users module: profile + address CRUD (users_profiles, users_addresses, migration 004); roles on identity_users (migration 003) +- RBAC: owner-or-admin via shared/auth.ts injected from composition root; authz before existence checks; SQL scoped by user_id +- Identity: session-authenticator export + role in model/responses; @fastify/cookie moved to app root +- Zero new dependencies; tests: unit 52 + integration 22; live smoke covered 401/403/200 paths; gates APPROVED diff --git a/work/runtime-status.json b/work/runtime-status.json index 0d59fa1..17029bf 100644 --- a/work/runtime-status.json +++ b/work/runtime-status.json @@ -1,139 +1,13 @@ { - "feature_id": "F-005", + "feature_id": "F-006", "stage": "close", "agent": "leader", - "action": "F-005 cerrada con gates aprobados", + "action": "Cierre F-006", "state": "done", "next_agent": "leader", "waiting_for": "Seleccionar una feature pending y actualizar este estado", - "updated_at": "2026-08-14T20:58:24Z", + "updated_at": "2026-08-15T07:27:44Z", "timeline": [ - { - "ts": "2026-08-14T20:39:25Z", - "agent": "leader", - "stage": "intake", - "state": "done", - "message": "Intake F-005 OK" - }, - { - "ts": "2026-08-14T20:39:25Z", - "agent": "architect", - "stage": "design", - "state": "running", - "message": "Diseño módulo identity (hexagonal)" - }, - { - "ts": "2026-08-14T20:41:17Z", - "agent": "architect", - "stage": "design", - "state": "done", - "message": "Diseño F-005 aprobado" - }, - { - "ts": "2026-08-14T20:41:17Z", - "agent": "implementer", - "stage": "build", - "state": "running", - "message": "Implementando módulo identity" - }, - { - "ts": "2026-08-14T20:51:41Z", - "agent": "implementer", - "stage": "build", - "state": "done", - "message": "Build F-005 completo con evidencia" - }, - { - "ts": "2026-08-14T20:53:01Z", - "agent": "implementer", - "stage": "build", - "state": "running", - "message": "Fix de review: cobertura COOKIE_SECURE" - }, - { - "ts": "2026-08-14T20:54:22Z", - "agent": "implementer", - "stage": "build", - "state": "done", - "message": "Fix de review aplicado y verificado" - }, - { - "ts": "2026-08-14T20:54:22Z", - "agent": "reviewer", - "stage": "review_gate", - "state": "running", - "message": "Review gate F-005" - }, - { - "ts": "2026-08-14T20:54:44Z", - "agent": "reviewer", - "stage": "review_gate", - "state": "done", - "message": "Review gate APPROVED" - }, - { - "ts": "2026-08-14T20:55:17Z", - "agent": "security", - "stage": "security_gate", - "state": "running", - "message": "Security gate F-005" - }, - { - "ts": "2026-08-14T20:55:17Z", - "agent": "security", - "stage": "security_gate", - "state": "done", - "message": "Security gate APPROVED" - }, - { - "ts": "2026-08-14T20:55:17Z", - "agent": "qa", - "stage": "qa_gate", - "state": "running", - "message": "QA gate F-005" - }, - { - "ts": "2026-08-14T20:55:36Z", - "agent": "qa", - "stage": "qa_gate", - "state": "done", - "message": "QA gate APPROVED" - }, - { - "ts": "2026-08-14T20:55:36Z", - "agent": "documenter", - "stage": "document", - "state": "running", - "message": "Documentación F-005" - }, - { - "ts": "2026-08-14T20:55:42Z", - "agent": "implementer", - "stage": "build", - "state": "running", - "message": "Docs README: auth contract" - }, - { - "ts": "2026-08-14T20:57:22Z", - "agent": "implementer", - "stage": "build", - "state": "done", - "message": "README auth contract documentado" - }, - { - "ts": "2026-08-14T20:57:22Z", - "agent": "documenter", - "stage": "document", - "state": "running", - "message": "Documentación F-005" - }, - { - "ts": "2026-08-14T20:58:18Z", - "agent": "documenter", - "stage": "document", - "state": "done", - "message": "Docs F-005 completas" - }, { "ts": "2026-08-14T20:58:24Z", "agent": "leader", @@ -147,6 +21,132 @@ "stage": "close", "state": "done", "message": "F-005 cerrada con gates aprobados" + }, + { + "ts": "2026-08-15T07:04:47Z", + "agent": "leader", + "stage": "intake", + "state": "running", + "message": "Intake F-006" + }, + { + "ts": "2026-08-15T07:04:48Z", + "agent": "leader", + "stage": "intake", + "state": "done", + "message": "Intake F-006 OK" + }, + { + "ts": "2026-08-15T07:04:48Z", + "agent": "architect", + "stage": "design", + "state": "running", + "message": "Diseño users + RBAC" + }, + { + "ts": "2026-08-15T07:07:20Z", + "agent": "architect", + "stage": "design", + "state": "done", + "message": "Diseño F-006 aprobado" + }, + { + "ts": "2026-08-15T07:07:20Z", + "agent": "implementer", + "stage": "build", + "state": "running", + "message": "Implementando users + RBAC" + }, + { + "ts": "2026-08-15T07:22:07Z", + "agent": "implementer", + "stage": "build", + "state": "done", + "message": "Build F-006 completo: users + RBAC, tests y verify verdes" + }, + { + "ts": "2026-08-15T07:22:07Z", + "agent": "reviewer", + "stage": "review_gate", + "state": "running", + "message": "Revisando users + RBAC" + }, + { + "ts": "2026-08-15T07:23:01Z", + "agent": "reviewer", + "stage": "review_gate", + "state": "done", + "message": "Review gate F-006 APPROVED" + }, + { + "ts": "2026-08-15T07:23:01Z", + "agent": "security", + "stage": "security_gate", + "state": "running", + "message": "Audit users + RBAC" + }, + { + "ts": "2026-08-15T07:23:41Z", + "agent": "security", + "stage": "security_gate", + "state": "done", + "message": "Security gate F-006 APPROVED" + }, + { + "ts": "2026-08-15T07:23:41Z", + "agent": "qa", + "stage": "qa_gate", + "state": "running", + "message": "Validación users + RBAC" + }, + { + "ts": "2026-08-15T07:24:24Z", + "agent": "qa", + "stage": "qa_gate", + "state": "done", + "message": "QA gate F-006 APPROVED" + }, + { + "ts": "2026-08-15T07:24:24Z", + "agent": "implementer", + "stage": "build", + "state": "running", + "message": "Actualización README requerida por document" + }, + { + "ts": "2026-08-15T07:25:35Z", + "agent": "implementer", + "stage": "build", + "state": "done", + "message": "README users + RBAC documentado" + }, + { + "ts": "2026-08-15T07:25:35Z", + "agent": "documenter", + "stage": "document", + "state": "running", + "message": "Cerrando documentación" + }, + { + "ts": "2026-08-15T07:25:55Z", + "agent": "documenter", + "stage": "document", + "state": "done", + "message": "Docs F-006 completas" + }, + { + "ts": "2026-08-15T07:25:56Z", + "agent": "leader", + "stage": "close", + "state": "running", + "message": "Verificación final antes de cerrar" + }, + { + "ts": "2026-08-15T07:27:44Z", + "agent": "leader", + "stage": "close", + "state": "done", + "message": "F-006 cerrada con gates aprobados" } ] }