From e2ee4820dcc2cc1c363550aafdf5257dc26bc6cc Mon Sep 17 00:00:00 2001 From: Ramon Date: Sat, 18 Jul 2026 04:40:39 +0200 Subject: [PATCH] fix: schoolbeheerder aan klas koppelen, systeemmanager-promotie dichten, UI-afstand (v0.4.07-beta) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - db/017: de class_teachers-trigger uit migratie 006 stond alleen 'teacher' toe en blokkeerde daarmee (met een verwarrende "groepsleiding en klas..." foutmelding) wat de applicatielaag allang toestond voor schoolbeheerder en systeemmanager - de trigger is nooit meegegroeid toen die rollen later ook aan een klas gekoppeld mochten worden. Nu ook admin/super toegestaan, super school-onafhankelijk zoals de rest van de app dat al behandelt. - rolwijziging kon een bestaande groepsleiding/schoolbeheerder via een simpel dropdown-veld naar systeemmanager promoveren, zonder enige bevestiging - in tegenstelling tot de nieuwe systeemmanager-aanmaak (met wachtwoord- bevestiging). 'super' is nu geen geldig doel meer voor PATCH .../users/:id (ook niet via het extraRoles-mechanisme); het rolwijzigingsveld zelf toont zich niet meer voor een bestaande systeemmanager. Alleen de aparte aanmaakstap (met bevestiging) kan nog een systeemmanager opleveren. - meer verticale ruimte tussen rijen in het beheerpaneel (was vrijwel 0), zodat je niet per ongeluk de verkeerde rij raakt - de "＋ ..."-toevoegknoppen staan nu rechts uitgelijnd i.p.v. links, zodat ze duidelijker als aparte actie ogen i.p.v. als onderdeel van de lijst Geverifieerd in een echte headless Chromium: rolwijziging toont voor een groepsleiding alleen nog teacher/admin als opties. --- VERSION | 2 +- db/017_class_teacher_roles.sql | 44 ++++++++++++++++++++++++++++++++++ public/css/teach.css | 12 +++++++++- public/js/admin.js | 12 ++++++---- public/js/core.js | 2 +- src/api.js | 17 ++++++++----- test/schools.test.js | 38 +++++++++++++++++++++++++++++ 7 files changed, 114 insertions(+), 13 deletions(-) create mode 100644 db/017_class_teacher_roles.sql diff --git a/VERSION b/VERSION index 43f08c8..29a1e95 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -0.4.06-beta +0.4.07-beta diff --git a/db/017_class_teacher_roles.sql b/db/017_class_teacher_roles.sql new file mode 100644 index 0000000..8622e41 --- /dev/null +++ b/db/017_class_teacher_roles.sql @@ -0,0 +1,44 @@ +-- v0.4.07-beta: schoolbeheerder (en systeemmanager) konden niet aan een klas +-- gekoppeld worden, met een verwarrende "groepsleiding en klas moeten bij +-- dezelfde school horen" - ook al staat dit in de applicatielaag (POST +-- /admin/classes/:id/teachers, src/api.js) al langer voor elke stafrol open. +-- +-- Oorzaak: de trigger teach_validate_class_teacher() (uit 006) is destijds +-- geschreven toen alleen groepsleiding aan een klas gekoppeld kon worden, en +-- is nooit meegegroeid toen schoolbeheerder/systeemmanager daar later bij +-- kwamen - de trigger stond dus in de weg van iets wat de app allang toestond. +CREATE OR REPLACE FUNCTION teach_validate_class_teacher() +RETURNS trigger LANGUAGE plpgsql AS $$ +BEGIN + IF NOT EXISTS ( + SELECT 1 + FROM classes c + JOIN users u ON u.id = NEW.user_id + WHERE c.id = NEW.class_id + AND ( + -- systeemmanager is school-onafhankelijk, mag overal aan gekoppeld worden + u.role = 'super' + OR ( + u.school_id = c.school_id + AND ( + u.role IN ('teacher', 'admin') + OR EXISTS ( + SELECT 1 FROM user_roles ur + WHERE ur.user_id = u.id AND ur.role IN ('teacher', 'admin') + ) + ) + ) + ) + ) THEN + RAISE EXCEPTION 'alleen staf uit dezelfde school (of een systeemmanager) kan aan een klas gekoppeld worden'; + END IF; + RETURN NEW; +END; +$$; + +-- trigger zelf blijft ongewijzigd (verwijst naar de herdefinieerde functie), +-- alleen opnieuw aangemaakt zodat deze migratie op zichzelf leesbaar is +DROP TRIGGER IF EXISTS trg_class_teachers_tenant ON class_teachers; +CREATE TRIGGER trg_class_teachers_tenant +BEFORE INSERT OR UPDATE ON class_teachers +FOR EACH ROW EXECUTE FUNCTION teach_validate_class_teacher(); diff --git a/public/css/teach.css b/public/css/teach.css index 7d3c1e7..e47b3b1 100644 --- a/public/css/teach.css +++ b/public/css/teach.css @@ -1276,6 +1276,13 @@ } .am-schoolbar-lbl{font-size:16px;} .am-schoolbar .am-sel{flex:1; min-width:0;} +/* de dichtgeklapte "＋ ..."-knop hoort rechts, niet als volle-breedte-rij + links uitgelijnd zoals een gebruikersrij - dat maakt meteen duidelijk dat + het om een aparte actie gaat, niet om nóg een item in de lijst. Eenmaal + uitgeklapt neemt het formulier zelf wél de volle breedte (align-self niet + gezet op .am-add, dus terug naar de stretch van de kolom-flex hierboven). */ +.am-add-wrap{display:flex; flex-direction:column;} +.am-add-wrap > .tbtn{align-self:flex-end;} /* een uitgeklapt toevoegformulier moet nooit op een bestaande-gebruikersrij lijken: duidelijk eigen kader (accentkleur, stippellijn) i.p.v. de neutrale achtergrond van .am-row/.am-row-tools, plus een titel die herhaalt wát je @@ -1905,7 +1912,10 @@ body.dark .world-fact{background:#1d3b2a;color:#a9e5b7}body.dark .world-fact .wo #settingsModal .am-list{gap:0; border:1px solid var(--line); border-radius:var(--radius-s); overflow:hidden; background:var(--surface);} #settingsModal .am-row{ background:transparent; border-radius:0; box-shadow:none; - border-bottom:1px solid var(--line); margin:0; padding:7px 10px; font-size:13.5px; + /* meer verticale ruimte dan voorheen (was 7px): rijen zonder gap ertussen + stonden zo dicht op elkaar dat je makkelijk de verkeerde raakte, zeker + op een telefoon */ + border-bottom:1px solid var(--line); margin:0; padding:12px 10px; font-size:13.5px; } #settingsModal .am-list > .am-row:last-child, #settingsModal .am-list > .am-add-wrap:last-child{border-bottom:none;} diff --git a/public/js/admin.js b/public/js/admin.js index 0742762..5a215f0 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -384,11 +384,15 @@ }); tools.appendChild(field("amFieldSchool", ss)); } - if(isStaff && can("users.role.change") && u.id!==currentUser.id){ - /* één functie per gebruiker: alleen de hoofdrol is te wijzigen (super); - het aparte extra-functies-systeem is uit de UI gehaald */ + /* rolwijziging: alleen tussen groepsleiding/schoolbeheerder onderling. + Systeemmanager is hier bewust nooit een optie - die rol ontstaat alleen + via de aparte "Nieuwe systeemmanager"-aanmaak (met bevestiging van het + eigen wachtwoord), nooit door een bestaand account te promoveren. Een + bestaande systeemmanager toont dit veld daarom ook niet: er is geen + veilige weg om die rol via dit simpele dropdown-veld te wijzigen. */ + if(isStaff && u.role!=="super" && can("users.role.change") && u.id!==currentUser.id){ const rs = h("select","am-sel"); - ["teacher","admin","super"].forEach(r=>rs.appendChild(new Option(T(roleKey(r)), r))); + ["teacher","admin"].forEach(r=>rs.appendChild(new Option(T(roleKey(r)), r))); rs.value = u.role; rs.addEventListener("change", async ()=>{ try{ await api("/admin/users/"+u.id, {method:"PATCH", body:{role: rs.value}}); reload(); } diff --git a/public/js/core.js b/public/js/core.js index a2fba94..d3d3269 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.4.06-beta"; +const VERSION = "0.4.07-beta"; (function(){ const tag = document.getElementById("verTag"); tag.textContent = "v"+VERSION; diff --git a/src/api.js b/src/api.js index 42ffe26..603125a 100644 --- a/src/api.js +++ b/src/api.js @@ -661,17 +661,22 @@ export default async function api(app) { } } // rolwijziging alleen tussen staf-rollen onderling: leerlingen blijven - // leerling en ouder-accounts kunnen nooit naar een beheerrol promoveren + // leerling en ouder-accounts kunnen nooit naar een beheerrol promoveren. + // 'super' is hier bewust GEEN geldig doel: een systeemmanager aanmaken kan + // alleen via POST /admin/users, met het eigen-wachtwoord-bevestigingsstap - + // niet door een bestaande schoolbeheerder/groepsleiding via deze + // eenvoudige rolwijziging te promoveren. Demoteren van een bestaande + // systeemmanager naar admin/teacher blijft wel mogelijk. if (b.role && can(req.user.allRoles, 'users.role.change') - && ['super', 'admin', 'teacher'].includes(b.role) + && ['admin', 'teacher'].includes(b.role) && ['super', 'admin', 'teacher'].includes(u.role)) - await pool.query('UPDATE users SET role = $1, school_id = $2 WHERE id = $3', - [b.role, b.role === 'super' ? null : u.school_id, u.id]); + await pool.query('UPDATE users SET role = $1 WHERE id = $2', [b.role, u.id]); // extra functies naast de hoofdrol (bv. een teacher die ook admin-rechten krijgt) - // alleen voor staf, zelfde recht als hoofdrol-wijziging; hoofdrol zelf telt niet - // ook nog als "extra" + // ook nog als "extra". Ook hier is 'super' bewust geen toegestane extra rol, + // om dezelfde reden als hierboven. if (b.extraRoles !== undefined && can(req.user.allRoles, 'users.role.change') && u.role !== 'pupil') { - const extra = [...new Set(b.extraRoles)].filter((r) => ['super', 'admin', 'teacher'].includes(r) && r !== u.role); + const extra = [...new Set(b.extraRoles)].filter((r) => ['admin', 'teacher'].includes(r) && r !== u.role); await pool.query('DELETE FROM user_roles WHERE user_id = $1', [u.id]); for (const role of extra) { await pool.query('INSERT INTO user_roles (user_id, role) VALUES ($1, $2) ON CONFLICT DO NOTHING', [u.id, role]); diff --git a/test/schools.test.js b/test/schools.test.js index 4967c0c..ca758bb 100644 --- a/test/schools.test.js +++ b/test/schools.test.js @@ -116,6 +116,33 @@ test('ouder-achtige rol krijgt geen promotie en geen klaskoppeling', async () => await app2.close(); }); +test('een bestaande groepsleiding of schoolbeheerder kan via rolwijziging nooit systeemmanager worden', async () => { + // zelfs een systeemmanager mag dit niet via de gewone rolwijziging - alleen + // via POST /admin/users (met de eigen-wachtwoord-bevestiging) mag een + // nieuwe systeemmanager ontstaan + const { app, calls } = await makeApp(superUser, { id: 7, role: 'teacher', school_id: 2 }); + const res = await app.inject({ method: 'PATCH', url: '/api/admin/users/7', cookies, + payload: { role: 'super' } }); + assert.equal(res.statusCode, 200, res.body); + assert.ok(!calls.some((c) => c.sql.startsWith('UPDATE users SET role')), + 'promotie naar systeemmanager via rolwijziging moet genegeerd worden'); + await app.close(); + const { app: app2, calls: calls2 } = await makeApp(superUser, { id: 8, role: 'admin', school_id: 2 }); + const res2 = await app2.inject({ method: 'PATCH', url: '/api/admin/users/8', cookies, + payload: { role: 'super' } }); + assert.equal(res2.statusCode, 200, res2.body); + assert.ok(!calls2.some((c) => c.sql.startsWith('UPDATE users SET role'))); + await app2.close(); + // ook niet via het extra-rollen-mechanisme + const { app: app3, calls: calls3 } = await makeApp(superUser, { id: 8, role: 'admin', school_id: 2 }); + const res3 = await app3.inject({ method: 'PATCH', url: '/api/admin/users/8', cookies, + payload: { extraRoles: ['super', 'teacher'] } }); + assert.equal(res3.statusCode, 200, res3.body); + assert.deepEqual(res3.json().extraRoles, ['teacher']); + assert.ok(!calls3.some((c) => c.sql.startsWith('INSERT INTO user_roles') && c.params.includes('super'))); + await app3.close(); +}); + test('systeemmanager koppelt een gebruiker aan een school en maakt hem weer los', async () => { // koppelen: school bestaat -> update + klas-/toewijzing-opruiming const { app, calls } = await makeApp(superUser, { id: 7, role: 'teacher', school_id: null, username: 'juf' }); @@ -296,6 +323,17 @@ test('migratie 016 hernoemt bestaande dubbele klasnamen vóórdat de unieke inde assert.doesNotMatch(sql, /DELETE FROM classes/); }); +test('migratie 017 laat schoolbeheerder en systeemmanager ook echt via de database toe aan een klas', async () => { + // regressie: de trigger uit 006 stond alleen 'teacher' toe en blokkeerde + // daarmee (met een verwarrende foutmelding) wat de applicatielaag + // (POST /admin/classes/:id/teachers) allang toestond voor admin/super - + // een mismatch die de gemockte-pool-tests principieel nooit konden vangen. + const sql = await readFile('db/017_class_teacher_roles.sql', 'utf8'); + assert.match(sql, /u\.role = 'super'/); + assert.match(sql, /u\.role IN \('teacher', 'admin'\)/); + assert.match(sql, /CREATE OR REPLACE FUNCTION teach_validate_class_teacher/); +}); + test('klas aanmaken weigert een dubbele naam binnen dezelfde school (409)', async () => { const { app } = await makeAppWithSchool(superUser, null); const pool = app.pg;