Fix three critical authorization vulnerabilities

mark-printed: was only checking the first check's account — now fetches
all check IDs upfront, verifies they all exist and share the same
account, then checks editor access once on that account.

PDF generation: was authorizing against the client-supplied account_id
but fetching checks by ID without confirming they belong to that account
— now rejects any check ID whose account_id doesn't match.

Role/account-assignment changes: active sessions for the affected user
are now deleted immediately via json_extract on the sessions table, so
demotions take effect at once rather than at session expiry (up to 7d).
This commit is contained in:
2026-03-19 22:44:11 -06:00
parent 4783cf8953
commit 0f00624e61
3 changed files with 24 additions and 9 deletions
+13 -4
View File
@@ -138,12 +138,21 @@ router.post('/mark-printed', (req, res) => {
if (!Array.isArray(ids) || ids.length === 0) { if (!Array.isArray(ids) || ids.length === 0) {
return res.status(400).json({ error: 'ids array required' }); return res.status(400).json({ error: 'ids array required' });
} }
// Verify editor access via the first check's account
const first = db.prepare('SELECT account_id FROM checks WHERE id = ?').get(ids[0]); // Fetch all checks and verify they all exist and belong to the same account
if (!first || !isEditorForAccount(req.session, first.account_id)) { const placeholders = ids.map(() => '?').join(',');
const rows = db.prepare(`SELECT id, account_id FROM checks WHERE id IN (${placeholders})`).all(...ids);
if (rows.length !== ids.length) {
return res.status(404).json({ error: 'One or more checks not found.' });
}
const accountIds = [...new Set(rows.map(r => r.account_id))];
if (accountIds.length > 1) {
return res.status(400).json({ error: 'All checks must belong to the same account.' });
}
if (!isEditorForAccount(req.session, accountIds[0])) {
return res.status(403).json({ error: 'Write access required.' }); return res.status(403).json({ error: 'Write access required.' });
} }
const placeholders = ids.map(() => '?').join(',');
db.prepare(`UPDATE checks SET printed = 1 WHERE id IN (${placeholders})`).run(...ids); db.prepare(`UPDATE checks SET printed = 1 WHERE id IN (${placeholders})`).run(...ids);
res.json({ updated: ids.length }); res.json({ updated: ids.length });
}); });
+4 -5
View File
@@ -20,24 +20,23 @@ router.post('/', async (req, res) => {
if (!Array.isArray(checkIds) || checkIds.length === 0) { if (!Array.isArray(checkIds) || checkIds.length === 0) {
return res.status(400).json({ error: 'checkIds must be a non-empty array' }); return res.status(400).json({ error: 'checkIds must be a non-empty array' });
} }
if (!isEditorForAccount(req.session, parseInt(account_id, 10))) { const resolvedAccountId = parseInt(account_id, 10);
if (!isEditorForAccount(req.session, resolvedAccountId)) {
return res.status(403).json({ error: 'Write access required.' }); return res.status(403).json({ error: 'Write access required.' });
} }
// Fetch checks in the order provided // Fetch checks in the order provided; verify each belongs to the declared account
let checks; let checks;
try { try {
checks = checkIds.map(id => { checks = checkIds.map(id => {
const check = db.prepare('SELECT * FROM checks WHERE id = ?').get(id); const check = db.prepare('SELECT * FROM checks WHERE id = ?').get(id);
if (!check) throw new Error(`Check ID ${id} not found`); if (!check) throw new Error(`Check ID ${id} not found`);
if (check.account_id !== resolvedAccountId) throw new Error(`Check ID ${id} does not belong to this account`);
return check; return check;
}); });
} catch (err) { } catch (err) {
return res.status(404).json({ error: err.message }); return res.status(404).json({ error: err.message });
} }
// Derive account from checks (all should belong to the same account)
const resolvedAccountId = account_id || checks[0].account_id;
const account = db.prepare('SELECT * FROM account WHERE id = ?').get(resolvedAccountId); const account = db.prepare('SELECT * FROM account WHERE id = ?').get(resolvedAccountId);
if (!account) { if (!account) {
return res.status(500).json({ error: 'No account configured.' }); return res.status(500).json({ error: 'No account configured.' });
+7
View File
@@ -95,6 +95,13 @@ router.put('/:id', async (req, res) => {
} }
} }
// If role or account assignments changed, invalidate all active sessions for this user
// so the new permissions take effect immediately rather than at session expiry.
if (role || Array.isArray(accounts)) {
db.prepare("DELETE FROM sessions WHERE CAST(json_extract(sess, '$.userId') AS INTEGER) = ?")
.run(parseInt(req.params.id, 10));
}
res.json(userWithAccounts(req.params.id)); res.json(userWithAccounts(req.params.id));
}); });