Skip to content

Commit aedbaef

Browse files
os-warrenclaude
andauthored
fix(plugin-auth): refuse a sign-up for an address that already exists, instead of a 200 for a row never written (#15738)
* wip(plugin-auth): refuse sign-up for an existing address instead of a synthetic 200 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * wip: changeset Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * chore(spec): regenerate references docs for the new ledger entry Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y * fix(plugin-auth): correct the self-hosting doc's measured claim, and make an unanswerable uniqueness probe loud Review round 1 on #15738: - content/docs/deployment/self-hosting.mdx published as MEASURED that a seeded person's registration under a widened posture answers 200 and persists nothing. That sentence is false after this branch: the request is now refused 422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL. Only the mechanism sentence moves; the bullet's conclusion (opening the posture is not enough on its own) is still true and stays. - hasExistingUserFor's catch fell through SILENTLY. The direction is correct and unchanged, but a failure specific to this query's shape re-opened #15587 with no signal, because the vendor's duplicate shield answers a synthetic 200 rather than raising. It now reports through audienceLogError, the facility the sibling probe already uses at its page ceiling. Pinned as case 7. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01XpTx2tbq3pZRYAdoGt6E6Y --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 5529a37 commit aedbaef

7 files changed

Lines changed: 571 additions & 6 deletions

File tree

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,28 @@
1+
---
2+
"@objectstack/plugin-auth": minor
3+
"@objectstack/spec": patch
4+
---
5+
6+
`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)
7+
8+
**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.
9+
10+
### What was measured
11+
12+
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.
13+
14+
### The mechanism
15+
16+
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.
17+
18+
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.
19+
20+
### The fix
21+
22+
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.
23+
24+
**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.
25+
26+
**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.
27+
28+
`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.

content/docs/deployment/self-hosting.mdx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -557,11 +557,13 @@ this step.
557557
**Two remedies that look like they work, measured:**
558558

559559
- **Opening the audience posture is not enough on its own.** With the posture
560-
widened to `email_domain`, a seeded person's own registration answers `200`
561-
and persists *nothing* — no new row, no account, and their sign-in is still
562-
`401`. A fresh address does get an account, but every posture other than
563-
`invite_only` forces email verification on, so its first sign-in is refused
564-
`403 EMAIL_NOT_VERIFIED` until a mail transport delivers the link.
560+
widened to `email_domain`, a seeded person's own registration is refused
561+
`422 USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL` and nothing is written — the same
562+
answer the `invite_only` default already gives for an address the directory
563+
holds, so the widened posture buys that person no login and their sign-in
564+
stays `401`. A fresh address does get an account, but every posture other
565+
than `invite_only` forces email verification on, so its first sign-in is
566+
refused `403 EMAIL_NOT_VERIFIED` until a mail transport delivers the link.
565567
- **A hand-written credential row authenticates nothing.** The `sys_account`
566568
row shape is public; the format of the secret stored in its `password`
567569
column is the platform's own. A row carrying a plaintext password is refused

content/docs/references/api/contract.mdx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@ const result = ApiErrorSchema.parse(data);
2727

2828
| Property | Type | Required | Description |
2929
| :--- | :--- | :--- | :--- |
30-
| **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) |
30+
| **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) |
3131
| **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) |
3232
| **message** | `string` || Readable error message |
3333
| **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);
333333
* `UPLOAD_SESSION_EXPIRED`
334334
* `UPLOAD_SESSION_NOT_FOUND`
335335
* `USER_ALREADY_EXISTS`
336+
* `USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL`
336337
* `VALIDATION_FAILED`
337338
* `VERSION_NOT_FOUND`
338339
* `VERSION_NOT_RESTORABLE`

content/docs/references/api/error-code-ledger.mdx

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,7 @@ const result = ErrorCode.parse(data);
449449
* `UPLOAD_SESSION_EXPIRED`
450450
* `UPLOAD_SESSION_NOT_FOUND`
451451
* `USER_ALREADY_EXISTS`
452+
* `USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL`
452453
* `VALIDATION_FAILED`
453454
* `VERSION_NOT_FOUND`
454455
* `VERSION_NOT_RESTORABLE`

packages/plugins/plugin-auth/src/auth-manager.ts

