From 03cfad7ceb200499c7f383493f240ecffaf81203 Mon Sep 17 00:00:00 2001 From: Dotta <34892728+cryppadotta@users.noreply.github.com> Date: Thu, 6 Aug 2026 22:18:08 -0500 Subject: [PATCH] feat(apps): connect Notion through MCP OAuth (#11009) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## 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 Co-authored-by: Claude Fable 5 --- doc/connections/CONNECTOR-PLAYBOOK.md | 364 ++++++++ packages/shared/src/app-definitions.test.ts | 10 +- .../shared/src/app-definitions/notion.json | 11 +- packages/shared/src/index.ts | 1 + packages/shared/src/types/app-definition.ts | 3 +- .../shared/src/validators/app-definition.ts | 2 +- scripts/ingest-app-definitions.mjs | 4 +- scripts/screenshot-notion-connect-flow.mjs | 96 +++ .../src/__tests__/tool-access-service.test.ts | 569 ++++++++++++- server/src/services/tool-access.ts | 805 ++++++++++++++++-- ui/src/App.tsx | 3 +- ui/src/pages/apps/AppDetail.test.tsx | 64 ++ ui/src/pages/apps/AppDetail.tsx | 3 +- ui/src/pages/apps/AppsConnect.test.tsx | 319 ++++++- ui/src/pages/apps/AppsConnect.tsx | 415 +++++++-- ui/src/pages/apps/Browse.test.tsx | 15 +- ui/src/pages/apps/Browse.tsx | 20 +- ui/src/pages/apps/Connections.tsx | 4 +- ui/src/pages/apps/app-connect-policy.test.ts | 28 + ui/src/pages/apps/app-connect-policy.ts | 14 + .../pages/apps/app-detail/AdvancedPanel.tsx | 36 +- ui/src/pages/apps/app-detail/SetupPanel.tsx | 11 +- ui/src/pages/apps/store-cards.tsx | 4 + .../stories/notion-connect-flow.stories.tsx | 177 ++++ 24 files changed, 2820 insertions(+), 158 deletions(-) create mode 100644 scripts/screenshot-notion-connect-flow.mjs create mode 100644 ui/src/pages/apps/app-connect-policy.test.ts create mode 100644 ui/src/pages/apps/app-connect-policy.ts create mode 100644 ui/storybook/stories/notion-connect-flow.stories.tsx diff --git a/doc/connections/CONNECTOR-PLAYBOOK.md b/doc/connections/CONNECTOR-PLAYBOOK.md index 2c954e7090..d1f9402798 100644 --- a/doc/connections/CONNECTOR-PLAYBOOK.md +++ b/doc/connections/CONNECTOR-PLAYBOOK.md @@ -183,6 +183,93 @@ Recommended defaults for a new catalog entry: If a gallery card cannot pass this path against a real vendor, de-list it or mark it unavailable until the missing auth, transport, or governance dependency is fixed. +## MCP-Direct Connections (Hosted MCP + OAuth) + +Many vendors now expose an official hosted MCP server whose authorization +server is discovered from the MCP endpoint itself, instead of documenting fixed +OAuth URLs. For these connectors the manifest's `oauth` block is a hint at +most; the broker resolves endpoints at connect time: + +1. `GET ` unauthenticated returns `401` with a `WWW-Authenticate` + header naming the protected-resource metadata URL (RFC 9728). +2. `GET /.well-known/oauth-protected-resource[/]` names the + authorization server(s). +3. `GET /.well-known/oauth-authorization-server` (RFC 8414) yields + `authorization_endpoint`, `token_endpoint`, and — when the vendor supports + dynamic registration — `registration_endpoint`. + +The broker implements this in `discoverOAuthEndpoints` +(`server/src/services/tool-access.ts`); connections with transport +`mcp_remote` and `auth: "oauth"` prefer discovered endpoints over manifest +hints. Keep manifest hints current anyway so reviewers can read the expected +endpoints without running discovery. + +### Dynamic client registration (RFC 7591) + +Vendors whose authorization server advertises a `registration_endpoint` and +supports public clients (`token_endpoint_auth_method: "none"` plus PKCE S256) +need **no pre-provisioned OAuth app at all**. At first connect the broker +registers a client on the fly and stores it on the connection: + +- Registration request: `client_name` `Paperclip ()`, + `redirect_uris` = the instance's own callback, `grant_types` + `["authorization_code", "refresh_token"]`, `response_types` `["code"]`, + `token_endpoint_auth_method` `"none"`. +- The issued `client_id` is persisted in the connection's OAuth config and any + issued `client_secret` becomes a `company_secrets` ref. The registered + client is **reused** for every later authorize/refresh on that connection — + re-registering orphans prior grants on providers that bind grants to the + client. +- Env-registered clients always win: when + `PAPERCLIP_TOOL_OAUTH__CLIENT_ID/_SECRET` are configured, the + broker uses them (`customer` ownership) and skips registration. List both + `customer` and `dcr` in the method's `ownershipModes` when the vendor + supports both. + +**DCR needs neither Paperclip ID nor Paperclip Connect.** DCR is always +instance-local (ratified in the PAP-14828 connector-service spec, section 10 +item 8.4: "DCR is always instance-local; the service has no DCR involvement"). +Each instance registers its own public client with the vendor and uses its own +`/api/tools/oauth/callback` redirect. **Cloud-hosted and self-hosted instances +use the SAME path** — the only per-instance difference is the hostname inside +the redirect URI. `id.paperclip.ing` authenticates operators only and never +holds resource tokens; `connect.paperclip.ing` is a fallback only for +providers that genuinely require a pre-registered public redirect, which a DCR +provider by definition does not. + +### Redirect-URI constraints + +Vendors restrict what `redirect_uris` a dynamic client may register. Record +the probed constraint in the `AppDefinition` `redirectConstraints` field and +enforce it before starting OAuth. The first supported value is +`https-or-loopback-http` (Notion's rule): HTTPS on any host — public or +private — or plain HTTP only on loopback (`localhost`, `*.localhost`, `::1`, +`127.0.0.0/8`). A plain-HTTP non-loopback origin fails fast with +`oauth_redirect_origin_unsupported` ("This provider requires an HTTPS or +loopback origin. Configure TLS before connecting.") and a pointer to the TLS +deployment docs, instead of a confusing vendor-side `invalid_redirect_uri`. +Probe the constraint with real registration attempts before writing the +manifest — the redirect-URI rule and browser-reachability are independent +axes; a private HTTPS host can be fine even when plain HTTP is not. + +### Documentation standards for every connection doc + +Every connection doc — playbook appendix, proposal, or user-facing doc — +must include all three of the following (they are part of the template below): + +1. **Service involvement statement.** Say explicitly whether Paperclip ID or + Paperclip Connect participates in the flow. For RFC 7591 DCR providers the + answer is always: neither — DCR is instance-local and cloud vs self-hosted + use the same path. +2. **Sequence diagram + exact endpoints.** A sequence diagram of how the + connection works, and the exact paths/endpoints used for auth: authorize, + token, registration (if DCR), and the Paperclip callback. Keep mermaid + sources next to the doc; do not put semicolons inside mermaid message text + (they parse as statement separators). +3. **Administrator setup instructions.** Step-by-step: what (if anything) an + admin must register — callback URLs? client credentials? nothing, for DCR? — + where to register it, and how to verify the connection works end to end. + ## Template Copy this section into a connector proposal or implementation issue. @@ -208,6 +295,25 @@ Copy this section into a connector proposal or implementation issue. - Secret storage: company_secrets refs only - Revocation behavior: +## Connection Flow (mandatory) + +- Sequence diagram: +- Auth endpoints (exact paths): + - Authorize: + - Token: + - Registration (if DCR): + - Discovery (.well-known), if any: + - Paperclip callback: `/api/tools/oauth/callback` (or n/a) +- Redirect constraints (probed): none / https-or-loopback-http / requires-public-redirect +- Paperclip ID / Paperclip Connect involvement: <"none — DCR is instance-local; cloud and self-hosted use the same path" for RFC 7591 providers; otherwise name the role> + +## Administrator Setup (mandatory) + +- What the admin must register (callback URLs? client credentials? nothing for DCR?): +- Where to register it: +- Instance prerequisites (TLS, base URL, feature flags): +- How to verify the connection works: + ## Resource Filters - Required filters: @@ -363,3 +469,261 @@ Linear's real-vendor evidence belongs in [PAP-12373](/PAP/issues/PAP-12373). The Connector proposals now target the versioned `AppDefinition` contract in `packages/shared/src/types/app-definition.ts`. Seed data is one JSON file per provider under `packages/shared/src/app-definitions/`; regenerate Wave 1 with `pnpm connections:ingest-app-definitions`. The generator parses all 99 captured templates, validates required placeholders, OAuth ownership modes, and API-key placement, and produces deterministic output for review. FIRST-30 remains authoritative for `riskTier` and `requiredResourceFilters`; managed ownership modes stay data-visible but runtime-hidden until availability is injected. +## Appendix: Notion Dry Run (MCP-Direct With DCR) + +This dry run applies the template to Notion, the first MCP-direct connector to +ship with RFC 7591 dynamic client registration (PAP-16637; server +implementation PAP-16649, PR #11009). Unlike the Linear appendix, every +endpoint and constraint below comes from a live request log, not vendor docs +alone. + +### Vendor + +- App key: `notion` +- App name: Notion +- First-30 classification: MCP-direct. Notion ships an official hosted MCP + server; its ~20 `notion-*` tools map directly to Paperclip grants. +- Reason for classification: no shim or wrapper needed — the hosted server + speaks Streamable HTTP, which `server/src/services/mcp-http.ts` already + handles. The FIRST-30 matrix's "thin wrapper for block/database policy" is + explicitly deferred; v1 enforcement is gateway policy plus filters-as-config. +- Security tier: S3 — workspace content read/write, but no payments, tenant + admin, or production infrastructure. +- Plugin needed: No. Gallery card, OAuth connect, filters, catalog, profiles, + policies, and audit cover the UX. + +### Transport And Auth + +- Transport: `mcp_remote` +- Endpoint: `https://mcp.notion.com/mcp` (Streamable HTTP; `/sse` fallback exists) +- Auth mode: OAuth, endpoints resolved by discovery (RFC 9728 → RFC 8414), + public client via RFC 7591 DCR with PKCE S256 mandatory. +- Ownership modes: `dcr` (default, zero setup) and `customer` + (env-registered classic integration via + `PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_ID/_SECRET`, which always wins when set). +- Token behavior: access tokens last ~8 h (`expires_in` authoritative). + Refresh tokens **rotate on every refresh** — the old token is invalidated + (at most 2 valid per grant) and replaying a stale one can revoke the whole + grant, so the broker persists the rotated token before publishing the new + access token and serializes refresh per connection. Absolute expiry 180 + days, inactivity expiry 30 days. `invalid_grant` on refresh is terminal: + clear tokens, require re-auth, never retry. +- Secret storage: access/refresh tokens and any DCR `client_secret` are + `company_secrets` refs; the DCR `client_id` persists on the connection and + is reused — re-registering would orphan prior grants. +- Revocation behavior: disabling or revoking the connection removes + `notion-*` tools from agent sessions and denies brokered execution on the + next gateway check. + +### Connection Flow (mandatory) + +Paperclip ID / Paperclip Connect involvement: **none — DCR is instance-local** +(PAP-14828 spec section 10 item 8.4); **cloud-hosted and self-hosted use the +same path**. The only per-instance difference is the hostname in the redirect +URI. + +Auth endpoints (exact paths, from the live discovery chain): + +| Role | Endpoint | +| --- | --- | +| MCP server | `https://mcp.notion.com/mcp` | +| Protected-resource metadata (RFC 9728) | `https://mcp.notion.com/.well-known/oauth-protected-resource/mcp` | +| AS metadata (RFC 8414) | `https://mcp.notion.com/.well-known/oauth-authorization-server` | +| Authorize | `https://mcp.notion.com/authorize` | +| Token (exchange + refresh) | `https://mcp.notion.com/token` | +| Registration (RFC 7591 DCR) | `https://mcp.notion.com/register` | +| Paperclip connect (wizard) | `POST /api/companies/:companyId/tools/apps/connect` | +| Paperclip OAuth start | `POST /api/tools/oauth/:connectionId/start` | +| Paperclip callback | `GET /api/tools/oauth/callback` | + +Redirect constraints (probed): `https-or-loopback-http`. + +```mermaid +sequenceDiagram + autonumber + actor U as User's browser + participant UI as Paperclip UI
/PAP/apps/connect?source=notion + participant S as Paperclip instance server
(cloud or self-hosted — same path) + participant M as mcp.notion.com
(MCP server + OAuth AS) + participant N as Notion web
(app.notion.com, notion.com) + + U->>UI: Click "Connect" (deep link ?source=notion) + UI->>S: POST /companies/:id/tools/apps/connect { appKey: "notion" } + S->>M: GET /.well-known/oauth-protected-resource (RFC 9728) + M-->>S: authorization_servers → mcp.notion.com + S->>M: GET /.well-known/oauth-authorization-server (RFC 8414) + M-->>S: authorize / token / registration endpoints + alt First connect on this instance (no stored client, no env client) + S->>M: POST registration_endpoint (RFC 7591 DCR, public client, PKCE-only) + M-->>S: client_id (persisted, REUSED for every later connect) + else Client already known + S->>S: Reuse stored DCR client_id (or env-registered client if configured) + end + S-->>UI: auth.startUrl (authorize URL + PKCE S256 challenge + state) + UI->>U: Redirect browser to startUrl + U->>M: GET /authorize?client_id + code_challenge + state + M->>N: 302 to app.notion.com/install-integration + N->>N: notion.com/login (only if signed out) + N-->>U: Consent page: pick workspace, approve integration + U->>S: 302 to GET /api/tools/oauth/callback?code&state (instance's OWN callback) + S->>M: POST token_endpoint (code + code_verifier) + M-->>S: access_token (~8 h) + rotating refresh_token + S->>S: Store tokens as company_secrets refs (server-side only) + S-->>U: Redirect to wizard actions/review step (?oauth=connected) + Note over S,M: Later: agent runs reach notion-* tools via the managed MCP gateway.
Server refreshes ahead of use — each refresh ROTATES the refresh token. +``` + +### Dry-Run Request Log (PAP-16649, 2026-08-06/07) + +The verified request sequence for a first connect: + +1. `GET https://mcp.notion.com/mcp` → `401` with `WWW-Authenticate` naming + `https://mcp.notion.com/.well-known/oauth-protected-resource/mcp`. +2. `GET https://mcp.notion.com/.well-known/oauth-protected-resource/mcp` → + `200`; authorization server `https://mcp.notion.com`, scope `default`. +3. `GET https://mcp.notion.com/.well-known/oauth-authorization-server` → + `200`; `/authorize`, `/token`, `/register`; `token_endpoint_auth_method` + `none` supported; PKCE `S256` supported. +4. `POST https://mcp.notion.com/register` (RFC 7591). +5. Browser `GET https://mcp.notion.com/authorize` → Notion consent + (`app.notion.com/install-integration`, `notion.com/login` if signed out). +6. `POST https://mcp.notion.com/token` for code exchange and every refresh. +7. `POST https://mcp.notion.com/mcp` for MCP traffic. + +Redirect-URI probes against `/register`: + +| Probed `redirect_uris` value | Result | +| --- | --- | +| `http://paperclip-dev:3100/api/tools/oauth/callback` | 400 `invalid_redirect_uri` — "Redirect URI must use HTTPS unless it is a loopback HTTP URI" | +| `https://paperclip-dev:3100/api/tools/oauth/callback` | Accepted — private host is fine over HTTPS | +| `http://localhost:3100/api/tools/oauth/callback` | Accepted | +| `http://127.0.0.1:3100/api/tools/oauth/callback` | Accepted | + +Hence `redirectConstraints: "https-or-loopback-http"` in `notion.json`, and +the broker's fail-fast `oauth_redirect_origin_unsupported` error for +plain-HTTP non-loopback origins. + +### Administrator Setup (mandatory) + +- What the admin must register: **nothing**. Notion's authorization server + supports RFC 7591 DCR, so the instance registers its own public client on + first connect. No Notion integration, no client credentials, no callback + registration, no Paperclip ID or Paperclip Connect involvement. +- Optional escape hatch: to use a pre-registered classic Notion integration + instead, set `PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_ID` and + `PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_SECRET`; the env client always takes + precedence (`customer` ownership). +- Instance prerequisites: the instance base URL must be HTTPS on any host or + loopback HTTP (Notion's redirect-URI rule). A plain-HTTP non-loopback origin + gets "This provider requires an HTTPS or loopback origin. Configure TLS + before connecting." — add TLS first (e.g. a tailscale cert, as + paperclip-dev did). The `enableApps` experimental setting must be on for + `/apps/*` routes. The connecting user must be allowed to install + integrations in their Notion workspace. +- How to verify: visit `/PAP/apps/connect?source=notion`, complete the Notion + consent flow, and land on the wizard's actions step listing `notion-*` + tools. Then confirm an agent run sees Notion tools through the runtime MCP + gateway and that a write call (e.g. `notion-create-pages`) opens an + ask-first action request. + +### Resource Filters + +- Required filters: workspace, page, database (per FIRST-30). +- Optional filters: object type, database/data-source scope. +- Write-enabling filters: workspace plus page/database scope for + create/update. +- Enforced by: gateway policy plus filters-as-config in v1; the FIRST-30 + "thin wrapper for block/database policy" is explicitly deferred. Notion-side + scoping also applies — the consent step lets the user share only selected + pages/databases with the integration. + +### Manifest Sketch + +The shipped `packages/shared/src/app-definitions/notion.json` (regenerate via +`pnpm connections:ingest-app-definitions`): + +```json +{ + "schemaVersion": 1, + "slug": "notion", + "name": "Notion", + "description": "Read and update pages in your Notion workspace.", + "urlPatterns": ["https://mcp.notion.com/*"], + "methods": [ + { + "key": "mcp-oauth", + "transport": "mcp_remote", + "auth": "oauth", + "ownershipModes": ["customer", "dcr"], + "defaults": { "serverUrl": "https://mcp.notion.com/mcp" }, + "riskTier": "S3", + "requiredResourceFilters": ["workspace", "page", "database"] + } + ], + "redirectConstraints": "https-or-loopback-http" +} +``` + +### Actions + +Notion's hosted server exposes ~20 `notion-*` tools. Representative risk +classes below; the full catalog review with per-tool defaults is PAP-16652 +(P4), and changed-action quarantine applies as usual. + +| Tool | Risk | Default status | Filters | Approval default | Audit fields | Negative case | +| --- | --- | --- | --- | --- | --- | --- | +| `notion-search` | read | active after catalog review; plan-gated by Notion (needs Notion AI) — may list but fail at call time | workspace | allow when profile includes Notion reads | query summary, result count | Ungranted agent cannot invoke. | +| `notion-fetch` | read | active after catalog review | workspace, page, database | allow when profile includes Notion reads | page/database id | Fetch outside shared pages fails Notion-side and is audited. | +| `notion-create-pages` | write | active only after review; changed versions quarantined | workspace, page, database | ask-first by default | parent id, title hash, created page id | Missing workspace/page filter denies. | +| `notion-update-page` | write | active only after review; changed versions quarantined | workspace, page | ask-first by default | page id, redaction summary | Revoked connection blocks retry. | +| `notion-query-data-sources` | read | active after catalog review | workspace, database | allow when profile includes Notion reads | data-source id, result count | Granted agent cannot query a disallowed database. | + +No destructive Notion action ships in the first pass; any future +delete/archive/bulk action starts quarantined pending SecurityEngineer review. + +### Wizard Path + +1. Operator opens `/PAP/apps/connect?source=notion` (or the Notion gallery + card → Connect). The deep link POSTs connect immediately and redirects the + browser to `auth.startUrl`. +2. Operator completes Notion consent (workspace picker → approve). +3. Notion redirects to the instance's own `GET /api/tools/oauth/callback`; + Paperclip exchanges the code, stores token material in `company_secrets`, + and returns the operator to the wizard (`?oauth=connected`). +4. Operator confirms resource filters and default ask-first writes. +5. Paperclip runs health check and catalog refresh; `notion-*` tools appear + on the actions step. +6. Write actions stay ask-first until the operator approves calls or creates + narrow trust rules. + +Error state: on a plain-HTTP non-loopback instance, step 1 fails fast with +the TLS guidance error above — the operator never reaches Notion. + +### Governance Defaults + +- Default profile: Notion read actions for the selected scope; writes opt-in. +- Policy defaults: ask-first for `notion-create-pages`, `notion-update-page`, + and comment writes; block any unreviewed destructive action. +- Quarantine: new or schema-changed write actions receive + `quarantineReason: "pending_review"` and are hidden from agent tool lists. +- Rate limits: per-connection search/fetch budget to protect vendor quota. +- Audit: log connect, DCR registration, config/filter changes, grant changes, + action requests, allowed/denied calls, token refresh failures, revoke, and + catalog quarantine events. + +### Validation Hook + +End-to-end evidence belongs to PAP-16654 (P6) and the PAP-12373 matrix: + +- Zero-setup OAuth connect succeeds on + `https://paperclip-dev.tail29c1aa.ts.net/PAP/apps/connect?source=notion` + with no pre-provisioned OAuth env vars (proves DCR). +- Catalog discovery lists `notion-*` tools; new/changed risky actions are + quarantined. +- An agent run sees Notion tools through the managed runtime MCP gateway. +- `notion-create-pages` opens ask-first review and executes only after + approval. +- Revocation removes Notion tools and blocks execution. +- Audit rows prove actor, run/issue context, connection, tool, decision, + reason code, and outcome. + diff --git a/packages/shared/src/app-definitions.test.ts b/packages/shared/src/app-definitions.test.ts index 8e93700c44..489f5e0595 100644 --- a/packages/shared/src/app-definitions.test.ts +++ b/packages/shared/src/app-definitions.test.ts @@ -4,9 +4,11 @@ import { appDefinitionsSchema } from "./validators/app-definition.js"; describe("AppDefinition catalog",()=>{ it("validates all Wave 1 definitions",()=>expect(()=>appDefinitionsSchema.parse(APP_DEFINITIONS)).not.toThrow()); it("contains twelve reviewed providers",()=>expect(APP_DEFINITIONS.map((app)=>app.slug)).toEqual(["zapier","github","slack","notion","linear","google-sheets","context7","oauth-generic","api-key-generic","sentry","vercel","anthropic"])); - it.each([ - ["notion",["read_content","update_content"]], - ["linear",["read","write"]], - ])("preserves required OAuth scopes for %s",(slug,scopes)=>expect(APP_DEFINITIONS.find((app)=>app.slug===slug)?.methods[0]?.defaults?.scopesHint).toEqual(scopes)); + it("uses discovery-first Notion MCP OAuth metadata",()=>{ + const notion=APP_DEFINITIONS.find((app)=>app.slug==="notion"); + expect(notion?.redirectConstraints).toBe("https-or-loopback-http"); + expect(notion?.methods[0]?.defaults).toEqual({serverUrl:"https://mcp.notion.com/mcp"}); + }); + it("preserves required Linear OAuth scopes",()=>expect(APP_DEFINITIONS.find((app)=>app.slug==="linear")?.methods[0]?.defaults?.scopesHint).toEqual(["read","write"])); it("enforces method and field invariants",()=>{for(const app of APP_DEFINITIONS)for(const method of app.methods){if(method.auth==="api_key")expect(method.keyPlacement).toBeTruthy();if(method.auth==="oauth")expect(method.ownershipModes.length).toBeGreaterThan(0);for(const field of method.credentialFields??[])if(field.required&&field.type!=="checkbox")expect(field.placeholder).toBeTruthy()}}); }); diff --git a/packages/shared/src/app-definitions/notion.json b/packages/shared/src/app-definitions/notion.json index 8e95443e8d..fe5ab22b2f 100644 --- a/packages/shared/src/app-definitions/notion.json +++ b/packages/shared/src/app-definitions/notion.json @@ -24,13 +24,7 @@ ], "whenToUse": "Use the provider-hosted connection for the quickest setup.", "defaults": { - "serverUrl": "https://mcp.notion.com/mcp", - "authorizationEndpoint": "https://api.notion.com/v1/oauth/authorize", - "tokenEndpoint": "https://api.notion.com/v1/oauth/token", - "scopesHint": [ - "read_content", - "update_content" - ] + "serverUrl": "https://mcp.notion.com/mcp" }, "guidanceMd": "Connect Notion for workspace content. Share only the pages and databases agents should use.", "riskTier": "S3", @@ -40,5 +34,6 @@ "database" ] } - ] + ], + "redirectConstraints": "https-or-loopback-http" } diff --git a/packages/shared/src/index.ts b/packages/shared/src/index.ts index cbb2b9e49b..512f54aeeb 100644 --- a/packages/shared/src/index.ts +++ b/packages/shared/src/index.ts @@ -1393,6 +1393,7 @@ export type { AppDefinition, ConnectionMethodDef, FieldDef, + OAuthRedirectConstraints, QuotaWindow, ProviderQuotaResult, } from "./types/index.js"; diff --git a/packages/shared/src/types/app-definition.ts b/packages/shared/src/types/app-definition.ts index abbc3abfc4..69a7c8606b 100644 --- a/packages/shared/src/types/app-definition.ts +++ b/packages/shared/src/types/app-definition.ts @@ -1,5 +1,6 @@ import type { ToolConnectionOwnership, ToolConnectionTransport } from "./tool-access.js"; export type AppCategory = "ai"|"analytics"|"commerce"|"communication"|"content"|"data"|"developer"|"productivity"|"other"; +export type OAuthRedirectConstraints = "https-or-loopback-http"; export interface FieldDef { key:string; label:string; type:"text"|"password"|"textarea"|"datetime"|"select"|"checkbox"; required?:boolean; placeholder?:string; helperMd?:string; secret?:boolean; prefix?:string; validation?:{pattern?:string;maxLength?:number}; options?:Array<{value:string;label:string}> } export interface ConnectionMethodDef { key:string; transport:ToolConnectionTransport; auth:"oauth"|"api_key"|"none"; ownershipModes:ToolConnectionOwnership[]; whenToUse:string; defaults?:{serverUrl?:string;discoveryUrl?:string|null;serviceHost?:string;templateKey?:string;authorizationEndpoint?:string;tokenEndpoint?:string;metadataUrl?:string;scopesHint?:string[]}; tenantFields?:FieldDef[]; extensionFields?:FieldDef[]; credentialFields?:FieldDef[]; keyPlacement?:{location:"header"|"query"|"body_json"|"env";name:string;prefix?:string|null}; guidanceMd:string; consoleLinks?:{register?:string;keys?:string;settings?:string;docs?:string}; warnings?:string[]; variants?:Array<{key:string;label:string;whenToUse:string;tenantFields?:FieldDef[]}>; riskTier:"S1"|"S2"|"S3"|"S4"; requiredResourceFilters?:string[] } -export interface AppDefinition { schemaVersion:1; slug:string; name:string; description:string; categories:AppCategory[]; featured?:boolean; branding:{logoUrl:string;darkLogoUrl?:string;backgroundColor?:string;accentColor?:string}; urlPatterns:string[]; docsUrl?:string; methods:ConnectionMethodDef[]; suggestable?:boolean; availability?:{available:boolean;reason?:string;robotEmail?:string}; ownershipAvailability?:Partial> } +export interface AppDefinition { schemaVersion:1; slug:string; name:string; description:string; categories:AppCategory[]; featured?:boolean; branding:{logoUrl:string;darkLogoUrl?:string;backgroundColor?:string;accentColor?:string}; urlPatterns:string[]; docsUrl?:string; redirectConstraints?:OAuthRedirectConstraints; methods:ConnectionMethodDef[]; suggestable?:boolean; availability?:{available:boolean;reason?:string;robotEmail?:string}; ownershipAvailability?:Partial> } diff --git a/packages/shared/src/validators/app-definition.ts b/packages/shared/src/validators/app-definition.ts index 4aa559b2ae..11ca80f0f0 100644 --- a/packages/shared/src/validators/app-definition.ts +++ b/packages/shared/src/validators/app-definition.ts @@ -2,5 +2,5 @@ import { z } from "zod"; import { toolConnectionOwnershipSchema, toolConnectionTransportSchema } from "./tool-access.js"; const field=z.object({key:z.string().min(1),label:z.string().min(1),type:z.enum(["text","password","textarea","datetime","select","checkbox"]),required:z.boolean().optional(),placeholder:z.string().optional(),helperMd:z.string().optional(),secret:z.boolean().optional(),prefix:z.string().optional()}).superRefine((v,c)=>{if(v.required&&v.type!=="checkbox"&&!v.placeholder)c.addIssue({code:"custom",message:"Required fields need placeholders",path:["placeholder"]})}); export const connectionMethodDefSchema=z.object({key:z.string().min(1),transport:toolConnectionTransportSchema,auth:z.enum(["oauth","api_key","none"]),ownershipModes:z.array(toolConnectionOwnershipSchema).min(1),whenToUse:z.string().min(1),defaults:z.object({serverUrl:z.string().url().optional(),discoveryUrl:z.string().url().nullable().optional(),serviceHost:z.string().optional(),templateKey:z.string().optional(),authorizationEndpoint:z.string().url().optional(),tokenEndpoint:z.string().url().optional(),metadataUrl:z.string().url().optional(),scopesHint:z.array(z.string()).optional()}).optional(),tenantFields:z.array(field).optional(),extensionFields:z.array(field).optional(),credentialFields:z.array(field).optional(),keyPlacement:z.object({location:z.enum(["header","query","body_json","env"]),name:z.string().min(1),prefix:z.string().nullable().optional()}).optional(),guidanceMd:z.string().min(1),consoleLinks:z.object({register:z.string().url().optional(),keys:z.string().url().optional(),settings:z.string().url().optional(),docs:z.string().url().optional()}).optional(),warnings:z.array(z.string()).optional(),variants:z.array(z.object({key:z.string(),label:z.string(),whenToUse:z.string(),tenantFields:z.array(field).optional()})).optional(),riskTier:z.enum(["S1","S2","S3","S4"]),requiredResourceFilters:z.array(z.string()).optional()}).superRefine((v,c)=>{if(v.auth==="api_key"&&!v.keyPlacement)c.addIssue({code:"custom",message:"API-key methods require keyPlacement",path:["keyPlacement"]})}); -export const appDefinitionSchema=z.object({schemaVersion:z.literal(1),slug:z.string().regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/),name:z.string().min(1),description:z.string().min(1),categories:z.array(z.enum(["ai","analytics","commerce","communication","content","data","developer","productivity","other"])).min(1),featured:z.boolean().optional(),branding:z.object({logoUrl:z.string().url(),darkLogoUrl:z.string().url().optional(),backgroundColor:z.string().optional(),accentColor:z.string().optional()}),urlPatterns:z.array(z.string()),docsUrl:z.string().url().optional(),methods:z.array(connectionMethodDefSchema).min(1),suggestable:z.boolean().optional(),availability:z.object({available:z.boolean(),reason:z.string().optional(),robotEmail:z.string().optional()}).optional(),ownershipAvailability:z.object({platform_shared:z.boolean().optional(),platform_provisioned:z.boolean().optional(),customer:z.boolean().optional(),dcr:z.boolean().optional()}).optional()}); +export const appDefinitionSchema=z.object({schemaVersion:z.literal(1),slug:z.string().regex(/^[a-z0-9]+(?:-[a-z0-9]+)*$/),name:z.string().min(1),description:z.string().min(1),categories:z.array(z.enum(["ai","analytics","commerce","communication","content","data","developer","productivity","other"])).min(1),featured:z.boolean().optional(),branding:z.object({logoUrl:z.string().url(),darkLogoUrl:z.string().url().optional(),backgroundColor:z.string().optional(),accentColor:z.string().optional()}),urlPatterns:z.array(z.string()),docsUrl:z.string().url().optional(),redirectConstraints:z.enum(["https-or-loopback-http"]).optional(),methods:z.array(connectionMethodDefSchema).min(1),suggestable:z.boolean().optional(),availability:z.object({available:z.boolean(),reason:z.string().optional(),robotEmail:z.string().optional()}).optional(),ownershipAvailability:z.object({platform_shared:z.boolean().optional(),platform_provisioned:z.boolean().optional(),customer:z.boolean().optional(),dcr:z.boolean().optional()}).optional()}); export const appDefinitionsSchema=z.array(appDefinitionSchema).superRefine((v,c)=>{const s=new Set();v.forEach((a,i)=>{if(s.has(a.slug))c.addIssue({code:"custom",message:"Duplicate slug",path:[i,"slug"]});s.add(a.slug)})}); diff --git a/scripts/ingest-app-definitions.mjs b/scripts/ingest-app-definitions.mjs index 498a37594f..05fc773ff0 100644 --- a/scripts/ingest-app-definitions.mjs +++ b/scripts/ingest-app-definitions.mjs @@ -7,7 +7,7 @@ const apps=[ ["zapier","Zapier","Reach thousands of apps through your Zapier account.","productivity","zapier.com",["https://mcp.zapier.com/*"],method("mcp-key","mcp_remote","api_key",{serverUrl:"https://mcp.zapier.com/api/mcp"},"S3","Create a Zapier MCP connection, then paste its token here.",{credentialFields:[field("authorization","Zapier MCP token","Paste your Zapier token")],keyPlacement:{location:"header",name:"Authorization",prefix:"Bearer "}})], ["github","GitHub","Read code and pull requests, and coordinate repository work.","developer","github.com",["https://api.githubcopilot.com/mcp/*"],method("mcp-key","mcp_remote","api_key",{serverUrl:"https://api.githubcopilot.com/mcp/"},"S3","Create a fine-grained token limited to the repositories agents should use.",{credentialFields:[field("authorization","GitHub token","github_pat_...")],keyPlacement:{location:"header",name:"Authorization",prefix:"Bearer "},requiredResourceFilters:["organization","repository"]})], ["slack","Slack","Search channels and coordinate team communication.","communication","slack.com",["https://mcp.slack.com/*"],method("mcp-oauth","mcp_remote","oauth",{serverUrl:"https://mcp.slack.com/mcp",authorizationEndpoint:"https://slack.com/oauth/v2/authorize",tokenEndpoint:"https://slack.com/api/oauth.v2.access",scopesHint:["channels:read","chat:write","search:read"]},"S3","Connect a Slack workspace and limit access to the channels agents need.",{requiredResourceFilters:["workspace","channel"]})], -["notion","Notion","Read and update pages in your Notion workspace.","content","notion.so",["https://mcp.notion.com/*"],method("mcp-oauth","mcp_remote","oauth",{serverUrl:"https://mcp.notion.com/mcp",authorizationEndpoint:"https://api.notion.com/v1/oauth/authorize",tokenEndpoint:"https://api.notion.com/v1/oauth/token",scopesHint:["read_content","update_content"]},"S3","Connect Notion for workspace content. Share only the pages and databases agents should use.",{requiredResourceFilters:["workspace","page","database"]})], +["notion","Notion","Read and update pages in your Notion workspace.","content","notion.so",["https://mcp.notion.com/*"],method("mcp-oauth","mcp_remote","oauth",{serverUrl:"https://mcp.notion.com/mcp"},"S3","Connect Notion for workspace content. Share only the pages and databases agents should use.",{requiredResourceFilters:["workspace","page","database"]}),{redirectConstraints:"https-or-loopback-http"}], ["linear","Linear","Create, update, and read Linear issues.","productivity","linear.app",["https://mcp.linear.app/*"],method("mcp-oauth","mcp_remote","oauth",{serverUrl:"https://mcp.linear.app/mcp",authorizationEndpoint:"https://linear.app/oauth/authorize",tokenEndpoint:"https://api.linear.app/oauth/token",scopesHint:["read","write"]},"S2","Register a Linear OAuth app and add Paperclip's redirect URI before connecting.",{requiredResourceFilters:["workspace","team","project"]})], ["google-sheets","Google Sheets","Read and update selected spreadsheets.","data","sheets.google.com",["https://docs.google.com/spreadsheets/*","https://sheets.google.com/*"],method("local","local_stdio","none",{templateKey:"paperclip.google-sheets"},"S3","Share each spreadsheet with the Paperclip robot email, then paste the sheet links.",{requiredResourceFilters:["spreadsheet"]})], ["context7","Context7","Look up current documentation for software libraries.","developer","context7.com",["https://mcp.context7.com/*"],method("mcp","mcp_remote","none",{serverUrl:"https://mcp.context7.com/mcp"},"S1","Connect Context7 to give agents current library documentation.")], @@ -16,7 +16,7 @@ const apps=[ ["sentry","Sentry","Investigate errors, releases, and production issues.","developer","sentry.io",["https://mcp.sentry.dev/*"],method("mcp-oauth","mcp_remote","oauth",{serverUrl:"https://mcp.sentry.dev/mcp",discoveryUrl:"https://sentry.io/.well-known/oauth-authorization-server"},"S2","Connect the Sentry organization and projects agents need for incident work.",{requiredResourceFilters:["organization","project","environment"]})], ["vercel","Vercel","Inspect projects, deployments, and runtime logs.","developer","vercel.com",["https://mcp.vercel.com/*"],method("mcp-oauth","mcp_remote","oauth",{serverUrl:"https://mcp.vercel.com/mcp"},"S3","Connect the Vercel team and projects agents should operate.",{requiredResourceFilters:["team","project","environment"]})], ["anthropic","Anthropic","Use Anthropic APIs with a restricted key.","ai","anthropic.com",["https://api.anthropic.com/*"],method("api-key","rest_api","api_key",{serviceHost:"api.anthropic.com"},"S3","Create a key in the Anthropic Console and rotate it if it has been exposed.",{credentialFields:[field("apiKey","API key","sk-ant-api03-...")],keyPlacement:{location:"header",name:"x-api-key"}})], -].map(([slug,name,description,category,domain,urlPatterns,m])=>({schemaVersion:1,slug,name,description,categories:[category],featured:["zapier","github","slack","notion","linear"].includes(slug),branding:{logoUrl:favicon(domain)},urlPatterns,methods:[m]})); +].map(([slug,name,description,category,domain,urlPatterns,m,extra={}])=>({schemaVersion:1,slug,name,description,categories:[category],featured:["zapier","github","slack","notion","linear"].includes(slug),branding:{logoUrl:favicon(domain)},urlPatterns,methods:[m],...extra})); const parseTableRow=(line)=>line.slice(1,-1).split("|").map((cell)=>cell.trim()); const parseCapture=(fileName)=>{ const markdown=fs.readFileSync(path.join(corpus,fileName),"utf8"); diff --git a/scripts/screenshot-notion-connect-flow.mjs b/scripts/screenshot-notion-connect-flow.mjs new file mode 100644 index 0000000000..bfa11f819c --- /dev/null +++ b/scripts/screenshot-notion-connect-flow.mjs @@ -0,0 +1,96 @@ +#!/usr/bin/env node +// Capture the PAP-16650 Notion connection-flow Storybook stories. +// Usage: node scripts/screenshot-notion-connect-flow.mjs + +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 ", + ); + 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); +}); diff --git a/server/src/__tests__/tool-access-service.test.ts b/server/src/__tests__/tool-access-service.test.ts index d3e8f799a4..7a938d8356 100644 --- a/server/src/__tests__/tool-access-service.test.ts +++ b/server/src/__tests__/tool-access-service.test.ts @@ -3240,11 +3240,214 @@ describeEmbeddedPostgres("tool access service", () => { await expect(db.select().from(toolOauthStates)).resolves.toHaveLength(0); }); - it("refreshes expired OAuth access tokens before remote app calls", async () => { + it("discovers Notion MCP OAuth metadata, registers one public client, and reuses it", async () => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_SECRET", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_SECRET", ""); + const company = await createCompany(db); + const service = toolAccessService(db); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "notion", + name: "Notion DCR", + }); + const redirectUri = "https://paperclip-dev.tail29c1aa.ts.net/api/tools/oauth/callback"; + const registrationBodies: Array> = []; + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => { + const href = String(url); + if (href === "https://mcp.notion.com/.well-known/oauth-protected-resource/mcp") { + return mcpHttpResponse({ + authorization_servers: ["https://mcp.notion.com"], + scopes_supported: ["default"], + }); + } + if (href === "https://mcp.notion.com/.well-known/oauth-authorization-server") { + return mcpHttpResponse({ + issuer: "https://mcp.notion.com", + authorization_endpoint: "https://mcp.notion.com/authorize", + token_endpoint: "https://mcp.notion.com/token", + registration_endpoint: "https://mcp.notion.com/register", + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + }); + } + if (href === "https://mcp.notion.com/register") { + expect(init?.method).toBe("POST"); + registrationBodies.push(JSON.parse(String(init?.body)) as Record); + return mcpHttpResponse({ + client_id: "notion-dcr-client", + client_secret: "notion-dcr-secret", + redirect_uris: [redirectUri], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }); + } + throw new Error(`unexpected fetch ${href}`); + }); + + const [first, concurrent] = await Promise.all([ + service.startOAuth(company.id, connected.connectionId, { + redirectUri, + actor: { actorType: "user", actorId: "board" }, + }), + service.startOAuth(company.id, connected.connectionId, { + redirectUri, + actor: { actorType: "user", actorId: "board" }, + }), + ]); + + expect(new URL(first.authorizationUrl).origin).toBe("https://mcp.notion.com"); + expect(new URL(concurrent.authorizationUrl).searchParams.get("client_id")).toBe("notion-dcr-client"); + expect(registrationBodies).toEqual([{ + client_name: "Paperclip (paperclip-dev.tail29c1aa.ts.net)", + redirect_uris: [redirectUri], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }]); + + fetchMock.mockClear(); + const reused = await service.startOAuth(company.id, connected.connectionId, { + redirectUri, + actor: { actorType: "user", actorId: "board" }, + }); + expect(new URL(reused.authorizationUrl).searchParams.get("client_id")).toBe("notion-dcr-client"); + expect(fetchMock).not.toHaveBeenCalled(); + + const [connection] = await db.select().from(toolConnections).where(eq(toolConnections.id, connected.connectionId)); + expect(connection).toMatchObject({ ownership: "dcr" }); + expect(connection.config).toMatchObject({ + oauth: { + provider: "notion", + clientId: "notion-dcr-client", + clientRegistrationSource: "dcr", + clientTokenEndpointAuthMethod: "none", + clientRedirectUri: redirectUri, + registrationUrl: "https://mcp.notion.com/register", + }, + }); + expect(connection.credentialSecretRefs).toEqual([ + expect.objectContaining({ configPath: "oauth.client_secret", required: false }), + ]); + expect(JSON.stringify(connection.config)).not.toContain("notion-dcr-secret"); + }); + + it.each([ + [ + "a confidential token endpoint auth method", + { client_id: "notion-dcr-client", token_endpoint_auth_method: "client_secret_basic" }, + "token_endpoint_auth_method", + ], + [ + "a different redirect URI", + { client_id: "notion-dcr-client", redirect_uris: ["https://attacker.example/callback"] }, + "redirect_uris", + ], + [ + "a reduced grant set", + { client_id: "notion-dcr-client", grant_types: ["authorization_code"] }, + "grant_types", + ], + [ + "missing response types", + { client_id: "notion-dcr-client", response_types: undefined }, + "response_types", + ], + [ + "an oversized client id", + { client_id: "x".repeat(4_097) }, + "client_id", + ], + ])("rejects DCR responses that return %s", async (_label, registrationResponse, field) => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_SECRET", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_SECRET", ""); + const company = await createCompany(db); + const service = toolAccessService(db); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "notion", + name: `Notion invalid DCR ${field}`, + }); + const redirectUri = "https://paperclip-dev.tail29c1aa.ts.net/api/tools/oauth/callback"; + vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const href = String(url); + if (href === "https://mcp.notion.com/.well-known/oauth-protected-resource/mcp") { + return mcpHttpResponse({ authorization_servers: ["https://mcp.notion.com"] }); + } + if (href === "https://mcp.notion.com/.well-known/oauth-authorization-server") { + return mcpHttpResponse({ + authorization_endpoint: "https://mcp.notion.com/authorize", + token_endpoint: "https://mcp.notion.com/token", + registration_endpoint: "https://mcp.notion.com/register", + code_challenge_methods_supported: ["S256"], + token_endpoint_auth_methods_supported: ["none"], + }); + } + if (href === "https://mcp.notion.com/register") { + return mcpHttpResponse({ + client_id: "notion-dcr-client", + redirect_uris: [redirectUri], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + ...registrationResponse, + }); + } + throw new Error(`unexpected fetch ${href}`); + }); + + await expect(service.startOAuth(company.id, connected.connectionId, { + redirectUri, + actor: { actorType: "user", actorId: "board" }, + })).rejects.toMatchObject({ + status: 502, + details: { + code: "oauth_dcr_response_invalid", + field, + }, + }); + + const [connection] = await db + .select() + .from(toolConnections) + .where(eq(toolConnections.id, connected.connectionId)); + expect(connection.ownership).not.toBe("dcr"); + expect((connection.config.oauth as Record | undefined)?.clientId).toBeUndefined(); + }); + + it("fails fast when Notion DCR is attempted from a non-loopback HTTP origin", async () => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_NOTION_CLIENT_ID", ""); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_CLIENT_ID", ""); + const company = await createCompany(db); + const service = toolAccessService(db); + const connected = await service.connectGalleryApp(company.id, { + galleryKey: "notion", + name: "Notion invalid origin", + }); + const fetchMock = vi.spyOn(globalThis, "fetch"); + + await expect(service.startOAuth(company.id, connected.connectionId, { + redirectUri: "http://paperclip-dev:3100/api/tools/oauth/callback", + actor: { actorType: "user", actorId: "board" }, + })).rejects.toMatchObject({ + status: 422, + message: "This provider requires an HTTPS or loopback origin. Configure TLS before connecting.", + details: expect.objectContaining({ + code: "oauth_redirect_origin_unsupported", + docsPath: "docs/deploy", + }), + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + + it("leases rotating OAuth refresh tokens across service instances before concurrent remote app calls", async () => { vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_SECRET", "slack-client-secret"); const company = await createCompany(db); const service = toolAccessService(db); + const concurrentService = toolAccessService(db); const connect = await service.connectGalleryApp(company.id, { galleryKey: "slack", name: "Slack refresh" }); const start = await service.startOAuth(company.id, connect.connectionId, { @@ -3252,6 +3455,7 @@ describeEmbeddedPostgres("tool access service", () => { actor: { actorType: "user", actorId: "board" }, }); const state = new URL(start.authorizationUrl).searchParams.get("state")!; + let refreshCallCount = 0; vi.spyOn(globalThis, "fetch").mockImplementation(async (url, init) => { const href = String(url); if (href === "https://slack.com/api/oauth.v2.access") { @@ -3270,6 +3474,8 @@ describeEmbeddedPostgres("tool access service", () => { } expect(body.get("grant_type")).toBe("refresh_token"); expect(body.get("refresh_token")).toBe("refresh-token"); + refreshCallCount += 1; + await new Promise((resolve) => setTimeout(resolve, 10)); return { ok: true, json: async () => ({ @@ -3311,9 +3517,14 @@ describeEmbeddedPostgres("tool access service", () => { }) .where(eq(toolConnections.id, connect.connectionId)); - const health = await service.checkHealth(connect.connectionId); + const [health, concurrentHealth] = await Promise.all([ + service.checkHealth(connect.connectionId), + concurrentService.checkHealth(connect.connectionId), + ]); expect(health.connection.healthStatus).toBe("ok"); + expect(concurrentHealth.connection.healthStatus).toBe("ok"); + expect(refreshCallCount).toBe(1); const fetchCalls = vi.mocked(globalThis.fetch).mock.calls; const mcpCalls = fetchCalls.filter(([url]) => String(url) === "https://mcp.slack.com/mcp"); expect(mcpCalls.at(-1)?.[1]?.headers).toEqual(expect.objectContaining({ Authorization: "Bearer new-access-token" })); @@ -3336,6 +3547,231 @@ describeEmbeddedPostgres("tool access service", () => { ])); }); + it("treats invalid_grant as terminal without replaying a rotated refresh token", async () => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_SECRET", "slack-client-secret"); + const company = await createCompany(db); + const service = toolAccessService(db); + const connect = await service.connectGalleryApp(company.id, { galleryKey: "slack", name: "Slack invalid grant" }); + const start = await service.startOAuth(company.id, connect.connectionId, { + redirectUri: "http://paperclip.test/api/tools/oauth/callback", + actor: { actorType: "user", actorId: "board" }, + }); + const state = new URL(start.authorizationUrl).searchParams.get("state")!; + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const href = String(url); + if (href === "https://slack.com/api/oauth.v2.access") { + return mcpHttpResponse({ + ok: true, + access_token: "expired-access-token", + refresh_token: "single-use-refresh-token", + expires_in: 3600, + token_type: "Bearer", + }); + } + if (href === "https://mcp.slack.com/mcp") { + return mcpHttpResponse({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", result: { tools: [] } }); + } + throw new Error(`unexpected fetch ${href}`); + }); + await service.completeOAuthCallback({ + state, + code: "oauth-code", + redirectUri: "http://paperclip.test/api/tools/oauth/callback", + actor: { actorType: "user", actorId: "board" }, + }); + const [connected] = await db.select().from(toolConnections).where(eq(toolConnections.id, connect.connectionId)); + await db.update(toolConnections).set({ + config: { + ...connected.config, + oauth: { + ...(connected.config.oauth as Record), + expiresAt: "2000-01-01T00:00:00.000Z", + }, + }, + }).where(eq(toolConnections.id, connect.connectionId)); + + let refreshCallCount = 0; + fetchMock.mockImplementation(async (url, init) => { + const href = String(url); + if (href === "https://slack.com/api/oauth.v2.access") { + const body = init?.body as URLSearchParams; + expect(body.get("grant_type")).toBe("refresh_token"); + expect(body.get("refresh_token")).toBe("single-use-refresh-token"); + refreshCallCount += 1; + await new Promise((resolve) => setTimeout(resolve, 10)); + return { + ok: false, + status: 400, + headers: { get: () => null }, + json: async () => ({ error: "invalid_grant", error_description: "Refresh token was already used" }), + } as unknown as Response; + } + throw new Error(`unexpected fetch ${href}`); + }); + + const results = await Promise.allSettled([ + service.checkHealth(connect.connectionId), + service.checkHealth(connect.connectionId), + ]); + expect(results).toEqual([ + expect.objectContaining({ status: "rejected" }), + expect.objectContaining({ status: "rejected" }), + ]); + for (const result of results) { + expect(result.status === "rejected" ? result.reason : null).toMatchObject({ + details: expect.objectContaining({ code: "oauth_reauthorization_required" }), + }); + } + expect(refreshCallCount).toBe(1); + const [reauthorizationRequired] = await db + .select() + .from(toolConnections) + .where(eq(toolConnections.id, connect.connectionId)); + expect(reauthorizationRequired).toMatchObject({ + status: "draft", + enabled: false, + healthStatus: "error", + }); + expect(reauthorizationRequired.credentialSecretRefs.map((ref) => ref.configPath)).not.toContain("oauth.access_token"); + expect(reauthorizationRequired.credentialSecretRefs.map((ref) => ref.configPath)).not.toContain("oauth.refresh_token"); + expect(reauthorizationRequired.credentialRefs.map((ref) => ref.name)).not.toContain("oauth.access_token"); + }); + + it("does not disable a connection when invalid_grant used a superseded refresh-token version", async () => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_SECRET", "slack-client-secret"); + const company = await createCompany(db); + const service = toolAccessService(db); + const connect = await service.connectGalleryApp(company.id, { + galleryKey: "slack", + name: "Slack stale invalid grant", + }); + const start = await service.startOAuth(company.id, connect.connectionId, { + redirectUri: "http://paperclip.test/api/tools/oauth/callback", + actor: { actorType: "user", actorId: "board" }, + }); + const state = new URL(start.authorizationUrl).searchParams.get("state")!; + const fetchMock = vi.spyOn(globalThis, "fetch").mockImplementation(async (url) => { + const href = String(url); + if (href === "https://slack.com/api/oauth.v2.access") { + return mcpHttpResponse({ + ok: true, + access_token: "expired-access-token", + refresh_token: "submitted-refresh-token", + expires_in: 3600, + token_type: "Bearer", + }); + } + if (href === "https://mcp.slack.com/mcp") { + return mcpHttpResponse({ jsonrpc: "2.0", id: "paperclip-catalog-refresh", result: { tools: [] } }); + } + throw new Error(`unexpected fetch ${href}`); + }); + await service.completeOAuthCallback({ + state, + code: "oauth-code", + redirectUri: "http://paperclip.test/api/tools/oauth/callback", + actor: { actorType: "user", actorId: "board" }, + }); + const [connected] = await db + .select() + .from(toolConnections) + .where(eq(toolConnections.id, connect.connectionId)); + const refreshRef = connected.credentialSecretRefs.find((ref) => ref.configPath === "oauth.refresh_token")!; + await db.update(toolConnections).set({ + config: { + ...connected.config, + oauth: { + ...(connected.config.oauth as Record), + expiresAt: "2000-01-01T00:00:00.000Z", + }, + }, + }).where(eq(toolConnections.id, connect.connectionId)); + + fetchMock.mockImplementation(async (url, init) => { + const href = String(url); + if (href === "https://slack.com/api/oauth.v2.access") { + const body = init?.body as URLSearchParams; + expect(body.get("refresh_token")).toBe("submitted-refresh-token"); + await secretService(db).rotate(refreshRef.secretId, { value: "newer-refresh-token" }); + return { + ok: false, + status: 400, + headers: { get: () => null }, + json: async () => ({ error: "invalid_grant" }), + } as unknown as Response; + } + throw new Error(`unexpected fetch ${href}`); + }); + + await expect(service.checkHealth(connect.connectionId)).rejects.toMatchObject({ + status: 502, + details: expect.objectContaining({ code: "oauth_refresh_superseded" }), + }); + const [preserved] = await db + .select() + .from(toolConnections) + .where(eq(toolConnections.id, connect.connectionId)); + expect(preserved).toMatchObject({ status: "active", enabled: false }); + expect(preserved.credentialSecretRefs.map((ref) => ref.configPath)).toEqual(expect.arrayContaining([ + "oauth.access_token", + "oauth.refresh_token", + ])); + expect((preserved.config.oauth as Record).refreshLease).toBeUndefined(); + }); + + it("fails closed instead of replaying a refresh token after an abandoned lease", async () => { + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_ID", "slack-client-id"); + vi.stubEnv("PAPERCLIP_TOOL_OAUTH_SLACK_CLIENT_SECRET", "slack-client-secret"); + const company = await createCompany(db); + const service = toolAccessService(db); + const fixture = await createOAuthConnection(db, company.id); + const refreshSecret = await secretService(db).create(company.id, { + provider: "local_encrypted", + name: `OAuth refresh ${randomUUID()}`, + key: `oauth.refresh.${randomUUID()}`, + value: "possibly-consumed-refresh-token", + }); + await db.insert(companySecretBindings).values({ + companyId: company.id, + secretId: refreshSecret.id, + targetType: "tool_connection", + targetId: fixture.connection.id, + configPath: "oauth.refresh_token", + }); + await db.update(toolConnections).set({ + config: { + ...fixture.connection.config, + oauth: { + ...(fixture.connection.config.oauth as Record), + expiresAt: "2000-01-01T00:00:00.000Z", + refreshLease: { + id: "abandoned-refresh", + expiresAt: "2000-01-01T00:00:00.000Z", + }, + }, + }, + credentialSecretRefs: [ + ...fixture.connection.credentialSecretRefs, + { + secretId: refreshSecret.id, + versionSelector: "latest", + configPath: "oauth.refresh_token", + required: false, + label: "OAuth refresh token", + }, + ], + }).where(eq(toolConnections.id, fixture.connection.id)); + const fetchMock = vi.spyOn(globalThis, "fetch"); + + await expect(service.checkHealth(fixture.connection.id)).rejects.toMatchObject({ + status: 502, + details: expect.objectContaining({ code: "oauth_refresh_outcome_unknown" }), + }); + expect(fetchMock).not.toHaveBeenCalled(); + }); + it("uses OAuth client credentials for shared machine-to-machine MCP connections", async () => { vi.stubEnv("PAPERCLIP_TOOL_OAUTH_M2M_CLIENT_ID", "m2m-client-id"); vi.stubEnv("PAPERCLIP_TOOL_OAUTH_M2M_CLIENT_SECRET", "m2m-client-secret"); @@ -4650,6 +5086,99 @@ describeEmbeddedPostgres("tool access service", () => { }); }); + it("resolves Notion reads as allowed, mutations as ask-first, and denies cross-company use", async () => { + const company = await createCompany(db); + const otherCompany = await createCompany(db); + const agent = await createAgent(db, company.id); + const service = toolAccessService(db); + const [application] = await db.insert(toolApplications).values({ + companyId: company.id, + applicationKey: `app-gallery:notion:${randomUUID()}`, + name: "Notion workspace", + type: "mcp_http", + status: "draft", + metadata: { sourceTemplateKey: "notion", galleryKey: "notion" }, + }).returning(); + const [connection] = await db.insert(toolConnections).values({ + companyId: company.id, + applicationId: application.id, + name: "Notion workspace", + uid: `notion/${randomUUID()}`, + transport: "mcp_remote", + authKind: "oauth", + status: "draft", + enabled: false, + config: { + url: "https://mcp.notion.com/mcp", + sourceTemplateKey: "notion", + quarantineNewEntries: true, + }, + transportConfig: { url: "https://mcp.notion.com/mcp" }, + healthStatus: "ok", + }).returning(); + mockToolsList([ + { name: "notion-fetch", annotations: { readOnlyHint: true } }, + // These two mutations do not contain a generic create/update verb. + { name: "notion-move-pages" }, + { name: "notion-duplicate-page", annotations: { readOnlyHint: true } }, + ]); + + const refresh = await service.refreshCatalog(connection.id); + const fetchEntry = refresh.catalog.find((entry) => entry.toolName === "notion-fetch")!; + const moveEntry = refresh.catalog.find((entry) => entry.toolName === "notion-move-pages")!; + const duplicateEntry = refresh.catalog.find((entry) => entry.toolName === "notion-duplicate-page")!; + expect(refresh.catalog).toEqual(expect.arrayContaining([ + expect.objectContaining({ id: fetchEntry.id, riskLevel: "read", isReadOnly: true }), + expect.objectContaining({ id: moveEntry.id, riskLevel: "write", isWrite: true }), + expect.objectContaining({ id: duplicateEntry.id, riskLevel: "write", isWrite: true }), + ])); + + await expect(service.finishGalleryAppConnection(otherCompany.id, connection.id, { + enabledCatalogEntryIds: [fetchEntry.id], + askFirstCatalogEntryIds: [], + access: "all_agents", + })).rejects.toMatchObject({ status: 404 }); + + await service.finishGalleryAppConnection(company.id, connection.id, { + enabledCatalogEntryIds: [fetchEntry.id, moveEntry.id, duplicateEntry.id], + askFirstCatalogEntryIds: [moveEntry.id, duplicateEntry.id], + access: "all_agents", + }); + + const policyService = toolAccessPolicyService(db); + const decide = (catalogEntryId: string, toolName: string) => policyService.decide({ + companyId: company.id, + actor: { actorType: "agent", actorId: agent.id }, + request: { connectionId: connection.id, catalogEntryId, toolName, arguments: {} }, + }); + await expect(decide(fetchEntry.id, fetchEntry.toolName)).resolves.toMatchObject({ + decision: "allow", + reasonCode: "allow_profile", + }); + await expect(decide(moveEntry.id, moveEntry.toolName)).resolves.toMatchObject({ + decision: "require_approval", + reasonCode: "requires_approval_policy", + }); + await expect(decide(duplicateEntry.id, duplicateEntry.toolName)).resolves.toMatchObject({ + decision: "require_approval", + reasonCode: "requires_approval_policy", + }); + + await expect(policyService.decide({ + companyId: otherCompany.id, + actor: { actorType: "user", actorId: "other-board" }, + request: { + connectionId: connection.id, + catalogEntryId: fetchEntry.id, + toolName: fetchEntry.toolName, + arguments: {}, + }, + })).resolves.toMatchObject({ + decision: "deny", + reasonCode: "deny_missing_tool", + }); + }); + it("rolls back gallery app finish when a later write fails after clearing profile state", async () => { const company = await createCompany(db); const service = toolAccessService(db); @@ -6348,4 +6877,40 @@ describe("classifyRisk", () => { expect(risk("list_items", { writeHint: true })).toBe("write"); expect(risk("list_items", { readOnlyHint: false })).toBe("write"); }); + + it("classifies the reviewed Notion MCP catalog with provider-scoped defaults", () => { + const notionRisk = (name: string, annotations?: Record) => + classifyRisk({ name, annotations }, "notion"); + const readTools = [ + "notion-search", + "notion-fetch", + "notion-query-data-sources", + "notion-query-database-view", + "notion-query-meeting-notes", + "notion-get-comments", + "notion-get-teams", + "notion-get-users", + "notion-get-async-task", + ]; + const writeTools = [ + "notion-create-pages", + "notion-update-page", + "notion-convert-page-to-skill", + "notion-move-pages", + "notion-duplicate-page", + "notion-create-database", + "notion-create-folder", + "notion-update-data-source", + "notion-create-view", + "notion-update-view", + "notion-create-comment", + ]; + + for (const toolName of readTools) expect(notionRisk(toolName)).toBe("read"); + for (const toolName of writeTools) { + expect(notionRisk(toolName, { readOnlyHint: true })).toBe("write"); + } + expect(notionRisk("notion-delete-page")).toBe("destructive"); + expect(classifyRisk({ name: "move_pages" })).toBe("read"); + }); }); diff --git a/server/src/services/tool-access.ts b/server/src/services/tool-access.ts index 0abcf38019..a399f9fba8 100644 --- a/server/src/services/tool-access.ts +++ b/server/src/services/tool-access.ts @@ -128,16 +128,42 @@ type ActorInfo = { const ACTIVE_BROKER_RUN_STATUSES = new Set(["running"]); const REMOTE_HTTP_REDIRECT_STATUSES = new Set([301, 302, 303, 307, 308]); const MAX_REMOTE_HTTP_REDIRECTS = 5; +const MAX_OAUTH_DCR_CLIENT_ID_LENGTH = 4_096; +const MAX_OAUTH_DCR_CLIENT_SECRET_LENGTH = 16_384; +const OAUTH_REFRESH_LEASE_MS = 120_000; +const OAUTH_REFRESH_LEASE_WAIT_MS = 30_000; +const OAUTH_REFRESH_LEASE_POLL_MS = 25; type OAuthProviderEndpoints = { provider: string; scopes: string[]; authorizationUrl: string; tokenUrl: string; + registrationUrl?: string | null; + codeChallengeMethodsSupported?: string[]; + tokenEndpointAuthMethodsSupported?: string[]; grantType?: "authorization_code" | "client_credentials"; metadataUrl?: string | null; }; +const oauthRegistrationFlights = new Map>(); + +async function oauthSingleFlight( + flights: Map>, + key: string, + operation: () => Promise, +): Promise { + const existing = flights.get(key) as Promise | undefined; + if (existing) return existing; + const pending = operation(); + flights.set(key, pending); + try { + return await pending; + } finally { + if (flights.get(key) === pending) flights.delete(key); + } +} + type ToolAccessServiceOptions = { deploymentMode?: DeploymentMode; deploymentExposure?: DeploymentExposure; @@ -1210,23 +1236,63 @@ function verbMatches(toolName: string, verbs: string): boolean { return new RegExp(`\\b(${verbs})\\b|(^|[:._-])(${verbs})([:._-]|$)`).test(normalized); } -export function classifyRisk(tool: McpToolDescriptor): ToolRiskLevel { +const NOTION_READ_TOOLS = new Set([ + "notion-fetch", + "notion-get-async-task", + "notion-get-comments", + "notion-get-teams", + "notion-get-users", + "notion-query-data-sources", + "notion-query-database-view", + "notion-query-meeting-notes", + "notion-search", +]); + +const NOTION_WRITE_TOOLS = new Set([ + "notion-convert-page-to-skill", + "notion-create-comment", + "notion-create-database", + "notion-create-folder", + "notion-create-pages", + "notion-create-view", + "notion-duplicate-page", + "notion-move-pages", + "notion-update-data-source", + "notion-update-page", + "notion-update-view", +]); + +function normalizedProviderToolName(toolName: string): string { + return toolName + .replace(/([a-z0-9])([A-Z])/g, "$1-$2") + .toLowerCase() + .replace(/[:._-]+/g, "-"); +} + +export function classifyRisk(tool: McpToolDescriptor, sourceTemplateKey?: string | null): ToolRiskLevel { const annotations = tool.annotations ?? {}; if (annotations.destructiveHint === true || annotations.destructive === true) return "destructive"; + const normalizedToolName = normalizedProviderToolName(tool.name); + // Notion's hosted MCP catalog contains mutations whose names do not use one + // of the generic create/update/delete verbs (move, duplicate, and convert). + // Keep all reviewed tools explicit so provider changes are visible in code, + // while an annotation may still escalate a known read to a write. + if (sourceTemplateKey === "notion" && NOTION_WRITE_TOOLS.has(normalizedToolName)) return "write"; if (annotations.readOnlyHint === false || annotations.writeHint === true) return "write"; + if (sourceTemplateKey === "notion" && NOTION_READ_TOOLS.has(normalizedToolName)) return "read"; if (verbMatches(tool.name, "delete|remove|destroy|unpublish")) return "destructive"; if (verbMatches(tool.name, "create|update|write|set|send|publish|post|mutate|mark|archive")) return "write"; return "read"; } -function descriptorHash(tool: McpToolDescriptor): string { +function descriptorHash(tool: McpToolDescriptor, riskLevel: ToolRiskLevel): string { return stableHash({ name: tool.name, title: tool.title ?? null, description: tool.description ?? null, inputSchema: tool.inputSchema ?? {}, annotations: tool.annotations ?? {}, - riskLevel: classifyRisk(tool), + riskLevel, }); } @@ -1247,6 +1313,24 @@ function sanitizeHttpFailure(error: unknown): { status: ToolConnectionHealthStat code: "oauth_refresh_missing", }; } + if (code === "oauth_reauthorization_required") { + return { + status: "error", + message: "OAuth authorization expired. Reconnect this app to continue.", + code: "oauth_reauthorization_required", + }; + } + if ( + code === "oauth_refresh_in_progress" + || code === "oauth_refresh_superseded" + || code === "oauth_refresh_outcome_unknown" + ) { + return { + status: "error", + message: error.message, + code, + }; + } if (code === "binding_missing" || code === "secret_deleted" || code === "secret_inactive" || code === "version_missing") { return { status: "missing_secret", @@ -1288,6 +1372,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const policySvc = toolAccessPolicyService(db); const now = options.now ?? (() => new Date()); const runtimeSupervisor = createToolRuntimeSupervisor(db, options); + // This map only removes duplicate work inside one service instance. The + // database refresh lease below is the cross-process serialization boundary. + const oauthRefreshFlights = new Map>(); function allowPrivateRemoteEndpoints() { return options.deploymentMode !== "authenticated" || options.deploymentExposure !== "public"; @@ -1725,7 +1812,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} ?? readConfigString(broker, "credentialConfigPath") ?? readConfigString(broker, "secretConfigPath"); const configuredName = readConfigString(broker, "parentCredentialName") ?? readConfigString(broker, "credentialName"); - const secretCandidates = connection.credentialSecretRefs.filter((ref) => ref.configPath !== "oauth.access_token" && ref.configPath !== "oauth.refresh_token"); + const secretCandidates = connection.credentialSecretRefs.filter((ref) => + ref.configPath !== "oauth.access_token" + && ref.configPath !== "oauth.refresh_token" + && ref.configPath !== "oauth.client_secret" + ); const secretRef = configuredPath ? connection.credentialSecretRefs.find((ref) => ref.configPath === configuredPath) : secretCandidates.find((ref) => ref.configPath === "credentials.deploy_token") @@ -2798,8 +2889,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} provider: endpoints.provider, authorizationUrl: endpoints.authorizationUrl, tokenUrl: endpoints.tokenUrl, + registrationUrl: endpoints.registrationUrl ?? null, metadataUrl: endpoints.metadataUrl ?? null, scopes: endpoints.scopes, + codeChallengeMethodsSupported: endpoints.codeChallengeMethodsSupported ?? [], + tokenEndpointAuthMethodsSupported: endpoints.tokenEndpointAuthMethodsSupported ?? [], grantType: endpoints.grantType ?? "authorization_code", discoveredAt: new Date().toISOString(), }, @@ -2946,9 +3040,12 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} let quarantinedCount = 0; const quarantineOnRefresh = shouldQuarantineNewEntries(connection) && connection.status === "active"; const safeDefault = asRecord(connection.config).safeDefault === true; + const sourceTemplateKey = typeof asRecord(connection.config).sourceTemplateKey === "string" + ? String(asRecord(connection.config).sourceTemplateKey) + : null; for (const descriptor of descriptors) { - const riskLevel = classifyRisk(descriptor); - const hash = descriptorHash(descriptor); + const riskLevel = classifyRisk(descriptor, sourceTemplateKey); + const hash = descriptorHash(descriptor, riskLevel); const schemaHash = stableHash(descriptor.inputSchema ?? {}); const existing = existingByName.get(descriptor.name); const changed = existing && (existing.versionHash !== hash || existing.schemaHash !== schemaHash); @@ -3642,7 +3739,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }; } - function oauthClientForConnection( + function configuredOAuthClientForConnection( connection: typeof toolConnections.$inferSelect, provider: string, ) { @@ -3657,6 +3754,43 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} return oauthClientConfig(provider); } + async function oauthClientForConnection( + connection: typeof toolConnections.$inferSelect, + provider: string, + actor?: ActorInfo, + ) { + const configured = configuredOAuthClientForConnection(connection, provider); + if (configured.clientId) return configured; + const oauth = oauthConfig(connection); + const clientId = typeof oauth.clientId === "string" && oauth.clientId.trim() + ? oauth.clientId.trim() + : null; + if (!clientId) return configured; + const clientSecretRef = oauth.clientRegistrationSource === "dcr" + ? undefined + : connection.credentialSecretRefs.find((ref) => ref.configPath === "oauth.client_secret"); + const clientSecret = clientSecretRef + ? await secrets.resolveSecretValue( + connection.companyId, + clientSecretRef.secretId, + clientSecretRef.versionSelector ?? "latest", + { + consumerType: "tool_connection", + consumerId: connection.id, + configPath: "oauth.client_secret", + actorType: actor?.actorType ?? "system", + actorId: actor?.actorId ?? null, + }, + ) + : null; + return { + clientIdEnv: null, + clientSecretEnv: null, + clientId, + clientSecret, + }; + } + function base64UrlSha256(input: string) { return createHash("sha256").update(input).digest("base64url"); } @@ -3736,6 +3870,10 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} function oauthProviderForConnection(connection: typeof toolConnections.$inferSelect, metadataUrl?: string | null): string { const oauth = oauthConfig(connection); if (typeof oauth.provider === "string" && oauth.provider.trim()) return oauth.provider.trim(); + const sourceTemplateKey = typeof connection.config.sourceTemplateKey === "string" + ? connection.config.sourceTemplateKey.trim() + : ""; + if (sourceTemplateKey) return sourceTemplateKey; const url = metadataUrl ?? remoteEndpoint(connection.config); try { return new URL(url).hostname.replace(/[^a-z0-9]+/gi, "_").replace(/^_+|_+$/g, "").toLowerCase() || "generic"; @@ -3767,7 +3905,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} function oauthSecretRef( connection: typeof toolConnections.$inferSelect, - configPath: "oauth.access_token" | "oauth.refresh_token", + configPath: "oauth.access_token" | "oauth.refresh_token" | "oauth.client_secret", ) { return connection.credentialSecretRefs.find((ref) => ref.configPath === configPath) ?? null; } @@ -3820,21 +3958,34 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} if (!metadata) return null; let authorizationUrl = typeof metadata.authorization_endpoint === "string" ? metadata.authorization_endpoint : null; let tokenUrl = typeof metadata.token_endpoint === "string" ? metadata.token_endpoint : null; - if (!authorizationUrl || !tokenUrl) { - for (const authMetadataUrl of await authServerMetadataUrls(metadata)) { - const authMetadata = await fetchJsonRecord(authMetadataUrl); - if (!authMetadata) continue; - authorizationUrl = authorizationUrl ?? (typeof authMetadata.authorization_endpoint === "string" ? authMetadata.authorization_endpoint : null); - tokenUrl = tokenUrl ?? (typeof authMetadata.token_endpoint === "string" ? authMetadata.token_endpoint : null); - if (authorizationUrl && tokenUrl) break; + let registrationUrl = typeof metadata.registration_endpoint === "string" ? metadata.registration_endpoint : null; + let scopes = normalizeOauthScopes(metadata.scopes_supported); + let codeChallengeMethodsSupported = normalizeOauthScopes(metadata.code_challenge_methods_supported); + let tokenEndpointAuthMethodsSupported = normalizeOauthScopes(metadata.token_endpoint_auth_methods_supported); + for (const authMetadataUrl of await authServerMetadataUrls(metadata)) { + const authMetadata = await fetchJsonRecord(authMetadataUrl); + if (!authMetadata) continue; + authorizationUrl = authorizationUrl ?? (typeof authMetadata.authorization_endpoint === "string" ? authMetadata.authorization_endpoint : null); + tokenUrl = tokenUrl ?? (typeof authMetadata.token_endpoint === "string" ? authMetadata.token_endpoint : null); + registrationUrl = registrationUrl ?? (typeof authMetadata.registration_endpoint === "string" ? authMetadata.registration_endpoint : null); + if (scopes.length === 0) scopes = normalizeOauthScopes(authMetadata.scopes_supported); + if (codeChallengeMethodsSupported.length === 0) { + codeChallengeMethodsSupported = normalizeOauthScopes(authMetadata.code_challenge_methods_supported); } + if (tokenEndpointAuthMethodsSupported.length === 0) { + tokenEndpointAuthMethodsSupported = normalizeOauthScopes(authMetadata.token_endpoint_auth_methods_supported); + } + if (authorizationUrl && tokenUrl) break; } if (!authorizationUrl || !tokenUrl) return null; return { provider: oauthProviderForConnection(connection, metadataUrl), - scopes: normalizeOauthScopes(metadata.scopes_supported), + scopes, authorizationUrl, tokenUrl, + registrationUrl, + codeChallengeMethodsSupported, + tokenEndpointAuthMethodsSupported, metadataUrl, }; } @@ -3863,6 +4014,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} scopes, authorizationUrl: configuredAuthorizationUrl, tokenUrl: configuredTokenUrl, + registrationUrl: typeof oauth.registrationUrl === "string" ? oauth.registrationUrl : null, + codeChallengeMethodsSupported: normalizeOauthScopes(oauth.codeChallengeMethodsSupported), + tokenEndpointAuthMethodsSupported: normalizeOauthScopes(oauth.tokenEndpointAuthMethodsSupported), grantType, metadataUrl: typeof oauth.metadataUrl === "string" ? oauth.metadataUrl : hints?.metadataUrl ?? null, }; @@ -3874,6 +4028,10 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} ].filter((value): value is string => Boolean(value)); if (metadataCandidates.length === 0) { const endpoint = new URL(await assertRemoteEndpointAllowed(connection.config)); + const protectedResourcePath = endpoint.pathname === "/" + ? "/.well-known/oauth-protected-resource" + : `/.well-known/oauth-protected-resource${endpoint.pathname}`; + metadataCandidates.push(new URL(protectedResourcePath, endpoint.origin).toString()); metadataCandidates.push(new URL("/.well-known/oauth-protected-resource", endpoint.origin).toString()); metadataCandidates.push(new URL("/.well-known/oauth-authorization-server", endpoint.origin).toString()); metadataCandidates.push(new URL("/.well-known/openid-configuration", endpoint.origin).toString()); @@ -3912,10 +4070,18 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const smokeLabEndpoints = smokeLabOAuthEndpoints(connection, redirectUri); const sourceTemplateKey = typeof connection.config.sourceTemplateKey === "string" ? connection.config.sourceTemplateKey : null; const galleryEntry = sourceTemplateKey ? getConnectableAppDefinition(sourceTemplateKey) : null; + const galleryMethod = galleryEntry ? connectionMethodFor(galleryEntry) : null; + const hasCompleteGalleryEndpointHints = Boolean( + galleryMethod?.defaults?.authorizationEndpoint && galleryMethod.defaults.tokenEndpoint, + ); + const discovered = connection.transport === "mcp_remote" && !hasCompleteGalleryEndpointHints + ? await discoverOAuthEndpoints(connection, challenge) + : null; const endpoints = smokeLabEndpoints + ?? discovered ?? (galleryEntry && connectionMethodFor(galleryEntry).auth === "oauth" - ? await oauthProviderEndpoints(galleryEntry) - : await discoverOAuthEndpoints(connection, challenge)); + ? await oauthProviderEndpoints(galleryEntry) + : await discoverOAuthEndpoints(connection, challenge)); if (!endpoints) throw unprocessable("This app connection does not advertise OAuth sign in"); assertNotSmokeLabOAuthEndpoints(connection, endpoints); return endpoints; @@ -3934,7 +4100,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} async function createOrRotateOAuthSecret(input: { companyId: string; connection: typeof toolConnections.$inferSelect; - configPath: "oauth.access_token" | "oauth.refresh_token"; + configPath: "oauth.access_token" | "oauth.refresh_token" | "oauth.client_secret"; label: string; value: string; actor?: ActorInfo; @@ -3963,6 +4129,273 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }; } + function assertOAuthRedirectConstraints(app: AppDefinition | null, redirectUri: string) { + if (app?.redirectConstraints !== "https-or-loopback-http") return; + let redirect: URL; + try { + redirect = new URL(redirectUri); + } catch { + throw unprocessable("OAuth callback URL is invalid", { code: "oauth_redirect_uri_invalid" }); + } + const hostname = redirect.hostname.replace(/^\[|\]$/g, "").toLowerCase(); + const isLoopback = hostname === "localhost" + || hostname.endsWith(".localhost") + || hostname === "::1" + || /^127(?:\.\d{1,3}){3}$/.test(hostname); + if (redirect.protocol === "https:" || (redirect.protocol === "http:" && isLoopback)) return; + throw unprocessable( + "This provider requires an HTTPS or loopback origin. Configure TLS before connecting.", + { + code: "oauth_redirect_origin_unsupported", + redirectConstraints: app.redirectConstraints, + docsPath: "docs/deploy", + }, + ); + } + + function invalidOAuthDcrResponse(field: string, reason: string): HttpError { + return new HttpError(502, "OAuth provider returned incompatible dynamic client metadata", { + code: "oauth_dcr_response_invalid", + field, + reason, + }); + } + + function parseOAuthDcrString( + record: Record, + field: "client_id" | "client_secret", + input: { required: boolean; maxLength: number }, + ): string | null { + const value = record[field]; + if (value === undefined || value === null) { + if (input.required) throw invalidOAuthDcrResponse(field, "missing"); + return null; + } + if (typeof value !== "string" || value.length === 0 || value.length > input.maxLength) { + throw invalidOAuthDcrResponse(field, "invalid_string"); + } + if (field === "client_id" && value.trim() !== value) { + throw invalidOAuthDcrResponse(field, "invalid_string"); + } + return value; + } + + function assertOAuthDcrArray( + record: Record, + field: "redirect_uris" | "grant_types" | "response_types", + expected: string[], + ) { + if (record[field] === undefined) throw invalidOAuthDcrResponse(field, "missing"); + const value = record[field]; + if ( + !Array.isArray(value) + || value.length !== expected.length + || value.some((entry) => typeof entry !== "string" || entry.length === 0 || entry.length > 2_048) + ) { + throw invalidOAuthDcrResponse(field, "invalid_array"); + } + const actual = [...value].sort(); + const required = [...expected].sort(); + if (actual.some((entry, index) => entry !== required[index])) { + throw invalidOAuthDcrResponse(field, "registered_value_mismatch"); + } + } + + function parseOAuthDcrTimestamp(record: Record, field: string): number | null { + const value = record[field]; + if (value === undefined || value === null) return null; + if (typeof value !== "number" || !Number.isSafeInteger(value) || value < 0) { + throw invalidOAuthDcrResponse(field, "invalid_timestamp"); + } + return value; + } + + async function registerOAuthClient(input: { + connection: typeof toolConnections.$inferSelect; + endpoints: OAuthProviderEndpoints; + redirectUri: string; + actor?: ActorInfo; + }) { + if (!input.endpoints.registrationUrl) { + throw unprocessable("OAuth provider does not advertise dynamic client registration", { + code: "oauth_dcr_not_supported", + }); + } + if ( + input.endpoints.codeChallengeMethodsSupported?.length + && !input.endpoints.codeChallengeMethodsSupported.includes("S256") + ) { + throw unprocessable("OAuth provider does not support the required PKCE S256 method", { + code: "oauth_pkce_s256_required", + }); + } + if ( + input.endpoints.tokenEndpointAuthMethodsSupported?.length + && !input.endpoints.tokenEndpointAuthMethodsSupported.includes("none") + ) { + throw unprocessable("OAuth provider does not support public dynamic clients", { + code: "oauth_dcr_public_client_unsupported", + }); + } + + const host = new URL(input.redirectUri).host; + const requestedMetadata = { + client_name: `Paperclip (${host})`, + redirect_uris: [input.redirectUri], + grant_types: ["authorization_code", "refresh_token"], + response_types: ["code"], + token_endpoint_auth_method: "none", + }; + const response = await fetchRemoteHttpUrl(input.endpoints.registrationUrl, { + method: "POST", + headers: { "content-type": "application/json" }, + body: JSON.stringify(requestedMetadata), + }); + const record = asRecord(await response.json().catch(() => ({})) as unknown); + if (!response.ok) { + const providerError = typeof record.error === "string" ? record.error : null; + const message = typeof record.error_description === "string" + ? record.error_description + : "OAuth dynamic client registration failed"; + throw new HttpError(502, message, { + code: "oauth_dynamic_client_registration_failed", + providerError, + status: response.status, + }); + } + const clientId = parseOAuthDcrString(record, "client_id", { + required: true, + maxLength: MAX_OAUTH_DCR_CLIENT_ID_LENGTH, + })!; + const clientSecret = parseOAuthDcrString(record, "client_secret", { + required: false, + maxLength: MAX_OAUTH_DCR_CLIENT_SECRET_LENGTH, + }); + assertOAuthDcrArray(record, "redirect_uris", requestedMetadata.redirect_uris); + assertOAuthDcrArray(record, "grant_types", requestedMetadata.grant_types); + assertOAuthDcrArray(record, "response_types", requestedMetadata.response_types); + if ( + record.token_endpoint_auth_method !== requestedMetadata.token_endpoint_auth_method + ) { + throw invalidOAuthDcrResponse("token_endpoint_auth_method", "registered_value_mismatch"); + } + const clientIdIssuedAt = parseOAuthDcrTimestamp(record, "client_id_issued_at"); + const clientSecretExpiresAt = parseOAuthDcrTimestamp(record, "client_secret_expires_at"); + if (clientSecretExpiresAt !== null && clientSecret === null) { + throw invalidOAuthDcrResponse("client_secret_expires_at", "client_secret_missing"); + } + const existingClientSecretRef = oauthSecretRef(input.connection, "oauth.client_secret"); + const nextCredentialSecretRefs = input.connection.credentialSecretRefs.filter( + (ref) => ref.configPath !== "oauth.client_secret", + ); + if (clientSecret) { + const clientSecretRef = await createOrRotateOAuthSecret({ + companyId: input.connection.companyId, + connection: input.connection, + configPath: "oauth.client_secret", + label: "OAuth client secret", + value: clientSecret, + actor: input.actor, + }); + nextCredentialSecretRefs.push(clientSecretRef); + } else if (existingClientSecretRef && oauthConfig(input.connection).clientId === clientId) { + nextCredentialSecretRefs.push(existingClientSecretRef); + } + + const oauth = oauthConfig(input.connection); + const nextConfig = { + ...input.connection.config, + oauth: { + ...oauth, + provider: input.endpoints.provider, + authorizationUrl: input.endpoints.authorizationUrl, + tokenUrl: input.endpoints.tokenUrl, + registrationUrl: input.endpoints.registrationUrl, + metadataUrl: input.endpoints.metadataUrl ?? null, + scopes: input.endpoints.scopes, + codeChallengeMethodsSupported: input.endpoints.codeChallengeMethodsSupported ?? [], + tokenEndpointAuthMethodsSupported: input.endpoints.tokenEndpointAuthMethodsSupported ?? [], + clientId, + clientRegistrationSource: "dcr", + clientTokenEndpointAuthMethod: "none", + clientRedirectUri: input.redirectUri, + clientIdIssuedAt, + clientSecretExpiresAt, + }, + }; + const [updated] = await db + .update(toolConnections) + .set({ + ownership: "dcr", + config: nextConfig, + transportConfig: nextConfig, + credentialSecretRefs: nextCredentialSecretRefs, + updatedAt: now(), + }) + .where(and( + eq(toolConnections.id, input.connection.id), + eq(toolConnections.companyId, input.connection.companyId), + )) + .returning(); + if (!updated) throw notFound("Tool connection not found"); + await syncCredentialBindings(updated); + return updated; + } + + async function ensureOAuthClient(input: { + connection: typeof toolConnections.$inferSelect; + endpoints: OAuthProviderEndpoints; + redirectUri: string; + galleryEntry: AppDefinition | null; + actor?: ActorInfo; + }) { + const configured = configuredOAuthClientForConnection(input.connection, input.endpoints.provider); + if (configured.clientId) return { connection: input.connection, client: configured }; + const oauth = oauthConfig(input.connection); + if ( + typeof oauth.clientId === "string" + && oauth.clientId.trim() + && oauth.clientRedirectUri === input.redirectUri + ) { + return { + connection: input.connection, + client: await oauthClientForConnection(input.connection, input.endpoints.provider, input.actor), + }; + } + const method = input.galleryEntry ? connectionMethodFor(input.galleryEntry) : null; + if (!method?.ownershipModes.includes("dcr")) { + throw unprocessable(`OAuth client id is not configured for ${input.endpoints.provider}`); + } + + const key = `${input.connection.id}:${input.redirectUri}`; + return oauthSingleFlight(oauthRegistrationFlights, key, async () => { + const latest = await getConnectionRow(input.connection.id, input.connection.companyId); + const latestConfigured = configuredOAuthClientForConnection(latest, input.endpoints.provider); + if (latestConfigured.clientId) return { connection: latest, client: latestConfigured }; + const latestOauth = oauthConfig(latest); + if ( + typeof latestOauth.clientId === "string" + && latestOauth.clientId.trim() + && latestOauth.clientRedirectUri === input.redirectUri + ) { + return { + connection: latest, + client: await oauthClientForConnection(latest, input.endpoints.provider, input.actor), + }; + } + const registered = await registerOAuthClient({ + connection: latest, + endpoints: input.endpoints, + redirectUri: input.redirectUri, + actor: input.actor, + }); + return { + connection: registered, + client: await oauthClientForConnection(registered, input.endpoints.provider, input.actor), + }; + }); + } + async function exchangeOAuthToken(input: { tokenUrl: string; clientId: string; @@ -3998,12 +4431,24 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const payload = await response.json().catch(() => ({})) as unknown; const record = asRecord(payload); if (!response.ok || record.ok === false) { + const providerError = typeof record.error === "string" ? record.error : null; const message = typeof record.error_description === "string" ? record.error_description - : typeof record.error === "string" - ? record.error + : providerError + ? providerError : "OAuth token exchange failed"; - throw new HttpError(502, message, { code: "oauth_token_exchange_failed", status: response.status }); + if (input.grantType === "refresh_token" && providerError === "invalid_grant") { + throw new HttpError(422, "OAuth authorization has expired. Reconnect this app to continue.", { + code: "oauth_reauthorization_required", + providerError, + status: response.status, + }); + } + throw new HttpError(502, message, { + code: "oauth_token_exchange_failed", + providerError, + status: response.status, + }); } const accessToken = typeof record.access_token === "string" ? record.access_token : null; if (!accessToken) throw new HttpError(502, "OAuth provider did not return an access token", { code: "oauth_access_token_missing" }); @@ -4018,8 +4463,158 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} }; } - async function maybeRefreshOAuthCredentials( + function withoutOAuthRefreshLease(oauth: Record) { + const { refreshLease: _refreshLease, ...rest } = oauth; + return rest; + } + + function oauthRefreshLeaseId(connection: typeof toolConnections.$inferSelect): string | null { + const lease = asRecord(oauthConfig(connection).refreshLease); + return typeof lease.id === "string" && lease.id ? lease.id : null; + } + + async function clearOAuthRefreshLease( connection: typeof toolConnections.$inferSelect, + leaseId: string, + ) { + const latest = await getConnectionRow(connection.id, connection.companyId); + if (oauthRefreshLeaseId(latest) !== leaseId) return latest; + const nextConfig = { + ...latest.config, + oauth: withoutOAuthRefreshLease(oauthConfig(latest)), + }; + const [updated] = await db + .update(toolConnections) + .set({ config: nextConfig, transportConfig: nextConfig, updatedAt: now() }) + .where(and( + eq(toolConnections.id, latest.id), + eq(toolConnections.companyId, latest.companyId), + sql`${toolConnections.config} -> 'oauth' -> 'refreshLease' ->> 'id' = ${leaseId}`, + )) + .returning(); + return updated ?? getConnectionRow(connection.id, connection.companyId); + } + + async function acquireOAuthRefreshLease( + connection: typeof toolConnections.$inferSelect, + ): Promise<{ connection: typeof toolConnections.$inferSelect; leaseId: string | null }> { + const waitDeadline = Date.now() + OAUTH_REFRESH_LEASE_WAIT_MS; + while (true) { + const latest = await getConnectionRow(connection.id, connection.companyId); + const latestExpiresAtMs = oauthExpiresAtMs(latest); + if (latestExpiresAtMs && latestExpiresAtMs > Date.now() + 60_000) { + return { connection: latest, leaseId: null }; + } + + const oauth = oauthConfig(latest); + const currentLease = asRecord(oauth.refreshLease); + const currentLeaseExpiresAt = typeof currentLease.expiresAt === "string" + ? Date.parse(currentLease.expiresAt) + : Number.NaN; + const currentLeaseId = typeof currentLease.id === "string" && currentLease.id + ? currentLease.id + : null; + const leaseIsActive = currentLeaseId !== null + && Number.isFinite(currentLeaseExpiresAt) + && currentLeaseExpiresAt > Date.now(); + if (!currentLeaseId) { + const leaseId = randomUUID(); + const claimedAt = now(); + const nextConfig = { + ...latest.config, + oauth: { + ...withoutOAuthRefreshLease(oauth), + refreshLease: { + id: leaseId, + expiresAt: new Date(Date.now() + OAUTH_REFRESH_LEASE_MS).toISOString(), + }, + }, + }; + const [claimed] = await db + .update(toolConnections) + .set({ config: nextConfig, transportConfig: nextConfig, updatedAt: claimedAt }) + .where(and( + eq(toolConnections.id, latest.id), + eq(toolConnections.companyId, latest.companyId), + sql`${toolConnections.config} = ${JSON.stringify(latest.config)}::jsonb`, + sql`${toolConnections.config} #>> '{oauth,refreshLease,id}' is null`, + )) + .returning(); + if (claimed) return { connection: claimed, leaseId }; + } + + if (currentLeaseId && !leaseIsActive) { + throw new HttpError(422, "The previous OAuth refresh did not finish. Reconnect this app before retrying.", { + code: "oauth_refresh_outcome_unknown", + setupUrl: connectionSetupUrl(latest), + reconnectUrl: connectionReconnectUrl(latest), + }); + } + + if (Date.now() >= waitDeadline) { + throw conflict("OAuth credential refresh is already in progress", { + code: "oauth_refresh_in_progress", + retryable: true, + }); + } + await new Promise((resolve) => setTimeout(resolve, OAUTH_REFRESH_LEASE_POLL_MS)); + } + } + + async function markOAuthReauthorizationRequired( + connection: typeof toolConnections.$inferSelect, + guard: { + leaseId: string; + refreshSecretId: string; + refreshTokenVersion: number; + }, + ) { + const oauth = oauthConfig(connection); + const nextCredentialSecretRefs = connection.credentialSecretRefs.filter( + (ref) => ref.configPath !== "oauth.access_token" && ref.configPath !== "oauth.refresh_token", + ); + const nextCredentialRefs = connection.credentialRefs.filter((ref) => ref.name !== "oauth.access_token"); + const nextConfig = { + ...connection.config, + oauth: { + ...withoutOAuthRefreshLease(oauth), + expiresAt: null, + reauthorizationRequiredAt: now().toISOString(), + }, + }; + const [updated] = await db + .update(toolConnections) + .set({ + status: "draft", + enabled: false, + healthStatus: "error", + healthMessage: "OAuth authorization expired. Reconnect this app to continue.", + lastError: "oauth_reauthorization_required", + config: nextConfig, + transportConfig: nextConfig, + credentialSecretRefs: nextCredentialSecretRefs, + credentialRefs: nextCredentialRefs, + updatedAt: now(), + }) + .where(and( + eq(toolConnections.id, connection.id), + eq(toolConnections.companyId, connection.companyId), + sql`${toolConnections.config} -> 'oauth' -> 'refreshLease' ->> 'id' = ${guard.leaseId}`, + sql`exists ( + select 1 from ${companySecrets} + where ${companySecrets.id} = ${guard.refreshSecretId} + and ${companySecrets.companyId} = ${connection.companyId} + and ${companySecrets.latestVersion} = ${guard.refreshTokenVersion} + )`, + )) + .returning(); + if (updated) await syncCredentialBindings(updated); + return updated ?? null; + } + + async function refreshOAuthCredentials( + connection: typeof toolConnections.$inferSelect, + leaseId: string, actor?: ActorInfo, accessContext?: { actorSource?: "local_implicit" | "session" | "board_key" | "agent_key" | "agent_jwt" | "cloud_tenant"; @@ -4042,10 +4637,21 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} reconnectUrl: connectionReconnectUrl(connection), }); } - const client = oauthClientForConnection(connection, oauth.provider); + const client = await oauthClientForConnection(connection, oauth.provider, actor); if (!client.clientId) throw unprocessable(`OAuth client id is not configured for ${oauth.provider}`); - const refreshToken = refreshRef - ? await secrets.resolveSecretValue(connection.companyId, refreshRef.secretId, refreshRef.versionSelector ?? "latest", { + const [refreshSecret] = refreshRef + ? await db + .select({ latestVersion: companySecrets.latestVersion }) + .from(companySecrets) + .where(and( + eq(companySecrets.id, refreshRef.secretId), + eq(companySecrets.companyId, connection.companyId), + )) + .limit(1) + : [undefined]; + const refreshTokenVersion = refreshSecret?.latestVersion ?? null; + const refreshToken = refreshRef && refreshTokenVersion !== null + ? await secrets.resolveSecretValue(connection.companyId, refreshRef.secretId, refreshTokenVersion, { consumerType: "tool_connection", consumerId: connection.id, configPath: "oauth.refresh_token", @@ -4056,14 +4662,56 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} heartbeatRunId: accessContext?.heartbeatRunId, }) : null; - const token = await exchangeOAuthToken({ - tokenUrl: oauth.tokenUrl, - clientId: client.clientId, - clientSecret: client.clientSecret, - grantType, - scopes: normalizeOauthScopes(oauth.scopes).length > 0 ? normalizeOauthScopes(oauth.scopes) : normalizeOauthScopes(oauth.scope), - refreshToken, - }); + let token: Awaited>; + try { + token = await exchangeOAuthToken({ + tokenUrl: oauth.tokenUrl, + clientId: client.clientId, + clientSecret: client.clientSecret, + grantType, + scopes: normalizeOauthScopes(oauth.scopes).length > 0 ? normalizeOauthScopes(oauth.scopes) : normalizeOauthScopes(oauth.scope), + refreshToken, + }); + } catch (error) { + if (error instanceof HttpError && asRecord(error.details).code === "oauth_reauthorization_required") { + const marked = refreshRef && refreshTokenVersion !== null + ? await markOAuthReauthorizationRequired(connection, { + leaseId, + refreshSecretId: refreshRef.secretId, + refreshTokenVersion, + }) + : null; + if (!marked) { + const latest = await getConnectionRow(connection.id, connection.companyId); + const latestExpiresAtMs = oauthExpiresAtMs(latest); + if (latestExpiresAtMs && latestExpiresAtMs > Date.now() + 60_000) return latest; + throw conflict("OAuth credentials changed while refresh was in progress. Retry the request.", { + code: "oauth_refresh_superseded", + retryable: true, + }); + } + throw new HttpError(error.status, error.message, { + ...asRecord(error.details), + setupUrl: connectionSetupUrl(connection), + reconnectUrl: connectionReconnectUrl(connection), + }); + } + throw error; + } + // Rotating providers invalidate the submitted refresh token immediately. + // Persist its replacement before the new access token can be returned to a + // caller, so a crash cannot leave the grant with only the consumed token. + let nextRefreshRef: Awaited> | null = null; + if (token.refreshToken) { + nextRefreshRef = await createOrRotateOAuthSecret({ + companyId: connection.companyId, + connection, + configPath: "oauth.refresh_token", + label: "OAuth refresh token", + value: token.refreshToken, + actor, + }); + } const accessRef = await createOrRotateOAuthSecret({ companyId: connection.companyId, connection, @@ -4073,26 +4721,18 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} actor, }); const nextCredentialSecretRefs = [ - ...connection.credentialSecretRefs.filter((ref) => ref.configPath !== "oauth.access_token"), + ...connection.credentialSecretRefs.filter((ref) => + ref.configPath !== "oauth.access_token" + && (!nextRefreshRef || ref.configPath !== "oauth.refresh_token") + ), accessRef, + ...(nextRefreshRef ? [nextRefreshRef] : []), ]; - if (token.refreshToken) { - const nextRefreshRef = await createOrRotateOAuthSecret({ - companyId: connection.companyId, - connection, - configPath: "oauth.refresh_token", - label: "OAuth refresh token", - value: token.refreshToken, - actor, - }); - const filtered = nextCredentialSecretRefs.filter((ref) => ref.configPath !== "oauth.refresh_token"); - nextCredentialSecretRefs.splice(0, nextCredentialSecretRefs.length, ...filtered, nextRefreshRef); - } const expiresAt = token.expiresIn ? new Date(Date.now() + token.expiresIn * 1000).toISOString() : null; const nextConfig = { ...connection.config, oauth: { - ...oauth, + ...withoutOAuthRefreshLease(oauth), grantType: grantType === "client_credentials" ? grantType : oauth.grantType ?? "authorization_code", expiresAt, scope: token.scope ?? oauth.scope ?? null, @@ -4127,12 +4767,54 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} ], updatedAt: new Date(), }) - .where(eq(toolConnections.id, connection.id)) + .where(and( + eq(toolConnections.id, connection.id), + eq(toolConnections.companyId, connection.companyId), + sql`${toolConnections.config} -> 'oauth' -> 'refreshLease' ->> 'id' = ${leaseId}`, + )) .returning(); - await syncCredentialBindings(updated); + if (!updated) { + throw conflict("OAuth credentials changed while refresh was in progress. Retry the request.", { + code: "oauth_refresh_superseded", + retryable: true, + }); + } + const previousBindingKeys = new Set(connection.credentialSecretRefs.map( + (ref) => `${ref.secretId}:${ref.configPath}`, + )); + const nextBindingKeys = new Set(nextCredentialSecretRefs.map( + (ref) => `${ref.secretId}:${ref.configPath}`, + )); + const bindingsChanged = previousBindingKeys.size !== nextBindingKeys.size + || [...previousBindingKeys].some((key) => !nextBindingKeys.has(key)); + if (bindingsChanged) await syncCredentialBindings(updated); return updated; } + async function maybeRefreshOAuthCredentials( + connection: typeof toolConnections.$inferSelect, + actor?: ActorInfo, + accessContext?: { + actorSource?: "local_implicit" | "session" | "board_key" | "agent_key" | "agent_jwt" | "cloud_tenant"; + issueId?: string | null; + heartbeatRunId?: string | null; + }, + ): Promise { + const oauth = oauthConfig(connection); + if (typeof oauth.tokenUrl !== "string" || typeof oauth.provider !== "string") return connection; + const expiresAtMs = oauthExpiresAtMs(connection); + if (expiresAtMs && expiresAtMs > Date.now() + 60_000) return connection; + return oauthSingleFlight(oauthRefreshFlights, connection.id, async () => { + const lease = await acquireOAuthRefreshLease(connection); + if (!lease.leaseId) return lease.connection; + try { + return await refreshOAuthCredentials(lease.connection, lease.leaseId, actor, accessContext); + } finally { + await clearOAuthRefreshLease(lease.connection, lease.leaseId).catch(() => undefined); + } + }); + } + function policyNameForApp(connection: typeof toolConnections.$inferSelect, entry: typeof toolCatalogEntries.$inferSelect) { const base = `Ask first ${connection.id.slice(0, 8)} ${entry.toolName}`; return base.length <= 160 ? base : base.slice(0, 160); @@ -4740,13 +5422,24 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} connectionId: string, input: { redirectUri: string; actor: ActorInfo; subjectUserId?: string; scopes?: string[]; returnTo?: string; issueId?: string }, ): Promise { - const connection = await getConnectionRow(connectionId, companyId); + let connection = await getConnectionRow(connectionId, companyId); if (connection.status === "archived") throw conflict("Archived app connections cannot start sign in"); + const sourceTemplateKey = typeof connection.config.sourceTemplateKey === "string" ? connection.config.sourceTemplateKey : null; + const galleryEntry = sourceTemplateKey ? getConnectableAppDefinition(sourceTemplateKey) : null; + assertOAuthRedirectConstraints(galleryEntry, input.redirectUri); const endpoints = await oauthEndpointsForConnection(connection, null, input.redirectUri); if (endpoints.grantType === "client_credentials") { throw unprocessable("This app uses shared machine credentials and does not need browser sign in"); } - const client = oauthClientForConnection(connection, endpoints.provider); + const resolvedClient = await ensureOAuthClient({ + connection, + endpoints, + redirectUri: input.redirectUri, + galleryEntry, + actor: input.actor, + }); + connection = resolvedClient.connection; + const client = resolvedClient.client; if (!client.clientId) throw unprocessable(`OAuth client id is not configured for ${endpoints.provider}`); await db.delete(toolOauthStates).where(lt(toolOauthStates.expiresAt, new Date())); @@ -4837,8 +5530,11 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} provider: endpoints.provider, authorizationUrl: endpoints.authorizationUrl, tokenUrl: endpoints.tokenUrl, + registrationUrl: endpoints.registrationUrl ?? null, metadataUrl: endpoints.metadataUrl ?? null, scopes: endpoints.scopes, + codeChallengeMethodsSupported: endpoints.codeChallengeMethodsSupported ?? [], + tokenEndpointAuthMethodsSupported: endpoints.tokenEndpointAuthMethodsSupported ?? [], grantType: "authorization_code", clientIdEnv: client.clientIdEnv, clientSecretEnv: client.clientSecret ? client.clientSecretEnv : null, @@ -4896,8 +5592,9 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} let connection = await getConnectionRow(stateRow.connectionId, stateRow.companyId); const sourceTemplateKey = typeof connection.config.sourceTemplateKey === "string" ? connection.config.sourceTemplateKey : null; const galleryEntry = sourceTemplateKey ? getConnectableAppDefinition(sourceTemplateKey) : null; + assertOAuthRedirectConstraints(galleryEntry, input.redirectUri); const endpoints = await oauthEndpointsForConnection(connection, null, input.redirectUri); - const client = oauthClientForConnection(connection, endpoints.provider); + const client = await oauthClientForConnection(connection, endpoints.provider, input.actor); if (!client.clientId) throw unprocessable(`OAuth client id is not configured for ${endpoints.provider}`); const token = await exchangeOAuthToken({ @@ -5000,7 +5697,7 @@ export function toolAccessService(db: Db, options: ToolAccessServiceOptions = {} const nextConfig = { ...connection.config, oauth: { - ...oauthConfig(connection), + ...withoutOAuthRefreshLease(oauthConfig(connection)), provider: endpoints.provider, authorizationUrl: endpoints.authorizationUrl, tokenUrl: endpoints.tokenUrl, diff --git a/ui/src/App.tsx b/ui/src/App.tsx index f772a71251..0e309f4fa2 100644 --- a/ui/src/App.tsx +++ b/ui/src/App.tsx @@ -60,6 +60,7 @@ import { ProfileDetailRoute } from "./pages/tools/profiles/ProfileDetailRoute"; import { Connections } from "./pages/apps/Connections"; import { Browse } from "./pages/apps/Browse"; import { AppsConnect } from "./pages/apps/AppsConnect"; +import { canEnterAppsConnect } from "./pages/apps/app-connect-policy"; import { AppsReview } from "./pages/apps/AppsReview"; import { AppDetail } from "./pages/apps/AppDetail"; import { AppNotConnected } from "./pages/apps/AppNotConnected"; @@ -308,7 +309,7 @@ function boardRoutes() { function AppsConnectEntryRoute() { const location = useLocation(); const searchParams = new URLSearchParams(location.search); - return searchParams.get("byo") === "1" ? : ; + return canEnterAppsConnect(searchParams) ? : ; } function InboxRootRedirect() { diff --git a/ui/src/pages/apps/AppDetail.test.tsx b/ui/src/pages/apps/AppDetail.test.tsx index 2aa7bd2199..5125be35c8 100644 --- a/ui/src/pages/apps/AppDetail.test.tsx +++ b/ui/src/pages/apps/AppDetail.test.tsx @@ -23,6 +23,7 @@ const startOAuthMock = vi.hoisted(() => vi.fn()); const mockNavigate = vi.hoisted(() => vi.fn()); const mockParams = vi.hoisted(() => ({ connectionId: "conn-1", tab: "setup" as string | undefined })); const navigateComponentMock = vi.hoisted(() => vi.fn()); +const navigateTopLevelMock = vi.hoisted(() => vi.fn()); vi.mock("@/api/tools", () => ({ toolsApi: { @@ -57,6 +58,10 @@ vi.mock("@/api/agents", () => ({ }, })); +vi.mock("@/lib/browserNavigation", () => ({ + navigateTopLevel: (target: string) => navigateTopLevelMock(target), +})); + vi.mock("@/lib/router", () => ({ useParams: () => mockParams, useNavigate: () => mockNavigate, @@ -371,6 +376,40 @@ describe("AppDetail", () => { ).toBe(true); }); + it("matches connected Notion guidance to the reconnect action", async () => { + getConnectionMock.mockResolvedValue(connection({ + name: "Notion", + config: { + sourceTemplateKey: "notion", + oauth: { + provider: "notion", + connectedAt: "2026-08-06T20:00:00.000Z", + }, + }, + })); + listGalleryMock.mockResolvedValue({ + apps: [{ + key: "notion", + name: "Notion", + logoUrl: "https://example.com/notion.png", + tagline: "Search and update your Notion workspace.", + description: "Give agents governed access to Notion.", + authKind: "oauth", + transportTemplate: { transport: "mcp_remote", url: "https://mcp.notion.com/mcp" }, + credentialFields: [], + recommendedDefaults: {}, + urlPatterns: [], + }], + }); + + await renderAppDetail(); + + expect(container.textContent).toContain( + "Your workspace authorization is active. Reconnect any time to replace it.", + ); + expect(container.textContent).not.toContain("Sign in again any time"); + }); + it("lets Google Sheets connections add spreadsheet links from setup", async () => { mockParams.tab = "setup"; getConnectionMock.mockResolvedValue(connection({ @@ -681,4 +720,29 @@ describe("AppDetail", () => { expect(container.textContent).toContain("Token expired."); expect(container.textContent).toContain("Who can use it"); }); + + it("shows terminal OAuth failures as reconnect-required sign-in", async () => { + mockParams.tab = "permissions"; + getConnectionMock.mockResolvedValue(connection({ + authKind: "oauth", + healthStatus: "failed", + healthMessage: "Authorization expired (invalid_grant).", + })); + + await renderAppDetail(); + + expect(container.textContent).toContain("Reconnect required"); + expect(container.textContent).toContain("Authorization expired (invalid_grant)."); + expect(container.querySelector('input[placeholder="Paste your new key"]')).toBeNull(); + + await act(async () => { + Array.from(container.querySelectorAll("button")) + .find((button) => button.textContent?.trim() === "Reconnect") + ?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(startOAuthMock).toHaveBeenCalledWith("conn-1"); + expect(navigateTopLevelMock).toHaveBeenCalledWith("http://example.test/oauth"); + }); }); diff --git a/ui/src/pages/apps/AppDetail.tsx b/ui/src/pages/apps/AppDetail.tsx index 010311c37a..9d1222df80 100644 --- a/ui/src/pages/apps/AppDetail.tsx +++ b/ui/src/pages/apps/AppDetail.tsx @@ -22,6 +22,7 @@ import { accessApi } from "@/api/access"; import { authApi } from "@/api/auth"; import { buildCompanyUserLabelMap } from "@/lib/company-members"; import { installPayload, installStateFrom, type InstallState } from "@/lib/tool-installs"; +import { navigateTopLevel } from "@/lib/browserNavigation"; import { Button } from "@/components/ui/button"; import { Input } from "@/components/ui/input"; import { Skeleton } from "@/components/ui/skeleton"; @@ -239,7 +240,7 @@ export function AppDetail() { const startOAuth = useMutation({ mutationFn: () => toolsApi.startOAuth(connectionId), onSuccess: ({ authorizationUrl }) => { - window.location.assign(authorizationUrl); + navigateTopLevel(authorizationUrl); }, onError: (error) => pushToast({ diff --git a/ui/src/pages/apps/AppsConnect.test.tsx b/ui/src/pages/apps/AppsConnect.test.tsx index ac2b0fde6c..966e8893b8 100644 --- a/ui/src/pages/apps/AppsConnect.test.tsx +++ b/ui/src/pages/apps/AppsConnect.test.tsx @@ -5,24 +5,33 @@ import { createRoot } from "react-dom/client"; import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { CONNECTABLE_APP_DEFINITIONS } from "@paperclipai/shared"; import { afterEach, beforeEach, describe, expect, it, vi } from "vitest"; +import { queryKeys } from "@/lib/queryKeys"; import { AppsConnect } from "./AppsConnect"; const listGalleryMock = vi.hoisted(() => vi.fn()); +const listApplicationsMock = vi.hoisted(() => vi.fn()); +const listConnectionsMock = vi.hoisted(() => vi.fn()); const connectAppMock = vi.hoisted(() => vi.fn()); +const startOAuthMock = vi.hoisted(() => vi.fn()); const finishAppMock = vi.hoisted(() => vi.fn()); const putConnectionInstallsMock = vi.hoisted(() => vi.fn()); const listAgentsMock = vi.hoisted(() => vi.fn()); const mockNavigate = vi.hoisted(() => vi.fn()); +const navigateTopLevelMock = vi.hoisted(() => vi.fn()); const mockSearch = vi.hoisted(() => ({ value: "" })); const mockParams = vi.hoisted(() => ({ appKey: undefined as string | undefined })); const ZAPIER = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "zapier")!; +const NOTION = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "notion")!; const GOOGLE_SHEETS = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "google-sheets")!; vi.mock("@/api/tools", () => ({ toolsApi: { listGallery: (companyId: string) => listGalleryMock(companyId), + listApplications: (companyId: string) => listApplicationsMock(companyId), + listConnections: (companyId: string) => listConnectionsMock(companyId), connectApp: (companyId: string, input: unknown) => connectAppMock(companyId, input), + startOAuth: (connectionId: string) => startOAuthMock(connectionId), finishApp: (companyId: string, connectionId: string, input: unknown) => finishAppMock(companyId, connectionId, input), putConnectionInstalls: (connectionId: string, installs: unknown) => @@ -34,6 +43,10 @@ vi.mock("@/api/agents", () => ({ agentsApi: { list: (companyId: string) => listAgentsMock(companyId) }, })); +vi.mock("@/lib/browserNavigation", () => ({ + navigateTopLevel: (target: string) => navigateTopLevelMock(target), +})); + vi.mock("@/lib/router", () => ({ useNavigate: () => mockNavigate, useParams: () => mockParams, @@ -129,6 +142,14 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { ZAPIER, ], }); + listApplicationsMock.mockResolvedValue({ applications: [] }); + listConnectionsMock.mockResolvedValue({ connections: [] }); + startOAuthMock.mockResolvedValue({ + connectionId: "conn-notion", + provider: "notion", + authorizationUrl: "https://mcp.notion.com/authorize?state=resumed", + expiresAt: "2099-01-01T00:00:00.000Z", + }); finishAppMock.mockResolvedValue({}); putConnectionInstallsMock.mockResolvedValue({ connectionId: "conn-1", installs: [] }); connectAppMock.mockResolvedValue({ @@ -150,12 +171,12 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { vi.clearAllMocks(); }); - async function render() { + async function render(queryClient?: QueryClient) { const root = createRoot(container); - const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + const client = queryClient ?? new QueryClient({ defaultOptions: { queries: { retry: false } } }); await act(async () => { root.render( - + , ); @@ -190,6 +211,298 @@ describe("AppsConnect — Connect with a link (M4 frame)", () => { expect(container.textContent).not.toContain("Pick the app you want your agents to use."); }); + it("auto-starts the allowlisted Notion source deep link and opens provider sign-in", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + connectAppMock.mockResolvedValueOnce({ + connectionId: "conn-notion", + application: { id: "app-notion", name: "Notion" }, + connection: { id: "conn-notion" }, + actions: { readOnly: [], canMakeChanges: [] }, + catalog: [], + suggestedDefaults: {}, + auth: { kind: "oauth", startUrl: "https://mcp.notion.com/authorize?state=opaque" }, + }); + + await render(); + + expect(connectAppMock).toHaveBeenCalledTimes(1); + expect(connectAppMock).toHaveBeenCalledWith("company-1", { + galleryKey: "notion", + name: "Notion", + credentialValues: {}, + configValues: undefined, + applicationId: undefined, + }); + expect(navigateTopLevelMock).toHaveBeenCalledWith( + "https://mcp.notion.com/authorize?state=opaque", + ); + }); + + it("shows an in-flight state while Paperclip prepares Notion sign-in", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + connectAppMock.mockReturnValueOnce(new Promise(() => {})); + + await render(); + + expect(container.textContent).toContain("Connect Notion"); + expect(container.textContent).toContain("Preparing secure sign-in"); + expect(container.textContent).toContain("Preparing…"); + expect(connectAppMock).toHaveBeenCalledTimes(1); + }); + + it("resumes an existing Notion OAuth connection instead of creating another draft", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + listApplicationsMock.mockResolvedValueOnce({ + applications: [{ + id: "app-notion", + status: "draft", + metadata: { sourceTemplateKey: "notion" }, + }], + }); + listConnectionsMock.mockResolvedValueOnce({ + connections: [{ + id: "conn-existing", + applicationId: "app-notion", + authKind: "oauth", + status: "draft", + config: { sourceTemplateKey: "notion" }, + transportConfig: {}, + }], + }); + startOAuthMock.mockResolvedValueOnce({ + connectionId: "conn-existing", + provider: "notion", + authorizationUrl: "https://mcp.notion.com/authorize?state=existing", + expiresAt: "2099-01-01T00:00:00.000Z", + }); + + await render(); + + expect(connectAppMock).not.toHaveBeenCalled(); + expect(startOAuthMock).toHaveBeenCalledWith("conn-existing"); + expect(navigateTopLevelMock).toHaveBeenCalledWith( + "https://mcp.notion.com/authorize?state=existing", + ); + }); + + it("waits for fresh connection data before creating a Notion OAuth draft", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + const queryClient = new QueryClient({ defaultOptions: { queries: { retry: false } } }); + queryClient.setQueryData(queryKeys.tools.applications("company-1"), { applications: [] }); + queryClient.setQueryData(queryKeys.tools.connections("company-1"), { connections: [] }); + + let resolveApplications!: (value: unknown) => void; + let resolveConnections!: (value: unknown) => void; + listApplicationsMock.mockReturnValueOnce(new Promise((resolve) => { + resolveApplications = resolve; + })); + listConnectionsMock.mockReturnValueOnce(new Promise((resolve) => { + resolveConnections = resolve; + })); + + await render(queryClient); + + expect(connectAppMock).not.toHaveBeenCalled(); + expect(startOAuthMock).not.toHaveBeenCalled(); + + resolveApplications({ + applications: [{ + id: "app-notion", + status: "draft", + metadata: { sourceTemplateKey: "notion" }, + }], + }); + resolveConnections({ + connections: [{ + id: "conn-refreshed", + applicationId: "app-notion", + authKind: "oauth", + status: "draft", + config: { sourceTemplateKey: "notion" }, + transportConfig: {}, + }], + }); + await flushReact(); + await flushReact(); + + expect(connectAppMock).not.toHaveBeenCalled(); + expect(startOAuthMock).toHaveBeenCalledWith("conn-refreshed"); + }); + + it("resumes Notion OAuth after a failed connection lookup is retried", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + listApplicationsMock + .mockRejectedValueOnce(new Error("Lookup unavailable")) + .mockResolvedValueOnce({ + applications: [{ + id: "app-notion", + status: "draft", + metadata: { sourceTemplateKey: "notion" }, + }], + }); + listConnectionsMock + .mockResolvedValueOnce({ connections: [] }) + .mockResolvedValueOnce({ + connections: [{ + id: "conn-after-retry", + applicationId: "app-notion", + authKind: "oauth", + status: "draft", + config: { sourceTemplateKey: "notion" }, + transportConfig: {}, + }], + }); + + await render(); + + expect(container.textContent).toContain("couldn’t check for an existing connection"); + await act(async () => { + buttonByText("Try again")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(connectAppMock).not.toHaveBeenCalled(); + expect(startOAuthMock).toHaveBeenCalledWith("conn-after-retry"); + expect(navigateTopLevelMock).toHaveBeenCalledWith( + "https://mcp.notion.com/authorize?state=resumed", + ); + }); + + it("restores the Notion lookup error when retrying still fails", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + listApplicationsMock.mockRejectedValue(new Error("Lookup unavailable")); + + await render(); + + await act(async () => { + buttonByText("Try again")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(container.textContent).toContain("couldn’t check for an existing connection"); + expect(buttonByText("Try again")).toBeTruthy(); + expect(connectAppMock).not.toHaveBeenCalled(); + expect(startOAuthMock).not.toHaveBeenCalled(); + }); + + it("retries OAuth on the prepared connection without creating another draft", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + connectAppMock.mockResolvedValueOnce({ + connectionId: "conn-prepared", + application: { id: "app-notion", name: "Notion" }, + connection: { id: "conn-prepared" }, + actions: { readOnly: [], canMakeChanges: [] }, + catalog: [], + suggestedDefaults: {}, + auth: { kind: "oauth", startUrl: null }, + }); + startOAuthMock + .mockRejectedValueOnce(new Error("Provider unavailable")) + .mockResolvedValueOnce({ + connectionId: "conn-prepared", + provider: "notion", + authorizationUrl: "https://mcp.notion.com/authorize?state=retry", + expiresAt: "2099-01-01T00:00:00.000Z", + }); + + await render(); + + expect(container.textContent).toContain("Provider unavailable"); + await act(async () => { + buttonByText("Try again")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + + expect(connectAppMock).toHaveBeenCalledTimes(1); + expect(startOAuthMock).toHaveBeenCalledTimes(2); + expect(startOAuthMock).toHaveBeenLastCalledWith("conn-prepared"); + expect(navigateTopLevelMock).toHaveBeenCalledWith( + "https://mcp.notion.com/authorize?state=retry", + ); + }); + + it("recovers a response-lost Notion draft before retrying creation", async () => { + mockSearch.value = "source=notion"; + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + listApplicationsMock + .mockResolvedValueOnce({ applications: [] }) + .mockResolvedValueOnce({ + applications: [{ + id: "app-response-lost", + status: "draft", + metadata: { sourceTemplateKey: "notion" }, + }], + }); + listConnectionsMock + .mockResolvedValueOnce({ connections: [] }) + .mockResolvedValueOnce({ + connections: [{ + id: "conn-response-lost", + applicationId: "app-response-lost", + authKind: "oauth", + status: "draft", + config: { sourceTemplateKey: "notion" }, + transportConfig: {}, + }], + }); + connectAppMock.mockRejectedValueOnce(new Error("Response lost")); + startOAuthMock.mockResolvedValueOnce({ + connectionId: "conn-response-lost", + provider: "notion", + authorizationUrl: "https://mcp.notion.com/authorize?state=recovered", + expiresAt: "2099-01-01T00:00:00.000Z", + }); + + await render(); + + expect(container.textContent).toContain("Response lost"); + await act(async () => { + buttonByText("Try again")?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + await flushReact(); + await flushReact(); + + expect(connectAppMock).toHaveBeenCalledTimes(1); + expect(startOAuthMock).toHaveBeenCalledWith("conn-response-lost"); + expect(navigateTopLevelMock).toHaveBeenCalledWith( + "https://mcp.notion.com/authorize?state=recovered", + ); + }); + + it("keeps non-allowlisted OAuth apps blocked", async () => { + const slack = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "slack")!; + mockParams.appKey = "slack"; + listGalleryMock.mockResolvedValueOnce({ apps: [slack] }); + + await render(); + + expect(connectAppMock).not.toHaveBeenCalled(); + expect(mockNavigate).toHaveBeenCalledWith("/apps/connect", { replace: true }); + }); + + it("routes the enabled Notion gallery tile through the generic source deep link", async () => { + listGalleryMock.mockResolvedValueOnce({ apps: [NOTION] }); + await render(); + + const notionTile = buttonContaining("Notion"); + expect(notionTile?.disabled).toBe(false); + expect(notionTile?.textContent).toContain("Connect"); + + await act(async () => { + notionTile?.dispatchEvent(new MouseEvent("click", { bubbles: true })); + }); + expect(mockNavigate).toHaveBeenCalledWith("/apps/connect?source=notion"); + }); + it("choosing No and clicking Check link connects with no credentials", async () => { await render(); await gotoLinkFrame(container, "https://www.example.com/actions"); diff --git a/ui/src/pages/apps/AppsConnect.tsx b/ui/src/pages/apps/AppsConnect.tsx index a1fbd41ea3..2de37c4a24 100644 --- a/ui/src/pages/apps/AppsConnect.tsx +++ b/ui/src/pages/apps/AppsConnect.tsx @@ -17,6 +17,8 @@ import type { Agent, AppDefinition, ConnectToolAppResult, + ToolApplication, + ToolConnection, ToolAppConnectionActionSummary, } from "@paperclipai/shared"; import { credentialConfigPath, getAppDefinitionForUrl, getAvailableConnectionMethod } from "@paperclipai/shared"; @@ -40,11 +42,14 @@ import { ToggleSwitch } from "@/components/ui/toggle-switch"; import { Skeleton } from "@/components/ui/skeleton"; import { cn } from "@/lib/utils"; import { copyTextToClipboard } from "@/lib/clipboard"; +import { navigateTopLevel } from "@/lib/browserNavigation"; import { AppLogo } from "./AppLogo"; +import { appSourceConnectHref, isMcpDirectOAuthConnectSlug } from "./app-connect-policy"; import { parseGoogleSheetIds } from "./google-sheets"; import { autoExtendNotice, INSTALL_ALL_WARNING, installInfoNotice, installPayload } from "@/lib/tool-installs"; type Step = "gallery" | "key" | "actions" | "who" | "install" | "success"; +export type OAuthConnectPhase = "entry" | "starting" | "redirecting" | "error"; const ROUTE_STAGE_BY_STEP: Partial> = { key: "setup", @@ -88,6 +93,36 @@ function isGoogleSheetsEntry(entry: AppDefinition | null): boolean { return entry?.slug === "google-sheets"; } +function appSourceSlug(application: ToolApplication): string | null { + const metadata = application.metadata; + if (!metadata) return null; + const source = metadata.sourceTemplateKey ?? metadata.galleryKey; + return typeof source === "string" ? source : null; +} + +function connectionSourceSlug(connection: ToolConnection): string | null { + const source = connection.config?.sourceTemplateKey ?? connection.transportConfig.sourceTemplateKey; + return typeof source === "string" ? source : null; +} + +function reusableOAuthConnection( + sourceSlug: string | null, + applications: ToolApplication[], + connections: ToolConnection[], +): ToolConnection | null { + if (!sourceSlug) return null; + const matchingApplicationIds = new Set( + applications + .filter((application) => application.status !== "archived" && appSourceSlug(application) === sourceSlug) + .map((application) => application.id), + ); + return connections.find((connection) => + connection.status !== "archived" && + connection.authKind === "oauth" && + (matchingApplicationIds.has(connection.applicationId) || connectionSourceSlug(connection) === sourceSlug) + ) ?? null; +} + export function AppsConnect() { const navigate = useNavigate(); const routeParams = useParams<{ appKey?: string }>(); @@ -96,7 +131,10 @@ export function AppsConnect() { const { pushToast } = useToast(); const [searchParams] = useSearchParams(); const appKey = routeParams.appKey ?? searchParams.get("appKey") ?? undefined; - const zapierSource = searchParams.get("source") === "zapier"; + const sourceSlug = searchParams.get("source")?.trim() || null; + const directOAuthSource = isMcpDirectOAuthConnectSlug(sourceSlug) ? sourceSlug : null; + const requestedAppKey = appKey ?? directOAuthSource ?? undefined; + const zapierSource = sourceSlug === "zapier"; // Prefill arrives from the app page for reconnects; read once so later // wizard navigation doesn't fight the URL. @@ -109,7 +147,7 @@ export function AppsConnect() { }; }); - const [step, setStep] = useState(appKey || prefill.link || zapierSource ? "key" : "gallery"); + const [step, setStep] = useState(requestedAppKey || prefill.link || zapierSource ? "key" : "gallery"); const [entry, setEntry] = useState(null); const [galleryName, setGalleryName] = useState(""); const [linkUrl, setLinkUrl] = useState(prefill.link); @@ -125,6 +163,10 @@ export function AppsConnect() { const [agentIds, setAgentIds] = useState>(new Set()); const [installMode, setInstallMode] = useState("none"); const [installAgentIds, setInstallAgentIds] = useState>(new Set()); + const [oauthPhase, setOAuthPhase] = useState("entry"); + const [oauthError, setOAuthError] = useState(null); + const directOAuthStartedRef = useRef(false); + const directOAuthRetryingRef = useRef(false); const openGallery = () => { setEntry(null); @@ -157,12 +199,142 @@ export function AppsConnect() { queryFn: () => toolsApi.listGallery(selectedCompanyId!), enabled: !!selectedCompanyId, }); + const applicationsQuery = useQuery({ + queryKey: queryKeys.tools.applications(selectedCompanyId ?? "__none__"), + queryFn: () => toolsApi.listApplications(selectedCompanyId!), + enabled: !!selectedCompanyId && !!directOAuthSource, + refetchOnMount: "always", + }); + const connectionsQuery = useQuery({ + queryKey: queryKeys.tools.connections(selectedCompanyId ?? "__none__"), + queryFn: () => toolsApi.listConnections(selectedCompanyId!), + enabled: !!selectedCompanyId && !!directOAuthSource, + refetchOnMount: "always", + }); + const existingOAuthConnection = useMemo( + () => reusableOAuthConnection( + directOAuthSource, + applicationsQuery.data?.applications ?? [], + connectionsQuery.data?.connections ?? [], + ), + [applicationsQuery.data, connectionsQuery.data, directOAuthSource], + ); + + const directOAuthEntry = entry && + getAvailableConnectionMethod(entry)?.auth === "oauth" && + isMcpDirectOAuthConnectSlug(entry.slug) + ? entry + : null; + + const setAppStep = (nextStep: Step) => { + setStep(nextStep); + if (entry) navigate(appConnectHref(entry.slug, nextStep)); + }; + + const oauthStartMutation = useMutation({ + mutationFn: (connectionId: string) => toolsApi.startOAuth(connectionId), + onSuccess: ({ authorizationUrl }) => { + setOAuthPhase("redirecting"); + navigateTopLevel(authorizationUrl); + }, + onError: (error) => { + const details = error instanceof ApiError && error.body && typeof error.body === "object" + ? (error.body as { details?: { code?: unknown } }).details + : null; + setOAuthPhase("error"); + setOAuthError( + details?.code === "invalid_grant" + ? "Your authorization expired or was revoked. Reconnect to continue." + : error instanceof Error + ? error.message + : "Paperclip couldn’t start secure sign-in. Try again.", + ); + }, + }); + const startOAuth = oauthStartMutation.mutate; + + const connectMutation = useMutation({ + mutationFn: (entryOverride?: AppDefinition) => { + const connectEntry = entryOverride ?? entry; + if (connectEntry) { + const sheetIds = isGoogleSheetsEntry(connectEntry) ? parseGoogleSheetIds(googleSheetsLinks).ids : []; + const trimmedGalleryName = galleryName.trim(); + return toolsApi.connectApp(selectedCompanyId!, { + galleryKey: connectEntry.slug, + name: trimmedGalleryName || connectEntry.name, + credentialValues: credentials, + configValues: isGoogleSheetsEntry(connectEntry) ? { allowedSpreadsheetIds: sheetIds } : undefined, + applicationId: prefill.applicationId, + }); + } + const trimmedKey = linkNeedsKey ? linkKey.trim() : ""; + const trimmedName = linkName.trim(); + return toolsApi.connectApp(selectedCompanyId!, { + link: linkUrl, + name: trimmedName || undefined, + credentialValues: trimmedKey ? { [LINK_CREDENTIAL_CONFIG_PATH]: trimmedKey } : undefined, + applicationId: prefill.applicationId, + }); + }, + onSuccess: (result) => { + if (result.auth?.kind === "oauth") { + setConnectResult(result); + const startUrl = result.auth.startUrl?.trim(); + if (!startUrl) { + setOAuthPhase("starting"); + startOAuth(result.connectionId); + return; + } + setOAuthPhase("redirecting"); + navigateTopLevel(startUrl); + return; + } + setConnectResult(result); + const defaults: Record = {}; + for (const a of result.actions.readOnly) defaults[a.catalogEntryId] = true; + for (const a of result.actions.canMakeChanges) defaults[a.catalogEntryId] = false; + setEnabled(defaults); + setInstallMode("none"); + setInstallAgentIds(new Set()); + setAppStep("actions"); + }, + onError: (error) => { + const details = error instanceof ApiError && error.body && typeof error.body === "object" + ? (error.body as { details?: { code?: unknown } }).details + : null; + if (isMcpDirectOAuthConnectSlug(requestedAppKey)) { + setOAuthPhase("error"); + setOAuthError( + details?.code === "invalid_grant" + ? "Your authorization expired or was revoked. Reconnect to continue." + : error instanceof Error + ? error.message + : "Paperclip couldn’t start secure sign-in. Try again.", + ); + return; + } + const oauthRequired = details?.code === "oauth_challenge"; + pushToast({ + title: oauthRequired ? "Sign-in required" : "Couldn’t connect", + body: oauthRequired + ? "This app needs you to sign in - coming soon." + : error instanceof Error + ? error.message + : "Please check your key and try again.", + tone: "error", + }); + }, + }); + const connectApp = connectMutation.mutate; useEffect(() => { - if (!appKey || galleryQuery.isLoading || !galleryQuery.data) return; + if (!requestedAppKey || galleryQuery.isLoading || !galleryQuery.data) return; - const requestedEntry = galleryQuery.data.apps.find((candidate) => candidate.slug === appKey); - if (!requestedEntry || getAvailableConnectionMethod(requestedEntry)?.auth === "oauth" || requestedEntry.availability?.available === false) { + const requestedEntry = galleryQuery.data.apps.find((candidate) => candidate.slug === requestedAppKey); + const method = requestedEntry ? getAvailableConnectionMethod(requestedEntry) : null; + const directOAuth = method?.auth === "oauth" && isMcpDirectOAuthConnectSlug(requestedEntry?.slug); + const unsupportedOAuth = method?.auth === "oauth" && !directOAuth; + if (!requestedEntry || unsupportedOAuth || requestedEntry.availability?.available === false) { setEntry(null); setStep("gallery"); navigate("/apps/connect", { replace: true }); @@ -184,61 +356,42 @@ export function AppsConnect() { setInstallMode("none"); setInstallAgentIds(new Set()); setStep("key"); - }, [appKey, entry?.slug, galleryQuery.data, galleryQuery.isLoading, navigate]); - const setAppStep = (nextStep: Step) => { - setStep(nextStep); - if (entry) navigate(appConnectHref(entry.slug, nextStep)); - }; + if (directOAuth && ( + !applicationsQuery.isFetchedAfterMount || + !connectionsQuery.isFetchedAfterMount + )) return; + if (directOAuth && directOAuthRetryingRef.current) return; + if (directOAuth && (applicationsQuery.isError || connectionsQuery.isError)) { + setOAuthPhase("error"); + setOAuthError("Paperclip couldn’t check for an existing connection. Try again."); + return; + } - const connectMutation = useMutation({ - mutationFn: () => { - if (entry) { - const sheetIds = isGoogleSheetsEntry(entry) ? parseGoogleSheetIds(googleSheetsLinks).ids : []; - const trimmedGalleryName = galleryName.trim(); - return toolsApi.connectApp(selectedCompanyId!, { - galleryKey: entry.slug, - name: trimmedGalleryName || undefined, - credentialValues: credentials, - configValues: isGoogleSheetsEntry(entry) ? { allowedSpreadsheetIds: sheetIds } : undefined, - applicationId: prefill.applicationId, - }); + if (directOAuth && !directOAuthStartedRef.current) { + directOAuthStartedRef.current = true; + setOAuthError(null); + setOAuthPhase("starting"); + if (existingOAuthConnection) { + startOAuth(existingOAuthConnection.id); + } else { + connectApp(requestedEntry); } - const trimmedKey = linkNeedsKey ? linkKey.trim() : ""; - const trimmedName = linkName.trim(); - return toolsApi.connectApp(selectedCompanyId!, { - link: linkUrl, - name: trimmedName || undefined, - credentialValues: trimmedKey ? { [LINK_CREDENTIAL_CONFIG_PATH]: trimmedKey } : undefined, - applicationId: prefill.applicationId, - }); - }, - onSuccess: (result) => { - setConnectResult(result); - const defaults: Record = {}; - for (const a of result.actions.readOnly) defaults[a.catalogEntryId] = true; - for (const a of result.actions.canMakeChanges) defaults[a.catalogEntryId] = false; - setEnabled(defaults); - setInstallMode("none"); - setInstallAgentIds(new Set()); - setAppStep("actions"); - }, - onError: (error) => { - const details = error instanceof ApiError && error.body && typeof error.body === "object" - ? (error.body as { details?: { code?: unknown } }).details - : null; - const oauthRequired = details?.code === "oauth_challenge"; - pushToast({ - title: oauthRequired ? "Sign-in required" : "Couldn’t connect", - body: oauthRequired - ? "This app needs you to sign in - coming soon." - : error instanceof Error - ? error.message - : "Please check your key and try again.", - tone: "error", - }); - }, - }); + } + }, [ + applicationsQuery.isError, + applicationsQuery.isFetchedAfterMount, + connectApp, + connectionsQuery.isError, + connectionsQuery.isFetchedAfterMount, + entry?.slug, + existingOAuthConnection, + galleryQuery.data, + galleryQuery.isLoading, + navigate, + requestedAppKey, + startOAuth, + ]); const finishMutation = useMutation({ mutationFn: async () => { @@ -280,6 +433,55 @@ export function AppsConnect() { return
Select a company to connect apps.
; } + if (directOAuthEntry && step === "key") { + return ( + { + setOAuthError(null); + setOAuthPhase("starting"); + const connectionId = connectResult?.connectionId ?? existingOAuthConnection?.id; + if (connectionId) { + startOAuth(connectionId); + return; + } + + // The create request may have reached the server even when its + // response did not reach the browser. Re-read both resources before + // creating again so Retry resumes that durable draft instead of + // duplicating it. + directOAuthRetryingRef.current = true; + try { + const [applicationsResult, connectionsResult] = await Promise.all([ + applicationsQuery.refetch(), + connectionsQuery.refetch(), + ]); + if (applicationsResult.isError || connectionsResult.isError) { + setOAuthPhase("error"); + setOAuthError("Paperclip couldn’t check for an existing connection. Try again."); + return; + } + const refreshedConnection = reusableOAuthConnection( + directOAuthSource, + applicationsResult.data?.applications ?? [], + connectionsResult.data?.connections ?? [], + ); + if (refreshedConnection) { + startOAuth(refreshedConnection.id); + } else { + connectMutation.mutate(directOAuthEntry); + } + } finally { + directOAuthRetryingRef.current = false; + } + }} + onCancel={() => navigate("/apps/browse")} + /> + ); + } + const appName = connectResult?.application.name ?? entry?.name ?? @@ -326,6 +528,13 @@ export function AppsConnect() { byo={searchParams.get("byo") === "1"} source={searchParams.get("source")} onPick={(picked) => { + if ( + getAvailableConnectionMethod(picked)?.auth === "oauth" && + isMcpDirectOAuthConnectSlug(picked.slug) + ) { + navigate(appSourceConnectHref(picked.slug)); + return; + } setEntry(picked); setGalleryName(picked.name); setLinkUrl(""); @@ -388,7 +597,7 @@ export function AppsConnect() { return; } } - connectMutation.mutate(); + connectMutation.mutate(undefined); }} /> )} @@ -407,7 +616,7 @@ export function AppsConnect() { onKeyChange={setLinkKey} submitting={connectMutation.isPending} onBack={() => setStep("gallery")} - onConnect={() => connectMutation.mutate()} + onConnect={() => connectMutation.mutate(undefined)} /> )} @@ -417,7 +626,7 @@ export function AppsConnect() { onLinkChange={setLinkUrl} submitting={connectMutation.isPending} onBack={() => navigate("/apps/browse")} - onConnect={() => connectMutation.mutate()} + onConnect={() => connectMutation.mutate(undefined)} /> )} @@ -533,6 +742,85 @@ function StepHeader({ ); } +export function OAuthConnectStateScreen({ + entry, + phase, + error, + onRetry, + onCancel, +}: { + entry: AppDefinition; + phase: OAuthConnectPhase; + error?: string | null; + onRetry: () => void; + onCancel: () => void; +}) { + const status = phase === "entry" + ? { + title: `Connect ${entry.name} to Paperclip`, + body: `Paperclip will open ${entry.name} so you can choose a workspace and approve access.`, + } + : phase === "starting" + ? { + title: "Preparing secure sign-in", + body: `Paperclip is creating a secure ${entry.name} connection.`, + } + : phase === "redirecting" + ? { + title: `Opening ${entry.name}`, + body: `Continue in ${entry.name} to choose a workspace and approve access.`, + } + : { + title: `${entry.name} couldn’t connect`, + body: error ?? "Paperclip couldn’t start secure sign-in. Try again.", + }; + + return ( +
+ +
+
+ + {phase === "error" ? ( + + ) : phase === "entry" ? ( + + ) : ( + + )} + +
+

{status.title}

+

{status.body}

+
+
+ +
+ {phase === "error" ? ( + + ) : ( + + )} + +
+

+ + Your authorization stays in Paperclip’s encrypted secret store. +

+
+
+ ); +} + function ZapierConnectStep({ link, onLinkChange, @@ -675,12 +963,13 @@ function GalleryStep({ {filtered.map((app) => { const copy = appCopyFor(app.slug, app.description); const oauth = getAvailableConnectionMethod(app)?.auth === "oauth"; + const oauthBlocked = oauth && !isMcpDirectOAuthConnectSlug(app.slug); const unavailable = app.availability?.available === false; return ( + ) : ( + + )} ); diff --git a/ui/src/pages/apps/app-detail/SetupPanel.tsx b/ui/src/pages/apps/app-detail/SetupPanel.tsx index 6d03124296..105e57204c 100644 --- a/ui/src/pages/apps/app-detail/SetupPanel.tsx +++ b/ui/src/pages/apps/app-detail/SetupPanel.tsx @@ -46,7 +46,7 @@ export function SetupPanel({ {hasOAuthSignIn && ( ).connectedAt)} - isSmokeLabFixture={isSmokeLabFixture} + providerName={appDefinitionSlug(galleryEntry) === "notion" ? "Notion" : isSmokeLabFixture ? "Smoke OAuth" : "OAuth"} disabled={oauthStartDisabled} onStart={onStartOAuth} /> @@ -58,26 +58,25 @@ export function SetupPanel({ function OAuthConnectionSection({ connected, - isSmokeLabFixture, + providerName, disabled, onStart, }: { connected: boolean; - isSmokeLabFixture: boolean; + providerName: string; disabled: boolean; onStart: () => void; }) { - const providerName = isSmokeLabFixture ? "Smoke OAuth" : "OAuth"; return (

- {connected ? `Connected with ${providerName}` : `Connect with ${providerName}`} + {connected ? `${providerName} connected` : `Connect with ${providerName}`}

{connected - ? "Sign in again to replace this connection's OAuth session." + ? "Your workspace authorization is active. Reconnect any time to replace it." : "Open the provider's consent page to finish connecting this app."}

diff --git a/ui/src/pages/apps/store-cards.tsx b/ui/src/pages/apps/store-cards.tsx index 0cc505c12c..2bda5a0c20 100644 --- a/ui/src/pages/apps/store-cards.tsx +++ b/ui/src/pages/apps/store-cards.tsx @@ -1,6 +1,7 @@ import { ServerCog, Wrench } from "lucide-react"; import { Link } from "@/lib/router"; import { advancedTabHref } from "@/pages/tools/tool-tabs"; +import { appSourceConnectHref } from "./app-connect-policy"; /** Popular gallery keys surfaced first in the Browse store (PAP-13254, door 1). */ export const POPULAR_KEYS = ["zapier", "github", "slack", "notion", "linear"]; @@ -11,6 +12,9 @@ export const BYO_CONNECT_HREF = "/apps/connect?byo=1"; /** Zapier connects with the complete MCP URL issued by Zapier. */ export const ZAPIER_CONNECT_HREF = "/apps/connect?byo=1&source=zapier"; +/** MCP-direct OAuth apps enter through the generic source deep link. */ +export const NOTION_CONNECT_HREF = appSourceConnectHref("notion"); + /** * First-class "Connect your own tool" card (PAP-12371, Finding C; PAP-13254). * Lives in Browse as a persistent row and launches the guided URL flow. diff --git a/ui/storybook/stories/notion-connect-flow.stories.tsx b/ui/storybook/stories/notion-connect-flow.stories.tsx new file mode 100644 index 0000000000..b5f4e58455 --- /dev/null +++ b/ui/storybook/stories/notion-connect-flow.stories.tsx @@ -0,0 +1,177 @@ +import { useMemo } from "react"; +import type { Meta, StoryObj } from "@storybook/react-vite"; +import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { + CONNECTABLE_APP_DEFINITIONS, + type AppDefinition, + type ToolConnection, +} from "@paperclipai/shared"; +import { queryKeys } from "@/lib/queryKeys"; +import { Browse } from "@/pages/apps/Browse"; +import { AppLogo } from "@/pages/apps/AppLogo"; +import { + OAuthConnectStateScreen, + type OAuthConnectPhase, +} from "@/pages/apps/AppsConnect"; +import { SetupPanel } from "@/pages/apps/app-detail/SetupPanel"; +import { ReconnectCard } from "@/pages/apps/app-detail/AdvancedPanel"; + +const COMPANY_ID = "company-storybook"; +const NOTION = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "notion") as AppDefinition; +const ZAPIER = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "zapier") as AppDefinition; +const GITHUB = CONNECTABLE_APP_DEFINITIONS.find((app) => app.slug === "github") as AppDefinition; + +function seededClient() { + const client = new QueryClient({ + defaultOptions: { + queries: { staleTime: Infinity, gcTime: Infinity, retry: false, refetchOnMount: false }, + }, + }); + client.setQueryData(queryKeys.apps.gallery(COMPANY_ID), { apps: [NOTION, ZAPIER, GITHUB] }); + return client; +} + +function BrowseHost() { + const client = useMemo(() => seededClient(), []); + return ( + +
+ +
+ + ); +} + +function OAuthStateHost({ phase, error }: { phase: OAuthConnectPhase; error?: string }) { + return ( +
+ undefined} + onCancel={() => undefined} + /> +
+ ); +} + +function notionConnection(overrides: Partial = {}): ToolConnection { + return { + id: "connection-notion", + companyId: COMPANY_ID, + applicationId: "application-notion", + name: "Notion", + uid: "notion-storybook", + connectionKind: "managed", + ownership: "dcr", + transport: "mcp_remote", + authKind: "oauth", + status: "active", + transportConfig: { url: "https://mcp.notion.com/mcp" }, + config: { + url: "https://mcp.notion.com/mcp", + sourceTemplateKey: "notion", + oauth: { provider: "notion", connectedAt: "2026-08-06T19:00:00.000Z" }, + }, + credentialSecretRefs: [], + credentialRefs: [], + healthStatus: "healthy", + healthMessage: null, + healthCheckedAt: new Date("2026-08-06T19:00:00.000Z"), + lastError: null, + enabled: true, + createdByAgentId: null, + createdByUserId: "board-user", + createdAt: new Date("2026-08-06T18:55:00.000Z"), + updatedAt: new Date("2026-08-06T19:00:00.000Z"), + ...overrides, + }; +} + +function ConnectedHost() { + return ( +
+
+ +
+

Notion

+

Connected app setup

+
+
+ undefined} + appToggleDisabled={false} + onUpdateConfig={() => undefined} + configUpdateDisabled={false} + onStartOAuth={() => undefined} + oauthStartDisabled={false} + /> +
+ ); +} + +function ReconnectRequiredHost() { + return ( +
+
+

Notion

+

Connection needs attention

+
+ undefined} + /> +
+ ); +} + +const meta: Meta = { + title: "Apps/Notion MCP connect flow (PAP-16650)", + parameters: { layout: "fullscreen" }, +}; +export default meta; + +type Story = StoryObj; + +export const BrowseEntry: Story = { + name: "1 — Browse entry", + render: () => , +}; + +export const ConnectEntry: Story = { + name: "2 — Connect entry", + render: () => , +}; + +export const InFlight: Story = { + name: "3 — In flight", + render: () => , +}; + +export const Connected: Story = { + name: "4 — Connected", + render: () => , +}; + +export const ConnectError: Story = { + name: "5 — Connect error", + render: () => ( + + ), +}; + +export const ReconnectRequired: Story = { + name: "6 — Reconnect required", + render: () => , +};