From b98ffc61c7b0eab5218ab92b24b7094ebbd43781 Mon Sep 17 00:00:00 2001 From: Daniel Date: Fri, 11 Sep 2026 20:41:18 +0200 Subject: [PATCH] fix: expired invitations can be cleared too, revoked ones still cannot MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An expired code is as dead as a used one and just as accounted for, so it is now deletable. The rule the code enforces is the one that matters: a code that could still be redeemed is never deleted, because that takes it off the list without taking it out of anybody's inbox — the holder keeps something that looks valid, it quietly stops working, and nothing is left to say who had it. One condition, shared by the single delete and the bulk clear: (used_at IS NOT NULL OR (revoked_at IS NULL AND expires_at <= NOW())) Written that way rather than as "used OR past its date" because the second form also catches a revoked code whose date has since passed — a row the list still labels revoked and offers no delete on, so the button and the query would have disagreed about the same row. Revoked codes keep their rows. Revoking records a decision somebody took, and a handful of them is not the clutter a pile of expired codes is. Verified against the live database across every state: active refused, used deleted, expired deleted, revoked refused, and revoked-with-a-past-date refused rather than slipping through as expired. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU --- public/js/admin.js | 30 ++++++++++++++++++------------ src/routes/adminConfig.js | 18 +++++++++--------- src/utils/registrationInvites.js | 29 +++++++++++++++++------------ test/backend-hardening.test.js | 32 +++++++++++++++++++------------- 4 files changed, 63 insertions(+), 46 deletions(-) diff --git a/public/js/admin.js b/public/js/admin.js index 622710c6..c83947d1 100644 --- a/public/js/admin.js +++ b/public/js/admin.js @@ -1765,15 +1765,20 @@ initImageSettings(); .catch(function(err) { showToast(err.message, 'error'); }); } + // Used, or expired without being redeemed. A revoked code keeps its row: it + // records a decision somebody took, and it is not cluttering anything the way + // a pile of expired codes does. + var SPENT_STATUS = ['used', 'expired']; + // Spent invitations in one go, which is what a cluttered list actually wants. // Confirmed first: it is a delete, even if everything it removes is finished. function clearUsedInvites() { - showConfirm('Delete every used invitation? Unused and revoked ones are kept.', function() { - fetch('/api/admin/invites/used', { method: 'DELETE', headers: getAuthHeaders() }) + showConfirm('Delete every used and expired invitation? Live and revoked ones are kept.', function() { + fetch('/api/admin/invites/spent', { method: 'DELETE', headers: getAuthHeaders() }) .then(function(r) { return r.json(); }) .then(function(data) { if (!data.success) throw new Error(data.error || 'Could not clear them'); - showToast('Removed ' + data.removed + ' used invitation' + (data.removed === 1 ? '' : 's'), 'success'); + showToast('Removed ' + data.removed + ' spent invitation' + (data.removed === 1 ? '' : 's'), 'success'); loadInvites(); }) .catch(function(err) { showToast(err.message, 'error'); }); @@ -1799,21 +1804,22 @@ initImageSettings(); '' + esc(row.note || '') + '' + '' + esc(when) + who + '' + (row.status === 'active' ? '' : '') + - // Delete is offered on spent codes only. One that has not been used may - // still be in somebody's inbox: taking it off this list would not take - // it out of their hands, and nothing would then say who held it. - // Revoking is what stops a live code, and it leaves the row behind. - (row.status === 'used' - ? '' + // Delete is offered on spent codes only — used, or expired unredeemed. + // One that could still be redeemed may be sitting in somebody's inbox: + // taking it off this list would not take it out of their hands, and + // nothing would then say who held it. Revoking is what stops a live + // code, and it leaves the row behind, marked. + (SPENT_STATUS.indexOf(row.status) !== -1 + ? '' : '') + ''; }).join(''); - var used = rows.filter(function(row) { return row.status === 'used'; }).length; + var spent = rows.filter(function(row) { return SPENT_STATUS.indexOf(row.status) !== -1; }).length; var clear = document.getElementById('btn-clear-used-invites'); if (clear) { - clear.hidden = used === 0; - clear.textContent = 'Clear ' + used + ' used'; + clear.hidden = spent === 0; + clear.textContent = 'Clear ' + spent + ' spent'; } } } diff --git a/src/routes/adminConfig.js b/src/routes/adminConfig.js index 57b67dad..7845692b 100644 --- a/src/routes/adminConfig.js +++ b/src/routes/adminConfig.js @@ -739,16 +739,16 @@ router.post('/invites/:id/revoke', async function(req, res) { } catch (e) { return serverError(res, 'Invite revoke', e, 'Could not revoke the invitation'); } }); -// Clearing away spent invitations. Used ones only — an unused code may still be -// in somebody's inbox, and deleting the row takes it off the list without taking -// it out of their hands, leaving nothing to say who held it. Revoke is what -// stops a live code, and it leaves the row behind, marked. -router.delete('/invites/used', async function(req, res) { +// Clearing away spent invitations — used, or expired without being used. A code +// that could still be redeemed is never deleted: that would take it off the list +// without taking it out of anybody's inbox, leaving nothing to say who held it. +// Revoke is what stops a live code, and it leaves the row behind, marked. +router.delete('/invites/spent', async function(req, res) { try { - var removed = await invites.removeUsed(); - logger.audit(req.user.id, 'invite_delete', 'Cleared ' + removed + ' used invitations', req, { category: 'admin' }); + var removed = await invites.removeSpent(); + logger.audit(req.user.id, 'invite_delete', 'Cleared ' + removed + ' spent invitations', req, { category: 'admin' }); res.json({ success: true, removed: removed }); - } catch (e) { return serverError(res, 'Invite clear', e, 'Could not clear used invitations'); } + } catch (e) { return serverError(res, 'Invite clear', e, 'Could not clear spent invitations'); } }); router.delete('/invites/:id', async function(req, res) { @@ -756,7 +756,7 @@ router.delete('/invites/:id', async function(req, res) { if (!await invites.remove(req.params.id)) { // Said plainly rather than as "not found": the row is very likely there, // and the reason it cannot go is worth knowing. - return res.status(409).json({ error: 'Only a used invitation can be deleted. Revoke it instead.' }); + return res.status(409).json({ error: 'That invitation can still be used. Revoke it instead.' }); } logger.audit(req.user.id, 'invite_delete', 'Deleted invitation ' + req.params.id, req, { category: 'admin' }); res.json({ success: true }); diff --git a/src/utils/registrationInvites.js b/src/utils/registrationInvites.js index 1fe20ed2..9e7ab565 100644 --- a/src/utils/registrationInvites.js +++ b/src/utils/registrationInvites.js @@ -89,28 +89,33 @@ async function revoke(id, adminUserId) { return result.changes > 0; } +// Spent: used, or run out of time without being used. The one thing never +// deletable is a code that could still be redeemed — deleting that takes it off +// the list without taking it out of anybody's inbox, so the holder keeps +// something that looks valid, it quietly stops working, and nothing is left to +// say who had it. Revoke is what stops a live code, and it leaves the row. +// +// Written to match the status the list shows rather than as +// "used OR expires_at <= NOW()", which would also catch a revoked code whose +// date had passed — one the screen still labels revoked, with no delete on it. +var SPENT = '(used_at IS NOT NULL OR (revoked_at IS NULL AND expires_at <= NOW()))'; + /** * Delete a spent invitation. * - * Used codes only. A code that has not been used yet is one somebody may still - * be holding: deleting it takes it out of the list without taking it out of - * their inbox, and there is then no record of who it went to or why it stopped - * working. Revoke does that job — it leaves the row, marked. This is only for - * clearing away codes whose whole story is already told. - * - * Returns false for a code that is not spent, which the caller reports rather + * Returns false for one that is still live, which the caller reports rather * than treating as a missing row. */ async function remove(id) { var result = await db().run( - 'DELETE FROM registration_invites WHERE id = $1 AND used_at IS NOT NULL', [id]); + 'DELETE FROM registration_invites WHERE id = $1 AND ' + SPENT, [id]); return result.changes > 0; } // Every spent invitation at once, which is what "they clutter the list" asks -// for. Same rule: nothing unused is touched. -async function removeUsed() { - var result = await db().run('DELETE FROM registration_invites WHERE used_at IS NOT NULL'); +// for. Same rule: nothing still redeemable is touched. +async function removeSpent() { + var result = await db().run('DELETE FROM registration_invites WHERE ' + SPENT); return result.changes || 0; } @@ -153,7 +158,7 @@ module.exports = { list, revoke, remove, - removeUsed, + removeSpent, claim, inviteOnly }; diff --git a/test/backend-hardening.test.js b/test/backend-hardening.test.js index 8d57d02d..471ba477 100644 --- a/test/backend-hardening.test.js +++ b/test/backend-hardening.test.js @@ -153,28 +153,34 @@ test('.env.example documents every variable the app reads', () => { // registration_enabled is open-or-closed. Invite-only is the middle setting, // and it has to hold up against someone probing codes. -test('an invitation can only be deleted once it has been used', () => { +test('an invitation is deletable only once it can no longer be used', () => { const src = read('src/utils/registrationInvites.js'); const route = read('src/routes/adminConfig.js'); - // An unused code may still be in somebody's inbox. Deleting the row takes it - // off the list without taking it out of their hands, and nothing is then left - // to say who held it or why it stopped working. Revoke does that job and - // leaves the row behind, marked. - assert.match(src, /DELETE FROM registration_invites WHERE id = \$1 AND used_at IS NOT NULL/); - assert.match(src, /DELETE FROM registration_invites WHERE used_at IS NOT NULL/, 'and in bulk'); + // A code that could still be redeemed may be sitting in somebody's inbox. + // Deleting the row takes it off the list without taking it out of their + // hands: it quietly stops working and nothing is left to say who held it. + // Revoke does that job and leaves the row behind, marked. + assert.match(src, /var SPENT = '\(used_at IS NOT NULL OR \(revoked_at IS NULL AND expires_at <= NOW\(\)\)\)';/); + assert.match(src, /DELETE FROM registration_invites WHERE id = \$1 AND ' \+ SPENT/); + assert.match(src, /DELETE FROM registration_invites WHERE ' \+ SPENT/, 'and in bulk'); + // Written to match the status the list shows: "used OR expired" would also + // catch a revoked code past its date, which the screen still calls revoked + // and offers no delete on. + assert.match(src, /revoked_at IS NULL AND expires_at <= NOW\(\)/); // Refused with the reason, not as a missing row: the row is very likely there. - assert.match(route, /Only a used invitation can be deleted\. Revoke it instead\./); + assert.match(route, /That invitation can still be used\. Revoke it instead\./); assert.match(route, /res\.status\(409\)/); - // The bulk route is declared before /invites/:id, or "used" is read as an id. - assert.ok(route.indexOf("router.delete('/invites/used'") < route.indexOf("router.delete('/invites/:id'")); + // The bulk route is declared before /invites/:id, or "spent" is read as an id. + assert.ok(route.indexOf("router.delete('/invites/spent'") < route.indexOf("router.delete('/invites/:id'")); // And the button is only offered where it can work. const js = read('public/js/admin.js'); - assert.match(js, /row\.status === 'used'\s*\n?\s*\? '