From 5ae50e9a714427237c8144c0952160e993fb17c0 Mon Sep 17 00:00:00 2001 From: Dotta Date: Fri, 21 Aug 2026 02:29:46 +0000 Subject: [PATCH] fix: close suspended credential access paths Sweep delegated personal access when a membership is suspended and prune every surviving reference before destroying the user's secrets. Co-Authored-By: Paperclip --- server/src/__tests__/access-service.test.ts | 79 ++++++++++++++++++++- server/src/services/access.ts | 64 ++++++++++++++--- 2 files changed, 133 insertions(+), 10 deletions(-) diff --git a/server/src/__tests__/access-service.test.ts b/server/src/__tests__/access-service.test.ts index d579dcebe1f..ab78f7a0b64 100644 --- a/server/src/__tests__/access-service.test.ts +++ b/server/src/__tests__/access-service.test.ts @@ -289,6 +289,9 @@ describeEmbeddedPostgres("access service", () => { key: `oauth-${randomUUID()}`, name: `OAuth ${randomUUID()}`, }).returning().then((rows) => rows[0]!); + await db.update(toolConnections).set({ + credentialSecretRefs: [{ secretId: secret.id, configPath: "oauth.access_token" }], + }).where(eq(toolConnections.id, connection.id)); const grant = await db.insert(connectionGrants).values({ companyId: company.id, connectionId: connection.id, @@ -307,6 +310,7 @@ describeEmbeddedPostgres("access service", () => { connectionId: connection.id, kind: "organization", isDefault: true, + credentialSecretRefs: [{ secretId: secret.id, configPath: "oauth.access_token" }], }).returning().then((rows) => rows[0]!); await db.insert(connectionGrantMembers).values({ companyId: company.id, @@ -323,11 +327,84 @@ describeEmbeddedPostgres("access service", () => { expect(await db.select().from(connectionGrantMembers)).toHaveLength(0); expect(await db.select().from(companySecrets).where(eq(companySecrets.id, secret.id))).toHaveLength(0); expect(await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id))) - .toEqual([expect.objectContaining({ status: "revoked" })]); + .toEqual([expect.objectContaining({ status: "revoked", credentialSecretRefs: [] })]); + expect(await db.select().from(connectionGrants).where(eq(connectionGrants.id, organizationGrant.id))) + .toEqual([expect.objectContaining({ credentialSecretRefs: [] })]); + expect(await db.select().from(toolConnections).where(eq(toolConnections.id, connection.id))) + .toEqual([expect.objectContaining({ credentialSecretRefs: [] })]); expect(await db.select().from(toolAccessAuditEvents).where(eq(toolAccessAuditEvents.reasonCode, "membership_removed"))) .toHaveLength(1); }); + it("revokes delegated personal connection access when membership is suspended", async () => { + const { company } = 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 agent", + role: "worker", + adapterType: "process", + adapterConfig: {}, + }).returning().then((rows) => rows[0]!); + const application = await db.insert(toolApplications).values({ + companyId: company.id, + applicationKey: `app-${randomUUID()}`, + name: "Personal mail", + type: "mcp", + status: "active", + }).returning().then((rows) => rows[0]!); + const connection = await db.insert(toolConnections).values({ + companyId: company.id, + applicationId: application.id, + name: "Mail", + uid: `mail-${randomUUID()}`, + connectionKind: "managed", + ownership: "customer", + transport: "mcp_remote", + authKind: "oauth", + credentialPolicy: "per_user", + }).returning().then((rows) => rows[0]!); + const definition = await db.insert(userSecretDefinitions).values({ + companyId: company.id, + key: `oauth-${randomUUID()}`, + name: "Personal OAuth token", + }).returning().then((rows) => rows[0]!); + const secret = await db.insert(companySecrets).values({ + companyId: company.id, + scope: "user", + ownerUserId: member.principalId, + userSecretDefinitionId: definition.id, + key: `oauth-${randomUUID()}`, + name: `OAuth ${randomUUID()}`, + }).returning().then((rows) => rows[0]!); + const grant = await db.insert(connectionGrants).values({ + companyId: company.id, + connectionId: connection.id, + kind: "user", + subjectUserId: member.principalId, + credentialSecretRefs: [{ secretId: secret.id, configPath: "oauth.access_token" }], + }).returning().then((rows) => rows[0]!); + await db.insert(connectionGrantDelegations).values({ + companyId: company.id, + grantId: grant.id, + agentId: agent.id, + createdByUserId: member.principalId, + }); + + await accessService(db).updateMember(company.id, member.id, { status: "suspended" }); + + expect(await db.select().from(connectionGrantDelegations)).toHaveLength(0); + expect(await db.select().from(companySecrets).where(eq(companySecrets.id, secret.id))).toHaveLength(0); + expect(await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id))) + .toEqual([expect.objectContaining({ status: "revoked", credentialSecretRefs: [] })]); + }); + it("allows owner and admin role-default grants to manage environments", async () => { const { company, owner } = await createCompanyWithOwner(db); const access = accessService(db); diff --git a/server/src/services/access.ts b/server/src/services/access.ts index f58714cf4f1..58e5265ead2 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -10,6 +10,7 @@ import { issues, principalPermissionGrants, toolAccessAuditEvents, + toolConnections, } from "@paperclipai/db"; import type { PermissionKey, PrincipalType } from "@paperclipai/shared"; import { conflict } from "../errors.js"; @@ -49,9 +50,18 @@ export function accessService(db: Db) { eq(connectionGrants.subjectUserId, userId), )); const grantIds = ownedGrants.map((grant) => grant.id); - const secretIds = [...new Set(ownedGrants.flatMap((grant) => + const referencedSecretIds = [...new Set(ownedGrants.flatMap((grant) => grant.credentialSecretRefs.map((ref) => ref.secretId), ))]; + const ownedSecrets = referencedSecretIds.length === 0 ? [] : await tx.select({ + id: companySecrets.id, + }).from(companySecrets).where(and( + eq(companySecrets.companyId, companyId), + eq(companySecrets.scope, "user"), + eq(companySecrets.ownerUserId, userId), + inArray(companySecrets.id, referencedSecretIds), + )); + const secretIds = ownedSecrets.map((secret) => secret.id); const removedDelegations = grantIds.length === 0 ? [] : await tx .delete(connectionGrantDelegations) .where(and( @@ -71,14 +81,41 @@ export function accessService(db: Db) { eq(connectionGrants.companyId, companyId), inArray(connectionGrants.id, grantIds), )); - if (secretIds.length > 0) { - await tx.delete(companySecrets).where(and( - eq(companySecrets.companyId, companyId), - eq(companySecrets.scope, "user"), - eq(companySecrets.ownerUserId, userId), - inArray(companySecrets.id, secretIds), - )); + } + if (secretIds.length > 0) { + const removedSecretIds = new Set(secretIds); + const [grantRefs, connectionRefs] = await Promise.all([ + tx.select({ + id: connectionGrants.id, + credentialSecretRefs: connectionGrants.credentialSecretRefs, + }).from(connectionGrants).where(eq(connectionGrants.companyId, companyId)), + tx.select({ + id: toolConnections.id, + credentialSecretRefs: toolConnections.credentialSecretRefs, + }).from(toolConnections).where(eq(toolConnections.companyId, companyId)), + ]); + for (const grant of grantRefs) { + const credentialSecretRefs = grant.credentialSecretRefs.filter( + (ref) => !removedSecretIds.has(ref.secretId), + ); + if (credentialSecretRefs.length !== grant.credentialSecretRefs.length) { + await tx.update(connectionGrants).set({ credentialSecretRefs, updatedAt: now }) + .where(eq(connectionGrants.id, grant.id)); + } } + for (const connection of connectionRefs) { + const credentialSecretRefs = connection.credentialSecretRefs.filter( + (ref) => !removedSecretIds.has(ref.secretId), + ); + if (credentialSecretRefs.length !== connection.credentialSecretRefs.length) { + await tx.update(toolConnections).set({ credentialSecretRefs, updatedAt: now }) + .where(eq(toolConnections.id, connection.id)); + } + } + await tx.delete(companySecrets).where(and( + eq(companySecrets.companyId, companyId), + inArray(companySecrets.id, secretIds), + )); } await tx.delete(connectionGrantMembers).where(and( eq(connectionGrantMembers.companyId, companyId), @@ -851,12 +888,21 @@ export function accessService(db: Db) { } } + const now = new Date(); + if ( + existing.principalType === "user" && + existing.status !== "suspended" && + nextStatus === "suspended" + ) { + await sweepMemberConnectionAccess(tx, companyId, existing.principalId, now); + } + return tx .update(companyMemberships) .set({ membershipRole: nextMembershipRole, status: nextStatus, - updatedAt: new Date(), + updatedAt: now, }) .where(eq(companyMemberships.id, existing.id)) .returning()