From 8c860f2d50ab08f32d3a41515e14b3a741d6d3aa Mon Sep 17 00:00:00 2001 From: Dave Date: Sat, 19 Sep 2026 12:47:18 +0200 Subject: [PATCH] fix(auth): verify sessions against PocketBase instead of trusting the cookie MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit getCurrentUser() only decoded the pb_auth cookie locally and trusted its embedded role/client/active fields directly — the PocketBase JS SDK holds no secret to check a JWT signature against, so a completely forged cookie (garbage signature, role: "owner", any email) was accepted as a real session. Verified exploitable end-to-end: a raw Cookie header against the live app, no browser and no password, rendered the owner's home page. This mattered most for actions writing through the superuser connection (log-work.ts, summarise.ts, ingest.ts, and critically users.ts for account creation) — those never route the caller's token through PocketBase's own rule engine at all, so the app-level role check was the ONLY gate, and it was reading an unverified claim. Actions using the caller's own token (cards.ts, projects.ts) still made the wrong authorization DECISION from the forged role, even though PocketBase's rule engine would have rejected the actual data write. Fixed by having getCurrentUser() call authRefresh(), which asks PocketBase to confirm the token is genuine; a forged or tampered token is refused there and treated as signed-out. Wrapped in React's cache() so the extra round-trip happens once per request, not once per call site (several pages call this more than once). Re-fetching the live record also means a deactivated account (`active: false`) stops working on its very next request instead of only being blocked at a new login. Regression test builds the exact forged cookie used to verify this, confirmed red against the vulnerable code and green against the fix. Co-Authored-By: Claude Sonnet 5 --- src/lib/pb/server.ts | 56 ++++++++++-- .../integration/session-verification.test.ts | 85 +++++++++++++++++++ 2 files changed, 136 insertions(+), 5 deletions(-) create mode 100644 tests/integration/session-verification.test.ts diff --git a/src/lib/pb/server.ts b/src/lib/pb/server.ts index b3dfc67..0a60793 100644 --- a/src/lib/pb/server.ts +++ b/src/lib/pb/server.ts @@ -1,5 +1,6 @@ import "server-only"; +import { cache } from "react"; import PocketBase from "pocketbase"; import { cookies } from "next/headers"; @@ -15,6 +16,14 @@ export { AUTH_COOKIE }; * built fresh per request. Never share one across requests: PocketBase's auth * store is instance state, and a shared instance leaks one user's token into * another user's request. + * + * `loadFromCookie` only decodes the cookie locally — it does not verify the + * token's signature, so a client carries no proof here of who they claim to + * be. That is fine for THIS function: every write made with the resulting + * client still passes through PocketBase's own rule engine, which checks the + * real signature against the real request. It is getCurrentUser(), below, + * that turns a claim into an authorization decision, and that is where the + * token must actually be verified. */ export async function getServerPb(): Promise { const pb = new PocketBase(PB_URL); @@ -26,9 +35,46 @@ export async function getServerPb(): Promise { return pb; } -/** The signed-in user, or null. Does not throw. */ -export async function getCurrentUser(): Promise { +/** + * The signed-in user, verified against PocketBase — or null. Does not throw. + * + * `pb.authStore.loadFromCookie()` only decodes the cookie locally; the SDK + * holds no secret to check a signature against, so nothing about a decoded + * cookie confirms it is genuine. A cookie is just an HTTP header — anyone can + * send an arbitrary one directly, no browser required — so a forged cookie + * claiming `role: "owner"` previously passed straight through as a real + * session. `authRefresh()` asks PocketBase itself; a forged or tampered + * token gets refused there, which is the only place in this codebase that + * can actually tell the difference. + * + * This is the ONLY check standing between a crafted request and full owner + * impersonation for every action that writes through the superuser + * connection (getAdminPb() checks nothing itself, by design) — cards.ts-style + * actions using the caller's own token have PocketBase's rule engine as a + * second real gate, but those actions still make their authorization + * DECISION (e.g. which UI to render) from this function's answer, so it has + * to be correct too, not just eventually backstopped. + * + * Re-fetching the record here also means `role`, `client`, and `active` are + * always the live database values, never the client-supplied cookie's copy + * of them — a deactivated account stops working on its very next request, + * not only at its next login attempt. + * + * Wrapped in React's cache() so the extra round-trip to PocketBase happens + * once per request no matter how many places call this within one render — + * several pages in this app call it more than once. + */ +export const getCurrentUser = cache(async (): Promise => { const pb = await getServerPb(); - if (!pb.authStore.isValid || !pb.authStore.record) return null; - return pb.authStore.record as unknown as UserRecord; -} \ No newline at end of file + if (!pb.authStore.isValid) return null; + + try { + const { record } = await pb.collection("users").authRefresh(); + if (record.active === false) return null; + return record as unknown as UserRecord; + } catch { + // Invalid signature, expired token, or the account no longer exists — + // all of these mean "not a real session," never "trust the cookie." + return null; + } +}); \ No newline at end of file diff --git a/tests/integration/session-verification.test.ts b/tests/integration/session-verification.test.ts new file mode 100644 index 0000000..54c0485 --- /dev/null +++ b/tests/integration/session-verification.test.ts @@ -0,0 +1,85 @@ +import { describe, it, expect } from "vitest"; +import PocketBase from "pocketbase"; + +import { APP_URL, DEMO, requireServices, signIn } from "./helpers"; + +/** + * Regression guard for a live full-owner-impersonation hole. + * + * `getCurrentUser()` used to trust `pb.authStore.loadFromCookie()`'s decoded + * payload directly. That call only base64-decodes the cookie locally — the + * SDK holds no secret to check a signature against — so a completely forged + * cookie (garbage signature, `role: "owner"` claim, an email nobody owns) + * was accepted as a real owner session by every action and page in the app. + * Verified exploitable against this exact code path before the fix: a raw + * `Cookie` header, no browser, no password, full owner access. + * + * The fix makes `getCurrentUser()` call `authRefresh()`, which asks + * PocketBase itself to confirm the token. If this test ever passes again + * without the app being signed in as owner, that verification has been + * removed or bypassed. + */ + +/** A cookie shaped exactly like a real one, with a token PocketBase never issued. */ +function forgedOwnerCookie(): string { + const pb = new PocketBase("http://forged.invalid"); + + function b64url(obj: unknown): string { + return Buffer.from(JSON.stringify(obj)).toString("base64url"); + } + const header = b64url({ alg: "HS256", typ: "JWT" }); + const payload = b64url({ + id: "forged-owner-id", + type: "auth", + collectionId: "_pb_users_auth_", + exp: Math.floor(Date.now() / 1000) + 3600, + }); + const forgedToken = `${header}.${payload}.not_a_real_signature`; + + // Load a fabricated record onto the auth store so exportToCookie() embeds + // it — this is exactly what a legitimate login also does, only here the + // token and record were never issued by PocketBase. + pb.authStore.save(forgedToken, { + id: "forged-owner-id", + email: "attacker@example.com", + role: "owner", + client: "", + active: true, + } as never); + + return pb.authStore.exportToCookie({}, "pb_auth"); +} + +describe("a forged session cookie cannot impersonate the owner", () => { + it("refuses a request bearing a self-signed, PocketBase-never-issued cookie", async () => { + await requireServices(); + + const response = await fetch(`${APP_URL}/`, { + headers: { cookie: forgedOwnerCookie() }, + redirect: "manual", + }); + + // A verified session check treats this as signed-out: redirected to + // /login, never a 200 rendering the owner's home page. + if (response.status >= 300 && response.status < 400) { + expect(response.headers.get("location")).toContain("/login"); + } else { + // Some deployments render the redirect target directly rather than + // a 3xx — either is acceptable, but the OWNER'S home page must not + // have rendered. + const html = await response.text(); + expect(html).not.toContain("Everything you are building"); + } + }); + + it("still accepts a real, freshly signed-in session", async () => { + await requireServices(); + + const cookie = await signIn(DEMO.ownerEmail, DEMO.ownerPassword); + const response = await fetch(`${APP_URL}/`, { + headers: { cookie }, + }); + + expect(response.status).toBe(200); + }); +});