From 933206a1b8622df33cc94289566f75cc8eac87ec Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 21:06:50 -0500 Subject: [PATCH] Implement web session/token revocation (#30) Web sessions were stateless JWTs with no server-side store: the revocation hooks in session.service were stubs that only logged. As a result web logout was client-side only (a copied cookie stayed valid until natural JWT expiry) and a password change never invalidated existing sessions. The mobile bearer flow already had revocable, DB-stored tokens; this brings the web/cookie flow to parity. Two-layer revocation, both enforced in requireAuth (which already loads the fresh user row each request): - Per-session denylist: new `revoked_sessions` table keyed on the JWT `jti` (already minted per session). A single logout adds this session's jti; rows self-expire at the token's own exp and are pruned on boot. New model `revokedSessions` mirrors the `mobileSessions` db/model split. - Per-user cutoff: new `users.tokens_valid_after` column. A password change (and the new `invalidateSessions` helper) bumps it to NOW(); any token whose iat is at or before the cutoff is rejected. The comparison is inclusive so a token minted in the same wall-clock second as the change is still revoked. Wiring: - session.service: revokeSession / invalidateSession / invalidateAllUserSessions now delegate to the stores; sessions carry `expiresAt` (JWT exp) so logout can set a self-pruning denylist row. - /logout gains best-effort attachSession so the controller can revoke this session's jti and log auth.logout; stays a no-op for anonymous callers. - users.model.update bumps the cutoff whenever the password hash is rotated. - schema.sql: revoked_sessions table + tokens_valid_after column, added to the CREATE and to the idempotent migration block (ensureSchema on boot). Verified end-to-end against the local dev DB: a captured cookie is rejected after logout, and an existing session is rejected after a password change while re-login with the new password succeeds. Full server test suite green (96). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_019rao86n5cXpwAyjdBFEshV --- server/db/schema.sql | 21 ++++++ server/src/auth/session.middleware.js | 24 +++++++ server/src/auth/session.service.js | 63 ++++++++++++++---- .../revokedSessions/revokedSessions.db.js | 40 ++++++++++++ .../revokedSessions/revokedSessions.model.js | 29 +++++++++ server/src/model/users/users.db.js | 8 +++ server/src/model/users/users.model.js | 10 +++ server/src/router/v1/auth/auth.controller.js | 18 ++++- server/src/router/v1/auth/auth.routes.js | 6 +- server/src/server.js | 10 +++ server/test/session.test.js | 65 +++++++++++++++++-- 11 files changed, 274 insertions(+), 20 deletions(-) create mode 100644 server/src/model/revokedSessions/revokedSessions.db.js create mode 100644 server/src/model/revokedSessions/revokedSessions.model.js diff --git a/server/db/schema.sql b/server/db/schema.sql index 6a77dc6..06fdd77 100644 --- a/server/db/schema.sql +++ b/server/db/schema.sql @@ -9,6 +9,9 @@ CREATE TABLE IF NOT EXISTS users ( role ENUM('admin','editor') NOT NULL DEFAULT 'admin', totp_secret VARCHAR(64) NULL, -- base32 TOTP secret (opt-in 2FA) totp_enabled TINYINT(1) NOT NULL DEFAULT 0, + -- Any session token issued before this instant is rejected (see requireAuth). + -- Bumped on password change / "log out everywhere". NULL = no cutoff yet. + tokens_valid_after DATETIME NULL, created_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, last_login_at DATETIME NULL ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; @@ -176,6 +179,22 @@ CREATE TABLE IF NOT EXISTS mobile_refresh_tokens ( INDEX idx_mrt_expires (expires_at) ) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; +-- Denylist of revoked web/cookie session tokens, keyed on the JWT `jti` minted +-- per session in createSession. A single logout adds this session's jti here; +-- requireAuth rejects any token whose jti is present. Rows self-expire: expires_at +-- mirrors the token's own exp, after which the JWT fails verification anyway, so +-- the row is dead weight and gets pruned. "Log out everywhere" / password change +-- do NOT use this table — they bump users.tokens_valid_after instead (one row vs. +-- one-per-session). This is the web/cookie analogue of mobile_refresh_tokens. +CREATE TABLE IF NOT EXISTS revoked_sessions ( + jti CHAR(36) PRIMARY KEY, -- the session's JWT jti (uuid v4) + user_id INT NULL, + expires_at DATETIME NOT NULL, -- mirrors the token exp (prune after) + revoked_at DATETIME NOT NULL DEFAULT CURRENT_TIMESTAMP, + CONSTRAINT fk_revoked_sessions_user FOREIGN KEY (user_id) REFERENCES users(id) ON DELETE CASCADE, + INDEX idx_revoked_sessions_expires (expires_at) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; + -- Discord bot control (Phase 1). Singleton row (id = 1) holding the bot's -- config — the token is encrypted at rest (bot_token_enc) the same way OAuth -- client secrets are, and is only ever decrypted server-side to push to the @@ -357,6 +376,8 @@ CREATE TABLE IF NOT EXISTS invite_log ( -- Opt-in TOTP two-factor columns for databases created before login hardening. ALTER TABLE users ADD COLUMN IF NOT EXISTS totp_secret VARCHAR(64) NULL; ALTER TABLE users ADD COLUMN IF NOT EXISTS totp_enabled TINYINT(1) NOT NULL DEFAULT 0; +-- Session-revocation cutoff for databases created before token revocation landed. +ALTER TABLE users ADD COLUMN IF NOT EXISTS tokens_valid_after DATETIME NULL; ALTER TABLE wiki_pages ADD COLUMN IF NOT EXISTS excerpt VARCHAR(400) NULL; ALTER TABLE wiki_pages ADD COLUMN IF NOT EXISTS category_id INT NULL; diff --git a/server/src/auth/session.middleware.js b/server/src/auth/session.middleware.js index d539fac..9582127 100644 --- a/server/src/auth/session.middleware.js +++ b/server/src/auth/session.middleware.js @@ -15,6 +15,18 @@ const sessionService = require('./session.service') const users = require('../model/users/users.model') const log = require('../utils/logger')('session') +// True if this session was issued at or before the user's tokens_valid_after +// cutoff (i.e. revoked by a password change / log-out-everywhere). Both the JWT +// iat and the cutoff are second-granular, so the comparison is inclusive: a token +// minted in the same second as the bump must still be revoked (otherwise it would +// survive its full lifetime through that 1s alignment). The only cost is that a +// re-login within the same second as the change is rejected until the next second +// — a self-healing blip, and far preferable to leaving a stale token valid. +function isBeforeCutoff(session, tokensValidAfter) { + if (!tokensValidAfter || session.createdAt == null) return false + return session.createdAt <= new Date(tokensValidAfter).getTime() +} + // Best-effort: if the request carries a valid session token, attach the decoded // session (no DB hit), its auth method, and request metadata. Never rejects — // anonymous requests simply pass through with req.session undefined. @@ -38,6 +50,18 @@ async function requireAuth(req, res, next) { try { const user = await users.getById(session.userId) if (!user) return res.status(401).json({ message: 'Unauthorized' }) // deleted since token issued + + // Revocation, enforced here (not in stateless token verification): + // 1. per-user cutoff — password change / "log out everywhere" bumps + // tokens_valid_after; any token issued before it is dead. + // 2. per-session denylist — a single logout adds this jti to revoked_sessions. + if (isBeforeCutoff(session, user.tokens_valid_after)) { + return res.status(401).json({ message: 'Unauthorized' }) + } + if (await sessionService.isSessionRevoked(session.sessionId)) { + return res.status(401).json({ message: 'Unauthorized' }) + } + req.user = user req.session = session req.authMethod = session.authMethod diff --git a/server/src/auth/session.service.js b/server/src/auth/session.service.js index 547fffd..ca6cc68 100644 --- a/server/src/auth/session.service.js +++ b/server/src/auth/session.service.js @@ -14,16 +14,20 @@ // role, // authMethod, // 'local' | 'totp' | 'mobile' | 'sso' // createdAt, // ms epoch the token was issued (JWT iat) +// expiresAt, // ms epoch the token expires (JWT exp), or null // lastSeenAt, // ms epoch this session was last validated // } // -// NOTE: revocation/invalidation are stubs. JWTs are stateless, so there is no -// server-side session store yet — these are documented hook points for a future -// store (e.g. a denylist of jti, or mobile refresh-token records). +// Revocation for web/cookie sessions is backed by two stores: a per-session jti +// denylist (revoked_sessions — single logout) and a per-user cutoff +// (users.tokens_valid_after — password change / log out everywhere). requireAuth +// consults both. The functions here are the seam the controllers call. const crypto = require('crypto') const token = require('./token') +const revokedSessions = require('../model/revokedSessions/revokedSessions.model') +const users = require('../model/users/users.model') const log = require('../utils/logger')('session') // Valid authentication methods. 'local'/'totp' are the web flows; 'mobile' is the @@ -43,6 +47,7 @@ function sessionFromDecoded(decoded, now = Date.now()) { role: decoded.role, authMethod: decoded.authMethod || 'local', createdAt: decoded.iat ? decoded.iat * 1000 : null, + expiresAt: decoded.exp ? decoded.exp * 1000 : null, lastSeenAt: now, } } @@ -179,23 +184,52 @@ function sessionMeta(req) { return { ip, userAgent, deviceHash } } -// ── Revocation / invalidation (stubs) ────────────────────────────────────── -// JWTs are stateless: there is no store to revoke against yet. These are the -// hook points a future session store (jti denylist, mobile refresh records) -// will implement. They log and report success so callers can wire them in now. +// ── Revocation / invalidation ────────────────────────────────────────────── +// Web/cookie sessions are JWTs, so revocation is enforced by requireAuth reading +// two server-side stores these functions write: +// • revoked_sessions — a per-session jti denylist (single logout) +// • users.tokens_valid_after — a per-user cutoff (log out everywhere) +// A jti + its expiry (from the decoded token) are needed to denylist one session; +// invalidating all of a user's sessions only needs their id. -function revokeSession(sessionId) { - log.info('revokeSession (stub — no session store yet)', { sessionId }) +// Revoke a single session by its jti. Needs the token's expiry so the denylist +// row can self-prune once the JWT would fail verification anyway. Idempotent. +async function revokeSession(sessionId, { userId = null, expiresAt } = {}) { + if (!sessionId) { + log.warn('revokeSession called without a sessionId (jti) — nothing to revoke') + return false + } + // Fall back to the max JWT lifetime if the caller didn't pass the token's exp, + // so the denylist row still outlives any token carrying this jti. + const exp = expiresAt || Date.now() + token.cookieMaxAge() + await revokedSessions.revoke({ jti: sessionId, userId, expiresAt: exp }) + log.info('session revoked', { sessionId, userId }) return true } -function invalidateSession(sessionId) { - log.info('invalidateSession (stub — no session store yet)', { sessionId }) - return true +// Alias kept for callers that speak of "invalidating" one session. +async function invalidateSession(sessionId, opts) { + return revokeSession(sessionId, opts) } -function invalidateAllUserSessions(userId) { - log.info('invalidateAllUserSessions (stub — no session store yet)', { userId }) +// Has this session (jti) been individually revoked? Used by requireAuth on every +// authenticated request. Broad "valid after" cutoffs are checked separately by +// the middleware against the fresh user row it already loads. +async function isSessionRevoked(sessionId) { + if (!sessionId) return false + return revokedSessions.isRevoked(sessionId) +} + +// Invalidate every session a user holds (password change / log out everywhere) +// by advancing their tokens_valid_after cutoff. Covers cookie sessions issued +// before now regardless of jti. +async function invalidateAllUserSessions(userId) { + if (!userId) { + log.warn('invalidateAllUserSessions called without a userId') + return false + } + await users.invalidateSessions(userId) + log.info('all user sessions invalidated', { userId }) return true } @@ -209,6 +243,7 @@ module.exports = { sessionMeta, revokeSession, invalidateSession, + isSessionRevoked, invalidateAllUserSessions, // Mobile bearer sessions. createMobileSession, diff --git a/server/src/model/revokedSessions/revokedSessions.db.js b/server/src/model/revokedSessions/revokedSessions.db.js new file mode 100644 index 0000000..44324e2 --- /dev/null +++ b/server/src/model/revokedSessions/revokedSessions.db.js @@ -0,0 +1,40 @@ +const { query } = require('../../utils/db') + +// SQL for the revoked_sessions denylist. Rows are keyed on a session's JWT `jti` +// and carry the token's own expiry so they can be pruned once the underlying JWT +// would fail verification anyway. This is the web/cookie analogue of +// mobile_refresh_tokens (opaque, DB-stored, revocable). + +// Add a jti to the denylist. INSERT IGNORE makes a repeat logout of the same +// session a harmless no-op (the PK already exists). Returns rows changed. +async function add({ jti, userId = null, expiresAt }) { + const res = await query( + `INSERT IGNORE INTO revoked_sessions (jti, user_id, expires_at) + VALUES (?, ?, ?)`, + [jti, userId, new Date(expiresAt)], + ) + return Number(res.affectedRows || 0) +} + +// True if this jti is on the denylist and not yet past its stored expiry. Past +// expiry the token itself is already invalid, so a lingering row need not match. +async function isRevoked(jti) { + if (!jti) return false + const rows = await query( + 'SELECT 1 FROM revoked_sessions WHERE jti = ? AND expires_at > NOW() LIMIT 1', + [jti], + ) + return rows.length > 0 +} + +// Housekeeping: drop rows whose token has already expired. Returns rows removed. +async function pruneExpired() { + const res = await query('DELETE FROM revoked_sessions WHERE expires_at < NOW()') + return Number(res.affectedRows || 0) +} + +module.exports = { + add, + isRevoked, + pruneExpired, +} diff --git a/server/src/model/revokedSessions/revokedSessions.model.js b/server/src/model/revokedSessions/revokedSessions.model.js new file mode 100644 index 0000000..5177e42 --- /dev/null +++ b/server/src/model/revokedSessions/revokedSessions.model.js @@ -0,0 +1,29 @@ +// Web/cookie session denylist. Thin logic layer over revokedSessions.db — mirrors +// the users/mobileSessions split (.db = SQL, .model = the API the rest of the app +// calls). A "revoked session" is a single JWT jti added on logout; requireAuth +// checks isRevoked on every authenticated request. Broad invalidation +// ("everywhere" / password change) does NOT live here — it bumps +// users.tokens_valid_after instead. + +const db = require('./revokedSessions.db') + +// Add a session's jti to the denylist (single-session logout). Idempotent. +async function revoke({ jti, userId, expiresAt }) { + return db.add({ jti, userId, expiresAt }) +} + +// True if the given jti has been revoked (and its token hasn't expired yet). +async function isRevoked(jti) { + return db.isRevoked(jti) +} + +// Drop denylist rows whose token has already expired. +async function pruneExpired() { + return db.pruneExpired() +} + +module.exports = { + revoke, + isRevoked, + pruneExpired, +} diff --git a/server/src/model/users/users.db.js b/server/src/model/users/users.db.js index 14595b7..470469f 100644 --- a/server/src/model/users/users.db.js +++ b/server/src/model/users/users.db.js @@ -54,6 +54,13 @@ async function touchLastLogin(id) { return query('UPDATE users SET last_login_at = NOW() WHERE id = ?', [id]) } +// Move the "tokens valid after" cutoff to now, invalidating every session token +// issued before this instant (password change / log out everywhere). requireAuth +// compares each session's issued-at against this column. +async function bumpTokensValidAfter(id) { + return query('UPDATE users SET tokens_valid_after = NOW() WHERE id = ?', [id]) +} + // Store a (not-yet-enabled) TOTP secret for a user. Enabling is a separate step // so a secret is never trusted until the user has confirmed one code. async function setTotpSecret(id, secret) { @@ -78,6 +85,7 @@ module.exports = { countUsers, countAdmins, touchLastLogin, + bumpTokensValidAfter, setTotpSecret, enableTotp, disableTotp, diff --git a/server/src/model/users/users.model.js b/server/src/model/users/users.model.js index ae0b45e..de34b28 100644 --- a/server/src/model/users/users.model.js +++ b/server/src/model/users/users.model.js @@ -58,9 +58,18 @@ async function update(id, { username, password, role }) { if (role !== undefined) fields.role = role if (password) fields.password_hash = await bcrypt.hash(password, SALT_ROUNDS) await usersDb.updateUser(id, fields) + // A password change must revoke existing sessions ("change password to log + // everyone out"), so bump the cutoff whenever the hash was rotated. + if (password) await usersDb.bumpTokensValidAfter(id) return getById(id) } +// Invalidate every session token this user currently holds ("log out everywhere") +// by advancing their tokens_valid_after cutoff to now. +async function invalidateSessions(id) { + return usersDb.bumpTokensValidAfter(id) +} + async function remove(id) { return usersDb.deleteUser(id) } @@ -85,6 +94,7 @@ module.exports = { validatePassword, list, update, + invalidateSessions, remove, count, countAdmins, diff --git a/server/src/router/v1/auth/auth.controller.js b/server/src/router/v1/auth/auth.controller.js index 423bc81..089f32a 100644 --- a/server/src/router/v1/auth/auth.controller.js +++ b/server/src/router/v1/auth/auth.controller.js @@ -96,8 +96,24 @@ async function loginTotp(req, res) { } } -function logout(req, res) { +// Clear the caller's cookie AND revoke this session server-side, so a copy of the +// token (proxy log, shared machine, XSS-exfiltrated cookie) can't keep being used +// after logout. attachSession populated req.session (best-effort) with the jti + +// expiry; if there was no valid session, there's simply nothing to revoke. +async function logout(req, res) { clearAuthCookie(req, res) + try { + if (req.session?.sessionId) { + await sessionService.revokeSession(req.session.sessionId, { + userId: req.session.userId, + expiresAt: req.session.expiresAt, + }) + await activity.log({ req, userId: req.session.userId, action: 'auth.logout' }) + } + } catch (err) { + // Never fail the logout on a revocation/logging hiccup — the cookie is cleared. + log.error('logout revoke error', err) + } return res.json({ message: 'Logged out.' }) } diff --git a/server/src/router/v1/auth/auth.routes.js b/server/src/router/v1/auth/auth.routes.js index 2cfd2b3..3f238db 100644 --- a/server/src/router/v1/auth/auth.routes.js +++ b/server/src/router/v1/auth/auth.routes.js @@ -3,6 +3,7 @@ const { body } = require('express-validator') const { login, loginTotp, logout, me, HONEYPOT_FIELD } = require('./auth.controller') const { isLoggedIn } = require('../../../utils/auth') +const { attachSession } = require('../../../auth/session.middleware') const { loginLimiter } = require('../../../middleware/rateLimit') const { slowLogin, backoffGuard } = require('../../../middleware/loginProtection') const validate = require('../../../middleware/validate') @@ -65,8 +66,11 @@ authRouter.post( authRouter.post( '/logout', // #swagger.tags = ['Auth'] - // #swagger.summary = 'Log out (clear the session cookie)' + // #swagger.summary = 'Log out (clear the cookie and revoke this session)' /* #swagger.responses[200] = { description: 'Logged out', content: { "application/json": { schema: { $ref: "#/components/schemas/Message" } } } } */ + // Best-effort attach (never rejects) so the controller can revoke this session's + // jti — logout stays a no-op for an already-anonymous caller. + attachSession, logout, ) authRouter.get( diff --git a/server/src/server.js b/server/src/server.js index c61821b..4a91e1f 100644 --- a/server/src/server.js +++ b/server/src/server.js @@ -7,6 +7,7 @@ const botScore = require('./middleware/botScore') const { ensureSchema, close } = require('./utils/db') const { seedDefaults, createInitialAdminFromEnv } = require('../db/seed') const settings = require('./model/settings/settings.model') +const revokedSessions = require('./model/revokedSessions/revokedSessions.model') const mailer = require('./utils/mailer') const createLogger = require('./utils/logger') const { evaluateBotInternalKey } = require('./utils/botInternalKey') @@ -52,6 +53,15 @@ async function start() { await seedDefaults() await createInitialAdminFromEnv() + // Clear out session-denylist rows whose token has already expired (dead weight). + // Best-effort — a prune failure must never block startup. + try { + const pruned = await revokedSessions.pruneExpired() + if (pruned) log.info(`pruned ${pruned} expired revoked-session row(s)`) + } catch (err) { + log.warn('revoked-session prune failed', { error: err.message }) + } + const mode = await settings.get('site_mode') log.info(`site mode: ${String(mode || 'live').toUpperCase()}`) diff --git a/server/test/session.test.js b/server/test/session.test.js index 37bb01b..9965ae0 100644 --- a/server/test/session.test.js +++ b/server/test/session.test.js @@ -11,6 +11,8 @@ const assert = require('node:assert/strict') const sessionService = require('../src/auth/session.service') const authFacade = require('../src/utils/auth') +const revokedSessions = require('../src/model/revokedSessions/revokedSessions.model') +const usersModel = require('../src/model/users/users.model') const db = require('../src/utils/db') after(() => db.close()) @@ -36,6 +38,9 @@ test('createSession → validateSession round-trips a Session object', () => { assert.equal(session.authMethod, 'local') assert.ok(session.sessionId, 'sessionId (jti) is present') assert.equal(typeof session.createdAt, 'number') + // exp is carried so logout can set a self-pruning denylist row expiry. + assert.equal(typeof session.expiresAt, 'number') + assert.ok(session.expiresAt > session.createdAt, 'expiresAt is after createdAt') // Validating the same token off a request yields the same identity. const validated = sessionService.validateSession(reqWithCookie(token)) @@ -86,10 +91,62 @@ test('validateSession / decodeIdentity return null for missing or garbage input' assert.equal(sessionService.decodeIdentity('not-a-jwt'), null) }) -test('revoke / invalidate stubs report success without throwing', () => { - assert.equal(sessionService.revokeSession('sid-1'), true) - assert.equal(sessionService.invalidateSession('sid-1'), true) - assert.equal(sessionService.invalidateAllUserSessions(USER.id), true) +test('revokeSession denylists the jti with the token expiry', async () => { + // Stub the store (the DB is intentionally unreachable in these tests) and + // capture what the seam persists. sessionService holds the same module object, + // so overwriting the method here is what it calls. + const calls = [] + const orig = revokedSessions.revoke + revokedSessions.revoke = async (args) => { calls.push(args); return 1 } + try { + const exp = Date.now() + 60_000 + const ok = await sessionService.revokeSession('sid-1', { userId: USER.id, expiresAt: exp }) + assert.equal(ok, true) + assert.equal(calls.length, 1) + assert.equal(calls[0].jti, 'sid-1') + assert.equal(calls[0].userId, USER.id) + assert.equal(calls[0].expiresAt, exp) + } finally { + revokedSessions.revoke = orig + } +}) + +test('revokeSession is a no-op (returns false) without a sessionId', async () => { + let called = false + const orig = revokedSessions.revoke + revokedSessions.revoke = async () => { called = true; return 1 } + try { + assert.equal(await sessionService.revokeSession(undefined), false) + assert.equal(called, false, 'nothing is persisted when there is no jti') + } finally { + revokedSessions.revoke = orig + } +}) + +test('isSessionRevoked delegates to the denylist (and short-circuits on null)', async () => { + const orig = revokedSessions.isRevoked + revokedSessions.isRevoked = async (jti) => jti === 'revoked-sid' + try { + assert.equal(await sessionService.isSessionRevoked('revoked-sid'), true) + assert.equal(await sessionService.isSessionRevoked('fresh-sid'), false) + assert.equal(await sessionService.isSessionRevoked(null), false) + } finally { + revokedSessions.isRevoked = orig + } +}) + +test('invalidateAllUserSessions bumps the user cutoff (and guards a missing id)', async () => { + const ids = [] + const orig = usersModel.invalidateSessions + usersModel.invalidateSessions = async (id) => { ids.push(id); return undefined } + try { + assert.equal(await sessionService.invalidateAllUserSessions(USER.id), true) + assert.deepEqual(ids, [USER.id]) + assert.equal(await sessionService.invalidateAllUserSessions(undefined), false) + assert.deepEqual(ids, [USER.id], 'no bump when userId is missing') + } finally { + usersModel.invalidateSessions = orig + } }) test('sessionMeta derives ip / userAgent / deviceHash from the request', () => {