mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-27 02:44:50 +01:00
fix: serialize OAuth completion with member removal
Paperclip-Task: PAP-17797
This commit is contained in:
1 parent
3ae042330e
commit
34ae70aa73
5 files changed
+200
-64
No files matched your search
@@ -240,6 +240,13 @@ async function createAgent(db: ReturnType<typeof createDb>, companyId: string, s
|
||||
}
|
||||
|
||||
async function createIssueAndRun(db: ReturnType<typeof createDb>, 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 () => {
|
||||
|
||||
@@ -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();
|
||||
}
|
||||
|
||||
@@ -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;
|
||||
|
||||
|
||||
@@ -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),
|
||||
|
||||
@@ -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),
|
||||
|
||||
Reference in new issue
Block a user