From 4ffb69ca61c9fd707cdadd7211c126782b0b1aaa Mon Sep 17 00:00:00 2001 From: Ramon Date: Fri, 17 Jul 2026 22:27:20 +0200 Subject: [PATCH] fix: gebruiker verwijderen toont echte oorzaak i.p.v. kale serverfout (v0.3.97-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - DELETE /admin/users/:id: hele handler (niet alleen deleteUserFully) in één try/catch, zodat ook een fout vóór het verwijderen zelf (bv. de opzoekquery) de echte reden teruggeeft i.p.v. terug te vallen op de generieke 500 "serverfout" - generieke errorhandler logt voortaan altijd het volledige fouteobject en geeft ingelogde gebruikers de werkelijke foutmelding mee i.p.v. alleen "serverfout" - test toegevoegd die dit nieuwe pad dekt --- VERSION | 2 +- public/js/core.js | 2 +- src/api.js | 38 +++++++++++++++++++++++--------------- test/schools.test.js | 14 ++++++++++++++ 4 files changed, 39 insertions(+), 17 deletions(-) diff --git a/VERSION b/VERSION index 44df9c6..d13a08a 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.3.96-beta +0.3.97-beta diff --git a/public/js/core.js b/public/js/core.js index 46f62bd..5a27b0c 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.96-beta"; +const VERSION = "0.3.97-beta"; (function(){ const tag = document.getElementById("verTag"); tag.textContent = "v"+VERSION; diff --git a/src/api.js b/src/api.js index cfb132c..7e4eff4 100644 --- a/src/api.js +++ b/src/api.js @@ -65,8 +65,12 @@ export default async function api(app) { app.setErrorHandler((err, req, reply) => { if (reply.statusCode >= 400 && reply.statusCode < 500) return reply.send({ error: err.message }); - req.log.error(err); - reply.code(500).send({ error: 'serverfout' }); + req.log.error({ err }, 'onverwachte serverfout'); + // Voor ingelogde staf (die dit sowieso al kan zien in de detailweergave) + // geven we de echte oorzaak mee i.p.v. een kale "serverfout": anders is + // een fout als deze niet te diagnosticeren zonder in de serverlogs te kijken. + const detail = req.user ? `serverfout: ${err.message}` : 'serverfout'; + reply.code(500).send({ error: detail }); }); // ---- version -------------------------------------------------------------- @@ -694,25 +698,29 @@ export default async function api(app) { 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]); - const u = r.rows[0]; - if (!u) return fail(reply, 404, 'gebruiker onbekend'); - if (Number(u.id) === Number(req.user.id)) return fail(reply, 400, 'je kunt jezelf niet verwijderen'); - if (!sameSchool(req, u)) return fail(reply, 403, 'geen rechten'); - if (teacherOnly(req) && u.role !== 'pupil') return fail(reply, 403, 'geen rechten'); - 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'); + // Alles vanaf hier (ook de opzoek- en rechtencontroles, niet alleen de + // verwijdering zelf) in één try/catch: anders valt een fout die vóór + // deleteUserFully() optreedt (bv. een kapotte query) terug op de kale + // "serverfout" van de generieke handler i.p.v. een zichtbare reden. try { + const r = await pool.query('SELECT * FROM users WHERE id = $1', [req.params.id]); + const u = r.rows[0]; + if (!u) return fail(reply, 404, 'gebruiker onbekend'); + if (Number(u.id) === Number(req.user.id)) return fail(reply, 400, 'je kunt jezelf niet verwijderen'); + if (!sameSchool(req, u)) return fail(reply, 403, 'geen rechten'); + if (teacherOnly(req) && u.role !== 'pupil') return fail(reply, 403, 'geen rechten'); + 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 deleteUserFully(u.id); + return { ok: true }; } catch (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. + // Zou niet meer moeten voorkomen (alle afhankelijke rijen worden in + // deleteUserFully 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 }; }); // ---- toewijzingen (leerling-omgeving) ------------------------------------------ diff --git a/test/schools.test.js b/test/schools.test.js index a9430a6..45a7a06 100644 --- a/test/schools.test.js +++ b/test/schools.test.js @@ -232,6 +232,20 @@ test('als verwijderen tóch faalt, komt de echte reden terug (geen kale serverfo await app.close(); }); +test('faalt de opzoekquery vóór deleteUserFully, dan ook de echte reden i.p.v. kale serverfout', async () => { + const { app } = await makeApp(superUser, { id: 9, role: 'teacher', school_id: 2, username: 'juf' }); + const pool = app.pg; + const orig = pool.query.bind(pool); + pool.query = async (sql, params) => { + if (sql.startsWith('SELECT * FROM users WHERE id')) { const e = new Error('connection terminated'); throw e; } + return orig(sql, params); + }; + 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, /connection terminated/); + 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/);