From e6178b7fbf0a220d839fa751cdeec98a5c698859 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 21 Aug 2026 03:18:46 +0000 Subject: [PATCH] fix: centralize membership suspension cleanup Paperclip-Task: PAP-17797 --- .../access-routes-permissions-upgrade.test.ts | 91 +++++++++ server/src/routes/access.ts | 175 ++---------------- 2 files changed, 104 insertions(+), 162 deletions(-) diff --git a/server/src/__tests__/access-routes-permissions-upgrade.test.ts b/server/src/__tests__/access-routes-permissions-upgrade.test.ts index b7180581a5d..7f9d440cfe9 100644 --- a/server/src/__tests__/access-routes-permissions-upgrade.test.ts +++ b/server/src/__tests__/access-routes-permissions-upgrade.test.ts @@ -5,10 +5,15 @@ import { and, eq } from "drizzle-orm"; import { afterAll, afterEach, beforeAll, describe, expect, it, vi } from "vitest"; import { activityLog, + agents, companies, companyMemberships, + connectionGrantDelegations, + connectionGrants, createDb, principalPermissionGrants, + toolApplications, + toolConnections, } from "@paperclipai/db"; import { getEmbeddedPostgresTestSupport, @@ -90,8 +95,13 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () = afterEach(async () => { await db.delete(activityLog); + await db.delete(connectionGrantDelegations); + await db.delete(connectionGrants); + await db.delete(toolConnections); + await db.delete(toolApplications); await db.delete(principalPermissionGrants); await db.delete(companyMemberships); + await db.delete(agents); await db.delete(companies); }); @@ -164,4 +174,85 @@ describeEmbeddedPostgres("access routes permissions upgrade compatibility", () = grantedByUserId: owner.principalId, }); }); + + it("sweeps personal connection access when the member route suspends a user", async () => { + const { company, owner } = await createCompanyWithOwner(db); + const member = await db.insert(companyMemberships).values({ + companyId: company.id, + principalType: "user", + principalId: `member-${randomUUID()}`, + status: "active", + membershipRole: "member", + }).returning().then((rows) => rows[0]!); + const agent = await db.insert(agents).values({ + companyId: company.id, + name: "Delegated route agent", + role: "worker", + adapterType: "process", + adapterConfig: {}, + }).returning().then((rows) => rows[0]!); + const application = await db.insert(toolApplications).values({ + companyId: company.id, + applicationKey: `route-app-${randomUUID()}`, + name: "Route personal app", + type: "mcp", + status: "active", + }).returning().then((rows) => rows[0]!); + const connection = await db.insert(toolConnections).values({ + companyId: company.id, + applicationId: application.id, + name: "Route personal connection", + uid: `route-connection-${randomUUID()}`, + connectionKind: "managed", + ownership: "customer", + transport: "mcp_remote", + authKind: "oauth", + credentialPolicy: "per_user", + status: "active", + enabled: true, + }).returning().then((rows) => rows[0]!); + const grant = await db.insert(connectionGrants).values({ + companyId: company.id, + connectionId: connection.id, + kind: "user", + subjectUserId: member.principalId, + status: "active", + }).returning().then((rows) => rows[0]!); + await db.insert(connectionGrantDelegations).values({ + companyId: company.id, + grantId: grant.id, + agentId: agent.id, + createdByUserId: member.principalId, + }); + + const res = await request(await createApp(db, company.id, owner.principalId)) + .patch(`/api/companies/${company.id}/members/${member.id}`) + .send({ status: "suspended" }); + + expect(res.status, JSON.stringify(res.body)).toBe(200); + expect(res.body.status).toBe("suspended"); + expect(await db.select().from(connectionGrantDelegations)).toHaveLength(0); + expect(await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id))) + .toEqual([expect.objectContaining({ status: "revoked" })]); + + await request(await createApp(db, company.id, owner.principalId)) + .patch(`/api/companies/${company.id}/members/${member.id}/role-and-grants`) + .send({ status: "active", grants: [] }) + .expect(200); + await db.update(connectionGrants).set({ status: "active" }).where(eq(connectionGrants.id, grant.id)); + await db.insert(connectionGrantDelegations).values({ + companyId: company.id, + grantId: grant.id, + agentId: agent.id, + createdByUserId: member.principalId, + }); + + const permissionsRoute = await request(await createApp(db, company.id, owner.principalId)) + .patch(`/api/companies/${company.id}/members/${member.id}/role-and-grants`) + .send({ status: "suspended", grants: [] }); + expect(permissionsRoute.status, JSON.stringify(permissionsRoute.body)).toBe(200); + expect(await db.select().from(connectionGrantDelegations)).toHaveLength(0); + expect(await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id))) + .toEqual([expect.objectContaining({ status: "revoked" })]); + }); }); diff --git a/server/src/routes/access.ts b/server/src/routes/access.ts index a1b166e7185..e5d8e53257d 100644 --- a/server/src/routes/access.ts +++ b/server/src/routes/access.ts @@ -14,7 +14,7 @@ import path from "node:path"; import { fileURLToPath } from "node:url"; import { Router } from "express"; import type { Request } from "express"; -import { and, desc, eq, gt, inArray, isNotNull, isNull, lte, ne, sql } from "drizzle-orm"; +import { and, desc, eq, gt, inArray, isNotNull, isNull, lte, ne } from "drizzle-orm"; import type { Db } from "@paperclipai/db"; import { assets, @@ -47,7 +47,7 @@ import { PERMISSION_KEYS, isUuidLike, } from "@paperclipai/shared"; -import type { DeploymentExposure, DeploymentMode, HumanCompanyMembershipRole, PermissionKey } from "@paperclipai/shared"; +import type { DeploymentExposure, DeploymentMode, HumanCompanyMembershipRole } from "@paperclipai/shared"; import { forbidden, conflict, @@ -121,11 +121,6 @@ const INVITE_TOKEN_MAX_RETRIES = 5; const COMPANY_INVITE_TTL_MS = 72 * 60 * 60 * 1000; const INVITE_RESOLUTION_DNS_TIMEOUT_MS = 3_000; -type MemberGrantPayload = { - permissionKey: PermissionKey; - scope?: Record | null; -}; - export function createInviteToken() { const suffix = randomBytes(INVITE_TOKEN_ENTROPY_BYTES).toString("base64url"); return `${INVITE_TOKEN_PREFIX}${suffix}`; @@ -4496,69 +4491,7 @@ export function accessRoutes( if (!memberToUpdate) throw notFound("Member not found"); await assertCanManageCompanyMember(req, access, companyId, memberToUpdate); - const updated = await db.transaction(async (tx) => { - await tx.execute(sql` - select ${companyMemberships.id} - from ${companyMemberships} - where ${companyMemberships.companyId} = ${companyId} - and ${companyMemberships.principalType} = 'user' - and ${companyMemberships.status} = 'active' - and ${companyMemberships.membershipRole} = 'owner' - for update - `); - - const existing = await tx - .select() - .from(companyMemberships) - .where( - and( - eq(companyMemberships.companyId, companyId), - eq(companyMemberships.id, memberId), - ), - ) - .then((rows) => rows[0] ?? null); - if (!existing) return null; - - const nextMembershipRole = - req.body.membershipRole !== undefined - ? req.body.membershipRole - : existing.membershipRole; - const nextStatus = req.body.status ?? existing.status; - - if ( - existing.principalType === "user" && - existing.status === "active" && - existing.membershipRole === "owner" && - (nextStatus !== "active" || nextMembershipRole !== "owner") - ) { - const activeOwnerCount = await tx - .select({ id: companyMemberships.id }) - .from(companyMemberships) - .where( - and( - eq(companyMemberships.companyId, companyId), - eq(companyMemberships.principalType, "user"), - eq(companyMemberships.status, "active"), - eq(companyMemberships.membershipRole, "owner"), - ), - ) - .then((rows) => rows.length); - if (activeOwnerCount <= 1) { - throw conflict("Cannot remove the last active owner"); - } - } - - return tx - .update(companyMemberships) - .set({ - membershipRole: nextMembershipRole, - status: nextStatus, - updatedAt: new Date(), - }) - .where(eq(companyMemberships.id, existing.id)) - .returning() - .then((rows) => rows[0] ?? existing); - }); + const updated = await access.updateMember(companyId, memberId, req.body); if (!updated) throw notFound("Member not found"); await logActivity(db, { @@ -4593,98 +4526,16 @@ export function accessRoutes( if (!memberToUpdate) throw notFound("Member not found"); await assertCanManageCompanyMember(req, access, companyId, memberToUpdate); - const updated = await db.transaction(async (tx) => { - await tx.execute(sql` - select ${companyMemberships.id} - from ${companyMemberships} - where ${companyMemberships.companyId} = ${companyId} - and ${companyMemberships.principalType} = 'user' - and ${companyMemberships.status} = 'active' - and ${companyMemberships.membershipRole} = 'owner' - for update - `); - - const existing = await tx - .select() - .from(companyMemberships) - .where( - and( - eq(companyMemberships.companyId, companyId), - eq(companyMemberships.id, memberId), - ), - ) - .then((rows) => rows[0] ?? null); - if (!existing) return null; - - const nextMembershipRole = - req.body.membershipRole !== undefined - ? req.body.membershipRole - : existing.membershipRole; - const nextStatus = req.body.status ?? existing.status; - - if ( - existing.principalType === "user" && - existing.status === "active" && - existing.membershipRole === "owner" && - (nextStatus !== "active" || nextMembershipRole !== "owner") - ) { - const activeOwnerCount = await tx - .select({ id: companyMemberships.id }) - .from(companyMemberships) - .where( - and( - eq(companyMemberships.companyId, companyId), - eq(companyMemberships.principalType, "user"), - eq(companyMemberships.status, "active"), - eq(companyMemberships.membershipRole, "owner"), - ), - ) - .then((rows) => rows.length); - if (activeOwnerCount <= 1) { - throw conflict("Cannot remove the last active owner"); - } - } - - const now = new Date(); - const updatedMember = await tx - .update(companyMemberships) - .set({ - membershipRole: nextMembershipRole, - status: nextStatus, - updatedAt: now, - }) - .where(eq(companyMemberships.id, existing.id)) - .returning() - .then((rows) => rows[0] ?? existing); - - await tx - .delete(principalPermissionGrants) - .where( - and( - eq(principalPermissionGrants.companyId, companyId), - eq(principalPermissionGrants.principalType, existing.principalType), - eq(principalPermissionGrants.principalId, existing.principalId), - ), - ); - - const grants = (req.body.grants ?? []) as MemberGrantPayload[]; - if (grants.length > 0) { - await tx.insert(principalPermissionGrants).values( - grants.map((grant) => ({ - companyId, - principalType: existing.principalType, - principalId: existing.principalId, - permissionKey: grant.permissionKey, - scope: grant.scope ?? null, - grantedByUserId: req.actor.userId ?? null, - createdAt: now, - updatedAt: now, - })), - ); - } - - return updatedMember; - }); + const updated = await access.updateMemberAndPermissions( + companyId, + memberId, + { + membershipRole: req.body.membershipRole, + status: req.body.status, + grants: req.body.grants ?? [], + }, + req.actor.userId ?? null, + ); if (!updated) throw notFound("Member not found"); await logActivity(db, {