Lines changed: 132 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2059,6 +2059,57 @@ export class AuthManager {
20592059
message: refusal.errorDescription ?? refusal.error,
20602060
});
20612061
}
2062+
2063+
// ── [#15587] The UNIQUENESS refusal, raised here for the same
2064+
// reason the audience refusal above is — and STRICTLY after it.
2065+
//
2066+
// Same shield, its other arm. `shouldReturnGenericDuplicateResponse`
2067+
// (sign-up.mjs:163) is on whenever `requireEmailVerification` is on
2068+
// OR `autoSignIn === false`, and it guards TWO sites: the 403 catch
2069+
// at :235 (what the audience block above steps around) and the
2070+
// duplicate pre-check at :199. On the second, the vendor finds the
2071+
// existing row, logs it, and returns `buildGenericDuplicateResponse()`
2072+
// — a 200 carrying a freshly `generateId()`-ed user that is never
2073+
// written — INSTEAD of throwing USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL.
2074+
//
2075+
// We turn that shield on ourselves: a posture that permits
2076+
// self-registration FORCES `requireEmailVerification` on (see
2077+
// `createAuthInstance`), so `email_domain` and `open` sign-ups for an
2078+
// address that already exists answered 200 while `invite_only`
2079+
// answered 422 on the same population. Measured on a real ObjectQL
2080+
// engine with the posture held CONSTANT and only the verification flag
2081+
// moved, so the divergence is the flag's, not the posture's: zero
2082+
// inserts reach the engine, no `sys_account` appears, and the next
2083+
// sign-in is 401 with nothing anywhere explaining it. The silent
2084+
// success shape, on the recovery path a locked-out deployment walks.
2085+
//
2086+
// ORDER IS LOAD-BEARING: this runs only for a caller the posture
2087+
// ALREADY ADMITTED. Asking it first would hand an uninvited stranger
2088+
// an account-existence oracle under the `invite_only` DEFAULT (422 for
2089+
// a real address vs 403 for an unknown one) — inventing on the closed
2090+
// posture exactly what the vendor's shield exists to prevent. After
2091+
// the gate, `invite_only` is untouched: a stranger still gets
2092+
// SELF_REGISTRATION_CLOSED and learns nothing.
2093+
//
2094+
// UNCONDITIONAL, not a mirror of the vendor's predicate: the platform
2095+
// owns this refusal at one seam, so "an address that already has a
2096+
// `sys_user` row is refused" is one fact under every posture and every
2097+
// verification setting — rather than a contract that is a function of
2098+
// a vendor internal, and that a widened shield would silently reopen.
2099+
// Nothing is lost when the shield is off and the vendor would have
2100+
// answered: the code and message are the vendor's OWN constant, so the
2101+
// two lanes are byte-identical by construction rather than by copying.
2102+
// (The vendor's `onExistingUserSignUp` hook is not wired anywhere in
2103+
// this repo, and its timing-equalizing password hash equalizes against
2104+
// an oracle this 422 states outright.)
2105+
const signUpAddress = typeof ctx?.body?.email === 'string' ? ctx.body.email : '';
2106+
if (signUpAddress && (await this.hasExistingUserFor(signUpAddress))) {
2107+
const { APIError, BASE_ERROR_CODES } = await import('@better-auth/core/error');
2108+
throw APIError.from(
2109+
'UNPROCESSABLE_ENTITY',
2110+
BASE_ERROR_CODES.USER_ALREADY_EXISTS_USE_ANOTHER_EMAIL,
2111+
);
2112+
}
20622113
// fall through — the vendor still decides everything it owns
20632114
}
20642115

