Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions .changeset/signup-existing-address-explicit-refusal.md
Original file line number Diff line number Diff line change
@@ -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.
12 changes: 7 additions & 5 deletions content/docs/deployment/self-hosting.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion content/docs/references/api/contract.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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`. |
Expand Down Expand Up @@ -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`
Expand Down
1 change: 1 addition & 0 deletions content/docs/references/api/error-code-ledger.mdx
Original file line number Diff line number Diff line change
Expand Up @@ -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`
Expand Down
132 changes: 132 additions & 0 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down Expand Up @@ -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;

/**
Expand Down Expand Up @@ -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<boolean> {
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<boolean> {
const rows = await this.findPermissionSetRows(setName);
Expand Down
Loading
Loading