From e3036580ce32b68663cb785f86841525848d06c9 Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 17 Jul 2026 21:39:29 +0200 Subject: [PATCH] fix: gebruiker verwijderen werkt weer (herstel foreign-key cascade) (v0.3.95-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Verwijderen gaf op de live-database een 500 "serverfout". Oorzaak: op databases die zijn ontstaan vóór alle foreign keys ON DELETE CASCADE waren, houdt een tabel de oude constraint (CREATE TABLE IF NOT EXISTS werkt een bestaande constraint nooit bij), waardoor DELETE FROM users een foreign-key-fout geeft - Migratie 015: zoekt elke FK naar users(id) die niet cascade en niet set-null is en zet die alsnog op ON DELETE CASCADE (bewuste SET NULL-constraints blijven ongemoeid). Draait bij de volgende deploy en is idempotent - Verwijderroute vangt een resterende FK-fout (23503) op als een begrijpelijke 409 i.p.v. een kale serverfout - Geverifieerd: alle INSERT INTO users-paden vangen al 23505 op (gebruikersnaam uniek per school); dezelfde naam in verschillende scholen is bewust toegestaan (inloggen kiest de school) en staat sinds 0.3.94 per school gegroepeerd - 3 nieuwe servertests (FK-fout -> 409, normale verwijdering, migratie-inhoud); 97 tests groen Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_0149FgUQvuwxKKEdvQGmNngF --- VERSION | 2 +- db/015_user_fk_cascade.sql | 33 +++++++++++++++++++++++++++++++ public/js/core.js | 2 +- src/api.js | 10 +++++++++- test/schools.test.js | 40 ++++++++++++++++++++++++++++++++++++++ 5 files changed, 84 insertions(+), 3 deletions(-) create mode 100644 db/015_user_fk_cascade.sql diff --git a/VERSION b/VERSION index 9c8a451..e9344b5 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.94-beta +0.3.95-beta diff --git a/db/015_user_fk_cascade.sql b/db/015_user_fk_cascade.sql new file mode 100644 index 0000000..a6854a5 --- /dev/null +++ b/db/015_user_fk_cascade.sql @@ -0,0 +1,33 @@ +-- v0.3.95-beta: een gebruiker moet altijd te verwijderen zijn. +-- +-- Alle foreign keys die naar users(id) verwijzen horen ON DELETE CASCADE te +-- zijn (of bewust SET NULL, zoals site_content.updated_by). Databases die zijn +-- ontstaan vóór een tabel z'n cascade kreeg houden echter de oude constraint: +-- `CREATE TABLE IF NOT EXISTS` werkt een bestaande tabel nooit bij. Op zo'n +-- database geeft het verwijderen van een gebruiker een foreign-key-fout (500 +-- "serverfout"). Deze migratie zoekt elke FK naar users die NIET cascade en +-- NIET set-null is, en zet die alsnog op ON DELETE CASCADE - de bewuste +-- SET NULL-constraints blijven ongemoeid. +DO $$ +DECLARE + r RECORD; +BEGIN + FOR r IN + SELECT con.conname AS name, + con.conrelid::regclass::text AS tbl, + att.attname AS col + FROM pg_constraint con + JOIN pg_attribute att + ON att.attrelid = con.conrelid AND att.attnum = con.conkey[1] + WHERE con.contype = 'f' + AND con.confrelid = 'users'::regclass + AND array_length(con.conkey, 1) = 1 -- alle user-FK's zijn 1 kolom + AND con.confdeltype NOT IN ('c', 'n') -- niet cascade, niet set null + LOOP + EXECUTE format('ALTER TABLE %s DROP CONSTRAINT %I', r.tbl, r.name); + EXECUTE format( + 'ALTER TABLE %s ADD CONSTRAINT %I FOREIGN KEY (%I) REFERENCES users(id) ON DELETE CASCADE', + r.tbl, r.name, r.col); + RAISE NOTICE 'FK % op %(%): ON DELETE CASCADE hersteld', r.name, r.tbl, r.col; + END LOOP; +END $$; diff --git a/public/js/core.js b/public/js/core.js index 4000aec..dd196e8 100644 --- a/public/js/core.js +++ b/public/js/core.js @@ -2,7 +2,7 @@ "use strict"; /* version — shown until /api/version resolves (or if the fetch fails, e.g. offline). Kept in sync by hand with the VERSION file at the repo root on every release. */ -const VERSION = "0.3.94-beta"; +const VERSION = "0.3.95-beta"; (function(){ const tag = document.getElementById("verTag"); tag.textContent = "v"+VERSION; diff --git a/src/api.js b/src/api.js index 989438c..d7a71d4 100644 --- a/src/api.js +++ b/src/api.js @@ -662,7 +662,15 @@ export default async function api(app) { if (teacherOnly(req) && !(u.class_id && (await classOwnedByTeacher(u.class_id, req.user.id)))) return fail(reply, 403, 'groepsleiding beheert alleen leerlingen uit eigen klassen'); if (u.role === 'super' && !req.user.allRoles.includes('super')) return fail(reply, 403, 'geen rechten'); - await pool.query('DELETE FROM users WHERE id = $1', [u.id]); + try { + await pool.query('DELETE FROM users WHERE id = $1', [u.id]); + } catch (e) { + // 23503 = foreign_key_violation: een gekoppelde tabel is (nog) niet + // ON DELETE CASCADE. Migratie 015 herstelt dat; tot die tijd een + // begrijpelijke melding i.p.v. een kale "serverfout". + if (e.code === '23503') return fail(reply, 409, 'verwijderen mislukt: er hangen nog gekoppelde gegevens aan dit account'); + throw e; + } return { ok: true }; }); diff --git a/test/schools.test.js b/test/schools.test.js index d924770..711cec1 100644 --- a/test/schools.test.js +++ b/test/schools.test.js @@ -4,6 +4,7 @@ import Fastify from 'fastify'; import cookie from '@fastify/cookie'; import rateLimit from '@fastify/rate-limit'; import api from '../src/api.js'; +import { readFile } from 'node:fs/promises'; function makeApp(user, targetUser) { const calls = []; @@ -193,6 +194,45 @@ test('systeemmanager verplaatst een klas naar een andere school; losmaken kan ni await app.close(); await app2.close(); await app3.close(); }); +test('gebruiker verwijderen: FK-fout wordt een nette 409 i.p.v. serverfout', async () => { + const calls = []; + const pool = { + async query(sql, params = []) { + calls.push({ sql, params }); + if (sql.includes('FROM sessions s JOIN users u')) return { rows: [superUser] }; + if (sql.includes('FROM user_roles')) return { rows: [] }; + if (sql.startsWith('SELECT * FROM users WHERE id')) return { rows: [{ id: 9, role: 'teacher', school_id: 2, username: 'juf' }] }; + if (sql.startsWith('DELETE FROM users')) { const e = new Error('fk'); e.code = '23503'; throw e; } + return { rows: [] }; + }, + }; + const app = Fastify({ trustProxy: 2 }); + await app.register(cookie); + await app.register(rateLimit, { global: false }); + app.decorate('pg', pool); + await app.register(api, { prefix: '/api' }); + await app.ready(); + const res = await app.inject({ method: 'DELETE', url: '/api/admin/users/9', cookies }); + assert.equal(res.statusCode, 409, res.body); + assert.match(res.json().error, /gekoppelde gegevens/); + await app.close(); +}); + +test('gebruiker verwijderen slaagt normaal (DELETE uitgevoerd)', async () => { + const { app, calls } = await makeApp(superUser, { id: 9, role: 'pupil', school_id: 2, username: 'lena' }); + const res = await app.inject({ method: 'DELETE', url: '/api/admin/users/9', cookies }); + assert.equal(res.statusCode, 200, res.body); + assert.ok(calls.some((c) => c.sql.startsWith('DELETE FROM users'))); + await app.close(); +}); + +test('migratie 015 herstelt ON DELETE CASCADE voor user-foreign-keys', async () => { + const sql = await readFile('db/015_user_fk_cascade.sql', 'utf8'); + assert.match(sql, /confrelid = 'users'::regclass/); + assert.match(sql, /ON DELETE CASCADE/); + assert.match(sql, /confdeltype NOT IN \('c', 'n'\)/); +}); + test('hernoemen weigert een lege of te lange naam', async () => { const { app } = await makeApp(superUser); const res = await app.inject({ method: 'PATCH', url: '/api/admin/schools/5', cookies,