From aed04c2ba04b2538f09f66810095c874a980b307 Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 18 Apr 2026 13:25:58 -0500 Subject: [PATCH] Protect company member removal permissions --- packages/shared/src/types/access.ts | 4 + server/src/__tests__/access-service.test.ts | 47 +++++++++ server/src/routes/access.ts | 100 +++++++++++++++++++- server/src/services/access.ts | 16 +++- ui/src/api/access.ts | 4 + ui/src/pages/CompanyAccess.test.tsx | 60 ++++++++++++ ui/src/pages/CompanyAccess.tsx | 75 +++++++++------ 7 files changed, 272 insertions(+), 34 deletions(-) diff --git a/packages/shared/src/types/access.ts b/packages/shared/src/types/access.ts index 0102dd529fe..015d51792da 100644 --- a/packages/shared/src/types/access.ts +++ b/packages/shared/src/types/access.ts @@ -47,6 +47,10 @@ export interface CompanyMemberRecord extends CompanyMembership { membershipRole: HumanCompanyMembershipRole | null; user: AccessUserProfile | null; grants: PrincipalPermissionGrant[]; + removal?: { + canArchive: boolean; + reason: string | null; + }; } export interface CompanyMembersResponse { diff --git a/server/src/__tests__/access-service.test.ts b/server/src/__tests__/access-service.test.ts index 7ca629a1bad..f9ac3e64c9b 100644 --- a/server/src/__tests__/access-service.test.ts +++ b/server/src/__tests__/access-service.test.ts @@ -5,6 +5,7 @@ import { companies, companyMemberships, createDb, + instanceUserRoles, issues, principalPermissionGrants, } from "@paperclipai/db"; @@ -54,6 +55,7 @@ describeEmbeddedPostgres("access service", () => { afterEach(async () => { await db.delete(issues); await db.delete(principalPermissionGrants); + await db.delete(instanceUserRoles); await db.delete(companyMemberships); await db.delete(companies); }); @@ -174,4 +176,49 @@ describeEmbeddedPostgres("access service", () => { .then((rows) => rows[0]!); expect(historicalIssue.assigneeUserId).toBe(member.principalId); }); + + it("rejects instance-level company access removal for self and protected users", async () => { + const { company, owner } = await createCompanyWithOwner(db); + const access = accessService(db); + + await expect( + access.setUserCompanyAccess(owner.principalId, [], { actorUserId: owner.principalId }), + ).rejects.toThrow("You cannot remove yourself"); + + const admin = await db + .insert(companyMemberships) + .values({ + companyId: company.id, + principalType: "user", + principalId: `admin-${randomUUID()}`, + status: "active", + membershipRole: "admin", + }) + .returning() + .then((rows) => rows[0]!); + + await expect( + access.setUserCompanyAccess(admin.principalId, [], { actorUserId: owner.principalId }), + ).rejects.toThrow("Owners and admins cannot be removed from company access"); + + const operator = await db + .insert(companyMemberships) + .values({ + companyId: company.id, + principalType: "user", + principalId: `operator-${randomUUID()}`, + status: "active", + membershipRole: "operator", + }) + .returning() + .then((rows) => rows[0]!); + await db.insert(instanceUserRoles).values({ + userId: operator.principalId, + role: "instance_admin", + }); + + await expect( + access.setUserCompanyAccess(operator.principalId, [], { actorUserId: owner.principalId }), + ).rejects.toThrow("Instance admins cannot be removed from company access"); + }); }); diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index daf08d166d7..459a3bd6da7 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -39,7 +39,7 @@ import { updateUserCompanyAccessSchema, PERMISSION_KEYS } from "@paperclipai/shared"; -import type { DeploymentExposure, DeploymentMode, PermissionKey } from "@paperclipai/shared"; +import type { DeploymentExposure, DeploymentMode, HumanCompanyMembershipRole, PermissionKey } from "@paperclipai/shared"; import { forbidden, conflict, @@ -1047,6 +1047,87 @@ async function loadCompanyMemberRecords( })); } +type CompanyMemberRecord = Awaited>[number]; + +const humanRoleRank: Record = { + viewer: 1, + operator: 2, + admin: 3, + owner: 4, +}; + +async function resolveActorHumanRole( + req: Request, + access: ReturnType, + companyId: string, +): Promise { + if (req.actor.type !== "board") return null; + if (isLocalImplicit(req) || req.actor.isInstanceAdmin) return "owner"; + const userId = req.actor.userId ?? null; + if (!userId) return null; + const membership = await access.getMembership(companyId, "user", userId); + if (membership?.status !== "active" || !membership.membershipRole) return null; + return normalizeHumanRole(membership.membershipRole, "operator"); +} + +async function getProtectedMemberReason( + req: Request, + access: ReturnType, + companyId: string, + member: { principalId: string; principalType: string; membershipRole: string | null }, +): Promise { + if (member.principalType !== "user") return "Only human company members can be removed."; + if (req.actor.type !== "board") return "Board access is required to remove members."; + if (member.principalId === req.actor.userId) return "You cannot remove yourself."; + if (await access.isInstanceAdmin(member.principalId)) { + return "Instance admins cannot be removed from company access."; + } + + const targetRole = member.membershipRole + ? normalizeHumanRole(member.membershipRole, "operator") + : "operator"; + if (targetRole === "owner") return "Board owners cannot be removed from company access."; + if (targetRole === "admin") return "Company admins cannot be removed from company access."; + + const actorRole = await resolveActorHumanRole(req, access, companyId); + if (!actorRole) return "Only active company members can remove users."; + if (humanRoleRank[targetRole] >= humanRoleRank[actorRole]) { + return "You can only remove users below your company role."; + } + + return null; +} + +async function assertCanManageCompanyMember( + req: Request, + access: ReturnType, + companyId: string, + member: { principalId: string; principalType: string; membershipRole: string | null }, +) { + const reason = await getProtectedMemberReason(req, access, companyId, member); + if (reason) throw forbidden(reason); +} + +async function addCompanyMemberRemovalAccess( + req: Request, + access: ReturnType, + companyId: string, + members: CompanyMemberRecord[], +) { + return Promise.all( + members.map(async (member) => { + const reason = await getProtectedMemberReason(req, access, companyId, member); + return { + ...member, + removal: { + canArchive: !reason, + reason, + }, + }; + }), + ); +} + async function loadCompanyUserDirectory(db: Db, companyId: string) { const members = await db .select({ @@ -3610,7 +3691,7 @@ export function accessRoutes( loadCompanyAccessSummary(req, access, companyId), ]); res.json({ - members, + members: await addCompanyMemberRemovalAccess(req, access, companyId, members), access: currentAccess, }); }); @@ -3629,6 +3710,9 @@ export function accessRoutes( const companyId = req.params.companyId as string; const memberId = req.params.memberId as string; await assertCompanyPermission(req, companyId, "users:manage_permissions"); + const memberToUpdate = await access.getMemberById(companyId, memberId); + if (!memberToUpdate) throw notFound("Member not found"); + await assertCanManageCompanyMember(req, access, companyId, memberToUpdate); const updated = await db.transaction(async (tx) => { await tx.execute(sql` @@ -3723,6 +3807,9 @@ export function accessRoutes( const companyId = req.params.companyId as string; const memberId = req.params.memberId as string; await assertCompanyPermission(req, companyId, "users:manage_permissions"); + const memberToUpdate = await access.getMemberById(companyId, memberId); + if (!memberToUpdate) throw notFound("Member not found"); + await assertCanManageCompanyMember(req, access, companyId, memberToUpdate); const updated = await db.transaction(async (tx) => { await tx.execute(sql` @@ -3847,6 +3934,9 @@ export function accessRoutes( const companyId = req.params.companyId as string; const memberId = req.params.memberId as string; await assertCompanyPermission(req, companyId, "users:manage_permissions"); + const memberToArchive = await access.getMemberById(companyId, memberId); + if (!memberToArchive) throw notFound("Member not found"); + await assertCanManageCompanyMember(req, access, companyId, memberToArchive); const result = await access.archiveMember(companyId, memberId, { reassignment: req.body.reassignment ?? null, @@ -3885,6 +3975,9 @@ export function accessRoutes( const companyId = req.params.companyId as string; const memberId = req.params.memberId as string; await assertCompanyPermission(req, companyId, "users:manage_permissions"); + const memberToUpdate = await access.getMemberById(companyId, memberId); + if (!memberToUpdate) throw notFound("Member not found"); + await assertCanManageCompanyMember(req, access, companyId, memberToUpdate); const updated = await access.setMemberPermissions( companyId, memberId, @@ -4006,7 +4099,8 @@ export function accessRoutes( const userId = req.params.userId as string; await access.setUserCompanyAccess( userId, - req.body.companyIds ?? [] + req.body.companyIds ?? [], + { actorUserId: req.actor.userId ?? null }, ); res.json(await loadUserCompanyAccessResponse(db, access, userId)); } diff --git a/server/src/services/access.ts b/server/src/services/access.ts index 44d7155367c..faac70da407 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -451,13 +451,27 @@ export function accessService(db: Db) { .orderBy(sql`${companyMemberships.createdAt} desc`); } - async function setUserCompanyAccess(userId: string, companyIds: string[]) { + async function setUserCompanyAccess( + userId: string, + companyIds: string[], + options: { actorUserId?: string | null } = {}, + ) { const existing = await listUserCompanyAccess(userId); const existingByCompany = new Map(existing.map((row) => [row.companyId, row])); const target = new Set(companyIds); await db.transaction(async (tx) => { const toArchive = existing.filter((row) => !target.has(row.companyId) && row.status !== "archived"); + if (toArchive.length > 0 && options.actorUserId && options.actorUserId === userId) { + throw conflict("You cannot remove yourself"); + } + if (toArchive.length > 0 && (await isInstanceAdmin(userId))) { + throw conflict("Instance admins cannot be removed from company access"); + } + const protectedArchives = toArchive.filter((row) => row.membershipRole === "owner" || row.membershipRole === "admin"); + if (protectedArchives.length > 0) { + throw conflict("Owners and admins cannot be removed from company access"); + } const activeOwnerArchives = toArchive.filter( (row) => row.status === "active" && row.membershipRole === "owner", ); diff --git a/ui/src/api/access.ts b/ui/src/api/access.ts index c4c1d5cde4a..c8f83134e71 100644 --- a/ui/src/api/access.ts +++ b/ui/src/api/access.ts @@ -126,6 +126,10 @@ export type CompanyMember = { updatedAt: string; user: { id: string; email: string | null; name: string | null; image: string | null } | null; grants: CompanyMemberGrant[]; + removal?: { + canArchive: boolean; + reason: string | null; + }; }; export type ArchiveCompanyMemberResponse = { diff --git a/ui/src/pages/CompanyAccess.test.tsx b/ui/src/pages/CompanyAccess.test.tsx index 2ba0a45c512..eefb7d73054 100644 --- a/ui/src/pages/CompanyAccess.test.tsx +++ b/ui/src/pages/CompanyAccess.test.tsx @@ -322,4 +322,64 @@ describe("CompanyAccess", () => { root.unmount(); }); }); + + it("shows protected member removal reasons from the API", async () => { + listMembersMock.mockResolvedValueOnce({ + members: [ + { + id: "member-admin", + companyId: "company-1", + principalType: "user", + principalId: "admin-user", + status: "active", + membershipRole: "admin", + createdAt: "2026-04-10T00:00:00.000Z", + updatedAt: "2026-04-10T00:00:00.000Z", + user: { + id: "admin-user", + email: "admin@paperclip.local", + name: "Admin User", + image: null, + }, + grants: [], + removal: { + canArchive: false, + reason: "Company admins cannot be removed from company access.", + }, + }, + ], + access: { + currentUserRole: "owner", + canManageMembers: true, + canInviteUsers: true, + canApproveJoinRequests: false, + }, + }); + + const root = createRoot(container); + const queryClient = new QueryClient({ + defaultOptions: { queries: { retry: false } }, + }); + + await act(async () => { + root.render( + + + , + ); + }); + await flushReact(); + await flushReact(); + + expect(container.textContent).toContain("Company admins cannot be removed from company access."); + const removeButton = Array.from(container.querySelectorAll("button")).find( + (button) => button.textContent?.includes("Remove"), + ); + expect(removeButton).toBeTruthy(); + expect(removeButton).toHaveProperty("disabled", true); + + await act(async () => { + root.unmount(); + }); + }); }); diff --git a/ui/src/pages/CompanyAccess.tsx b/ui/src/pages/CompanyAccess.tsx index 1a9f9be5575..51ff8d69c7f 100644 --- a/ui/src/pages/CompanyAccess.tsx +++ b/ui/src/pages/CompanyAccess.tsx @@ -348,37 +348,52 @@ export function CompanyAccess() { {members.length === 0 ? (
No user memberships found for this company yet.
) : ( - members.map((member) => ( -
-
-
{member.user?.name?.trim() || member.user?.email || member.principalId}
-
{member.user?.email || member.principalId}
+ members.map((member) => { + const removalReason = member.removal?.reason ?? null; + const canArchive = member.removal?.canArchive ?? true; + return ( +
+
+
{member.user?.name?.trim() || member.user?.email || member.principalId}
+
{member.user?.email || member.principalId}
+
+
+ {member.membershipRole + ? HUMAN_COMPANY_MEMBERSHIP_ROLE_LABELS[member.membershipRole] + : "Unset"} +
+
+ + {member.status.replace("_", " ")} + +
+
{formatGrantSummary(member)}
+
+
+ + +
+ {removalReason ? ( +
{removalReason}
+ ) : null} +
-
- {member.membershipRole - ? HUMAN_COMPANY_MEMBERSHIP_ROLE_LABELS[member.membershipRole] - : "Unset"} -
-
- - {member.status.replace("_", " ")} - -
-
{formatGrantSummary(member)}
-
- - -
-
- )) + ); + }) )}