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
48 changes: 48 additions & 0 deletions .changeset/first-session-membership-ordering.md
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
---
"@objectstack/plugin-auth": patch
---

fix(auth): a user's first session no longer predates their membership, so its audit rows carry a tenant (#8245, #8247)

`session.create.before` resolves a session's `activeOrganizationId` from the
caller's `sys_member` row. The ADR-0093 D2 reconciler that **writes** that row is
composed into `user.create.after`, and better-auth defers it past the sign-up
transaction — so the session sign-up mints ran first, found no membership, and
carried no active organization. Structurally, for every new user, on every
deployment.

That first session was not a harmless intermediate. Its `login` audit row takes
its tenant from `session.activeOrganizationId`, so the row landed with a NULL
tenant and the SecurityPlugin's RLS predicate (`organization_id =
current_user.organization_id`) hid it from every reader **permanently** —
nothing back-fills a written ledger row, and the rows lost this way are exactly
the ones describing account creation.

The membership now settles at the seam that needs it: when the active-org lookup
finds nothing, the reconciler runs and the lookup is repeated, so the first
session mints *with* its organization.

**This changes ordering, not policy.** It calls the same reconciler with the same
membership policy and the same target-organization resolution that
`user.create.after` uses — both now share one assembly point on the manager — so
the outcome is exactly what would have happened a moment later:

- `invite-only` binds nobody, and those sessions still mint with no active
organization;
- a multi-organization deployment resolves no unambiguous target and binds
nobody, unchanged;
- a user who already holds a membership never reaches the new branch, and no
second membership is ever written;
- owner-preference in the active-org selection is unchanged, because the
selection is one function called on both sides of the settle.

Cost is paid only where there is something to fix. A deployment that binds nobody
stops at the reconciler's own policy check without touching the store, and the
repeat lookup is gated on an outcome meaning a membership now exists — so an
ordinary sign-in issues no extra query.

Unchanged: `user.create.after` still reconciles (the creation paths that mint no
session at all — admin create-user, bulk import, SSO JIT — are untouched), the
host `session.create.before` hook still chains first and still wins,
`autoActiveOrganization: false` still opts out entirely, and a failing engine
still never breaks session creation.
156 changes: 125 additions & 31 deletions packages/plugins/plugin-auth/src/auth-manager.ts
Original file line numberDiff line numberDiff line change
Expand Up@@ -48,7 +48,11 @@ import {
removalBlockedByOwnerTarget,
} from './remove-member-permission-guard.js';
import { isPlaceholderEmail } from './placeholder-email.js';
import { reconcileMembership, type MembershipPolicy } from './reconcile-membership.js';
import {
reconcileMembership,
type MembershipPolicy,
type ReconcileOutcome,
} from './reconcile-membership.js';
import type { TenancyService } from './tenancy-service.js';
import { OtpSendGuard, assertOtpCooldownSeconds } from './otp-send-guard.js';
import type { CounterStore } from './rate-limit-storage.js';
Expand DownExpand Up@@ -3019,6 +3023,58 @@ export class AuthManager {
return this.config.membershipPolicy ?? 'auto';
}

/**
* [ADR-0093 D2] Run the membership reconciler for one user — the ONE place
* this manager assembles its inputs.
*
* Two seams call it, and the whole point is that they cannot disagree:
*
* - `user.create.after`, the creation seam every path flows through (email
* signup, admin create-user, bulk import, SSO JIT);
* - `session.create.before`, which settles the membership before resolving
* the session's active organization so a user's FIRST session is not
* minted tenant-less (#8247 rule 2 / #8245).
*
* Assembling the deps at each call site instead would let the two drift on
* the axis that matters most: the POLICY. `getMembershipPolicy()` reads a
* live platform setting (#5152) — a captured constructor option would keep
* one seam auto-binding after an admin switched the deployment to
* `invite-only`, which is the exact defect that made the accessor exist. The
* target-org resolution is shared for the same reason: "which organization"
* must never be answered two ways.
*
* Never throws — `reconcileMembership` already guarantees that, and the guard
* stands anyway because both callers are hooks where a bookkeeping failure
* must not fail user creation or sign-in. The OUTCOME is returned (rather
* than swallowed) so the session seam can tell "a membership now exists" from
* "policy says there will never be one" and skip a pointless re-read;
* `undefined` means the reconciler could not be consulted at all.
*/
private async settleMembership(userId: unknown): Promise<ReconcileOutcome | undefined> {
try {
const result = await reconcileMembership(
this.config.dataEngine,
typeof userId === 'string' && userId ? userId : undefined,
{
// #5152 — read through the accessor, not `this.config` directly: it is
// the single source the backfill path reads too.
policy: this.getMembershipPolicy(),
resolveTargetOrg: async () => {
const tenancy = this.config.getTenancy?.();
// Single-org → default org; multi-org → none (invite/JIT own it).
return tenancy ? await tenancy.defaultOrgId() : null;
},
logger: this.config.logger,
},
);
return result.outcome;
} catch {
// reconcileMembership never throws, but guard regardless — membership
// bookkeeping must never break user creation or session creation.
return undefined;
}
}

/**
* Inject (or replace) the outbound email service used by better-auth
* callbacks. Safe to call after construction but BEFORE the first
Expand DownExpand Up@@ -4370,6 +4426,30 @@ export class AuthManager {
// never fails on this bookkeeping. Opt out via `autoActiveOrganization:
// false`.
const hostSessionBefore = (host as any)?.session?.create?.before;

