Skip to content

fix(plugin-auth): organization/remove-member answers a permission denial as 403, not as the only-owner 400 - #8316

Merged
os-zhuang merged 3 commits into
mainfrom
claude/issue-8289-remove-member-denial-status
Aug 13, 2026
Merged

fix(plugin-auth): organization/remove-member answers a permission denial as 403, not as the only-owner 400#8316
os-zhuang merged 3 commits into
mainfrom
claude/issue-8289-remove-member-denial-status

Conversation

@os-zhuang

@os-zhuangos-zhuang commented Aug 13, 2026

Copy link
Copy Markdown
Contributor

Fixes#8289

Where the 400 is minted

Not in our packages — inside the pinned vendor. Re-verified triage's grep on current origin/main (2efd2c9bc, not the 35e7417 triage measured at): no producer of YOU_CANNOT_LEAVE_THE_ORGANIZATION_AS_THE_ONLY_OWNER exists anywhere in packages/ or apps/. The only local hits for "only owner" are prose in last-admin-guard.ts and invitation-role-cap.ts.

The string is minted in better-auth@1.7.0-rc.2, dist/plugins/organization/routes/crud-members.mjs, in the removeMember handler:

constroles=toBeRemovedMember.role.split(",");constcreatorRole=ctx.context.orgOptions?.creatorRole||"owner";if(roles.includes(creatorRole)){// (3a) a PERMISSION rule, wearing the invariant's code and statusif(!member.role.split(",").map(r=>r.trim()).includes(creatorRole))throwAPIError.from("BAD_REQUEST",YOU_CANNOT_LEAVE_THE_ORGANIZATION_AS_THE_ONLY_OWNER);// (3b) the genuine invariantif(owners.length<=1)throwAPIError.from("BAD_REQUEST",YOU_CANNOT_LEAVE_THE_ORGANIZATION_AS_THE_ONLY_OWNER);}// (4) the real permission check — ordered AFTER both of the aboveif(!awaithasPermission({role: member.role,permissions: {member: ["delete"]},}))throwAPIError.from("UNAUTHORIZED",YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_MEMBER);

Branch (3a) is the defect: "only an owner may remove an owner" is a permission rule, reported with the sole-owner invariant's message and a 400, and ordered ahead of the route's real permission check at (4). Whenever the target is an owner and the caller is not, (3a) short-circuits and the invariant answers a question it was never asked.

That also explains the filer's step 2 — adding a second owner changed nothing because (3a) fires before the owner count at (3b) is ever consulted. Every clause of the message can be false simultaneously.

Branch (4) carries a second, smaller defect: UNAUTHORIZED (401), where every sibling denial answers FORBIDDEN (403). That is why a plain member removing another plain member was also not matching sibling parity.

Fix shape, and why this one

No fork, no vendoring, no patched dependency. The correction lives in our bridge — the global before-hook in auth-manager.ts, delegating to a new remove-member-permission-guard.ts.

It has to be a before-hook, not a response remap. better-auth pins the HTTP status when the handler throws: dispatch.mjs calls toResponse(response, { status: result.status }) with the original error's statusCode, and better-call's toResponse resolves init?.status ?? data.statusCodeinit wins. So an after-hook can replace the body but not the status, which would yield a 400 carrying a YOU_ARE_NOT_ALLOWED_TO_* code: a worse answer than the one being fixed. A before-hook throw is dispatched through toResponse(before, { headers }) with no status in init, so the thrown 403 stands — the same mechanism the existing /sso/register FORBIDDEN gate relies on.

Two properties keep the guard from changing policy, both pinned by tests:

  1. Silent on the sole-owner path. When the caller removes themselves while carrying the creator role, the guard returns and lets the vendor answer. The exemption is exact: (3b) can only fire when caller and target are the same person (it needs both to carry the creator role while at most one such member exists), so this removes the guard from the invariant's way completely and exempts nothing else.
  2. Never widens a refusal. The role test reproduces the vendor's (3a) predicate literally — including the asymmetry where the target's roles are split without trim() and the caller's with it. A "cleaned up" predicate would refuse inputs the vendor allows, which would be a policy change smuggled in under an envelope fix. The permission half calls the vendor's own exported hasPermission, never a local re-derivation, so there is no second spelling of the authorization question.

Net: the refusal set is byte-for-byte the vendor's; only the envelope changes.

Ablation (predict → mutate → observe)

Mutation: git checkout origin/main -- packages/plugins/plugin-auth/src/auth-manager.ts (removes the hook, the method and the roles-map stash; the guard module and tests stay). Predictions were written down before running the mutation, and are reproduced here unchanged.

#AssertionPredictedObservedMatch
1step 1: plain member removes the ownerFAIL 400 ONLY_OWNERFAIL 400 ONLY_OWNER
2step 2: second owner added, member removes ownerFAIL 400 ONLY_OWNERFAIL 400 ONLY_OWNER
3plain member removes another plain memberFAIL 401FAIL 401
4ADMIN removes an ownerFAIL 400 ONLY_OWNERFAIL 400 ONLY_OWNER
5parity with sibling update-member-roleFAIL removal 400 ≠ 403FAIL 400 ≠ 403
6managed-extension-fields plugin accountingFAIL on the stale assertionFAIL: "names plugins auth-manager.ts no longer imports: hasPermission"
7genuine sole-owner refusal on self-removalPASSPASS
8sole-owner guard through organization/leavePASSPASS
9non-last owner may still leavePASSPASS
10owner removes the other owner → 200PASSPASS
11owner removes a plain member → 200PASSPASS
12admin may still remove a plain member → 200PASSPASS

Tests 6 failed | 1118 passed (1124) under the mutation; 1124 passed restored. 12/12 predictions matched.

Row 6 was the one genuinely new prediction rather than a replay of the pre-fix baseline, and it is the interesting one: it proves the AUTH_MANAGER_PLUGINS skip entry added for hasPermission is self-cleaning — remove the import and the guard immediately reports the entry as stale, so the entry cannot rot into a permanent hole.

The pass set (7–12) is what makes the ablation meaningful in the other direction: the guard only ever adds a pre-emptive refusal on inputs the vendor already refuses, so it cannot be the reason any of those six work. Had any flipped, the guard would be changing policy rather than the envelope.

Sole-owner guard and legitimate paths still work

Pinned explicitly, not merely left alone:

  • sole owner removing themselves → still 400 YOU_CANNOT_LEAVE_THE_ORGANIZATION_AS_THE_ONLY_OWNER
  • sole owner via organization/leave → still 400, same code
  • a non-last owner leaving → still 200 (the invariant is about the last owner, so this pins that the guard did not over-refuse)
  • owner removes the other owner → 200
  • owner removes a plain member → 200
  • admin removes a plain member → 200

Gates run locally

GateResult
pnpm --filter @objectstack/plugin-auth test1124 passed (48 files)
pnpm --filter @objectstack/plugin-auth typecheckclean
check:engine-double-contractOK — 186 pinned, 133 DEBT, 2 exempt
check:nul-bytesOK — 7549 files
check:error-code-casingOK — 3863 files (ADR-0112)

The new test file's engine double routes update through assertEngineUpdateDispatch and delete through assertEngineDeleteDispatch, and enforces sys_member's declared {organization_id, user_id} UNIQUE index — that last one matters here specifically, because every assertion reads the membership rows back to prove a removal did or did not happen, and the owner count is exactly the fact the vendor's sole-owner branch turns on. No baseline or ledger was touched.

Out of scope, filed separately

#8317 — better-auth's org-role matching is case- and whitespace-sensitive (split(',') with no trim/lowercase), so a sys_member.role of Owner reads as an owner to our #5942 grade ladder and as a plain member to the vendor; an admin can then remove such an owner. Not fixed here on purpose: correcting it would change who is refused, under cover of a response-shape fix. Filed unassigned for triage.

Related: #8092 (the console shows the Remove member affordance to plain members — objectui side, deliberately untouched here).


Generated by Claude Code


Generated by Claude Code

Pins the measured defect before any fix: better-auth 1.7.0-rc.2's removeMember
orders its owner-target branch AHEAD of hasPermission, so a non-owner caller
gets 400 YOU_CANNOT_LEAVE_THE_ORGANIZATION_AS_THE_ONLY_OWNER instead of a
permission refusal. 5 assertions red, 6 green (the must-not-break set).
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 4:55am

Request Review

@github-actions

github-actionsBot commented Aug 13, 2026

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.

better-auth 1.7.0-rc.2 orders removeMember's 'only an owner may remove an
owner' rule ahead of its real permission check and reports it with the
sole-owner invariant's code and a 400. Answer the permission class in the
global before-hook instead, with the 403 YOU_ARE_NOT_ALLOWED_TO_DELETE_THIS_MEMBER
envelope the sibling endpoints use.
The guard stays silent on the self-removal path, so the genuine sole-owner
invariant remains the vendor's; the permission half is decided by the vendor's
own exported hasPermission, so there is no second spelling of it.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PEVB6w7D7uCszR9Mw1BL73
@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tooling labels Aug 13, 2026
@os-zhuang
os-zhuang marked this pull request as ready for review August 13, 2026 05:13
@os-zhuang
os-zhuang added this pull request to the merge queueAug 13, 2026
Merged via the queue into main with commit c797473Aug 13, 2026
27 checks passed
@os-zhuang
os-zhuang deleted the claude/issue-8289-remove-member-denial-status branch August 13, 2026 05:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

2 participants

@os-zhuang@claude