diff --git a/.changeset/signup-existing-address-explicit-refusal.md b/.changeset/signup-existing-address-explicit-refusal.md new file mode 100644 index 0000000000..f642783331 --- /dev/null +++ b/.changeset/signup-existing-address-explicit-refusal.md @@ -0,0 +1,28 @@ +--- +"@objectstack/plugin-auth": minor +"@objectstack/spec": patch +--- + +`POST /sign-up/email` for an address that already has a `sys_user` row is refused explicitly, instead of answering 200 for a row that is never written (#15587) + +**This is a wire-behaviour change on one lane**: a call that answers `200 {"token":null,"user":{…}}` today answers `422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` after this change. Nothing is newly admitted — the response that changes is one that reported a creation that never happened. + +### What was measured + +Under audience posture `email_domain` (domain allowlisted, `selfRegistrationPermissionSet` resolvable), a sign-up for an address that already carried a `sys_user` row answered **200 with a freshly minted user id** and persisted nothing: no new `sys_user`, no `sys_account`, and the next sign-in a `401` with nothing anywhere explaining it. The same call on the same population under the `invite_only` default was refused honestly with `422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL`. An operator, a provisioning script or the console reading the status code concludes the account exists — and this sits directly on the recovery path a locked-out deployment walks, where widening the posture to let a seeded person register is exactly the remedy an operator is pointed at. + +### The mechanism + +better-auth's sign-up route computes `shouldReturnGenericDuplicateResponse = requireEmailVerification || autoSignIn === false` and, when it is on, answers a duplicate with a synthetic in-memory user instead of throwing. **No insert is attempted and nothing is swallowed**: the vendor's `findUserByEmail` short-circuits ahead of `createUser`, which is why no row and no credential appear. + +The posture is not itself the cause — it is only what arms the shield: a posture that permits self-registration **forces** `requireEmailVerification` on. Holding the posture constant at the `invite_only` default and moving only that flag reproduces the divergence exactly, which also means the defect was never confined to the widened postures: `emailAndPassword.autoSignIn: false` arms the same shield under any posture. + +### The fix + +The uniqueness refusal is raised on the `/sign-up/email` before-hook, the same seam and the same reason the audience-posture refusal is already raised there, and built from better-auth's own `BASE_ERROR_CODES` entry so both lanes answer byte-identically. + +**Order is load-bearing: it runs only for a caller the posture already admitted.** Asking uniqueness first would hand an uninvited stranger an account-existence oracle under the `invite_only` default (422 for a real address versus 403 for an unknown one). After the gate, `invite_only` is untouched — a stranger still gets `SELF_REGISTRATION_CLOSED` and learns nothing. + +**Operators of `open` / `email_domain` should know what the honest refusal costs:** on those postures a caller the audience gate admits can now distinguish an address that has an account from one that does not, where the synthetic 200 previously hid it. That is the disclosure the `invite_only` lane has always made to an invitation holder, and the platform's answer for a widened posture is now the same fact rather than a false receipt. + +`USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` is registered in the ADR-0112 error-code ledger under `@objectstack/plugin-auth`: the platform now **emits** it rather than only passing it through, and an emitted-but-unregistered code is the silent fourth state that ledger exists to prevent. diff --git a/content/docs/deployment/self-hosting.mdx b/content/docs/deployment/self-hosting.mdx index 88e19ea799..34985fe037 100644 --- a/content/docs/deployment/self-hosting.mdx +++ b/content/docs/deployment/self-hosting.mdx @@ -557,11 +557,13 @@ this step. **Two remedies that look like they work, measured:** - **Opening the audience posture is not enough on its own.** With the posture - widened to `email_domain`, a seeded person's own registration answers `200` - and persists *nothing* — no new row, no account, and their sign-in is still - `401`. A fresh address does get an account, but every posture other than - `invite_only` forces email verification on, so its first sign-in is refused - `403 EMAIL_NOT_VERIFIED` until a mail transport delivers the link. + widened to `email_domain`, a seeded person's own registration is refused + `422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` and nothing is written — the same + answer the `invite_only` default already gives for an address the directory + holds, so the widened posture buys that person no login and their sign-in + stays `401`. A fresh address does get an account, but every posture other + than `invite_only` forces email verification on, so its first sign-in is + refused `403 EMAIL_NOT_VERIFIED` until a mail transport delivers the link. - **A hand-written credential row authenticates nothing.** The `sys_account` row shape is public; the format of the secret stored in its `password` column is the platform's own. A row carrying a plaintext password is refused diff --git a/content/docs/references/api/contract.mdx b/content/docs/references/api/contract.mdx index b5fb06793a..76362bb2dc 100644 --- a/content/docs/references/api/contract.mdx +++ b/content/docs/references/api/contract.mdx @@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data); | Property | Type | Required | Description | | :--- | :--- | :--- | :--- | -| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +293 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | +| **code** | `Enum<'VALIDATION_ERROR' \| 'INVALID_FIELD' \| 'MISSING_REQUIRED_FIELD' \| 'INVALID_FORMAT' \| 'VALUE_TOO_LONG' \| 'VALUE_TOO_SHORT' \| 'VALUE_OUT_OF_RANGE' \| … +294 more>` | ✅ | Error code (e.g. VALIDATION_ERROR; StandardErrorCode ∪ the ledger the serving side registers — ERROR_CODE_LEDGER for framework packages) | | **declaredCode** | `string` | optional | The producer-declared code, verbatim, when it is not a member of the closed `code` vocabulary — the open, author-authored channel (app-specific spellings; ADR-0112) | | **message** | `string` | ✅ | Readable error message | | **userMessage** | `string` | optional | Producer-marked user-facing refusal text, verbatim. Present exactly when the producer opted in at throw time; consumers render it to end users and keep their generic substitution for anything unmarked. Status-agnostic; never replaces `message`. | @@ -333,6 +333,7 @@ const result = ApiErrorSchema.parse(data); * `UPLOAD_SESSION_EXPIRED` * `UPLOAD_SESSION_NOT_FOUND` * `USER_ALREADY_EXISTS` +* `USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` * `VALIDATION_FAILED` * `VERSION_NOT_FOUND` * `VERSION_NOT_RESTORABLE` diff --git a/content/docs/references/api/error-code-ledger.mdx b/content/docs/references/api/error-code-ledger.mdx index 590e7ec045..c457dbaf44 100644 --- a/content/docs/references/api/error-code-ledger.mdx +++ b/content/docs/references/api/error-code-ledger.mdx @@ -449,6 +449,7 @@ const result = ErrorCode.parse(data); * `UPLOAD_SESSION_EXPIRED` * `UPLOAD_SESSION_NOT_FOUND` * `USER_ALREADY_EXISTS` +* `USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` * `VALIDATION_FAILED` * `VERSION_NOT_FOUND` * `VERSION_NOT_RESTORABLE` diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index 0faf7791f7..c3b78c6798 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -2059,6 +2059,57 @@ export class AuthManager { message: refusal.errorDescription ?? refusal.error, }); } + + // ── [#15587] The UNIQUENESS refusal, raised here for the same + // reason the audience refusal above is — and STRICTLY after it. + // + // Same shield, its other arm. `shouldReturnGenericDuplicateResponse` + // (sign-up.mjs:163) is on whenever `requireEmailVerification` is on + // OR `autoSignIn === false`, and it guards TWO sites: the 403 catch + // at :235 (what the audience block above steps around) and the + // duplicate pre-check at :199. On the second, the vendor finds the + // existing row, logs it, and returns `buildGenericDuplicateResponse()` + // — a 200 carrying a freshly `generateId()`-ed user that is never + // written — INSTEAD of throwing USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL. + // + // We turn that shield on ourselves: a posture that permits + // self-registration FORCES `requireEmailVerification` on (see + // `createAuthInstance`), so `email_domain` and `open` sign-ups for an + // address that already exists answered 200 while `invite_only` + // answered 422 on the same population. Measured on a real ObjectQL + // engine with the posture held CONSTANT and only the verification flag + // moved, so the divergence is the flag's, not the posture's: zero + // inserts reach the engine, no `sys_account` appears, and the next + // sign-in is 401 with nothing anywhere explaining it. The silent + // success shape, on the recovery path a locked-out deployment walks. + // + // ORDER IS LOAD-BEARING: this runs only for a caller the posture + // ALREADY ADMITTED. Asking it first would hand an uninvited stranger + // an account-existence oracle under the `invite_only` DEFAULT (422 for + // a real address vs 403 for an unknown one) — inventing on the closed + // posture exactly what the vendor's shield exists to prevent. After + // the gate, `invite_only` is untouched: a stranger still gets + // SELF_REGISTRATION_CLOSED and learns nothing. + // + // UNCONDITIONAL, not a mirror of the vendor's predicate: the platform + // owns this refusal at one seam, so "an address that already has a + // `sys_user` row is refused" is one fact under every posture and every + // verification setting — rather than a contract that is a function of + // a vendor internal, and that a widened shield would silently reopen. + // Nothing is lost when the shield is off and the vendor would have + // answered: the code and message are the vendor's OWN constant, so the + // two lanes are byte-identical by construction rather than by copying. + // (The vendor's `onExistingUserSignUp` hook is not wired anywhere in + // this repo, and its timing-equalizing password hash equalizes against + // an oracle this 422 states outright.) + const signUpAddress = typeof ctx?.body?.email === 'string' ? ctx.body.email : ''; + if (signUpAddress && (await this.hasExistingUserFor(signUpAddress))) { + const { APIError, BASE_ERROR_CODES } = await import('@better-auth/core/error'); + throw APIError.from( + 'UNPROCESSABLE_ENTITY', + BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL, + ); + } // fall through — the vendor still decides everything it owns } @@ -3906,6 +3957,14 @@ export class AuthManager { * probe's read is narrowed to one address and its page chain is exhausted — * so the value only trades round trips against page size. */ + /** + * [#15587] Page bound for the sign-up uniqueness probe + * ({@link hasExistingUserFor}). One matching row IS the answer, so this only + * has to be large enough that a store which folds case or accents cannot + * push the real row off the page behind near-misses. + */ + private static readonly EXISTING_USER_PROBE_LIMIT = 50; + private static readonly PENDING_INVITATION_PROBE_PAGE = 50; /** @@ -4239,6 +4298,79 @@ export class AuthManager { } } + /** + * [#15587] Does a `sys_user` row already carry this address? Asked on the + * `/sign-up/email` before-hook so the uniqueness refusal is raised as an + * explicit 422 instead of being converted into a synthetic 200 by + * better-auth's anti-enumeration shield — see the call site for the + * mechanism and for why the question is asked AFTER the audience gate. + * + * ## Fail-open here is not fail-open overall + * + * An unanswerable probe returns `false` and the request FALLS THROUGH to the + * vendor, which runs its own `findUserByEmail` and decides. This pre-check + * only ever NARROWS an answer the vendor was going to give: it can turn a + * synthetic 200 into the honest 422, and it can never admit a creation the + * vendor would have refused. That is the opposite of + * {@link hasPendingInvitationFor}, whose `false` must fail CLOSED because it + * grants a carve-out — the two neighbours differ on purpose. + * + * ## Matching, and why the JS re-check narrows + * + * The address is pushed into the query (`sys_user.email` carries a declared + * index) with a bounded page, and every returned row is re-checked in JS + * against the normalized target. `=` folds case on some collations (MySQL's + * default) and folds ACCENTS with it, so the store alone could report a row + * for an address that is merely accent-adjacent — a refusal nothing + * justifies. Case-only differences still match, which is correct: the + * vendor lowercases `user.email` on `createUser`, so a case variant IS the + * same account. The re-check therefore only ever removes false positives — + * the safe direction for a rule whose output is a refusal. + * + * A page that comes back FULL is not evidence of anything beyond it, but it + * does not need to be: one matching row is the whole answer, and no row in + * a full page matching means the store answered a different question than + * we asked (a driver ignoring the predicate), which falls through to the + * vendor exactly like an unanswerable probe. + */ + private async hasExistingUserFor(email: string): Promise { + const engine = this.config.dataEngine; + if (!engine || typeof (engine as any).find !== 'function') return false; + const target = email.trim().toLowerCase(); + if (!target) return false; + try { + const reader = withSystemReadContext(engine) as any; + const raw = await reader.find(SystemObjectName.USER, { + where: { email: target }, + limit: AuthManager.EXISTING_USER_PROBE_LIMIT, + }); + const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : []; + return rows.some( + (row) => typeof row?.email === 'string' && row.email.trim().toLowerCase() === target, + ); + } catch (error) { + // The fall-through DIRECTION stays (see above): the vendor runs its own + // `findUserByEmail` and still decides. What must not stay is the + // SILENCE. A total engine outage is loud by itself — the vendor's read + // goes through the same engine and the request answers 500 — but a + // failure specific to THIS query shape, or a transient one, is answered + // by the duplicate shield with a synthetic 200, which is #15587 exactly: + // the defect re-opens with no other signal anywhere. Measured by driving + // a throw scoped to this probe's own signature: the pre-fix response came + // back, and nothing named the probe. So the refusal that did not happen + // says so here, at the same level and through the same facility the + // sibling probe uses at its page ceiling. + this.audienceLogError( + '[audience] the sign-up existing-user probe could not be answered, so the uniqueness ' + + 'refusal was NOT raised for this request — falling through to better-auth, whose ' + + 'duplicate shield answers a synthetic 200 when email verification is forced on. ' + + 'A sign-up for an already-registered address may report success and write nothing.', + { error: error instanceof Error ? error.message : String(error) }, + ); + return false; + } + } + /** At least one ACTIVE `sys_permission_set` row carries the declared name. */ private async selfRegistrationSetResolvable(setName: string): Promise { const rows = await this.findPermissionSetRows(setName); diff --git a/packages/plugins/plugin-auth/src/signup-existing-address-refusal.test.ts b/packages/plugins/plugin-auth/src/signup-existing-address-refusal.test.ts new file mode 100644 index 0000000000..94b2ab276c --- /dev/null +++ b/packages/plugins/plugin-auth/src/signup-existing-address-refusal.test.ts @@ -0,0 +1,395 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#15587] A sign-up for an address that ALREADY carries a `sys_user` row is + * refused explicitly — never answered 200 for a row that is never written. + * + * ## The defect, and which of its two candidate mechanisms it actually was + * + * Measured: under posture `email_domain` (domain allowlisted, permission set + * resolvable), `POST /sign-up/email` for an existing address answered **200** + * with a freshly minted user id, persisted **nothing** — no new `sys_user`, no + * `sys_account` — and the next sign-in was 401 with nothing anywhere + * explaining it. The same call on the same population under the `invite_only` + * default answered **422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL**. + * + * The card left the mechanism open: was the response synthesized on the + * forced-email-verification lane ahead of the uniqueness refusal, or was an + * insert attempted and swallowed? It is the FIRST, and cases ⓪ and ① are that + * finding pinned rather than asserted: + * + * - better-auth 1.7.2 `dist/api/routes/sign-up.mjs` computes + * `shouldReturnGenericDuplicateResponse = requireEmailVerification || + * autoSignIn === false` (:163). On a duplicate it does NOT reach + * `createUser`: `findUserByEmail` hits at :199 and returns + * `buildGenericDuplicateResponse()` — a 200 carrying a `generateId()`-ed + * user built in memory — instead of throwing at :212. + * - So no insert is attempted and nothing is swallowed. Case ① asserts that + * directly by counting engine `insert` calls across the request. + * - And the POSTURE is not the cause: it is only what turns the shield on. A + * self-registration-permitting posture FORCES `requireEmailVerification` + * (`createAuthInstance`). Case ⓪ holds the posture CONSTANT at the + * `invite_only` default and moves only that flag — 422 becomes the + * synthetic 200 — which is what makes "before the uniqueness refusal" a + * measurement rather than a reading of vendor source. + * + * ## Why a real engine + * + * The population predicate and the uniqueness check both live below the fake + * doubles: the probe reads `sys_user` through `withSystemReadContext` on the + * real ObjectQL engine, and the duplicate the vendor finds is a real row in a + * real table. A pin built on mocks passes against the unfixed code. + * `@objectstack/driver-sql` + better-sqlite3 `:memory:`, plugin-auth's own + * `authIdentityObjects`, driven through `AuthManager.handleRequest` — the + * card's own harness. + * + * ## The controls, and what each one would catch + * + * A refusal is cheap to make unconditional, so the cases that must be able to + * go red are the ones asserting the fix did NOT become "always refuse" and did + * NOT invent a new oracle: + * + * - ③ a NEW address on the same allowlisted domain is still admitted; + * - ④ the ORDER: under the `invite_only` default an UNINVITED stranger asking + * about an address that really exists still gets `SELF_REGISTRATION_CLOSED`, + * not 422. Asking uniqueness before the audience gate would hand that + * stranger an account-existence oracle on the DEFAULT posture — inventing + * on the closed door exactly what the vendor's shield exists to prevent. + * - ⑤ the two lanes answer byte-identically. The refusal is built from + * better-auth's own `BASE_ERROR_CODES` entry, so this is a drift detector: + * if the vendor re-words its message, the lanes part and this case reds. + */ + +import { describe, it, expect, afterEach } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { AuthManager } from './auth-manager.js'; +import { authIdentityObjects } from './manifest.js'; +import { SELF_REGISTRATION_CLOSED } from './audience-posture.js'; + +const BASE = 'http://localhost:3000'; +const AUTH = `${BASE}/api/v1/auth`; +const SECRET = 'test-secret-at-least-32-chars-long-15587'; +const PASSWORD = 'S3cure!Passw0rd-15587'; +const EXISTING = 'alice@corp.example'; + +/** The refusal the platform already ships on the `invite_only` lane. */ +const ALREADY_EXISTS = 'USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL'; + +/** + * The RBAC object the audience gate resolves `selfRegistrationPermissionSet` + * against lives in `@objectstack/plugin-security`; it is declared locally with + * only the columns the resolver reads (the `last-admin-guard` / + * `sso-register-platform-admin-gate` precedent) so a fixture adds no + * dependency edge to plugin-auth. + */ +const sysPermissionSet = { + name: 'sys_permission_set', + label: 'Permission Set', + fields: { + id: { name: 'id', type: 'text' as const, primaryKey: true }, + name: { name: 'name', type: 'text' as const }, + label: { name: 'label', type: 'text' as const }, + active: { name: 'active', type: 'boolean' as const }, + }, +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + const engine = engines.pop(); + try { + await (engine as unknown as { destroy?(): Promise })?.destroy?.(); + } catch { + /* noop */ + } + } +}); + +async function bootEngine(): Promise { + const engine = new ObjectQL(); + engines.push(engine); + engine.registerDriver( + new SqlDriver({ + client: 'better-sqlite3', + connection: { filename: ':memory:' }, + useNullAsDefault: true, + }), + true, + ); + await engine.init(); + for (const object of authIdentityObjects) { + engine.registry.registerObject(object as never, '@objectstack/plugin-auth'); + } + engine.registry.registerObject(sysPermissionSet as never, '@objectstack/plugin-security'); + await engine.syncSchemas(); + return engine; +} + +const SYSTEM = { context: { isSystem: true } } as never; + +/** The card's population: three human `sys_user` rows, zero `sys_account`. */ +async function seedPopulation(engine: ObjectQL): Promise { + for (const [id, email] of [ + ['usr_alice', EXISTING], + ['usr_bob', 'bob@corp.example'], + ['usr_carol', 'carol@corp.example'], + ] as const) { + await engine.insert('sys_user', { id, email, name: id }, SYSTEM); + } + await engine.insert( + 'sys_permission_set', + { id: 'ps_member_default', name: 'member_default', label: 'member_default', active: true }, + SYSTEM, + ); +} + +async function seedPendingInvitation(engine: ObjectQL, email: string): Promise { + await engine.insert( + 'sys_invitation', + { + id: `inv_${Math.random().toString(36).slice(2, 10)}`, + email: email.trim().toLowerCase(), + status: 'pending', + organization_id: 'org_15587', + role: 'member', + inviter_id: 'usr_bob', + expires_at: new Date(Date.now() + 3_600_000), + }, + SYSTEM, + ); +} + +function makeManager(engine: ObjectQL, config: Record = {}): AuthManager { + return new AuthManager({ + secret: SECRET, + baseUrl: BASE, + dataEngine: engine as never, + ...config, + } as never); +} + +/** The widened posture the card measured, and the recovery path it sits on. */ +const EMAIL_DOMAIN_POSTURE = { + audience: { + posture: 'email_domain', + allowedEmailDomains: ['corp.example'], + selfRegistrationPermissionSet: 'member_default', + }, +}; + +function signUp(manager: AuthManager, email: string): Promise { + return manager.handleRequest( + new Request(`${AUTH}/sign-up/email`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: BASE }, + body: JSON.stringify({ email, password: PASSWORD, name: 'Someone' }), + }), + ); +} + +function signIn(manager: AuthManager, email: string): Promise { + return manager.handleRequest( + new Request(`${AUTH}/sign-in/email`, { + method: 'POST', + headers: { 'content-type': 'application/json', origin: BASE }, + body: JSON.stringify({ email, password: PASSWORD }), + }), + ); +} + +async function readAll(engine: ObjectQL, object: string): Promise[]> { + const rows = await engine.find(object, { limit: 100 }, SYSTEM); + return (Array.isArray(rows) ? rows : []) as Record[]; +} + +/** + * Make ONLY the uniqueness probe's own read fail, leaving every other engine + * read working. This is the reviewer-measured shape from #15738: a total + * outage is loud by itself (the vendor's read goes through the same engine and + * the request answers 500), so the interesting failure is one specific to this + * query's signature — `sys_user` filtered by `email`. + */ +function breakExistingUserProbe(engine: ObjectQL): void { + const original = (engine as any).find.bind(engine); + (engine as any).find = (object: string, options?: any, ...rest: any[]) => { + if (object === 'sys_user' && typeof options?.where?.email === 'string') { + throw new Error('probe-scoped failure (simulated)'); + } + return original(object, options, ...rest); + }; +} + +/** A logger that records what the manager writes at error/warn level. */ +function recordingLogger(): { lines: string[]; logger: Record } { + const lines: string[] = []; + const push = (m: string) => { lines.push(String(m)); }; + return { lines, logger: { error: push, warn: push, info: () => {}, debug: () => {} } }; +} + +/** Count every insert attempt that reaches the engine, by object name. */ +function instrumentInserts(engine: ObjectQL): string[] { + const calls: string[] = []; + const original = (engine as any).insert.bind(engine); + (engine as any).insert = (...args: any[]) => { + calls.push(String(args[0])); + return original(...args); + }; + return calls; +} + +describe('[#15587] sign-up for an address that already exists is refused, not falsely receipted', () => { + it('⓪ THE MECHANISM: posture held constant, only requireEmailVerification moved — the honest 422 becomes a synthetic 200', async () => { + // Both legs run the DEFAULT `invite_only` posture with a pending + // invitation, so the audience gate admits identically and the only moving + // part is the flag that arms better-auth's anti-enumeration shield. This + // is what establishes "synthesized on the forced-verification lane" as a + // measurement rather than a reading of vendor source. + const shieldOffEngine = await bootEngine(); + await seedPopulation(shieldOffEngine); + await seedPendingInvitation(shieldOffEngine, EXISTING); + const shieldOff = await signUp(makeManager(shieldOffEngine), EXISTING); + + const shieldOnEngine = await bootEngine(); + await seedPopulation(shieldOnEngine); + await seedPendingInvitation(shieldOnEngine, EXISTING); + const shieldOn = await signUp( + makeManager(shieldOnEngine, { emailAndPassword: { requireEmailVerification: true } }), + EXISTING, + ); + + // The vendor's own answer when the shield is off — unchanged by this card. + expect(shieldOff.status).toBe(422); + expect(((await shieldOff.json()) as { code?: string }).code).toBe(ALREADY_EXISTS); + // …and with the shield on it is now the SAME answer. Before the fix this + // leg was `200 {"token":null,"user":{…synthetic…}}`. + expect(shieldOn.status).toBe(422); + expect(((await shieldOn.json()) as { code?: string }).code).toBe(ALREADY_EXISTS); + }); + + it('① THE DEFECT: posture email_domain, existing address — 422, nothing written, no insert even ATTEMPTED', async () => { + const engine = await bootEngine(); + await seedPopulation(engine); + const manager = makeManager(engine, EMAIL_DOMAIN_POSTURE); + const before = await readAll(engine, 'sys_user'); + const inserts = instrumentInserts(engine); + + const res = await signUp(manager, EXISTING); + + expect(res.status, `expected an explicit refusal, got: ${await res.clone().text()}`).toBe(422); + const body = (await res.json()) as { code?: string; user?: unknown }; + expect(body.code).toBe(ALREADY_EXISTS); + // ⛔ The shape this card is about: a 200 carrying a user id no row holds. + expect(body.user).toBeUndefined(); + + // Nothing was written, and — the mechanism finding — nothing was tried. + // A swallowed insert would show `sys_user` here. + expect(inserts).toEqual([]); + const after = await readAll(engine, 'sys_user'); + expect(after.map((u) => u.id).sort()).toEqual(before.map((u) => u.id).sort()); + expect(after.filter((u) => u.email === EXISTING).map((u) => u.id)).toEqual(['usr_alice']); + expect(await readAll(engine, 'sys_account')).toEqual([]); + }); + + it('② the refusal is the whole story: no credential appeared, so sign-in still refuses — and now something explains it', async () => { + const engine = await bootEngine(); + await seedPopulation(engine); + const manager = makeManager(engine, EMAIL_DOMAIN_POSTURE); + + const up = await signUp(manager, EXISTING); + const inRes = await signIn(manager, EXISTING); + + // The 401 was never the bug — being told 200 first was. + expect(up.status).toBe(422); + expect(inRes.status).toBe(401); + }); + + it('③ CONTROL: the fix did not become "always refuse" — a NEW address on the allowlisted domain is still admitted and really persists', async () => { + const engine = await bootEngine(); + await seedPopulation(engine); + const manager = makeManager(engine, EMAIL_DOMAIN_POSTURE); + + const res = await signUp(manager, 'dave@corp.example'); + + expect(res.status, `a new allowlisted registrant was refused: ${await res.clone().text()}`) + .toBeLessThan(300); + const users = await readAll(engine, 'sys_user'); + expect(users.map((u) => u.email)).toContain('dave@corp.example'); + // …and unlike the synthetic 200, this one left a credential behind. + expect((await readAll(engine, 'sys_account')).length).toBe(1); + }); + + it('④ ORDER: on the invite_only DEFAULT an uninvited stranger still gets SELF_REGISTRATION_CLOSED for a REAL address — no new existence oracle', async () => { + const engine = await bootEngine(); + await seedPopulation(engine); + const manager = makeManager(engine); + + const real = await signUp(manager, EXISTING); + const unknown = await signUp(manager, 'nobody@corp.example'); + + // Indistinguishable — which is the point. A uniqueness check asked BEFORE + // the audience gate would answer 422 here and 403 below. + expect(real.status).toBe(403); + expect(unknown.status).toBe(403); + expect(((await real.json()) as { code?: string }).code).toBe(SELF_REGISTRATION_CLOSED); + expect(((await unknown.json()) as { code?: string }).code).toBe(SELF_REGISTRATION_CLOSED); + }); + + it('⑤ the two lanes are byte-identical — the refusal is built from better-auth\'s own constant, so vendor re-wording reds here', async () => { + const vendorEngine = await bootEngine(); + await seedPopulation(vendorEngine); + await seedPendingInvitation(vendorEngine, EXISTING); + const vendorLane = await signUp(makeManager(vendorEngine), EXISTING); + + const ourEngine = await bootEngine(); + await seedPopulation(ourEngine); + const ourLane = await signUp(makeManager(ourEngine, EMAIL_DOMAIN_POSTURE), EXISTING); + + expect(ourLane.status).toBe(vendorLane.status); + expect(await ourLane.json()).toEqual(await vendorLane.json()); + }); + + it('⑦ an UNANSWERABLE probe keeps the fall-through direction but is never SILENT about it', async () => { + // Reviewer-measured on #15738: with a throw scoped to this probe's own + // signature the pre-fix response came back — 200, fresh id, no row — and + // nothing anywhere named the probe. The direction is deliberate and stays + // (the vendor still decides); what is pinned here is that the refusal that + // did NOT happen says so, so a query-shape-specific or transient failure + // cannot re-open #15587 with zero signal. + const engine = await bootEngine(); + await seedPopulation(engine); + const { lines, logger } = recordingLogger(); + const manager = makeManager(engine, { ...EMAIL_DOMAIN_POSTURE, logger }); + breakExistingUserProbe(engine); + + const res = await signUp(manager, EXISTING); + + // Direction unchanged: fell through to better-auth, which under a forced- + // verification posture answers with its synthetic duplicate response. + expect(res.status).toBeLessThan(300); + // …and the probe said so. This is the assertion the reviewer's measurement + // had nothing to match. + const named = lines.filter((l) => l.includes('existing-user probe could not be answered')); + expect(named.length, `nothing named the probe; logged: ${JSON.stringify(lines)}`).toBe(1); + expect(named[0]).toContain('uniqueness refusal was NOT raised'); + // Still nothing written — the fall-through did not invent a row either. + expect(await readAll(engine, 'sys_account')).toEqual([]); + }); + + it('⑥ the shield\'s OTHER trigger: autoSignIn:false arms it under any posture, and that lane is refused too', async () => { + // `shouldReturnGenericDuplicateResponse` is `requireEmailVerification || + // autoSignIn === false`, so the defect was never confined to the widened + // postures — a deployment that merely turns auto-sign-in off reached it on + // the `invite_only` default. + const engine = await bootEngine(); + await seedPopulation(engine); + await seedPendingInvitation(engine, EXISTING); + const manager = makeManager(engine, { emailAndPassword: { autoSignIn: false } }); + + const res = await signUp(manager, EXISTING); + + expect(res.status, `autoSignIn:false lane was not refused: ${await res.clone().text()}`).toBe(422); + expect(((await res.json()) as { code?: string }).code).toBe(ALREADY_EXISTS); + expect(await readAll(engine, 'sys_account')).toEqual([]); + }); +}); diff --git a/packages/spec/src/api/error-code-ledger.zod.ts b/packages/spec/src/api/error-code-ledger.zod.ts index b1a5e83f58..617a803094 100644 --- a/packages/spec/src/api/error-code-ledger.zod.ts +++ b/packages/spec/src/api/error-code-ledger.zod.ts @@ -417,6 +417,12 @@ export const ERROR_CODE_LEDGER = { 'SSO_REGISTER_FAILED', 'SSO_REGISTER_FORBIDDEN', 'USER_ALREADY_EXISTS', // pass-through from better-auth + // [#15587] Raised by US on `/sign-up/email` for an address that already + // carries a `sys_user` row, using better-auth's own BASE_ERROR_CODES entry + // so the refusal is byte-identical whichever lane produces it. Registered + // here because the platform now EMITS it rather than only passing it + // through: an emitted-but-unregistered code is the silent fourth state. + 'USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL', 'VALIDATION_FAILED', ], '@objectstack/plugin-sharing': [