screentinker/server/services/activity.js
ScreenTinker f289609380 fix(auth): back break-glass recovery with a revocable, auditable grant
scripts/reset-admin.js mints a JWT carrying `recovery: true`, and middleware/auth.js
accepted that claim on its own with no database involvement. Three consequences:

- NOT REVOCABLE. The only way to invalidate an outstanding recovery token was to rotate
  JWT_SECRET, which logs out every user on the instance.
- NOT ENUMERABLE. Nobody could answer "is a recovery token outstanding right now?"
- NOT AUDITED. The synthetic id ('recovery-<nonce>') is not a users row, so every
  activity_log insert for it failed the user_id foreign key and was swallowed by a catch —
  a break-glass session left no trace at all.

A `recovery_grants` row per minted token turns all three around: DELETE revokes, SELECT
enumerates, expires_at bounds, and used_at + source_ip record when and from where it was
first exercised. The migration is additive and idempotent, so re-running is a no-op and a
code-only rollback just leaves an unused table.

The grant is session-scoped, NOT single-use-per-request. Recovery means many requests —
load the dashboard, list users, reset a password — so consuming the grant on the first
would make break-glass unusable, a worse outcome than the narrow replay window it closes.
Revocation and expiry are the controls; used_at is the audit stamp.

Also fixed, because it is the mechanism that hid this: logActivity now rewrites a
'recovery-*' id to a NULL user_id with the identity in `details`, so break-glass actions
are actually recorded instead of failing the FK; and a dropped audit row now logs a loud
[AUDIT-DROP] line naming the action and increments a counter, rather than vanishing into
console.error.

The token is written to a 0600 file instead of stdout — under systemd or Docker, printing
it meant journald captured a live admin credential well past its lifetime. Added --list
and --revoke-all.

In-flight recovery tokens minted before this change stop working; they live one hour and
were unrevocable, which is the problem being fixed. Minting already required a working DB,
so redeeming against one is not a new dependency.

test/session-token-resolution.test.js now mints a real grant for its recovery token, so
its assertions keep testing that break-glass is refused on those surfaces for lack of a
users row — not for the unrelated new reason that the token is invalid.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-07-26 14:49:03 -05:00

116 lines
5.3 KiB
JavaScript

const { db } = require('../db/database');
const proxyaddr = require('proxy-addr');
const { trustedProxies } = require('../config/cloudflareIps');
// Gate function: returns true when an immediate TCP peer is one we trust
// to populate forwarding headers (Cloudflare edges, loopback, link-local,
// unique-local). Mirrors what `app.set('trust proxy', trustedProxies)` does
// for X-Forwarded-For so that CF-Connecting-IP is held to the same standard.
const isTrustedPeer = proxyaddr.compile(trustedProxies);
// Resolve the real client IP for logging.
//
// Cloudflare always sets `CF-Connecting-IP` to the original client address
// when it proxies a request. We prefer that header — but only when the
// connection's immediate peer is a trusted CF/loopback address; otherwise
// any random visitor could spoof the header by hitting the origin directly.
//
// Falls back to req.ip (which Express resolves via the trust-proxy table)
// so local dev and any non-CF deployment keep working unchanged.
function getClientIp(req) {
if (!req) return null;
const cf = req.headers && req.headers['cf-connecting-ip'];
if (typeof cf === 'string' && cf.length > 0) {
const peer = req.socket && req.socket.remoteAddress;
if (peer && isTrustedPeer(peer, 0)) return cf;
}
return req.ip || null;
}
// Phase 2.2 writer-leak fix: activity_log rows now stamp workspace_id so
// tenant-scoped queries don't miss new events. Callers pass the workspace
// when known; the middleware below sources it from resolveTenancy. When
// workspaceId is null but a device_id is provided, fall back to the device's
// workspace - matches the backfill rule for consistency.
function logActivity(userId, action, details = null, deviceId = null, ipAddress = null, workspaceId = null) {
try {
// A break-glass identity ('recovery-<jti>') is synthetic and has no users row, so
// activity_log.user_id's foreign key rejects it and the row is lost — which is exactly
// why a recovery session used to leave no trail whatsoever. Record it with a NULL
// user_id and the identity in `details`, so the action IS audited.
if (typeof userId === 'string' && userId.startsWith('recovery-')) {
details = `[break-glass ${userId}] ${details || ''}`.trim();
userId = null;
}
let ws = workspaceId || null;
if (!ws && deviceId) {
const d = db.prepare('SELECT workspace_id FROM devices WHERE id = ?').get(deviceId);
ws = d?.workspace_id || null;
}
db.prepare(
'INSERT INTO activity_log (user_id, device_id, action, details, ip_address, workspace_id) VALUES (?, ?, ?, ?, ?, ?)'
).run(userId || null, deviceId || null, action, details || null, ipAddress || null, ws);
} catch (e) {
// LOUD on purpose. A silently-dropped audit row is how a break-glass session went
// unrecorded for months: the insert failed a foreign key, this catch swallowed it, and
// nothing anywhere reported that the audit trail had a hole in it. If this fires, the
// audit log is INCOMPLETE and that is worth someone's attention.
console.error(`[AUDIT-DROP] activity_log insert FAILED — the audit trail is incomplete. action=${action} user=${userId || 'null'} device=${deviceId || 'null'}: ${e.message}`);
auditDrops++;
}
}
// Count of audit rows we failed to persist, so the gap is observable rather than only
// greppable in stdout.
let auditDrops = 0;
function auditDropCount() { return auditDrops; }
function getActivity(options = {}) {
const { userId, deviceId, limit = 50, offset = 0 } = options;
let sql = `SELECT al.*, u.name as user_name, u.email as user_email
FROM activity_log al LEFT JOIN users u ON al.user_id = u.id WHERE 1=1`;
const params = [];
if (userId) { sql += ' AND al.user_id = ?'; params.push(userId); }
if (deviceId) { sql += ' AND al.device_id = ?'; params.push(deviceId); }
sql += ' ORDER BY al.created_at DESC LIMIT ? OFFSET ?';
params.push(limit, offset);
return db.prepare(sql).all(...params);
}
// Prune old activity logs (keep 90 days)
function pruneActivityLog() {
db.prepare("DELETE FROM activity_log WHERE created_at < strftime('%s','now') - (90 * 86400)").run();
}
// Express middleware to auto-log API mutations
function activityLogger(req, res, next) {
const originalJson = res.json.bind(res);
res.json = function(data) {
// Only log successful mutations
if (['POST', 'PUT', 'PATCH', 'DELETE'].includes(req.method) && res.statusCode < 400) {
const action = `${req.method} ${req.baseUrl || ''}${req.route?.path || req.path}`;
const userId = req.user?.id;
const deviceId = req.params?.id || req.params?.deviceId || req.body?.device_id;
const details = summarizeAction(req);
logActivity(userId, action, details, deviceId, getClientIp(req), req.workspaceId || null);
}
return originalJson(data);
};
next();
}
function summarizeAction(req) {
const parts = [];
if (req.body?.name) parts.push(`name: ${req.body.name}`);
if (req.body?.filename) parts.push(`file: ${req.body.filename}`);
if (req.body?.pairing_code) parts.push('device paired');
if (req.body?.plan_id) parts.push(`plan: ${req.body.plan_id}`);
if (req.file?.originalname) parts.push(`uploaded: ${req.file.originalname}`);
return parts.join(', ') || null;
}
module.exports = { logActivity, getActivity, pruneActivityLog, activityLogger, getClientIp, auditDropCount };