fix: an invitation can only be deleted once it has been used
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 49s
Forgejo Docker Build / Root app tests (push) Successful in 46s
Forgejo Android APK / Build signed APK (push) Successful in 2m2s
Forgejo Docker Build / Build Docker image (push) Successful in 17s
Forgejo Docker Build / Deploy to the host (push) Failing after 1s
Some checks failed
Forgejo Android APK / Root app tests (push) Successful in 49s
Forgejo Docker Build / Root app tests (push) Successful in 46s
Forgejo Android APK / Build signed APK (push) Successful in 2m2s
Forgejo Docker Build / Build Docker image (push) Successful in 17s
Forgejo Docker Build / Deploy to the host (push) Failing after 1s
The delete button was offered on every invitation regardless of state, and the query behind it deleted any row it was given. Deleting an unused code takes it off the list without taking it out of anybody's inbox: the person still holds something that looks like a valid invitation, it silently stops working, and there is no longer a record of who it went to or why. Revoke is what stops a live code — it leaves the row behind, marked. So the delete is now for spent codes only, in three places rather than one: the query carries AND used_at IS NOT NULL, the route answers 409 with the reason instead of pretending the row is missing, and the button is rendered only on a used row. A "Clear N used" control alongside, since the complaint was clutter and clearing them one at a time is not much of an answer. Same rule — nothing unused or revoked is touched — and it confirms first, because it is still a delete. The bulk route is declared before /invites/:id, or Express reads "used" as an id. Verified against the live database: deleting an unused invitation is refused and the row survives, deleting a used one works, the bulk clear removes only used ones, and the unused probe row was still there afterwards. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01Dv6sqaY6Vq3ChZHMem3cnU
This commit is contained in:
parent
22683f3584
commit
7b084c7edf
5 changed files with 97 additions and 3 deletions
|
|
@ -48,6 +48,10 @@
|
|||
</div>
|
||||
|
||||
<div id="admin-invites-list" style="display:flex;flex-direction:column;gap:4px;max-height:340px;overflow-y:auto;"></div>
|
||||
<!-- Only ever clears spent codes. Hidden until there are some. -->
|
||||
<div style="display:flex;justify-content:flex-end;margin-top:8px;">
|
||||
<button id="btn-clear-used-invites" class="btn-sm btn-ghost" type="button" hidden style="color:var(--red);"></button>
|
||||
</div>
|
||||
|
||||
</div>
|
||||
</div>
|
||||
|
|
|
|||
|
|
@ -1688,6 +1688,7 @@ initImageSettings();
|
|||
if (revoke) inviteAction(revoke.dataset.id, 'revoke');
|
||||
var del = e.target.closest('.admin-invite-delete');
|
||||
if (del) inviteAction(del.dataset.id, 'delete');
|
||||
if (e.target.closest('#btn-clear-used-invites')) clearUsedInvites();
|
||||
var copy = e.target.closest('.admin-invite-copy');
|
||||
if (copy && navigator.clipboard) {
|
||||
navigator.clipboard.writeText(copy.dataset.code)
|
||||
|
|
@ -1764,6 +1765,21 @@ initImageSettings();
|
|||
.catch(function(err) { showToast(err.message, 'error'); });
|
||||
}
|
||||
|
||||
// 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() })
|
||||
.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');
|
||||
loadInvites();
|
||||
})
|
||||
.catch(function(err) { showToast(err.message, 'error'); });
|
||||
}, { danger: true, confirmText: 'Delete' });
|
||||
}
|
||||
|
||||
function renderInvites(rows) {
|
||||
var container = document.getElementById('admin-invites-list');
|
||||
if (!container) return;
|
||||
|
|
@ -1783,9 +1799,22 @@ initImageSettings();
|
|||
'<span style="flex:1;min-width:0;overflow-wrap:anywhere;">' + esc(row.note || '') + '</span>' +
|
||||
'<span style="font-size:11px;color:var(--g500);">' + esc(when) + who + '</span>' +
|
||||
(row.status === 'active' ? '<button type="button" class="btn-sm btn-ghost admin-invite-revoke" data-id="' + esc(String(row.id)) + '">Revoke</button>' : '') +
|
||||
'<button type="button" class="btn-sm btn-ghost admin-invite-delete" data-id="' + esc(String(row.id)) + '" style="color:var(--red);" title="Remove from this list"><i class="fas fa-trash"></i></button>' +
|
||||
// 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'
|
||||
? '<button type="button" class="btn-sm btn-ghost admin-invite-delete" data-id="' + esc(String(row.id)) + '" style="color:var(--red);" title="Delete this used invitation"><i class="fas fa-trash"></i></button>'
|
||||
: '') +
|
||||
'</div>';
|
||||
}).join('');
|
||||
|
||||
var used = rows.filter(function(row) { return row.status === 'used'; }).length;
|
||||
var clear = document.getElementById('btn-clear-used-invites');
|
||||
if (clear) {
|
||||
clear.hidden = used === 0;
|
||||
clear.textContent = 'Clear ' + used + ' used';
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -739,9 +739,25 @@ 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) {
|
||||
try {
|
||||
var removed = await invites.removeUsed();
|
||||
logger.audit(req.user.id, 'invite_delete', 'Cleared ' + removed + ' used invitations', req, { category: 'admin' });
|
||||
res.json({ success: true, removed: removed });
|
||||
} catch (e) { return serverError(res, 'Invite clear', e, 'Could not clear used invitations'); }
|
||||
});
|
||||
|
||||
router.delete('/invites/:id', async function(req, res) {
|
||||
try {
|
||||
if (!await invites.remove(req.params.id)) return res.status(404).json({ error: 'Not found' });
|
||||
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.' });
|
||||
}
|
||||
logger.audit(req.user.id, 'invite_delete', 'Deleted invitation ' + req.params.id, req, { category: 'admin' });
|
||||
res.json({ success: true });
|
||||
} catch (e) { return serverError(res, 'Invite delete', e, 'Could not delete the invitation'); }
|
||||
|
|
|
|||
|
|
@ -89,11 +89,31 @@ async function revoke(id, adminUserId) {
|
|||
return result.changes > 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* 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
|
||||
* than treating as a missing row.
|
||||
*/
|
||||
async function remove(id) {
|
||||
var result = await db().run('DELETE FROM registration_invites WHERE id = $1', [id]);
|
||||
var result = await db().run(
|
||||
'DELETE FROM registration_invites WHERE id = $1 AND used_at IS NOT NULL', [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');
|
||||
return result.changes || 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Claim a code for a registration, atomically.
|
||||
*
|
||||
|
|
@ -133,6 +153,7 @@ module.exports = {
|
|||
list,
|
||||
revoke,
|
||||
remove,
|
||||
removeUsed,
|
||||
claim,
|
||||
inviteOnly
|
||||
};
|
||||
|
|
|
|||
|
|
@ -153,6 +153,30 @@ 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', () => {
|
||||
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');
|
||||
|
||||
// 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, /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'"));
|
||||
|
||||
// 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*\? '<button type="button" class="btn-sm btn-ghost admin-invite-delete/);
|
||||
assert.match(js, /Delete every used invitation\? Unused and revoked ones are kept\./);
|
||||
assert.match(js, /clear\.hidden = used === 0;/, 'and hidden when there are none');
|
||||
});
|
||||
|
||||
test('registration invites are single-use, expiring, and safe to store', () => {
|
||||
const fs = require('node:fs');
|
||||
const path = require('node:path');
|
||||
|
|
|
|||
Loading…
Reference in a new issue