diff --git a/AGENTS.md b/AGENTS.md index f37d0631c7..b8a706566a 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -205,7 +205,7 @@ All user-facing strings must use i18n — no hardcoded copy. Use `ApplicationError` with typed `ErrorCode` from `@openops/shared`: -~~~typescript +```typescript import { ApplicationError, ErrorCode } from '@openops/shared'; try { @@ -222,7 +222,7 @@ try { } throw err; } -~~~ +``` The Fastify error handler in `packages/server/api/src/app/helper/error-handler.ts` maps `ErrorCode` values to HTTP status codes automatically. diff --git a/docs/oauth-design.md b/docs/oauth-design.md new file mode 100644 index 0000000000..b00ae5db4c --- /dev/null +++ b/docs/oauth-design.md @@ -0,0 +1,608 @@ +# External Agent OAuth — Design + +**Date:** 2026-07-27 +**Status:** Approved +**Linear:** Fixes OPS-4673 +**Supersedes:** the `feat/mcp-oauth-authentication` spike in `openops-internal` and its +three specs (2026-07-20 base, 2026-07-21 hardening, 2026-07-21 generalization). This is +a fresh design informed by an adversarial security audit of that spike. + +## Problem + +OpenOps ships a Python **FastMCP** server (`mcp-server/`) that exposes a filtered set of +OpenOps API routes as MCP tools. Today it authenticates with a single static +`AUTH_TOKEN` env var used as a `Bearer` JWT on every API call. That works for the +built-in AI chat (the Node API spawns it over stdio and injects a short-lived `SERVICE` +JWT) but not for **external agents** — Claude Code, Codex, Claude.ai/ChatGPT connectors, +M365 Copilot, partner CLIs — which need a self-service, revocable credential that works +when SSO is enabled and password login is disabled (OPS-4673). + +## Requirements (locked) + +1. **Clients:** all of — M365 Copilot (strictest: OAuth 2.1 + DCR + Streamable HTTP, + valid discovery, no API keys), Claude.ai/ChatGPT web connectors, dev CLIs + (Claude Code/Codex, loopback redirects), and custom/partner agents calling the + **REST API directly** with an OAuth token (no MCP in between). +2. **Deployments:** cloud **and** self-hosted → the authorization server ships inside + the OpenOps product (Node API) and delegates login to whatever auth the deployment + uses. No dependency on Frontegg or any external IdP. +3. **Topology:** one MCP server co-deployed per OpenOps instance (path-routed on the + same public host). Not multi-tenant. +4. **Connections:** a user may hold **several independent connections**, including + more than one for the same agent. Each is authorized, listed and revoked on its + own. Single full-access scope per resource in v1 (`mcp`, `api`). +5. **Projects:** every OAuth-issued token carries a required `project_id` claim and + acts only on that project, so an individual token's authority is fixed for its whole + life and cannot be redirected by changing stored state. A _connection_ is not fixed: + it can act wherever its user can, by asking for a different project when it gets a + token. That mirrors how enterprise's `POST /v1/authentication/switch-project` issues + a new token per project rather than mutating state — the switch produces a new + credential, never a rewritten one. The bound is the user's own membership, re-read on + every mint and again on every request, so a connection can never reach further than + its user could in the browser. This edition has one project per organization, so + there is usually nowhere else to go; the mechanism is the same either way. +6. **Revocation is a hard requirement:** users/admins revoke a connection and it stops + working promptly. + +## Standards targeted + +- **OAuth 2.1** (PKCE mandatory, refresh rotation, exact redirect matching). +- **MCP Authorization spec 2025-11-25**: RFC 9728 Protected Resource Metadata + + `WWW-Authenticate`; RFC 8414 AS metadata **and** OIDC-Discovery-compatible document; + RFC 8707 resource indicators; DCR (RFC 7591) now, CIMD (SEP-991) as a follow-up. +- **RFC 8693** token exchange (RS → API-audience tokens; no token passthrough). +- **RFC 7009** revocation; **RFC 9207** `iss` authorization-response parameter. +- Honest metadata only: nothing advertised that isn't actually served. + +## Audit findings this design must fix (from the spike review) + +| ID | Finding | Fix in this design | +| ----- | ------------------------------------------------------------------------------------------------------------------------------------- | ----------------------------------------------------------------------------------------------------------------------------------- | +| H1 | Refresh-token reuse undetected | Token **families**; reuse revokes the family | +| H2 | Grant revocation didn't revoke refresh tokens | Revocation cascades via indexed `grantId` | +| H3 | Consent forgeable from URL params (one-click account grant) | Server-side **pending-authorization record**; consent references an opaque `request_id`; client metadata rendered from DB only | +| H4 | Open redirect on Deny | Deny goes through the server; redirect validated against registered URIs | +| M1 | Code/refresh consumption race (read-then-write) | Atomic conditional `UPDATE … WHERE consumedAt IS NULL` | +| M2 | Static form-field exchange secret, `change-me` default, unrate-limited | RS is a **confidential client** with a generated high-entropy secret (hashed at rest), `client_secret_basic`, rate-limited failures | +| M3 | Audience deny-list in one handler; websockets bypass | **Positive** audience enforcement inside `extractPrincipal` (single chokepoint) | +| M4 | One HS256 secret signs everything; fake `jwks_uri` | Dedicated **RS256 keypair + real JWKS** for OAuth tokens | +| M5 | In-process `_active_project_by_user` map (cross-session leakage, restart loss) | No mutable server-side project state: the project is a claim on each token, and switching mints a new token rather than editing one | +| M6 | 2× remote exchange per tool call; proceeds unauthenticated on failure | Local JWKS validation; exchange only to mint API tokens, cached, **fail-closed** | +| M7 | Non-RFC 6749 error bodies | Dedicated OAuth error serializer for `/v1/oauth/*` | +| M8 | Phantom grants at consent | Grant created at code redemption, not at consent (repeat authorizations are intentionally separate connections) | +| L1–L6 | DCR validation gaps, cleanup gaps, migration nits, lying metadata, cookie-over-bearer precedence, `/switch-project` minting primitive | Addressed in the relevant sections below | + +## Architecture + +### Roles + +- **Node API (Fastify)** — OAuth 2.1 **Authorization Server** (new module + `packages/server/api/src/app/oauth/`) _and_ a protected resource: the `direct` token + model lets CLIs call the REST API with an OAuth token (`aud = api`). Login/consent + ride the existing app session, so SSO and password deployments both work. +- **Python FastMCP server (`mcp-server/`)** — MCP **Resource Server** over Streamable + HTTP. Validates inbound bearers **locally** via the AS JWKS (FastMCP `JWTVerifier` + + `RemoteAuthProvider`). Never forwards the client token: per tool call it exchanges it + (RFC 8693) for a separate short-lived API-audience token, cached, fail-closed. +- **Resource registry** (static config in the AS): `mcp` (canonical URI = public MCP + URL, token model `exchange`) and `api` (canonical URI = API URL, token model + `direct`). `resource` on `/authorize` and `/token` is validated against it + (`invalid_target` otherwise) and binds the token `aud`. + +### End-to-end flow + +1. Client → `https:///mcp` unauthenticated → `401` + + `WWW-Authenticate: Bearer resource_metadata="…"`. +2. Client fetches `/.well-known/oauth-protected-resource[/mcp]` (served by RS) → learns + the AS issuer. +3. Client fetches AS metadata (RFC 8414 and/or OIDC discovery), registers via DCR, + opens `/oauth/authorize` with PKCE (S256) + `state` + `resource`. +4. AS validates everything, persists a **pending-authorization record**, sends the + browser to Settings → Connected apps with only an opaque `request_id`. + Unauthenticated users go through normal app login (SSO-aware) first. +5. The consent dialog fetches client metadata **from the server by `request_id`** (never + from URL params), user approves/denies. Approve → single-use code bound to the + record; deny → server-validated `error=access_denied` redirect. Dismissing the dialog + denies, so a client is never left waiting on a decision the user has walked away + from. Both redirects carry `state` and `iss` (RFC 9207). +6. Client exchanges code at `/oauth/token` (PKCE verifier + `resource`) → RS256 access + token (`aud` = resource) + rotating refresh token. Grant activated/upserted here. +7. MCP calls: RS validates locally via JWKS (issuer + audience + exp), exchanges for an + API-audience token (cached ≈60s, fail-closed), calls the API. Direct clients skip + the RS and hit the API with their `aud=api` token. +8. API-side: `extractPrincipal` verifies signature by `kid`, enforces `aud=api` + positively, maps claims → `SERVICE` principal with the user's **real project role**, + and checks grant status (cached ≈60s) → revocation cuts access in ~1 minute. + +## Tokens & keys + +### Why a dedicated asymmetric keypair + +Today every JWT (sessions, worker ~100y tokens, engine, AI-chat) is HS256 under the one +`OPS_JWT_SECRET`. Symmetric signing means whoever can _verify_ can also _mint_ — so the +verification key can never be shared with the Python RS (forcing the spike into remote +validation per request), and a single leak forges every principal type. + +OAuth-issued tokens are therefore signed with a **dedicated RS256 keypair**: + +- The **public key** is published at a real `GET /.well-known/jwks.json`; the RS (and + any future resource server) validates tokens locally, in-process. An AS blip no + longer takes down MCP traffic. +- RS256 over EdDSA purely for client compatibility (M365, Python/Node stacks all verify + RS256 out of the box). Ed25519 is a documented follow-up. +- **Two isolated trust domains:** the internal HS256 world is untouched (zero + regression on workers/engine/sessions); compromising the OAuth key forges only + OAuth tokens — which remain subject to the per-request grant-status check, so the + damage is revocable. Compromising `OPS_JWT_SECRET` no longer exposes external-agent + auth and vice versa. + +### Key management + +- **Bootstrap:** on first boot the API generates an RSA-2048 keypair, encrypts the + private key with the existing AES-256-CBC mechanism (`encrypt-compress.ts`, same + protection level as app-connection credentials), stores it in `oauth_signing_key`, + serves the public half in the JWKS. Zero new config for self-hosted; multi-instance + replicas share the key via the DB (creation is guarded by a unique active-key + constraint so concurrent boots converge). +- **Override:** optional system prop pointing at an operator-provided PEM (Vault/KMS + users) — the DB path is a default, not a cage. +- **Rotation (`kid`-based):** generate key #2, publish both in JWKS, sign new tokens + with #2, drop #1 from JWKS after every #1-signed token has expired (access TTL is + 15 min, so the horizon is short). Admin-triggerable; also the recovery path for a + suspected key compromise. Every OAuth JWT header carries its `kid`; + `extractPrincipal` dispatches on it (legacy internal `kid: '1'` → HS256 path). + +### Token shapes + +| Token | Form | TTL (default, configurable) | Notes | +| ------------------- | ------------------------------- | --------------------------------- | ----------------------------------------------------------------------------------------------------------------------------- | +| Authorization code | 32B CSPRNG, SHA-256 hash stored | 60 s, single-use (atomic consume) | Bound to client, redirect_uri, PKCE challenge, resource, user, pending request | +| Access token | RS256 JWT | **15 min** | Claims: `iss`, `sub` (userId), `aud` (resource audience), `exp`, `iat`, `jti`, `client_id`, `scope`, `grant_id`, `project_id` | +| Refresh token | 32B CSPRNG, SHA-256 hash stored | 30 d absolute; rotates on use | Carries `grantId` + `familyId` (both indexed) | +| Exchanged API token | RS256 JWT, `aud = api` | ~5 min | Minted at token-exchange for the grant's active project; never returned to end clients | + +Opaque secrets are never stored in plaintext; comparisons are hash-lookup or +timing-safe. All token responses set `Cache-Control: no-store`. + +## Authorization server surface + +All under `/v1/oauth/*` + well-known routes, registered **only when +`OPS_OAUTH_ENABLED=true`**, as public routes in the security chain (each endpoint does +its own auth), with a dedicated **RFC 6749 error serializer** (`{"error": +"invalid_grant", "error_description": …}`, correct 400/401 statuses) instead of the +ApplicationError envelope. + +- `GET /.well-known/oauth-authorization-server` and + `GET /.well-known/openid-configuration` — same truthful document: issuer, endpoints, + `code` response type, `authorization_code`/`refresh_token` grants, S256, + `token_endpoint_auth_methods_supported: ["none","client_secret_basic"]`, real + `jwks_uri`, scopes. **No** fake id-token fields. Served from the issuer origin only + (the spike's RS-origin copy with mismatched issuer is dropped — strict RFC 8414 + clients reject it). +- `GET /.well-known/jwks.json` — active + retiring public keys. +- `POST /oauth/register` (DCR, public): validates and bounds every field + (`redirect_uris` ≤ 10, https or loopback only, length caps, `grant_types` whitelist — + **enforced later at `/token`**, L1), returns RFC 7591 bodies/errors. Rate-limited + per-IP (existing rate-limit module). Registered clients are `token_endpoint_auth_method: +none` (public, PKCE-only). +- `GET /oauth/authorize` — requires a logged-in app session (redirects into normal + login, SSO-aware, then back). Validates client, **exact** redirect_uri (https or + loopback; loopback matches any port per RFC 8252), PKCE S256-only, known `resource`, + scope ⊆ resource scopes. On unknown client/unregistered redirect_uri: render an error + page, **never redirect**. On success: persist `oauth_pending_authorization` + (~10 min TTL, single-use) and redirect the browser to Settings → Connected apps with + only `?request_id=`. +- `GET /oauth/requests/{id}` (USER session) — consent-page data: client name + from the **DB**, scopes, resource id. Any signed-in user holding the (unguessable) + request id can read it: the record is not bound to a user until the decision is + submitted. +- `POST /oauth/requests/{id}/decision` (USER session) — `{approve: boolean}`. + Atomically consumes the pending record (its single-use consumption is the CSRF/replay + barrier; the session cookie is `sameSite: lax`, and the route additionally requires a + custom header to defeat form-post CSRF). Approve → upsert grant (see below), issue + code, return the validated redirect URL (`code`, `state`, `iss`). Deny → + `error=access_denied` redirect URL, equally validated. The frontend only ever + navigates to server-returned URLs (fixes H3 + H4). +- `POST /oauth/token` (public, rate-limited with failure-weighted limits): + - `authorization_code` — atomic single-use consume; verify PKCE (timing-safe), + client, redirect_uri, resource; enforce the client's registered `grant_types`; + mint access + refresh (new `familyId`), activate the grant. + - `refresh_token` — atomic rotate; **reuse of a rotated/revoked token revokes the + entire family** (H1) and logs a security event; checks grant active + user active + on every rotation (H2); re-binds `resource`. Accepts an optional `project_id` to + move the connection, checked against membership **before** the token is consumed, so + a refused switch does not cost a working credential. + - `urn:ietf:params:oauth:grant-type:token-exchange` — **RS-only**: authenticated via + `client_secret_basic` with the RS's confidential client (secret generated at + provisioning, stored hashed, timing-safe compare, rate-limited failures — M2). + Validates the subject token (signature, `aud = mcp`, exp), checks grant active + + user active + membership of the target project, mints the ~5 min `aud=api` token. + The target defaults to the project the subject token names; the RS may pass + `project_id` to act elsewhere, which is how an agent switches project. It cannot + widen beyond the user's own membership, which is re-read here on every exchange. +- `POST /oauth/revoke` (RFC 7009, public with client identification): revokes by + refresh token → marks grant + family revoked. +- `GET /oauth/grants` / `DELETE /oauth/grants/{id}` (USER, project-scoped policy): + connected-apps management. Delete = revoke grant **and cascade-revoke all its refresh + tokens** (indexed `grantId` UPDATE — H2). +- `GET /oauth/projects` (USER **or SERVICE**): the projects the caller may act in, and + which one they are acting in now. `SERVICE` is allowed because this is the one route a + connection calls about itself, to find out where it can switch to; it exposes only the + names of projects the caller already reaches. + +### Grant model + +`oauth_grant` — one row per **connection**: one completed authorization for one +client and user. Created at code redemption (**not** at consent, so an +authorization the client never finished is not shown as a connection): +`id`, `clientId`, `userId`, `resourceId`, +`status (active|revoked)`, `createdAt`, `lastUsedAt`, `revokedAt`. + +The index on `(clientId, userId)` is deliberately **not unique**. Authorizing the +same agent again creates another connection rather than mutating the first, so a +user can run several agents — or several installs of one agent — side by side and +revoke any one of them without disturbing the others. `projectId` is fixed at +authorization time and never mutated (see requirement 5). + +Because reconnecting accumulates rows, the cleanup job removes **dead** +connections: those with no unrevoked refresh token left and unused for 30 days. A +connection with any usable refresh token is never touched. + +Revocation semantics, per connection: revoked grant → token exchange refuses (MCP +cutoff), the API's grant-status check refuses (direct cutoff), and refresh +refuses, so no new tokens can be minted. Access-token TTL (15 min) is the absolute +worst case, and other connections are unaffected. + +### Project authorization + +The project a token may act on is a **required `project_id` claim**, minted by the +authorization server and never asserted by the client. Each individual token is +immutable — the project it names is fixed for its whole life, so a leaked token's blast +radius is fixed with it. + +The claim is a _selector, not a grant of authority_. Every request that presents an +OAuth token re-authorizes the named project, so withdrawing someone's access takes +effect at their next request rather than at token expiry. + +**Switching project.** Because the claim is a selector, a connection is not confined to +one project — it acts wherever the user can, exactly as their browser session does. A +client asks for a different project when getting a token, and membership decides: + +- `POST /token` with `grant_type=refresh_token` and `project_id` — how a direct API + client (CLI, partner agent) moves. +- `POST /token` with the token-exchange grant and `project_id` — how a resource server + moves on an agent's behalf. This is the path an MCP client such as Claude Code takes, + since it cannot mint tokens itself. +- `GET /v1/oauth/projects` — where a connection may go, and where it is now. Allows + `SERVICE` so the connection itself can ask. + +A project the user is not a member of is refused with `invalid_target` (RFC 8707), and +on the refresh path the refusal happens **before** the token is consumed, so asking for +the wrong project does not cost a working credential. Nothing is stored: a switch lasts +exactly as long as the token it produced, and `oauth_grant.projectId` continues to +record where the connection started rather than where it is. + +The security property is that a switch can never reach further than the user can. The +bound is their own membership, re-read on every mint and again on every request. + +The three questions the server asks about projects sit behind one factory, +`getOAuthProjectMembershipService()` — following the convention used by +`authentication-service-factory` and friends, where an edition overrides behaviour +by swapping the import in the factory file: + +- `getDefaultForUser(user)` — where a newly authorized connection starts. +- `getForUser(user, projectId)` — whether the user may act there, and as what role. +- `listForUser(user)` — every project the connection may switch to. + +`listForUser` must stay consistent with `getForUser`: if it returned less than +`getForUser` permits, a client would be shown one destination while the token endpoint +allowed another it was never told about. This edition answers all three from the +organization's projects with role `ADMIN`, matching the session login path. An edition +with real project membership maps them onto its own lookups (in the enterprise fork, +`usersService.getLandingProjectForUser` and `usersService.getUserProject`, which +already return `{ project, projectRole }`) and gets real per-project roles and +multi-project switching with no change to the OAuth code. `projectRole` is deliberately +typed as `string` here because the role enum lives in enterprise-only shared code. + +**The project lives on the refresh token, not the grant.** It was on the grant first, +used as the default when a refresh named no project — which meant a plain renewal put the +connection back where it started, silently discarding a switch. An agent would have moved +to another project and drifted back roughly 15 minutes later, with nothing to attribute it +to. The refresh token is the credential chain, so it is what carries the current project: +rotation copies it forward unless the client asks to move, and renewing a credential +therefore yields an equivalent one. That also left the grant's copy unread, so it is +gone — `test/unit/oauth/tokens.service.test.ts` pins the behaviour. + +### Data model (new tables) + +- `oauth_signing_key` — `id` (kid), `privateKeyEncrypted`, `publicKeyPem`, + `status (active|retiring|retired)`, timestamps. A partial unique index over + `status = 'active'` is what makes concurrent replica boots converge on one key. +- `oauth_client` — DCR clients + the provisioned RS confidential client: + `id`, `clientName`, `redirectUris` (jsonb), `grantTypes` (jsonb), + `tokenEndpointAuthMethod`, `clientSecretHash` (nullable), timestamps. + Usage is recorded per connection on the grant, not per client. No `scope`: a client may + send one at registration, but what a token gets is decided by the resource it names, so + storing the request would be a second answer nothing reads. +- `oauth_pending_authorization` — `id` (opaque request_id), `clientId` (FK), + `redirectUri`, `codeChallenge`, `resource`, `scope`, `state`, `expiresAt`, + `consumedAt`. No `userId`: the acting user is not known until the decision is + submitted (see deviation 2). +- `oauth_authorization_code` — `codeHash` (unique), `clientId` (FK), `userId`, + `redirectUri`, `codeChallenge`, `resource`, `scope`, `expiresAt`, `consumedAt`. +- `oauth_refresh_token` — `tokenHash` (unique), `grantId` (FK, **indexed**), + `familyId` (**indexed**), `clientId`, `resource`, `scope`, `projectId`, `expiresAt` + (**indexed**), `revokedAt`. No `userId`: the grant records the acting user and is + authoritative, so a copy here could only ever disagree. `projectId` **is** here rather + than on the grant: it is where the chain is currently acting, so a rotation carries it + forward and a plain renewal stays put. +- `oauth_grant` — as above; FKs with `ON DELETE CASCADE`; no defaulted-to-`''` + columns (L3); `(clientId, userId)` indexed but **not** unique. No `scope`: it would + restate `resourceId`, since each resource grants exactly one. No `projectId` either — + see below. `revokedAt` is write-only on purpose — `status` is what code branches on, + and this answers "when" for anyone auditing later. + +All single-use consumption (pending record, code, refresh rotation) is an atomic +conditional `UPDATE … WHERE … AND consumedAt IS NULL` branching on affected rows (M1). + +## API-side enforcement (Node) + +- **`extractPrincipal` dispatch by `kid`:** HS256 legacy path unchanged. RS256 OAuth + path: verify against local keys, require `aud` = API audience (**positive** + enforcement — an `aud=mcp` token can never authenticate anywhere in the API, + including the websocket path, M3), map claims → `SERVICE` principal with `sub` as + userId, the grant's active project, and the user's **real project role** resolved + from membership (no hardcoded ADMIN); reject missing membership or inactive user. +- **Grant-status check:** for principals carrying `grant_id`, a cached (≈60 s, + in-process; Redis when available) single-row status read; revoked → 401. +- **Bearer/cookie precedence (L5):** the `Authorization` header wins over the `token` + cookie in `access-token-authn-handler.ts`, with regression tests for the app's + cookie-based flows. +- Route policies: OAuth-derived `SERVICE` principals flow through the existing ~40 + `[USER, SERVICE]` route policies unchanged. +- **`SERVICE`, never `USER`.** `ProjectAuthzHandler` rejects a request naming a project + other than the principal's, but enterprise's `/switch-project` is on that handler's + ignore list, because minting a token for another project is its whole job. What keeps + an OAuth connection out of it is its policy, + `getUnscopedRoutePolicy([PrincipalType.USER])`: a `SERVICE` principal gets + `403 invalid route for principal type`. Do not build the OAuth principal as `USER`, + and do not add `SERVICE` to `/switch-project`. OAuth connections switch project + through the token endpoint instead (below), which is the same capability with the + membership check kept in one place. Pinned by + `test/unit/oauth/oauth-principal.test.ts`. +- **`SERVICE` is in `DEFAULT_ALLOWED_PRINCIPAL_TYPES`**, so a route that declares no + policy is reachable by an OAuth token. The project guard still applies, so this is a + project-scoped reachability question rather than a cross-project one — but it means + the set an OAuth connection can touch is "everything not explicitly restricted", + not "everything explicitly opened". Worth keeping in mind when adding routes. + +## Python resource server (`mcp-server/`) + +- `MCP_TRANSPORT=stdio` (unchanged, internal AI chat) or `http` (Streamable HTTP, + `stateless_http=true`). +- **Auth:** FastMCP `JWTVerifier` (`jwks_uri`, `issuer`, `audience = MCP canonical +URI`) wrapped in `RemoteAuthProvider` → serves RFC 9728 PRM (root and path-aware + variants) and enforces local validation. ASGI middleware adds + `WWW-Authenticate: Bearer resource_metadata="…"` on 401 (kept from the spike — it + was correct). Origin-header validation per MCP 2025-11-25 (403 on bad Origin). +- **Downstream calls:** httpx request hook obtains the API token from an + **exchange-token cache** keyed by `(sha256(subject token), projectId)` with TTL + `min(remaining subject exp, 60 s)`; on miss, calls `/v1/oauth/token` + (token-exchange) authenticated with its confidential-client credentials + (`client_secret_basic`, from env, provisioned at deploy). **Fail-closed:** exchange + failure aborts the tool call with an MCP auth error; no request ever leaves without + an `Authorization` header (M6). +- **No project switching:** a connection acts on the project fixed on its grant. + Multi-project access is enterprise (requirement 5), so the resource server keeps + no project state of its own — which is also what removes the class of bug behind + audit finding M5 rather than merely relocating it. +- No AS metadata is served from the RS origin. + +## Consent UI (react-ui) + +Consent and connection management live in one place — `/settings/connected-apps` — so the +user decides where they later review and revoke. + +- **Consent dialog**, shown over that page when the URL carries a `request_id`. Reads + only the id, fetches `GET /v1/oauth/requests/{id}` and renders the client name **from + the server**, never from URL params. Plain-language copy naming what is granted, + including that the connection may act in any project the user has access to. Approve + and Deny both POST the decision and navigate to the server-returned URL only. + Dismissing counts as Deny. The response deliberately carries **no project**: a + connection is not confined to one, and naming the project it starts in would read as a + limit that does not exist. +- **Connected apps list** on the same page: one row per authorization — not per + application, since two connections for the same agent are independently revocable — + with client name, when connected, when last used, and Disconnect behind a + confirmation. Hidden entirely by the `CONNECTED_APPS_ENABLED` flag when OAuth is off, + since every route it depends on is then unregistered. All strings i18n; `react` skill + patterns. + +## Abuse controls & hygiene + +- Rate limits (existing module, per-IP): `/register`, `/authorize`, `/token` + (failure-weighted so refresh cadence is never throttled), exchange failures. +- Cleanup job (existing system-jobs): indexed range-deletes of expired pending + records, codes, and expired refresh tokens; stale-client removal via + `NOT EXISTS` query (no full-table loads); runs hourly. + - **Retention is anchored to expiry, including for revoked rows.** A rotated refresh + token is kept until the moment it could no longer be presented anyway, because that + is exactly the window in which a replay must be recognised as _reuse_ — which revokes + the family and logs a security event — rather than reported as an unknown token. An + independent, shorter window would quietly turn a replay of an older token into a + plain `invalid refresh token`: still rejected, but with the compromise signal lost + precisely because the token was old. Growth is bounded by the refresh TTL, so pick + that TTL with the table in mind rather than adding a second knob here. + - The **handler is registered on every boot**, including when OAuth is disabled, and + returns immediately in that case. The schedule lives in Redis and outlives the boot + that created it, so an instance that enabled OAuth once and later turned it off still + has the job firing; with no handler registered the worker fails it hourly. +- Security telemetry: log DCR registrations, refresh-reuse family revocations, exchange + auth failures, revocations. + +## Configuration (system props) + +| Prop | Default | Purpose | +| ------------------------------------------------------------------------------------------------------------------------------------------- | ------------------------- | -------------------------------- | +| `OPS_OAUTH_ENABLED` | `false` | Registers AS routes + well-known | +| `OPS_OAUTH_ISSUER_URL` | derived from frontend URL | `iss`, metadata | +| `OPS_OAUTH_ACCESS_TOKEN_TTL_SECONDS` | 900 | | +| `OPS_OAUTH_REFRESH_TOKEN_TTL_DAYS` | 30 | | +| `OPS_OAUTH_SIGNING_KEY_PEM_PATH` | unset | operator-managed key override | +| `OPS_MCP_RESOURCE_URL` | unset | canonical MCP resource URI | +| RS env: `MCP_TRANSPORT`, `MCP_OAUTH_ISSUER`, `MCP_RESOURCE_URI`, `MCP_CLIENT_ID`, `MCP_CLIENT_SECRET`, `API_BASE_URL`, `OPENAPI_SCHEMA_URL` | | | + +Deploy: path routing on the public host — `/mcp` + PRM → RS; `/v1/oauth/*` + +well-known → Node API. + +## Testing + +Every audit finding becomes a regression test. Highlights: + +- **Protocol:** PKCE fail/pass, code replay (including **concurrent** replay — M1), + expiry, cross-client code, redirect mismatch, unknown resource, state/iss round-trip, + DCR field bounds, registered-grant-type enforcement. +- **Consent binding:** decision without a pending record fails; expired/consumed + record fails; client name rendered from DB; deny redirect validated (H3/H4). +- **Refresh:** rotation; family revocation on reuse; grant-revoked → refresh refused; + absolute expiry (H1/H2). +- **Audience:** `aud=mcp` token rejected by REST **and websocket**; `aud=api` accepted; + legacy HS256 tokens unaffected (M3); exchanged token unusable at the RS. +- **Exchange:** requires RS client credentials; revoked grant/inactive user refused; + cache respects TTL; fail-closed on AS outage (M6). +- **Keys:** boot generation idempotent across concurrent replicas; rotation keeps old + tokens valid until expiry; JWKS serves retiring keys. +- **Python:** JWKS validation (valid/expired/wrong-aud/wrong-iss), PRM contents, 401 + challenge header, project-switch persistence. +- **E2E:** scripted MCP client (DCR → authorize → consent → token → tool call → + refresh → revoke → cutoff) with SSO on and off; stdio AI-chat regression; CLI-style + direct flow (loopback + `resource=api`). + +## Phasing + +- **P1 — AS core:** signing keys + JWKS, entities/migrations, DCR, pending-auth + record + authorize, token endpoint (code/refresh/exchange, atomic consumption, + families), grants + revocation, OAuth error serializer, discovery docs, rate limits, + cleanup. Unit + integration tests. +- **P2 — API enforcement:** `extractPrincipal` kid dispatch + positive audience, + real-role principal mapping, grant-status check, bearer-over-cookie. Regression + suite. +- **P3 — Python RS:** http transport, JWKS verifier, PRM + challenge middleware, + exchange client + cache + fail-closed, project-switch tool. +- **P4 — UI:** consent dialog, connected-apps settings page. +- **P5 — Deploy/E2E:** config, path routing, Docker, E2E matrix. + +Status: P1, P2 and P4 are complete in this repository. P3 lives in the `openops-mcp` +repository and is complete apart from the project-switch tool — the API side of switching +is built and tested here, but nothing in the resource server calls it yet. P5 is +outstanding in `openops-mcp`: Dockerfile, README, and the Helm values for +`openops-cloud/helm-chart`. + +The switch tool needs somewhere to hold the selection, and **that is where audit finding +M5 came from** — an in-process `_active_project_by_user` map that leaked across sessions +and vanished on restart. HTTP mode runs `stateless_http`, so the same map would work on +one instance and silently diverge across replicas. Whatever holds the selection has to be +per-connection and either shared or carried in the request; reaching for a process-local +dictionary would reintroduce the finding this design set out to fix. + +## Verification + +Unit tests cover each module in isolation. Because those use in-memory +repositories, the guarantees that depend on database semantics are covered +separately in `test/integration/ce/oauth/`: that single-use consumption of codes, +pending records and refresh tokens is atomic under concurrency; that revocation +cascades to one connection's tokens only; and that the cleanup job deletes what has +expired and nothing else. Writing those found a real defect — date cutoffs bound as +ISO strings are compared _textually_ by drivers that store a different textual +format, which matched every row including future ones. Cutoffs are now bound as +`Date` objects (`oauth-query.ts`). + +The integration harness runs on SQLite with schema synchronisation, which is not +the production driver. It does replace hand-written mocks with a real ORM and real +SQL, which is where the risk was. + +## Deviations found during implementation + +Recorded here because each one changes what the code does versus what this +document originally specified. + +1. **Project role is resolved through a seam, not hardcoded.** The design called + for the user's "real project role", which this edition cannot provide: it has no + per-project role model, and session logins hardcode `'ADMIN'` too. Rather than + hardcode it in the OAuth path as well, the role comes from + `getOAuthProjectMembershipService().getForUser(...)`, which returns `'ADMIN'` + here and the member's actual role in an edition that has one. The v1 scope model + is still coarse — a single full-access scope means a connected agent can do what + its user can — but the role is no longer baked into OAuth code. +2. **No `userId` on the pending authorization record.** `GET /authorize` is + reachable before the user has logged in, so the acting user is not known when + the record is written; it is taken from the session when the decision is + submitted and recorded on the grant. +3. **Authorization codes carry no `grantId`.** The grant is created when the code + is redeemed, which is after the code exists. The code references the client + and user instead. +4. **A failed redemption consumes the code.** The code is claimed before PKCE and + the other parameters are checked, so one wrong `code_verifier` burns it. This + is deliberate — it allows exactly one verifier guess per code — and the cost + is only that a party who already holds a code can deny the legitimate client + that one code. +5. **The project moved onto the token; switching then came back, deliberately.** The + design originally wanted an all-projects grant with runtime switching. That became a + required `project_id` claim instead, which is what resolves audit finding M5 — no + mutable project state for sessions to share — and fixes each token's authority for + its whole life. + + An intermediate step went further and refused to build any switching at all, on the + grounds that enterprise's `/switch-project` already mints a token per project. That + was wrong twice over. `/switch-project` is unreachable for an OAuth connection (it + requires `PrincipalType.USER` and session cookies), so refusing to build a mechanism + did not defer the capability to enterprise — it removed it. And an agent confined to + one project is not the parity users expect from a tool acting on their behalf. + + Switching is therefore built here, at the token endpoint, where the membership check + already lives (see _Project authorization_). The claim stays immutable per token; what + moves is the connection, by asking for a new token. That keeps M5 resolved — still no + mutable server-side state — while making the capability reachable. + +6. **Revocation is effectively immediate on a single instance**, not merely + within the ~60 s cache TTL: revoking busts the in-process grant cache. The TTL + bound applies across replicas, whose caches are not invalidated. +7. **Six columns the design named were removed as write-only.** `oauth_client.lastUsedAt` + (usage is meaningful per connection, on the grant) and `oauth_signing_key.alg` (one + algorithm, reported by the JWKS from a constant) went first. A later sweep took + `oauth_client.scope`, `oauth_grant.scope` and `oauth_refresh_token.userId` for the + same reason — each was written, and in two cases echoed through an API response, but + never consulted for a decision. Scope is settled by the resource; the acting user is + settled by the grant. `oauth_grant.revokedAt` was kept despite being write-only: it is + an audit answer to "when", which `status` alone cannot give. + + `oauth_grant.projectId` was the sixth, and the only one whose removal fixed a bug + rather than just saving a column. It was read — as the default when a refresh named no + project — and that default was wrong: a plain renewal returned the connection to where + it started, discarding a switch made minutes earlier. The project moved to + `oauth_refresh_token`, which is the chain being rotated, so renewal now preserves it. + +8. **Bearer now beats the session cookie** in `access-token-authn-handler.ts` + (was cookie-first). A caller presenting a token is stating which identity it + wants; preferring an ambient cookie would authenticate it as someone else. + +## Deferred (tracked follow-ups) + +- CIMD client registration (SEP-991) — accept URL client_ids. +- Fine-grained scopes (read/write, per-capability) + incremental consent (SEP-835). +- DPoP sender-constrained tokens. +- Ed25519 signing option. +- Multi-tenant/central MCP topology (would reuse the JWKS trust model as-is). +- A user-supplied label per connection. Connections are currently told apart by + client name, creation time and last use, which is thin when someone connects the + same agent from two machines. + +## Out of scope + +- Per-project _consent_. A connection is authorized against the user's account and may + act in any project they can reach, which is the parity a tool acting on someone's + behalf needs. Letting a user grant one project and withhold another would be a + finer-grained consent model, and belongs with the scopes work below. +- API keys / PATs (M365 Copilot cannot use them). +- RFC 7592 client management endpoints. +- Changes to internal HS256 token flows (sessions, worker, engine, AI-chat stdio). diff --git a/docs/oauth-manual-testing.md b/docs/oauth-manual-testing.md new file mode 100644 index 0000000000..bb897dbe3e --- /dev/null +++ b/docs/oauth-manual-testing.md @@ -0,0 +1,197 @@ +# Testing external-agent OAuth locally + +How to exercise the OAuth 2.1 authorization server by hand. Design: +`docs/oauth-design.md` (OPS-4673). + +The whole chain works end to end: an MCP client discovers the server, registers, +opens the browser at the consent screen, and receives a token. Two ways to test it +— [by hand with the script](#walk-the-whole-flow), which needs no browser, or +[with a real client](#connect-a-real-client), which is what users will do. + +The MCP resource server lives in its own repository, `openops-mcp`. + +## Start the API with OAuth on + +OAuth is off by default and every route 404s until it is enabled. Postgres is +required — the migration is registered for Postgres only. + +```bash +docker compose up -d --wait + +export $(grep -v '^#' .env | xargs) # your usual local settings +export PATH="$PWD/node_modules/.bin:$PATH" # the block rebuild step needs nx + +export OPS_OAUTH_ENABLED=true +export OPS_OAUTH_ISSUER_URL=http://localhost:3000 # public base URL of this API +export OPS_MCP_RESOURCE_URL=http://localhost:3020/mcp +export OPS_OAUTH_RS_CLIENT_SECRET=$(openssl rand -hex 32) + +npx nx build server-api && node dist/packages/server/api/main.js +``` + +First boot generates the RS256 signing keypair and logs +`OAuth authorization server enabled`. Nothing else is needed: the keypair is +created automatically and stored encrypted. + +Sanity check, in another shell: + +```bash +curl -s localhost:3000/.well-known/oauth-authorization-server | jq +curl -s localhost:3000/v1/oauth/jwks.json | jq '.keys[0] | {kty, alg, kid}' +``` + +## Walk the whole flow + +```bash +tools/oauth-flow.sh # api resource — a CLI or partner agent calling REST directly +tools/oauth-flow.sh mcp # mcp resource — adds the token-exchange step +``` + +Pass `OPS_OAUTH_RS_CLIENT_SECRET` with the same value the API was started with; +the `mcp` mode authenticates as the resource server. The script registers a +client, authorizes, approves consent, redeems the code, calls the API, rotates +the refresh token, and revokes the connection — printing the token claims at each +step so you can see what a client actually receives. + +The two modes differ in one way that matters: with `mcp`, the client's own token +is **refused** by the API (401) and has to be exchanged for a separate +API-audience token first. That is the no-token-passthrough rule, and the script +asserts it. + +## Connect a real client + +This is the path a user takes, and the only one that exercises the consent screen. +You need the frontend running (`npx nx serve react-ui`, port 4200) as well as the +API, and `OPS_FRONTEND_URL` pointing at it — that is what the authorize endpoint +redirects the browser to. + +Start the MCP resource server from the `openops-mcp` repository: + +```bash +cd ../openops-mcp +MCP_TRANSPORT=http \ +OPENOPS_API_URL=http://localhost:3000 \ +OPENOPS_MCP_ROUTES=config/routes.oss.yaml \ +OPENOPS_MCP_ISSUER=http://localhost:3000 \ +OPENOPS_MCP_RESOURCE_URL=http://localhost:3020/mcp \ +OPENOPS_MCP_CLIENT_SECRET="$OPS_OAUTH_RS_CLIENT_SECRET" \ +uv run openops-mcp +``` + +Then point a client at it. With Claude Code: + +```bash +claude mcp add --transport http openops http://localhost:3020/mcp +``` + +The client discovers the authorization server, registers itself, and opens your +browser at **Settings → Connected apps**, with the consent dialog over it. Sign in +if you are not already. Approving sends the browser back to the client, which +redeems the code and lists the tools. + +Worth confirming while you are here: + +- **The project is named in the dialog**, and it matches `project_id` in the + issued token — that claim is what every later request is authorized against. +- **Cancelling** returns the client to its callback with `error=access_denied`. + So does dismissing the dialog: the client is waiting on its redirect, and + telling it no beats leaving it to time out. +- **Reloading the page** after deciding shows the expired-request message rather + than a second consent dialog. The pending record is single-use. +- **Connecting a second client** (or the same one again) produces an independent + connection. Both appear as separate rows on that page, and disconnecting one + leaves the other working — which is the point of the per-connection model. +- **The page is hidden** when `OPS_OAUTH_ENABLED` is false, because every route it + depends on is unregistered. + +## Switching project + +A connection acts wherever the user can, not only where it started. With a token in +hand: + +```bash +# Where may this connection go, and where is it now? +curl -s localhost:3000/v1/oauth/projects -H "Authorization: Bearer $TOKEN" | jq + +# Move a direct API client. +curl -s -X POST localhost:3000/v1/oauth/token \ + -d "grant_type=refresh_token&refresh_token=$REFRESH&client_id=$CID&project_id=$OTHER" | jq + +# Move a resource server on an agent's behalf — the Claude Code path. +curl -s -X POST localhost:3000/v1/oauth/token \ + -u "openops-mcp-rs:$OPS_OAUTH_RS_CLIENT_SECRET" \ + -d "grant_type=urn:ietf:params:oauth:grant-type:token-exchange&subject_token=$MCP_TOKEN&project_id=$OTHER" | jq +``` + +Naming a project the user is not a member of returns `invalid_target`, and on the +refresh path the refusal happens before the token is consumed — so a wrong guess does +not cost a working connection. Decode `project_id` from the returned access token to +confirm the move. + +This edition has one project per organization, so there is usually nowhere else to go. +To exercise it, add a second project to the same organization — note that +`tablesDatabaseToken` must be a genuinely encrypted value, since the API decrypts it at +boot and will refuse to start on a malformed one. + +## Things worth poking at by hand + +Each of these should produce a clean OAuth error, never a 500: + +```bash +CID=$(curl -s -X POST localhost:3000/v1/oauth/register -H 'Content-Type: application/json' \ + -d '{"client_name":"Probe","redirect_uris":["http://127.0.0.1:41100/callback"]}' | jq -r .client_id) +AUTH="localhost:3000/v1/oauth/authorize?client_id=$CID&redirect_uri=http%3A%2F%2F127.0.0.1%3A41100%2Fcallback&response_type=code&code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM&code_challenge_method=S256&resource=http%3A%2F%2Flocalhost%3A3020%2Fmcp" + +# Unregistered redirect_uri: renders an error, must NOT redirect (open-redirect boundary) +curl -si "localhost:3000/v1/oauth/authorize?client_id=$CID&redirect_uri=https%3A%2F%2Fattacker.example%2Fsteal&response_type=code&code_challenge=E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM&code_challenge_method=S256&resource=http%3A%2F%2Flocalhost%3A3020%2Fmcp" | head -1 + +# Missing PKCE: redirects back to the *registered* uri with error + state + iss +curl -si "localhost:3000/v1/oauth/authorize?client_id=$CID&redirect_uri=http%3A%2F%2F127.0.0.1%3A41100%2Fcallback&response_type=code&resource=http%3A%2F%2Flocalhost%3A3020%2Fmcp&state=s" | grep -i location + +# Registration refuses non-loopback http and consent-skipping grants +curl -s -X POST localhost:3000/v1/oauth/register -H 'Content-Type: application/json' \ + -d '{"client_name":"E","redirect_uris":["http://evil.example/cb"]}' | jq +curl -s -X POST localhost:3000/v1/oauth/register -H 'Content-Type: application/json' \ + -d '{"client_name":"E","redirect_uris":["https://a.example/cb"],"grant_types":["implicit"]}' | jq + +# Consent decision without the anti-CSRF header. Needs a session first — without +# one you get `missing access token`, because the route requires a logged-in user +# before it looks at anything else. +curl -s -c /tmp/ck -X POST localhost:3000/v1/authentication/sign-in \ + -H 'Content-Type: application/json' \ + -d '{"email":"local-admin@openops.com","password":"12345678"}' -o /dev/null +curl -s -b /tmp/ck -X POST "localhost:3000/v1/oauth/requests/anything/decision" \ + -H 'Content-Type: application/json' -d '{"approve":true}' | jq +# -> invalid_request: the x-openops-consent header is required +``` + +To check that **connections are independent**, run `tools/oauth-flow.sh` twice +without revoking in between, then look at Settings → Connected apps (or +`GET /v1/oauth/grants`): two rows for the same client, each revocable on its own. + +## Inspecting state + +```bash +docker exec postgres psql -U postgres -d openops -c \ + "SELECT id, \"clientId\", \"projectId\", status, \"lastUsedAt\" FROM oauth_grant ORDER BY created DESC;" + +docker exec postgres psql -U postgres -d openops -c \ + "SELECT \"grantId\", \"familyId\", \"revokedAt\" IS NOT NULL AS revoked FROM oauth_refresh_token ORDER BY created DESC;" +``` + +The hourly cleanup job is registered at boot. Confirm it is scheduled with: + +```bash +docker exec redis redis-cli zrange "bull:system-job-queue:repeat" 0 -1 | grep oauth +``` + +## Resetting between runs + +```bash +docker exec postgres psql -U postgres -d openops -c \ + "DROP TABLE IF EXISTS oauth_refresh_token, oauth_authorization_code, + oauth_pending_authorization, oauth_grant, oauth_client, oauth_signing_key CASCADE; + DELETE FROM migrations WHERE name = 'CreateOAuthTables1785312000000';" +``` + +The migration re-runs on the next boot and a fresh signing key is generated. diff --git a/packages/react-ui/src/app/common/components/project-settings-layout.tsx b/packages/react-ui/src/app/common/components/project-settings-layout.tsx index d11c3bfb80..7df911624c 100644 --- a/packages/react-ui/src/app/common/components/project-settings-layout.tsx +++ b/packages/react-ui/src/app/common/components/project-settings-layout.tsx @@ -1,32 +1,13 @@ import { FlagId } from '@openops/shared'; import { t } from 'i18next'; -import { Settings, Sparkles, SunMoon } from 'lucide-react'; +import { Plug, Settings, Sparkles, SunMoon } from 'lucide-react'; +import { useMemo } from 'react'; import SidebarLayout from '@/app/common/components/sidebar-layout'; import { flagsHooks } from '@/app/common/hooks/flags-hooks'; const iconSize = 20; -const baseNavItems = [ - { - title: t('General'), - href: '/settings/general', - icon: , - }, -]; - -const appearanceNavItem = { - title: t('Appearance'), - href: '/settings/appearance', - icon: , -}; - -const aiNavItem = { - title: t('OpenOps AI'), - href: '/settings/ai', - icon: , -}; - interface SettingsLayoutProps { children: React.ReactNode; } @@ -38,11 +19,53 @@ export default function ProjectSettingsLayout({ FlagId.DARK_THEME_ENABLED, ).data; - const sidebarNavItems = [ - ...baseNavItems, - ...(showAppearanceSettings ? [appearanceNavItem] : []), - aiNavItem, - ]; + // Hidden unless the instance can actually accept external connections: with OAuth + // off, every route the page depends on is unregistered. + const showConnectedApps = flagsHooks.useFlag( + FlagId.CONNECTED_APPS_ENABLED, + ).data; + + /* + * Titles are resolved here rather than in module-scope constants (OPS-4318). + * + * A production build can place this module in a chunk that evaluates before the entry + * chunk runs `i18n.init()`. `t()` returns undefined until then, and a title captured + * in a top-level constant would freeze that undefined — a nav item with no text, in + * builds only. Inside the component the call happens at render, long after init. + */ + const sidebarNavItems = useMemo( + () => [ + { + title: t('General'), + href: '/settings/general', + icon: , + }, + ...(showAppearanceSettings + ? [ + { + title: t('Appearance'), + href: '/settings/appearance', + icon: , + }, + ] + : []), + { + title: t('OpenOps AI'), + href: '/settings/ai', + icon: , + }, + ...(showConnectedApps + ? [ + { + title: t('Connected apps'), + href: '/settings/connected-apps', + icon: , + }, + ] + : []), + ], + [showAppearanceSettings, showConnectedApps], + ); return {children}; } diff --git a/packages/react-ui/src/app/constants/query-keys.ts b/packages/react-ui/src/app/constants/query-keys.ts index 2bea8e4af6..d6b09ba7f9 100644 --- a/packages/react-ui/src/app/constants/query-keys.ts +++ b/packages/react-ui/src/app/constants/query-keys.ts @@ -61,6 +61,10 @@ export const QueryKeys = { // Cloud cloudUserInfo: 'cloud-user-info', + // OAuth + oauthConsentRequest: 'oauth-consent-request', + connectedApps: 'connected-apps', + // Connections appConnections: 'app-connections', appConnection: 'app-connection', diff --git a/packages/react-ui/src/app/features/oauth/components/connected-apps-list.tsx b/packages/react-ui/src/app/features/oauth/components/connected-apps-list.tsx new file mode 100644 index 0000000000..1d47fbb19e --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/components/connected-apps-list.tsx @@ -0,0 +1,117 @@ +import { formatUtils } from '@/app/lib/utils'; +import { Button } from '@openops/components/ui'; +import { t } from 'i18next'; +import { Plug } from 'lucide-react'; +import { ConnectedApp, OAuthResourceId } from '../lib/oauth-api'; + +/** + * How the application reaches OpenOps. Worth showing because it is the one thing that + * distinguishes otherwise identical rows, and it is not derivable from anything else the + * row displays. + */ +const describeResource = (resourceId: OAuthResourceId | null): string => { + if (resourceId === 'mcp') { + return t('via the MCP server'); + } + if (resourceId === 'api') { + return t('via the API'); + } + return t('unknown connection type'); +}; + +type ConnectedAppsListProps = { + apps: ConnectedApp[]; + onRevoke: (app: ConnectedApp) => void; + revokingId: string | null; +}; + +const EmptyState = () => ( +
+ + + {t('No applications are connected')} + + + {t( + 'When you connect an AI agent or another application to OpenOps, it will appear here and you can disconnect it at any time.', + )} + +
+); + +const ConnectedAppRow = ({ + app, + onRevoke, + isRevoking, +}: { + app: ConnectedApp; + onRevoke: (app: ConnectedApp) => void; + isRevoking: boolean; +}) => ( +
+
+ {/* Stands in for the product logo an integration card shows. Connected + applications are self-registered, so there is no artwork to use. */} +
+ +
+ +
+ + {app.clientName} + + + {describeResource(app.resourceId)} + {' · '} + {t('connected')} {formatUtils.formatDate(new Date(app.created))} + {' · '} + {app.lastUsedAt + ? `${t('last used')} ${formatUtils.formatDate( + new Date(app.lastUsedAt), + )}` + : t('never used')} + +
+
+ + +
+); + +/** + * One row per authorization, not per application. Connecting the same application + * twice produces two rows, and each is disconnected on its own — which is what lets a + * user keep one agent working while cutting off another. + */ +const ConnectedAppsList = ({ + apps, + onRevoke, + revokingId, +}: ConnectedAppsListProps) => { + if (apps.length === 0) { + return ; + } + + return ( +
+ {apps.map((app) => ( + + ))} +
+ ); +}; + +ConnectedAppsList.displayName = 'ConnectedAppsList'; +export { ConnectedAppsList }; diff --git a/packages/react-ui/src/app/features/oauth/components/consent-dialog.tsx b/packages/react-ui/src/app/features/oauth/components/consent-dialog.tsx new file mode 100644 index 0000000000..501ccc317f --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/components/consent-dialog.tsx @@ -0,0 +1,98 @@ +import { + Button, + Dialog, + DialogContent, + DialogDescription, + DialogFooter, + DialogHeader, + DialogTitle, +} from '@openops/components/ui'; +import { t } from 'i18next'; +import { OAuthConsentRequest } from '../lib/oauth-api'; + +type ConsentDialogProps = { + request: OAuthConsentRequest; + onApprove: () => void; + onDeny: () => void; + isDeciding: boolean; +}; + +/** + * What the connection will be able to do, in the user's terms. + * + * Stated as the upper bound and not varied by resource. A connection to the MCP server + * reaches the API by exchanging its token for an API one, and how much of the API the + * MCP server exposes is a deployment setting this screen cannot see — so promising + * anything narrower here would be a promise it cannot keep. + */ +const describeAccess = (): string[] => [ + t('View your workflows, runs, and connections'), + t('Create and change workflows on your behalf'), + t('Run workflows and retry runs'), + // The widest thing being granted, so it is stated rather than implied. Naming one + // project here instead would read as a limit, and there is no limit to read: a + // connection can move to any project its user can reach. + t('Act in any project you have access to'), +]; + +const ConsentDialog = ({ + request, + onApprove, + onDeny, + isDeciding, +}: ConsentDialogProps) => ( + { + if (!open && !isDeciding) { + onDeny(); + } + }} + > + + + {t('Authorize access')} + + {request.clientName}{' '} + {t('is asking to access OpenOps as you.')} + + + +
+
+ + {t('It will be able to:')} + +
    + {describeAccess().map((item) => ( +
  • + {item} +
    • + ))} +
+
+ +

+ {t( + 'Only continue if you started this from the application named above. You can disconnect it later from this page.', + )} +

+
+ + + + + + + +); + +ConsentDialog.displayName = 'ConsentDialog'; +export { ConsentDialog }; diff --git a/packages/react-ui/src/app/features/oauth/hooks/tests/use-connected-apps.test.tsx b/packages/react-ui/src/app/features/oauth/hooks/tests/use-connected-apps.test.tsx new file mode 100644 index 0000000000..5bd2d6823d --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/hooks/tests/use-connected-apps.test.tsx @@ -0,0 +1,111 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook, waitFor } from '@testing-library/react'; +import { ReactNode } from 'react'; +import { ConnectedApp, oauthApi } from '../../lib/oauth-api'; +import { useConnectedApps } from '../use-connected-apps'; + +jest.mock('../../lib/oauth-api', () => ({ + oauthApi: { listConnectedApps: jest.fn(), revokeConnectedApp: jest.fn() }, +})); + +const mockedList = oauthApi.listConnectedApps as jest.Mock; +const mockedRevoke = oauthApi.revokeConnectedApp as jest.Mock; + +const app = (id: string, clientName = 'Claude Code'): ConnectedApp => ({ + id, + clientName, + resourceId: 'mcp', + created: '2026-07-01T10:00:00.000Z', + lastUsedAt: null, +}); + +// Retries are disabled here only to keep the failure cases fast. Unlike the consent +// request, which is single-use and opts out in the hook, retrying this list is +// reasonable behaviour — it just is not what these tests are about. +const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + +); + +const render = () => renderHook(() => useConnectedApps(), { wrapper }); + +beforeEach(() => { + jest.clearAllMocks(); + mockedList.mockResolvedValue([app('grant-1'), app('grant-2')]); + mockedRevoke.mockResolvedValue(undefined); +}); + +describe('useConnectedApps', () => { + it('lists the connections the user has granted', async () => { + const { result } = render(); + + await waitFor(() => expect(result.current.apps).toHaveLength(2)); + expect(result.current.apps?.map((a) => a.id)).toEqual([ + 'grant-1', + 'grant-2', + ]); + }); + + it('revokes only the connection asked for', async () => { + const { result } = render(); + await waitFor(() => expect(result.current.apps).toHaveLength(2)); + + await act(async () => result.current.revoke('grant-2')); + + // Two rows can belong to the same application, so the id is what identifies + // which authorization to cut off. react-query passes its own context as a second + // argument, so only the first is asserted. + expect(mockedRevoke).toHaveBeenCalledTimes(1); + expect(mockedRevoke.mock.calls[0][0]).toBe('grant-2'); + }); + + it('refetches the list after revoking so the row disappears', async () => { + const { result } = render(); + await waitFor(() => expect(result.current.apps).toHaveLength(2)); + + mockedList.mockResolvedValue([app('grant-1')]); + await act(async () => result.current.revoke('grant-2')); + + await waitFor(() => expect(result.current.apps).toHaveLength(1)); + expect(result.current.apps?.[0].id).toBe('grant-1'); + }); + + it('reports which connection is being revoked, and only that one', async () => { + let finish: () => void = () => undefined; + mockedRevoke.mockImplementation( + () => new Promise((resolve) => (finish = resolve)), + ); + + const { result } = render(); + await waitFor(() => expect(result.current.apps).toHaveLength(2)); + + act(() => result.current.revoke('grant-2')); + await waitFor(() => expect(result.current.revokingId).toBe('grant-2')); + + await act(async () => finish()); + await waitFor(() => expect(result.current.revokingId).toBeNull()); + }); + + it('surfaces a failed revoke and refetches so the row is not wrongly removed', async () => { + mockedRevoke.mockRejectedValue(new Error('gone')); + const { result } = render(); + await waitFor(() => expect(result.current.apps).toHaveLength(2)); + + await act(async () => result.current.revoke('grant-2')); + + await waitFor(() => expect(result.current.revokeError).not.toBeNull()); + expect(result.current.apps).toHaveLength(2); + }); + + it('surfaces a failed load', async () => { + mockedList.mockRejectedValue(new Error('oauth disabled')); + + const { result } = render(); + + await waitFor(() => expect(result.current.loadError).not.toBeNull()); + expect(result.current.apps).toBeUndefined(); + }); +}); diff --git a/packages/react-ui/src/app/features/oauth/hooks/tests/use-oauth-consent.test.tsx b/packages/react-ui/src/app/features/oauth/hooks/tests/use-oauth-consent.test.tsx new file mode 100644 index 0000000000..d86b224ed7 --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/hooks/tests/use-oauth-consent.test.tsx @@ -0,0 +1,107 @@ +import { QueryClient, QueryClientProvider } from '@tanstack/react-query'; +import { act, renderHook, waitFor } from '@testing-library/react'; +import { ReactNode } from 'react'; +import { oauthApi, OAuthConsentRequest } from '../../lib/oauth-api'; +import { useOAuthConsent } from '../use-oauth-consent'; + +jest.mock('../../lib/oauth-api', () => ({ + oauthApi: { getConsentRequest: jest.fn(), decide: jest.fn() }, +})); + +const mockedGetConsentRequest = oauthApi.getConsentRequest as jest.Mock; +const mockedDecide = oauthApi.decide as jest.Mock; + +const REQUEST: OAuthConsentRequest = { + requestId: 'req-1', + clientName: 'Claude Code', +}; + +const assign = jest.fn(); + +// Deliberately left at react-query's defaults, which retry failed queries. The hook is +// responsible for opting out, so overriding it here would hide that. +const wrapper = ({ children }: { children: ReactNode }) => ( + + {children} + +); + +const render = (requestId: string | null) => + renderHook(() => useOAuthConsent(requestId), { wrapper }); + +beforeAll(() => { + Object.defineProperty(window, 'location', { + value: { assign }, + writable: true, + }); +}); + +beforeEach(() => { + jest.clearAllMocks(); + mockedGetConsentRequest.mockResolvedValue(REQUEST); + mockedDecide.mockResolvedValue({ redirectTo: 'https://client/cb?code=abc' }); +}); + +describe('useOAuthConsent', () => { + it('exposes the pending request once loaded', async () => { + const { result } = render('req-1'); + + await waitFor(() => expect(result.current.request).toEqual(REQUEST)); + expect(mockedGetConsentRequest).toHaveBeenCalledWith('req-1'); + }); + + it('does not ask the server for a request that was never identified', async () => { + const { result } = render(null); + + await waitFor(() => expect(result.current.isLoading).toBe(false)); + expect(mockedGetConsentRequest).not.toHaveBeenCalled(); + }); + + it('sends the browser to the redirect the server returned when approving', async () => { + const { result } = render('req-1'); + await waitFor(() => expect(result.current.request).toEqual(REQUEST)); + + await act(async () => result.current.approve()); + + expect(mockedDecide).toHaveBeenCalledWith('req-1', true); + // A full navigation, because the destination belongs to the calling client. + expect(assign).toHaveBeenCalledWith('https://client/cb?code=abc'); + }); + + it('sends the browser to the error redirect when denying', async () => { + mockedDecide.mockResolvedValue({ + redirectTo: 'https://client/cb?error=access_denied', + }); + const { result } = render('req-1'); + await waitFor(() => expect(result.current.request).toEqual(REQUEST)); + + await act(async () => result.current.deny()); + + expect(mockedDecide).toHaveBeenCalledWith('req-1', false); + expect(assign).toHaveBeenCalledWith( + 'https://client/cb?error=access_denied', + ); + }); + + it('surfaces a failed load without retrying it', async () => { + mockedGetConsentRequest.mockRejectedValue(new Error('expired')); + + const { result } = render('req-1'); + + await waitFor(() => expect(result.current.loadError).not.toBeNull()); + expect(result.current.request).toBeUndefined(); + // A pending request is single-use: re-reading it cannot succeed. + expect(mockedGetConsentRequest).toHaveBeenCalledTimes(1); + }); + + it('surfaces a failed decision and leaves the browser where it is', async () => { + mockedDecide.mockRejectedValue(new Error('gone')); + const { result } = render('req-1'); + await waitFor(() => expect(result.current.request).toEqual(REQUEST)); + + await act(async () => result.current.approve()); + + await waitFor(() => expect(result.current.decisionError).not.toBeNull()); + expect(assign).not.toHaveBeenCalled(); + }); +}); diff --git a/packages/react-ui/src/app/features/oauth/hooks/use-connected-apps.ts b/packages/react-ui/src/app/features/oauth/hooks/use-connected-apps.ts new file mode 100644 index 0000000000..42a7269333 --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/hooks/use-connected-apps.ts @@ -0,0 +1,54 @@ +import { QueryKeys } from '@/app/constants/query-keys'; +import { useMutation, useQuery, useQueryClient } from '@tanstack/react-query'; +import { useCallback } from 'react'; +import { ConnectedApp, oauthApi } from '../lib/oauth-api'; + +type UseConnectedApps = { + apps: ConnectedApp[] | undefined; + isLoading: boolean; + loadError: Error | null; + revoke: (grantId: string) => void; + revokingId: string | null; + revokeError: Error | null; +}; + +/** + * The applications this user has connected, and the ability to disconnect one. + * + * Each row is a separate authorization rather than a separate application: connecting + * the same client twice produces two, and revoking one leaves the other working. + */ +export const useConnectedApps = (): UseConnectedApps => { + const queryClient = useQueryClient(); + + const { + data: apps, + isLoading, + error: loadError, + } = useQuery({ + queryKey: [QueryKeys.connectedApps], + queryFn: oauthApi.listConnectedApps, + }); + + const { + mutate, + variables: revokingId, + isPending: isRevoking, + error: revokeError, + } = useMutation({ + mutationFn: oauthApi.revokeConnectedApp, + onSettled: () => + queryClient.invalidateQueries({ queryKey: [QueryKeys.connectedApps] }), + }); + + const revoke = useCallback((grantId: string) => mutate(grantId), [mutate]); + + return { + apps, + isLoading, + loadError: loadError as Error | null, + revoke, + revokingId: isRevoking ? revokingId ?? null : null, + revokeError: revokeError as Error | null, + }; +}; diff --git a/packages/react-ui/src/app/features/oauth/hooks/use-oauth-consent.ts b/packages/react-ui/src/app/features/oauth/hooks/use-oauth-consent.ts new file mode 100644 index 0000000000..db8fafa26f --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/hooks/use-oauth-consent.ts @@ -0,0 +1,63 @@ +import { QueryKeys } from '@/app/constants/query-keys'; +import { useMutation, useQuery } from '@tanstack/react-query'; +import { useCallback } from 'react'; +import { oauthApi, OAuthConsentRequest } from '../lib/oauth-api'; + +type UseOAuthConsent = { + request: OAuthConsentRequest | undefined; + isLoading: boolean; + loadError: Error | null; + approve: () => void; + deny: () => void; + isDeciding: boolean; + decisionError: Error | null; +}; + +/** + * Loads a pending authorization request and records the user's decision. + * + * The request is single-use: the server consumes it when a decision arrives, so this + * never retries and never refetches. A second read would fail, and a second decision + * is exactly what the single-use record exists to prevent. + */ +export const useOAuthConsent = (requestId: string | null): UseOAuthConsent => { + const { + data: request, + isLoading, + error: loadError, + } = useQuery({ + queryKey: [QueryKeys.oauthConsentRequest, requestId], + queryFn: () => oauthApi.getConsentRequest(requestId as string), + enabled: requestId !== null, + retry: false, + staleTime: Infinity, + refetchOnWindowFocus: false, + }); + + const { + mutate, + isPending: isDeciding, + error: decisionError, + } = useMutation({ + mutationFn: (approve: boolean) => + oauthApi.decide(requestId as string, approve), + onSuccess: ({ redirectTo }) => { + // A full navigation, not a router push: the destination belongs to the client + // that started the flow. The server only ever returns a registered redirect URI. + window.location.assign(redirectTo); + }, + }); + + const approve = useCallback(() => mutate(true), [mutate]); + const deny = useCallback(() => mutate(false), [mutate]); + + return { + request, + isLoading: requestId !== null && isLoading, + loadError: loadError as Error | null, + approve, + deny, + isDeciding, + decisionError: decisionError as Error | null, + }; +}; diff --git a/packages/react-ui/src/app/features/oauth/lib/oauth-api.ts b/packages/react-ui/src/app/features/oauth/lib/oauth-api.ts new file mode 100644 index 0000000000..6d82671c00 --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/lib/oauth-api.ts @@ -0,0 +1,61 @@ +import { api } from '@/app/lib/api'; + +/** + * Required on the decision. A cross-site form post cannot set a custom header, which + * is what stops a third party from driving the decision on a logged-in user's behalf. + */ +const CONSENT_HEADER = 'x-openops-consent'; + +export type OAuthResourceId = 'api' | 'mcp'; + +export type OAuthConsentRequest = { + requestId: string; + clientName: string; +}; + +export type OAuthConsentDecision = { + /** Where to send the browser next. Always one of the client's registered URIs. */ + redirectTo: string; +}; + +/** One authorization the user granted. Each is revocable on its own. */ +export type ConnectedApp = { + id: string; + clientName: string; + resourceId: OAuthResourceId | null; + created: string; + lastUsedAt: string | null; +}; + +type ListConnectedAppsResponse = { + data: ConnectedApp[]; +}; + +const getConsentRequest = (requestId: string): Promise => + api.get(`/v1/oauth/requests/${requestId}`); + +const decide = ( + requestId: string, + approve: boolean, +): Promise => + api.post( + `/v1/oauth/requests/${requestId}/decision`, + { approve }, + undefined, + { [CONSENT_HEADER]: '1' }, + ); + +const listConnectedApps = (): Promise => + api + .get('/v1/oauth/grants') + .then((response) => response.data); + +const revokeConnectedApp = (grantId: string): Promise => + api.delete(`/v1/oauth/grants/${grantId}`); + +export const oauthApi = { + getConsentRequest, + decide, + listConnectedApps, + revokeConnectedApp, +}; diff --git a/packages/react-ui/src/app/features/oauth/lib/tests/oauth-api.test.ts b/packages/react-ui/src/app/features/oauth/lib/tests/oauth-api.test.ts new file mode 100644 index 0000000000..52462ede25 --- /dev/null +++ b/packages/react-ui/src/app/features/oauth/lib/tests/oauth-api.test.ts @@ -0,0 +1,63 @@ +import { api } from '@/app/lib/api'; +import { oauthApi } from '../oauth-api'; + +jest.mock('@/app/lib/api', () => ({ + api: { get: jest.fn(), post: jest.fn(), delete: jest.fn() }, +})); + +const mockedGet = api.get as jest.Mock; +const mockedPost = api.post as jest.Mock; +const mockedDelete = api.delete as jest.Mock; + +describe('oauthApi', () => { + beforeEach(() => { + mockedGet.mockReset().mockResolvedValue({}); + mockedPost.mockReset().mockResolvedValue({ redirectTo: 'https://client' }); + mockedDelete.mockReset().mockResolvedValue(undefined); + }); + + it('reads a pending request by id', async () => { + await oauthApi.getConsentRequest('req-1'); + + expect(mockedGet).toHaveBeenCalledWith('/v1/oauth/requests/req-1'); + }); + + it('sends the consent header with the decision', async () => { + await oauthApi.decide('req-1', true); + + // The server refuses a decision without this header, which is what stops a + // cross-site form post from answering on a signed-in user's behalf. + expect(mockedPost).toHaveBeenCalledWith( + '/v1/oauth/requests/req-1/decision', + { approve: true }, + undefined, + { 'x-openops-consent': '1' }, + ); + }); + + it('unwraps the connected apps list', async () => { + mockedGet.mockResolvedValue({ data: [{ id: 'grant-1' }] }); + + await expect(oauthApi.listConnectedApps()).resolves.toEqual([ + { id: 'grant-1' }, + ]); + expect(mockedGet).toHaveBeenCalledWith('/v1/oauth/grants'); + }); + + it('revokes one connection by its own id', async () => { + await oauthApi.revokeConnectedApp('grant-2'); + + expect(mockedDelete).toHaveBeenCalledWith('/v1/oauth/grants/grant-2'); + }); + + it('carries a denial through as approve false', async () => { + await oauthApi.decide('req-1', false); + + expect(mockedPost).toHaveBeenCalledWith( + '/v1/oauth/requests/req-1/decision', + { approve: false }, + undefined, + expect.anything(), + ); + }); +}); diff --git a/packages/react-ui/src/app/lib/api.ts b/packages/react-ui/src/app/lib/api.ts index b754975ef7..4a09026195 100644 --- a/packages/react-ui/src/app/lib/api.ts +++ b/packages/react-ui/src/app/lib/api.ts @@ -62,11 +62,12 @@ export const api = { url: string, body?: TBody, params?: TParams, + headers: Record = {}, ) => request(url, { method: 'POST', data: body, - headers: { 'Content-Type': 'application/json' }, + headers: { 'Content-Type': 'application/json', ...headers }, params: params, }), diff --git a/packages/react-ui/src/app/router.tsx b/packages/react-ui/src/app/router.tsx index 57895df73a..53de7047ba 100644 --- a/packages/react-ui/src/app/router.tsx +++ b/packages/react-ui/src/app/router.tsx @@ -46,6 +46,10 @@ import GeneralPage from './routes/settings/general'; import { SignInPage } from './routes/sign-in'; import { SignUpPage } from './routes/sign-up'; +const ConnectedAppsPage = lazy( + () => import('@/app/routes/settings/connected-apps'), +); + const SettingsRerouter = () => { const { hash } = useLocation(); const fragmentWithoutHash = hash.slice(1).toLowerCase(); @@ -290,6 +294,24 @@ const createRoutes = ({ routes.push(...regularLoginRoutes); } + routes.push({ + path: 'settings/connected-apps', + element: ( + }> + + + + + + + + + + + ), + errorElement: , + }); + const redirectRoutes = [ { path: 'redirect', diff --git a/packages/react-ui/src/app/routes/settings/connected-apps/connected-apps-page.tsx b/packages/react-ui/src/app/routes/settings/connected-apps/connected-apps-page.tsx new file mode 100644 index 0000000000..e9e23551b3 --- /dev/null +++ b/packages/react-ui/src/app/routes/settings/connected-apps/connected-apps-page.tsx @@ -0,0 +1,136 @@ +import { ConnectedAppsList } from '@/app/features/oauth/components/connected-apps-list'; +import { ConsentDialog } from '@/app/features/oauth/components/consent-dialog'; +import { useConnectedApps } from '@/app/features/oauth/hooks/use-connected-apps'; +import { useOAuthConsent } from '@/app/features/oauth/hooks/use-oauth-consent'; +import { ConnectedApp } from '@/app/features/oauth/lib/oauth-api'; +import { + Alert, + AlertDescription, + AlertTitle, + ConfirmationDialog, + LoadingSpinner, +} from '@openops/components/ui'; +import { t } from 'i18next'; +import { useCallback, useState } from 'react'; +import { useSearchParams } from 'react-router-dom'; + +const REQUEST_ID_PARAM = 'request_id'; + +const PageError = ({ + title, + description, +}: { + title: string; + description: string; +}) => ( + + {title} + {description} + +); + +const ConnectedAppsPage = () => { + const [searchParams] = useSearchParams(); + const requestId = searchParams.get(REQUEST_ID_PARAM); + + const consent = useOAuthConsent(requestId); + const { apps, isLoading, loadError, revoke, revokingId, revokeError } = + useConnectedApps(); + + const [appToRevoke, setAppToRevoke] = useState(null); + + const confirmRevoke = useCallback(() => { + if (appToRevoke) { + revoke(appToRevoke.id); + setAppToRevoke(null); + } + }, [appToRevoke, revoke]); + + const cancelRevoke = useCallback(() => setAppToRevoke(null), []); + + return ( + // Same shape as the other settings routes, so the page title and description read + // the same wherever you land (see `routes/settings/ai`). +
+
+

