fix(users): invalidate sessions on password change (issue #94 bug 5)

This commit is contained in:
Zac Gaetano 2026-05-26 07:39:14 -04:00
parent 6ee284e3f6
commit 3ebe5d6639

View file

@ -74,6 +74,7 @@ router.patch('/:id', async (req, res, next) => {
try {
const { display_name, role, password } = req.body;
const sets = []; const vals = [];
let passwordChanged = false;
if (display_name !== undefined) {
sets.push(`display_name = $${sets.length + 1}`);
@ -91,6 +92,7 @@ router.patch('/:id', async (req, res, next) => {
const hashed = await bcrypt.hash(password, 12);
sets.push(`password_hash = $${sets.length + 1}`);
vals.push(hashed);
passwordChanged = true;
}
if (!sets.length) return res.status(400).json({ error: 'Nothing to update' });
@ -101,6 +103,61 @@ router.patch('/:id', async (req, res, next) => {
vals
);
if (!rows.length) return res.status(404).json({ error: 'User not found' });
// BUG FIX #5: Invalidate all active sessions for this user when their
// password is changed. Without this, an attacker who has already stolen a
// session cookie retains access even after the password is rotated, and a
// user who changes their own password doesn't log out other devices.
//
// Implementation note: express-session stores sessions keyed by session ID,
// not by userId. The standard way to invalidate by userId is to query the
// session store. We support two common stores:
//
// 1. connect-pg-simple (Postgres): DELETE FROM sessions WHERE …
// 2. connect-redis (Redis): requires iterating keys (expensive).
//
// We use a best-effort approach: if the session store exposes a `db`
// (pg-simple) we DELETE directly. Otherwise we log a warning — operators
// should configure a session store that supports this.
if (passwordChanged && req.sessionStore) {
try {
const store = req.sessionStore;
if (typeof store.query === 'function') {
// connect-pg-simple exposes the pool as store.pool / store.client
// The session data is a JSON blob; we match on the userId field.
const pgPool = store.pool || store.client;
if (pgPool) {
await pgPool.query(
`DELETE FROM sessions WHERE sess->>'userId' = $1`,
[req.params.id]
);
console.log(`[users] Invalidated sessions for user ${req.params.id} after password change`);
}
} else if (typeof store.client === 'object' && typeof store.client.keys === 'function') {
// connect-redis: scan for session keys containing this userId.
// This is O(n) over all sessions — acceptable for small deployments.
const prefix = store.prefix || 'sess:';
const keys = await store.client.keys(`${prefix}*`);
for (const key of keys) {
try {
const raw = await store.client.get(key);
if (!raw) continue;
const data = JSON.parse(raw);
if (String(data.userId) === String(req.params.id)) {
await store.client.del(key);
}
} catch { /* skip malformed sessions */ }
}
console.log(`[users] Invalidated Redis sessions for user ${req.params.id} after password change`);
} else {
console.warn('[users] Session store does not support programmatic invalidation — existing sessions for this user remain valid after password change');
}
} catch (sessionErr) {
// Non-fatal: the password is already changed; just log the failure.
console.error('[users] Failed to invalidate sessions after password change:', sessionErr.message);
}
}
res.json(rows[0]);
} catch (err) { next(err); }
});