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-zhuang os-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.ts loginEventFor), 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

Claude and 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

vercel Bot commented Aug 13, 2026

Copy link
Copy Markdown

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

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 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 free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

audit: a user's FIRST session predates their membership, so every audit row written in that window carries a NULL tenant and is invisible to RLS readers

1 participant