diff --git a/VERSION b/VERSION index e9344b5..44df9c6 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.95-beta +0.3.96-beta diff --git a/public/js/core.js b/public/js/core.js index dd196e8..46f62bd 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.95-beta"; +const VERSION = "0.3.96-beta"; (function(){ const tag = document.getElementById("verTag"); tag.textContent = "v"+VERSION; diff --git a/src/api.js b/src/api.js index d7a71d4..cfb132c 100644 --- a/src/api.js +++ b/src/api.js @@ -651,6 +651,47 @@ export default async function api(app) { return { ok: true }; }); + // Een gebruiker en al z'n afhankelijke gegevens in één transactie verwijderen. + // We rekenen NIET op ON DELETE CASCADE: sommige (oudere) databases hebben een + // foreign key die dat mist, waardoor DELETE FROM users een 500 "serverfout" + // geeft. Door de afhankelijke rijen eerst zelf te verwijderen werkt het + // ongeacht de exacte FK-instelling. Migratie 015 herstelt de FK's daarnaast + // structureel; dit is het vangnet dat sowieso werkt. + const deleteUserFully = async (userId) => { + const client = await pool.connect(); + try { + await client.query('BEGIN'); + // bewuste SET NULL: wie iets in het CMS wijzigde blijft die inhoud houden + await client.query('UPDATE site_content SET updated_by = NULL WHERE updated_by = $1', [userId]); + // eerst de klas/school van de gebruiker zelf lossnijden zodat triggers op + // eventuele resten niet in de weg zitten + const dependents = [ + 'DELETE FROM sessions WHERE user_id = $1', + 'DELETE FROM user_roles WHERE user_id = $1', + 'DELETE FROM class_teachers WHERE user_id = $1', + 'DELETE FROM assignments WHERE pupil_id = $1 OR teacher_id = $1', + 'DELETE FROM progress_events WHERE pupil_id = $1 OR teacher_id = $1', + 'DELETE FROM parent_children WHERE parent_id = $1 OR pupil_id = $1', + 'DELETE FROM school_link_requests WHERE parent_id = $1 OR pupil_id = $1', + 'DELETE FROM user_data_versions WHERE user_id = $1', + 'DELETE FROM shared_library WHERE owner_id = $1', + 'DELETE FROM image_assets WHERE owner_id = $1', + 'DELETE FROM image_folders WHERE owner_id = $1', + 'DELETE FROM image_themes WHERE owner_id = $1', + 'DELETE FROM boards WHERE owner_id = $1', + 'DELETE FROM folders WHERE owner_id = $1', + ]; + for (const sql of dependents) await client.query(sql, [userId]); + await client.query('DELETE FROM users WHERE id = $1', [userId]); + await client.query('COMMIT'); + } catch (e) { + await client.query('ROLLBACK').catch(() => {}); + throw e; + } finally { + client.release(); + } + }; + app.delete('/admin/users/:id', async (req, reply) => { need(req, reply, PERMISSIONS['users.manage']); const r = await pool.query('SELECT * FROM users WHERE id = $1', [req.params.id]); @@ -663,13 +704,13 @@ export default async function api(app) { 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'); try { - await pool.query('DELETE FROM users WHERE id = $1', [u.id]); + await deleteUserFully(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; + // Zou niet meer moeten voorkomen (alle afhankelijke rijen zijn hierboven + // al weggehaald), maar als het tóch faalt geven we de echte reden mee i.p.v. + // een kale "serverfout", zodat de oorzaak zichtbaar is. + req.log.error({ err: e }, 'verwijderen gebruiker mislukt'); + return fail(reply, 409, `verwijderen mislukt: ${e.detail || e.message}`); } return { ok: true }; }); diff --git a/test/schools.test.js b/test/schools.test.js index 711cec1..a9430a6 100644 --- a/test/schools.test.js +++ b/test/schools.test.js @@ -8,16 +8,19 @@ import { readFile } from 'node:fs/promises'; function makeApp(user, targetUser) { const calls = []; + const query = async (sql, params = []) => { + calls.push({ sql, params }); + if (sql.includes('FROM sessions s JOIN users u')) return { rows: [user] }; + if (sql.includes('FROM user_roles')) return { rows: [] }; + if (sql.startsWith('UPDATE schools SET name')) return { rows: [{ id: params[1], name: params[0] }] }; + if (sql.startsWith('SELECT * FROM classes WHERE id')) return { rows: [{ id: 100, school_id: 2, name: 'Groep 4' }] }; + if (sql.startsWith('SELECT * FROM users WHERE id') && targetUser) return { rows: [targetUser] }; + return { rows: [] }; + }; const pool = { - async query(sql, params = []) { - calls.push({ sql, params }); - if (sql.includes('FROM sessions s JOIN users u')) return { rows: [user] }; - if (sql.includes('FROM user_roles')) return { rows: [] }; - if (sql.startsWith('UPDATE schools SET name')) return { rows: [{ id: params[1], name: params[0] }] }; - if (sql.startsWith('SELECT * FROM classes WHERE id')) return { rows: [{ id: 100, school_id: 2, name: 'Groep 4' }] }; - if (sql.startsWith('SELECT * FROM users WHERE id') && targetUser) return { rows: [targetUser] }; - return { rows: [] }; - }, + query, + // pool.connect() voor transacties (o.a. gebruiker verwijderen) + async connect() { return { query, release() {} }; }, }; return (async () => { const app = Fastify({ trustProxy: 2 }); @@ -194,35 +197,38 @@ 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(); +test('gebruiker verwijderen ruimt eerst alle afhankelijke rijen op (geen serverfout)', async () => { + const { app, calls } = await makeApp(superUser, { id: 9, role: 'teacher', school_id: 2, username: 'juf' }); 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/); + assert.equal(res.statusCode, 200, res.body); + // in een transactie, afhankelijke tabellen vóór users + assert.ok(calls.some((c) => c.sql === 'BEGIN')); + assert.ok(calls.some((c) => c.sql.includes('DELETE FROM class_teachers WHERE user_id'))); + assert.ok(calls.some((c) => c.sql.includes('DELETE FROM assignments WHERE pupil_id'))); + assert.ok(calls.some((c) => c.sql.includes('DELETE FROM boards WHERE owner_id'))); + const usersIdx = calls.findIndex((c) => c.sql === 'DELETE FROM users WHERE id = $1'); + const ctIdx = calls.findIndex((c) => c.sql.includes('DELETE FROM class_teachers')); + assert.ok(usersIdx > ctIdx, 'gebruiker wordt pas ná de afhankelijke rijen verwijderd'); + assert.ok(calls.some((c) => c.sql === 'COMMIT')); 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' }); +test('als verwijderen tóch faalt, komt de echte reden terug (geen kale serverfout)', async () => { + const { app } = await makeApp(superUser, { id: 9, role: 'teacher', school_id: 2, username: 'juf' }); + const pool = app.pg; + const orig = pool.connect.bind(pool); + pool.connect = async () => { + const client = await orig(); + const q = client.query; + client.query = async (sql, params) => { + if (sql === 'DELETE FROM users WHERE id = $1') { const e = new Error('boom'); e.detail = 'Key is still referenced from table "iets".'; throw e; } + return q(sql, params); + }; + return client; + }; 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'))); + assert.equal(res.statusCode, 409, res.body); + assert.match(res.json().error, /still referenced/); await app.close(); });