mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-19 10:36:26 +02:00
fix(connections): unblock personal MCP auth discovery (#13497)
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Connections let people give agents access to external tools. > - A personal connection needs the current user's authorization. > - A new MCP URL must be probed before Paperclip can discover its sign-in method. > - Requiring a personal grant before that probe prevents sign-in from starting. > - This pull request permits the initial probe for a creator-owned draft with no credentials. > - People can complete personal setup while later requests retain authorization checks. ## Linked Issues or Issue Description **What happened?** Connecting an unknown MCP URL with "Just me" failed with HTTP 502 and "This connection needs the current user's authorization". Paperclip checked for a personal grant before contacting the provider. The health wrapper also changed the expected authorization error into a server error. **Expected behavior** Discover OAuth and start browser sign-in. Create a personal grant after consent. For a public endpoint, discover its tools and create the empty personal grant after a successful probe. Keep missing authorization on later health checks as HTTP 422. **Steps to reproduce** 1. Add an unknown remote MCP URL with no saved credentials. 2. Select "Just me". 3. Check the link. Before this fix, the request fails before sign-in or tool discovery. **Paperclip version or commit** The three original regressions fail against `6cfe4acff` with the service fix removed and pass with it restored. **Deployment mode** The defect was reported in production and reproduced in local server tests with isolated PostgreSQL. Related work: Refs #11831. Refs #11144. Searches found no duplicate fix. ## What Changed - Allow an initial credential-free probe only for the creating user's personal draft with unknown authentication and no supplied credentials. - Leave OAuth grant creation to the callback. Create an empty personal grant only after a public probe succeeds. - Preserve the personal grant for URLs that already contain a credential. - Make empty personal grant creation conflict-safe without overwriting a concurrent grant or duplicating its creation audit. - Commit the empty grant and audit atomically. Retain the established public draft identity after a later catalog failure, so a failed retry cannot remove a successful retry's grant. - Run the following catalog/default-profile step in its own transaction, so failures discard partial catalog, profile, binding, and audit changes without deleting the established identity. - Verify archived personal connections retain their owner: another user is rejected before probing, while the original owner can resume setup. - Preserve `user_authorization_required` and HTTP 422 in health failures. - Cover the connect and OAuth callback routes, real loopback HTTP, credential-bearing URLs, and later health checks. - Document personal setup and the test fixtures. ## Verification - Red/green: the original three tests failed with the exact reported error before the fix and passed after it. - The credential-bearing personal URL regression also failed before its guard was added. - Focused suite: `pnpm --filter @paperclipai/server exec vitest run src/__tests__/generic-mcp-connection.test.ts src/__tests__/tool-access-service.test.ts` passed all 385 tests across the two suites. An earlier run had a socket hang-up in an existing agent-permissions test; the unchanged suite passed on rerun. - The concurrent rollback regression failed before its fix because the successful retry's grant was deleted. It now verifies the grant and draft survive and a later normal health check succeeds. - Database fault injection during profile-entry insertion reproduced partial catalog writes before the transaction fix. The regression now verifies unchanged catalog rows, no partial profile/bindings, a retained grant, and successful retry. - CI's first serialized-server shard 3 attempt failed an existing peer-agent mutation test (the real run-context guard ran despite the test's mock). The test passed in isolation and all 108 tests in that suite passed unchanged locally. The single failed-shard rerun passed without code changes. - Final-commit CI: all 32 applicable checks passed on `639f037987352cab6084c4ebfa5dbf7b0aed6046`, including all 385 affected tests, the full test matrix, browser suite, build, typecheck, release checks, and security checks. The two Storybook-only checks were not applicable and skipped. Greptile is 5/5 with all review threads resolved. [Successful CI run](https://github.com/paperclipai/paperclip/actions/runs/35027478353). - `pnpm -r typecheck` passed. - `pnpm smoke:mcp-fixtures -- --require-paperclip` passed. - `pnpm build` passed. - Full local `pnpm test:run` was attempted: its general-server group finished with 12,372 passed, 4 failed, and 70 skipped tests. The run started before review edits; its two MCP failures used the old cached service (including an insert without the new conflict clause). All 385 focused tests pass on the final code. The other failures were existing workspace-cleanup and runtime-port tests; their unchanged suites passed on rerun (66 passed, and 25 passed/3 skipped). The local command stopped before later groups. The final-commit CI matrix is the full-suite merge gate; this local run is not claimed as green. ## Risks - The initial probe must not become a general authorization bypass. It is restricted to the creating user's draft. Normal health checks retain authorization enforcement. - Public endpoints get a personal grant with no secrets only after they answer successfully. Credential-bearing URLs keep their existing grant. - The concurrent-probe regression seeds the catalog and default profile to isolate grant creation. Existing first-time catalog/profile creation races are outside this change; this does not claim to make the entire setup flow concurrency-safe. - No database migration or UI change is required. OAuth tests use a simulated provider; the public endpoint test uses real loopback HTTP. ## Model Used OpenAI Codex, GPT-6-based assistant for regression tests and PR preparation; a GPT-5-based Codex assistant assisted with the initial implementation. Exact runtime model IDs and context-window sizes are not exposed in this session. Both used reasoning, repository tools, and code execution. ## Checklist - [x] I have included a thinking path that traces from project context to this change - [x] I have specified the model used (with version and capability details) - [x] I have checked ROADMAP.md and confirmed this PR does not duplicate planned core work - [x] I have searched GitHub for duplicate or related PRs and linked them above - [x] I have either (a) linked existing issues with `Fixes: #` / `Closes #` / `Refs #` OR (b) described the issue in-PR following the relevant issue template - [x] I have not referenced internal/instance-local Paperclip issues or links (only public GitHub `#NNN` / `github.com/paperclipai/paperclip` URLs) - [x] My branch name describes the change (e.g. `docs/...`, `fix/...`) and contains no internal Paperclip ticket id or instance-derived details - [x] I have run tests locally and they pass - [x] I have added or updated tests where applicable - [x] I have updated relevant documentation to reflect my changes - [x] I have considered and documented any risks above - [x] All Paperclip CI gates are green - [x] Greptile is 5/5 with no open P2s, recommendations, or follow-ups - [x] I will address all Greptile and reviewer comments before requesting merge --------- Co-authored-by: Paperclip <noreply@paperclip.ing>
This commit is contained in:
1 parent
544c3476a8
commit
9adeabb590
4 files changed
+433
-11
No files matched your search
@@ -50,6 +50,18 @@ step through review, access and install. Reads are enabled for review;
|
||||
state-changing actions start off; newly discovered actions are quarantined until
|
||||
reviewed. That is the same treatment a curated connection gets.
|
||||
|
||||
For **Just me**, the first probe of a new URL with no supplied credentials runs
|
||||
before a personal authorization exists. If the server requires OAuth, Paperclip
|
||||
creates the personal grant only after sign-in succeeds. If the server is public,
|
||||
Paperclip creates a personal grant with no credentials after the probe succeeds.
|
||||
That successful public probe saves the draft identity and its grant. A later
|
||||
catalog-refresh failure leaves them available for retry instead of undoing a
|
||||
grant that another setup attempt may already be using.
|
||||
Catalog and default-profile writes after that probe are atomic: a failure rolls
|
||||
back that step while retaining the established draft identity.
|
||||
Later health checks still require the user's authorization and return an
|
||||
actionable `422` error when it is missing.
|
||||
|
||||
### Advanced authentication
|
||||
|
||||
Collapsed by default. Open it when the server's docs are specific:
|
||||
@@ -177,9 +189,10 @@ through this page with either a key or browser sign-in.
|
||||
## Verifying
|
||||
|
||||
Deterministic coverage lives in
|
||||
`server/src/__tests__/generic-mcp-connection.test.ts`, which stands up an
|
||||
in-process MCP server plus authorization server. It needs no network and no
|
||||
vendor credentials, and every case connects by URL without naming a gallery app.
|
||||
`server/src/__tests__/generic-mcp-connection.test.ts` uses a simulated MCP/OAuth
|
||||
provider and a real loopback HTTP server for the personal public-URL regression.
|
||||
It needs no vendor credentials. Tests create an isolated PostgreSQL database
|
||||
and close their servers after use.
|
||||
A credentialed vendor smoke (for example live PostHog OAuth) may be recorded by
|
||||
QA but is not required for deterministic verification.
|
||||
|
||||
|
||||
@@ -1,4 +1,6 @@
|
||||
import { randomUUID } from "node:crypto";
|
||||
import { once } from "node:events";
|
||||
import { createServer } from "node:http";
|
||||
import { Writable } from "node:stream";
|
||||
import express from "express";
|
||||
import pino from "pino";
|
||||
@@ -409,7 +411,7 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
expect(profiles).toEqual([]);
|
||||
});
|
||||
|
||||
it("vaults a credential-bearing MCP URL and never returns or logs its token", async () => {
|
||||
it.each(["organization", "user"] as const)("vaults a credential-bearing MCP URL for %s and never returns or logs its token", async (grantKind) => {
|
||||
const secretUrl = `${MCP_URL}?token=zapier-secret®ion=us`;
|
||||
const publicUrl = `${MCP_URL}?region=us`;
|
||||
const requests: string[] = [];
|
||||
@@ -432,7 +434,7 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
|
||||
const response = await request(app)
|
||||
.post(`/api/companies/${company.id}/tools/apps/connect`)
|
||||
.send({ link: secretUrl, name: "Token URL fixture" })
|
||||
.send({ link: secretUrl, name: "Token URL fixture", grantKind })
|
||||
.expect(201);
|
||||
|
||||
expect(requests).toContain(secretUrl);
|
||||
@@ -448,8 +450,18 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
response.body.connectionId,
|
||||
));
|
||||
expect(connection!.config.url).toBe(publicUrl);
|
||||
expect(connection!.credentialSecretRefs).toEqual([
|
||||
expect.objectContaining({ configPath: "remote.url", label: "MCP server URL" }),
|
||||
const expectedRefs = [expect.objectContaining({ configPath: "remote.url", label: "MCP server URL" })];
|
||||
expect(connection!.credentialSecretRefs).toEqual(grantKind === "user" ? [] : expectedRefs);
|
||||
const grants = await db.select().from(connectionGrants).where(eq(
|
||||
connectionGrants.connectionId,
|
||||
response.body.connectionId,
|
||||
));
|
||||
expect(grants).toEqual([
|
||||
expect.objectContaining({
|
||||
kind: grantKind,
|
||||
credentialSecretRefs: expectedRefs,
|
||||
...(grantKind === "user" ? { subjectUserId: "board-user" } : {}),
|
||||
}),
|
||||
]);
|
||||
expect(await db.select().from(companySecrets)).toHaveLength(1);
|
||||
expect(JSON.stringify(await db.select().from(activityLog))).not.toContain("zapier-secret");
|
||||
@@ -911,6 +923,281 @@ describeEmbeddedPostgres("generic remote MCP connections", () => {
|
||||
.toEqual(["oauth.access_token", "oauth.refresh_token"]);
|
||||
});
|
||||
|
||||
it("discovers OAuth for a personal URL connection before its user grant exists", async () => {
|
||||
const fixture = installMcpOAuthFixture({ auth: "oauth" });
|
||||
const company = await createCompany(db);
|
||||
const app = createRouteApp(db);
|
||||
vi.stubEnv("PAPERCLIP_PUBLIC_URL", PUBLIC_BASE_URL);
|
||||
const actor = { actorType: "user" as const, actorId: "board-user" };
|
||||
|
||||
const response = await request(app)
|
||||
.post(`/api/companies/${company.id}/tools/apps/connect`)
|
||||
.send({
|
||||
link: MCP_URL,
|
||||
name: "Fixture personal OAuth",
|
||||
grantKind: "user",
|
||||
});
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(201);
|
||||
const connected = response.body;
|
||||
|
||||
expect(connected.auth).toMatchObject({
|
||||
kind: "oauth",
|
||||
issuer: ISSUER,
|
||||
resource: MCP_URL,
|
||||
startUrl: expect.any(String),
|
||||
});
|
||||
expect(fixture.requestsTo("/mcp")[0]!.headers.authorization).toBeUndefined();
|
||||
await expect(
|
||||
db
|
||||
.select()
|
||||
.from(connectionGrants)
|
||||
.where(eq(connectionGrants.connectionId, connected.connectionId)),
|
||||
).resolves.toHaveLength(0);
|
||||
|
||||
const authorizationUrl = new URL(connected.auth.startUrl);
|
||||
const code = fixture.issueAuthorizationCode(connected.auth.startUrl);
|
||||
await request(app)
|
||||
.get("/api/tools/oauth/callback")
|
||||
.set("Accept", "text/html")
|
||||
.query({ state: authorizationUrl.searchParams.get("state")!, code, iss: ISSUER })
|
||||
.expect(303);
|
||||
|
||||
const grants = await db
|
||||
.select()
|
||||
.from(connectionGrants)
|
||||
.where(eq(connectionGrants.connectionId, connected.connectionId));
|
||||
expect(grants).toEqual([
|
||||
expect.objectContaining({
|
||||
kind: "user",
|
||||
subjectUserId: actor.actorId,
|
||||
status: "active",
|
||||
}),
|
||||
]);
|
||||
});
|
||||
|
||||
it("does not let another user take over an archived personal URL connection", async () => {
|
||||
const fixture = installMcpOAuthFixture({ auth: "oauth" });
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db);
|
||||
const input = { link: MCP_URL, name: "Archived personal URL", grantKind: "user" as const };
|
||||
const first = await service.connectGalleryApp(company.id, input, { actorType: "user", actorId: "board-user" });
|
||||
await db.update(toolConnections).set({ status: "archived" }).where(eq(toolConnections.id, first.connectionId));
|
||||
await db.update(toolApplications).set({ status: "archived" }).where(eq(toolApplications.id, first.application.id));
|
||||
await db.insert(companyMemberships).values({
|
||||
companyId: company.id, principalType: "user", principalId: "other-user", status: "active", membershipRole: "admin",
|
||||
});
|
||||
fixture.fetchMock.mockClear();
|
||||
|
||||
await expect(service.connectGalleryApp(company.id, input, {
|
||||
actorType: "user", actorId: "other-user",
|
||||
})).rejects.toMatchObject({ status: 403, message: "Only the existing personal identity can reconnect this connection" });
|
||||
|
||||
expect(fixture.fetchMock).not.toHaveBeenCalled();
|
||||
await expect(db.select().from(connectionGrants)).resolves.toHaveLength(0);
|
||||
await expect(service.getConnection(first.connectionId)).resolves.toMatchObject({
|
||||
status: "archived", createdByUserId: "board-user",
|
||||
});
|
||||
const resumed = await service.connectGalleryApp(company.id, input, { actorType: "user", actorId: "board-user" });
|
||||
expect(resumed.connectionId).toBe(first.connectionId);
|
||||
expect(resumed.auth).toMatchObject({ kind: "oauth" });
|
||||
});
|
||||
|
||||
it("creates one personal grant when two public URL setup retries probe concurrently", async () => {
|
||||
const fixture = installMcpOAuthFixture({ auth: "public" });
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db);
|
||||
const actor = { actorType: "user" as const, actorId: "board-user" };
|
||||
const first = await service.connectGalleryApp(company.id, {
|
||||
link: MCP_URL, name: "Concurrent personal URL", grantKind: "user",
|
||||
}, actor);
|
||||
await service.connectGalleryApp(company.id, {
|
||||
link: MCP_URL, grantKind: "user", resumeConnectionId: first.connectionId,
|
||||
}, actor);
|
||||
// Model an interrupted draft with catalog/defaults but no personal grant.
|
||||
// This isolates the grant race from first-time catalog/profile insertion.
|
||||
await db.delete(connectionGrants).where(eq(connectionGrants.connectionId, first.connectionId));
|
||||
await db.delete(toolAccessAuditEvents).where(eq(toolAccessAuditEvents.connectionId, first.connectionId));
|
||||
fixture.fetchMock.mockRestore();
|
||||
let probes = 0;
|
||||
let release!: () => void;
|
||||
const bothProbed = new Promise<void>((resolve) => { release = resolve; });
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
|
||||
probes += 1;
|
||||
if (probes === 2) release();
|
||||
await bothProbed;
|
||||
return jsonResponse({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", result: { tools: FIXTURE_TOOLS } });
|
||||
});
|
||||
|
||||
const results = await Promise.allSettled([0, 1].map(() => service.connectGalleryApp(company.id, {
|
||||
link: MCP_URL, grantKind: "user", resumeConnectionId: first.connectionId,
|
||||
}, actor)));
|
||||
|
||||
for (const result of results) {
|
||||
if (result.status === "rejected") throw result.reason;
|
||||
}
|
||||
expect(results.map((result) => result.status)).toEqual(["fulfilled", "fulfilled"]);
|
||||
await expect(db.select().from(connectionGrants).where(eq(connectionGrants.connectionId, first.connectionId)))
|
||||
.resolves.toEqual([expect.objectContaining({ kind: "user", subjectUserId: actor.actorId, status: "active", credentialSecretRefs: [] })]);
|
||||
await expect(db.select().from(toolAccessAuditEvents).where(and(
|
||||
eq(toolAccessAuditEvents.connectionId, first.connectionId),
|
||||
eq(toolAccessAuditEvents.action, "connection_grant.created"),
|
||||
))).resolves.toHaveLength(1);
|
||||
});
|
||||
|
||||
it("keeps a successful personal setup when the grant-creating retry later fails", async () => {
|
||||
const fixture = installMcpOAuthFixture({ auth: "public" });
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db);
|
||||
const actor = { actorType: "user" as const, actorId: "board-user" };
|
||||
const first = await service.connectGalleryApp(company.id, {
|
||||
link: MCP_URL, name: "Personal retry rollback", grantKind: "user",
|
||||
}, actor);
|
||||
const retryInput = { link: MCP_URL, grantKind: "user" as const, resumeConnectionId: first.connectionId };
|
||||
await service.connectGalleryApp(company.id, retryInput, actor);
|
||||
await db.delete(connectionGrants).where(eq(connectionGrants.connectionId, first.connectionId));
|
||||
await db.update(toolConnections).set({ status: "archived" }).where(eq(toolConnections.id, first.connectionId));
|
||||
await db.update(toolApplications).set({ status: "archived", archivedAt: new Date() }).where(eq(toolApplications.id, first.application.id));
|
||||
fixture.fetchMock.mockRestore();
|
||||
let calls = 0;
|
||||
let catalogStarted!: () => void;
|
||||
const catalogPending = new Promise<void>((resolve) => { catalogStarted = resolve; });
|
||||
let failCatalog!: () => void;
|
||||
const releaseCatalog = new Promise<void>((resolve) => { failCatalog = resolve; });
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(async () => {
|
||||
calls += 1;
|
||||
if (calls === 2) {
|
||||
// The first retry has created its grant but has not finished setup.
|
||||
catalogStarted();
|
||||
await releaseCatalog;
|
||||
throw new Error("first retry catalog unavailable");
|
||||
}
|
||||
return jsonResponse({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", result: { tools: FIXTURE_TOOLS } });
|
||||
});
|
||||
const failure = service.connectGalleryApp(company.id, {
|
||||
link: MCP_URL, name: "Personal retry rollback", grantKind: "user",
|
||||
}, actor).then(() => null, (error: unknown) => error);
|
||||
await Promise.race([catalogPending, failure.then((error) => { throw error ?? new Error("Retry finished before the catalog probe"); })]);
|
||||
try {
|
||||
const successfulRetry = await service.connectGalleryApp(company.id, retryInput, actor);
|
||||
expect(successfulRetry.connectionId).toBe(first.connectionId);
|
||||
} finally {
|
||||
failCatalog();
|
||||
}
|
||||
expect(await failure).toMatchObject({ status: 502 });
|
||||
await expect(db.select().from(connectionGrants).where(eq(connectionGrants.connectionId, first.connectionId)))
|
||||
.resolves.toEqual([expect.objectContaining({ kind: "user", subjectUserId: actor.actorId, status: "active", credentialSecretRefs: [] })]);
|
||||
await expect(service.getConnection(first.connectionId)).resolves.toMatchObject({ status: "draft", credentialPolicy: "per_user" });
|
||||
const [application] = await db.select().from(toolApplications).where(eq(toolApplications.id, first.application.id));
|
||||
expect(application.status).toBe("draft");
|
||||
await expect(service.checkHealth(first.connectionId, actor)).resolves.toMatchObject({ connection: { healthStatus: "ok" } });
|
||||
});
|
||||
|
||||
it("rolls back partial catalog and profile writes without removing the established public identity", async () => {
|
||||
const fixture = installMcpOAuthFixture({ auth: "public" });
|
||||
const company = await createCompany(db);
|
||||
const service = toolAccessService(db);
|
||||
const actor = { actorType: "user" as const, actorId: "board-user" };
|
||||
const input = { link: MCP_URL, name: "Personal atomic catalog", grantKind: "user" as const };
|
||||
const first = await service.connectGalleryApp(company.id, input, actor);
|
||||
const catalogBefore = await db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, first.connectionId));
|
||||
await db.delete(connectionGrants).where(eq(connectionGrants.connectionId, first.connectionId));
|
||||
await db.update(toolConnections).set({ status: "archived" }).where(eq(toolConnections.id, first.connectionId));
|
||||
await db.update(toolApplications).set({ status: "archived", archivedAt: new Date() }).where(eq(toolApplications.id, first.application.id));
|
||||
fixture.fetchMock.mockRestore();
|
||||
vi.spyOn(globalThis, "fetch").mockImplementation(async () => jsonResponse({
|
||||
jsonrpc: "2.0", id: "paperclip-catalog-refresh",
|
||||
result: { tools: [...FIXTURE_TOOLS, { name: "new_tool", description: "Partial catalog addition" }] },
|
||||
}));
|
||||
await db.execute(sql`
|
||||
CREATE FUNCTION test_personal_profile_failure() RETURNS trigger LANGUAGE plpgsql AS $$
|
||||
BEGIN RAISE EXCEPTION 'personal profile fixture failure'; END $$
|
||||
`);
|
||||
await db.execute(sql`
|
||||
CREATE TRIGGER test_personal_profile_failure BEFORE INSERT ON tool_profile_entries
|
||||
FOR EACH ROW EXECUTE FUNCTION test_personal_profile_failure()
|
||||
`);
|
||||
try {
|
||||
await expect(service.connectGalleryApp(company.id, input, actor)).rejects.toThrow();
|
||||
await expect(db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, first.connectionId)))
|
||||
.resolves.toEqual(catalogBefore);
|
||||
await expect(db.select().from(toolProfiles).where(eq(toolProfiles.companyId, company.id))).resolves.toHaveLength(0);
|
||||
await expect(db.select().from(toolProfileBindings).where(eq(toolProfileBindings.companyId, company.id))).resolves.toHaveLength(0);
|
||||
await expect(db.select().from(connectionGrants).where(eq(connectionGrants.connectionId, first.connectionId)))
|
||||
.resolves.toEqual([expect.objectContaining({ kind: "user", status: "active", credentialSecretRefs: [] })]);
|
||||
await expect(service.getConnection(first.connectionId)).resolves.toMatchObject({ status: "draft", credentialPolicy: "per_user" });
|
||||
} finally {
|
||||
await db.execute(sql`DROP TRIGGER test_personal_profile_failure ON tool_profile_entries`);
|
||||
await db.execute(sql`DROP FUNCTION test_personal_profile_failure()`);
|
||||
}
|
||||
const retry = await service.connectGalleryApp(company.id, { ...input, resumeConnectionId: first.connectionId }, actor);
|
||||
expect(retry.catalog).toHaveLength(3);
|
||||
await expect(db.select().from(toolProfiles).where(eq(toolProfiles.companyId, company.id))).resolves.toHaveLength(1);
|
||||
await expect(db.select().from(toolProfileBindings).where(eq(toolProfileBindings.companyId, company.id))).resolves.toHaveLength(1);
|
||||
});
|
||||
|
||||
it("creates an empty user grant after a personal public URL probe succeeds", async () => {
|
||||
// Use a real loopback MCP server here: neither fetch nor the transport is mocked.
|
||||
const receivedMethods: string[] = [];
|
||||
const mcpServer = createServer(async (req, res) => {
|
||||
const chunks: Buffer[] = [];
|
||||
for await (const chunk of req) chunks.push(Buffer.from(chunk));
|
||||
const body = JSON.parse(Buffer.concat(chunks).toString("utf8"));
|
||||
receivedMethods.push(body.method);
|
||||
res.writeHead(200, { "Content-Type": "application/json" });
|
||||
res.end(JSON.stringify({ jsonrpc: "2.0", id: body.id, result: { tools: FIXTURE_TOOLS } }));
|
||||
});
|
||||
mcpServer.listen(0, "127.0.0.1");
|
||||
await once(mcpServer, "listening");
|
||||
const address = mcpServer.address();
|
||||
if (!address || typeof address === "string") throw new Error("Missing MCP fixture port");
|
||||
try {
|
||||
const company = await createCompany(db);
|
||||
const app = createRouteApp(db, {
|
||||
deploymentMode: "local_trusted",
|
||||
deploymentExposure: "private",
|
||||
});
|
||||
const actor = { actorType: "user" as const, actorId: "board-user" };
|
||||
|
||||
const response = await request(app)
|
||||
.post(`/api/companies/${company.id}/tools/apps/connect`)
|
||||
.send({
|
||||
link: `http://127.0.0.1:${address.port}/mcp`,
|
||||
name: "Fixture personal public",
|
||||
grantKind: "user",
|
||||
});
|
||||
expect(response.status, JSON.stringify(response.body)).toBe(201);
|
||||
const connected = response.body;
|
||||
expect(receivedMethods).toContain("tools/list");
|
||||
|
||||
expect(connected.connection).toMatchObject({
|
||||
status: "draft",
|
||||
credentialPolicy: "per_user",
|
||||
});
|
||||
expect(connected.catalog.map((entry: { toolName: string }) => entry.toolName).sort()).toEqual([
|
||||
"create_insight",
|
||||
"list_insights",
|
||||
]);
|
||||
await expect(
|
||||
db
|
||||
.select()
|
||||
.from(connectionGrants)
|
||||
.where(eq(connectionGrants.connectionId, connected.connectionId)),
|
||||
).resolves.toEqual([
|
||||
expect.objectContaining({
|
||||
kind: "user",
|
||||
subjectUserId: actor.actorId,
|
||||
credentialSecretRefs: [],
|
||||
status: "active",
|
||||
}),
|
||||
]);
|
||||
} finally {
|
||||
await new Promise<void>((resolve, reject) => {
|
||||
mcpServer.close((error) => error ? reject(error) : resolve());
|
||||
mcpServer.closeAllConnections();
|
||||
});
|
||||
}
|
||||
});
|
||||
|
||||
it("completes organization OAuth with a single database connection", async () => {
|
||||
const fixture = installMcpOAuthFixture({ auth: "oauth" });
|
||||
const company = await createCompany(db);
|
||||
|
||||
@@ -13184,6 +13184,39 @@ describeEmbeddedPostgres("tool access service", () => {
|
||||
await expect(db.select().from(toolCatalogEntries)).resolves.toHaveLength(0);
|
||||
});
|
||||
|
||||
it("keeps missing personal authorization as a client-actionable health failure", async () => {
|
||||
const company = await createCompany(db);
|
||||
const service = createTestToolAccessService(db);
|
||||
const { connection } = await createRemoteToolFixture(db, company.id);
|
||||
await db
|
||||
.delete(connectionGrants)
|
||||
.where(eq(connectionGrants.connectionId, connection.id));
|
||||
await db
|
||||
.update(toolConnections)
|
||||
.set({
|
||||
credentialPolicy: "per_user",
|
||||
createdByUserId: "board",
|
||||
})
|
||||
.where(eq(toolConnections.id, connection.id));
|
||||
|
||||
await expect(
|
||||
service.checkHealth(connection.id, {
|
||||
actorType: "user",
|
||||
actorId: "board",
|
||||
}),
|
||||
).rejects.toMatchObject({
|
||||
status: 422,
|
||||
details: expect.objectContaining({
|
||||
code: "user_authorization_required",
|
||||
}),
|
||||
});
|
||||
|
||||
await expect(service.getConnection(connection.id)).resolves.toMatchObject({
|
||||
healthStatus: "error",
|
||||
healthMessage: "This connection needs the current user's authorization",
|
||||
});
|
||||
});
|
||||
|
||||
it("reuses and revives an existing application when connecting with applicationId", async () => {
|
||||
const company = await createCompany(db);
|
||||
const service = createTestToolAccessService(db);
|
||||
|
||||
@@ -2775,6 +2775,7 @@ function healthFailureHttpStatus(failure: {
|
||||
code: string;
|
||||
}): number {
|
||||
if (failure.status === "missing_secret") return 422;
|
||||
if (failure.code === "user_authorization_required") return 422;
|
||||
if (failure.code === "composio_api_key_rejected") return 422;
|
||||
if (failure.code === "tool_connection_transport_unsupported") return 422;
|
||||
if (failure.code.endsWith("_endpoint_rejected")) return 422;
|
||||
@@ -2805,6 +2806,9 @@ function sanitizeHttpFailure(error: unknown): {
|
||||
}
|
||||
if (error instanceof HttpError) {
|
||||
const code = asRecord(error.details).code;
|
||||
if (code === "user_authorization_required") {
|
||||
return { status: "error", message: error.message, code };
|
||||
}
|
||||
if (code === "tool_connection_transport_unsupported") {
|
||||
return { status: "error", message: error.message, code };
|
||||
}
|
||||
@@ -7537,6 +7541,7 @@ export function toolAccessService(
|
||||
async function checkConnectionHealth(
|
||||
connectionId: string,
|
||||
actor?: ActorInfo,
|
||||
options: { allowUnauthenticatedProbe?: boolean } = {},
|
||||
): Promise<ToolConnectionHealthCheckResult> {
|
||||
const connection = await getConnectionRow(connectionId);
|
||||
if (connection.connectionPurpose === "ai") return { connection: toConnection(connection), runtimeSlot: null };
|
||||
@@ -7584,12 +7589,21 @@ export function toolAccessService(
|
||||
await validateAgentMailConnection(connection);
|
||||
} else if (connection.transport === "mcp_remote") {
|
||||
await assertComposioConnectedAccountActive(connection);
|
||||
const canProbeWithoutAuthorization =
|
||||
options.allowUnauthenticatedProbe === true &&
|
||||
connection.status === "draft" &&
|
||||
connection.authKind === "none" &&
|
||||
connection.credentialPolicy === "per_user" &&
|
||||
actor?.actorType === "user" &&
|
||||
actor.actorId === connection.createdByUserId;
|
||||
const credentialHeaders =
|
||||
connection.credentialSource === "vercel_connect"
|
||||
? await resolveCredentialHeaders(connection, actor, {
|
||||
forceRefresh: true,
|
||||
})
|
||||
: undefined;
|
||||
: canProbeWithoutAuthorization
|
||||
? {}
|
||||
: undefined;
|
||||
await remoteTools(connection, credentialHeaders, actor);
|
||||
} else if (isComposioConnection(connection)) {
|
||||
await validateComposioConnection(connection);
|
||||
@@ -12789,6 +12803,7 @@ export function toolAccessService(
|
||||
previous: typeof connectionGrants.$inferSelect | null;
|
||||
current: typeof connectionGrants.$inferSelect;
|
||||
} | null = null;
|
||||
let personalPublicSetupEstablished = false;
|
||||
|
||||
try {
|
||||
const credentialFields =
|
||||
@@ -13238,9 +13253,24 @@ export function toolAccessService(
|
||||
};
|
||||
}
|
||||
|
||||
// A pasted MCP URL starts with unknown auth. For a personal connection,
|
||||
// there is deliberately no empty active grant yet, so probe this one
|
||||
// creator-owned draft without credentials. A 401 can then discover OAuth
|
||||
// and leave grant creation to the callback; a public endpoint gets an
|
||||
// empty personal grant below so later catalog refreshes use the same
|
||||
// identity policy.
|
||||
const unauthenticatedPersonalProbe = Boolean(
|
||||
!galleryEntry &&
|
||||
genericAuthKind === "none" &&
|
||||
credentialSecretRefs.length === 0 &&
|
||||
personalIdentityUserId &&
|
||||
!retainedPersonalIdentity?.grant,
|
||||
);
|
||||
let health: ToolConnectionHealthCheckResult;
|
||||
try {
|
||||
health = await checkConnectionHealth(connectionRow.id, actor);
|
||||
health = await checkConnectionHealth(connectionRow.id, actor, {
|
||||
allowUnauthenticatedProbe: unauthenticatedPersonalProbe,
|
||||
});
|
||||
} catch (error) {
|
||||
if (
|
||||
!galleryEntry &&
|
||||
@@ -13288,6 +13318,52 @@ export function toolAccessService(
|
||||
}
|
||||
throw error;
|
||||
}
|
||||
if (unauthenticatedPersonalProbe) {
|
||||
const personalConnectionId = connectionRow.id;
|
||||
const grant = await db.transaction(async (tx) => {
|
||||
const [createdGrant] = await tx
|
||||
.insert(connectionGrants)
|
||||
.values({
|
||||
companyId,
|
||||
connectionId: personalConnectionId,
|
||||
kind: "user",
|
||||
subjectUserId: personalIdentityUserId!,
|
||||
credentialSecretRefs: [],
|
||||
status: "active",
|
||||
isDefault: false,
|
||||
createdByUserId: personalIdentityUserId!,
|
||||
})
|
||||
.onConflictDoNothing()
|
||||
.returning();
|
||||
if (createdGrant) {
|
||||
await tx.insert(toolAccessAuditEvents).values({
|
||||
companyId,
|
||||
connectionId: personalConnectionId,
|
||||
actorType: "user",
|
||||
actorId: personalIdentityUserId!,
|
||||
action: "connection_grant.created",
|
||||
outcome: "success",
|
||||
reasonCode: "personal_identity_created",
|
||||
details: { kind: "user", credentialSecretRefCount: 0 },
|
||||
});
|
||||
}
|
||||
return createdGrant;
|
||||
});
|
||||
if (!grant) {
|
||||
// Another setup attempt may have created this user's grant while
|
||||
// both probes were in flight. Reuse only an active personal grant;
|
||||
// never overwrite credentials, revive a revoked grant, or claim its
|
||||
// audit/rollback ownership in this attempt.
|
||||
const existingGrant = await vaultGrantForConnection(connectionRow, actor);
|
||||
if (existingGrant?.kind !== "user" || existingGrant.subjectUserId !== personalIdentityUserId) {
|
||||
throw conflict("The personal credential changed during setup. Please try again.");
|
||||
}
|
||||
}
|
||||
// A successful public probe establishes this identity, like OAuth
|
||||
// consent does. Keep its draft and grant if catalog/default discovery
|
||||
// later fails: another retry may already be using the committed grant.
|
||||
personalPublicSetupEstablished = true;
|
||||
}
|
||||
if (galleryEntry?.slug === COMPOSIO_GALLERY_KEY) {
|
||||
const [application] = await db
|
||||
.select()
|
||||
@@ -13306,10 +13382,22 @@ export function toolAccessService(
|
||||
};
|
||||
}
|
||||
const restoreDraftDefaults = Boolean(revivedConnectionPrevious);
|
||||
const refresh = await refreshCatalog(connectionRow.id, actor, {
|
||||
const refreshOptions = {
|
||||
enableAllByDefault: restoreDraftDefaults,
|
||||
restoreDraftDefaults,
|
||||
});
|
||||
};
|
||||
const catalogConnectionId = connectionRow.id;
|
||||
// Keep the established public identity outside this transaction while
|
||||
// making catalog, defaults, and their audits one all-or-nothing step.
|
||||
const refresh: ToolCatalogRefreshResult = personalPublicSetupEstablished
|
||||
? await db.transaction(async (tx): Promise<ToolCatalogRefreshResult> =>
|
||||
toolAccessService(tx as unknown as Db, options).refreshCatalog(
|
||||
catalogConnectionId,
|
||||
actor,
|
||||
refreshOptions,
|
||||
),
|
||||
)
|
||||
: await refreshCatalog(catalogConnectionId, actor, refreshOptions);
|
||||
const [application] = await db
|
||||
.select()
|
||||
.from(toolApplications)
|
||||
@@ -13328,6 +13416,7 @@ export function toolAccessService(
|
||||
},
|
||||
};
|
||||
} catch (error) {
|
||||
if (personalPublicSetupEstablished) throw error;
|
||||
let identityRollbackError: unknown = null;
|
||||
let preserveConcurrentRevival = false;
|
||||
if (connectionRow && revivedConnectionPrevious) {
|
||||
|
||||
Reference in new issue
Block a user