fix(sessions): keep an ended session's row until it expires

A session renewed from a token issued before sessions were tracked has
an id derived from that token, and the renewal inserts it only when no
row by that id exists. The nightly purge deleted revoked rows straight
away, so the next request with the still valid old token inserted the
session again as an active one and got a fresh tracked token with a new
lifetime. A logout, an ended session or a sign out of the others held
only until the purge ran.

The purge now deletes expired rows only, and a revoked row stays until
its own expiry. A derived row is recorded with an expiry no earlier
than the old token's, so by the time it goes the old token is refused
by its signature check anyway.
This commit is contained in:
Maurice
2026-10-08 17:55:20 +02:00
parent e94525afd3
commit 648f63c626
9 changed files with 104 additions and 24 deletions
+1 -1
View File
@@ -96,7 +96,7 @@ SQLite is the engine today and Postgres is the planned second one. Code under `s
## Cross-cutting pieces
- **`src/nest/common/`**: the global exception filter (error envelope), the Zod validation pipe, the idempotency interceptor (`X-Idempotency-Key` replay), and `validate-route-guards.ts` (boot-time ratchet refusing any public route not on its allow-list).
- **Auth is default-deny**: the global auth guard and the MFA policy guard run as `APP_GUARD`s, followed by the managed-install guard (`managed.guard.ts`), which refuses the routes a centrally administered install withholds from its own admin — it runs last so strangers still get a 401, never a route-revealing 403. Routes opt out with `@Public()`/`@OptionalAuth()`. The global guard stands down for routes declaring their own guard chain, so `@UseGuards(JwtAuthGuard, TripAccessGuard)` + `@RequirePermission` stacks work. Session is a cookie (`trek_session`); `@CurrentUser()` reads the user. Every session token carries a `jti` naming its `user_sessions` row, and `verifyJwtAndLoadUser` refuses one whose row is revoked, expired or gone (logout, `/api/auth/sessions`, a password change or reset, an MFA disable all end rows; `session-purge.job.ts` deletes the dead ones nightly). A token without a `jti` predates the table and passes until it expires, so an upgrade signs nobody out. The `token` field in the login-type JSON bodies and in the SSO exchange (`GET /api/auth/oidc/exchange`, which also answers `success: true`) is deprecated: kept for API clients, read by nothing of ours. **Never gate a multipart upload with a guard** — guards run before the parser and the client sees ECONNRESET instead of your 403; check in the handler.
- **Auth is default-deny**: the global auth guard and the MFA policy guard run as `APP_GUARD`s, followed by the managed-install guard (`managed.guard.ts`), which refuses the routes a centrally administered install withholds from its own admin — it runs last so strangers still get a 401, never a route-revealing 403. Routes opt out with `@Public()`/`@OptionalAuth()`. The global guard stands down for routes declaring their own guard chain, so `@UseGuards(JwtAuthGuard, TripAccessGuard)` + `@RequirePermission` stacks work. Session is a cookie (`trek_session`); `@CurrentUser()` reads the user. Every session token carries a `jti` naming its `user_sessions` row, and `verifyJwtAndLoadUser` refuses one whose row is revoked, expired or gone (logout, `/api/auth/sessions`, a password change or reset, an MFA disable all end rows; `session-purge.job.ts` deletes the expired ones nightly; a revoked row waits for its own expiry, because for a session renewed from a pre-`jti` token, whose id is derived from that token, the revoked row is all that keeps the token from inserting it again). A token without a `jti` predates the table and passes until it expires, so an upgrade signs nobody out. The `token` field in the login-type JSON bodies and in the SSO exchange (`GET /api/auth/oidc/exchange`, which also answers `success: true`) is deprecated: kept for API clients, read by nothing of ours. **Never gate a multipart upload with a guard** — guards run before the parser and the client sees ECONNRESET instead of your 403; check in the handler.
- **Logging**: `src/nest/audit/audit-log.logger.ts` (`logInfo`/`logWarn`/`logError`/`logDebug`) prints each line to the console and queues it for `data/logs/trek.log`, which `log-file.ts` appends in batches off the event loop and rotates at 10 MB over five files; `index.ts` flushes what is queued on exit. Never write the log file synchronously from a request path.
- **Request correlation** (`src/nest/common/request-correlation.ts`): every HTTP request runs under an id (a well-formed incoming `X-Request-Id`, else a fresh UUID) that the response carries in `X-Request-Id`, and every log line written inside it is tagged `[http <id>]`. The exception filter leaves a 5xx on `res.locals` so the access log writes the request line and the stack as one entry. The other entry points (MCP tool/resource/prompt calls via the registry's `around` seam, WebSocket messages in `TrekWsAdapter`, plugin RPC dispatches in the supervisor, cron ticks in `CronRegistrarService`) each run under their own id through `traceEntry()` (`nest/audit/entry-trace.logger.ts`), which logs one line per call: debug on success, debug as `skipped` when the call declines its work on purpose (a cron tick whose lease another process holds), warn or error with the reason on failure. A new entry point goes through `traceEntry()` too.
- **Process safety nets** (`src/index.ts`, `src/shutdown.ts`): `unhandledRejection` and `uncaughtException` are logged to trek.log and end in the same orderly shutdown as SIGTERM, with exit code 1; a second fatal error during it exits at once. A shutdown first marks `ReadinessService` (`nest/health/`) as draining, so `/api/health/ready` answers 503 for the whole drain. The HTTP server keeps idle keep-alive connections for `HTTP_KEEP_ALIVE_TIMEOUT_MS` (95 s, above the 60 s of nginx and the 90 s of Traefik) with the headers timeout one second above it (`applyProxyTimeouts` in `bootstrap.ts`).
@@ -217,11 +217,16 @@ export class UserSessionsRepository extends TrekRepository<UserSessions> {
}
/**
* 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
* nothing depends on keeping them.
* The nightly purge: `DELETE FROM user_sessions WHERE expires_at <= ?`.
*
* A revoked row stays until its own expiry. A session renewed from a token
* issued before sessions were tracked has an id derived from that token,
* and only its revoked row stops the still valid old token from inserting
* it again as a fresh, active session. Its `expires_at` is never earlier
* than the old token's expiry, so once it has passed, the old token is dead
* too and the row can go.
*/
async deleteInactive(now: string): Promise<number> {
return await this.nativeDelete({ $or: [{ expires_at: { $lte: now } }, { revoked_at: { $ne: null } }] });
return await this.nativeDelete({ expires_at: { $lte: now } });
}
}
@@ -4,11 +4,11 @@ import { CronRegistrarService } from '../scheduling/cron-registrar.service';
import { SessionsService } from './sessions.service';
/**
* Nightly removal of the `user_sessions` rows that no longer let anyone in:
* expired ones, and revoked ones. Every sign-in writes a row, so without this
* the table would grow by one row per login for as long as the install runs.
* Deleting a revoked row changes nothing for its token, which a missing row
* refuses just the same.
* Nightly removal of the expired `user_sessions` rows. Every sign-in writes a
* row, so without this the table would grow by one row per login for as long
* as the install runs. A revoked row waits for its own expiry: for a session
* renewed from a token from before sessions were tracked, that row is what
* keeps the old token from bringing the session back.
*/
@Injectable()
export class SessionPurgeJob implements OnApplicationBootstrap {
@@ -25,7 +25,7 @@ export class SessionPurgeJob implements OnApplicationBootstrap {
async tick(now: Date = new Date()): Promise<void> {
try {
const removed = await this.sessions.purgeInactive(now);
if (removed > 0) logInfo(`Session purge: removed ${removed} expired or revoked session(s)`);
if (removed > 0) logInfo(`Session purge: removed ${removed} expired session(s)`);
} catch (err: unknown) {
logError(`Session purge: ${err instanceof Error ? err.message : String(err)}`);
}
+20 -2
View File
@@ -119,6 +119,12 @@ export class SessionsService {
* only if it is not there yet, so all of them land on one session. If that
* session was ended in the meantime, nothing is renewed.
*
* The old token stays valid until its own expiry whatever happens to the
* session, so the row is what refuses it after an end. Its `expires_at` is
* therefore never earlier than the old token's, and the nightly purge
* keeps a revoked row until that has passed: a purged row would let the
* old token insert the session again as an active one.
*
* @txStandalone one statement, standing on its own like `renew`.
*/
private async renewUntracked(claims: RenewableSessionClaims, client: SessionClient): Promise<string | null> {
@@ -127,11 +133,12 @@ export class SessionsService {
const jti = legacySessionId(claims.token);
const token = this.sign(user, claims.remember, jti);
const { iat, exp } = this.lifetimeOf(token);
const keepUntil = Math.max(exp, expiryOf(claims.token) ?? exp);
await this.sessions.insertSessionIfAbsent({
id: jti,
user_id: user.id,
created_at: dbNow(new Date(iat * 1000)),
expires_at: dbNow(new Date(exp * 1000)),
expires_at: dbNow(new Date(keepUntil * 1000)),
user_agent: clipUserAgent(client.userAgent),
});
return (await this.sessions.findActive(jti, user.id, dbNow())) ? token : null;
@@ -185,7 +192,11 @@ export class SessionsService {
return this.sessions.revokeAllForUser(userId, dbNow(), exceptId);
}
/** Remove the rows that refuse their token anyway: expired or revoked. */
/**
* Remove the expired rows. A revoked row is kept until it expires too, so
* a session derived from a token from before tracking cannot come back
* (see `renewUntracked`).
*/
async purgeInactive(now: Date): Promise<number> {
return this.sessions.deleteInactive(dbNow(now));
}
@@ -208,6 +219,13 @@ export class SessionsService {
}
}
/** The `exp` claim of a token, when it has a numeric one. */
function expiryOf(token: string): number | undefined {
const decoded = jwt.decode(token);
const exp = decoded !== null && typeof decoded === 'object' ? (decoded as { exp?: unknown }).exp : undefined;
return typeof exp === 'number' ? exp : undefined;
}
function clipUserAgent(agent: string | null | undefined): string | null {
return agent ? agent.slice(0, USER_AGENT_MAX_LENGTH) : null;
}
+24 -2
View File
@@ -336,7 +336,7 @@ describe('Sessions e2e (sign-in sessions that can be ended)', () => {
expect((await me(there)).status).toBe(401);
}, 15000);
it('the nightly purge removes the ended and expired rows and leaves the live session working', async () => {
it('the nightly purge removes the expired rows, keeps an ended one until it expires, and leaves the live session working', async () => {
const { user, password } = freshUser('sess-purge');
const live = await signIn(user.email, password, 'Live');
const ended = await signIn(user.email, password, 'Ended');
@@ -349,8 +349,30 @@ describe('Sessions e2e (sign-in sessions that can be ended)', () => {
const job = moduleRef.get(SessionPurgeJob);
await withRequestContext(moduleRef.get(MikroORM), () => job.tick());
expect(sessionRows(db as never, user.id).map((row) => row.id)).toEqual([sessionIdOf(live)]);
expect(sessionRows(db as never, user.id).map((row) => row.id).sort()).toEqual([sessionIdOf(live), sessionIdOf(ended)].sort());
expect((await me(live)).status).toBe(200);
expect((await me(ended)).status).toBe(401);
}, 15000);
it('a session renewed from a token from before tracking stays ended through the nightly purge', async () => {
const { user } = freshUser('sess-legacy-purge');
const old = sessionCookie(user.id, 0, { lifetime: 86400, consumed: 60000 });
const first = await request(server).get('/api/auth/me').set('Cookie', old).set('User-Agent', 'Old');
const renewedCookie = ((first.headers['set-cookie'] ?? []) as unknown as string[]).find((c) => c.startsWith('trek_session='))!;
const renewed = /^(trek_session=[^;]+)/.exec(renewedCookie)![1];
expect((await request(server).post('/api/auth/logout').set('Cookie', renewed)).status).toBe(200);
expect((await me(renewed)).status).toBe(401);
const job = moduleRef.get(SessionPurgeJob);
await withRequestContext(moduleRef.get(MikroORM), () => job.tick());
// The old token itself lives out its own expiry, but it is not renewed into the ended session again.
const again = await request(server).get('/api/auth/me').set('Cookie', old).set('User-Agent', 'Old');
expect(again.status).toBe(200);
expect(((again.headers['set-cookie'] ?? []) as unknown as string[]).some((c) => c.startsWith('trek_session='))).toBe(false);
expect(sessionRows(db as never, user.id)).toEqual([expect.objectContaining({ id: sessionIdOf(renewed), revoked_at: expect.any(String) })]);
const list = await request(server).get('/api/auth/sessions').set('Cookie', old);
expect(list.body.sessions).toEqual([]);
}, 15000);
});
@@ -127,17 +127,20 @@ describe('UserSessionsRepository', () => {
expect(row('theirs')).toEqual(expect.objectContaining({ revoked_at: null }));
});
it('SESSREPO-008: deleteInactive removes the expired and the revoked rows and keeps the live ones', async () => {
it('SESSREPO-008: deleteInactive removes the expired rows, revoked or not, and keeps the rest', async () => {
const { user } = createUser(testDb);
await add('live', user.id);
await add('expired', user.id, { expires_at: NOW });
await add('revoked', user.id);
testDb.prepare("UPDATE user_sessions SET revoked_at = ? WHERE id = 'revoked'").run(EARLIER);
await add('revoked-expired', user.id, { expires_at: EARLIER });
testDb.prepare("UPDATE user_sessions SET revoked_at = ? WHERE id IN ('revoked', 'revoked-expired')").run(EARLIER);
expect(await sessions.deleteInactive(NOW)).toBe(2);
expect(row('live')).toBeDefined();
expect(row('expired')).toBeUndefined();
expect(row('revoked')).toBeUndefined();
expect(row('revoked-expired')).toBeUndefined();
// Revoked but not yet expired: kept, it still refuses the token it was derived from.
expect(row('revoked')).toEqual(expect.objectContaining({ revoked_at: EARLIER }));
});
it('SESSREPO-009: the rows go with their user', async () => {
@@ -34,7 +34,7 @@ describe('SessionPurgeJob', () => {
const now = new Date('2026-10-08T03:45:00.000Z');
await make(purge).job.tick(now);
expect(purge).toHaveBeenCalledWith(now);
expect(logMock.logInfo).toHaveBeenCalledWith('Session purge: removed 3 expired or revoked session(s)');
expect(logMock.logInfo).toHaveBeenCalledWith('Session purge: removed 3 expired session(s)');
});
it('SESSPURGE-003: stays quiet when there was nothing to purge', async () => {
@@ -173,6 +173,34 @@ describe('SessionsService.renew of a token from before tracking', () => {
expect(testDb.prepare('SELECT COUNT(*) AS n FROM user_sessions WHERE user_id = ?').get(user.id)).toEqual({ n: 1 });
});
it('SESS-017: a session derived from it that was ended stays ended through the nightly purge', async () => {
const { user } = createUser(testDb);
const legacy = untracked(user.id);
const first = claimsOf((await svc.renew({ id: user.id, pv: 0, token: legacy }))!);
expect(await svc.revoke(user.id, first.jti)).toBe(true);
// The purge that night, while the old token is still valid.
expect(await svc.purgeInactive(new Date())).toBe(0);
expect(await svc.renew({ id: user.id, pv: 0, token: legacy })).toBeNull();
expect(rowOf(first.jti)?.revoked_at).not.toBeNull();
expect(await svc.list(user.id)).toEqual([]);
// Once the old token has expired the row may go, since the token is refused by then.
const legacyExp = (jwt.decode(legacy) as { exp: number }).exp;
expect(await svc.purgeInactive(new Date((legacyExp + SESSION_DURATION_SECONDS + 60) * 1000))).toBe(1);
expect(() => jwt.verify(legacy, JWT_SECRET, { algorithms: ['HS256'], clockTimestamp: legacyExp + 60 })).toThrow();
});
it('SESS-018: the derived session is kept at least as long as the old token lives', async () => {
const { user } = createUser(testDb);
// An old token with a longer life than the one it is renewed into.
const legacy = jwt.sign({ id: user.id, pv: 0 }, JWT_SECRET, { algorithm: 'HS256', expiresIn: SESSION_DURATION_REMEMBER_SECONDS });
const renewed = claimsOf((await svc.renew({ id: user.id, pv: 0, token: legacy }))!);
expect(renewed.exp - renewed.iat).toBe(SESSION_DURATION_SECONDS);
expect(rowOf(renewed.jti)?.expires_at).toBe(textOf((jwt.decode(legacy) as { exp: number }).exp));
});
it('SESS-014: two different tokens of the same user stay two sessions', async () => {
const { user } = createUser(testDb);
const a = claimsOf((await svc.renew({ id: user.id, pv: 0, token: untracked(user.id) }))!);
@@ -244,19 +272,23 @@ describe('SessionsService list and revoke', () => {
expect(rowOf(mine.jti)?.revoked_at).not.toBeNull();
});
it('SESS-011: purgeInactive removes the expired and revoked rows', async () => {
it('SESS-011: purgeInactive removes the expired rows and keeps a revoked one until it expires', async () => {
const { user } = createUser(testDb);
const live = claimsOf(await svc.issue({ id: user.id, pv: 0 }, true));
const ended = claimsOf(await svc.issue({ id: user.id, pv: 0 }, true));
const short = claimsOf(await svc.issue({ id: user.id, pv: 0 }, false));
await svc.revoke(user.id, ended.jti);
// Just past the default lifetime: the short session has expired, the remembered one has not.
// Just past the default lifetime: the short session has expired, the remembered ones have not.
expect(SESSION_DURATION_REMEMBER_SECONDS).toBeGreaterThan(SESSION_DURATION_SECONDS + 60);
expect(await svc.purgeInactive(new Date(Date.now() + (SESSION_DURATION_SECONDS + 60) * 1000))).toBe(2);
expect(await svc.purgeInactive(new Date(Date.now() + (SESSION_DURATION_SECONDS + 60) * 1000))).toBe(1);
expect(rowOf(live.jti)).toBeDefined();
expect(rowOf(ended.jti)).toBeUndefined();
expect(rowOf(ended.jti)?.revoked_at).not.toBeNull();
expect(rowOf(short.jti)).toBeUndefined();
// Past the long lifetime as well: the revoked row goes with the live one.
expect(await svc.purgeInactive(new Date(Date.now() + (SESSION_DURATION_REMEMBER_SECONDS + 60) * 1000))).toBe(2);
expect(rowOf(ended.jti)).toBeUndefined();
});
});
+1 -1
View File
@@ -52,7 +52,7 @@ A session ends when:
`GET /api/auth/sessions` lists the active sessions of your account, most recently used first, with when each started, when it was last used (refreshed at most every few minutes), when it expires, the browser's User-Agent at sign-in (cut to 256 characters) and which one is the current one. No IP address is stored. The settings screen does not show this list yet; the routes are there for it and for API clients. On a demo instance the shared demo account sees only its own session in the list and cannot end sessions (403), since its sessions are other visitors' browsers.
Sessions issued before this was introduced carry no session id. They keep working until they expire (or until the password changes, which ends them through the password version), are not listed, and are not ended by **revoke-others**. Once such a session passes half its lifetime, the sliding renewal replaces it with a tracked one. That session's id is derived from the old token, so the many requests a page load sends at once all renew it into the same single session instead of listing the browser several times. Expired and ended session rows are removed by a nightly job.
Sessions issued before this was introduced carry no session id. They keep working until they expire (or until the password changes, which ends them through the password version), are not listed, and are not ended by **revoke-others**. Once such a session passes half its lifetime, the sliding renewal replaces it with a tracked one. That session's id is derived from the old token, so the many requests a page load sends at once all renew it into the same single session instead of listing the browser several times. A nightly job removes the session rows that have expired. An ended session's row stays until its own expiry, so a session renewed from such an old token cannot be brought back by that token once it was ended.
A backup restore and the hourly demo reset replace the whole database file. Both carry the sessions that are active at that moment into the new file, for every account it holds under the same id and email, so neither signs anybody out by itself.