diff --git a/apps/web/src/app/api/trpc/[trpc]/route.ts b/apps/web/src/app/api/trpc/[trpc]/route.ts index bc546ae..5b8461b 100644 --- a/apps/web/src/app/api/trpc/[trpc]/route.ts +++ b/apps/web/src/app/api/trpc/[trpc]/route.ts @@ -2,6 +2,7 @@ import { createTRPCContext, loadRoleDefaults } from "@capakraken/api"; import { appRouter } from "@capakraken/api/router"; import { prisma } from "@capakraken/db"; import { fetchRequestHandler } from "@trpc/server/adapters/fetch"; +import { getToken } from "next-auth/jwt"; import type { NextRequest } from "next/server"; import { auth } from "~/server/auth.js"; @@ -42,9 +43,19 @@ const handler = async (req: NextRequest) => { // Sessions kicked by concurrent-session limits or manual logout are rejected immediately. // Fail-open: if the table doesn't exist yet (pending migration) the check is skipped. // In E2E test mode the jwt callback skips registration, so skip validation too. + // + // We decode the JWT directly (not session.user.jti) because the session + // token is client-visible and therefore must not carry internal + // session-revocation identifiers — see security ticket #41. const isE2eTestMode = process.env["E2E_TEST_MODE"] === "true"; if (session?.user && !isE2eTestMode) { - const jti = (session.user as typeof session.user & { jti?: string }).jti; + const secret = process.env["AUTH_SECRET"] ?? process.env["NEXTAUTH_SECRET"] ?? ""; + const cookieName = + (process.env["AUTH_URL"] ?? "").startsWith("https://") || process.env["VERCEL"] === "1" + ? "__Host-authjs.session-token" + : "authjs.session-token"; + const jwt = secret ? await getToken({ req, secret, salt: cookieName }) : null; + const jti = (jwt?.["sid"] as string | undefined) ?? undefined; if (jti) { try { const activeSession = await prisma.activeSession.findUnique({ where: { jti } }); diff --git a/apps/web/src/server/auth-config.test.ts b/apps/web/src/server/auth-config.test.ts new file mode 100644 index 0000000..606f98a --- /dev/null +++ b/apps/web/src/server/auth-config.test.ts @@ -0,0 +1,79 @@ +/** + * Cookie-hardening regression tests — security ticket #41. + * + * auth.config.ts uses module-level env reads, so we reset modules and stub + * the relevant variables before each importing the module freshly. + */ + +import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; + +function originalEnvSnapshot() { + return { + AUTH_URL: process.env["AUTH_URL"], + NEXTAUTH_URL: process.env["NEXTAUTH_URL"], + VERCEL: process.env["VERCEL"], + NODE_ENV: process.env["NODE_ENV"], + }; +} + +describe("auth.config cookies", () => { + let snapshot: ReturnType; + + beforeEach(() => { + snapshot = originalEnvSnapshot(); + delete process.env["AUTH_URL"]; + delete process.env["NEXTAUTH_URL"]; + delete process.env["VERCEL"]; + vi.resetModules(); + }); + + afterEach(() => { + for (const [k, v] of Object.entries(snapshot)) { + if (v === undefined) delete process.env[k]; + else process.env[k] = v; + } + vi.resetModules(); + }); + + it("sets secure=true and __Host- prefix when AUTH_URL is https", async () => { + process.env["AUTH_URL"] = "https://app.example.com"; + const { authConfig } = await import("./auth.config.js"); + expect(authConfig.cookies?.sessionToken?.options?.secure).toBe(true); + expect(authConfig.cookies?.sessionToken?.name).toBe("__Host-authjs.session-token"); + expect(authConfig.cookies?.callbackUrl?.name).toBe("__Host-authjs.callback-url"); + expect(authConfig.cookies?.csrfToken?.name).toBe("__Host-authjs.csrf-token"); + }); + + it("sets secure=false on http deployment", async () => { + process.env["AUTH_URL"] = "http://localhost:3000"; + const { authConfig } = await import("./auth.config.js"); + expect(authConfig.cookies?.sessionToken?.options?.secure).toBe(false); + expect(authConfig.cookies?.sessionToken?.name).toBe("authjs.session-token"); + }); + + it("ignores NODE_ENV — secure flag tied to AUTH_URL scheme only", async () => { + // Staging: NODE_ENV=production but AUTH_URL is plain http → still insecure. + // The point is that the flag should NOT depend on NODE_ENV any more. + // (process.env.NODE_ENV is read-only in the Next.js tsconfig; force via index.) + (process.env as Record)["NODE_ENV"] = "production"; + process.env["AUTH_URL"] = "http://staging.internal"; + const { authConfig } = await import("./auth.config.js"); + expect(authConfig.cookies?.sessionToken?.options?.secure).toBe(false); + }); + + it("uses __Host- prefix on Vercel even without explicit AUTH_URL", async () => { + process.env["VERCEL"] = "1"; + const { authConfig } = await import("./auth.config.js"); + expect(authConfig.cookies?.sessionToken?.options?.secure).toBe(true); + expect(authConfig.cookies?.sessionToken?.name).toBe("__Host-authjs.session-token"); + }); + + it("keeps sameSite=strict, httpOnly=true, path=/ in all configurations", async () => { + process.env["AUTH_URL"] = "https://app.example.com"; + const { authConfig } = await import("./auth.config.js"); + const opts = authConfig.cookies?.sessionToken?.options; + expect(opts?.sameSite).toBe("strict"); + expect(opts?.httpOnly).toBe(true); + expect(opts?.path).toBe("/"); + }); +}); diff --git a/apps/web/src/server/auth.config.ts b/apps/web/src/server/auth.config.ts index 6d7e6dd..c93094e 100644 --- a/apps/web/src/server/auth.config.ts +++ b/apps/web/src/server/auth.config.ts @@ -3,6 +3,35 @@ import type { NextAuthConfig } from "next-auth"; // Edge-safe auth config — no native modules (no argon2, no prisma). // Used by auth-edge.ts (middleware) to verify JWT sessions without // pulling in Node.js-only packages into the Edge runtime. + +// Secure cookies whenever the deployment URL is https, not only when +// NODE_ENV === "production". Staging over HTTPS must also ship Secure +// cookies, otherwise the session token is MITM-interceptable. The check +// happens at module-eval time — that's fine because the AUTH_URL / Next.js +// deployment URL does not change between requests. +function isHttpsDeployment(): boolean { + const explicit = (process.env["AUTH_URL"] ?? process.env["NEXTAUTH_URL"] ?? "").trim(); + if (explicit.startsWith("https://")) return true; + // Vercel sets VERCEL=1 and the URL is always https there. + if (process.env["VERCEL"] === "1") return true; + return false; +} + +const useSecure = isHttpsDeployment(); + +// Cookie name with __Host- prefix when secure. The __Host- prefix is an +// additional browser-enforced hardening (RFC 6265bis §4.1.3.2) that only +// accepts the cookie if Secure=true, Path="/", and no Domain attribute — +// preventing subdomain takeover from rewriting the session cookie. +const cookiePrefix = useSecure ? "__Host-" : ""; + +const baseCookieOptions = { + httpOnly: true, + sameSite: "strict" as const, + path: "/", + secure: useSecure, +}; + export const authConfig = { pages: { signIn: "/auth/signin", @@ -10,36 +39,21 @@ export const authConfig = { providers: [], session: { strategy: "jwt", - maxAge: 28800, // 8 hours absolute timeout - updateAge: 1800, // refresh token every 30 minutes + maxAge: 28800, // 8 hours absolute timeout + updateAge: 1800, // refresh token every 30 minutes }, cookies: { sessionToken: { - name: "authjs.session-token", - options: { - httpOnly: true, - sameSite: "strict" as const, - path: "/", - secure: process.env.NODE_ENV === "production", - }, + name: `${cookiePrefix}authjs.session-token`, + options: baseCookieOptions, }, callbackUrl: { - name: "authjs.callback-url", - options: { - httpOnly: true, - sameSite: "strict" as const, - path: "/", - secure: process.env.NODE_ENV === "production", - }, + name: `${cookiePrefix}authjs.callback-url`, + options: baseCookieOptions, }, csrfToken: { - name: "authjs.csrf-token", - options: { - httpOnly: true, - sameSite: "strict" as const, - path: "/", - secure: process.env.NODE_ENV === "production", - }, + name: `${cookiePrefix}authjs.csrf-token`, + options: baseCookieOptions, }, }, } satisfies NextAuthConfig; diff --git a/apps/web/src/server/auth.test.ts b/apps/web/src/server/auth.test.ts index c14481b..c7ace51 100644 --- a/apps/web/src/server/auth.test.ts +++ b/apps/web/src/server/auth.test.ts @@ -14,12 +14,29 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; // ── next-auth imports next/server without .js extension which fails in vitest // node env. Mock the whole module so the error classes can be imported. +// Capture the config passed to NextAuth() so callbacks can be invoked. +const nextAuthCalls: Array<{ + callbacks?: { + jwt?: (...args: unknown[]) => unknown; + session?: (...args: unknown[]) => unknown; + }; +}> = []; vi.mock("next-auth", () => { class CredentialsSignin extends Error { code = "credentials"; } return { - default: vi.fn().mockReturnValue({ handlers: {}, auth: vi.fn() }), + default: vi.fn( + (cfg: { + callbacks?: { + jwt?: (...args: unknown[]) => unknown; + session?: (...args: unknown[]) => unknown; + }; + }) => { + nextAuthCalls.push(cfg); + return { handlers: {}, auth: vi.fn() }; + }, + ), CredentialsSignin, }; }); @@ -82,6 +99,63 @@ describe("MFA CredentialsSignin error classes — code property", () => { }); }); +describe("session() — does not leak JTI to client", () => { + const sessionCb = nextAuthCalls[0]?.callbacks?.session; + if (!sessionCb) { + it.skip("session callback not captured", () => {}); + return; + } + + it("never assigns token.sid onto session.user.jti", async () => { + const session = await sessionCb({ + session: { user: { email: "x@e.com" }, expires: "2030-01-01" }, + token: { sub: "u1", role: "USER", sid: "secret-session-id" }, + }); + const user = (session as { user: Record }).user; + expect(user["jti"]).toBeUndefined(); + expect(user["sid"]).toBeUndefined(); + expect(user["id"]).toBe("u1"); + expect(user["role"]).toBe("USER"); + }); +}); + +describe("jwt() — concurrent-session enforcement is fail-closed", () => { + const jwtCb = nextAuthCalls[0]?.callbacks?.jwt; + if (!jwtCb) { + it.skip("jwt callback not captured", () => {}); + return; + } + + beforeEach(() => { + prismaMock.systemSettings.findUnique.mockReset(); + prismaMock.activeSession.create.mockReset(); + prismaMock.activeSession.findMany.mockReset(); + prismaMock.activeSession.deleteMany.mockReset(); + }); + + it("throws if activeSession.create fails", async () => { + prismaMock.systemSettings.findUnique.mockResolvedValue({ maxConcurrentSessions: 3 }); + prismaMock.activeSession.create.mockRejectedValue(new Error("db down")); + + await expect(jwtCb({ token: {}, user: { id: "u1", role: "USER" } })).rejects.toThrow( + /Session registration failed/, + ); + }); + + it("returns the token when session-registry writes succeed", async () => { + prismaMock.systemSettings.findUnique.mockResolvedValue({ maxConcurrentSessions: 3 }); + prismaMock.activeSession.create.mockResolvedValue({}); + prismaMock.activeSession.findMany.mockResolvedValue([]); + + const result = (await jwtCb({ token: {}, user: { id: "u1", role: "USER" } })) as Record< + string, + unknown + >; + expect(result["role"]).toBe("USER"); + expect(typeof result["sid"]).toBe("string"); + }); +}); + describe("authorize() — login timing / enumeration defence", () => { const authorize = credentialsCalls[0]?.authorize; diff --git a/apps/web/src/server/auth.ts b/apps/web/src/server/auth.ts index 7cd26f6..c1f302c 100644 --- a/apps/web/src/server/auth.ts +++ b/apps/web/src/server/auth.ts @@ -267,10 +267,9 @@ const config = { if (token.role) { (session.user as typeof session.user & { role: string }).role = token.role as string; } - // Use token.sid (not token.jti) to avoid conflict with Auth.js's internal JWT ID claim - if (token.sid) { - (session.user as typeof session.user & { jti: string }).jti = token.sid as string; - } + // Do NOT expose token.sid on session.user — the JTI is an internal + // session-revocation token and must stay inside the encrypted JWT. + // Server-side handlers that need it decode the JWT via getToken(). return session; }, async jwt({ token, user }) { @@ -289,7 +288,11 @@ const config = { const isE2eTestMode = process.env["E2E_TEST_MODE"] === "true"; if (isE2eTestMode) return token; - // Enforce concurrent session limit (kick-oldest strategy) + // Enforce concurrent session limit (kick-oldest strategy). + // This MUST fail-closed: if session-registry writes fail we cannot + // honour the configured session cap, so we must refuse to mint a + // session. Previously this path swallowed errors and logged-only, + // which let a DB-degradation scenario bypass the session cap. try { const settings = await prisma.systemSettings.findUnique({ where: { id: "singleton" }, @@ -297,12 +300,10 @@ const config = { }); const maxSessions = settings?.maxConcurrentSessions ?? 3; - // Register this new session await prisma.activeSession.create({ data: { userId: user.id!, jti }, }); - // Count active sessions and delete the oldest if over the limit const activeSessions = await prisma.activeSession.findMany({ where: { userId: user.id! }, orderBy: { createdAt: "asc" }, @@ -320,8 +321,11 @@ const config = { ); } } catch (err) { - // Non-blocking: don't prevent login if session tracking fails - logger.error({ err }, "Failed to enforce concurrent session limit"); + logger.error( + { err, userId: user.id }, + "Failed to register active session — refusing to mint JWT", + ); + throw new Error("Session registration failed"); } } return token;