From 1775fac5fd112eadb06aa0aa013bced29cd02d7a Mon Sep 17 00:00:00 2001 From: Hornwitser Date: Mon, 7 Jul 2025 22:42:49 +0200 Subject: [PATCH] Refactor sessions to frequently rotate In order to minimise the window of opportunity to steal a session, automatically rotate it onto a new session on a frequent basis. This makes a session cookie older than the automatic rollover time less likely to grant access and more likely to be detected. Should a stolen session cookie get rotated while the attacker is using it, the user will be notificed that their session has been taken the next time they open the app if the user re-visits the website before the session is discarded. --- docs/admin/config.md | 17 +++++ docs/dev/sessions.md | 13 ++++ nuxt.config.ts | 4 + server/api/admin/database-import.post.ts | 2 +- server/api/admin/user.patch.ts | 19 ++++- server/api/auth/account.delete.ts | 8 +- server/api/auth/account.patch.ts | 4 +- server/api/auth/account.post.ts | 2 +- server/api/auth/session.delete.ts | 7 +- server/api/auth/session.get.ts | 10 +-- server/api/events.ts | 5 +- server/api/schedule.patch.ts | 6 +- server/api/schedule.ts | 4 +- server/api/subscribe.post.ts | 2 +- server/api/unsubscribe.post.ts | 2 +- server/api/users/index.get.ts | 6 +- server/database.ts | 34 ++------- server/utils/session.ts | 96 ++++++++++++++++++++---- 18 files changed, 168 insertions(+), 73 deletions(-) create mode 100644 docs/admin/config.md create mode 100644 docs/dev/sessions.md diff --git a/docs/admin/config.md b/docs/admin/config.md new file mode 100644 index 0000000..1dbc6c8 --- /dev/null +++ b/docs/admin/config.md @@ -0,0 +1,17 @@ + +# Configuration + +## Environment Variables + +### NUXT_SESSION_EXPIRES_TIMEOUT + +Time in seconds before a session is considered expired and need to be rotated over into a new session. When an endpoint using a session is hit after the session expires but before the session is discarded a new session is created as the successor with a new expiry and discard timeout. The old session then considered to have been superceeded and any requests using the old session will result in a 403 Forbidden with the message the session has been taken. + +### NUXT_SESSION_DISCARD_TIMEOUT + +Time in seconds before a session is deleted from the client and server, resulting in the user having to authenticate again if the session wasn't rotated over into a new session before this timeout. + +This should be several times greater that `NUXT_SESSION_EXPIRES_TIMEOUT`. diff --git a/docs/dev/sessions.md b/docs/dev/sessions.md new file mode 100644 index 0000000..6ae7967 --- /dev/null +++ b/docs/dev/sessions.md @@ -0,0 +1,13 @@ + +# Sessions + +When a user creates a new account or logs in to an existing account a session is created on the server and linked to the user's browser via a session cookie. This cookie contains a unique identifier along with a HMAC signature created using the server's cookie secret key. Since this unique identifier stored on the user's device is a technical requirement to securely do what the user is requesting the user's consent to its storage can be assumed. + +Sessions have two future times associated with them: The expiration time is the point in time after the session will be recreated and the cookie reassigned, and the discard time is when the session is deleted from both the client and the server. The expiriation time is short, by default 1 hour, while the discard time is long, by default 2 weeks. + +When a request is made to a session that's past the expiration time a new session is created to replace the existing one, and the session cookie is updated with the new session. The purpose of this is to reduce the time window a stolen session can be used in without being detected. If a request is made using a session that has already been replaced the server responds with a message saying the "session has been taken". + +Sessions are created for a limited timespan, purpose and access level, and expires after the timespan is over, the purpose is fulfilled or the access level changes. For example if the user's account is promoted from regular to crew the session will no longer be valid and will be recreated as a new session with the new access level on the next request. diff --git a/nuxt.config.ts b/nuxt.config.ts index abf656d..71b5233 100644 --- a/nuxt.config.ts +++ b/nuxt.config.ts @@ -4,6 +4,8 @@ */ // https://nuxt.com/docs/api/configuration/nuxt-config const enableDevtools = !process.env.DISABLE_DEV_TOOLS +const oneHourSeconds = 60 * 60; +const oneDaySeconds = 24 * oneHourSeconds; export default defineNuxtConfig({ experimental: { renderJsonPayloads: true }, compatibilityDate: '2024-11-01', @@ -21,6 +23,8 @@ export default defineNuxtConfig({ }, runtimeConfig: { cookieSecretKeyFile: "", + sessionExpiresTimeout: 1 * oneHourSeconds, + sessionDiscardTimeout: 14 * oneDaySeconds, vapidSubject: "", vapidPrivateKeyFile: "", public: { diff --git a/server/api/admin/database-import.post.ts b/server/api/admin/database-import.post.ts index 3449a84..f4e6019 100644 --- a/server/api/admin/database-import.post.ts +++ b/server/api/admin/database-import.post.ts @@ -40,7 +40,7 @@ export default defineEventHandler(async (event) => { // Only keep sessions that match the account id in both sets to avoid // resurrecting deleted sessions. This will still cause session cross // pollution if a snapshot from another instance is loaded here. - return current && current.account.id === session.account.id; + return current?.accountId !== undefined && current.accountId === session.accountId; })); await writeSubscriptions(snapshot.subscriptions); await writeSchedule(snapshot.schedule); diff --git a/server/api/admin/user.patch.ts b/server/api/admin/user.patch.ts index 0acc771..bdcf099 100644 --- a/server/api/admin/user.patch.ts +++ b/server/api/admin/user.patch.ts @@ -2,7 +2,7 @@ SPDX-FileCopyrightText: © 2025 Hornwitser SPDX-License-Identifier: AGPL-3.0-or-later */ -import { readUsers, writeUsers } from "~/server/database"; +import { readSessions, readUsers, writeSessions, writeUsers } from "~/server/database"; import { apiUserPatchSchema } from "~/shared/types/api"; import { z } from "zod/v4-mini"; import { broadcastEvent } from "~/server/streams"; @@ -29,7 +29,8 @@ export default defineEventHandler(async (event) => { } - if (patch.type) { + let accessChanged = false; + if (patch.type && patch.type !== user.type) { if (patch.type === "anonymous" || user.type === "anonymous") { throw createError({ status: 409, @@ -38,6 +39,7 @@ export default defineEventHandler(async (event) => { }); } user.type = patch.type; + accessChanged = true; } if (patch.name) { if (user.type === "anonymous") { @@ -54,7 +56,18 @@ export default defineEventHandler(async (event) => { broadcastEvent({ type: "user-update", data: serverUserToApi(user), - }) + }); + + // Expire sessions with the user in it if the access changed + if (accessChanged) { + const sessions = await readSessions(); + for (const session of sessions) { + if (session.accountId === user.id) { + session.expiresAtMs = 0; + } + } + await writeSessions(sessions); + } // Update Schedule counts. await updateScheduleInterestedCounts(users); diff --git a/server/api/auth/account.delete.ts b/server/api/auth/account.delete.ts index bf62f7f..7506531 100644 --- a/server/api/auth/account.delete.ts +++ b/server/api/auth/account.delete.ts @@ -9,20 +9,20 @@ import { import { broadcastEvent, cancelAccountStreams } from "~/server/streams"; export default defineEventHandler(async (event) => { - const serverSession = await requireServerSession(event); + const serverSession = await requireServerSessionWithUser(event); let users = await readUsers(); // Remove sessions for this user const removedSessionIds = new Set(); let sessions = await readSessions(); sessions = sessions.filter(session => { - if (session.account.id === serverSession.account.id) { + if (session.accountId === serverSession.accountId) { removedSessionIds.add(session.id); return false; } return true; }); - cancelAccountStreams(serverSession.account.id); + cancelAccountStreams(serverSession.accountId); await writeSessions(sessions); await deleteCookie(event, "session"); @@ -34,7 +34,7 @@ export default defineEventHandler(async (event) => { await writeSubscriptions(subscriptions); // Remove the user - const account = users.find(user => user.id === serverSession.account.id)!; + const account = users.find(user => user.id === serverSession.accountId)!; const now = new Date().toISOString(); account.deleted = true; account.updatedAt = now; diff --git a/server/api/auth/account.patch.ts b/server/api/auth/account.patch.ts index dc228e9..75c65ea 100644 --- a/server/api/auth/account.patch.ts +++ b/server/api/auth/account.patch.ts @@ -9,7 +9,7 @@ import { z } from "zod/v4-mini"; export default defineEventHandler(async (event) => { - const session = await requireServerSession(event); + const session = await requireServerSessionWithUser(event); const { success, error, data: patch } = apiAccountPatchSchema.safeParse(await readBody(event)); if (!success) { throw createError({ @@ -39,7 +39,7 @@ export default defineEventHandler(async (event) => { } const users = await readUsers(); - const account = users.find(user => user.id === session.account.id); + const account = users.find(user => user.id === session.accountId); if (!account) { throw Error("Account does not exist"); } diff --git a/server/api/auth/account.post.ts b/server/api/auth/account.post.ts index d393cfe..527215c 100644 --- a/server/api/auth/account.post.ts +++ b/server/api/auth/account.post.ts @@ -6,7 +6,7 @@ import { readUsers, writeUsers, nextUserId, type ServerUser } from "~/server/dat import { broadcastEvent } from "~/server/streams"; export default defineEventHandler(async (event) => { - let session = await getServerSession(event); + let session = await getServerSession(event, false); if (session) { throw createError({ status: 409, diff --git a/server/api/auth/session.delete.ts b/server/api/auth/session.delete.ts index 29c6756..ca24eaf 100644 --- a/server/api/auth/session.delete.ts +++ b/server/api/auth/session.delete.ts @@ -2,12 +2,15 @@ SPDX-FileCopyrightText: © 2025 Hornwitser SPDX-License-Identifier: AGPL-3.0-or-later */ +import { readUsers } from "~/server/database"; import { cancelSessionStreams } from "~/server/streams"; export default defineEventHandler(async (event) => { - const session = await getServerSession(event); + const session = await getServerSession(event, true); if (session) { - if (session.account.type === "anonymous") { + const users = await readUsers(); + const account = users.find(user => user.id === session.accountId); + if (account?.type === "anonymous") { throw createError({ status: 409, message: "Cannot log out of an anonymous account", diff --git a/server/api/auth/session.get.ts b/server/api/auth/session.get.ts index 99c7107..f63f355 100644 --- a/server/api/auth/session.get.ts +++ b/server/api/auth/session.get.ts @@ -2,23 +2,23 @@ SPDX-FileCopyrightText: © 2025 Hornwitser SPDX-License-Identifier: AGPL-3.0-or-later */ -import { readSubscriptions } from "~/server/database"; +import { readSubscriptions, readUsers } from "~/server/database"; import type { ApiSession } from "~/shared/types/api"; export default defineEventHandler(async (event): Promise => { - const session = await getServerSession(event); + const session = await getServerSession(event, false); if (!session) return; + const users = await readUsers(); + const account = users.find(user => user.id === session.accountId); const subscriptions = await readSubscriptions(); const push = Boolean( subscriptions.find(sub => sub.type === "push" && sub.sessionId === session.id) ); - await refreshServerSession(event, session); - return { id: session.id, - account: session.account, + account, push, }; }) diff --git a/server/api/events.ts b/server/api/events.ts index 92cb35f..140c52e 100644 --- a/server/api/events.ts +++ b/server/api/events.ts @@ -6,8 +6,7 @@ import { pipeline } from "node:stream"; import { addStream, deleteStream } from "~/server/streams"; export default defineEventHandler(async (event) => { - const session = await getServerSession(event); - const accountId = session?.account.id; + const session = await getServerSession(event, false); const encoder = new TextEncoder(); const source = event.headers.get("x-forwarded-for"); @@ -26,7 +25,7 @@ export default defineEventHandler(async (event) => { deleteStream(stream.writable); } }) - addStream(stream.writable, session?.id, accountId); + addStream(stream.writable, session?.id, session?.accountId); // Workaround to properly handle stream errors. See https://github.com/unjs/h3/issues/986 setHeader(event, "Access-Control-Allow-Origin", "*"); diff --git a/server/api/schedule.patch.ts b/server/api/schedule.patch.ts index 364818d..49d365b 100644 --- a/server/api/schedule.patch.ts +++ b/server/api/schedule.patch.ts @@ -9,9 +9,9 @@ import { apiScheduleSchema } from "~/shared/types/api"; import { applyUpdatesToArray } from "~/shared/utils/update"; export default defineEventHandler(async (event) => { - const session = await requireServerSession(event); + const session = await requireServerSessionWithUser(event); - if (session.account.type !== "admin" && session.account.type !== "crew") { + if (session.access !== "admin" && session.access !== "crew") { throw createError({ status: 403, statusMessage: "Forbidden", @@ -45,7 +45,7 @@ export default defineEventHandler(async (event) => { } // Validate edit restrictions for crew - if (session.account.type === "crew") { + if (session.access === "crew") { if (update.locations?.length) { throw createError({ status: 403, diff --git a/server/api/schedule.ts b/server/api/schedule.ts index 4c59437..a9ab848 100644 --- a/server/api/schedule.ts +++ b/server/api/schedule.ts @@ -5,7 +5,7 @@ import { readSchedule } from "~/server/database"; export default defineEventHandler(async (event) => { - const session = await getServerSession(event); + const session = await getServerSession(event, false); const schedule = await readSchedule(); - return canSeeCrew(session?.account.type) ? schedule : filterSchedule(schedule); + return canSeeCrew(session?.access) ? schedule : filterSchedule(schedule); }); diff --git a/server/api/subscribe.post.ts b/server/api/subscribe.post.ts index 1528239..9c4a76a 100644 --- a/server/api/subscribe.post.ts +++ b/server/api/subscribe.post.ts @@ -11,7 +11,7 @@ const subscriptionSchema = z.strictObject({ }); export default defineEventHandler(async (event) => { - const session = await requireServerSession(event); + const session = await requireServerSessionWithUser(event); const { success, error, data: body } = subscriptionSchema.safeParse(await readBody(event)); if (!success) { throw createError({ diff --git a/server/api/unsubscribe.post.ts b/server/api/unsubscribe.post.ts index 415a7c9..61a3d51 100644 --- a/server/api/unsubscribe.post.ts +++ b/server/api/unsubscribe.post.ts @@ -5,7 +5,7 @@ import { readSubscriptions, writeSubscriptions } from "~/server/database"; export default defineEventHandler(async (event) => { - const session = await requireServerSession(event); + const session = await requireServerSessionWithUser(event); const subscriptions = await readSubscriptions(); const existingIndex = subscriptions.findIndex( sub => sub.type === "push" && sub.sessionId === session.id diff --git a/server/api/users/index.get.ts b/server/api/users/index.get.ts index 9cb4ac2..f6365fd 100644 --- a/server/api/users/index.get.ts +++ b/server/api/users/index.get.ts @@ -5,13 +5,13 @@ import { readUsers } from "~/server/database" export default defineEventHandler(async (event) => { - const session = await requireServerSession(event); + const session = await requireServerSessionWithUser(event); const users = await readUsers(); - if (session.account.type === "admin") { + if (session.access === "admin") { return users.map(serverUserToApi); } - if (session.account.type === "crew") { + if (session.access === "crew") { return users.filter(u => u.type === "crew" || u.type === "admin").map(serverUserToApi); } throw createError({ diff --git a/server/database.ts b/server/database.ts index eeae873..b369e24 100644 --- a/server/database.ts +++ b/server/database.ts @@ -7,15 +7,14 @@ import type { ApiSchedule, ApiSubscription, ApiUserType } from "~/shared/types/a import type { Id } from "~/shared/types/common"; export interface ServerSession { - id: number, - account: ServerUser, + id: Id, + access: ApiUserType, + accountId?: number, + expiresAtMs: number, + discardAtMs: number, + successor?: Id, }; -interface StoredSession { - id: number, - accountId: number, -} - export interface ServerUser { id: Id, updatedAt: string, @@ -131,26 +130,9 @@ export async function nextSessionId() { } export async function readSessions() { - const users = await readUsers(); - const sessions: ServerSession[] = []; - for (const stored of await readJson(sessionsPath, [])) { - const user = users.find(user => user.id === stored.accountId); - if (user) { - sessions.push({ - id: stored.id, - account: user, - }); - } - } - return sessions; + return readJson(sessionsPath, []) } export async function writeSessions(sessions: ServerSession[]) { - const stored: StoredSession[] = sessions.map( - session => ({ - id: session.id, - accountId: session.account.id, - }), - ); - await writeFile(sessionsPath, JSON.stringify(stored, undefined, "\t") + "\n", "utf-8"); + await writeFile(sessionsPath, JSON.stringify(sessions, undefined, "\t") + "\n", "utf-8"); } diff --git a/server/utils/session.ts b/server/utils/session.ts index 4575eb9..c33d3c7 100644 --- a/server/utils/session.ts +++ b/server/utils/session.ts @@ -3,9 +3,16 @@ SPDX-License-Identifier: AGPL-3.0-or-later */ import type { H3Event } from "h3"; -import { nextSessionId, readSessions, readSubscriptions, type ServerSession, type ServerUser, writeSessions, writeSubscriptions } from "~/server/database"; - -const oneYearSeconds = 365 * 24 * 60 * 60; +import { + nextSessionId, + readSessions, + readSubscriptions, + readUsers, + type ServerSession, + type ServerUser, + writeSessions, + writeSubscriptions +} from "~/server/database"; async function removeSessionSubscription(sessionId: number) { const subscriptions = await readSubscriptions(); @@ -40,48 +47,105 @@ export async function clearServerSession(event: H3Event) { export async function setServerSession(event: H3Event, account: ServerUser) { const sessions = await readSessions(); + const runtimeConfig = useRuntimeConfig(event); await clearServerSessionInternal(event, sessions); + const now = Date.now(); const newSession: ServerSession = { - account, + accountId: account.id, + access: account.type, + expiresAtMs: now + runtimeConfig.sessionExpiresTimeout * 1000, + discardAtMs: now + runtimeConfig.sessionDiscardTimeout * 1000, id: await nextSessionId(), }; sessions.push(newSession); await writeSessions(sessions); - await setSignedCookie(event, "session", String(newSession.id), oneYearSeconds) + await setSignedCookie(event, "session", String(newSession.id), runtimeConfig.sessionDiscardTimeout) } -export async function refreshServerSession(event: H3Event, session: ServerSession) { - await setSignedCookie(event, "session", String(session.id), oneYearSeconds) +async function rotateSession(event: H3Event, sessions: ServerSession[], session: ServerSession) { + const runtimeConfig = useRuntimeConfig(event); + const users = await readUsers(); + const account = users.find(user => user.id === session.accountId); + const now = Date.now(); + const newSession: ServerSession = { + accountId: account?.id, + access: account?.type ?? "anonymous", + expiresAtMs: now + runtimeConfig.sessionExpiresTimeout * 1000, + discardAtMs: now + runtimeConfig.sessionDiscardTimeout * 1000, + id: await nextSessionId(), + }; + session.successor = newSession.id; + sessions.push(newSession); + await writeSessions(sessions); + await setSignedCookie(event, "session", String(newSession.id), runtimeConfig.sessionDiscardTimeout) + return newSession; } -export async function getServerSession(event: H3Event) { +export async function getServerSession(event: H3Event, ignoreExpired: boolean) { const sessionCookie = await getSignedCookie(event, "session"); if (sessionCookie) { const sessionId = parseInt(sessionCookie, 10); const sessions = await readSessions(); - return sessions.find(session => session.id === sessionId); + const session = sessions.find(session => session.id === sessionId); + if (session) { + if (!ignoreExpired && session.successor !== undefined) { + throw createError({ + statusCode: 403, + statusMessage: "Forbidden", + message: "Session has been taken by another agent.", + }); + } + const now = Date.now(); + if (now >= session?.discardAtMs) { + return undefined; + } + if (!ignoreExpired && now >= session?.expiresAtMs) { + return await rotateSession(event, sessions, session); + } + } + return session; } } -export async function requireServerSession(event: H3Event) { - const session = await getServerSession(event); +export async function requireServerSession(event: H3Event, message: string) { + const session = await getServerSession(event, false); if (!session) throw createError({ - status: 401, - message: "Account session required", + statusCode: 401, + statusMessage: "Unauthorized", + message, }); return session; } +export async function requireServerSessionWithUser(event: H3Event) { + const message = "User session required"; + const session = await requireServerSession(event, message); + const users = await readUsers(); + const account = users.find(user => user.id === session.accountId); + if (session.accountId === undefined || !account) + throw createError({ + statusCode: 401, + statusMessage: "Uauthorized", + message, + }); + return { ...session, accountId: session.accountId }; +} + + export async function requireServerSessionWithAdmin(event: H3Event) { - const session = await requireServerSession(event); - if (session.account.type !== "admin") { + const message = "Admin session required"; + const session = await requireServerSession(event, message); + const users = await readUsers(); + const account = users.find(user => user.id === session.accountId); + if (session.access !== "admin" || account?.type !== "admin") { throw createError({ statusCode: 403, statusMessage: "Forbidden", + message, }); } - return session; + return { ...session, accountId: session.accountId }; }