From 474beb0dfb5024549838254f73a472d2b282b5ea Mon Sep 17 00:00:00 2001 From: Maurice Date: Thu, 8 Oct 2026 17:24:26 +0200 Subject: [PATCH] fix(sessions): carry the signed-in sessions across a demo reset and a restore Both swap the whole database file. The demo reset copies a baseline saved at first seed, before anyone logged in, so it brings back an empty user_sessions table; a backup restore brings the backup's own rows, or none for a backup from before sessions were tracked. A session token whose row is missing is refused, so the hourly reset signed every demo visitor and the demo admin out, and the admin who ran a restore got a 401 on the next request. Before session tracking only the password version applied, and those tokens survived. Both paths now read the active rows with their owner's email before the swap and put them back after the reopen, in a fresh fork, for every account the new file still holds under the same id and email. A row the new file already has is left as it is, so a session that ended there stays ended, and a session whose id now names somebody else (a backup from another install) is dropped instead of handed to them. The password version check still refuses a token the restored account no longer matches. Reading or writing them is best effort and never fails the reset or the restore. --- .../repositories/UserSessions.repository.ts | 73 +++++++++++ server/src/demo/demo-reset.ts | 22 ++++ server/src/nest/backup/backup.impl.ts | 44 +++++++ .../UserSessions.repository.test.ts | 57 +++++++++ .../unit/demo/demo-reset-sessions.test.ts | 120 ++++++++++++++++++ server/tests/unit/nest/backup.impl.test.ts | 57 +++++++++ wiki/Backups.md | 2 + wiki/Demo-Mode.md | 2 + wiki/Security-Hardening.md | 2 + 9 files changed, 379 insertions(+) create mode 100644 server/tests/unit/demo/demo-reset-sessions.test.ts diff --git a/server/src/db/repositories/UserSessions.repository.ts b/server/src/db/repositories/UserSessions.repository.ts index 40d25f588..570c1797a 100644 --- a/server/src/db/repositories/UserSessions.repository.ts +++ b/server/src/db/repositories/UserSessions.repository.ts @@ -1,4 +1,5 @@ import type { UserSessions } from '../entities/UserSessions.entity'; +import { Users } from '../entities/Users.entity'; import type { AssertRowKeys } from './_shared/rows'; import { TrekRepository } from './_shared/trek-repository'; @@ -18,6 +19,14 @@ const _userSessionRowKeys: AssertRowKeys = true; /** What a session list shows: the row without its owner, who is the caller. */ export type UserSessionListRow = Omit; +/** + * An active session read before a database file swap, with the email its + * owner had then, so it is put back only for that same account. + */ +export interface CarriedUserSessionRow extends Omit { + email: string; +} + /** The columns a new sign-in writes; `last_seen_at` starts at `created_at`. */ export interface NewUserSessionRow { id: string; @@ -124,6 +133,70 @@ export class UserSessionsRepository extends TrekRepository { ); } + /** + * Every active session with its owner's email, read before the database + * file is swapped (a demo reset, a backup restore): + * `SELECT s.* FROM user_sessions s WHERE s.revoked_at IS NULL AND s.expires_at > ?`, + * then `SELECT id, email FROM users WHERE id IN (...)`. + */ + async listActiveToCarry(now: string): Promise { + const rows = await this.find({ revoked_at: null, expires_at: { $gt: now } }, { orderBy: { id: 'asc' } }); + if (rows.length === 0) return []; + const emails = await this.ownerEmails(rows.map((row) => row.user.id)); + const carried: CarriedUserSessionRow[] = []; + for (const row of rows) { + const email = emails.get(row.user.id); + if (email === undefined) continue; + carried.push({ + id: row.id, + user_id: row.user.id, + email, + created_at: row.created_at, + last_seen_at: row.last_seen_at, + expires_at: row.expires_at, + user_agent: row.user_agent ?? null, + }); + } + return carried; + } + + /** + * Put the sessions `listActiveToCarry` read back after the swap: + * `INSERT INTO user_sessions (...) VALUES (...) ON CONFLICT (id) DO NOTHING` + * for each one whose user is still there under the same id and the same + * email. A row the swapped-in file already holds is left as it is, revoked + * or not, and a session whose id now belongs to somebody else (a backup + * from another install) is dropped rather than handed to them. Answers how + * many were offered for insert. + */ + async restoreCarried(rows: readonly CarriedUserSessionRow[]): Promise { + if (rows.length === 0) return 0; + const emails = await this.ownerEmails(rows.map((row) => row.user_id)); + const kept = rows.filter((row) => emails.get(row.user_id) === row.email); + if (kept.length === 0) return 0; + await this.upsertMany( + kept.map((row) => ({ + id: row.id, + user: row.user_id, + created_at: row.created_at, + last_seen_at: row.last_seen_at, + expires_at: row.expires_at, + revoked_at: null, + user_agent: row.user_agent, + })), + { onConflictFields: ['id'], onConflictAction: 'ignore' }, + ); + return kept.length; + } + + /** `SELECT id, email FROM users WHERE id IN (...)`, as a map. */ + private async ownerEmails(userIds: readonly number[]): Promise> { + const owners = await this.getEntityManager() + .getRepository(Users) + .find({ id: { $in: [...new Set(userIds)] } }, { fields: ['id', 'email'] }); + return new Map(owners.map((owner) => [owner.id, owner.email])); + } + /** * The nightly purge: `DELETE FROM user_sessions WHERE expires_at <= ? OR revoked_at IS NOT NULL`. * A row that is gone refuses its token just as a revoked one does, so diff --git a/server/src/demo/demo-reset.ts b/server/src/demo/demo-reset.ts index 5c64bfab8..e05103d4e 100644 --- a/server/src/demo/demo-reset.ts +++ b/server/src/demo/demo-reset.ts @@ -5,6 +5,9 @@ import { readEnv } from '../app-config'; import { resolveDataPaths } from '../app-config/data-paths'; import { DemoRepository, type DemoAdminCredentialsRow, type DemoInstanceKeyRow } from '../db/repositories/DemoRepository'; import { DatabaseConnectionLostError, type DatabaseBackupStrategy } from '../nest/database/database-backup.interface'; +import type { CarriedUserSessionRow } from '../db/repositories/UserSessions.repository'; +import { UserSessions } from '../db/entities/UserSessions.entity'; +import { dbNow } from '../db/types'; const baselinePath = path.join(resolveDataPaths().dataDir, 'travel-baseline.db'); @@ -76,6 +79,17 @@ async function resetDemoUser(database: DatabaseBackupStrategy): Promise { console.error('[Demo Reset] Failed to read instance API keys:', e instanceof Error ? e.message : e); } + // Everyone signed in stays signed in. The baseline was saved at first seed, + // before anybody logged in, so it holds no session rows, and a session + // token whose row is gone is refused: without this every demo visitor and + // the demo admin would be signed out on the hour. + let liveSessions: CarriedUserSessionRow[] = []; + try { + liveSessions = await preSwapEm.getRepository(UserSessions).listActiveToCarry(dbNow()); + } catch (e: unknown) { + console.error('[Demo Reset] Failed to read the active sessions:', e instanceof Error ? e.message : e); + } + // Flush the WAL into the main file before the connection closes. Best effort. try { await database.checkpoint(); @@ -128,6 +142,14 @@ async function resetDemoUser(database: DatabaseBackupStrategy): Promise { console.error('[Demo Reset] Failed to restore instance API keys:', e instanceof Error ? e.message : e); } } + + if (liveSessions.length) { + try { + await freshEm.getRepository(UserSessions).restoreCarried(liveSessions); + } catch (e: unknown) { + console.error('[Demo Reset] Failed to restore the active sessions:', e instanceof Error ? e.message : e); + } + } }); console.log('[Demo Reset] Database restored from baseline'); diff --git a/server/src/nest/backup/backup.impl.ts b/server/src/nest/backup/backup.impl.ts index 5e58dcfff..e8762eae7 100644 --- a/server/src/nest/backup/backup.impl.ts +++ b/server/src/nest/backup/backup.impl.ts @@ -4,6 +4,10 @@ import { pipeline } from 'node:stream/promises'; import { readEnv } from '../../app-config'; import { resolveDataPaths } from '../../app-config/data-paths'; import fs from 'fs'; +import { RequestContext } from '@mikro-orm/core'; +import type { CarriedUserSessionRow } from '../../db/repositories/UserSessions.repository'; +import { UserSessions } from '../../db/entities/UserSessions.entity'; +import { dbNow } from '../../db/types'; import { logError, logWarn } from '../audit/audit-log.logger'; import type { DatabaseBackupStrategy } from '../database/database-backup.interface'; import { VALID_INTERVALS } from './auto-backup.settings'; @@ -410,6 +414,44 @@ export async function rehydrateUploads(storage: StorageService, extractedUploads } } +/** + * The sessions signed in right now, read before the swap. The restored file + * brings the backup's own `user_sessions` rows (none at all for a backup from + * before sessions were tracked), and a session token whose row is missing is + * refused, so without carrying these across the admin who ran the restore and + * everybody else would be signed out by it. Best effort, like the snapshot: + * a missing request context or an unreadable table carries nothing. + */ +async function readSessionsToCarry(): Promise { + try { + const em = RequestContext.getEntityManager(); + if (!em) return []; + return await em.getRepository(UserSessions).listActiveToCarry(dbNow()); + } catch (err) { + logWarn(`Restore: could not read the active sessions (${err instanceof Error ? err.message : String(err)})`); + return []; + } +} + +/** + * Put the carried sessions into the restored database, in a fresh fork so no + * entity read from the replaced file is flushed into it. Only a session whose + * user is still there under the same id and email comes back; the password + * version check still refuses a token the restored account no longer matches. + */ +async function restoreCarriedSessions(rows: readonly CarriedUserSessionRow[]): Promise { + const em = RequestContext.getEntityManager(); + if (!em || rows.length === 0) return; + try { + await RequestContext.create(em, async () => { + const fresh = RequestContext.getEntityManager() ?? em; + await fresh.getRepository(UserSessions).restoreCarried(rows); + }); + } catch (err) { + logWarn(`Restore: could not keep the active sessions (${err instanceof Error ? err.message : String(err)})`); + } +} + export async function restoreFromZip({ storage, database }: BackupDeps, zipPath: string): Promise { const extractDir = path.join(dataDir, `restore-${Date.now()}`); let reinitFailed: unknown; @@ -424,6 +466,7 @@ export async function restoreFromZip({ storage, database }: BackupDeps, zipPath: } await database.keepCopyBeforeRestore(); + const liveSessions = await readSessionsToCarry(); try { // Closes the connection, swaps the database in and reopens it, migrating @@ -465,6 +508,7 @@ export async function restoreFromZip({ storage, database }: BackupDeps, zipPath: // nothing to read, and the restore is already reported as "restart // required" below — rehydrating into a stale/guessed config would be worse. await storage.reloadConfig(); + await restoreCarriedSessions(liveSessions); const extractedUploads = path.join(extractDir, 'uploads'); if (fs.existsSync(extractedUploads)) { diff --git a/server/tests/unit/db/repositories/UserSessions.repository.test.ts b/server/tests/unit/db/repositories/UserSessions.repository.test.ts index 6deba0d35..0eb9be9a6 100644 --- a/server/tests/unit/db/repositories/UserSessions.repository.test.ts +++ b/server/tests/unit/db/repositories/UserSessions.repository.test.ts @@ -146,4 +146,61 @@ describe('UserSessionsRepository', () => { testDb.prepare('DELETE FROM users WHERE id = ?').run(user.id); expect(row('s1')).toBeUndefined(); }); + + it('SESSREPO-010: listActiveToCarry reads every active session with the email of its owner', async () => { + const { user } = createUser(testDb, { email: 'carry-a@example.test' }); + const { user: other } = createUser(testDb, { email: 'carry-b@example.test' }); + await add('a1', user.id, { user_agent: 'A' }); + await add('b1', other.id); + await add('expired', user.id, { expires_at: NOW }); + await add('revoked', other.id); + testDb.prepare("UPDATE user_sessions SET revoked_at = ? WHERE id = 'revoked'").run(EARLIER); + + expect(await sessions.listActiveToCarry(NOW)).toEqual([ + { id: 'a1', user_id: user.id, email: 'carry-a@example.test', created_at: EARLIER, last_seen_at: EARLIER, expires_at: LATER, user_agent: 'A' }, + { id: 'b1', user_id: other.id, email: 'carry-b@example.test', created_at: EARLIER, last_seen_at: EARLIER, expires_at: LATER, user_agent: null }, + ]); + expect(await sessions.listActiveToCarry(LATER)).toEqual([]); + }); + + it('SESSREPO-011: restoreCarried puts the rows back for the same account only, and keeps a row already there', async () => { + const { user } = createUser(testDb, { email: 'kept@example.test' }); + const { user: renamed } = createUser(testDb, { email: 'renamed@example.test' }); + await add('kept', user.id, { user_agent: 'Kept' }); + await add('present', user.id); + await add('elsewhere', renamed.id); + const carried = await sessions.listActiveToCarry(NOW); + + // What a swap does: the rows are gone (or ended), and one id now names somebody else. + testDb.prepare("DELETE FROM user_sessions WHERE id IN ('kept', 'elsewhere')").run(); + testDb.prepare("UPDATE user_sessions SET revoked_at = ? WHERE id = 'present'").run(EARLIER); + testDb.prepare("UPDATE users SET email = 'someone-else@example.test' WHERE id = ?").run(renamed.id); + t.clear(); + + expect(await sessions.restoreCarried(carried)).toBe(2); + expect(row('kept')).toStrictEqual({ + id: 'kept', + user_id: user.id, + created_at: EARLIER, + last_seen_at: EARLIER, + expires_at: LATER, + revoked_at: null, + user_agent: 'Kept', + }); + // The swapped-in file's own row wins, ended or not. + expect(row('present')).toEqual(expect.objectContaining({ revoked_at: EARLIER })); + expect(row('elsewhere')).toBeUndefined(); + }); + + it('SESSREPO-012: restoreCarried drops a session whose user is gone, and does nothing for none', async () => { + const { user } = createUser(testDb, { email: 'gone@example.test' }); + await add('orphan', user.id); + const carried = await sessions.listActiveToCarry(NOW); + testDb.prepare('DELETE FROM users WHERE id = ?').run(user.id); + t.clear(); + + expect(await sessions.restoreCarried(carried)).toBe(0); + expect(row('orphan')).toBeUndefined(); + expect(await sessions.restoreCarried([])).toBe(0); + }); }); diff --git a/server/tests/unit/demo/demo-reset-sessions.test.ts b/server/tests/unit/demo/demo-reset-sessions.test.ts new file mode 100644 index 000000000..6be82a225 --- /dev/null +++ b/server/tests/unit/demo/demo-reset-sessions.test.ts @@ -0,0 +1,120 @@ +/** + * The hourly demo reset keeps everybody signed in. + * + * The reset copies the baseline over the live database file. That baseline + * was saved at first seed, before anybody logged in, so it holds no + * `user_sessions` rows, and a session token whose row is gone is refused. + * Without carrying the active rows across the swap, every demo visitor and + * the demo admin would get a 401 on the hour. + * + * The connection lifecycle is stubbed the way demo-reset-path.test.ts stubs + * it, and the stubbed `reinitialize()` stands in for the file swap by + * emptying `user_sessions`, which is exactly what the baseline brings back. + * The reads and writes around it run for real against the snapshot schema. + */ +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest'; +import fs from 'node:fs'; +import path from 'node:path'; +import type Database from 'better-sqlite3'; +import { createSnapshotTestDb } from '../../helpers/db-mock'; +import { createTestOrm, type TestOrm } from '../../helpers/test-orm'; +import { createUser } from '../../helpers/factories'; +import { withRequestContext } from '../../../src/nest/database/request-context'; + +const LIVE_DB = path.join(path.sep, 'srv', 'trek', 'travel.db'); + +const { handleStub, swap, databaseModule } = vi.hoisted(() => { + const handleStub = { name: '' }; + const swap: { run: () => void } = { run: () => undefined }; + return { + handleStub, + swap, + databaseModule: { + getRawConnection: () => handleStub, + closeDb: vi.fn(), + reinitialize: vi.fn(async () => { + swap.run(); + }), + }, + }; +}); +vi.mock('../../../src/db/database', () => databaseModule); + +const BASELINE = path.resolve(__dirname, '..', '..', '..', 'data', 'travel-baseline.db'); + +import { resetDemoUser } from '../../../src/demo/demo-reset'; +import { SessionsService } from '../../../src/nest/sessions/sessions.service'; +import { verifyJwtAndLoadUser } from '../../../src/nest/auth/jwt-verify'; +import { Users } from '../../../src/db/entities/Users.entity'; +import { UserSessions } from '../../../src/db/entities/UserSessions.entity'; + +describe('demo reset and the signed-in sessions', () => { + let ormDb: Database.Database; + let t: TestOrm; + + beforeEach(async () => { + handleStub.name = LIVE_DB; + vi.spyOn(console, 'log').mockImplementation(() => undefined); + ormDb = createSnapshotTestDb(); + t = await createTestOrm(ormDb); + swap.run = () => { + ormDb.exec('DELETE FROM user_sessions'); + }; + vi.spyOn(fs, 'copyFileSync').mockImplementation(() => undefined); + vi.spyOn(fs, 'unlinkSync').mockImplementation(() => undefined); + vi.spyOn(fs, 'existsSync').mockImplementation(((p: fs.PathLike) => String(p) === BASELINE) as typeof fs.existsSync); + }); + + afterEach(async () => { + vi.restoreAllMocks(); + await t.close(); + ormDb.close(); + }); + + const verify = (token: string) => verifyJwtAndLoadUser(token, t.repo(Users), t.repo(UserSessions)); + + it('DEMORESET-SESS-001: a session signed in before the reset still authenticates after it', async () => { + const { user } = createUser(ormDb, { email: 'demo-visitor@example.test' }); + const sessions = new SessionsService(t.repo(UserSessions)); + const token = await sessions.issue({ id: user.id, pv: 0 }, undefined, { userAgent: 'Visitor' }); + expect((await verify(token))?.id).toBe(user.id); + + await withRequestContext(t.orm, () => resetDemoUser()); + t.clear(); + + expect(databaseModule.reinitialize).toHaveBeenCalled(); + expect((await verify(token))?.id).toBe(user.id); + expect(ormDb.prepare('SELECT user_id, user_agent, revoked_at FROM user_sessions').all()).toEqual([ + { user_id: user.id, user_agent: 'Visitor', revoked_at: null }, + ]); + }); + + it('DEMORESET-SESS-002: a session that had already ended stays ended', async () => { + const { user } = createUser(ormDb, { email: 'demo-ended@example.test' }); + const sessions = new SessionsService(t.repo(UserSessions)); + const token = await sessions.issue({ id: user.id, pv: 0 }); + await sessions.revokeAll(user.id); + + await withRequestContext(t.orm, () => resetDemoUser()); + t.clear(); + + expect(await verify(token)).toBeNull(); + expect(ormDb.prepare('SELECT COUNT(*) AS n FROM user_sessions').get()).toEqual({ n: 0 }); + }); + + it('DEMORESET-SESS-003: a session whose account the baseline does not hold is not brought back', async () => { + const { user } = createUser(ormDb, { email: 'registered-after-seed@example.test' }); + const sessions = new SessionsService(t.repo(UserSessions)); + const token = await sessions.issue({ id: user.id, pv: 0 }); + swap.run = () => { + ormDb.exec('DELETE FROM user_sessions'); + ormDb.prepare('DELETE FROM users WHERE id = ?').run(user.id); + }; + + await withRequestContext(t.orm, () => resetDemoUser()); + t.clear(); + + expect(await verify(token)).toBeNull(); + expect(ormDb.prepare('SELECT COUNT(*) AS n FROM user_sessions').get()).toEqual({ n: 0 }); + }); +}); diff --git a/server/tests/unit/nest/backup.impl.test.ts b/server/tests/unit/nest/backup.impl.test.ts index 04a47b191..c423a0b5a 100644 --- a/server/tests/unit/nest/backup.impl.test.ts +++ b/server/tests/unit/nest/backup.impl.test.ts @@ -1385,6 +1385,63 @@ describe('BACKUP-045 restoreFromZip — full success path (no uploads)', () => { expect(dbMock.closeDb).toHaveBeenCalled(); }); + it('BACKUP-045i: the sessions signed in before the restore are put back after the reopen', async () => { + setupSuccessfulExtraction(); + setupAllTablesPresent(); + const callOrder: string[] = []; + const carried = [{ id: 'sess-1', user_id: 1, email: 'admin@example.test', created_at: 'a', last_seen_at: 'b', expires_at: 'c', user_agent: null }]; + const sessionsRepo = { + listActiveToCarry: vi.fn(async () => { callOrder.push('read'); return carried; }), + restoreCarried: vi.fn(async () => { callOrder.push('restore'); return 1; }), + }; + const { RequestContext } = await import('@mikro-orm/core'); + const em = vi.spyOn(RequestContext, 'getEntityManager').mockReturnValue({ getRepository: () => sessionsRepo } as never); + const fork = vi.spyOn(RequestContext, 'create').mockImplementation(((_em: unknown, next: () => unknown) => next()) as never); + onTestFinished(() => { em.mockRestore(); fork.mockRestore(); }); + dbMock.closeDb.mockImplementation(() => { callOrder.push('closeDb'); }); + dbMock.reinitialize.mockImplementation(() => { callOrder.push('reinitialize'); }); + fsMock.unlinkSync.mockReturnValue(undefined); + fsMock.copyFileSync.mockReturnValue(undefined); + fsMock.rmSync.mockReturnValue(undefined); + fsMock.existsSync.mockImplementation((p: string) => !String(p).includes('uploads') && !String(p).endsWith('.encryption_key')); + + const result = await restoreFromZip(stubStorage(), '/data/tmp/upload.zip'); + + expect(result).toEqual({ success: true }); + expect(callOrder).toEqual(['read', 'closeDb', 'reinitialize', 'restore']); + expect(sessionsRepo.restoreCarried).toHaveBeenCalledWith(carried); + // Put back in a fresh fork, not through the one that read the replaced file. + expect(fork).toHaveBeenCalledTimes(1); + }); + + it('BACKUP-045j: sessions that cannot be read or put back do not fail the restore', async () => { + setupSuccessfulExtraction(); + setupAllTablesPresent(); + const sessionsRepo = { + listActiveToCarry: vi.fn().mockRejectedValueOnce(new Error('no such table: user_sessions')).mockResolvedValueOnce([{ id: 's' }]), + restoreCarried: vi.fn().mockRejectedValue(new Error('database is locked')), + }; + const { RequestContext } = await import('@mikro-orm/core'); + const em = vi.spyOn(RequestContext, 'getEntityManager').mockReturnValue({ getRepository: () => sessionsRepo } as never); + const fork = vi.spyOn(RequestContext, 'create').mockImplementation(((_em: unknown, next: () => unknown) => next()) as never); + onTestFinished(() => { em.mockRestore(); fork.mockRestore(); }); + fsMock.unlinkSync.mockReturnValue(undefined); + fsMock.copyFileSync.mockReturnValue(undefined); + fsMock.rmSync.mockReturnValue(undefined); + fsMock.existsSync.mockImplementation((p: string) => !String(p).includes('uploads') && !String(p).endsWith('.encryption_key')); + + const unreadable = await restoreFromZip(stubStorage(), '/data/tmp/upload.zip'); + expect(unreadable).toEqual({ success: true }); + expect(logMock.logWarn).toHaveBeenCalledWith(expect.stringContaining('could not read the active sessions (no such table: user_sessions)')); + expect(sessionsRepo.restoreCarried).not.toHaveBeenCalled(); + + setupSuccessfulExtraction(); + setupAllTablesPresent(); + const unwritable = await restoreFromZip(stubStorage(), '/data/tmp/upload.zip'); + expect(unwritable).toEqual({ success: true }); + expect(logMock.logWarn).toHaveBeenCalledWith(expect.stringContaining('could not keep the active sessions (database is locked)')); + }); + it('BACKUP-045c — reinitialize is called even when copyFileSync throws', async () => { setupSuccessfulExtraction(); setupAllTablesPresent(); diff --git a/wiki/Backups.md b/wiki/Backups.md index 7a309013e..8255e0f4f 100644 --- a/wiki/Backups.md +++ b/wiki/Backups.md @@ -46,6 +46,8 @@ Before restoring, TREK runs integrity checks on the uploaded database: The database being replaced is kept as `pre-restore-