Skip to content

fix(auth): settle the membership before the first session resolves its active organization, so its audit rows carry a tenant - #8473

Merged
os-zhuang merged 2 commits into
mainfrom
claude/issue-8247-signup-membership-ordering
Aug 13, 2026
Merged

fix(auth): settle the membership before the first session resolves its active organization, so its audit rows carry a tenant#8473
os-zhuang merged 2 commits into
mainfrom
claude/issue-8247-signup-membership-ordering

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes#8245
Part of #8247 — ruling rule 2 (the signup-ordering half). The declared-semantics half (rules 1 and 3) is #8471, which carries ADR-0123 and waits for the maintainer.

Declaration note

The Duplicate Fix Guard was briefly red on this PR: #8471's body used a GitHub closing keyword in front of #8245 while describing this PR, so both PRs declared #8245 and neither declared what it actually fixes. #8471 has been corrected to declare #8208; this PR keeps #8245, and both are Part of the parent decision card #8247, which neither closes alone.

This PR carries no ADR document. Its only adr-shaped file is scripts/adr-anchors/…auth-manager.ts.json, an anchor registry entry — not a docs/adr/** record. Confirmed both ways: ADR maintainer approval reports success here, and the gate's governed surface is the literal path prefix docs/adr/ (ADR_PATH_PREFIX in scripts/check-adr-merge-approval.mjs), which an anchor file does not match. So this PR is eligible for the normal flow; only #8471 has to wait for a human merge.

The ordering

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 runs first, finds no membership, and carries no active organization — structurally, for every new user, on every deployment.

That first session is not a harmless intermediate. Its login audit row takes its tenant from session.activeOrganizationId (auth-session-audit.tsloginEventFor), so the row lands with a NULL tenant and the SecurityPlugin's RLS predicate hides 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 premise is still live on origin/main, verbatim, in the dogfood suite's own words:

session.create.before derives activeOrganizationId from the caller's sys_member row, and ADR-0093's reconciler runs on user.create.after, which better-auth defers past the signup transaction. So a user's very FIRST session — the one sign-up mints — legitimately predates their membership and carries no active org; its ledger row therefore has no tenant, and no seam downstream can invent one.

The fix

When the active-org lookup finds nothing, the reconciler runs and the lookup is repeated. The first session then mints with its organization.

This changes ordering, not policy — and that is the property to preserve. The session seam calls the same reconciler with the same policy and the same target-organization resolution user.create.after uses; both now share one assembly point (AuthManager.settleMembership). The outcome is byte-for-byte 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;
  • an already-bound user never reaches the new branch, and no second membership is written;
  • owner-preference in the selection is unchanged, because the selection is one function called on both sides of the settle — a second, "simpler" lookup after the settle would make a freshly-bound user's active org depend on which path found it.

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.

One assembly point, deliberately. Assembling the reconciler's deps at each call site would let the two seams drift on the axis that matters most: the policy. getMembershipPolicy() reads a live platform setting (#5152) — a captured constructor option leaves one seam auto-binding after an admin switches the deployment to invite-only, which is the exact defect that made the accessor exist.

Cost. 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. Pinned as a test, not asserted in prose.

Scope note. Under a walled posture tenancy.defaultOrgId() returns null by design (the framework never guesses a target org), so this fix binds nobody there — it helps the single-posture deployments where a default organization is unambiguous. A walled deployment's org-less callers are governed by #8471's declared semantics instead. The two halves are complementary, not overlapping.

Tests — first-session-membership-ordering.test.ts, 13 cases

The fix is an ordering change, so the vacuity trap is a test that passes on the broken build because it never establishes the membership was absent when the session was minted. Every case starts from a store with no sys_member row and asserts on the first session; a PRECONDITION case pins that starting state rather than assuming it.

The policy cases are not decoration — they are what makes this safe to land, and they assert on the store (insert never called, row count unchanged), not merely on the returned draft: a policy saying "no auto-bind" must not be satisfied by binding and then declining to stamp.

#8245's chain is measured end to end: the session produced by the hook is fed to loginEventFor, and the resulting audit event carries the tenant.

Ablation (predictions written before the mutation, then measured)

Restoring auth-manager.ts to origin/main and re-running: 2 red, 11 green — exactly the first-session pin and the login-audit-event pin, and nothing else. Every policy case, the already-bound case, the owner-preference case, the host-hook case and the broken-engine case stayed green, which is the evidence that the blast radius is the ordering and not the reconciler's semantics.

Full package suite: 52 files, 1186 tests, all green.check:adr-anchors, check:nul-bytes, check:adr-0087-registration, check:changeset-no-major, check:type-check-coverage all green locally (the ratchet's TEST_DEBT count is unmoved).

Why this is a separate PR from #8471

Not stylistic — forced, and measurably:

The landing order is the favourable one too: this PR reduces how often the no-active-organization state occurs at signup, before #8471 starts refusing writes in it.

Files

  • packages/plugins/plugin-auth/src/auth-manager.tssettleMembership (the shared assembly point), the selectActiveOrg extraction, and the settle-then-re-read in defaultActiveOrg.
  • packages/plugins/plugin-auth/src/first-session-membership-ordering.test.ts — new.
  • scripts/adr-anchors/packages__plugins__plugin-auth__src__auth-manager.ts.json — ADR-0093 added, with an invariant naming the two seams and why they must not drift.
  • .changeset/first-session-membership-ordering.md.

Generated by Claude Code

Claudeand others added 2 commits August 13, 2026 14:45
…s active organization (#8245)
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@vercel

vercelBot commented Aug 13, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
ProjectDeploymentActionsUpdated (UTC)
objectstackIgnoredIgnoredAug 13, 2026 2:48pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth.

8 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx(via @objectstack/plugin-auth)
  • content/docs/deployment/production-readiness.mdx(via @objectstack/plugin-auth)
  • content/docs/kernel/contracts/cache-service.mdx(via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx(via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx(via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx(via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx(via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx(via @objectstack/plugin-auth)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx(via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx(via @objectstack/plugin-auth)

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

1 participant

@os-zhuang