mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-24 10:13:11 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Connections need durable credentials that survive authorization and token rotation. > - Storage, registration, refresh coordination and credential selection lived in the general tool service. > - Each has a different ownership boundary: secrets, process-wide registration, database leases and per-service refresh work. > - This pull request gives those responsibilities named modules and passes their narrow capabilities to consumers. > - The benefit is visible credential ownership while retaining current concurrency and failure behavior. ## Linked Issues or Issue Description Depends on #17159. Earlier stack: #17144 → #17146 → #17147 → #17155 → #17157 → #17159 → this PR. **What existing behavior does this improve?** Maintenance of OAuth credential persistence, refresh and provider credential access. **Current behavior** The 14,877-line service owns registration flights, refresh leases, secret writes and provider selection alongside app setup. **Proposed behavior** Dedicated owners handle credential storage, client registration, refresh leases, connection/grant refresh, GitHub continuity and Vercel credential access. Credential selection composes those owners. Authorization callbacks remain in the coordinator for the next extraction. **Reason and benefit** The entry point falls to 12,019 lines, a reduction of 2,858. Lease acquisition and cleanup can be reviewed together, independently of callback completion. ## What Changed - Extract credential owners and shared single-flight/connection-link primitives. The largest new module is 650 lines. - Preserve process-wide client-registration flights, per-service refresh maps and GitHub sweep state. - Preserve live cloud-connector lookup, options and environment reads; use the existing secret client and transaction-scoped clients. - Extend the real local HTTP/PostgreSQL OAuth fixture with concurrent refresh across two service instances and authenticated use of rotated credentials. ## Verification - The two extended HTTP integration cases pass before extraction for public and client-secret-basic clients. - After extraction, all 470 public service and HTTP integration cases pass, including refresh failure, abandoned leases, secret redaction and membership revocation cases. The fixture was then strengthened after review: a second expiry must use the persisted rotated refresh token, and the provider rejects the old token. Both cases pass before and after extraction; server typecheck also passes. - Server typecheck, module-boundary gate and diff whitespace checks pass. - All 233 pre-existing function/method bodies match by normalized syntax comparison. - Full typecheck/build will run at the stack handoff. The earlier interrupted local full suite remains unverified; all current-head CI gates pass, with Greptile 5/5 and no open review findings. - Prior-head CI failure retained: run 39708550066, general server shard 2/12, `native-session-executor.test.ts` case “preserves a real listening process and its marker when a health RPC times out” raised `runner_remote_process_identity_unavailable` / `process_identity_changed`. Both the fixture and runner implementation are unchanged. The isolated case passes without changing assertions or timeouts. The fresh full CI run passes for the test-only review fix; the prior failure remains recorded here. ## Risks - Changing state lifetime could permit duplicate refreshes or registrations. Existing concurrency cases and the real HTTP fixture exercise these paths. - A transaction-scoped secret write must stay inside its authority lock. Function bodies, await ordering and client arguments are preserved. - The fixture proves assembled local HTTP/database behavior, not live third-party qualification. - No schema, public API, policy or scheduling change is intended. ## Model Used OpenAI Codex, GPT-6 family, with repository tools and code execution. The exact serving model ID and context-window size are not exposed in this session. ## 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>
196 lines
11 KiB
TypeScript
196 lines
11 KiB
TypeScript
import { randomUUID } from "node:crypto";
|
|
import { mkdtemp, rm } from "node:fs/promises";
|
|
import { createServer } from "node:http";
|
|
import type { AddressInfo } from "node:net";
|
|
import { tmpdir } from "node:os";
|
|
import { join } from "node:path";
|
|
import express from "express";
|
|
import request from "supertest";
|
|
import { afterAll, beforeAll, describe, expect, it, vi } from "vitest";
|
|
import { companies, companyMemberships, createDb, connectionGrants, toolCatalogEntries, toolConnections, toolOauthStates } from "@paperclipai/db";
|
|
import { eq } from "drizzle-orm";
|
|
import { toolAccessService } from "../services/tool-access.js";
|
|
import { toolAccessRoutes } from "../routes/tool-access.js";
|
|
import { errorHandler } from "../middleware/index.js";
|
|
import { startEmbeddedPostgresTestDatabase } from "./helpers/embedded-postgres.js";
|
|
|
|
describe("tool OAuth over local HTTP", () => {
|
|
let fixture: Awaited<ReturnType<typeof startEmbeddedPostgresTestDatabase>>;
|
|
let db: ReturnType<typeof createDb>;
|
|
let keyDir: string;
|
|
|
|
beforeAll(async () => {
|
|
keyDir = await mkdtemp(join(tmpdir(), "tool-oauth-protocol-"));
|
|
vi.stubEnv("PAPERCLIP_SECRETS_MASTER_KEY_FILE", join(keyDir, "key"));
|
|
vi.stubEnv("PAPERCLIP_PUBLIC_URL", "http://127.0.0.1:3100");
|
|
vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_ID", "");
|
|
vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_SECRET", "");
|
|
fixture = await startEmbeddedPostgresTestDatabase("tool-oauth-protocol-");
|
|
db = createDb(fixture.connectionString);
|
|
});
|
|
|
|
afterAll(async () => {
|
|
await fixture?.cleanup();
|
|
vi.unstubAllEnvs();
|
|
if (keyDir) await rm(keyDir, { recursive: true, force: true });
|
|
});
|
|
|
|
it.each(["none", "client_secret_basic"] as const)("registers, exchanges, and activates a %s client through the routes", async (authMethod) => {
|
|
const userId = `oauth-user-${randomUUID()}`;
|
|
const [company] = await db.insert(companies).values({
|
|
name: "OAuth protocol fixture", issuePrefix: `O${randomUUID().slice(0, 5)}`,
|
|
}).returning();
|
|
await db.insert(companyMemberships).values({
|
|
companyId: company.id, principalType: "user", principalId: userId,
|
|
status: "active", membershipRole: "admin",
|
|
});
|
|
const calls: Array<{ path: string; authorization?: string; body: string }> = [];
|
|
let origin: string;
|
|
let currentAccessToken = "fixture-access";
|
|
let currentRefreshToken = "fixture-refresh";
|
|
const provider = createServer(async (req, res) => {
|
|
let body = "";
|
|
for await (const chunk of req) body += chunk;
|
|
const path = new URL(req.url!, origin).pathname;
|
|
calls.push({ path, authorization: req.headers.authorization, body });
|
|
const json = (value: unknown, status = 200) => {
|
|
res.writeHead(status, { "content-type": "application/json" });
|
|
res.end(JSON.stringify(value));
|
|
};
|
|
if (path.startsWith("/.well-known/oauth-protected-resource")) {
|
|
return json({ resource: `${origin}/mcp`, authorization_servers: [origin] });
|
|
}
|
|
if (path === "/.well-known/oauth-authorization-server") {
|
|
return json({
|
|
issuer: origin, authorization_endpoint: `${origin}/authorize`,
|
|
token_endpoint: `${origin}/token`, registration_endpoint: `${origin}/register`,
|
|
code_challenge_methods_supported: ["S256"],
|
|
token_endpoint_auth_methods_supported: [authMethod],
|
|
scopes_supported: ["tools.read", "offline_access"],
|
|
grant_types_supported: ["authorization_code", "refresh_token"],
|
|
});
|
|
}
|
|
if (path === "/register") {
|
|
return json({ ...JSON.parse(body), client_id: "fixture-client",
|
|
...(authMethod === "client_secret_basic" ? { client_secret: "fixture-secret" } : {}),
|
|
});
|
|
}
|
|
if (path === "/token") {
|
|
const refreshing = new URLSearchParams(body).get("grant_type") === "refresh_token";
|
|
if (refreshing) {
|
|
if (new URLSearchParams(body).get("refresh_token") !== currentRefreshToken) {
|
|
return json({ error: "invalid_grant" }, 400);
|
|
}
|
|
currentAccessToken = "fixture-access-rotated";
|
|
currentRefreshToken = "fixture-refresh-rotated";
|
|
}
|
|
return json({ access_token: currentAccessToken, refresh_token: currentRefreshToken,
|
|
expires_in: 3600, token_type: "Bearer", scope: "tools.read offline_access" });
|
|
}
|
|
if (path === "/mcp") {
|
|
if (req.headers.authorization !== `Bearer ${currentAccessToken}`) {
|
|
res.setHeader("www-authenticate", `Bearer resource_metadata="${origin}/.well-known/oauth-protected-resource/mcp"`);
|
|
return json({}, 401);
|
|
}
|
|
const rpc = JSON.parse(body);
|
|
if (rpc.method === "notifications/initialized") {
|
|
res.writeHead(202).end();
|
|
return;
|
|
}
|
|
return json({ jsonrpc: "2.0", id: rpc.id, result: rpc.method === "initialize"
|
|
? { protocolVersion: "2025-06-18", capabilities: { tools: {} }, serverInfo: { name: "fixture", version: "1" } }
|
|
: { tools: [{ name: "read_fixture", description: "Read fixture state", inputSchema: { type: "object", properties: {} }, annotations: { readOnlyHint: true } }] },
|
|
});
|
|
}
|
|
json({}, 404);
|
|
});
|
|
await new Promise<void>((resolve) => provider.listen(0, "127.0.0.1", resolve));
|
|
origin = `http://127.0.0.1:${(provider.address() as AddressInfo).port}`;
|
|
try {
|
|
const app = express();
|
|
app.use(express.json());
|
|
app.use((req, _res, next) => {
|
|
req.actor = { type: "board", userId, source: "local_implicit", isInstanceAdmin: true };
|
|
next();
|
|
});
|
|
app.use("/api", toolAccessRoutes(db, { deploymentMode: "local_trusted", deploymentExposure: "private", paperclipCloudConnector: null }));
|
|
app.use(errorHandler);
|
|
|
|
const connected = await request(app).post(`/api/companies/${company.id}/tools/apps/connect`)
|
|
.send({ link: `${origin}/mcp`, name: "Local OAuth" }).expect(201);
|
|
const authorization = new URL(connected.body.auth.startUrl);
|
|
expect(authorization.origin).toBe(origin);
|
|
expect(authorization.searchParams.get("client_id")).toBe("fixture-client");
|
|
expect(authorization.searchParams.get("code_challenge_method")).toBe("S256");
|
|
expect(authorization.searchParams.get("resource")).toBe(`${origin}/mcp`);
|
|
const state = authorization.searchParams.get("state")!;
|
|
const [pending] = await db.select().from(toolOauthStates).where(eq(toolOauthStates.state, state));
|
|
expect(pending.companyId).toBe(company.id);
|
|
|
|
const completed = await request(app).get("/api/tools/oauth/callback")
|
|
.query({ state, code: "fixture-code", iss: origin }).expect(200);
|
|
expect(completed.body.catalog).toEqual([expect.objectContaining({ toolName: "read_fixture", riskLevel: "read" })]);
|
|
await request(app).post(`/api/companies/${company.id}/tools/apps/${connected.body.connectionId}/finish`)
|
|
.send({ enabledCatalogEntryIds: completed.body.catalog.map((entry: { id: string }) => entry.id), askFirstCatalogEntryIds: [], access: "all_agents" }).expect(200);
|
|
|
|
const [connection] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.body.connectionId));
|
|
expect(connection).toMatchObject({ companyId: company.id, status: "active", enabled: true, ownership: "dcr" });
|
|
expect(connection.config.oauth).toMatchObject({ clientId: "fixture-client", clientTokenEndpointAuthMethod: authMethod, clientIssuer: origin });
|
|
expect(JSON.stringify(connection.config)).not.toMatch(/fixture-(access|refresh|secret)/);
|
|
expect(connection.credentialSecretRefs.map((ref) => ref.configPath)).toEqual(expect.arrayContaining(["oauth.access_token", "oauth.refresh_token"]));
|
|
expect(await db.select().from(toolCatalogEntries).where(eq(toolCatalogEntries.connectionId, connection.id)))
|
|
.toEqual([expect.objectContaining({ toolName: "read_fixture", status: "active", reviewedByUserId: userId })]);
|
|
const tokenCall = calls.find((call) => call.path === "/token")!;
|
|
const tokenBody = new URLSearchParams(tokenCall.body);
|
|
expect(tokenBody.get("code")).toBe("fixture-code");
|
|
expect(tokenBody.get("code_verifier")).toBe(pending.codeVerifier);
|
|
expect(tokenBody.get("resource")).toBe(`${origin}/mcp`);
|
|
expect(tokenCall.authorization).toBe(authMethod === "client_secret_basic"
|
|
? `Basic ${Buffer.from("fixture-client:fixture-secret").toString("base64")}` : undefined);
|
|
expect(tokenBody.has("client_secret")).toBe(false);
|
|
expect(calls.some((call) => call.path === "/mcp" && call.authorization === "Bearer fixture-access")).toBe(true);
|
|
expect(calls.filter((call) => call.path === "/register")).toHaveLength(1);
|
|
|
|
const [grant] = await db.select().from(connectionGrants).where(eq(connectionGrants.connectionId, connection.id));
|
|
await db.update(connectionGrants).set({ providerTenant: {
|
|
...grant.providerTenant,
|
|
oauth: { ...grant.providerTenant?.oauth, accessTokenExpiresAt: new Date(0).toISOString() },
|
|
} }).where(eq(connectionGrants.id, grant.id));
|
|
const options = { deploymentMode: "local_trusted" as const, deploymentExposure: "private" as const, paperclipCloudConnector: null };
|
|
const first = toolAccessService(db, options);
|
|
const second = toolAccessService(db, options);
|
|
const refreshInput = { companyId: company.id, connectionId: connection.id, grantId: grant.id, actor: { actorType: "user" as const, actorId: userId } };
|
|
const refreshed = await Promise.all([
|
|
first.refreshOAuthGrantCredentials(refreshInput),
|
|
second.refreshOAuthGrantCredentials(refreshInput),
|
|
]);
|
|
expect(refreshed.map(row => row.id)).toEqual([grant.id, grant.id]);
|
|
const exchanges = calls.filter(call => call.path === "/token");
|
|
expect(exchanges).toHaveLength(2);
|
|
const refreshBody = new URLSearchParams(exchanges[1].body);
|
|
expect(refreshBody.get("grant_type")).toBe("refresh_token");
|
|
expect(refreshBody.get("refresh_token")).toBe("fixture-refresh");
|
|
expect(refreshBody.get("resource")).toBe(`${origin}/mcp`);
|
|
expect(exchanges[1].authorization).toBe(tokenCall.authorization);
|
|
const [persistedGrant] = await db.select().from(connectionGrants).where(eq(connectionGrants.id, grant.id));
|
|
expect(persistedGrant.providerTenant?.oauth).not.toHaveProperty("refreshLease");
|
|
expect(Date.parse(persistedGrant.providerTenant!.oauth!.accessTokenExpiresAt!)).toBeGreaterThan(Date.now());
|
|
expect(JSON.stringify(persistedGrant)).not.toMatch(/fixture-(access|refresh|secret)/);
|
|
await first.checkHealth(connection.id, { actorType: "user", actorId: userId });
|
|
expect(calls.some(call => call.path === "/mcp" && call.authorization === "Bearer fixture-access-rotated")).toBe(true);
|
|
await db.update(connectionGrants).set({ providerTenant: {
|
|
...persistedGrant.providerTenant,
|
|
oauth: { ...persistedGrant.providerTenant?.oauth, accessTokenExpiresAt: new Date(0).toISOString() },
|
|
} }).where(eq(connectionGrants.id, grant.id));
|
|
await second.refreshOAuthGrantCredentials(refreshInput);
|
|
const allExchanges = calls.filter(call => call.path === "/token");
|
|
expect(allExchanges).toHaveLength(3);
|
|
expect(new URLSearchParams(allExchanges[2].body).get("refresh_token")).toBe("fixture-refresh-rotated");
|
|
expect(calls.filter(call => call.path === "/register")).toHaveLength(1);
|
|
} finally {
|
|
provider.closeAllConnections();
|
|
await new Promise<void>((resolve, reject) => provider.close((error) => error ? reject(error) : resolve()));
|
|
}
|
|
});
|
|
});
|