From 5c5ad05e2aac46ce8666a28de3267e646fb42a9f Mon Sep 17 00:00:00 2001 From: peaceshallom37-rgb Date: Wed, 29 Jul 2026 21:21:28 +0000 Subject: [PATCH] fix(database): add user soft-delete and fix audit_logs FK constraint (closes #344) - Add deleted_at TIMESTAMPTZ column to users table for GDPR-compliant soft-delete - Update audit_logs.user_id FK to ON DELETE SET NULL to prevent orphaned rows - Add idx_users_deleted_at partial index for efficient soft-delete filtering - Update UsersRepository: filter WHERE deleted_at IS NULL on all read queries - Add softDelete method to UsersRepository - Implement DELETE /users/me endpoint in UsersController with token revocation - Add USER_DELETE audit action to AuditAction enum - Update integration tests to verify deleted_at column exists - Create migration 2026082001 with up/down rollback support --- api/src/audit/audit-action.enum.ts | 2 + api/src/auth/users.repository.ts | 27 ++++++++++--- api/src/database.integration.spec.ts | 1 + api/src/users/users.controller.ts | 23 +++++++++++ api/src/users/users.service.ts | 32 +++++++++++++++ database/audit_logs.sql | 3 +- .../2026082001_add_user_soft_delete.down.sql | 28 +++++++++++++ .../2026082001_add_user_soft_delete.up.sql | 39 +++++++++++++++++++ database/migrations/README.md | 1 + database/schema.sql | 12 ++++-- 10 files changed, 159 insertions(+), 9 deletions(-) create mode 100644 database/migrations/2026082001_add_user_soft_delete.down.sql create mode 100644 database/migrations/2026082001_add_user_soft_delete.up.sql diff --git a/api/src/audit/audit-action.enum.ts b/api/src/audit/audit-action.enum.ts index 4a63887..7540a51 100644 --- a/api/src/audit/audit-action.enum.ts +++ b/api/src/audit/audit-action.enum.ts @@ -24,4 +24,6 @@ export enum AuditAction { STREAM_DELETE = "stream_delete", ROLE_CHANGE = "role_change", PROFILE_UPDATE = "profile_update", + /** Issue #344: recorded when a user soft-deletes their account via DELETE /users/me */ + USER_DELETE = "user_delete", } diff --git a/api/src/auth/users.repository.ts b/api/src/auth/users.repository.ts index 6764e6f..9786f3b 100644 --- a/api/src/auth/users.repository.ts +++ b/api/src/auth/users.repository.ts @@ -9,6 +9,7 @@ export interface User { password_hash: string created_at: Date password_changed_at?: Date + deleted_at?: Date | null } /** @@ -23,7 +24,7 @@ export class UsersRepository { async findByEmail(email: string): Promise { const { rows } = await this.pool.query( - "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE email = $1", + "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE email = $1 AND deleted_at IS NULL", [email], ) return rows[0] ?? null @@ -31,7 +32,7 @@ export class UsersRepository { async findByUsername(username: string): Promise { const { rows } = await this.pool.query( - "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE username = $1", + "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE username = $1 AND deleted_at IS NULL", [username], ) return rows[0] ?? null @@ -39,7 +40,7 @@ export class UsersRepository { async findById(id: number): Promise { const { rows } = await this.pool.query( - "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE id = $1", + "SELECT id, username, email, password_hash, created_at, password_changed_at FROM users WHERE id = $1 AND deleted_at IS NULL", [id], ) return rows[0] ?? null @@ -84,7 +85,7 @@ export class UsersRepository { const { rows } = await this.pool.query( `UPDATE users SET ${sets.join(", ")} - WHERE id = $${idx} + WHERE id = $${idx} AND deleted_at IS NULL RETURNING id, username, email, password_hash, created_at, password_changed_at`, values, ) @@ -100,10 +101,26 @@ export class UsersRepository { `UPDATE users SET password_hash = $1, password_changed_at = $2 - WHERE id = $3 + WHERE id = $3 AND deleted_at IS NULL RETURNING id, username, email, password_hash, created_at, password_changed_at`, [passwordHash, passwordChangedAt, id], ) return rows[0] } + + /** + * Soft-delete a user by setting `deleted_at` to the current timestamp. + * Returns the updated user row, or null if the user was not found or + * was already deleted. + */ + async softDelete(id: number): Promise { + const { rows } = await this.pool.query( + `UPDATE users + SET deleted_at = NOW() + WHERE id = $1 AND deleted_at IS NULL + RETURNING id, username, email, password_hash, created_at, password_changed_at, deleted_at`, + [id], + ) + return rows[0] ?? null + } } diff --git a/api/src/database.integration.spec.ts b/api/src/database.integration.spec.ts index c07b82a..4d3bba6 100644 --- a/api/src/database.integration.spec.ts +++ b/api/src/database.integration.spec.ts @@ -37,6 +37,7 @@ describe("Database Integration Tests", () => { expect(columns).toContain("email") expect(columns).toContain("password_hash") expect(columns).toContain("created_at") + expect(columns).toContain("deleted_at") }) it("has the streams table with foreign key to users", async () => { diff --git a/api/src/users/users.controller.ts b/api/src/users/users.controller.ts index 0823eb7..ca99808 100644 --- a/api/src/users/users.controller.ts +++ b/api/src/users/users.controller.ts @@ -1,6 +1,7 @@ import { Body, Controller, + Delete, Get, HttpCode, HttpStatus, @@ -11,6 +12,7 @@ import { } from "@nestjs/common" import { Throttle } from "@nestjs/throttler" import { + ApiNoContentResponse, ApiOkResponse, ApiOperation, ApiTags, @@ -88,4 +90,25 @@ export class UsersController { req.header("authorization") ?? "", ) } + + @Delete("me") + @HttpCode(HttpStatus.NO_CONTENT) + @Throttle({ default: { limit: 3, ttl: 60000 } }) + @ApiOperation({ + summary: "Delete user account", + description: + "Soft-deletes the authenticated user's account. " + + "Sets a deleted_at timestamp, revokes the current token, and " + + "prevents future authentication. Compliant with GDPR right-to-erasure.", + }) + @ApiNoContentResponse({ + description: "Account deleted successfully.", + }) + async deleteUser(@Req() req: Request): Promise { + const { userId } = (req as Request & { auth: { userId: number } }).auth + return this.usersService.deleteUser( + userId, + req.header("authorization") ?? "", + ) + } } diff --git a/api/src/users/users.service.ts b/api/src/users/users.service.ts index 6d52027..0061759 100644 --- a/api/src/users/users.service.ts +++ b/api/src/users/users.service.ts @@ -1,6 +1,7 @@ import { ConflictException, Injectable, + NotFoundException, UnauthorizedException, } from "@nestjs/common" import { JwtService } from "@nestjs/jwt" @@ -9,6 +10,7 @@ import { SafeUser, toSafeUser } from "../auth/auth.service" import { TokenDenylistService } from "../auth/token-denylist.service" import { User, UsersRepository } from "../auth/users.repository" import { AuditService } from "../audit/audit.service" +import { AuditAction } from "../audit/audit-action.enum" import { ChangePasswordDto } from "./dto/change-password.dto" import { UpdateProfileDto } from "./dto/update-profile.dto" @@ -109,6 +111,36 @@ export class UsersService { } } + /** + * Soft-delete the authenticated user (GDPR-compliant account deletion). + * Sets `deleted_at` on the user row and revokes the current token so + * the session is immediately invalidated. + */ + async deleteUser( + userId: number, + authorizationHeader: string, + ): Promise { + const user = await this.usersRepository.findById(userId) + if (!user) { + throw new NotFoundException("user not found") + } + + const deleted = await this.usersRepository.softDelete(userId) + if (!deleted) { + throw new NotFoundException("user not found or already deleted") + } + + const token = this.extractBearerToken(authorizationHeader) + await this.tokenDenylistService.revoke(token, 3600) + + await this.auditService.log( + userId, + AuditAction.USER_DELETE, + { email: user.email }, + "system", + ) + } + private signToken(user: User): string { return this.jwtService.sign({ sub: user.id, diff --git a/database/audit_logs.sql b/database/audit_logs.sql index a599e7a..4a74795 100644 --- a/database/audit_logs.sql +++ b/database/audit_logs.sql @@ -1,7 +1,8 @@ CREATE TABLE IF NOT EXISTS audit_logs ( id SERIAL PRIMARY KEY, - user_id INTEGER REFERENCES users(id), + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, action VARCHAR(100) NOT NULL, + metadata JSONB DEFAULT '{}', ip VARCHAR(45), created_at TIMESTAMPTZ DEFAULT NOW() ); diff --git a/database/migrations/2026082001_add_user_soft_delete.down.sql b/database/migrations/2026082001_add_user_soft_delete.down.sql new file mode 100644 index 0000000..4939d51 --- /dev/null +++ b/database/migrations/2026082001_add_user_soft_delete.down.sql @@ -0,0 +1,28 @@ +-- Migration: 2026082001_add_user_soft_delete (DOWN) +-- +-- Reverses 2026082001_add_user_soft_delete.up.sql. +-- +-- WARNING: rolling back will permanently DROP the deleted_at column. Any +-- previously soft-deleted rows will become indistinguishable from active +-- users. Only roll back when you are certain no rows have been +-- soft-deleted, or when you are rebuilding the database from scratch. + +BEGIN; + +-- 1. Drop the index before the column it depends on. +DROP INDEX IF EXISTS idx_users_deleted_at; + +-- 2. Remove the soft-delete column. +ALTER TABLE users + DROP COLUMN IF EXISTS deleted_at; + +-- 3. Revert the audit_logs FK to its original state (no ON DELETE clause, +-- which is equivalent to NO ACTION in Postgres). +ALTER TABLE audit_logs + DROP CONSTRAINT IF EXISTS audit_logs_user_id_fkey; + +ALTER TABLE audit_logs + ADD CONSTRAINT audit_logs_user_id_fkey + FOREIGN KEY (user_id) REFERENCES users(id); + +COMMIT; diff --git a/database/migrations/2026082001_add_user_soft_delete.up.sql b/database/migrations/2026082001_add_user_soft_delete.up.sql new file mode 100644 index 0000000..91187b6 --- /dev/null +++ b/database/migrations/2026082001_add_user_soft_delete.up.sql @@ -0,0 +1,39 @@ +-- Migration: 2026082001_add_user_soft_delete (UP) +-- +-- Addresses issue #344: users table has no soft-delete — deleted users +-- leave orphaned foreign keys. +-- +-- Changes: +-- 1. Drop the existing audit_logs FK and re-create it with +-- ON DELETE SET NULL so deleting a user (or soft-deleting) does +-- not leave orphaned audit_log rows referencing a stale user_id. +-- 2. Add `users.deleted_at TIMESTAMPTZ` for GDPR-compliant soft-delete. +-- 3. Add a covering index on `deleted_at` so `WHERE deleted_at IS NULL` +-- filters remain index-friendly as the table grows. + +BEGIN; + +-- 1. Fix audit_logs FK so it does not block user deletion. +-- The auto-generated constraint name follows the Postgres convention +-- `__fkey`. +ALTER TABLE audit_logs + DROP CONSTRAINT IF EXISTS audit_logs_user_id_fkey; + +ALTER TABLE audit_logs + ADD CONSTRAINT audit_logs_user_id_fkey + FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE SET NULL; + +-- 2. Add soft-delete column. +-- NULL means "not deleted" — the vast majority of rows will stay NULL, +-- keeping the index lean and writes fast. +ALTER TABLE users + ADD COLUMN IF NOT EXISTS deleted_at TIMESTAMPTZ; + +-- 3. Index to keep soft-delete-aware queries efficient. +-- Partial index on non-deleted rows keeps the B-tree small since most +-- rows never receive a deleted_at value. +CREATE INDEX IF NOT EXISTS idx_users_deleted_at + ON users(deleted_at) + WHERE deleted_at IS NOT NULL; + +COMMIT; diff --git a/database/migrations/README.md b/database/migrations/README.md index 71864f2..5d48824 100644 --- a/database/migrations/README.md +++ b/database/migrations/README.md @@ -86,6 +86,7 @@ psql -d "$DATABASE_URL" -f database/migrations/2026051501_add_stream_tags.down.s | `2026072501_add_composite_stream_events_index.up.sql` | `idx_stream_events_stream_id_created_at_desc` — composite index for the `WHERE stream_id = ? ORDER BY created_at DESC` query pattern | | `2026072801_alter_timestamp_to_timestamptz.up.sql` | Converts all `TIMESTAMP` columns to `TIMESTAMPTZ` across every table; rewrites `DEFAULT CURRENT_TIMESTAMP` → `DEFAULT NOW()` | | `2026080501_add_stream_visibility.up.sql` | `streams.visibility` (`public` \| `private`, default `private`), CHECK constraint, supporting index | +| `2026082001_add_user_soft_delete.up.sql` | `users.deleted_at`, `idx_users_deleted_at`, `audit_logs.user_id` FK changed to `ON DELETE SET NULL` | > **Note on `2026061001` / `2026061002`:** both migrations add the same > `users.password_hash` column. `2026061001_add_password_hash` is the diff --git a/database/schema.sql b/database/schema.sql index c157f95..9c20f46 100644 --- a/database/schema.sql +++ b/database/schema.sql @@ -6,9 +6,14 @@ CREATE TABLE IF NOT EXISTS users ( username VARCHAR(255) UNIQUE NOT NULL, email VARCHAR(255) UNIQUE NOT NULL, password_hash VARCHAR(255) NOT NULL, - created_at TIMESTAMPTZ DEFAULT NOW() + created_at TIMESTAMPTZ DEFAULT NOW(), + deleted_at TIMESTAMPTZ ); +CREATE INDEX IF NOT EXISTS idx_users_deleted_at + ON users(deleted_at) + WHERE deleted_at IS NOT NULL; + -- Streams table CREATE TABLE IF NOT EXISTS streams ( id SERIAL PRIMARY KEY, @@ -184,10 +189,11 @@ CREATE INDEX IF NOT EXISTS idx_webhook_deliveries_pending_next_attempt CREATE TABLE IF NOT EXISTS audit_logs ( id SERIAL PRIMARY KEY, - user_id INTEGER REFERENCES users(id), + user_id INTEGER REFERENCES users(id) ON DELETE SET NULL, action VARCHAR(100) NOT NULL, + metadata JSONB DEFAULT '{}', ip VARCHAR(45), - created_at TIMESTAMP DEFAULT CURRENT_TIMESTAMP + created_at TIMESTAMPTZ DEFAULT NOW() ); CREATE INDEX IF NOT EXISTS idx_audit_logs_user_id ON audit_logs(user_id);