diff --git a/.changeset/members-invitations-role-gating-4475.md b/.changeset/members-invitations-role-gating-4475.md new file mode 100644 index 000000000..9baa7269a --- /dev/null +++ b/.changeset/members-invitations-role-gating-4475.md @@ -0,0 +1,39 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/i18n': patch +--- + +Members & invitations tabs gate their affordances by org role instead of letting the server's 403 be the UI (#4475) + +A user whose organization role is `member` opened the workspace members page and +was shown an enabled **Invite member** button plus 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 had +committed to it. The Settings tab of the same page already gated correctly; the +members and invitations tabs never got the same treatment. + +The affordances are now narrowed to the roles that can actually use them, keyed +on the active member's role — the same source the role-change menu on this page +already reads. Which roles those are is **measured against the routes that +enforce them**, not assumed to be "owner": + +| affordance | route | permission | roles | +|---------------------|-----------------------------------|-------------------------|-------------------------------| +| Invite member | `/organization/invite-member` | `invitation:["create"]` | owner, admin, delegated_admin | +| Remove member | `/organization/remove-member` | `member:["delete"]` | owner, admin | +| Cancel invitation | `/organization/cancel-invitation` | `invitation:["cancel"]` | owner, admin | + +Three different gates, because `delegated_admin` holds `invitation:["create"]` +without `member:["delete"]` and deliberately without `cancel` — so it keeps the +invite button and the copy-link action while losing remove and cancel. A single +owner check could not express that. + +An actor left with no row action at all gets no menu rather than a trigger that +opens onto nothing, and the members page explains the absence where the Invite +button used to sit, in the Settings tab's own voice. An unresolved role is +treated as the least privileged, so nothing privileged is offered to a viewer +whose membership could not be read. + +Reading the pages is unaffected: the member list and the invitation ledger still +render in full. Whether `org_member` should be able to read the invitation +ledger at all is a separate, server-side question. diff --git a/packages/app-shell/src/console/organizations/__tests__/members-role-gating-4475.test.tsx b/packages/app-shell/src/console/organizations/__tests__/members-role-gating-4475.test.tsx new file mode 100644 index 000000000..1d8cfbb84 --- /dev/null +++ b/packages/app-shell/src/console/organizations/__tests__/members-role-gating-4475.test.tsx @@ -0,0 +1,414 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * objectui#4475 — the members / invitations tabs gate their affordances by the + * active member's org role, instead of offering every one of them to everybody + * and letting the server's 403 be the UI. + * + * ## The defect, as the card measured it + * + * A user whose org role is `member` opened `/organizations//members` and + * was shown an enabled **Invite member** button plus 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, inline, in raw server English. + * + * ## Why this file keys on `activeMember.role` and asserts three DIFFERENT gates + * + * The tempting fix is `role === 'owner'`. That is wrong in both directions, and + * the server says so. Measured against what actually enforces these routes — + * better-auth 1.6.26's `defaultStatements` / `adminAc` / `ownerAc` / `memberAc` + * (`plugins/organization/access/statement.mjs`), plus the `delegated_admin` + * role the framework registers on top of them + * (objectstack `packages/plugins/plugin-auth/src/auth-manager.ts`, the + * `defaultAc.newRole({ ...memberAc.statements, invitation: ['create'] })` + * block) — the three affordances on these two tabs sit behind three DIFFERENT + * permissions: + * + * | affordance | route | permission | who holds it | + * |---------------------|------------------------------|------------------------|-------------------------------| + * | Invite member | `/organization/invite-member`| `invitation:["create"]`| owner, admin, delegated_admin | + * | Remove member | `/organization/remove-member`| `member:["delete"]` | owner, admin | + * | Cancel invitation | `/organization/cancel-...` | `invitation:["cancel"]`| owner, admin | + * + * `delegated_admin` is the row that makes a single `isOwner` boolean unable to + * express this: it is built from `memberAc.statements` (so `member: []` — it + * may NOT remove anyone) with `invitation: ['create']` added (so it MAY + * invite), and deliberately WITHOUT `cancel`, because better-auth's cancel + * route checks only the permission and never invitation attribution — the + * server's own comment says so. Three gates, not one. + * + * So the assertions below are per-affordance, per-role, and they are the pin + * that a later "simplification" to `role === 'owner'` has to break. + * + * ## Reverse verification (what turns these red) + * + * Deleting the `canInviteMembers` guard turns the four `member`/unresolved + * invite assertions red; deleting `canRemoveMembers` turns the remove ones red + * and the "the Owner's row too" case red; widening `canInviteMembers` to plain + * grade>=admin turns the `delegated_admin` invite case red. The owner/admin + * cases are green on BOTH sides of the fix by construction — they are the + * must-not-change half, and they are what stops the gate being implemented as + * "hide it from everyone". + */ + +import '@testing-library/jest-dom/vitest'; +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import { render, screen, waitFor } from '@testing-library/react'; +import React from 'react'; + +// ── Mocks ──────────────────────────────────────────────────────────────────── + +const getMembers = vi.fn(); +const removeMember = vi.fn(); +const updateMemberRole = vi.fn(); +const listInvitations = vi.fn(); +const cancelInvitation = vi.fn(); +const inviteMember = vi.fn(); +const describeDelegableScope = vi.fn(); + +/** The role the ACTIVE member carries — the one input every gate reads. */ +const auth: { role: string | null } = { role: 'owner' }; + +// `importActual` spread, not a bare object: the real `orgRoleGrade` / +// `assignableOrgRoles` / `ORG_ROLE_*` are the subject's own dependencies, and +// stubbing them would make this file assert its own mock's opinion of the role +// model rather than the shipped one. +vi.mock('@object-ui/auth', async (importActual) => ({ + ...(await importActual()), + useAuth: () => ({ + getMembers, + removeMember, + updateMemberRole, + listInvitations, + cancelInvitation, + inviteMember, + describeDelegableScope, + isAuthenticated: true, + isLoading: false, + activeMember: auth.role === null ? null : { role: auth.role }, + }), +})); + +// The en oracle: `t()` returns the call site's inline `defaultValue`. This file +// is about which affordances RENDER, not about which language they render in — +// #4474's own file owns that. The pack-parity block at the bottom reads the +// real `builtInLocales`, which this spread keeps intact. +vi.mock('@object-ui/i18n', async (importActual) => ({ + ...(await importActual()), + useObjectTranslation: () => ({ + t: (key: string, opts?: { defaultValue?: string } & Record) => { + let out = opts?.defaultValue ?? key; + for (const [k, v] of Object.entries(opts ?? {})) { + if (k !== 'defaultValue') out = out.replace(new RegExp(`{{${k}}}`, 'g'), String(v)); + } + return out; + }, + }), +})); + +vi.mock('sonner', () => ({ toast: { success: vi.fn(), error: vi.fn() } })); + +vi.mock('react-router-dom', () => ({ + useOutletContext: () => ({ org: { id: 'org-42', name: 'Acme', slug: 'acme' } }), + useNavigate: () => vi.fn(), +})); + +vi.mock('@object-ui/components', () => { + const passthrough = (tag: string) => (p: any) => React.createElement(tag, p, p.children); + return { + Avatar: passthrough('div'), + AvatarFallback: passthrough('div'), + AvatarImage: (p: any) => , + Badge: ({ children, ...rest }: any) => {children}, + Button: ({ children, ...rest }: any) => , + Input: (p: any) => , + Label: passthrough('label'), + Separator: () =>
, + Dialog: ({ open, children }: any) => (open ?
{children}
: null), + DialogContent: passthrough('div'), + DialogDescription: passthrough('p'), + DialogFooter: passthrough('div'), + DialogHeader: passthrough('div'), + DialogTitle: passthrough('h2'), + AlertDialog: ({ open, children }: any) => (open ?
{children}
: null), + AlertDialogAction: ({ children, ...rest }: any) => , + AlertDialogCancel: ({ children, ...rest }: any) => , + AlertDialogContent: passthrough('div'), + AlertDialogDescription: passthrough('p'), + AlertDialogFooter: passthrough('div'), + AlertDialogHeader: passthrough('div'), + AlertDialogTitle: passthrough('h2'), + // Deliberately NOT gated on open state: the menu's items render inline, so + // "is Remove member reachable at all" is a DOM query rather than a click + // sequence. Pre-fix that is exactly how the card saw it. + DropdownMenu: ({ children }: any) =>
{children}
, + DropdownMenuContent: passthrough('div'), + DropdownMenuItem: ({ children, ...rest }: any) => , + DropdownMenuTrigger: ({ children }: any) => <>{children}, + Select: ({ value, onValueChange, children }: any) => ( + + ), + SelectContent: ({ children }: any) => <>{children}, + SelectItem: ({ value, children, ...rest }: any) => ( + + ), + SelectTrigger: ({ children }: any) => <>{children}, + SelectValue: () => null, + }; +}); + +vi.mock('lucide-react', () => { + const icon = () => ; + return { + Loader2: icon, + Copy: icon, + Check: icon, + MoreHorizontal: icon, + UserMinus: icon, + ShieldCheck: icon, + X: icon, + Mail: icon, + Upload: icon, + }; +}); + +import { builtInLocales } from '@object-ui/i18n'; +import { MembersPage } from '../manage/MembersPage'; +import { InvitationsPage } from '../manage/InvitationsPage'; + +// ── Harness ────────────────────────────────────────────────────────────────── + +const OWNER_ROW = 'm-1'; +const MEMBER_ROW = 'm-2'; + +beforeEach(() => { + vi.clearAllMocks(); + auth.role = 'owner'; + describeDelegableScope.mockResolvedValue(null); + getMembers.mockResolvedValue([ + { id: OWNER_ROW, role: 'owner', userId: 'u-1', user: { name: 'Ada', email: 'ada@x.test' } }, + { id: MEMBER_ROW, role: 'member', userId: 'u-2', user: { name: 'Bo', email: 'bo@x.test' } }, + ]); + listInvitations.mockResolvedValue([ + { id: 'inv-1', email: 'cy@x.test', role: 'member', status: 'pending' }, + ]); +}); + +const mountMembers = async () => { + render(); + await waitFor(() => expect(screen.getByTestId('members-page')).toBeInTheDocument()); +}; + +const mountInvitations = async () => { + render(); + await waitFor(() => expect(screen.getByTestId('invitations-page')).toBeInTheDocument()); +}; + +/** Every "Remove member" item currently in the DOM, whatever row it sits on. */ +const removeItems = () => screen.queryAllByText('Remove member'); + +// ───────────────────────────────────────────────────────────────────────────── +// The defect: a plain `member` is offered both write affordances +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4475 — a `member` is offered no member-management affordance', () => { + beforeEach(() => { + auth.role = 'member'; + }); + + it('shows no Invite member button', async () => { + await mountMembers(); + expect(screen.queryByTestId('invite-member-btn')).toBeNull(); + }); + + it('shows no Remove member action on ANY row — the Owner’s included', async () => { + await mountMembers(); + // Pre-fix this was 2: one per row, the owner row's being the card's + // headline ("including on the row of the workspace Owner"). + expect(removeItems()).toHaveLength(0); + }); + + it('renders no row action menu at all rather than an empty popup', async () => { + await mountMembers(); + // `assignableOrgRoles('member', …)` was already empty pre-fix, so once + // Remove goes the menu has nothing left. An affordance that opens onto + // nothing is worse than no affordance. + expect(screen.queryAllByLabelText('Member actions')).toHaveLength(0); + }); + + it('still lists the members — gating removes actions, never the page', async () => { + await mountMembers(); + expect(screen.getByTestId(`member-row-${OWNER_ROW}`)).toBeInTheDocument(); + expect(screen.getByTestId(`member-row-${MEMBER_ROW}`)).toBeInTheDocument(); + expect(screen.getByText('Ada')).toBeInTheDocument(); + }); + + it('explains the absence where the Invite button used to sit', async () => { + // The Settings tab's own convention, mirrored: the explanatory copy takes + // the PLACE of the affordance it replaces (there, the form; here, the + // button), so the space does not simply go blank. + await mountMembers(); + expect(screen.getByTestId('invite-restricted-note')).toHaveTextContent( + 'Only organization admins can invite members.', + ); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Must-not-change: the roles the server DOES admit keep both affordances +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4475 must-not-change — owner and admin keep every affordance', () => { + for (const role of ['owner', 'admin'] as const) { + it(`${role}: Invite member is present`, async () => { + auth.role = role; + await mountMembers(); + expect(screen.getByTestId('invite-member-btn')).toBeInTheDocument(); + expect(screen.queryByTestId('invite-restricted-note')).toBeNull(); + }); + + it(`${role}: Remove member is offered on every row`, async () => { + auth.role = role; + await mountMembers(); + expect(removeItems()).toHaveLength(2); + }); + + it(`${role}: the row action menu is present`, async () => { + auth.role = role; + await mountMembers(); + expect(screen.queryAllByLabelText('Member actions')).toHaveLength(2); + }); + } + + it('admin still cannot RE-ROLE an owner, and that narrowing is untouched', async () => { + // Pinned so the new remove gate cannot be mistaken for the role-item gate: + // `assignableOrgRoles('admin', 'owner')` is empty (better-auth's creatorRole + // protection), yet an admin DOES hold `member:["delete"]`. The owner row + // therefore keeps its menu, with Remove and no role items. + auth.role = 'admin'; + await mountMembers(); + const ownerRow = screen.getByTestId(`member-row-${OWNER_ROW}`); + expect(ownerRow.querySelector('[data-testid="member-role-owner"]')).toBeNull(); + expect(ownerRow.textContent).toContain('Remove member'); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// `delegated_admin` — the role a single `isOwner` boolean cannot express +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4475 — delegated_admin may invite but may NOT remove', () => { + beforeEach(() => { + auth.role = 'delegated_admin'; + }); + + it('sees the Invite member button (holds invitation:["create"])', async () => { + await mountMembers(); + expect(screen.getByTestId('invite-member-btn')).toBeInTheDocument(); + }); + + it('sees no Remove member action (built from memberAc — member: [])', async () => { + await mountMembers(); + expect(removeItems()).toHaveLength(0); + }); + + it('gets no row action menu, having neither remove nor any assignable role', async () => { + await mountMembers(); + expect(screen.queryAllByLabelText('Member actions')).toHaveLength(0); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// Fail-closed floor +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4475 — an unresolved active member is offered nothing', () => { + it('null activeMember: no invite, no remove', async () => { + auth.role = null; + await mountMembers(); + expect(screen.queryByTestId('invite-member-btn')).toBeNull(); + expect(removeItems()).toHaveLength(0); + }); + + it('an unknown role grades as an ordinary member', async () => { + auth.role = 'sales_rep'; + await mountMembers(); + expect(screen.queryByTestId('invite-member-btn')).toBeNull(); + expect(removeItems()).toHaveLength(0); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// The invitations tab, gated by the SAME measured rule +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4475 — the invitations tab gates copy-link and cancel', () => { + const copyBtn = () => screen.queryByLabelText('Copy invitation link'); + const cancelBtn = () => screen.queryByLabelText('Cancel invitation'); + + it('member: neither affordance is present', async () => { + auth.role = 'member'; + await mountInvitations(); + expect(copyBtn()).toBeNull(); + expect(cancelBtn()).toBeNull(); + }); + + it('member: the ledger itself still renders (objectstack#8095 owns that question)', async () => { + // Scope guard, pinned: whether `org_member` should READ the invitation + // ledger at all is escalated server-side and deliberately NOT decided here. + // This card only removes the WRITE affordances. + auth.role = 'member'; + await mountInvitations(); + expect(screen.getByTestId('invitation-row-inv-1')).toBeInTheDocument(); + expect(screen.getByText('cy@x.test')).toBeInTheDocument(); + }); + + for (const role of ['owner', 'admin'] as const) { + it(`${role}: keeps copy-link and cancel`, async () => { + auth.role = role; + await mountInvitations(); + expect(copyBtn()).toBeInTheDocument(); + expect(cancelBtn()).toBeInTheDocument(); + }); + } + + it('delegated_admin: may deliver an invitation, may not cancel one', async () => { + // The server's asymmetry, mirrored exactly: `invitation: ['create']` was + // added WITHOUT `cancel` because better-auth's cancel route checks only the + // permission and would therefore mean "cancel anyone's pending invitation". + auth.role = 'delegated_admin'; + await mountInvitations(); + expect(copyBtn()).toBeInTheDocument(); + expect(cancelBtn()).toBeNull(); + }); +}); + +// ───────────────────────────────────────────────────────────────────────────── +// #4474's channel: one new string, present in all ten packs +// ───────────────────────────────────────────────────────────────────────────── + +describe('#4475 — the new note is translatable in every pack', () => { + it('all ten packs define organization.members.inviteRestrictedNote', () => { + const langs = Object.keys(builtInLocales) as Array; + expect(langs.length).toBe(10); + for (const lang of langs) { + const value = (builtInLocales[lang] as any).organization?.members?.inviteRestrictedNote; + expect(typeof value, `${lang}.organization.members.inviteRestrictedNote`).toBe('string'); + expect(value.length, `${lang} value is empty`).toBeGreaterThan(0); + } + }); + + it('the en value is exactly the call site’s inline defaultValue', () => { + // `scripts/check-i18n-call-site-keys.mjs` enforces this too; asserting it + // here keeps the pair readable from the test that depends on the string. + expect((builtInLocales.en as any).organization.members.inviteRestrictedNote).toBe( + 'Only organization admins can invite members.', + ); + }); +}); diff --git a/packages/app-shell/src/console/organizations/manage/InvitationsPage.tsx b/packages/app-shell/src/console/organizations/manage/InvitationsPage.tsx index 012d37e91..061c23d8f 100644 --- a/packages/app-shell/src/console/organizations/manage/InvitationsPage.tsx +++ b/packages/app-shell/src/console/organizations/manage/InvitationsPage.tsx @@ -26,6 +26,7 @@ import { useObjectTranslation } from '@object-ui/i18n'; import { Loader2, Copy, Check, X, Mail } from 'lucide-react'; import { toast } from 'sonner'; import { useOrgContext } from './orgContext'; +import { canCancelInvitations, canInviteMembers } from './orgCapabilities'; import { resolveConsoleUrl } from '../resolveHomeUrl'; import { resolveOrgRoleLabel } from '../orgRoleLabel'; import { resolveOrgErrorMessage } from '../orgErrorMessage'; @@ -49,7 +50,24 @@ function statusBadgeVariant(status: string): 'outline' | 'default' | 'destructiv export function InvitationsPage() { const { t } = useObjectTranslation(); const { org } = useOrgContext(); - const { listInvitations, cancelInvitation } = useAuth(); + const { listInvitations, cancelInvitation, activeMember } = useAuth(); + + /* objectui#4475 — the two pending-row affordances answer to two DIFFERENT + server gates, so they get two predicates rather than one "can administer + invitations" flag: + + - cancel -> `invitation:["cancel"]`, which is owner/admin; + - copy link -> the delivery half of `invitation:["create"]`. The link IS + the invitation (anyone holding it can accept), so handing it out is the + issuing capability finishing its job — which is why a `delegated_admin`, + who may create invitations but deliberately may not cancel them, keeps + this one and loses the other. + + What a `member` may READ here is a separate, server-side question and is + escalated as objectstack#8095 — deliberately NOT decided by this card. Only + the write affordances are gated; the ledger still renders. */ + const canCancel = canCancelInvitations(activeMember?.role); + const canCopyLink = canInviteMembers(activeMember?.role); const [invitations, setInvitations] = useState([]); const [isLoading, setIsLoading] = useState(true); @@ -220,34 +238,38 @@ export function InvitationsPage() { {t(`organization.invitations.status.${inv.status}`, { defaultValue: inv.status })} - {inv.status === 'pending' && ( + {inv.status === 'pending' && (canCopyLink || canCancel) && (
- - + {canCopyLink && ( + + )} + {canCancel && ( + + )}
)} diff --git a/packages/app-shell/src/console/organizations/manage/MembersPage.tsx b/packages/app-shell/src/console/organizations/manage/MembersPage.tsx index c26850a95..04a059778 100644 --- a/packages/app-shell/src/console/organizations/manage/MembersPage.tsx +++ b/packages/app-shell/src/console/organizations/manage/MembersPage.tsx @@ -32,6 +32,7 @@ import { Loader2, MoreHorizontal, UserMinus, ShieldCheck } from 'lucide-react'; import { toast } from 'sonner'; import { useOrgContext } from './orgContext'; import { InviteMemberDialog } from './InviteMemberDialog'; +import { canInviteMembers, canRemoveMembers } from './orgCapabilities'; import { resolveOrgRoleLabel } from '../orgRoleLabel'; import { resolveOrgErrorMessage } from '../orgErrorMessage'; @@ -50,6 +51,14 @@ export function MembersPage() { const { org } = useOrgContext(); const { getMembers, removeMember, updateMemberRole, activeMember } = useAuth(); + /* objectui#4475 — what this viewer may actually do here. Both read the SAME + `activeMember.role` the role-change narrowing below already keys on (one + role source per screen), and both are measured against the server gate they + mirror — see `orgCapabilities`. `canInvite` is the wider set: it includes + `delegated_admin`, which may invite and may not remove. */ + const canInvite = canInviteMembers(activeMember?.role); + const canRemove = canRemoveMembers(activeMember?.role); + const [members, setMembers] = useState([]); const [isLoading, setIsLoading] = useState(true); const [error, setError] = useState(null); @@ -138,16 +147,38 @@ export function MembersPage() {

{t('organization.members.title', { defaultValue: 'Members' })} ({members.length})

- + {/* objectui#4475 — the Settings tab's convention, applied to this slot: + the explanatory copy takes the PLACE of the affordance it replaces + (there a form, here the button), in the same `text-sm + text-muted-foreground` voice, so the space says why instead of going + blank. It is not a disabled button: a control that exists only to + refuse is still an invitation to try. */} + {canInvite ? ( + + ) : ( +

+ {t('organization.members.inviteRestrictedNote', { + defaultValue: 'Only organization admins can invite members.', + })} +

+ )} {/* Member list */}
- {members.map((member) => ( + {members.map((member) => { + /* [framework #3697] Roles this actor may SET on THIS member — see the + menu below for what the list mirrors. Hoisted out of the JSX because + objectui#4475 needs its emptiness twice: an actor with no assignable + role AND no remove permission gets no menu at all, rather than a + trigger that opens onto nothing. */ + const assignable = assignableOrgRoles(activeMember?.role, member.role); + const hasRowActions = assignable.length > 0 || canRemove; + return (
)} - - - {/* Icon-only: this aria-label is the ONLY name a screen reader - gets for the row's action menu (objectui#4474). */} - - - - {/* [framework #3697] Roles this actor may SET on THIS member. - Mirrors better-auth's `update-member-role` route: it needs - the `member:["update"]` permission (owner/admin only — a - `delegated_admin` is built from `memberAc` and holds - `member: []`), and only an owner may set `owner` or re-role - someone who already is one. An actor who may re-role nobody - gets no items rather than three that would 403. */} - {assignableOrgRoles(activeMember?.role, member.role).map((role) => ( - handleChangeRole(member, role)} - disabled={member.role === role} - data-testid={`member-role-${role}`} - > - - {t(ORG_ROLE_LABELS[role].key, { - defaultValue: ORG_ROLE_LABELS[role].defaultValue, + {hasRowActions && ( + + + {/* Icon-only: this aria-label is the ONLY name a screen reader + gets for the row's action menu (objectui#4474). */} + + + + {/* Mirrors better-auth's `update-member-role` route: it needs + the `member:["update"]` permission (owner/admin only — a + `delegated_admin` is built from `memberAc` and holds + `member: []`), and only an owner may set `owner` or re-role + someone who already is one. An actor who may re-role nobody + gets no items rather than three that would 403. */} + {assignable.map((role) => ( + handleChangeRole(member, role)} + disabled={member.role === role} + data-testid={`member-role-${role}`} + > + + {t(ORG_ROLE_LABELS[role].key, { + defaultValue: ORG_ROLE_LABELS[role].defaultValue, + })} + + ))} + {/* objectui#4475 — the card's headline: this item used to be + unconditional, so a `member` was offered Remove on every + row INCLUDING the Owner's, and only the server's 403 (or, + on the remove route, a 400 from the lookup that runs first) + told them otherwise. `member:["delete"]` is owner/admin. */} + {canRemove && ( + setRemovingMember(member)} + > + + {t('organization.members.removeMember', { defaultValue: 'Remove member' })} + + )} + + + )}
- ))} + ); + })}
{/* Remove confirmation dialog */} @@ -262,13 +302,18 @@ export function MembersPage() { - {/* Invite dialog */} - fetchMembers()} - /> + {/* Invite dialog — not mounted for an actor who may not invite. Nothing + can open it either way (the trigger is gone), but leaving it out keeps + its delegable-scope fetch from running for a viewer who has no use for + the answer. */} + {canInvite && ( + fetchMembers()} + /> + )} ); } diff --git a/packages/app-shell/src/console/organizations/manage/orgCapabilities.ts b/packages/app-shell/src/console/organizations/manage/orgCapabilities.ts new file mode 100644 index 000000000..fcfbe595f --- /dev/null +++ b/packages/app-shell/src/console/organizations/manage/orgCapabilities.ts @@ -0,0 +1,110 @@ +/** + * ObjectUI + * Copyright (c) 2024-present ObjectStack Inc. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + */ + +/** + * Which member-management affordances the ACTIVE member may use — one predicate + * per server gate, so a screen can omit what would 403 instead of letting the + * rejection be the UI (objectui#4475). + * + * ## Why three predicates and not one `isOwner` + * + * The obvious fix for #4475 is a single owner check. It is wrong in both + * directions, and the server is what says so. These three affordances sit + * behind three DIFFERENT better-auth permissions, and the roles holding them + * are three different sets: + * + * | affordance | route | permission | + * |-------------------|---------------------------------|-------------------------| + * | invite a member | `/organization/invite-member` | `invitation:["create"]` | + * | remove a member | `/organization/remove-member` | `member:["delete"]` | + * | cancel invitation | `/organization/cancel-invitation` | `invitation:["cancel"]` | + * + * Measured against better-auth 1.6.26's + * `plugins/organization/access/statement.mjs` — `ownerAc` and `adminAc` both + * carry `invitation: ["create","cancel"]` and `member: [...,"delete"]`, while + * `memberAc` carries `invitation: []` and `member: []` — plus the fourth role + * the framework registers on top of them (objectstack + * `packages/plugins/plugin-auth/src/auth-manager.ts`): + * + * built[MEMBERSHIP_ROLE_DELEGATED_ADMIN] = defaultAc.newRole({ + * ...memberAc.statements, // member: [] — may NOT remove anyone + * invitation: ['create'], // …but MAY invite + * }); + * + * `delegated_admin` is the row a single boolean cannot express: it may invite + * and may not remove, and it deliberately has no `cancel` — 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"). Mirroring that asymmetry is the whole point; + * flattening it back into one check is the bug returning. + * + * ## These NARROW, they never decide + * + * Same contract as `invitableOrgRoles` / `assignableOrgRoles` next door in + * `@object-ui/auth`: the server re-checks every one of these routes, so the + * console is free to be helpful without being load-bearing. And they fail + * toward LESS — an absent or unreadable role grades below a plain member + * (`orgRoleGrade`'s documented floor), so an unverified caller is offered + * nothing. + * + * ## Where this lives + * + * The grade ladder itself stays in `@object-ui/auth`'s `org-roles.ts`, and this + * module composes it rather than restating it — `orgRoleGrade` and the role + * names are imported, never re-derived, so the closed ADR-0108 vocabulary keeps + * exactly one definition. + */ + +import { ORG_ROLE_ADMIN, ORG_ROLE_DELEGATED_ADMIN, orgRoleGrade } from '@object-ui/auth'; + +/** + * The grade at which better-auth's administrative permissions begin, asked of + * the ladder rather than written down as a number — `orgRoleGrade`'s scale is + * its own business and only `owner`/`admin` are auto-elevated server-side. + */ +const ADMIN_GRADE = orgRoleGrade(ORG_ROLE_ADMIN); + +/** + * `sys_member.role` is a COMMA-SEPARATED list server-side — better-auth's own + * routes do `member.role.split(",")` — so a role string may name several roles + * at once. Parsed the same way `orgRoleGrade` parses it, because a value it + * grades and a value we scan for `delegated_admin` must be the same value. + */ +function orgRoleNames(raw: unknown): string[] { + if (typeof raw !== 'string') return []; + return raw + .split(',') + .map((r) => r.trim().toLowerCase()) + .filter(Boolean); +} + +/** Holds `invitation:["create"]` — owner, admin, or a delegated admin. */ +export function canInviteMembers(actorRole: unknown): boolean { + if (orgRoleGrade(actorRole) >= ADMIN_GRADE) return true; + // The delegated grade is invisible to the ladder by design: it carries no + // ObjectStack authority, so it must NOT grade as an admin. It is exactly one + // reachable endpoint, and this is that endpoint. + return orgRoleNames(actorRole).includes(ORG_ROLE_DELEGATED_ADMIN); +} + +/** + * Holds `member:["delete"]` — owner and admin only. A `delegated_admin` is + * built from `memberAc.statements`, whose `member` list is empty. + */ +export function canRemoveMembers(actorRole: unknown): boolean { + return orgRoleGrade(actorRole) >= ADMIN_GRADE; +} + +/** + * Holds `invitation:["cancel"]` — owner and admin only. Deliberately NOT the + * same set as {@link canInviteMembers}: the framework grants the delegated + * grade `create` without `cancel`. + */ +export function canCancelInvitations(actorRole: unknown): boolean { + return orgRoleGrade(actorRole) >= ADMIN_GRADE; +} diff --git a/packages/i18n/src/locales/ar.ts b/packages/i18n/src/locales/ar.ts index fbf66ec0d..2b44bc28b 100644 --- a/packages/i18n/src/locales/ar.ts +++ b/packages/i18n/src/locales/ar.ts @@ -2563,6 +2563,7 @@ const ar = { members: { title: "الأعضاء", inviteMember: "دعوة عضو", + inviteRestrictedNote: "لا يمكن دعوة الأعضاء إلا لمسؤولي المؤسسة.", removeMember: "إزالة العضو", removeConfirmTitle: "إزالة العضو؟", removeConfirmDescription: "سيؤدي ذلك إلى إزالة {{name}} من المؤسسة، وسيفقد صلاحية الوصول فورًا.", diff --git a/packages/i18n/src/locales/de.ts b/packages/i18n/src/locales/de.ts index bfe75fec8..fd85afc9f 100644 --- a/packages/i18n/src/locales/de.ts +++ b/packages/i18n/src/locales/de.ts @@ -2556,6 +2556,7 @@ const de = { members: { title: "Mitglieder", inviteMember: "Mitglied einladen", + inviteRestrictedNote: "Nur Organisationsadministratoren können Mitglieder einladen.", removeMember: "Mitglied entfernen", removeConfirmTitle: "Mitglied entfernen?", removeConfirmDescription: "{{name}} wird aus der Organisation entfernt und verliert sofort den Zugriff.", diff --git a/packages/i18n/src/locales/en.ts b/packages/i18n/src/locales/en.ts index d24b898a2..79dccfe33 100644 --- a/packages/i18n/src/locales/en.ts +++ b/packages/i18n/src/locales/en.ts @@ -2788,6 +2788,7 @@ const en = { members: { title: 'Members', inviteMember: 'Invite member', + inviteRestrictedNote: 'Only organization admins can invite members.', removeMember: 'Remove member', removeConfirmTitle: 'Remove member?', removeConfirmDescription: 'This will remove {{name}} from the organization. They will lose access immediately.', diff --git a/packages/i18n/src/locales/es.ts b/packages/i18n/src/locales/es.ts index 7640cb212..ba38dc2cb 100644 --- a/packages/i18n/src/locales/es.ts +++ b/packages/i18n/src/locales/es.ts @@ -2560,6 +2560,7 @@ const es = { members: { title: "Miembros", inviteMember: "Invitar miembro", + inviteRestrictedNote: "Solo los administradores de la organización pueden invitar a miembros.", removeMember: "Eliminar miembro", removeConfirmTitle: "¿Eliminar miembro?", removeConfirmDescription: "Se eliminará a {{name}} de la organización y perderá el acceso de inmediato.", diff --git a/packages/i18n/src/locales/fr.ts b/packages/i18n/src/locales/fr.ts index 4f31c3615..1a8be0e46 100644 --- a/packages/i18n/src/locales/fr.ts +++ b/packages/i18n/src/locales/fr.ts @@ -2558,6 +2558,7 @@ const fr = { members: { title: "Membres", inviteMember: "Inviter un membre", + inviteRestrictedNote: "Seuls les administrateurs de l'organisation peuvent inviter des membres.", removeMember: "Retirer le membre", removeConfirmTitle: "Retirer ce membre ?", removeConfirmDescription: "{{name}} sera retiré de l'organisation et perdra immédiatement son accès.", diff --git a/packages/i18n/src/locales/ja.ts b/packages/i18n/src/locales/ja.ts index 09905adcd..6bb0dce25 100644 --- a/packages/i18n/src/locales/ja.ts +++ b/packages/i18n/src/locales/ja.ts @@ -2556,6 +2556,7 @@ const ja = { members: { title: "メンバー", inviteMember: "メンバーを招待", + inviteRestrictedNote: "組織の管理者のみがメンバーを招待できます。", removeMember: "メンバーを削除", removeConfirmTitle: "メンバーを削除しますか？", removeConfirmDescription: "{{name}} を組織から削除します。アクセス権は直ちに失われます。", diff --git a/packages/i18n/src/locales/ko.ts b/packages/i18n/src/locales/ko.ts index 4505a1739..a66a2c81c 100644 --- a/packages/i18n/src/locales/ko.ts +++ b/packages/i18n/src/locales/ko.ts @@ -2555,6 +2555,7 @@ const ko = { members: { title: "구성원", inviteMember: "구성원 초대", + inviteRestrictedNote: "조직 관리자만 구성원을 초대할 수 있습니다.", removeMember: "구성원 제거", removeConfirmTitle: "구성원을 제거할까요?", removeConfirmDescription: "{{name}}을(를) 조직에서 제거합니다. 접근 권한이 즉시 사라집니다.", diff --git a/packages/i18n/src/locales/pt.ts b/packages/i18n/src/locales/pt.ts index 10c08af02..8c511c558 100644 --- a/packages/i18n/src/locales/pt.ts +++ b/packages/i18n/src/locales/pt.ts @@ -2555,6 +2555,7 @@ const pt = { members: { title: "Membros", inviteMember: "Convidar membro", + inviteRestrictedNote: "Apenas administradores da organização podem convidar membros.", removeMember: "Remover membro", removeConfirmTitle: "Remover membro?", removeConfirmDescription: "Isso removerá {{name}} da organização. O acesso será perdido imediatamente.", diff --git a/packages/i18n/src/locales/ru.ts b/packages/i18n/src/locales/ru.ts index 25e0d0241..1a66ba885 100644 --- a/packages/i18n/src/locales/ru.ts +++ b/packages/i18n/src/locales/ru.ts @@ -2567,6 +2567,7 @@ const ru = { members: { title: "Участники", inviteMember: "Пригласить участника", + inviteRestrictedNote: "Только администраторы организации могут приглашать участников.", removeMember: "Удалить участника", removeConfirmTitle: "Удалить участника?", removeConfirmDescription: "{{name}} будет удалён из организации и сразу потеряет доступ.", diff --git a/packages/i18n/src/locales/zh.ts b/packages/i18n/src/locales/zh.ts index 802b8a1fe..e491f448d 100644 --- a/packages/i18n/src/locales/zh.ts +++ b/packages/i18n/src/locales/zh.ts @@ -2673,6 +2673,7 @@ const zh = { members: { title: '成员', inviteMember: '邀请成员', + inviteRestrictedNote: '只有组织管理员可以邀请成员。', removeMember: '移除成员', removeConfirmTitle: '移除成员？', removeConfirmDescription: '将把 {{name}} 从该组织中移除，其访问权限会立即失效。',