-
Notifications
You must be signed in to change notification settings - Fork 20
fix(auth): unwedge PGlite sign-up by routing single-user guard through the transaction adapter #952
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
e8770d1
974a001
c30113d
17f3010
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,161 @@ | ||
| /** | ||
| * Integration test for the single-user-mode sign-up guard in | ||
| * `auth/index.tsx` (`databaseHooks.user.create.before`). | ||
| * | ||
| * Pins two contracts: | ||
| * | ||
| * 1. The guard counts real humans correctly — the synthetic | ||
| * install_operator row and the legacy bootstrap-user row are | ||
| * excluded, so the FIRST human signup proceeds and the SECOND is | ||
| * refused with SIGN_UP_DISABLED_IN_SINGLE_USER_MODE. | ||
| * | ||
| * 2. The guard does not deadlock. Sign-up runs inside Better Auth's | ||
| * runWithTransaction, which reserves the only pooled connection in | ||
| * PGlite mode (LOBU_DISABLE_PREPARE=1 → pool max=1). The hook must | ||
| * reuse that transaction connection via ctx.internalAdapter rather | ||
| * than asking getDb() for a second one. Run under | ||
| * `bun run test:pglite` this test reproduces issue #947: a regression | ||
| * to a fresh getDb() query hangs the request and fails on timeout. | ||
| * | ||
| * The test is backend-agnostic — it talks to the auth handler over a | ||
| * Request, reads DATABASE_URL like the rest of the suite, and so runs | ||
| * unchanged against external Postgres (default) and PGlite | ||
| * (LOBU_TEST_BACKEND=pglite). | ||
| */ | ||
|
|
||
| import { verifyPassword } from "better-auth/crypto"; | ||
| import { afterEach, beforeEach, describe, expect, it } from "vitest"; | ||
| import { clearAuthCacheForTests, createAuth } from "../../../auth/index"; | ||
| import { getEnvFromProcess } from "../../../utils/env"; | ||
| import { cleanupTestDatabase, getTestDb } from "../../setup/test-db"; | ||
|
|
||
| const SIGN_UP_URL = "http://localhost/api/auth/sign-up/email"; | ||
|
|
||
| interface SignUpResult { | ||
| status: number; | ||
| body: Record<string, unknown>; | ||
| } | ||
|
|
||
| async function signUp(input: { | ||
| email: string; | ||
| password: string; | ||
| name: string; | ||
| }): Promise<SignUpResult> { | ||
| const auth = await createAuth(getEnvFromProcess()); | ||
| const res = await auth.handler( | ||
| new Request(SIGN_UP_URL, { | ||
| method: "POST", | ||
| headers: { "content-type": "application/json" }, | ||
| body: JSON.stringify(input), | ||
| }), | ||
| ); | ||
| const body = (await res.json().catch(() => ({}))) as Record<string, unknown>; | ||
| return { status: res.status, body }; | ||
| } | ||
|
|
||
| async function seedUser(id: string, principalKind: string): Promise<void> { | ||
| const sql = getTestDb(); | ||
| await sql` | ||
| INSERT INTO "user" (id, name, email, "emailVerified", principal_kind, "createdAt", "updatedAt") | ||
| VALUES ( | ||
| ${id}, | ||
| ${id}, | ||
| ${`${id}@seed.test`}, | ||
| true, | ||
| ${principalKind}, | ||
| NOW(), | ||
| NOW() | ||
| ) | ||
| ON CONFLICT (id) DO NOTHING | ||
| `; | ||
| } | ||
|
|
||
| describe("single-user-mode sign-up guard", () => { | ||
| const originalSingleUser = process.env.LOBU_SINGLE_USER; | ||
| const originalSecret = process.env.BETTER_AUTH_SECRET; | ||
|
|
||
| beforeEach(async () => { | ||
| await cleanupTestDatabase(); | ||
| process.env.LOBU_SINGLE_USER = "1"; | ||
| // Deterministic secret so credential hashing + session signing work. | ||
| process.env.BETTER_AUTH_SECRET = "a".repeat(64); | ||
| // createAuth() memoizes per-org instances (TtlCache). Other test files | ||
| // build the "__system__" instance with LOBU_SINGLE_USER unset; without | ||
| // busting the cache we'd reuse that instance and the guard closure would | ||
| // read the wrong flag. | ||
| clearAuthCacheForTests(); | ||
| }); | ||
|
|
||
| afterEach(() => { | ||
| if (originalSingleUser === undefined) delete process.env.LOBU_SINGLE_USER; | ||
| else process.env.LOBU_SINGLE_USER = originalSingleUser; | ||
| if (originalSecret === undefined) delete process.env.BETTER_AUTH_SECRET; | ||
| else process.env.BETTER_AUTH_SECRET = originalSecret; | ||
| // Don't leak our LOBU_SINGLE_USER=1 instance into the shared cache — | ||
| // a later file's createAuth() would otherwise reuse it. | ||
| clearAuthCacheForTests(); | ||
| }); | ||
|
|
||
| it("admits the first human signup and makes a sign-in-ready row", async () => { | ||
| // Completes (no #947 deadlock) and returns 200. | ||
| const first = await signUp({ | ||
| email: "first@local.test", | ||
| password: "firstpassword99", | ||
| name: "First", | ||
| }); | ||
| expect(first.status).toBe(200); | ||
| const userId = (first.body.user as { id?: string } | undefined)?.id; | ||
| expect(userId).toBeTruthy(); | ||
| if (!userId) throw new Error("signup returned no user id"); | ||
|
|
||
| const sql = getTestDb(); | ||
| // input:false means the column was never sent on INSERT — the DB | ||
| // default 'human' must have filled it in (not NULL). | ||
| const rows = (await sql` | ||
| SELECT principal_kind FROM "user" WHERE id = ${userId} | ||
| `) as unknown as Array<{ principal_kind: string }>; | ||
| expect(rows[0]?.principal_kind).toBe("human"); | ||
|
|
||
| // The credential row must verify against the submitted password — | ||
| // proves the create transaction committed, not just returned 200. | ||
| const accounts = (await sql` | ||
| SELECT "providerId", password FROM "account" WHERE "userId" = ${userId} | ||
| `) as unknown as Array<{ providerId: string; password: string | null }>; | ||
| expect(accounts[0]?.providerId).toBe("credential"); | ||
| const hash = accounts[0]?.password; | ||
| expect(hash).toBeTruthy(); | ||
| if (!hash) throw new Error("credential account has no password hash"); | ||
| expect(await verifyPassword({ hash, password: "firstpassword99" })).toBe( | ||
| true, | ||
| ); | ||
| }); | ||
|
|
||
| it("refuses signup once a human already exists", async () => { | ||
| // Seed a committed human directly (not via a prior signup) so the | ||
| // precondition has a clean happens-before and doesn't depend on | ||
| // cross-request visibility timing under the shared test pool. | ||
| await seedUser("existing-human", "human"); | ||
|
|
||
| const res = await signUp({ | ||
| email: "second@local.test", | ||
| password: "secondpassword99", | ||
| name: "Second", | ||
| }); | ||
| expect(res.status).toBe(403); | ||
| expect(res.body.code).toBe("SIGN_UP_DISABLED_IN_SINGLE_USER_MODE"); | ||
| }); | ||
|
|
||
| it("does not count install_operator or bootstrap-user as the existing human", async () => { | ||
| await seedUser("user_install_seed", "install_operator"); | ||
| await seedUser("bootstrap-user", "human"); | ||
|
|
||
| // Neither seeded row is a real human, so the first human signup | ||
| // must still be admitted. | ||
| const first = await signUp({ | ||
| email: "first@local.test", | ||
| password: "firstpassword99", | ||
| name: "First", | ||
| }); | ||
| expect(first.status).toBe(200); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -68,6 +68,17 @@ function gravatarUrl(email: string): string { | |
| // The config (OAuth providers) rarely changes, so 60s TTL is safe. | ||
| const authCache = new TtlCache<ReturnType<typeof betterAuth>>(60_000); | ||
|
|
||
| /** | ||
| * Drop every cached betterAuth instance. Production never needs this (env is | ||
| * stable per-process), but integration tests that flip env vars like | ||
| * LOBU_SINGLE_USER between cases must bust the cache, or a stale instance | ||
| * built under the previous env serves the request and the hook closures | ||
| * read the wrong flag. | ||
| */ | ||
| export function clearAuthCacheForTests(): void { | ||
| authCache.clear(); | ||
| } | ||
|
|
||
| /** | ||
| * Create a better-auth instance with all plugins configured. | ||
| * | ||
|
|
@@ -217,6 +228,22 @@ export async function createAuth(env: Env, request?: Request) { | |
| // Tokens are reusable for both login AND connectors | ||
| socialProviders, | ||
|
|
||
| user: { | ||
| additionalFields: { | ||
| // Declared so the where-clause in the single-user guard | ||
| // below resolves through BA's adapter. DB column has | ||
| // `NOT NULL DEFAULT 'human'` (db/migrations/...principal_kind.sql), | ||
| // so `input: false` lets the default fill in on signup. | ||
| principalKind: { | ||
| type: "string", | ||
| fieldName: "principal_kind", | ||
| input: false, | ||
| returned: false, | ||
| required: false, | ||
| }, | ||
| }, | ||
| }, | ||
|
|
||
| account: { | ||
| accountLinking: { | ||
| enabled: true, | ||
|
|
@@ -592,38 +619,39 @@ export async function createAuth(env: Env, request?: Request) { | |
| databaseHooks: { | ||
| user: { | ||
| create: { | ||
| before: async (user) => { | ||
| // Single-user-mode chokepoint. The /api/auth/* middleware | ||
| before: async (user, ctx) => { | ||
| // Single-user-mode chokepoint. The /api/auth/* URL filter | ||
| // in index.ts blocks /api/auth/sign-up/*, but Better Auth | ||
| // also creates users on magic-link verify and on OAuth | ||
| // also creates users on magic-link verify and OAuth | ||
| // callbacks — paths the URL guard never sees. This hook | ||
| // runs immediately before every user INSERT regardless of | ||
| // how the request arrived; if LOBU_SINGLE_USER is on and | ||
| // the deployment already has a user, refuse to create a | ||
| // second one. Closes the fork-via-magic-link / fork-via- | ||
| // social-login backdoor codex flagged. | ||
| // fires before every user INSERT, so it's the one place | ||
| // that closes the fork-via-magic-link / fork-via-OAuth | ||
| // backdoor. | ||
| // | ||
| // The count goes through ctx.internalAdapter so it joins | ||
| // the in-flight transaction connection. Calling getDb() | ||
| // here would request a second pool connection while | ||
| // sign-up's runWithTransaction holds the only one — | ||
| // deadlock in PGlite mode (pool max=1). See #947. | ||
| // Missing ctx (called outside the BA endpoint pipeline) | ||
| // throws via `ctx!` → BA returns FAILED_TO_CREATE_USER, | ||
| // which is the fail-closed posture we want. | ||
| if (env.LOBU_SINGLE_USER === "1") { | ||
| const { getDb } = await import("../db/client"); | ||
| const sql = getDb(); | ||
| // Exclude the synthetic install_operator row | ||
| // (auto-provisioned at boot in ensureInstallOperator) | ||
| // AND the legacy bootstrap-user row (pre-PR #902) | ||
| // from the "deployment already has a user" count, so | ||
| // the first human signup can still proceed in | ||
| // single-user mode against upgraded installs that | ||
| // still carry a bootstrap-user row. See | ||
| // docs/install-operator-bootstrap.md. | ||
| const rows = (await sql` | ||
| SELECT count(*)::int AS count | ||
| FROM "user" | ||
| WHERE principal_kind <> 'install_operator' | ||
| AND id <> 'bootstrap-user' | ||
| `) as unknown as Array<{ count: number }>; | ||
| const existing = rows[0]?.count ?? 0; | ||
| // (auto-provisioned by ensureInstallOperator) AND the | ||
| // legacy bootstrap-user row (pre-PR #902) so the | ||
| // first human signup still proceeds on upgraded | ||
| // installs. See docs/install-operator-bootstrap.md. | ||
| const existing = | ||
| await ctx!.context.internalAdapter.countTotalUsers([ | ||
| { | ||
| field: "principalKind", | ||
| operator: "ne", | ||
| value: "install_operator", | ||
| }, | ||
| { field: "id", operator: "ne", value: "bootstrap-user" }, | ||
| ]); | ||
| if (existing > 0) { | ||
| // APIError so Better Auth turns this into a structured | ||
| // JSON response with the right status code, not an | ||
| // unhandled 500 with an empty body. | ||
| throw new APIError("FORBIDDEN", { | ||
| code: "SIGN_UP_DISABLED_IN_SINGLE_USER_MODE", | ||
| message: | ||
|
|
@@ -705,7 +733,7 @@ export async function createAuth(env: Env, request?: Request) { | |
| }, | ||
| account: { | ||
| create: { | ||
| before: async (account) => { | ||
| before: async (account, ctx) => { | ||
| // Carve-out: refuse OAuth account-linking onto the synthetic | ||
| // install_operator user. The operator authenticates via | ||
| // ENCRYPTION_KEY; admitting social-login linking would pin a | ||
|
|
@@ -716,13 +744,19 @@ export async function createAuth(env: Env, request?: Request) { | |
| // password-hash row at boot. See | ||
| // docs/install-operator-bootstrap.md. | ||
| if (account.providerId !== "credential") { | ||
| const { getDb } = await import("../db/client"); | ||
| const sql = getDb(); | ||
| const rows = (await sql` | ||
| SELECT principal_kind FROM "user" | ||
| WHERE id = ${account.userId} LIMIT 1 | ||
| `) as unknown as Array<{ principal_kind: string }>; | ||
| if (rows[0]?.principal_kind === "install_operator") { | ||
| // Route through ctx.internalAdapter so the lookup | ||
| // shares the in-flight transaction connection on the | ||
| // one path that wraps in runWithTransaction — | ||
| // createOAuthUser, called from OAuth callback for new | ||
| // users. Avoids the PGlite pool-max=1 deadlock; see | ||
| // #947. `/link-social` and existing-user callback | ||
| // links aren't transactional today but stay safe. | ||
| const linkedUser = | ||
| await ctx!.context.internalAdapter.findUserById(account.userId); | ||
| const principalKind = ( | ||
| linkedUser as { principalKind?: string } | null | ||
| )?.principalKind; | ||
| if (principalKind === "install_operator") { | ||
|
Comment on lines
+754
to
+759
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Inconsistent fail-closed behavior compared to signup hook. The If Proposed fix for fail-closed consistency if (account.providerId !== "credential") {
+ if (!ctx) {
+ throw new APIError("INTERNAL_SERVER_ERROR", {
+ code: "ACCOUNT_LINK_NO_AUTH_CONTEXT",
+ message:
+ "Account linking rejected: missing auth context for install_operator guard.",
+ });
+ }
const linkedUser =
- await ctx?.context.internalAdapter.findUserById(
+ await ctx.context.internalAdapter.findUserById(
account.userId,
);🤖 Prompt for AI Agents |
||
| throw new APIError("FORBIDDEN", { | ||
| code: "ACCOUNT_LINKING_NOT_ALLOWED_FOR_INSTALL_OPERATOR", | ||
| message: | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.