mirror of
https://github.com/paperclipai/paperclip.git
synced 2026-10-06 21:05:21 +02:00
## Thinking Path > - Paperclip is the open source app people use to manage AI agents for work. > - Apps give agents governed access to external tools. > - The Apps gallery lists Notion, but the server required manually configured OAuth credentials. > - Notion's hosted MCP server supports OAuth discovery and dynamic client registration. > - Notion also requires HTTPS or a loopback HTTP redirect URI. > - This pull request adds a direct Notion MCP OAuth path with PKCE and reusable dynamic clients. > - It also adds the current Apps UI states for connect and reauthorization. > - The benefit is a secure Notion connection with no manual client credential setup. ## Linked Issues or Issue Description **What existing behavior does this improve?** The Apps gallery, Apps connect route, OAuth token lifecycle, and managed MCP gateway. **Subsystem affected** `server/`, `packages/shared/`, `scripts/`, and `ui/`. **Current behavior** The Notion gallery cards are disabled. The server uses the classic Notion OAuth endpoints and requires operator-supplied client credentials. It does not register an OAuth client from provider metadata. Concurrent refreshes can also replay a rotating refresh token. **Proposed behavior** Enable the Notion Apps flow. Discover OAuth metadata from `https://mcp.notion.com/mcp`. Register and reuse a public RFC 7591 client with PKCE. Require HTTPS or loopback HTTP callbacks. Serialize refreshes, store each rotated refresh token before the new access token can be used, and show a reconnect state for `invalid_grant`. **Reason and benefit** Operators can connect the built-in Notion MCP app without creating or copying OAuth credentials. Paperclip keeps dynamic clients and rotating tokens in the company secret store. **Breaking changes** None. Explicit environment client credentials still take priority. Existing Slack and Linear OAuth endpoint hints remain unchanged. Other OAuth apps remain disabled unless they are allowlisted. **Additional context** PR #10910 is a related, broader Connections v3 wizard replacement. This PR is the focused current Apps flow. The MCP Tool Gateway and Connected Apps items in `ROADMAP.md` cover this planned capability. ## What Changed - Classify all 20 reviewed Notion MCP tools with provider-scoped read and write defaults. - Require approval for selected Notion mutations, including move, duplicate, and convert actions that generic verb matching missed. - Preserve company-scoped connection and catalog resolution for Notion profiles and policies. - Add RFC 7591 dynamic client registration with `token_endpoint_auth_method=none` and mandatory PKCE. - Store the dynamic client ID on the connection and store any returned client secret in the company secret store. - Reuse the registered client for later connects and keep explicit environment credentials as the first choice. - Discover protected-resource and authorization-server metadata from the Notion MCP endpoint. - Add `redirectConstraints: "https-or-loopback-http"` to the generated Notion app definition and shared contract. - Reject non-loopback plain HTTP callbacks before network access with a TLS setup error. - Serialize client registration and token refresh operations within the server process. - Store a rotated refresh token before publishing the refreshed access token. - Treat `invalid_grant` as terminal and move the connection to a clear reauthorization state. - Add focused coverage for registration reuse, callback constraints, refresh rotation, and terminal grants. - Enable the Notion Apps route and add connect, redirect, success, error, and reconnect UI states. - Keep non-allowlisted OAuth apps blocked and cover the UI policy with regression tests. ## Verification - The focused Notion policy integration test passed with embedded PostgreSQL. - The focused 20-tool classification test passed. - The server typecheck passed on the governance head. - `pnpm -r typecheck` passed on the rebased head. - `pnpm --filter @paperclipai/server typecheck` passed after the security follow-up. - `pnpm exec vitest run server/src/__tests__/tool-access-service.test.ts -t \u0027DCR|refresh tokens|invalid_grant|abandoned lease\u0027` passed 10 focused security tests. - `pnpm build` passed on the rebased head. - `pnpm exec vitest run server/src/__tests__/tool-access-service.test.ts -t 'OAuth|oauth'` passed 14 tests. - `pnpm exec vitest run packages/shared/src/app-definitions.test.ts` passed 5 tests. - The complete server group passed 3,686 tests with 4 skipped. - The complete UI group passed 3,656 tests. - The full local runner found one environment-only CLI failure because this agent runtime injects static AWS credentials into a test that expects `AWS_PROFILE` only. `env -u AWS_ACCESS_KEY_ID -u AWS_SECRET_ACCESS_KEY pnpm exec vitest run cli/src/__tests__/secrets.test.ts` passed all 8 tests. - The prior UI verification passed 55 focused tests, `pnpm check:token-gates`, the Storybook build, and review of six 1440 x 1000 screenshots. - OAuth request sequence: protected-resource metadata `GET https://mcp.notion.com/.well-known/oauth-protected-resource/mcp`; authorization metadata `GET https://mcp.notion.com/.well-known/oauth-authorization-server`; dynamic registration `POST https://mcp.notion.com/register`; authorization `GET https://mcp.notion.com/authorize`; token exchange and refresh `POST https://mcp.notion.com/token`; MCP traffic `POST https://mcp.notion.com/mcp`. - The live metadata and registration probe confirmed that Notion accepts HTTPS and loopback HTTP redirects. It rejects a plain HTTP private hostname. - A later QA task owns the full browser consent and managed gateway tool-list dry run against a configured HTTPS deployment. ## Risks - Notion can add tools. Unrecognized names use the generic classifier, and new or changed risky tools stay quarantined after connection activation. - A deployment that uses a private non-loopback hostname must configure HTTPS before it can connect Notion. - Dynamic registration creates a provider-side client. Paperclip reuses it because registration does not provide a standard delete operation. - Refresh coordination uses a database CAS lease across service instances. An unclean crash leaves an uncertain lease and requires reconnect instead of risking refresh-token replay. - The current Apps surface overlaps with PR #10910. Merge order can require a small conflict resolution if that PR lands first. > For core feature work, check [`ROADMAP.md`](ROADMAP.md) first and discuss it in `#dev` before opening the PR. Feature PRs that overlap with planned core work may need to be redirected — check the roadmap first. See `CONTRIBUTING.md`. ## Model Used - OpenAI Codex on a GPT-5 runtime. The exact deployment ID and context window are not exposed. The runtime used reasoning, repository tools, code execution, and network tools. ## 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> Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
97 lines
3.1 KiB
JavaScript
97 lines
3.1 KiB
JavaScript
#!/usr/bin/env node
|
|
// Capture the PAP-16650 Notion connection-flow Storybook stories.
|
|
// Usage: node scripts/screenshot-notion-connect-flow.mjs <storybook-static-dir> <output-dir>
|
|
|
|
import http from "node:http";
|
|
import path from "node:path";
|
|
import fs from "node:fs/promises";
|
|
import { chromium } from "@playwright/test";
|
|
|
|
async function main() {
|
|
const [, , staticDir, outDir] = process.argv;
|
|
if (!staticDir || !outDir) {
|
|
console.error(
|
|
"usage: node scripts/screenshot-notion-connect-flow.mjs <storybook-static-dir> <output-dir>",
|
|
);
|
|
process.exit(1);
|
|
}
|
|
|
|
await fs.mkdir(outDir, { recursive: true });
|
|
const absStaticDir = path.resolve(staticDir);
|
|
const server = http.createServer(async (req, res) => {
|
|
try {
|
|
let urlPath = decodeURIComponent((req.url || "/").split("?")[0]);
|
|
if (urlPath.endsWith("/")) urlPath += "iframe.html";
|
|
const filePath = path.resolve(absStaticDir, `.${urlPath}`);
|
|
if (!filePath.startsWith(absStaticDir + path.sep) && filePath !== absStaticDir) {
|
|
res.writeHead(403);
|
|
res.end("Forbidden");
|
|
return;
|
|
}
|
|
|
|
const buf = await fs.readFile(filePath);
|
|
const ext = path.extname(filePath).toLowerCase();
|
|
const mime =
|
|
{
|
|
".html": "text/html; charset=utf-8",
|
|
".js": "application/javascript",
|
|
".css": "text/css",
|
|
".json": "application/json",
|
|
".svg": "image/svg+xml",
|
|
".png": "image/png",
|
|
".woff": "font/woff",
|
|
".woff2": "font/woff2",
|
|
".map": "application/json",
|
|
}[ext] || "application/octet-stream";
|
|
res.writeHead(200, { "content-type": mime });
|
|
res.end(buf);
|
|
} catch (error) {
|
|
res.writeHead(404);
|
|
res.end(String(error));
|
|
}
|
|
});
|
|
|
|
await new Promise((resolve) => server.listen(0, "127.0.0.1", resolve));
|
|
const address = server.address();
|
|
if (!address || typeof address === "string") throw new Error("Failed to start screenshot server");
|
|
const baseUrl = `http://127.0.0.1:${address.port}/iframe.html`;
|
|
|
|
const stories = [
|
|
["browse-entry", "01-browse-entry.png"],
|
|
["connect-entry", "02-connect-entry.png"],
|
|
["in-flight", "03-in-flight.png"],
|
|
["connected", "04-connected.png"],
|
|
["connect-error", "05-connect-error.png"],
|
|
["reconnect-required", "06-reconnect-required.png"],
|
|
];
|
|
|
|
const browser = await chromium.launch();
|
|
try {
|
|
for (const [story, file] of stories) {
|
|
const context = await browser.newContext({
|
|
viewport: { width: 1440, height: 1000 },
|
|
deviceScaleFactor: 2,
|
|
colorScheme: "light",
|
|
});
|
|
const page = await context.newPage();
|
|
await page.goto(
|
|
`${baseUrl}?id=apps-notion-mcp-connect-flow-pap-16650--${story}&viewMode=story`,
|
|
{ waitUntil: "networkidle" },
|
|
);
|
|
await page.waitForTimeout(500);
|
|
const out = path.join(outDir, file);
|
|
await page.screenshot({ path: out, fullPage: true });
|
|
console.log("wrote", out);
|
|
await context.close();
|
|
}
|
|
} finally {
|
|
await browser.close();
|
|
server.close();
|
|
}
|
|
}
|
|
|
|
main().catch((error) => {
|
|
console.error(error);
|
|
process.exit(1);
|
|
});
|