mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 18:19:05 +02:00
Protect company member removal permissions
This commit is contained in:
1 parent
63b4dc7c5e
commit
aed04c2ba0
7 files changed
+272
-34
No files matched your search
@@ -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 {
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
@@ -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<ReturnType<typeof loadCompanyMemberRecords>>[number];
|
||||
|
||||
const humanRoleRank: Record<HumanCompanyMembershipRole, number> = {
|
||||
viewer: 1,
|
||||
operator: 2,
|
||||
admin: 3,
|
||||
owner: 4,
|
||||
};
|
||||
|
||||
async function resolveActorHumanRole(
|
||||
req: Request,
|
||||
access: ReturnType<typeof accessService>,
|
||||
companyId: string,
|
||||
): Promise<HumanCompanyMembershipRole | null> {
|
||||
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<typeof accessService>,
|
||||
companyId: string,
|
||||
member: { principalId: string; principalType: string; membershipRole: string | null },
|
||||
): Promise<string | null> {
|
||||
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<typeof accessService>,
|
||||
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<typeof accessService>,
|
||||
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));
|
||||
}
|
||||
|
||||
@@ -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",
|
||||
);
|
||||
|
||||
@@ -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 = {
|
||||
|
||||
@@ -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(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<CompanyAccess />
|
||||
</QueryClientProvider>,
|
||||
);
|
||||
});
|
||||
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();
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -348,37 +348,52 @@ export function CompanyAccess() {
|
||||
{members.length === 0 ? (
|
||||
<div className="px-4 py-8 text-sm text-muted-foreground">No user memberships found for this company yet.</div>
|
||||
) : (
|
||||
members.map((member) => (
|
||||
<div
|
||||
key={member.id}
|
||||
className="grid grid-cols-[minmax(0,1.5fr)_120px_120px_minmax(0,1.2fr)_180px] gap-3 border-b border-border px-4 py-3 last:border-b-0"
|
||||
>
|
||||
<div className="min-w-0">
|
||||
<div className="truncate font-medium">{member.user?.name?.trim() || member.user?.email || member.principalId}</div>
|
||||
<div className="truncate text-xs text-muted-foreground">{member.user?.email || member.principalId}</div>
|
||||
members.map((member) => {
|
||||
const removalReason = member.removal?.reason ?? null;
|
||||
const canArchive = member.removal?.canArchive ?? true;
|
||||
return (
|
||||
<div
|
||||
key={member.id}
|
||||
className="grid grid-cols-[minmax(0,1.5fr)_120px_120px_minmax(0,1.2fr)_180px] gap-3 border-b border-border px-4 py-3 last:border-b-0"
|
||||
>
|
||||
<div className="min-w-0">
|
||||
<div className="truncate font-medium">{member.user?.name?.trim() || member.user?.email || member.principalId}</div>
|
||||
<div className="truncate text-xs text-muted-foreground">{member.user?.email || member.principalId}</div>
|
||||
</div>
|
||||
<div className="text-sm">
|
||||
{member.membershipRole
|
||||
? HUMAN_COMPANY_MEMBERSHIP_ROLE_LABELS[member.membershipRole]
|
||||
: "Unset"}
|
||||
</div>
|
||||
<div>
|
||||
<Badge variant={member.status === "active" ? "secondary" : member.status === "suspended" ? "destructive" : "outline"}>
|
||||
{member.status.replace("_", " ")}
|
||||
</Badge>
|
||||
</div>
|
||||
<div className="min-w-0 text-sm text-muted-foreground">{formatGrantSummary(member)}</div>
|
||||
<div className="space-y-1 text-right">
|
||||
<div className="flex justify-end gap-2">
|
||||
<Button size="sm" variant="outline" onClick={() => setEditingMemberId(member.id)}>
|
||||
Edit
|
||||
</Button>
|
||||
<Button
|
||||
size="sm"
|
||||
variant="outline"
|
||||
onClick={() => setRemovingMemberId(member.id)}
|
||||
disabled={!canArchive}
|
||||
title={removalReason ?? undefined}
|
||||
>
|
||||
<Trash2 className="mr-1 h-3.5 w-3.5" />
|
||||
Remove
|
||||
</Button>
|
||||
</div>
|
||||
{removalReason ? (
|
||||
<div className="text-xs text-muted-foreground">{removalReason}</div>
|
||||
) : null}
|
||||
</div>
|
||||
</div>
|
||||
<div className="text-sm">
|
||||
{member.membershipRole
|
||||
? HUMAN_COMPANY_MEMBERSHIP_ROLE_LABELS[member.membershipRole]
|
||||
: "Unset"}
|
||||
</div>
|
||||
<div>
|
||||
<Badge variant={member.status === "active" ? "secondary" : member.status === "suspended" ? "destructive" : "outline"}>
|
||||
{member.status.replace("_", " ")}
|
||||
</Badge>
|
||||
</div>
|
||||
<div className="min-w-0 text-sm text-muted-foreground">{formatGrantSummary(member)}</div>
|
||||
<div className="flex justify-end gap-2">
|
||||
<Button size="sm" variant="outline" onClick={() => setEditingMemberId(member.id)}>
|
||||
Edit
|
||||
</Button>
|
||||
<Button size="sm" variant="outline" onClick={() => setRemovingMemberId(member.id)}>
|
||||
<Trash2 className="mr-1 h-3.5 w-3.5" />
|
||||
Remove
|
||||
</Button>
|
||||
</div>
|
||||
</div>
|
||||
))
|
||||
);
|
||||
})
|
||||
)}
|
||||
</div>
|
||||
</section>
|
||||
|
||||
Reference in new issue
Block a user