feat(security): harden .picpeak restore robustness — sessions, roles, sequences
Implements the three restore-hardening items deferred from the #811 Codex review (all validated against a real Postgres, see __tests__/integration/ picpeakRestorePg.test.js). Backend-only; targets main (feature, not a backport). 1. Global session cutoff (utils/sessionCutoff.js). A restore reassigns admin/ customer/event ids, so ANY pre-restore JWT can rebind to a different restored principal. Revoking just the importing token wasn't enough. importFromPicpeak now stamps a unix-second cutoff in app_settings after the restore commits, and adminAuth / galleryAuth / verifyGalleryAccess / customerAuth reject any token whose iat predates it (cached 30s → one in-memory compare on the hot path). The operator's forced re-login mints a token past the cutoff, so it passes. 2. Role preservation across an RBAC replace (captureOperatorRole / preserveOperatorRole). The operator's role + granted permission NAMES are captured before the wipe; after roles/role_permissions are replaced the role is resolved by NAME against the restored data, and re-created with its grants if the backup omits it — so a crafted or cross-instance backup can't silently downgrade or lock out the operator. reinjectCurrentAdmin now returns the operator's id so the row can be re-pointed at the resolved role. 3. Postgres identity-sequence resync (resyncSequences). batchInsert writes explicit ids without advancing the sequences, so the next natural insert into any restored table collided on the PK. Runs AFTER commit (setval isn't transactional) and guards every table with a column-existence check — pg_get_serial_sequence RAISES on id-less tables like role_permissions. No-op on SQLite. Tests: SQLite unit tests for the cutoff and role preservation; a gated Postgres integration suite (npm run test:pg with PICPEAK_PG_TEST_URL) covering sequence resync, the id-less-table guard, explicit-id reinject, role re-creation, and a full cross-instance replaceAllTables run asserting operator preservation, role re-establishment, FK integrity, and collision-free post-restore inserts. Stacks on #811 (shares the reinject hardening); merge after it.
This commit is contained in:
@@ -33,6 +33,13 @@ jest.mock('../utils/tokenRevocation', () => ({
|
||||
isTokenRevoked: jest.fn(),
|
||||
}));
|
||||
|
||||
// The global session cutoff (added for .picpeak restore invalidation) queries
|
||||
// app_settings; stub it to "no cutoff" so it doesn't consume this suite's
|
||||
// one-shot db() mock. Its own behaviour is covered by utils/sessionCutoff.test.js.
|
||||
jest.mock('../utils/sessionCutoff', () => ({
|
||||
isTokenBeforeCutoff: jest.fn().mockResolvedValue(false),
|
||||
}));
|
||||
|
||||
jest.mock('../utils/tokenUtils', () => ({
|
||||
getCustomerTokenFromRequest: jest.fn(),
|
||||
}));
|
||||
|
||||
@@ -2,6 +2,7 @@ const jwt = require('jsonwebtoken');
|
||||
const { db } = require('../database/db');
|
||||
const { formatBoolean } = require('../utils/dbCompat');
|
||||
const { isTokenRevoked } = require('../utils/tokenRevocation');
|
||||
const { isTokenBeforeCutoff } = require('../utils/sessionCutoff');
|
||||
const logger = require('../utils/logger');
|
||||
const { getAdminTokenFromRequest, getGalleryTokenFromRequest } = require('../utils/tokenUtils');
|
||||
|
||||
@@ -38,6 +39,13 @@ async function adminAuth(req, res, next) {
|
||||
});
|
||||
return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' });
|
||||
}
|
||||
|
||||
// Reject any session issued before the global cutoff (set by a .picpeak
|
||||
// restore, which can reassign admin ids). Forces every pre-restore admin
|
||||
// session to re-authenticate against the restored data.
|
||||
if (await isTokenBeforeCutoff(decoded)) {
|
||||
return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' });
|
||||
}
|
||||
|
||||
// Verify token type
|
||||
if (decoded.type !== 'admin') {
|
||||
@@ -157,7 +165,12 @@ async function galleryAuth(req, res, next) {
|
||||
if (await isTokenRevoked(decoded)) {
|
||||
return res.status(401).json({ error: 'Session has been invalidated', code: 'TOKEN_REVOKED' });
|
||||
}
|
||||
|
||||
|
||||
// Reject sessions issued before the global restore cutoff.
|
||||
if (await isTokenBeforeCutoff(decoded)) {
|
||||
return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' });
|
||||
}
|
||||
|
||||
// Verify token type
|
||||
if (decoded.type !== 'gallery') {
|
||||
return res.status(403).json({ error: 'Invalid access token' });
|
||||
@@ -221,6 +234,11 @@ async function photoAuth(req, res, next) {
|
||||
return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' });
|
||||
}
|
||||
|
||||
// Reject sessions issued before the global restore cutoff.
|
||||
if (await isTokenBeforeCutoff(decoded)) {
|
||||
return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' });
|
||||
}
|
||||
|
||||
// Allow both admin and gallery tokens
|
||||
if (decoded.type === 'admin') {
|
||||
const admin = await db('admin_users')
|
||||
|
||||
@@ -13,6 +13,7 @@ const jwt = require('jsonwebtoken');
|
||||
const { db } = require('../database/db');
|
||||
const { formatBoolean } = require('../utils/dbCompat');
|
||||
const { isTokenRevoked } = require('../utils/tokenRevocation');
|
||||
const { isTokenBeforeCutoff } = require('../utils/sessionCutoff');
|
||||
const logger = require('../utils/logger');
|
||||
const { getCustomerTokenFromRequest } = require('../utils/tokenUtils');
|
||||
|
||||
@@ -61,6 +62,11 @@ async function customerAuth(req, res, next) {
|
||||
return res.status(401).json({ error: 'Token has been revoked', code: 'TOKEN_REVOKED' });
|
||||
}
|
||||
|
||||
// Reject sessions issued before the global restore cutoff.
|
||||
if (await isTokenBeforeCutoff(decoded)) {
|
||||
return res.status(401).json({ error: 'Session invalidated', code: 'SESSION_INVALIDATED' });
|
||||
}
|
||||
|
||||
if (decoded.type !== 'customer') {
|
||||
logger.warn('[customerAuth] wrong token type', {
|
||||
url: req.originalUrl,
|
||||
|
||||
@@ -186,18 +186,15 @@ router.post('/picpeak/import', adminAuth, requirePermission('backup.restore'), p
|
||||
// letting a crafted .picpeak take over every admin account (GHSA-qxfx-4493-4v8f).
|
||||
const result = await importFromPicpeak({ picpeakPath, currentAdminId: req.admin && req.admin.id });
|
||||
|
||||
// The restore rewrote admin_users, so ids may have shifted. The operator's
|
||||
// current JWT is bound only to the pre-restore admin id (adminAuth trusts
|
||||
// `decoded.id` — IP is logged, not enforced, and the backup controls
|
||||
// password_changed_at), which could now resolve to a DIFFERENT restored
|
||||
// account and silently grant its permissions. Force a fresh login instead
|
||||
// of trusting the old session: revoke the token and clear the cookie.
|
||||
// Clearing the cookie is the guarantee — it drops the operator's browser
|
||||
// session unconditionally. Revocation is the extra layer that also kills a
|
||||
// Bearer-header copy of the JWT; revokeToken() swallows DB errors and
|
||||
// returns false, so check the result and log loudly if the denylist write
|
||||
// didn't land (the operator should still re-login, which the cookie clear
|
||||
// forces).
|
||||
// The restore rewrote admin_users, so ids may have shifted. importFromPicpeak
|
||||
// already stamped a GLOBAL session cutoff (see setSessionsValidAfter), so
|
||||
// every JWT issued before the restore — admin, customer, gallery — now fails
|
||||
// auth. Here we additionally give the importing admin an immediate, clean
|
||||
// logout: revoke this token and clear the cookie so their browser drops the
|
||||
// session at once rather than on the next 401. Cookie clear is the
|
||||
// unconditional guarantee; revokeToken() swallows DB errors and returns
|
||||
// false, so check the result and log loudly if the denylist write didn't
|
||||
// land (the operator still re-logs-in, which the cookie clear forces).
|
||||
let tokenRevoked = false;
|
||||
try {
|
||||
if (req.token) {
|
||||
|
||||
@@ -23,6 +23,7 @@ const { db } = require('../database/db');
|
||||
const knexConfig = require('../../knexfile');
|
||||
const { getStoragePath } = require('../config/storage');
|
||||
const { hasColumnCached } = require('../utils/schemaCache');
|
||||
const { setSessionsValidAfter } = require('../utils/sessionCutoff');
|
||||
const logger = require('../utils/logger');
|
||||
const { PICPEAK_FORMAT_VERSION, EXCLUDED_TABLES, listDataTables } = require('./picpeakExportService');
|
||||
|
||||
@@ -105,7 +106,7 @@ function parseNdjson(filePath) {
|
||||
// preserved, its own FKs stay valid) to free the username;
|
||||
// - only when no row has the operator's email do we insert a fresh row.
|
||||
async function reinjectCurrentAdmin(trx, currentAdmin) {
|
||||
if (!currentAdmin) return;
|
||||
if (!currentAdmin) return null;
|
||||
|
||||
const emailMatch = await trx('admin_users')
|
||||
.whereRaw('lower(email) = lower(?)', [currentAdmin.email])
|
||||
@@ -135,6 +136,7 @@ async function reinjectCurrentAdmin(trx, currentAdmin) {
|
||||
if (field in currentAdmin) authUpdate[field] = currentAdmin[field];
|
||||
}
|
||||
await trx('admin_users').where({ id: emailMatch.id }).update(authUpdate);
|
||||
return emailMatch.id;
|
||||
} else {
|
||||
// The operator's email isn't in the backup, so nothing restored references
|
||||
// their id — a fresh row can't dangle a reference TO the operator. Null the
|
||||
@@ -149,6 +151,84 @@ async function reinjectCurrentAdmin(trx, currentAdmin) {
|
||||
const maxRow = await trx('admin_users').max({ m: 'id' }).first();
|
||||
snapshot.id = (Number(maxRow && maxRow.m) || 0) + 1;
|
||||
await trx('admin_users').insert(snapshot);
|
||||
return snapshot.id;
|
||||
}
|
||||
}
|
||||
|
||||
// Capture the operator's role and its granted permission NAMES before the wipe,
|
||||
// so preserveOperatorRole() can re-establish the operator's authorization after
|
||||
// the RBAC tables are replaced. Permission NAMES (not ids) are captured because
|
||||
// the restored permissions table reassigns ids. Returns null if the operator
|
||||
// has no role.
|
||||
async function captureOperatorRole(roleId) {
|
||||
if (!roleId) return null;
|
||||
const role = await db('roles').where({ id: roleId }).first();
|
||||
if (!role) return null;
|
||||
const permissions = await db('role_permissions')
|
||||
.join('permissions', 'permissions.id', 'role_permissions.permission_id')
|
||||
.where('role_permissions.role_id', roleId)
|
||||
.pluck('permissions.name');
|
||||
return { role, permissions };
|
||||
}
|
||||
|
||||
// Restore the operator's authorization after roles/role_permissions are
|
||||
// replaced. A restore rewrites the RBAC tables, so the operator's pre-restore
|
||||
// role_id may now name a different (or missing) role — a crafted backup could
|
||||
// silently downgrade them, and reinjectCurrentAdmin deliberately does NOT copy
|
||||
// role_id (it could dangle). Here we resolve the role by NAME against the
|
||||
// restored data: if a role with the operator's role name exists we trust it
|
||||
// (it's the backup the operator chose to restore); otherwise we re-create the
|
||||
// role from the captured snapshot and re-grant the captured permissions that
|
||||
// still exist, so the operator can never be locked out of their own instance.
|
||||
async function preserveOperatorRole(trx, operatorId, snapshot) {
|
||||
if (!operatorId || !snapshot || !snapshot.role) return;
|
||||
const { role, permissions } = snapshot;
|
||||
|
||||
let target = await trx('roles').whereRaw('lower(name) = lower(?)', [role.name]).first();
|
||||
if (!target) {
|
||||
const roleRow = { ...role };
|
||||
delete roleRow.id;
|
||||
const maxRole = await trx('roles').max({ m: 'id' }).first();
|
||||
const newRoleId = (Number(maxRole && maxRole.m) || 0) + 1; // sequence resynced post-commit
|
||||
roleRow.id = newRoleId;
|
||||
await trx('roles').insert(roleRow);
|
||||
if (permissions && permissions.length) {
|
||||
const perms = await trx('permissions').whereIn('name', permissions).select('id');
|
||||
if (perms.length) {
|
||||
await trx('role_permissions').insert(
|
||||
perms.map((p) => ({ role_id: newRoleId, permission_id: p.id }))
|
||||
);
|
||||
}
|
||||
}
|
||||
target = { id: newRoleId };
|
||||
}
|
||||
await trx('admin_users').where({ id: operatorId }).update({ role_id: target.id });
|
||||
}
|
||||
|
||||
// Fast-forward each restored table's Postgres identity sequence to its current
|
||||
// max(id). batchInsert writes explicit ids without advancing the sequence, so
|
||||
// the next natural insert into any restored table (a new event, an accepted
|
||||
// invitation, etc.) would otherwise collide on the primary key. Runs AFTER the
|
||||
// restore transaction commits (setval is non-transactional and would survive a
|
||||
// rollback) and guards every table with a column-existence check —
|
||||
// pg_get_serial_sequence RAISES on a table lacking an `id` column (e.g. the
|
||||
// composite-key role_permissions), so an unguarded call would abort here.
|
||||
// No-op on SQLite, whose AUTOINCREMENT tracks the high-water mark itself.
|
||||
async function resyncSequences(tables) {
|
||||
if (!isPostgres()) return;
|
||||
for (const table of tables) {
|
||||
try {
|
||||
if (!(await db.schema.hasColumn(table, 'id'))) continue;
|
||||
const res = await db.raw('SELECT pg_get_serial_sequence(?, ?) AS seq', [table, 'id']);
|
||||
const seq = res && res.rows && res.rows[0] && res.rows[0].seq;
|
||||
if (!seq) continue; // `id` isn't a serial/identity column
|
||||
await db.raw(
|
||||
'SELECT setval(?, (SELECT COALESCE(MAX(id), 1) FROM ??), (SELECT MAX(id) IS NOT NULL FROM ??))',
|
||||
[seq, table, table]
|
||||
);
|
||||
} catch (err) {
|
||||
logger.warn(`[picpeak-import] could not resync sequence for ${table}: ${err.message}`);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -188,7 +268,7 @@ function serialiseJsonColumns(rows, jsonCols) {
|
||||
// session_replication_role=replica on the trx connection, reset before commit;
|
||||
// sqlite: defer_foreign_keys so checks run at commit). knex_migrations is never
|
||||
// in the data set, so the target's schema/migration state is left intact.
|
||||
async function replaceAllTables(tables, dataDir, currentAdmin) {
|
||||
async function replaceAllTables(tables, dataDir, currentAdmin, roleSnapshot) {
|
||||
await db.transaction(async (trx) => {
|
||||
if (isPostgres()) {
|
||||
try {
|
||||
@@ -218,7 +298,10 @@ async function replaceAllTables(tables, dataDir, currentAdmin) {
|
||||
await trx.batchInsert(table, serialiseJsonColumns(rows, jsonCols), 100);
|
||||
}
|
||||
|
||||
await reinjectCurrentAdmin(trx, currentAdmin);
|
||||
const operatorId = await reinjectCurrentAdmin(trx, currentAdmin);
|
||||
if (operatorId && roleSnapshot) {
|
||||
await preserveOperatorRole(trx, operatorId, roleSnapshot);
|
||||
}
|
||||
|
||||
// Reset the pg session flag BEFORE the connection returns to the pool.
|
||||
if (isPostgres()) await trx.raw("SET session_replication_role = 'origin'");
|
||||
@@ -288,6 +371,9 @@ async function importFromPicpeak({ picpeakPath, currentAdminId }) {
|
||||
const currentAdmin = currentAdminId
|
||||
? await db('admin_users').where({ id: currentAdminId }).first()
|
||||
: null;
|
||||
// Capture the operator's role + granted permission names BEFORE the wipe so
|
||||
// their authorization can be re-established after the RBAC tables are replaced.
|
||||
const roleSnapshot = currentAdmin ? await captureOperatorRole(currentAdmin.role_id) : null;
|
||||
|
||||
const staging = await fsp.mkdtemp(path.join(os.tmpdir(), 'picpeak-import-'));
|
||||
try {
|
||||
@@ -316,7 +402,16 @@ async function importFromPicpeak({ picpeakPath, currentAdminId }) {
|
||||
logger.warn(`[picpeak-import] ignoring ${skipped.length} backup table(s) not present in this DB (or protected): ${skipped.join(', ')}`);
|
||||
}
|
||||
|
||||
await replaceAllTables(tables, dataDir, currentAdmin);
|
||||
await replaceAllTables(tables, dataDir, currentAdmin, roleSnapshot);
|
||||
|
||||
// Post-commit fixups (must NOT run inside the restore transaction):
|
||||
// - resync Postgres identity sequences left behind by the explicit-id
|
||||
// batchInsert, so the next natural insert doesn't collide;
|
||||
// - stamp a global session cutoff so every JWT issued before this restore
|
||||
// (admin, customer, gallery) stops authenticating — ids may have shifted.
|
||||
await resyncSequences(tables);
|
||||
await setSessionsValidAfter(Math.floor(Date.now() / 1000));
|
||||
|
||||
const filesRestored = await restoreFiles(staging);
|
||||
const usesExternalMedia = await detectExternalMedia();
|
||||
|
||||
@@ -334,4 +429,7 @@ module.exports = {
|
||||
readManifestFromZip,
|
||||
validateManifest,
|
||||
reinjectCurrentAdmin,
|
||||
captureOperatorRole,
|
||||
preserveOperatorRole,
|
||||
resyncSequences,
|
||||
};
|
||||
|
||||
@@ -0,0 +1,100 @@
|
||||
/**
|
||||
* Global session cutoff.
|
||||
*
|
||||
* A .picpeak restore rewrites admin_users / customer_accounts / events and can
|
||||
* reassign their primary keys, so any JWT issued BEFORE the restore may now
|
||||
* resolve to a different restored principal (auth middleware binds a token to
|
||||
* `decoded.id`; IP is only logged and the backup controls each row's
|
||||
* `password_changed_at`). Revoking the single importing token is not enough —
|
||||
* every pre-restore admin, customer, and gallery session must stop being
|
||||
* honoured.
|
||||
*
|
||||
* We record a single unix-second cutoff in app_settings and reject any token
|
||||
* whose `iat` predates it, across all three JWT auth paths. The operator's
|
||||
* forced re-login mints a token with `iat >= cutoff`, so it passes; everything
|
||||
* issued earlier is refused. The value is cached briefly so the common auth
|
||||
* path stays a single in-memory comparison.
|
||||
*/
|
||||
const { db } = require('../database/db');
|
||||
const logger = require('./logger');
|
||||
|
||||
const CUTOFF_KEY = 'security_sessions_valid_after';
|
||||
const CACHE_MS = 30 * 1000; // restores are rare; a short TTL keeps auth cheap
|
||||
|
||||
let cache = null; // { value: number, expiry: number }
|
||||
|
||||
async function readCutoffFromDb() {
|
||||
const row = await db('app_settings')
|
||||
.where('setting_key', CUTOFF_KEY)
|
||||
.first()
|
||||
.timeout(5000);
|
||||
if (!row || row.setting_value == null) return 0;
|
||||
let value = row.setting_value;
|
||||
// pg `json` returns a parsed number; sqlite returns the stored string.
|
||||
if (typeof value === 'string') {
|
||||
try { value = JSON.parse(value); } catch (_) { /* fall through to parseInt */ }
|
||||
}
|
||||
const seconds = parseInt(value, 10);
|
||||
return Number.isFinite(seconds) ? seconds : 0;
|
||||
}
|
||||
|
||||
/**
|
||||
* Cutoff as unix seconds (0 = no cutoff set). Cached for CACHE_MS. On a
|
||||
* transient DB error, returns the last known value (or 0) rather than blocking
|
||||
* auth — the cutoff is defence-in-depth layered on top of per-token revocation.
|
||||
*/
|
||||
async function getSessionsValidAfter() {
|
||||
const now = Date.now();
|
||||
if (cache && now < cache.expiry) return cache.value;
|
||||
try {
|
||||
const value = await readCutoffFromDb();
|
||||
cache = { value, expiry: now + CACHE_MS };
|
||||
return value;
|
||||
} catch (err) {
|
||||
logger.warn('[sessionCutoff] failed to read cutoff:', err.message);
|
||||
return cache ? cache.value : 0;
|
||||
}
|
||||
}
|
||||
|
||||
/** Persist a new cutoff (unix seconds) and refresh the in-process cache. */
|
||||
async function setSessionsValidAfter(unixSeconds) {
|
||||
await db('app_settings')
|
||||
.insert({
|
||||
setting_key: CUTOFF_KEY,
|
||||
setting_value: JSON.stringify(unixSeconds),
|
||||
setting_type: 'number',
|
||||
updated_at: new Date(),
|
||||
})
|
||||
.onConflict('setting_key')
|
||||
.merge({ setting_value: JSON.stringify(unixSeconds), setting_type: 'number', updated_at: new Date() });
|
||||
cache = { value: unixSeconds, expiry: Date.now() + CACHE_MS };
|
||||
}
|
||||
|
||||
/**
|
||||
* True when this token was issued before the global cutoff. Fail-open on any
|
||||
* error: the cutoff is defence-in-depth on top of per-token revocation and the
|
||||
* post-restore cookie clear, and must never turn a transient read failure into
|
||||
* an auth outage.
|
||||
*/
|
||||
async function isTokenBeforeCutoff(decoded) {
|
||||
try {
|
||||
if (!decoded || !decoded.iat) return false;
|
||||
const cutoff = await getSessionsValidAfter();
|
||||
if (!cutoff) return false;
|
||||
return decoded.iat < cutoff;
|
||||
} catch (err) {
|
||||
logger.warn('[sessionCutoff] check failed, allowing token:', err.message);
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
||||
/** Test-only: drop the in-process cache. */
|
||||
function _resetCache() { cache = null; }
|
||||
|
||||
module.exports = {
|
||||
CUTOFF_KEY,
|
||||
getSessionsValidAfter,
|
||||
setSessionsValidAfter,
|
||||
isTokenBeforeCutoff,
|
||||
_resetCache,
|
||||
};
|
||||
Reference in New Issue
Block a user