{t('Connected apps')}

+

+ {t( + 'AI agents and other applications you have allowed to act in OpenOps on your behalf. Disconnecting one takes effect immediately and does not affect the others.', + )} +

+ + {/* A pending request that cannot be read is almost always expired, already + answered, or a reloaded page — the single-use record is gone either way. */} + {requestId && consent.loadError && ( + + )} + + {consent.decisionError && ( + + )} + + {loadError && ( + + )} + + {revokeError && ( + + )} + + {isLoading ? ( +
+ +
+ ) : ( + + )} +
+ + {consent.request && ( + + )} + + !open && cancelRevoke()} + title={t('Disconnect this application?')} + description={t( + 'It will immediately lose access to OpenOps and will have to be authorized again to reconnect.', + )} + confirmButtonText={t('Disconnect')} + confirmButtonVariant="destructive" + onConfirm={confirmRevoke} + onCancel={cancelRevoke} + /> +
+ ); +}; + +ConnectedAppsPage.displayName = 'ConnectedAppsPage'; +export { ConnectedAppsPage }; diff --git a/packages/react-ui/src/app/routes/settings/connected-apps/index.tsx b/packages/react-ui/src/app/routes/settings/connected-apps/index.tsx new file mode 100644 index 0000000000..ee81a9487f --- /dev/null +++ b/packages/react-ui/src/app/routes/settings/connected-apps/index.tsx @@ -0,0 +1 @@ +export { ConnectedAppsPage as default } from './connected-apps-page'; diff --git a/packages/server/api/src/app/ai/mcp/openops-tools.ts b/packages/server/api/src/app/ai/mcp/openops-tools.ts index 6ad2d7d25b..ef940be9a9 100644 --- a/packages/server/api/src/app/ai/mcp/openops-tools.ts +++ b/packages/server/api/src/app/ai/mcp/openops-tools.ts @@ -2,6 +2,7 @@ import { createMCPClient } from '@ai-sdk/mcp'; import { StdioClientTransport } from '@modelcontextprotocol/sdk/client/stdio.js'; import { AppSystemProp, + logger, networkUtls, SharedSystemProp, system, @@ -33,38 +34,51 @@ const INCLUDED_PATHS: Record = { '/v1/app-connections/metadata': ['get'], }; -function filterOpenApiSchema(schema: OpenAPI.Document): OpenAPI.Document { - // eslint-disable-next-line @typescript-eslint/no-explicit-any - const filteredPaths: Record = {}; +/** + * The MCP server takes its allow-list as a file and reads the OpenAPI document from + * the API itself, so this writes `INCLUDED_PATHS` out in the shape it expects. Writing + * it rather than shipping a copy alongside the MCP server keeps this the only place + * the chat's exposed surface is declared. + * + * Entries the running API does not serve are dropped, which is what the old schema + * filter did implicitly. It matters more now: the MCP server refuses to start on an + * operation it cannot find, so passing a stale entry through would cost every tool + * rather than the one that drifted. + */ +function buildRouteList(schema: OpenAPI.Document): string { + const available = schema.paths ?? {}; - for (const [path, pathItem] of Object.entries(schema.paths ?? {})) { - if (!INCLUDED_PATHS[path]) continue; + const routes = Object.entries(INCLUDED_PATHS) + .map(([path, methods]) => { + const pathItem = available[path]; + const served = pathItem + ? methods.filter((method) => method in pathItem) + : []; - filteredPaths[path] = {}; - for (const [method, op] of Object.entries(pathItem)) { - if (INCLUDED_PATHS[path].includes(method.toLowerCase())) { - filteredPaths[path][method] = op; + if (served.length !== methods.length) { + logger.warn('Skipping MCP operations the API does not expose', { + path, + requested: methods, + served, + }); } - } - } - return { ...schema, paths: filteredPaths }; + return { path, methods: served }; + }) + .filter((route) => route.methods.length > 0); + + return JSON.stringify({ routes }); } -let cachedSchemaPath: string | undefined; +let cachedRoutesPath: string | undefined; -async function getOpenApiSchemaPath(app: FastifyInstance): Promise { - if (!cachedSchemaPath) { - const openApiSchema = app.swagger(); - const filteredSchema = filterOpenApiSchema(openApiSchema); - cachedSchemaPath = path.join(os.tmpdir(), 'openapi-schema.json'); - await fs.writeFile( - cachedSchemaPath, - JSON.stringify(filteredSchema), - 'utf-8', - ); +async function getRouteListPath(app: FastifyInstance): Promise { + if (!cachedRoutesPath) { + const routesPath = path.join(os.tmpdir(), 'openops-mcp-routes.json'); + await fs.writeFile(routesPath, buildRouteList(app.swagger()), 'utf-8'); + cachedRoutesPath = routesPath; } - return cachedSchemaPath; + return cachedRoutesPath; } export async function getOpenOpsTools( @@ -78,7 +92,7 @@ export async function getOpenOpsTools( const pythonPath = path.join(basePath, '.venv', 'bin', 'python'); const serverPath = path.join(basePath, 'main.py'); - const tempSchemaPath = await getOpenApiSchemaPath(app); + const routesPath = await getRouteListPath(app); const serviceToken = await accessTokenManager.generateServiceToken(userAuthToken); @@ -88,9 +102,12 @@ export async function getOpenOpsTools( command: pythonPath, args: [serverPath], env: { - OPENAPI_SCHEMA_PATH: tempSchemaPath, + // stdio: the server acts as one service principal, so the token is passed + // in rather than obtained per request as it is over HTTP. + MCP_TRANSPORT: 'stdio', AUTH_TOKEN: serviceToken, - API_BASE_URL: networkUtls.getInternalApiUrl(), + OPENOPS_MCP_ROUTES: routesPath, + OPENOPS_API_URL: networkUtls.getInternalApiUrl(), OPENOPS_MCP_SERVER_PATH: basePath, LOGZIO_TOKEN: system.get(SharedSystemProp.LOGZIO_TOKEN) ?? '', ENVIRONMENT: diff --git a/packages/server/api/test/unit/ai/openops-tools.test.ts b/packages/server/api/test/unit/ai/openops-tools.test.ts index 6a93497170..2e36b231c9 100644 --- a/packages/server/api/test/unit/ai/openops-tools.test.ts +++ b/packages/server/api/test/unit/ai/openops-tools.test.ts @@ -135,62 +135,15 @@ describe('getOpenOpsTools', () => { }, }; - const filteredSchema = { - openapi: '3.1', - paths: { - '/v1/files/{fileId}': { - get: { operationId: 'getFile' }, - }, - '/v1/flow-versions/': { - get: { operationId: 'getFlowVersions' }, - }, - '/v1/flows/': { - get: { operationId: 'getFlows' }, - }, - '/v1/flows/count': { - get: { operationId: 'getFlowsCount' }, - }, - '/v1/flows/{id}': { - get: { operationId: 'getFlow' }, - }, - '/v1/blocks/categories': { - get: { operationId: 'getBlockCategories' }, - }, - '/v1/blocks/': { - get: { operationId: 'getBlocks' }, - }, - '/v1/blocks/{scope}/{name}': { - get: { operationId: 'getBlockScopeName' }, - }, - '/v1/blocks/{name}': { - get: { operationId: 'getBlockName' }, - }, - '/v1/flow-runs/': { - get: { operationId: 'getFlowRuns' }, - }, - '/v1/flow-runs/{id}': { - get: { operationId: 'getFlowRun' }, - }, - '/v1/flow-runs/{id}/retry': { - post: { operationId: 'retryFlowRun' }, - }, - '/v1/app-connections/': { - get: { operationId: 'getAppConnections' }, - patch: { operationId: 'patchAppConnection' }, - }, - '/v1/app-connections/{id}': { - get: { operationId: 'getAppConnectionById' }, - }, - '/v1/app-connections/metadata': { - get: { operationId: 'getAppConnectionsMetadata' }, - }, - }, - }; - const mockApp = { swagger: jest.fn().mockReturnValue(mockOpenApiSchema), } as unknown as FastifyInstance; + const writtenRoutes = (): { path: string; methods: string[] }[] => { + const [, contents] = jest.mocked(fs.writeFile).mock.calls[0]; + return JSON.parse(contents as string).routes; + }; + beforeEach(() => { jest.clearAllMocks(); @@ -208,17 +161,42 @@ describe('getOpenOpsTools', () => { networkUtlsMock.getInternalApiUrl.mockReturnValue(mockApiBaseUrl); }); - it('should write the filtered OpenAPI schema to a file once and reuse it later', async () => { - const mockClient = { + // The written path is cached for the life of the process, so only the first call in + // this file performs the write. Both assertions about its contents live here. + it('should write only the allowed operations the API actually exposes', async () => { + createMcpClientMock.mockResolvedValue({ tools: jest.fn().mockResolvedValue(mockTools), - }; - createMcpClientMock.mockResolvedValue(mockClient); + }); await getOpenOpsTools(mockApp, 'auth-1'); - expect(fs.writeFile).toHaveBeenCalledWith( - path.join('/tmp', 'openapi-schema.json'), - JSON.stringify(filteredSchema), - 'utf-8', + + const [target] = jest.mocked(fs.writeFile).mock.calls[0]; + expect(target).toBe(path.join('/tmp', 'openops-mcp-routes.json')); + + // `/v1/blocks/options` is allow-listed but missing from this document. It must be + // left out: the MCP server refuses to start on an operation it cannot find, which + // would cost every other tool too. + expect(writtenRoutes()).toEqual([ + { path: '/v1/files/{fileId}', methods: ['get'] }, + { path: '/v1/flow-versions/', methods: ['get'] }, + { path: '/v1/flows/', methods: ['get'] }, + { path: '/v1/flows/count', methods: ['get'] }, + { path: '/v1/flows/{id}', methods: ['get'] }, + { path: '/v1/blocks/categories', methods: ['get'] }, + { path: '/v1/blocks/', methods: ['get'] }, + { path: '/v1/blocks/{scope}/{name}', methods: ['get'] }, + { path: '/v1/blocks/{name}', methods: ['get'] }, + { path: '/v1/flow-runs/', methods: ['get'] }, + { path: '/v1/flow-runs/{id}', methods: ['get'] }, + { path: '/v1/flow-runs/{id}/retry', methods: ['post'] }, + { path: '/v1/app-connections/', methods: ['get', 'patch'] }, + { path: '/v1/app-connections/{id}', methods: ['get'] }, + { path: '/v1/app-connections/metadata', methods: ['get'] }, + ]); + + expect(loggerMock.warn).toHaveBeenCalledWith( + 'Skipping MCP operations the API does not expose', + { path: '/v1/blocks/options', requested: ['post'], served: [] }, ); await getOpenOpsTools(mockApp, 'auth-2'); @@ -247,9 +225,10 @@ describe('getOpenOpsTools', () => { command: `${mockBasePath}/.venv/bin/python`, args: [`${mockBasePath}/main.py`], env: expect.objectContaining({ - OPENAPI_SCHEMA_PATH: expect.any(String), + MCP_TRANSPORT: 'stdio', AUTH_TOKEN: 'auth-service-token', - API_BASE_URL: mockApiBaseUrl, + OPENOPS_MCP_ROUTES: path.join('/tmp', 'openops-mcp-routes.json'), + OPENOPS_API_URL: mockApiBaseUrl, OPENOPS_MCP_SERVER_PATH: mockBasePath, LOGZIO_TOKEN: 'test-logzio-token', ENVIRONMENT: 'test-environment', diff --git a/tools/oauth-flow.sh b/tools/oauth-flow.sh new file mode 100755 index 0000000000..c8874e8b78 --- /dev/null +++ b/tools/oauth-flow.sh @@ -0,0 +1,181 @@ +#!/bin/bash +# +# Walks the external-agent OAuth flow end to end against a locally running API. +# See docs/oauth-manual-testing.md. +# +# Usage: +# tools/oauth-flow.sh # api resource (direct REST access, like a CLI) +# tools/oauth-flow.sh mcp # mcp resource (adds the token-exchange step) +# +set -euo pipefail + +RESOURCE_KIND="${1:-api}" +API="${OPS_OAUTH_TEST_API:-http://localhost:3000}" +EMAIL="${OPS_OAUTH_TEST_EMAIL:-local-admin@openops.com}" +PASSWORD="${OPS_OAUTH_TEST_PASSWORD:-12345678}" +MCP_RESOURCE="${OPS_MCP_RESOURCE_URL:-http://localhost:3020/mcp}" +RS_SECRET="${OPS_OAUTH_RS_CLIENT_SECRET:-}" +REDIRECT="http://127.0.0.1:41100/callback" + +# A fixed PKCE pair. Real clients generate one per request; a constant keeps this +# script readable and is not a weakness here because nothing is at stake locally. +VERIFIER="dBjftJeZ4CVP-mB92K27uhbUJU1p1r_wW1gFWFOEjXk" +CHALLENGE="E9Melhoa2OwvFrEMTJguCHaoeK1t8URWbuGJSstw-cM" + +WORK_DIR="$(mktemp -d)" +trap 'rm -rf "$WORK_DIR"' EXIT + +say() { printf '\n\033[1m%s\033[0m\n' "$*"; } +fail() { printf '\033[31mFAILED: %s\033[0m\n' "$*" >&2; exit 1; } + +claims() { + local jwt="$1" + python3 -c " +import base64, json, sys +payload = sys.argv[1].split('.')[1] +payload += '=' * (-len(payload) % 4) +decoded = json.loads(base64.urlsafe_b64decode(payload)) +shown = {k: decoded[k] for k in ('aud','sub','scope','grant_id','project_id') if k in decoded} +print(json.dumps(shown, indent=2))" "$jwt" +} + +json_get() { + local file="$1" key="$2" + python3 -c "import json,sys;print(json.load(open(sys.argv[1]))[sys.argv[2]])" "$file" "$key" +} + +# ---------------------------------------------------------------- preflight --- +say "Preflight" +curl -sf -o /dev/null "$API/v1/flags" || fail "API not reachable at $API" +if ! curl -sf -o /dev/null "$API/.well-known/oauth-authorization-server"; then + fail "OAuth is disabled. Start the API with OPS_OAUTH_ENABLED=true (see docs/oauth-manual-testing.md)" +fi +echo " API up, OAuth enabled" + +if [[ "$RESOURCE_KIND" == "mcp" ]]; then + RESOURCE="$MCP_RESOURCE" + [[ -n "$RS_SECRET" ]] || fail "mcp mode needs OPS_OAUTH_RS_CLIENT_SECRET (same value the API was started with)" + curl -s "$API/.well-known/oauth-authorization-server" | + grep -q '"mcp"' || fail "the API has no mcp resource configured (set OPS_MCP_RESOURCE_URL)" +else + RESOURCE="$(curl -s "$API/.well-known/oauth-authorization-server" | + python3 -c "import sys,json;print(json.load(sys.stdin)['issuer'])")" +fi +echo " resource: $RESOURCE" + +# --------------------------------------------------------------- discovery --- +say "1. Discovery (what a client reads first)" +curl -s "$API/.well-known/oauth-authorization-server" | python3 -m json.tool | head -14 +echo " jwks keys: $(curl -s "$API/v1/oauth/jwks.json" | + python3 -c "import sys,json;d=json.load(sys.stdin);print(len(d['keys']), d['keys'][0]['alg'])")" + +# ------------------------------------------------------------ registration --- +say "2. Dynamic client registration" +curl -s -X POST "$API/v1/oauth/register" -H 'Content-Type: application/json' \ + -d "{\"client_name\":\"Manual Test Client\",\"redirect_uris\":[\"$REDIRECT\"]}" \ + -o "$WORK_DIR/client.json" +CLIENT_ID="$(json_get "$WORK_DIR/client.json" client_id)" +echo " client_id: $CLIENT_ID" + +# --------------------------------------------------------------- authorize --- +say "3. Authorize (a real client opens this in a browser)" +AUTHORIZE_URL="$API/v1/oauth/authorize?client_id=$CLIENT_ID&redirect_uri=$( + python3 -c "import urllib.parse,sys;print(urllib.parse.quote(sys.argv[1],safe=''))" "$REDIRECT" +)&response_type=code&code_challenge=$CHALLENGE&code_challenge_method=S256&resource=$( + python3 -c "import urllib.parse,sys;print(urllib.parse.quote(sys.argv[1],safe=''))" "$RESOURCE" +)&state=manual-test-state" +LOCATION="$(curl -s -i "$AUTHORIZE_URL" | grep -i '^location:' | tr -d '\r' | sed 's/^[Ll]ocation: //')" +echo " browser would be sent to: $LOCATION" +REQUEST_ID="$(printf '%s' "$LOCATION" | sed -n 's/.*request_id=\([^&]*\).*/\1/p')" +[[ -n "$REQUEST_ID" ]] || fail "no request_id in the redirect — check the authorize parameters" + +# ------------------------------------------------------------------ consent --- +say "4. Consent (a browser would show the dialog on Settings -> Connected apps; driven directly here)" +curl -s -c "$WORK_DIR/cookies" -X POST "$API/v1/authentication/sign-in" \ + -H 'Content-Type: application/json' \ + -d "{\"email\":\"$EMAIL\",\"password\":\"$PASSWORD\"}" -o /dev/null || + fail "sign-in failed for $EMAIL" + +echo " what the consent screen would show:" +curl -s -b "$WORK_DIR/cookies" "$API/v1/oauth/requests/$REQUEST_ID" | python3 -m json.tool | sed 's/^/ /' + +curl -s -b "$WORK_DIR/cookies" -X POST "$API/v1/oauth/requests/$REQUEST_ID/decision" \ + -H 'Content-Type: application/json' -H 'x-openops-consent: 1' \ + -d '{"approve":true}' -o "$WORK_DIR/decision.json" +CODE="$(python3 -c " +import json, urllib.parse as u +q = u.parse_qs(u.urlparse(json.load(open('$WORK_DIR/decision.json'))['redirectTo']).query) +print(q['code'][0])")" +echo " approved; code issued (state and iss are echoed back to the client)" + +# -------------------------------------------------------------------- token --- +say "5. Redeem the code" +curl -s -X POST "$API/v1/oauth/token" \ + -d "grant_type=authorization_code&code=$CODE&client_id=$CLIENT_ID&redirect_uri=$REDIRECT&code_verifier=$VERIFIER&resource=$RESOURCE" \ + -o "$WORK_DIR/tokens.json" +grep -q access_token "$WORK_DIR/tokens.json" || fail "$(cat "$WORK_DIR/tokens.json")" +ACCESS_TOKEN="$(json_get "$WORK_DIR/tokens.json" access_token)" +REFRESH_TOKEN="$(json_get "$WORK_DIR/tokens.json" refresh_token)" +echo " claims in the client's token:" +claims "$ACCESS_TOKEN" | sed 's/^/ /' + +# ------------------------------------------------------- use it on the API --- +if [[ "$RESOURCE_KIND" == "mcp" ]]; then + say "6. Token exchange (what the MCP resource server does per tool call)" + BASIC="$(printf 'openops-mcp-rs:%s' "$RS_SECRET" | base64 | tr -d '\n')" + echo " the client's own token must NOT work against the API:" + echo " HTTP $(curl -s -o /dev/null -w '%{http_code}' -H "Authorization: Bearer $ACCESS_TOKEN" "$API/v1/flows") (expect 401)" + curl -s -X POST "$API/v1/oauth/token" -H "Authorization: Basic $BASIC" \ + -d "grant_type=urn:ietf:params:oauth:grant-type:token-exchange&subject_token=$ACCESS_TOKEN" \ + -o "$WORK_DIR/exchange.json" + grep -q access_token "$WORK_DIR/exchange.json" || fail "$(cat "$WORK_DIR/exchange.json")" + API_TOKEN="$(json_get "$WORK_DIR/exchange.json" access_token)" + echo " exchanged for a separate API-audience token:" + claims "$API_TOKEN" | sed 's/^/ /' +else + API_TOKEN="$ACCESS_TOKEN" +fi + +say "7. Call the API with it" +PROJECT_ID="$(python3 -c " +import base64, json +p = '$API_TOKEN'.split('.')[1]; p += '=' * (-len(p) % 4) +print(json.loads(base64.urlsafe_b64decode(p))['project_id'])")" +STATUS="$(curl -s -o "$WORK_DIR/flows.json" -w '%{http_code}' \ + -H "Authorization: Bearer $API_TOKEN" "$API/v1/flows?projectId=$PROJECT_ID")" +echo " GET /v1/flows -> HTTP $STATUS" +[[ "$STATUS" == "200" ]] || fail "the token was refused by the API" + +# ------------------------------------------------------------------ refresh --- +say "8. Refresh, and confirm the old token is single-use" +curl -s -X POST "$API/v1/oauth/token" \ + -d "grant_type=refresh_token&refresh_token=$REFRESH_TOKEN&client_id=$CLIENT_ID" \ + -o "$WORK_DIR/rotated.json" +grep -q access_token "$WORK_DIR/rotated.json" || fail "$(cat "$WORK_DIR/rotated.json")" +ROTATED_REFRESH="$(json_get "$WORK_DIR/rotated.json" refresh_token)" +echo " rotated; new refresh token issued" +echo " replaying the old one: $(curl -s -X POST "$API/v1/oauth/token" \ + -d "grant_type=refresh_token&refresh_token=$REFRESH_TOKEN&client_id=$CLIENT_ID" | + python3 -c "import sys,json;print(json.load(sys.stdin)['error_description'])")" +echo " (that also kills the rotated token — a replay means the chain is untrusted)" + +# ----------------------------------------------------- connections + revoke --- +say "9. Connected apps, and revoking one" +curl -s -b "$WORK_DIR/cookies" "$API/v1/oauth/grants" | python3 -c " +import sys, json +for g in json.load(sys.stdin)['data']: + print(f\" {g['clientName']} grant={g['id']} via={g['resourceId']} last used={g['lastUsedAt']}\")" + +GRANT_ID="$(python3 -c " +import base64, json +p = '$API_TOKEN'.split('.')[1]; p += '=' * (-len(p) % 4) +print(json.loads(base64.urlsafe_b64decode(p))['grant_id'])")" +curl -s -b "$WORK_DIR/cookies" -X DELETE "$API/v1/oauth/grants/$GRANT_ID" -o /dev/null +echo " revoked grant $GRANT_ID" +echo " API call with its still-unexpired token: HTTP $(curl -s -o /dev/null -w '%{http_code}' \ + -H "Authorization: Bearer $API_TOKEN" "$API/v1/flows?projectId=$PROJECT_ID") (expect 401)" +echo " refresh after revocation: $(curl -s -X POST "$API/v1/oauth/token" \ + -d "grant_type=refresh_token&refresh_token=$ROTATED_REFRESH&client_id=$CLIENT_ID" | + python3 -c "import sys,json;print(json.load(sys.stdin)['error_description'])")" + +say "Done — full flow verified for the '$RESOURCE_KIND' resource." diff --git a/tools/scripts/utils/files.ts b/tools/scripts/utils/files.ts index f657a60404..81e598e2b1 100644 --- a/tools/scripts/utils/files.ts +++ b/tools/scripts/utils/files.ts @@ -12,8 +12,7 @@ export type ProjectJson = { build?: { options?: { buildableProjectDepsInPackageJsonType?: - | 'peerDependencies' - | 'dependencies'; + 'peerDependencies' | 'dependencies'; updateBuildableProjectDepsInPackageJson: boolean; }; };