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
42 changes: 42 additions & 0 deletions .changeset/invitation-probe-page-ceiling.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
---
"@objectstack/plugin-auth": patch
---

fix(plugin-auth): the invitation carve-out stopped admitting past 200 pending invitations (#11770)

Under the `invite_only` audience posture that #11739 made the default, an
administrator could send an invitation and the invitee's account creation would
still be refused with `SELF_REGISTRATION_CLOSED` — silently, with no signal to
either party — as soon as the environment held more than 200 concurrently
pending invitations. A 500-employee onboarding is an ordinary way to reach that.

`AuthManager.hasPendingInvitationFor` answered "does this address hold a pending
invitation?" by reading at most 200 rows filtered only on `status = 'pending'`
and scanning them in memory for a case-insensitive email match. Past the first
page the invitee simply was not there, so the fail-closed `catch`-alike branch
applied the posture and refused a legitimate invitee.

The address now goes into the query — `sys_invitation.email` carries a declared
index — and the page chain is exhausted, so no row count can hide a live
invitation. A page is "pending invitations addressed to this one person", which
better-auth bounds by refusing a second pending invitation per organization, so
this is not a read of the environment's pending population on the sign-up path;
in practice it is a single indexed lookup where the old code always read 200
rows.

The in-memory scan existed on the stated belief that invitation addresses are
stored as the inviter typed them. Measured against the installed better-auth
1.7.1, that is wrong on both halves — `organization/invite-member` lowercases
the address before storing it, and `internalAdapter.createUser` lowercases the
registrant's before calling `validateUserInfo` — and the vendor's own
`findPendingInvitation` / `listUserInvitations` / `findMemberByEmail` all query
with `email.toLowerCase()`, so a mixed-case row was never redeemable through
`accept-invitation` anyway.

The row-side comparison is kept rather than deleted: `=` folds case on some
collations and folds accents with it, so every returned row is re-checked
against the normalized address — a case-only difference still matches, an
accent-only difference does not. Expiry stays in JS so a row with no readable
`expires_at` keeps reading as live. The security properties are unchanged:
`status = 'pending'` only, expiry still enforced, and an unanswerable probe
still means no carve-out.
Original file line numberDiff line numberDiff line change
Expand Up@@ -44,7 +44,14 @@ interface TablesEngine {
export function inviteForAudienceGate(engineOrManager: unknown, email: string): Promise<void> {
const row = {
id: `inv_audience_${Math.random().toString(36).slice(2, 10)}`,
email,
// [#11770] Stored NORMALIZED, because that is the only form the product
// writes: `organization/invite-member` lowercases the address before it
// reaches `createInvitation`, and the vendor's own reads
// (`findPendingInvitation`, `listUserInvitations`) look it up with
// `email.toLowerCase()`. A fixture holding the inviter's raw casing would
// pin a row shape no invitation route can produce and no accept route
// could redeem.
email: email.trim().toLowerCase(),
status: 'pending',
// A dedicated org id so suites that count THEIR invitations per
// organization never see these rows.
Expand Down
108 changes: 106 additions & 2 deletions packages/plugins/plugin-auth/src/audience-posture.test.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -493,8 +493,14 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739)
const engine = createMemoryEngine();
seedExistingUser(engine);
const manager = makeManager(engine);
seedPendingInvitation(engine, 'Bob@Acme.com');
const admitted = await signUp(manager, 'bob@acme.com');
// better-auth 1.7.1 normalizes BOTH sides before this gate ever runs:
// `organization/invite-member` lowercases the address it stores, and
// `internalAdapter.createUser` lowercases the registrant's before calling
// `validateUserInfo`. So the row holds the normalized form, and the
// case-insensitivity the invitee experiences is that the address THEY type
// may carry any case at all.
seedPendingInvitation(engine, 'bob@acme.com');
const admitted = await signUp(manager, 'Bob@Acme.com');
expect(admitted.status).toBeLessThan(300);

seedPendingInvitation(engine, 'late@acme.com', { expires_at: new Date(Date.now() - 60_000) });
Expand All@@ -503,6 +509,104 @@ describe('end of the chain: better-auth pipeline over the memory engine (#11739)
expect((await refused.json()).code).toBe(SELF_REGISTRATION_CLOSED);
});

// ── [#11770] The page boundary of the pending-invitation probe ────────────
//
// The carve-out's first spelling read `{ status: 'pending' }` with
// `limit: 200` and matched the address in JS, so an invitee outside that
// first page was refused `SELF_REGISTRATION_CLOSED` under the `invite_only`
// default — the invitation lane silently failing for the TAIL of a large
// rollout. The suite covered expiry and case-insensitivity but never crossed
// a page, which is precisely why the ceiling was invisible.

it('invite_only: an invitation past the page boundary still admits — a 500-person rollout has no silent tail (#11770)', async () => {
const engine = createMemoryEngine();
seedExistingUser(engine);
const manager = makeManager(engine);

// A 500-employee onboarding, with the target seeded LAST so it sits far
// outside the first page of any read that pages the pending POPULATION.
for (let i = 0; i < 500; i++) seedPendingInvitation(engine, `colleague${i}@acme.com`);
seedPendingInvitation(engine, 'last.hire@acme.com');

const pending = engine.tables.get('sys_invitation') ?? [];
expect(pending.length).toBe(501);
// The fixture really crosses the old boundary — assert the placement
// rather than trusting the loop above to have produced it.
expect(pending.findIndex((r: any) => r.email === 'last.hire@acme.com')).toBeGreaterThan(200);

const admitted = await signUp(manager, 'last.hire@acme.com');
expect(admitted.status).toBeLessThan(300);
expect((engine.tables.get('sys_user') ?? []).some((u: any) => u.email === 'last.hire@acme.com')).toBe(true);
});

it('the probe reads sys_invitation NARROWED by email — the work never depends on how many invitations exist (#11770)', async () => {
const readsForPopulation = async (otherPending: number) => {
const engine = createMemoryEngine();
seedExistingUser(engine);
const manager = makeManager(engine);
for (let i = 0; i < otherPending; i++) seedPendingInvitation(engine, `colleague${i}@acme.com`);
seedPendingInvitation(engine, 'targeted@acme.com');

const reads: any[] = [];
const find = engine.find.bind(engine);
engine.find = async (name: string, q: any = {}) => {
if (name === 'sys_invitation') reads.push(q);
return find(name, q);
};

const admitted = await signUp(manager, 'targeted@acme.com');
expect(admitted.status).toBeLessThan(300);
return reads;
};

const small = await readsForPopulation(1);
const large = await readsForPopulation(400);

// Every read carries BOTH predicates. Without the email predicate the only
// way to stay correct is to walk the whole pending population, which is an
// unbounded read on the self-serve sign-up path.
expect(small.length).toBeGreaterThan(0);
for (const q of [...small, ...large]) {
expect(q.where?.status).toBe('pending');
expect(q.where?.email).toBe('targeted@acme.com');
expect(q.offset ?? 0).toBe(0); // one page settled it, either way
}
// 2 pending rows or 401 — identical work. (The read count is >1 because
// the email sign-up route pre-checks the very decision the
// `validateUserInfo` gate then makes; see `validateAudienceAdmission`.)
expect(large.length).toBe(small.length);
});

it('a case-folding collation may answer with a differently-cased row (matched) but never a different address (#11770)', async () => {
// Some collations answer `email = 'x'` case-insensitively — and MySQL's
// default folds accents with it. The JS re-check is what keeps the probe's
// answer identical across drivers: a case-only difference still matches,
// an accent-only difference must not.
const foldingEngine = (stored: string) => {
const engine = createMemoryEngine();
seedExistingUser(engine);
seedPendingInvitation(engine, stored);
const find = engine.find.bind(engine);
engine.find = async (name: string, q: any = {}) => {
if (name !== 'sys_invitation') return find(name, q);
// The store ignores the email predicate entirely — the widest a
// folding collation could plausibly be.
const where: Record<string, unknown> = { ...(q.where ?? {}) };
delete where.email;
return find(name, { ...q, where });
};
return engine;
};

const cased = foldingEngine('Bob@Acme.com');
expect((await signUp(makeManager(cased), 'bob@acme.com')).status).toBeLessThan(300);

const accented = foldingEngine('bób@acme.com');
const refused = await signUp(makeManager(accented), 'bob@acme.com');
expect(refused.status).toBe(403);
expect((await refused.json()).code).toBe(SELF_REGISTRATION_CLOSED);
});

it('email_domain: on-list admitted AND the declared permission set really lands; off-list refused with EMAIL_DOMAIN_NOT_ALLOWED', async () => {
const engine = createMemoryEngine();
seedExistingUser(engine);
Expand Down
118 changes: 102 additions & 16 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -3467,6 +3467,21 @@ export class AuthManager {
*/
private static readonly BOOTSTRAP_USER_PROBE_LIMIT = 50;

/**
* Page size of the pending-invitation probe ({@link hasPendingInvitationFor}).
* Unlike the bootstrap probe's limit this is NOT a bound on the answer — the
* probe's read is narrowed to one address and its page chain is exhausted —
* so the value only trades round trips against page size.
*/
private static readonly PENDING_INVITATION_PROBE_PAGE = 50;

/**
* Termination guarantee for that page chain, not a ceiling on the answer:
* reaching it would mean 10k pending invitations addressed to a single
* person, and it is reported at error level rather than answered silently.
*/
private static readonly PENDING_INVITATION_PROBE_MAX_PAGES = 200;

/** `error` when the host logger carries it, else the guaranteed `warn` channel (#9754). */
private audienceLogError(message: string, meta?: Record<string, unknown>): void {
const logger = this.config.logger as
Expand DownExpand Up@@ -3632,12 +3647,55 @@ export class AuthManager {
}

/**
* A pending, unexpired `sys_invitation` row exists for this email. Rows are
* fetched by status and compared lowercased in JS because invitation
* addresses are stored as the inviter typed them while better-auth
* lowercases the registrant's — a case-sensitive store-side equality would
* dead-end `Bob@Acme.com`'s invitee. Bounded read (an environment's pending
* invitations are few); unanswerable ⇒ no carve-out (fail closed).
* A pending, unexpired `sys_invitation` row exists for this email.
*
* ## Why the address goes into the QUERY rather than into a memory scan
*
* The first spelling read `{ status: 'pending' }` with `limit: 200` and
* matched the address in JS. That is a CEILING, not a bound: past 200
* concurrently-pending invitations in one environment, an invitee outside
* the first page was not found, so under the `invite_only` default their
* registration was refused with `SELF_REGISTRATION_CLOSED` — the invitation
* lane silently failing for the TAIL of a large rollout, with no signal to
* the administrator or the invitee. A 500-employee onboarding reaches it.
*
* The scan existed to make the match case-insensitive, on the stated belief
* that invitation addresses are stored as the inviter typed them while
* better-auth lowercases the registrant's. Measured on the installed
* better-auth 1.7.1, that belief is wrong on BOTH halves — the vendor
* normalizes each side before this gate ever sees it:
*
* - `organization/invite-member` lowercases `ctx.body.email` and carries
* that value into `createInvitation` (and into the resend path), so the
* stored `sys_invitation.email` is already the normalized form;
* - `internalAdapter.createUser` lowercases `user.email` *before* it calls
* `validateUserInfo`, so the address this gate is asked about is already
* normalized too.
*
* The vendor's own reads agree: `findPendingInvitation`,
* `listUserInvitations` and `findMemberByEmail` all query with
* `email.toLowerCase()`. A mixed-case row would therefore be unredeemable
* by `accept-invitation` and invisible in the invitee's own inbox — the old
* tolerance admitted a registrant to an invitation they could never accept.
*
* So the address is pushed into the query (`sys_invitation.email` carries a
* declared index) and the page chain is EXHAUSTED. That removes the ceiling
* without putting a read of the environment's whole pending population on
* the self-serve sign-up path: a page here is "pending invitations
* addressed to this one person", which the vendor bounds by refusing a
* second pending invitation per organization.
*
* The JS comparison is KEPT and is not dead. `=` folds case on some
* collations (MySQL's default) and folds accents with it, so every returned
* row is re-checked against the normalized target: a case-only difference
* still matches — identical to the old behaviour on such a store — while an
* accent-only difference does not, so a folding collation cannot WIDEN what
* counts as an invitation. Expiry stays in JS for the reason it was there:
* a row with no readable `expires_at` keeps reading as live, which an
* `expires_at: { $gt: … }` predicate would silently narrow away.
*
* Unanswerable ⇒ no carve-out (fail CLOSED): the `catch` returns false, so
* the declared posture applies rather than an unverified admission.
*/
private async hasPendingInvitationFor(email: string): Promise<boolean> {
const engine = this.config.dataEngine;
Expand All@@ -3646,19 +3704,47 @@ export class AuthManager {
if (!target) return false;
try {
const reader = withSystemReadContext(engine) as any;
const raw = await reader.find('sys_invitation', { where: { status: 'pending' }, limit: 200 });
const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : [];
const nowMs = Date.now();
for (const row of rows) {
const rowEmail = typeof row?.email === 'string' ? row.email.trim().toLowerCase() : '';
if (rowEmail !== target) continue;
const expires = row?.expires_at ?? row?.expiresAt;
if (expires != null) {
const expMs = new Date(expires as any).getTime();
if (Number.isFinite(expMs) && expMs <= nowMs) continue;
const page = AuthManager.PENDING_INVITATION_PROBE_PAGE;
const seenIds = new Set<unknown>();
for (let pageIndex = 0; pageIndex < AuthManager.PENDING_INVITATION_PROBE_MAX_PAGES; pageIndex++) {
const offset = pageIndex * page;
const raw = await reader.find('sys_invitation', {
where: { status: 'pending', email: target },
limit: page,
// Omitted on the first page so the ordinary single-page read sends
// exactly the option shape every driver already answers.
...(offset > 0 ? { offset } : {}),
});
const rows: any[] = Array.isArray(raw) ? raw : Array.isArray(raw?.records) ? raw.records : [];
const idsBefore = seenIds.size;
for (const row of rows) {
if (row?.id != null) seenIds.add(row.id);
const rowEmail = typeof row?.email === 'string' ? row.email.trim().toLowerCase() : '';
if (rowEmail !== target) continue;
const expires = row?.expires_at ?? row?.expiresAt;
if (expires != null) {
const expMs = new Date(expires as any).getTime();
if (Number.isFinite(expMs) && expMs <= nowMs) continue;
}
return true;
}
return true;
// Exhausted: a short page is the end of the chain. The second arm is
// the termination guarantee against a driver that accepts `offset`
// and ignores it — a page that carried no row id this loop had not
// already read cannot carry a new answer either, and looping forever
// on the sign-up path would be worse than the ceiling being removed.
if (rows.length < page) return false;
if (seenIds.size === idsBefore && rows.some((r) => r?.id != null)) return false;
}
// Unreachable on any real population (that is 10k pending invitations
// addressed to ONE person). Loud rather than silent: the defect this
// method closes was a boundary nobody could see from either side.
this.audienceLogError(
'[audience] pending-invitation probe hit its page ceiling — the carve-out is being refused '
+ 'without having read every pending invitation for this address.',
{ pages: AuthManager.PENDING_INVITATION_PROBE_MAX_PAGES, pageSize: page },
);
return false;
} catch {
return false;
Expand Down
5 changes: 4 additions & 1 deletion packages/verify/src/harness.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -700,7 +700,10 @@ export async function bootStack(
'sys_invitation',
{
id: `inv_verify_${Math.random().toString(36).slice(2, 10)}`,
email,
// [#11770] Normalized, as the invitation route stores it — the
// audience probe reads `sys_invitation` by address, and better-auth
// lowercases both the stored address and the registrant's.
email: email.trim().toLowerCase(),
status: 'pending',
// A dedicated org id so fixtures counting THEIR invitations never
// see these rows.
Expand Down
Loading