diff --git a/server/src/db/repositories/UserSessions.repository.ts b/server/src/db/repositories/UserSessions.repository.ts index 570c1797a..17c0b5e6e 100644 --- a/server/src/db/repositories/UserSessions.repository.ts +++ b/server/src/db/repositories/UserSessions.repository.ts @@ -56,6 +56,25 @@ export class UserSessionsRepository extends TrekRepository { }); } + /** + * `INSERT INTO user_sessions (id, user_id, created_at, last_seen_at, expires_at, user_agent) + * VALUES (?, ?, ?, ?, ?, ?) ON CONFLICT (id) DO NOTHING`: a row that is + * already there, ended or not, is left exactly as it is. + */ + async insertSessionIfAbsent(row: NewUserSessionRow): Promise { + await this.upsertMany( + [{ + id: row.id, + user: row.user_id, + created_at: row.created_at, + last_seen_at: row.created_at, + expires_at: row.expires_at, + user_agent: row.user_agent, + }], + { onConflictFields: ['id'], onConflictAction: 'ignore' }, + ); + } + /** * The check behind every session token that carries an id: * `SELECT id, last_seen_at FROM user_sessions WHERE id = ? AND user_id = ? diff --git a/server/src/nest/auth/session-renewal.interceptor.ts b/server/src/nest/auth/session-renewal.interceptor.ts index dd3141c2f..d7f83b38b 100644 --- a/server/src/nest/auth/session-renewal.interceptor.ts +++ b/server/src/nest/auth/session-renewal.interceptor.ts @@ -20,7 +20,9 @@ import { setAuthCookie } from '../common/cookie'; * the decoded (not re-verified) token. * - A tracked token is re-signed under the same session id and the session's * expiry moves with it; a token from before sessions were tracked comes back - * as a tracked session. If the session ended in between, nothing is renewed. + * as a tracked session whose id is derived from that token, so the parallel + * requests of one page load renew it into one session, not several. If the + * session ended in between, nothing is renewed. * - The cookie is only renewed when it is the verified credential: extractToken * prefers the cookie over the Authorization header, so cookie-present + * req.user set means the guard verified the cookie. Bearer-only callers @@ -58,7 +60,7 @@ export class SessionRenewalInterceptor implements NestInterceptor { if (Date.now() / 1000 < halfLife) return; const token = await this.sessions.renew( - { id: req.user.id, pv: claims.pv, remember: claims.remember, jti: claims.jti }, + { id: req.user.id, pv: claims.pv, remember: claims.remember, jti: claims.jti, token: cookieToken }, sessionClientFrom(req), ); if (token) setAuthCookie(res, token, req, claims.remember); diff --git a/server/src/nest/sessions/sessions.service.ts b/server/src/nest/sessions/sessions.service.ts index 6f4b78ecd..57362d6d1 100644 --- a/server/src/nest/sessions/sessions.service.ts +++ b/server/src/nest/sessions/sessions.service.ts @@ -1,4 +1,4 @@ -import { randomUUID } from 'crypto'; +import { createHash, randomUUID } from 'crypto'; import { Injectable } from '@nestjs/common'; import { InjectRepository } from '@mikro-orm/nestjs'; import type { Request } from 'express'; @@ -23,6 +23,24 @@ export interface RenewableSessionClaims { pv?: number; remember?: boolean; jti?: string; + /** + * The token itself. A token from before sessions were tracked has no `jti`, + * and its session id is derived from it, so every renewal of that one token + * lands on the same session. + */ + token?: string; +} + +/** + * The session id a token from before sessions were tracked is renewed into: + * a SHA-256 of the token, laid out as a version 8 UUID (RFC 9562), so it + * passes the same id check a random one does. The same token always gives the + * same id; any other token, which differs at least in its signature, another. + */ +export function legacySessionId(token: string): string { + const h = createHash('sha256').update(token).digest('hex'); + const variant = ((parseInt(h.charAt(16), 16) & 0x3) | 0x8).toString(16); + return `${h.slice(0, 8)}-${h.slice(8, 12)}-8${h.slice(13, 16)}-${variant}${h.slice(17, 20)}-${h.slice(20, 32)}`; } /** The device description of a request: its User-Agent header, when it sent one. */ @@ -84,14 +102,41 @@ export class SessionsService { * the old token keeps working until its own expiry whatever happens next. */ async renew(claims: RenewableSessionClaims, client: SessionClient = {}): Promise { - const user = { id: claims.id, pv: claims.pv ?? 0 }; - if (claims.jti === undefined) return this.issue(user, claims.remember, client); - const token = this.sign(user, claims.remember, claims.jti); + if (claims.jti === undefined) return this.renewUntracked(claims, client); + const token = this.sign({ id: claims.id, pv: claims.pv ?? 0 }, claims.remember, claims.jti); const { exp } = this.lifetimeOf(token); const extended = await this.sessions.extendActive(claims.jti, claims.id, dbNow(), dbNow(new Date(exp * 1000))); return extended ? token : null; } + /** + * A token from before sessions were tracked, renewed into a tracked one. + * + * A page load sends many requests at once, and each of them past the token's + * half life renews it. With a random id every one of them would add a + * session, and the list would show one browser several times until they + * expired. The id is derived from the token instead and the row inserted + * 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. + * + * @txStandalone one statement, standing on its own like `renew`. + */ + private async renewUntracked(claims: RenewableSessionClaims, client: SessionClient): Promise { + const user = { id: claims.id, pv: claims.pv ?? 0 }; + if (claims.token === undefined) return this.issue(user, claims.remember, client); + const jti = legacySessionId(claims.token); + const token = this.sign(user, claims.remember, jti); + const { iat, exp } = this.lifetimeOf(token); + await this.sessions.insertSessionIfAbsent({ + id: jti, + user_id: user.id, + created_at: dbNow(new Date(iat * 1000)), + expires_at: dbNow(new Date(exp * 1000)), + user_agent: clipUserAgent(client.userAgent), + }); + return (await this.sessions.findActive(jti, user.id, dbNow())) ? token : null; + } + /** The user's active sessions, most recently used first, the one in `currentId` flagged. */ async list(userId: number, currentId?: string): Promise { const rows = await this.sessions.listActiveForUser(userId, dbNow()); diff --git a/server/tests/unit/db/repositories/UserSessions.repository.test.ts b/server/tests/unit/db/repositories/UserSessions.repository.test.ts index 0eb9be9a6..ec9ddfd6b 100644 --- a/server/tests/unit/db/repositories/UserSessions.repository.test.ts +++ b/server/tests/unit/db/repositories/UserSessions.repository.test.ts @@ -203,4 +203,23 @@ describe('UserSessionsRepository', () => { expect(row('orphan')).toBeUndefined(); expect(await sessions.restoreCarried([])).toBe(0); }); + + it('SESSREPO-013: insertSessionIfAbsent writes a new row and leaves an existing one alone', async () => { + const { user } = createUser(testDb); + await sessions.insertSessionIfAbsent({ id: 'derived', user_id: user.id, created_at: EARLIER, expires_at: LATER, user_agent: 'First' }); + testDb.prepare("UPDATE user_sessions SET revoked_at = ? WHERE id = 'derived'").run(NOW); + t.clear(); + + await sessions.insertSessionIfAbsent({ id: 'derived', user_id: user.id, created_at: NOW, expires_at: '2026-12-01 00:00:00', user_agent: 'Second' }); + + expect(row('derived')).toStrictEqual({ + id: 'derived', + user_id: user.id, + created_at: EARLIER, + last_seen_at: EARLIER, + expires_at: LATER, + revoked_at: NOW, + user_agent: 'First', + }); + }); }); diff --git a/server/tests/unit/nest/session-renewal.interceptor.test.ts b/server/tests/unit/nest/session-renewal.interceptor.test.ts index f6021eaf3..f4099da29 100644 --- a/server/tests/unit/nest/session-renewal.interceptor.test.ts +++ b/server/tests/unit/nest/session-renewal.interceptor.test.ts @@ -80,7 +80,7 @@ describe('SessionRenewalInterceptor', () => { const res = makeRes(); const t = token({ id: 1, pv: 3 }, 86400, 60000); // ~70% consumed await run(interceptor, { cookies: { trek_session: t }, user: { id: 1 } }, res); - expect(renew).toHaveBeenCalledWith({ id: 1, pv: 3, remember: undefined, jti: undefined }, { userAgent: null }); + expect(renew).toHaveBeenCalledWith({ id: 1, pv: 3, remember: undefined, jti: undefined, token: t }, { userAgent: null }); expect(res.cookie).toHaveBeenCalledWith('trek_session', 'renewed.jwt', expect.objectContaining({ maxAge: 86400000, httpOnly: true })); }); @@ -89,7 +89,7 @@ describe('SessionRenewalInterceptor', () => { const res = makeRes(); const t = token({ id: 1, pv: 0, remember: true }, 2592000, 1600000); // ~62% consumed await run(interceptor, { cookies: { trek_session: t }, user: { id: 1 } }, res); - expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: true, jti: undefined }, { userAgent: null }); + expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: true, jti: undefined, token: t }, { userAgent: null }); expect(res.cookie).toHaveBeenCalledWith('trek_session', 'renewed.jwt', expect.objectContaining({ maxAge: 2592000000 })); }); @@ -98,7 +98,7 @@ describe('SessionRenewalInterceptor', () => { const res = makeRes(); const t = token({ id: 1, pv: 0, remember: false }, 86400, 60000); await run(interceptor, { cookies: { trek_session: t }, user: { id: 1 } }, res); - expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: false, jti: undefined }, { userAgent: null }); + expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: false, jti: undefined, token: t }, { userAgent: null }); const options = res.cookie.mock.calls[0][2]; expect(options).not.toHaveProperty('maxAge'); }); @@ -108,7 +108,7 @@ describe('SessionRenewalInterceptor', () => { const res = makeRes(); const t = token({ id: 1, pv: 0 }, 86400, 60000); await run(interceptor, { cookies: { trek_session: t }, user: { id: 1 } }, res); - expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: undefined, jti: undefined }, { userAgent: null }); + expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: undefined, jti: undefined, token: t }, { userAgent: null }); expect(res.cookie).toHaveBeenCalledWith('trek_session', 'renewed.jwt', expect.objectContaining({ maxAge: 86400000 })); }); @@ -179,7 +179,7 @@ describe('SessionRenewalInterceptor', () => { const res = makeRes(); const t = jwt.sign({ id: 1, pv: 0, iat: Math.floor(Date.now() / 1000) - 60000 }, SECRET, { expiresIn: 86400, algorithm: 'HS256', jwtid: 'sid-1' }); await run(interceptor, { cookies: { trek_session: t }, user: { id: 1 }, headers: { 'user-agent': 'Firefox/130' } }, res); - expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: undefined, jti: 'sid-1' }, { userAgent: 'Firefox/130' }); + expect(renew).toHaveBeenCalledWith({ id: 1, pv: 0, remember: undefined, jti: 'sid-1', token: t }, { userAgent: 'Firefox/130' }); expect(res.cookie).toHaveBeenCalledWith('trek_session', 'renewed.jwt', expect.objectContaining({ maxAge: 86400000 })); }); diff --git a/server/tests/unit/nest/sessions/sessions.service.test.ts b/server/tests/unit/nest/sessions/sessions.service.test.ts index d3c8809c7..ebd416a3f 100644 --- a/server/tests/unit/nest/sessions/sessions.service.test.ts +++ b/server/tests/unit/nest/sessions/sessions.service.test.ts @@ -6,7 +6,8 @@ import { resetTestDb } from '../../../helpers/test-db'; import { createTestOrm, type TestOrm } from '../../../helpers/test-orm'; import { createUser } from '../../../helpers/factories'; import { UserSessions } from '../../../../src/db/entities/UserSessions.entity'; -import { SessionsService, USER_AGENT_MAX_LENGTH, sessionClientFrom } from '../../../../src/nest/sessions/sessions.service'; +import { SessionsService, USER_AGENT_MAX_LENGTH, legacySessionId, sessionClientFrom } from '../../../../src/nest/sessions/sessions.service'; +import { userSessionIdSchema } from '@trek/shared'; import type { Request } from 'express'; import { JWT_SECRET, SESSION_DURATION_REMEMBER_SECONDS, SESSION_DURATION_SECONDS } from '../../../../src/config'; @@ -137,6 +138,60 @@ describe('SessionsService.renew', () => { }); }); +describe('SessionsService.renew of a token from before tracking', () => { + /** A token as the server signed them before sessions were tracked: no jti. */ + function untracked(userId: number): string { + return jwt.sign({ id: userId, pv: 0 }, JWT_SECRET, { algorithm: 'HS256', expiresIn: SESSION_DURATION_SECONDS }); + } + + it('SESS-012: the requests of one page load renew it into a single session', async () => { + const { user } = createUser(testDb); + const legacy = untracked(user.id); + + // Every request of the load carries the same cookie and renews it. + const renewed: (string | null)[] = []; + for (let i = 0; i < 5; i++) renewed.push(await svc.renew({ id: user.id, pv: 0, token: legacy }, { userAgent: 'Page load' })); + + const ids = new Set(renewed.map((token) => claimsOf(token!).jti)); + expect(ids).toEqual(new Set([legacySessionId(legacy)])); + expect(testDb.prepare('SELECT id, user_agent, revoked_at FROM user_sessions WHERE user_id = ?').all(user.id)).toEqual([ + { id: legacySessionId(legacy), user_agent: 'Page load', revoked_at: null }, + ]); + expect(await svc.list(user.id)).toHaveLength(1); + }); + + it('SESS-013: a session derived from it that was ended is not brought back', 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); + + expect(await svc.renew({ id: user.id, pv: 0, token: legacy })).toBeNull(); + expect(rowOf(first.jti)?.revoked_at).not.toBeNull(); + expect(testDb.prepare('SELECT COUNT(*) AS n FROM user_sessions WHERE user_id = ?').get(user.id)).toEqual({ n: 1 }); + }); + + 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) }))!); + const b = claimsOf((await svc.renew({ id: user.id, pv: 0, token: jwt.sign({ id: user.id, pv: 0, n: 2 }, JWT_SECRET) }))!); + expect(a.jti).not.toBe(b.jti); + }); +}); + +describe('legacySessionId', () => { + it('SESS-015: is stable per token, differs between tokens and passes the session id check', () => { + const one = legacySessionId('header.payload.signature-one'); + expect(legacySessionId('header.payload.signature-one')).toBe(one); + expect(legacySessionId('header.payload.signature-two')).not.toBe(one); + for (const token of ['a', 'b', 'header.payload.signature-one', 'x'.repeat(500)]) { + const id = legacySessionId(token); + expect(id).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-8[0-9a-f]{3}-[89ab][0-9a-f]{3}-[0-9a-f]{12}$/); + expect(userSessionIdSchema.safeParse(id).success).toBe(true); + } + }); +}); + describe('SessionsService list and revoke', () => { it('SESS-007: lists the active sessions and flags the current one', async () => { const { user } = createUser(testDb); diff --git a/wiki/Security-Hardening.md b/wiki/Security-Hardening.md index 5747d6c72..d70458001 100644 --- a/wiki/Security-Hardening.md +++ b/wiki/Security-Hardening.md @@ -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. 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. Expired and ended session rows are removed by a nightly job. 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.