fix(app-shell): members and invitations tabs gate their affordances by org role (#4475) - #4505
Merged
Merged
Conversation
…y org role (#4475) A user whose org role is `member` was shown an enabled **Invite member** button and a per-row **Member actions** menu carrying **Remove member** — on every row, the workspace Owner's included. Nothing was hidden or disabled; the action only failed after the user committed to it. The Settings tab of the same page already gated correctly; these two tabs never got the same treatment. The roles are MEASURED against the routes that enforce them, not assumed to be "owner". better-auth 1.6.26's `organization/access/statement.mjs` plus the `delegated_admin` role the framework registers on top of it (objectstack `plugin-auth/src/auth-manager.ts`) give three DIFFERENT gates: invite -> `invitation:["create"]` -> owner, admin, delegated_admin remove -> `member:["delete"]` -> owner, admin cancel -> `invitation:["cancel"]` -> owner, admin `delegated_admin` is the row a single `isOwner` boolean cannot express: built from `memberAc.statements` (`member: []`) with `invitation: ['create']` added and `cancel` deliberately withheld. `orgCapabilities` composes the existing `orgRoleGrade` ladder rather than restating the closed ADR-0108 vocabulary. An actor left with no row action gets no menu rather than a trigger opening onto nothing, and the members page explains the absence where the Invite button sat, in the Settings tab's own `text-sm text-muted-foreground` voice. An unresolved role grades below a plain member, so nothing privileged is offered to a viewer whose membership could not be read. Reading is untouched: the member list and the invitation ledger still render. Whether `org_member` should read the ledger at all is objectstack#8095 and is deliberately not coupled here. Server verdict on the card's open probe: `remove-member` IS gated. The target lookup runs BEFORE the permission check, which is why the maintainer's non-existent-member probe returned `400 MEMBER_NOT_FOUND` and said nothing about gating — any real member id reaches `hasPermission({ member: ["delete"] })` and 401s. No server-side defect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017Qqyix2QcnpUC9XeYVDzx3
|
The latest updates on your projects. Learn more about Vercel for GitHub. |
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Contributor
✅ Console Performance Budget
📦 Bundle Size Report
Size Limits
|
Collaborator
Author
Continuation note (third session) — verify-only re-derivationA host restart killed the second session after it removed its worktree but before its report landed. This session re-created a fresh worktree at the pushed branch and re-derived the verification independently. No implementation was changed; the only new commits are two Branch brought current (twice)
The card's own delta is unchanged at 15 files / +724 / -84, and nothing outside Permission claims re-read at their sources (not inherited)Every number in the role-rule table above was re-read this session and holds:
remove-member verdict re-confirmed read-only (route read, never exercised): lookup throws Red-first re-derived from scratchReverted only the source half to Direction predicted before running, and matched: The 12 reds are exactly the gating cases (invite button, remove on every row, row menus, the delegated_admin and unresolved-role cases, both invitations affordances, and the two i18n pins). The 12 greens are exactly the must-not-change half — owner/admin affordances, the member list, the ledger, and the #3697 re-role narrowing. Correction to the
|
| gate | result |
|---|---|
vitest run packages/app-shell/src/console/organizations packages/i18n/src |
55 files, 892 passed |
tsc --noEmit -p tsconfig.json / -p tsconfig.test.json |
both clean; --listFiles confirms the new test, orgCapabilities.ts and both pages are in the test program |
eslint (organizations + locales) |
0 errors, 109 warnings — 94 no-explicit-any, 9 set-state-in-effect, 5 refs, 2 exhaustive-deps, matching the mix claimed above |
check:control-bytes |
OK, 4220 files scanned; plus a direct grep -naP self-scan of all changed files — clean |
check:i18n-keys / check:i18n-drift |
green; drift reports 1 key added, 0 en values changed |
changeset:check / presence |
green; 14 source files across 2 released packages, 1 changeset |
CI on 15c7ebe9b converged 20/20: 18 success, 2 skipped (dependabot, Test coverage), 0 failures — Lint, Type Check, all four test shards, Control Byte Scan and the three changeset gates all green.
PR intentionally left in draft for the PM.
Generated by Claude Code
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #4475
The defect
On
/_console/organizations//members, a user whose org role ismemberwas shown an enabled Invite member button and a per-row Member actions menu carrying Remove member — on every row, the workspace Owner's included. Nothing was hidden or disabled; the action only failed after the user committed to it. The Settings tab of the same page already gates correctly (form replaced by explanatory copy, Delete disabled, Leave enabled); the members and invitations tabs never got the same treatment.The measured role rule (what I keyed on)
Not
role === 'owner'— that is wrong in both directions, and the server says so. The three affordances on these two tabs sit behind three different better-auth permissions:/organization/invite-memberinvitation:["create"]/organization/remove-membermember:["delete"]/organization/cancel-invitationinvitation:["cancel"]Evidence, read from the code that enforces it:
plugins/organization/access/statement.mjs—ownerAcandadminAcboth carryinvitation: ["create","cancel"]andmember: ["create","update","delete"];memberAccarriesinvitation: []andmember: [].routes/crud-invites.mjsasserts{ invitation: ["create"] }for invite (line 98) and{ invitation: ["cancel"] }for cancel (line 437);routes/crud-members.mjsasserts{ member: ["delete"] }for remove (line 188).packages/plugins/plugin-auth/src/auth-manager.tsregisters the fourth role on top of those defaults (lines 1896-1901, verbatim):delegated_adminis the row a single boolean cannot express: it may invite, may not remove, and deliberately has nocancel— the framework's own comment records why (better-auth's cancel route checks only the permission and never invitation attribution, so granting it would mean "cancel anyone's pending invitation in the org"). That asymmetry is mirrored exactly rather than flattened.The actor's role comes from
activeMember.role— the same source the role-change narrowing (assignableOrgRoles, framework #3697) on this page already reads, so the screen keeps one role source. The neworgCapabilitiesmodule composes@object-ui/auth'sorgRoleGradeladder and role names rather than restating them, so the closed ADR-0108 vocabulary keeps exactly one definition. An unresolved role grades below a plain member (orgRoleGrade's documented floor), so nothing privileged is offered to a viewer whose membership could not be read.Copy-link on the invitations tab is gated with the invite predicate, not the cancel one: the link is the invitation, so handing it out is the issuing capability finishing its job — which is why a
delegated_adminkeeps it and still loses cancel.remove-member server verdict (the card's open probe — measured read-only)
The card noted that
remove-member's gating was unverified from the client because a probe with a non-existent member returned400 MEMBER_NOT_FOUND. Measured by reading the route, not by exercising it: the server does gate it. Incrud-members.mjsthe target lookup runs first (line 175) —— and only afterwards (line 185):
So the ordering explains the maintainer's
400exactly — the probe never reached the gate — and any real member id does reach it. No server-side defect; nothing to escalate to the objectstack lane. One observation, not filed as a defect: the ordering lets a non-permitted caller distinguish "member exists" (401) from "no such member" (400). It is upstream better-auth behaviour and reveals nothing a member cannot already read from the members list they are shown.Red-first
Verbatim DOM for a
memberviewer, captured from a throwaway probe test at both ends.Pre-fix — members page header and the Owner's row:
Post-fix — same viewer:
The whole suite run against the unfixed tree: 12 failed | 12 passed — the 12 passing being exactly the must-not-change half. After the fix: 24 passed.
Targeted reverse verification, direction predicted before running: widening
canInviteMembersback to a plaingrade >= adminreds precisely the twodelegated_admincases (invite button, copy-link) and nothing else —2 failed | 22 passed. Restored, green again.Must-not-change (green on both sides)
ownerandadminkeep the Invite button, Remove on every row, and the row menu (removeItems=2,memberActionsMenus=2).adminstill cannot re-role an Owner (better-auth'screatorRoleprotection) yet keeps Remove on that row — pinned so the new gate cannot be confused with the role-item gate.member; only write affordances go. Whetherorg_membershould read the ledger at all is objectstack#8095 and is not coupled here.SettingsPage.tsxis byte-identical (git diff origin/main -- SettingsPage.tsxis empty).Presentation
Where the Invite button was, the members page now puts the explanation, in the Settings tab's own
text-sm text-muted-foregroundvoice — the same convention (copy takes the place of the affordance it replaces), sized for a header slot rather than a form. Not a disabled button: a control that exists only to refuse is still an invitation to try. An actor left with no row action gets no menu, rather than a trigger that opens onto nothing. The invitations tab has no header affordance to replace, so it gets no copy — its per-row icons simply do not render.One new string,
organization.members.inviteRestrictedNote, added through the channel #4474 established (PR #4496): inlineuseObjectTranslationdefault plus all ten packs.resolveOrgRoleLabeland the error mapper are reused untouched.Verification
pnpm exec vitest run packages/app-shell/src/console/organizations packages/i18n/src→ 55 files, 892 passed (11 files / 91 tests app-shell console; 44 files / 801 tests i18n)tsc --noEmit -p packages/app-shell/tsconfig.json→ clean;tsc --noEmit -p packages/app-shell/tsconfig.test.json→ clean (--listFilesconfirms the new test andorgCapabilities.tsare both in that program, so the green is not an exclusion)eslint packages/app-shell/src/console/organizations packages/i18n/src/locales→ 0 errors, 109 warnings, none of them new-in-kind: 94@typescript-eslint/no-explicit-any(the component-mock passthroughs this suite's sibling files already use — 24 inacceptInvitationLink.mount, 17 inorg-i18n-holdouts-4474, 18 here), 9react-hooks/set-state-in-effecton untouched fetch effects (the one inMembersPage.tsxreproduces identically onorigin/main), 5react-hooks/refs, 2exhaustive-deps. The package'slintscript is a plaineslint .— no--max-warningsbudget.check:i18n-keys,check:i18n-drift,check:control-bytes,changeset:check,check-changeset-presence→ all green (drift reports1 key added, 0 en values changed; presence sees 14 source files across 2 released packages and 1 changeset).d.tsmeasured, not asserted: declarations emitted for the whole package from this tree and from the pre-fix tree, then diffed. The only difference in all ofapp-shellis the new internalconsole/organizations/manage/orgCapabilities.d.ts—MembersPage.d.tsandInvitationsPage.d.tsare byte-identical, and no file outsideorgCapabilities.d.tsitself mentions it (it is not re-exported from the package entry). Patch, never major.Continuation note (second session)
A host restart killed the first session between the push and its verification report, so this PR's verification was re-derived from scratch rather than inherited on trust:
origin/main(844ed3aea, 2 commits ahead: test(dom-leak): one attribute judge, shared by both gates (#4434) #4499 test-support, fix(plugin-dashboard): optionsFrom filters commit the raw value, not the display label (#4465) #4504 plugin-dashboard) — clean, no conflicts;pnpm installre-run for the new@object-ui/test-supportworkspace entry, then the full pass above re-run on the merged tree.origin/main(both pages, the ten locale packs, andorgCapabilities.tsremoved) while keeping the test file —git checkout origin/main -- (paths), nevergit stash. Result matched the prediction made before the run:Tests 12 failed | 12 passed (24), with the first failure verbatimAssertionError: expected (button …(1)) to be nullreceiving the realinvite-member-btnbutton, the secondexpected [ … ] to have a length of +0 but got 2for Remove items (one per row, the Owner's included), the third the samegot 2forMember actionsmenus. Restored to a clean tree;24 passed.statement.mjs,crud-invites.mjs,crud-members.mjs, objectstackauth-manager.ts) rather than carried over — two wordings were corrected against them: theauth-manager.tssnippet now quotes the realconst stmts = memberAc.statementsalias, and the eslint summary now reports the true rule mix (the earlier draft attributed all 109 warnings to one rule).Generated by Claude Code