fix(connections): honor identity after reconnect
Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
parent
de8a4e3c21
commit
e1337803f5
|
|
@ -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()}`;
|
||||
|
|
|
|||
|
|
@ -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) {
|
||||
|
|
|
|||
Loading…
Reference in New Issue