Skip to content

Commit c892829

Browse files
os-zhuangos-support-aiclaude
authored
fix(auth): settle the membership before the first session resolves its active organization, so its audit rows carry a tenant (#8473)
* fix(auth): settle the membership before the first session resolves its active organization (#8245) Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 * chore: changeset + ADR-0093 anchor for the first-session membership ordering Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73 --------- Co-authored-by: Claude <support@objectstack.ai> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent ff1e9b6 commit c892829

4 files changed

Lines changed: 442 additions & 32 deletions

File tree

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
---
2+
"@objectstack/plugin-auth": patch
3+
---
4+
5+
fix(auth): a user's first session no longer predates their membership, so its audit rows carry a tenant (#8245, #8247)
6+
7+
`session.create.before` resolves a session's `activeOrganizationId` from the
8+
caller's `sys_member` row. The ADR-0093 D2 reconciler that **writes** that row is
9+
composed into `user.create.after`, and better-auth defers it past the sign-up
10+
transaction — so the session sign-up mints ran first, found no membership, and
11+
carried no active organization. Structurally, for every new user, on every
12+
deployment.
13+
14+
That first session was not a harmless intermediate. Its `login` audit row takes
15+
its tenant from `session.activeOrganizationId`, so the row landed with a NULL
16+
tenant and the SecurityPlugin's RLS predicate (`organization_id =
17+
current_user.organization_id`) hid it from every reader **permanently**
18+
nothing back-fills a written ledger row, and the rows lost this way are exactly
19+
the ones describing account creation.
20+
21+
The membership now settles at the seam that needs it: when the active-org lookup
22+
finds nothing, the reconciler runs and the lookup is repeated, so the first
23+
session mints *with* its organization.
24+
25+
**This changes ordering, not policy.** It calls the same reconciler with the same
26+
membership policy and the same target-organization resolution that
27+
`user.create.after` uses — both now share one assembly point on the manager — so
28+
the outcome is exactly what would have happened a moment later:
29+
30+
- `invite-only` binds nobody, and those sessions still mint with no active
31+
organization;
32+
- a multi-organization deployment resolves no unambiguous target and binds
33+
nobody, unchanged;
34+
- a user who already holds a membership never reaches the new branch, and no
35+
second membership is ever written;
36+
- owner-preference in the active-org selection is unchanged, because the
37+
selection is one function called on both sides of the settle.
38+
39+
Cost is paid only where there is something to fix. A deployment that binds nobody
40+
stops at the reconciler's own policy check without touching the store, and the
41+
repeat lookup is gated on an outcome meaning a membership now exists — so an
42+
ordinary sign-in issues no extra query.
43+
44+
Unchanged: `user.create.after` still reconciles (the creation paths that mint no
45+
session at all — admin create-user, bulk import, SSO JIT — are untouched), the
46+
host `session.create.before` hook still chains first and still wins,
47+
`autoActiveOrganization: false` still opts out entirely, and a failing engine
48+
still never breaks session creation.

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

Lines changed: 125 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,11 @@ import {
4848
removalBlockedByOwnerTarget,
4949
} from './remove-member-permission-guard.js';
5050
import { isPlaceholderEmail } from './placeholder-email.js';
51-
import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js';
51+
import {
52+
reconcileMembership,
53+
type MembershipPolicy,
54+
type ReconcileOutcome,
55+
} from './reconcile-membership.js';
5256
import type { TenancyService } from './tenancy-service.js';
5357
import { OtpSendGuard, assertOtpCooldownSeconds } from './otp-send-guard.js';
5458
import type { CounterStore } from './rate-limit-storage.js';
@@ -3019,6 +3023,58 @@ export class AuthManager {
30193023
return this.config.membershipPolicy ?? 'auto';
30203024
}
30213025

3026+
/**
3027+
* [ADR-0093 D2] Run the membership reconciler for one user — the ONE place
3028+
* this manager assembles its inputs.
3029+
*
3030+
* Two seams call it, and the whole point is that they cannot disagree:
3031+
*
3032+
* - `user.create.after`, the creation seam every path flows through (email
3033+
* signup, admin create-user, bulk import, SSO JIT);
3034+
* - `session.create.before`, which settles the membership before resolving
3035+
* the session's active organization so a user's FIRST session is not
3036+
* minted tenant-less (#8247 rule 2 / #8245).
3037+
*
3038+
* Assembling the deps at each call site instead would let the two drift on
3039+
* the axis that matters most: the POLICY. `getMembershipPolicy()` reads a
3040+
* live platform setting (#5152) — a captured constructor option would keep
3041+
* one seam auto-binding after an admin switched the deployment to
3042+
* `invite-only`, which is the exact defect that made the accessor exist. The
3043+
* target-org resolution is shared for the same reason: "which organization"
3044+
* must never be answered two ways.
3045+
*
3046+
* Never throws — `reconcileMembership` already guarantees that, and the guard
3047+
* stands anyway because both callers are hooks where a bookkeeping failure
3048+
* must not fail user creation or sign-in. The OUTCOME is returned (rather
3049+
* than swallowed) so the session seam can tell "a membership now exists" from
3050+
* "policy says there will never be one" and skip a pointless re-read;
3051+
* `undefined` means the reconciler could not be consulted at all.
3052+
*/
3053+
private async settleMembership(userId: unknown): Promise<ReconcileOutcome | undefined> {
3054+
try {
3055+
const result = await reconcileMembership(
3056+
this.config.dataEngine,
3057+
typeof userId === 'string' && userId ? userId : undefined,
3058+
{
3059+
// #5152 — read through the accessor, not `this.config` directly: it is
3060+
// the single source the backfill path reads too.
3061+
policy: this.getMembershipPolicy(),
3062+
resolveTargetOrg: async () => {
3063+
const tenancy = this.config.getTenancy?.();
3064+
// Single-org → default org; multi-org → none (invite/JIT own it).
3065+
return tenancy ? await tenancy.defaultOrgId() : null;
3066+
},
3067+
logger: this.config.logger,
3068+
},
3069+
);
3070+
return result.outcome;
3071+
} catch {
3072+
// reconcileMembership never throws, but guard regardless — membership
3073+
// bookkeeping must never break user creation or session creation.
3074+
return undefined;
3075+
}
3076+
}
3077+
30223078
/**
30233079
* Inject (or replace) the outbound email service used by better-auth
30243080
* callbacks. Safe to call after construction but BEFORE the first
@@ -4370,6 +4426,30 @@ export class AuthManager {
43704426
// never fails on this bookkeeping. Opt out via `autoActiveOrganization:
43714427
// false`.
43724428
const hostSessionBefore = (host as any)?.session?.create?.before;
4429+
4430+
/**
4431+
* The membership → active-org selection, in ONE place: owner-preferred,
4432+
* else the oldest row. It is called twice below and both calls must select
4433+
* identically — a second, "simpler" lookup after the settle would silently
4434+
* make a freshly-bound user's active org depend on which path found it.
4435+
*/
4436+
const selectActiveOrg = async (reader: any, userId: string): Promise<string | undefined> => {
4437+
let row: any;
4438+
try {
4439+
row = await reader.findOne('sys_member', { where: { user_id: userId, role: 'owner' } });
4440+
} catch {
4441+
row = undefined;
4442+
}
4443+
if (!row?.organization_id) {
4444+
try {
4445+
row = await reader.findOne('sys_member', { where: { user_id: userId } });
4446+
} catch {
4447+
row = undefined;
4448+
}
4449+
}
4450+
return row?.organization_id;
4451+
};
4452+
43734453
const defaultActiveOrg = async (session: any) => {
43744454
try {
43754455
if (!session || session.activeOrganizationId) return;
@@ -4380,22 +4460,51 @@ export class AuthManager {
43804460
// sys_member is org/user-scoped in host stacks — read with the system
43814461
// context so the pre-session lookup (no org on the caller yet) works.
43824462
const reader = withSystemReadContext(engine);
4383-
let row: any;
4384-
try {
4385-
row = await reader.findOne('sys_member', {
4386-
where: { user_id: userId, role: 'owner' },
4387-
});
4388-
} catch {
4389-
row = undefined;
4390-
}
4391-
if (!row?.organization_id) {
4392-
try {
4393-
row = await reader.findOne('sys_member', { where: { user_id: userId } });
4394-
} catch {
4395-
row = undefined;
4463+
let orgId = await selectActiveOrg(reader, userId);
4464+
4465+
// [#8247 rule 2 / #8245] SETTLE THE MEMBERSHIP, THEN LOOK AGAIN.
4466+
//
4467+
// The ADR-0093 D2 reconciler is composed into `user.create.after`, and
4468+
// better-auth DEFERS that past the sign-up transaction. This hook runs
4469+
// inside it. So a user's very FIRST session is minted BEFORE the
4470+
// reconciler has bound them to anything, the lookup above finds no
4471+
// `sys_member` row, and the session carries no active organization —
4472+
// for every new user, on every deployment, structurally.
4473+
//
4474+
// That first session is not a harmless intermediate. Everything it
4475+
// writes is tenant-less: its `login` audit row is derived from
4476+
// `session.activeOrganizationId` (`auth-session-audit.ts`), so it lands
4477+
// with a NULL tenant and the SecurityPlugin's RLS predicate hides it
4478+
// from every reader FOREVER — nothing back-fills a written ledger row,
4479+
// and the rows lost this way are exactly the ones describing account
4480+
// creation (#8245).
4481+
//
4482+
// So the settle is hoisted HERE, to the seam that actually needs it,
4483+
// rather than the ordering being left to better-auth's hook scheduling.
4484+
//
4485+
// ⛔ THIS DOES NOT WIDEN WHO GETS BOUND, and that is the property to
4486+
// preserve if this is ever touched: it calls the SAME reconciler with
4487+
// the SAME policy and the SAME target-org resolution that
4488+
// `user.create.after` uses (one owner — `settleMembership`), so the
4489+
// outcome is byte-for-byte what would have happened a moment later.
4490+
// `invite-only` still binds nobody; multi-org still resolves no
4491+
// unambiguous target and binds nobody. Those users keep minting
4492+
// sessions with no active organization, which is the LEGAL state the
4493+
// #8247 ruling declares — this removes a race, never a policy.
4494+
//
4495+
// Cost is paid only where there is something to fix: a caller who
4496+
// already holds a membership never reaches this branch, and a
4497+
// deployment that binds nobody stops at the reconciler's own policy /
4498+
// target-org check without touching the store. The re-read is gated on
4499+
// an outcome that means a membership now EXISTS, so the common
4500+
// no-bind login costs no extra query at all.
4501+
if (!orgId) {
4502+
const outcome = await this.settleMembership(userId);
4503+
if (outcome === 'bound' || outcome === 'yielded') {
4504+
orgId = await selectActiveOrg(reader, userId);
43964505
}
43974506
}
4398-
const orgId = row?.organization_id;
4507+
43994508
if (!orgId) return;
44004509
return { data: { ...session, activeOrganizationId: orgId } };
44014510
} catch {
@@ -4455,22 +4564,7 @@ export class AuthManager {
44554564
// double bind. Best-effort — never fails user creation.
44564565
const hostUserAfter = (host as any)?.user?.create?.after;
44574566
const membershipReconciler = async (user: any) => {
4458-
try {
4459-
await reconcileMembership(this.config.dataEngine, user?.id, {
4460-
// #5152 — read through the accessor, not `this.config` directly: it is
4461-
// the single source the backfill path reads too.
4462-
policy: this.getMembershipPolicy(),
4463-
resolveTargetOrg: async () => {
4464-
const tenancy = this.config.getTenancy?.();
4465-
// Single-org → default org; multi-org → none (invite/JIT own it).
4466-
return tenancy ? await tenancy.defaultOrgId() : null;
4467-
},
4468-
logger: this.config.logger,
4469-
});
4470-
} catch {
4471-
// reconcileMembership never throws, but guard the hook regardless —
4472-
// membership bookkeeping must never break user creation.
4473-
}
4567+
await this.settleMembership(user?.id);
44744568
};
44754569
const userAfter = hostUserAfter
44764570
? async (user: any, ctx: any) => {

0 commit comments

Comments
 (0)