@@ -3906,6 +3957,14 @@ export class AuthManager {
39063957
* probe's read is narrowed to one address and its page chain is exhausted —
39073958
* so the value only trades round trips against page size.
39083959
*/
3960+
/**
3961+
* [#15587] Page bound for the sign-up uniqueness probe
3962+
* ({@link hasExistingUserFor}). One matching row IS the answer, so this only
3963+
* has to be large enough that a store which folds case or accents cannot
3964+
* push the real row off the page behind near-misses.
3965+
*/
3966+
private static readonly EXISTING_USER_PROBE_LIMIT = 50;
3967+
39093968
private static readonly PENDING_INVITATION_PROBE_PAGE = 50;
39103969

39113970
/**
@@ -4239,6 +4298,79 @@ export class AuthManager {
42394298
}
42404299
}
42414300

4301+
/**
4302+
* [#15587] Does a `sys_user` row already carry this address? Asked on the
4303+
* `/sign-up/email` before-hook so the uniqueness refusal is raised as an
4304+
* explicit 422 instead of being converted into a synthetic 200 by
4305+
* better-auth's anti-enumeration shield — see the call site for the
4306+
* mechanism and for why the question is asked AFTER the audience gate.
4307+
*
4308+
* ## Fail-open here is not fail-open overall
4309+
*
4310+
* An unanswerable probe returns `false` and the request FALLS THROUGH to the
4311+
* vendor, which runs its own `findUserByEmail` and decides. This pre-check
4312+
* only ever NARROWS an answer the vendor was going to give: it can turn a
4313+
* synthetic 200 into the honest 422, and it can never admit a creation the
4314+
* vendor would have refused. That is the opposite of
4315+
* {@link hasPendingInvitationFor}, whose `false` must fail CLOSED because it
4316+
* grants a carve-out — the two neighbours differ on purpose.
4317+
*
4318+
* ## Matching, and why the JS re-check narrows
4319+
*
4320+
* The address is pushed into the query (`sys_user.email` carries a declared
4321+
* index) with a bounded page, and every returned row is re-checked in JS
4322+
* against the normalized target. `=` folds case on some collations (MySQL's
4323+
* default) and folds ACCENTS with it, so the store alone could report a row
4324+
* for an address that is merely accent-adjacent — a refusal nothing
4325+
* justifies. Case-only differences still match, which is correct: the
4326+
* vendor lowercases `user.email` on `createUser`, so a case variant IS the
4327+
* same account. The re-check therefore only ever removes false positives —
4328+
* the safe direction for a rule whose output is a refusal.
4329+
*
4330+
* A page that comes back FULL is not evidence of anything beyond it, but it
4331+
* does not need to be: one matching row is the whole answer, and no row in
4332+
* a full page matching means the store answered a different question than
4333+
* we asked (a driver ignoring the predicate), which falls through to the
4334+
* vendor exactly like an unanswerable probe.
4335+
*/
4336+
private async hasExistingUserFor(email: string): Promise<boolean> {
4337+
const engine = this.config.dataEngine;
4338+
if (!engine || typeof (engine as any).find !== 'function') return false;
4339+
const target = email.trim().toLowerCase();
4340+
if (!target) return false;
4341+
try {
4342+
const reader = withSystemReadContext(engine) as any;
4343+
const raw = await reader.find(SystemObjectName.USER, {
4344+
where: { email: target },
4345+
limit: AuthManager.EXISTING_USER_PROBE_LIMIT,
4346+
});
4347+
const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : [];
4348+
return rows.some(
4349+
(row) => typeof row?.email === 'string' && row.email.trim().toLowerCase() === target,
4350+
);
4351+
} catch (error) {
4352+
// The fall-through DIRECTION stays (see above): the vendor runs its own
4353+
// `findUserByEmail` and still decides. What must not stay is the
4354+
// SILENCE. A total engine outage is loud by itself — the vendor's read
4355+
// goes through the same engine and the request answers 500 — but a
4356+
// failure specific to THIS query shape, or a transient one, is answered
4357+
// by the duplicate shield with a synthetic 200, which is #15587 exactly:
4358+
// the defect re-opens with no other signal anywhere. Measured by driving
4359+
// a throw scoped to this probe's own signature: the pre-fix response came
4360+
// back, and nothing named the probe. So the refusal that did not happen
4361+
// says so here, at the same level and through the same facility the
4362+
// sibling probe uses at its page ceiling.
4363+
this.audienceLogError(
4364+
'[audience] the sign-up existing-user probe could not be answered, so the uniqueness '
4365+
+ 'refusal was NOT raised for this request — falling through to better-auth, whose '
4366+
+ 'duplicate shield answers a synthetic 200 when email verification is forced on. '
4367+
+ 'A sign-up for an already-registered address may report success and write nothing.',
4368+
{ error: error instanceof Error ? error.message : String(error) },
4369+
);
4370+
return false;
4371+
}
4372+
}
4373+
42424374
/** At least one ACTIVE `sys_permission_set` row carries the declared name. */
42434375
private async selfRegistrationSetResolvable(setName: string): Promise<boolean> {
42444376
const rows = await this.findPermissionSetRows(setName);

0 commit comments

Comments
 (0)