From e1337803f5fbb4a57a3db97f8ed8b0d63b71e02b Mon Sep 17 00:00:00 2001 From: Dotta Date: Sat, 5 Sep 2026 10:15:09 -0500 Subject: [PATCH] fix(connections): honor identity after reconnect Co-Authored-By: Paperclip --- .../src/__tests__/tool-access-service.test.ts | 107 ++++++++++++++++++ server/src/services/tool-access.ts | 42 ++++--- 2 files changed, 136 insertions(+), 13 deletions(-) diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index 5963fc4105..03e9872b29 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -5231,6 +5231,113 @@ describeEmbeddedPostgres("tool access service", () => { } }); + it("replaces an archived dedicated GitHub identity with an explicitly selected personal identity", async () => { + const company = await createCompany(db); + const userId = `github-personal-revival-${randomUUID()}`; + await grantBoardUser(db, company.id, userId, [], "owner"); + const agent = await createAgent(db, company.id); + const connector = fakeGitHubConnector(company.id, `agent:${agent.id}`); + const originalClaim = connector.claim; + connector.claim = vi.fn(async (input) => ({ + ...await originalClaim(input), + subject: input.subject, + })); + const service = createTestToolAccessService(db, { paperclipCloudConnector: connector }); + const actor = { actorType: "user" as const, actorId: userId }; + const githubDefinition = getConnectableAppDefinition("github")!; + const previousOwnershipAvailability = githubDefinition.ownershipAvailability; + githubDefinition.ownershipAvailability = { ...previousOwnershipAvailability, platform_shared: true }; + vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const href = String(url); + if (href === "https://api.github.com/user") { + return mcpHttpResponse({ id: 42, login: "octocat" }); + } + if (href.includes("https://api.github.com/user/installations?")) { + return mcpHttpResponse({ installations: [{ + id: 101, + repository_selection: "selected", + html_url: "https://github.com/settings/installations/101", + account: { login: "paperclipai" }, + }] }); + } + if (href.includes("https://api.github.com/user/installations/101/repositories?")) { + return mcpHttpResponse({ total_count: 1, repositories: [] }); + } + if (href === GITHUB_CONNECTOR_PROFILES["github.code"].serverUrl) { + return mcpHttpResponse({ + jsonrpc: "2.0", + id: "paperclip-catalog-refresh", + result: { tools: [{ name: "get_pull_request", annotations: { readOnlyHint: true } }] }, + }); + } + throw new Error(`unexpected fetch ${href}`); + }); + + try { + const dedicated = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "managed", + grantKind: "agent", + subjectAgentId: agent.id, + name: "GitHub", + }, actor); + const dedicatedStart = await service.startOAuth(company.id, dedicated.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + subjectAgentId: agent.id, + }); + await service.completePaperclipCloudConnectorCallback({ + state: new URL(dedicatedStart.authorizationUrl).searchParams.get("state")!, + claimId: "github-dedicated-before-removal", + actor, + }); + await service.archiveConnection(dedicated.connectionId, company.id, actor); + + const personal = await service.connectGalleryApp(company.id, { + galleryKey: "github", + connectionMethodKey: "managed", + grantKind: "user", + name: "GitHub", + }, actor); + expect(personal.connectionId).toBe(dedicated.connectionId); + expect(personal.connection).toMatchObject({ + status: "draft", + credentialPolicy: "per_user", + }); + + const personalStart = await service.startOAuth(company.id, personal.connectionId, { + redirectUri: "https://paperclip.example/api/tools/oauth/cloud-connector/callback", + actor, + }); + const completed = await service.completePaperclipCloudConnectorCallback({ + state: new URL(personalStart.authorizationUrl).searchParams.get("state")!, + claimId: "github-personal-after-removal", + actor, + }); + expect(completed.connection).toMatchObject({ + status: "active", + credentialPolicy: "per_user", + }); + + const grants = await service.listConnectionGrants(personal.connectionId, company.id); + expect(grants.grants).toEqual(expect.arrayContaining([ + expect.objectContaining({ + kind: "agent", + subjectAgentId: agent.id, + status: "revoked", + }), + expect.objectContaining({ + kind: "user", + subjectUserId: userId, + status: "active", + }), + ])); + expect(grants.grants.some((grant) => grant.kind === "organization")).toBe(false); + } finally { + githubDefinition.ownershipAvailability = previousOwnershipAvailability; + } + }); + it("routes a managed Drive callback into the personal vault, filtered catalog, and provider-specific activity", async () => { const company = await createCompany(db); const userId = `drive-member-${randomUUID()}`; diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 6d55ae9357..3de522105b 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -8830,13 +8830,30 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} )); name = nextAvailableConnectionName(requestedName, connectionNames.map((row) => row.name)); } - const retainedGrantKind: ConnectionGrantKind | null = retainedConnection + const previousGrantKind: ConnectionGrantKind | null = retainedConnection ? retainedConnection.credentialPolicy === "per_user" ? "user" : retainedConnection.credentialPolicy === "per_agent" ? "agent" : "organization" : null; + // An explicit resume/application reconnect continues the retained identity. + // A fresh gallery connect may still reuse an archived row for stable history + // and company-unique naming, but an explicit Access choice is a new identity + // decision and must replace the archived policy. Without this distinction, + // removing a dedicated-agent connection and reconnecting the default personal + // account leaves `per_agent` behind and the OAuth callback cannot persist its + // user grant. + const retainsIdentity = Boolean( + retainedConnection + && ( + retainedConnection.status === "draft" + || requestedResumeConnection + || input.applicationId + || input.grantKind === undefined + ) + ); + const retainedGrantKind = retainsIdentity ? previousGrantKind : null; // The route can authorize an explicit resume before entering the service, // but name/source recovery happens here. Do not let a caller submit a // personal grant choice to pass the route and then inherit an implicitly @@ -8844,8 +8861,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // unrestricted instance operator; every authenticated user must still hold // current connection-manager authority before this retained row is touched. if ( - retainedGrantKind === "organization" - && input.grantKind === "user" + previousGrantKind + && input.grantKind + && previousGrantKind !== input.grantKind && actor?.actorType === "user" && actor.actorSource !== "local_implicit" ) { @@ -8868,7 +8886,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} eq(principalPermissionGrants.permissionKey, "tools:manage_connections"), )).limit(1); if (!roleCanManage && !explicitManagerGrant) { - throw forbidden("Only a company owner, admin, or member with connection-manager permission can share credentials with the organization."); + throw forbidden("Only a company owner, admin, or member with connection-manager permission can change this connection's credential identity."); } } const requestedGrantKind = retainedGrantKind ?? input.grantKind ?? "organization"; @@ -9069,6 +9087,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} // or API key, without carrying credentials across a method/provider change. const canRetainCredentialMaterial = Boolean( retainedConnection + && previousGrantKind === requestedGrantKind && galleryEntry && retainedSource === galleryEntry.slug && retainedMethodKey === method?.key, @@ -9076,13 +9095,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const retainedCredentialSecretRefs = canRetainCredentialMaterial ? (retainedPersonalIdentity?.grant?.credentialSecretRefs ?? retainedConnection?.credentialSecretRefs ?? []) : []; - // Only the personal path changes the policy; every existing gallery app keeps - // the shared default it has today. - const credentialPolicy: ToolConnectionCredentialPolicy | undefined = personalIdentityUserId + const credentialPolicy: ToolConnectionCredentialPolicy = requestedGrantKind === "user" ? "per_user" - : dedicatedAgentId + : requestedGrantKind === "agent" ? "per_agent" - : undefined; + : "shared"; const connectionOwnership = isPaperclipCloudConnectorStrategy(method?.oauthStrategy) ? "platform_shared" : "customer"; let applicationRow: typeof toolApplications.$inferSelect | null = null; let connectionRow: typeof toolConnections.$inferSelect | null = null; @@ -9273,9 +9290,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} credentialSecretRefs: connectionCredentialSecretRefs, credentialSource, externalCredential, - // Identity is immutable for a retained connection. A fresh - // connection still derives it from the explicit Access choice. - credentialPolicy: revivedConnectionPrevious.credentialPolicy, + credentialPolicy, updatedAt: new Date(), }).where(eq(toolConnections.id, revivedConnectionPrevious.id)).returning(); } else { @@ -9298,7 +9313,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} transportConfig: config, credentialRefs, credentialSecretRefs: connectionCredentialSecretRefs, - ...(credentialPolicy ? { credentialPolicy } : {}), + credentialPolicy, createdByAgentId: actor?.actorType === "agent" ? actor.actorId ?? null : null, createdByUserId: actor?.actorType === "user" ? actor.actorId ?? null : null, }).returning(); @@ -9484,6 +9499,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} credentialSecretRefs: revivedConnectionPrevious.credentialSecretRefs, credentialSource: revivedConnectionPrevious.credentialSource, externalCredential: revivedConnectionPrevious.externalCredential, + credentialPolicy: revivedConnectionPrevious.credentialPolicy, updatedAt: new Date(), }).where(eq(toolConnections.id, revivedConnectionPrevious.id)).catch(() => undefined); } else if (connectionRow) {