diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 87fcca94d7d..fc447a57712 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -240,6 +240,13 @@ async function createAgent(db: ReturnType, companyId: string, s } async function createIssueAndRun(db: ReturnType, companyId: string, agentId: string) { + await db.insert(companyMemberships).values({ + companyId, + principalType: "user", + principalId: "user-for-run", + status: "active", + membershipRole: "member", + }).onConflictDoNothing(); const [issue] = await db.insert(issues).values({ companyId, title: `Broker issue ${randomUUID()}`, @@ -936,6 +943,19 @@ describeEmbeddedPostgres("tool access service", () => { addresseeUserId: "user-for-run", idempotencyKey: `connection-delegation:${connection.id}:user-for-run:${agent.id}`, })]); + + await db.update(companyMemberships).set({ status: "suspended" }).where(and( + eq(companyMemberships.companyId, company.id), + eq(companyMemberships.principalId, "user-for-run"), + )); + const inactiveOwner = await request(app) + .post(`/api/agents/me/connections/${connection.id}/token`) + .send({}); + expect(inactiveOwner.status).toBe(403); + expect(inactiveOwner.body).toMatchObject({ + code: "grant_owner_membership_inactive", + }); + expect(inactiveOwner.body.error).toContain("not an active company member"); }); it("enforces organization grant audiences at token mint time", async () => { @@ -3640,6 +3660,32 @@ describeEmbeddedPostgres("tool access service", () => { expect(unchangedConnection.credentialSecretRefs.map((ref) => ref.secretId).sort()).toEqual(workspaceSecretIds); const [resolved] = await db.select().from(issueThreadInteractions).where(eq(issueThreadInteractions.id, interaction.id)); expect(resolved).toMatchObject({ status: "accepted", result: { version: 1, outcome: "accepted" } }); + + const versionCountBeforeSuspension = (await db.select().from(companySecretVersions).where( + inArray(companySecretVersions.secretId, grant.credentialSecretRefs.map((ref) => ref.secretId)), + )).length; + const retry = await service.startAuthorizationForAgent({ + companyId: company.id, + connectionId: connected.connectionId, + agentId: agent.id, + runId: run.id, + subjectUserId: "user-for-run", + scopes: ["users:read"], + redirectUri: "https://paperclip.example/api/tools/oauth/callback", + }); + await db.update(companyMemberships).set({ status: "suspended" }).where(and( + eq(companyMemberships.companyId, company.id), + eq(companyMemberships.principalId, "user-for-run"), + )); + await expect(service.completeOAuthCallback({ + state: new URL(retry.authorizationUrl).searchParams.get("state")!, + code: "user-authorization-code", + redirectUri: "https://paperclip.example/api/tools/oauth/callback", + actor: { actorType: "user", actorId: "user-for-run" }, + })).rejects.toMatchObject({ status: 403 }); + expect((await db.select().from(companySecretVersions).where( + inArray(companySecretVersions.secretId, grant.credentialSecretRefs.map((ref) => ref.secretId)), + )).length).toBe(versionCountBeforeSuspension); }); it("starts and completes OAuth app sign-in with PKCE state and secret-backed tokens", async () => { diff --git a/server/src/__tests__/tool-gateway.test.ts b/server/src/__tests__/tool-gateway.test.ts index c9fc2e3efba..c23340b216c 100644 --- a/server/src/__tests__/tool-gateway.test.ts +++ b/server/src/__tests__/tool-gateway.test.ts @@ -116,6 +116,16 @@ async function createIssueAndRun(db: Db, companyId: string, agentId: string) { return { project, issue, run }; } +async function createActiveMember(db: Db, companyId: string, userId: string) { + await db.insert(companyMemberships).values({ + companyId, + principalType: "user", + principalId: userId, + status: "active", + membershipRole: "member", + }); +} + async function allowToolsForAgent(db: Db, companyId: string, agentId: string, toolNames: string[]) { const profile = await db .insert(toolProfiles) @@ -1407,6 +1417,7 @@ rl.on("line", (line) => { const company = await createCompany(db); const agent = await createAgent(db, company.id); const { run } = await createIssueAndRun(db, company.id, agent.id); + await createActiveMember(db, company.id, "alice"); await db.update(heartbeatRuns).set({ responsibleUserId: "alice" }).where(eq(heartbeatRuns.id, run.id)); const values = { organization: `organization-${randomUUID()}`, @@ -1826,6 +1837,7 @@ rl.on("line", (line) => { const company = await createCompany(db); const agent = await createAgent(db, company.id); const { issue, run } = await createIssueAndRun(db, company.id, agent.id); + await createActiveMember(db, company.id, "carol"); await db.update(heartbeatRuns).set({ responsibleUserId: "carol" }).where(eq(heartbeatRuns.id, run.id)); const fake = await startFakeRemoteMcpServer((fakeRequest) => ({ body: { @@ -1902,6 +1914,7 @@ rl.on("line", (line) => { const company = await createCompany(db); const agent = await createAgent(db, company.id); const { issue, run } = await createIssueAndRun(db, company.id, agent.id); + await createActiveMember(db, company.id, "alice"); await db.update(heartbeatRuns).set({ responsibleUserId: "alice", invocationSource: "automation", @@ -1953,6 +1966,14 @@ rl.on("line", (line) => { await expect(gateway.executeTool({ sessionToken: session.token, tool: tool.name, parameters: {} })) .resolves.toMatchObject({ status: "completed", result: { content: "delegated" } }); expect(fake.requests).toHaveLength(1); + + await db.update(companyMemberships).set({ status: "suspended" }).where(and( + eq(companyMemberships.companyId, company.id), + eq(companyMemberships.principalId, "alice"), + )); + await expect(gateway.executeTool({ sessionToken: session.token, tool: tool.name, parameters: {} })) + .rejects.toMatchObject({ status: 403, reasonCode: "grant_owner_membership_inactive" }); + expect(fake.requests).toHaveLength(1); } finally { await fake.close(); } diff --git a/server/src/services/access.ts b/server/src/services/access.ts index 23472b3aeba..48709a4167d 100644 --- a/server/src/services/access.ts +++ b/server/src/services/access.ts @@ -315,6 +315,7 @@ export function accessService(db: Db) { .select() .from(companyMemberships) .where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.id, memberId))) + .for("update") .then((rows) => rows[0] ?? null); if (!existing) return null; @@ -467,6 +468,7 @@ export function accessService(db: Db) { .select() .from(companyMemberships) .where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.id, memberId))) + .for("update") .then((rows) => rows[0] ?? null); if (!existing) return null; if (existing.principalType !== "user") { @@ -874,6 +876,7 @@ export function accessService(db: Db) { .select() .from(companyMemberships) .where(and(eq(companyMemberships.companyId, companyId), eq(companyMemberships.id, memberId))) + .for("update") .then((rows) => rows[0] ?? null); if (!existing) return null; diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 694b0a2c3e6..aba0c3881e7 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -10,6 +10,7 @@ import { connectionTokenIssuances, authUsers, companies, + companyMemberships, companySecretBindings, companySecrets, userSecretDefinitions, @@ -7550,17 +7551,111 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} code: input.code, resource: endpoints.resource, }); - const [existingUserGrant] = stateRow.subjectUserId - ? await db.select().from(connectionGrants).where(and( + if (stateRow.subjectUserId) { + return db.transaction(async (tx) => { + // Serialize callback persistence with suspension/removal. Those paths + // lock this same membership row before sweeping personal credentials. + const [membership] = await tx.select({ id: companyMemberships.id }).from(companyMemberships).where(and( + eq(companyMemberships.companyId, connection.companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.principalId, stateRow.subjectUserId!), + eq(companyMemberships.status, "active"), + )).limit(1).for("update"); + if (!membership) { + throw forbidden("Your company membership is no longer active. Ask a company owner to restore access before you authorize this connection again."); + } + + const [existingUserGrant] = await tx.select().from(connectionGrants).where(and( eq(connectionGrants.companyId, connection.companyId), eq(connectionGrants.connectionId, connection.id), eq(connectionGrants.kind, "user"), - eq(connectionGrants.subjectUserId, stateRow.subjectUserId), - )).limit(1) - : [undefined]; - const subjectCredentialSecretRefs = stateRow.subjectUserId - ? existingUserGrant?.credentialSecretRefs ?? [] - : connection.credentialSecretRefs; + eq(connectionGrants.subjectUserId, stateRow.subjectUserId!), + )).limit(1); + const subjectCredentialSecretRefs = existingUserGrant?.credentialSecretRefs ?? []; + const accessRef = await createOrRotateOAuthSecret({ + companyId: connection.companyId, + connection, + configPath: "oauth.access_token", + label: "OAuth access token", + value: token.accessToken, + actor: input.actor, + existingRefs: subjectCredentialSecretRefs, + ownerUserId: stateRow.subjectUserId!, + }); + const nextCredentialSecretRefs = [ + ...subjectCredentialSecretRefs.filter((ref) => ref.configPath !== "oauth.access_token" && ref.configPath !== "oauth.refresh_token"), + accessRef, + ]; + if (token.refreshToken) { + nextCredentialSecretRefs.push(await createOrRotateOAuthSecret({ + companyId: connection.companyId, + connection, + configPath: "oauth.refresh_token", + label: "OAuth refresh token", + value: token.refreshToken, + actor: input.actor, + existingRefs: subjectCredentialSecretRefs, + ownerUserId: stateRow.subjectUserId!, + })); + } else { + const existingRefreshRef = subjectCredentialSecretRefs.find((ref) => ref.configPath === "oauth.refresh_token"); + if (existingRefreshRef) nextCredentialSecretRefs.push(existingRefreshRef); + } + + const grantValues = { + credentialSecretRefs: nextCredentialSecretRefs, + status: "active" as const, + revokedAt: null, + revokedByAgentId: null, + revokedByUserId: null, + updatedAt: new Date(), + }; + if (existingUserGrant) { + await tx.update(connectionGrants).set(grantValues).where(eq(connectionGrants.id, existingUserGrant.id)); + } else { + await tx.insert(connectionGrants).values({ + companyId: connection.companyId, + connectionId: connection.id, + kind: "user", + subjectUserId: stateRow.subjectUserId!, + ...grantValues, + isDefault: false, + createdByUserId: stateRow.subjectUserId!, + }); + } + if (stateRow.interactionId) { + await tx.update(issueThreadInteractions).set({ + status: "accepted", + result: { version: 1, outcome: "accepted" }, + resolvedByUserId: stateRow.subjectUserId!, + resolvedAt: new Date(), + updatedAt: new Date(), + }).where(and( + eq(issueThreadInteractions.id, stateRow.interactionId), + eq(issueThreadInteractions.companyId, connection.companyId), + )); + } + const [application] = await tx.select().from(toolApplications).where(eq(toolApplications.id, connection.applicationId)); + if (!application) throw new Error("OAuth connection application was not found"); + const catalog = (await tx.select().from(toolCatalogEntries).where(and( + eq(toolCatalogEntries.companyId, connection.companyId), + eq(toolCatalogEntries.connectionId, connection.id), + ))).map(toCatalogEntry); + return { + connectionId: connection.id, + application: toApplication(application), + connection: toConnection(connection), + catalog, + actions: groupedActions(catalog), + suggestedDefaults: galleryEntry + ? recommendedDefaultsForApp(galleryEntry, connectionMethodForConnection(galleryEntry, connection).key) + : { access: "all_agents", askFirstRiskLevels: ["write", "destructive"] }, + auth: null, + }; + }); + } + + const subjectCredentialSecretRefs = connection.credentialSecretRefs; const accessRef = await createOrRotateOAuthSecret({ companyId: connection.companyId, connection, @@ -7568,8 +7663,6 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} label: "OAuth access token", value: token.accessToken, actor: input.actor, - existingRefs: stateRow.subjectUserId ? subjectCredentialSecretRefs : undefined, - ownerUserId: stateRow.subjectUserId ?? undefined, }); const nextCredentialSecretRefs = [ ...subjectCredentialSecretRefs.filter((ref) => ref.configPath !== "oauth.access_token" && ref.configPath !== "oauth.refresh_token"), @@ -7583,66 +7676,12 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} label: "OAuth refresh token", value: token.refreshToken, actor: input.actor, - existingRefs: stateRow.subjectUserId ? subjectCredentialSecretRefs : undefined, - ownerUserId: stateRow.subjectUserId ?? undefined, })); } else { const existingRefreshRef = subjectCredentialSecretRefs.find((ref) => ref.configPath === "oauth.refresh_token"); if (existingRefreshRef) nextCredentialSecretRefs.push(existingRefreshRef); } const expiresAt = token.expiresIn ? new Date(Date.now() + token.expiresIn * 1000).toISOString() : null; - if (stateRow.subjectUserId) { - const grantValues = { - credentialSecretRefs: nextCredentialSecretRefs, - status: "active" as const, - revokedAt: null, - revokedByAgentId: null, - revokedByUserId: null, - updatedAt: new Date(), - }; - if (existingUserGrant) { - await db.update(connectionGrants).set(grantValues).where(eq(connectionGrants.id, existingUserGrant.id)); - } else { - await db.insert(connectionGrants).values({ - companyId: connection.companyId, - connectionId: connection.id, - kind: "user", - subjectUserId: stateRow.subjectUserId, - ...grantValues, - isDefault: false, - createdByUserId: stateRow.subjectUserId, - }); - } - if (stateRow.interactionId) { - await db.update(issueThreadInteractions).set({ - status: "accepted", - result: { version: 1, outcome: "accepted" }, - resolvedByUserId: stateRow.subjectUserId, - resolvedAt: new Date(), - updatedAt: new Date(), - }).where(and( - eq(issueThreadInteractions.id, stateRow.interactionId), - eq(issueThreadInteractions.companyId, connection.companyId), - )); - } - const [application] = await db.select().from(toolApplications).where(eq(toolApplications.id, connection.applicationId)); - if (!application) throw new Error("OAuth connection application was not found"); - const catalog = (await db.select().from(toolCatalogEntries).where(and( - eq(toolCatalogEntries.companyId, connection.companyId), - eq(toolCatalogEntries.connectionId, connection.id), - ))).map(toCatalogEntry); - return { - connectionId: connection.id, - application: toApplication(application), - connection: toConnection(connection), - catalog, - actions: groupedActions(catalog), - suggestedDefaults: galleryEntry - ? recommendedDefaultsForApp(galleryEntry, connectionMethodForConnection(galleryEntry, connection).key) - : { access: "all_agents", askFirstRiskLevels: ["write", "destructive"] }, - auth: null, - }; - } const nextConfig = { ...connection.config, oauth: { @@ -9306,6 +9345,19 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} : { type: "user" as const, userId: actingUserId }; let grant: typeof connectionGrants.$inferSelect | undefined; if (connection.credentialPolicy !== "shared" && actingUserId) { + const [membership] = await db.select({ id: companyMemberships.id }).from(companyMemberships).where(and( + eq(companyMemberships.companyId, connection.companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.principalId, actingUserId), + eq(companyMemberships.status, "active"), + )).limit(1); + if (!membership) { + await fail(403, "The personal grant owner is not an active company member", "denied", "grant_owner_membership_inactive", { + connection: { id: connection.id, uid: connection.uid, name: connection.name }, + subject: { type: "user", userId: actingUserId }, + remediation: { action: "restore_membership_or_reconnect" }, + }); + } [grant] = await db.select().from(connectionGrants).where(and( eq(connectionGrants.companyId, connection.companyId), eq(connectionGrants.connectionId, connection.id), diff --git a/server/src/services/tool-gateway.ts b/server/src/services/tool-gateway.ts index f4386594d39..5eef1239762 100644 --- a/server/src/services/tool-gateway.ts +++ b/server/src/services/tool-gateway.ts @@ -6,6 +6,7 @@ import { agents, approvals, companies, + companyMemberships, connectionGrantMembers, connectionGrantDelegations, connectionGrants, @@ -2721,6 +2722,19 @@ export function createToolGatewayService( const autonomous = run?.invocationSource === "automation" || run?.invocationSource === "timer"; const findUserGrant = async () => { if (!actingUserId) return undefined; + const [membership] = await db.select({ id: companyMemberships.id }).from(companyMemberships).where(and( + eq(companyMemberships.companyId, connection.companyId), + eq(companyMemberships.principalType, "user"), + eq(companyMemberships.principalId, actingUserId), + eq(companyMemberships.status, "active"), + )).limit(1); + if (!membership) { + throw new ToolGatewayHttpError(403, "The personal grant owner is not an active company member", "grant_owner_membership_inactive", { + connectionId: connection.id, + actingUserId, + remediation: { action: "restore_membership_or_reconnect" }, + }); + } const [grant] = await db.select().from(connectionGrants).where(and( eq(connectionGrants.companyId, connection.companyId), eq(connectionGrants.connectionId, connection.id),