/**
* The membership → active-org selection, in ONE place: owner-preferred,
* else the oldest row. It is called twice below and both calls must select
* identically — a second, "simpler" lookup after the settle would silently
* make a freshly-bound user's active org depend on which path found it.
*/
const selectActiveOrg = async (reader: any, userId: string): Promise<string | undefined> => {
let row: any;
try {
row = await reader.findOne('sys_member', { where: { user_id: userId, role: 'owner' } });
} catch {
row = undefined;
}
if (!row?.organization_id) {
try {
row = await reader.findOne('sys_member', { where: { user_id: userId } });
} catch {
row = undefined;
}
}
return row?.organization_id;
};

const defaultActiveOrg = async (session: any) => {
try {
if (!session || session.activeOrganizationId) return;
Expand All@@ -4380,22 +4460,51 @@ export class AuthManager {
// sys_member is org/user-scoped in host stacks — read with the system
// context so the pre-session lookup (no org on the caller yet) works.
const reader = withSystemReadContext(engine);
let row: any;
try {
row = await reader.findOne('sys_member', {
where: { user_id: userId, role: 'owner' },
});
} catch {
row = undefined;
}
if (!row?.organization_id) {
try {
row = await reader.findOne('sys_member', { where: { user_id: userId } });
} catch {
row = undefined;
let orgId = await selectActiveOrg(reader, userId);

// [#8247 rule 2 / #8245] SETTLE THE MEMBERSHIP, THEN LOOK AGAIN.
//
// The ADR-0093 D2 reconciler is composed into `user.create.after`, and
// better-auth DEFERS that past the sign-up transaction. This hook runs
// inside it. So a user's very FIRST session is minted BEFORE the
// reconciler has bound them to anything, the lookup above finds no
// `sys_member` row, and the session carries no active organization —
// for every new user, on every deployment, structurally.
//
// That first session is not a harmless intermediate. Everything it
// writes is tenant-less: its `login` audit row is derived from
// `session.activeOrganizationId` (`auth-session-audit.ts`), so it lands
// with a NULL tenant and the SecurityPlugin's RLS predicate hides it
// from every reader FOREVER — nothing back-fills a written ledger row,
// and the rows lost this way are exactly the ones describing account
// creation (#8245).
//
// So the settle is hoisted HERE, to the seam that actually needs it,
// rather than the ordering being left to better-auth's hook scheduling.
//
// ⛔ THIS DOES NOT WIDEN WHO GETS BOUND, and that is the property to
// preserve if this is ever touched: it calls the SAME reconciler with
// the SAME policy and the SAME target-org resolution that
// `user.create.after` uses (one owner — `settleMembership`), so the
// outcome is byte-for-byte what would have happened a moment later.
// `invite-only` still binds nobody; multi-org still resolves no
// unambiguous target and binds nobody. Those users keep minting
// sessions with no active organization, which is the LEGAL state the
// #8247 ruling declares — this removes a race, never a policy.
//
// Cost is paid only where there is something to fix: a caller who
// already holds a membership never reaches this branch, and a
// deployment that binds nobody stops at the reconciler's own policy /
// target-org check without touching the store. The re-read is gated on
// an outcome that means a membership now EXISTS, so the common
// no-bind login costs no extra query at all.
if (!orgId) {
const outcome = await this.settleMembership(userId);
if (outcome === 'bound' || outcome === 'yielded') {
orgId = await selectActiveOrg(reader, userId);
}
}
const orgId = row?.organization_id;

if (!orgId) return;
return { data: { ...session, activeOrganizationId: orgId } };
} catch {
Expand DownExpand Up@@ -4455,22 +4564,7 @@ export class AuthManager {
// double bind. Best-effort — never fails user creation.
const hostUserAfter = (host as any)?.user?.create?.after;
const membershipReconciler = async (user: any) => {
try {
await reconcileMembership(this.config.dataEngine, user?.id, {
// #5152 — read through the accessor, not `this.config` directly: it is
// the single source the backfill path reads too.
policy: this.getMembershipPolicy(),
resolveTargetOrg: async () => {
const tenancy = this.config.getTenancy?.();
// Single-org → default org; multi-org → none (invite/JIT own it).
return tenancy ? await tenancy.defaultOrgId() : null;
},
logger: this.config.logger,
});
} catch {
// reconcileMembership never throws, but guard the hook regardless —
// membership bookkeeping must never break user creation.
}
await this.settleMembership(user?.id);
};
const userAfter = hostUserAfter
? async (user: any, ctx: any) => {
Expand Down
Loading
Loading