Merge pull request 'Implement web session/token revocation (#30)' (#37) from bugfix/session-revocation-30 into main

Reviewed-on: UOM/website#37
Reviewed-by: Colby Whitlock <whitlocktech@gmail.com>

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;

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

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,

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,
+}

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,
+}

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,

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,

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.' })
 }
 

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(

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()}`)
 

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', () => {