From 22eb9cba0d647bba3d787619bf2d2ff968665daa Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Marc-Andr=C3=A9=20Bombeck?= Date: Sat, 27 Jun 2026 15:08:52 +0200 Subject: [PATCH] fix(mcp): close OAuth 2.1 resource-server gaps before the remote surface ships Harden the remote MCP endpoint and the in-repo OAuth bridge against the adversarial review of the milestone branch. The two ship-blockers made the "read-only, revocable" promise false; the rest tightened the surface. Audience binding at the resource server: a token whose only grant is the new `health:read` scope is the MCP read credential. The REST edge now accepts it on safe (read) methods and refuses it on writes/deletes with 403, so a connector token can read over `/mcp` and GET routes but can never POST/DELETE the record despite a "read-only" consent. The `/mcp` resolver is unaffected. Revocable, theft-detectable refresh tokens: refresh artifacts now anchor to a persistent connection row. Revoking a connection stops every future refresh and kills its access tokens; a replayed (already-rotated) refresh is treated as theft and revokes the whole connection. Connections surface in settings as the unit a user can disconnect. The old access token is revoked on rotation. Also: fail closed on the OAuth/`/mcp` surface when no canonical origin is configured and mark the discovery documents non-cacheable; honour the global API kill-switch on `/mcp`, `/token`, `/authorize`, and `/register`; cap the CIMD and registration body reads while streaming instead of after; show the verified client origin on consent; validate CIMD redirect-URI schemes; assert same-origin on the consent POST; and add a daily prune for expired connector tokens. New migration 0195 adds the connection anchor and links access tokens to it. --- messages/de.json | 11 +- messages/en.json | 11 +- messages/es.json | 11 +- messages/fr.json | 11 +- messages/it.json | 11 +- messages/pl.json | 11 +- .../migration.sql | 41 ++++ prisma/schema.prisma | 42 +++- .../__tests__/oauth-discovery.test.ts | 59 ++++++ .../oauth-authorization-server/route.ts | 13 +- .../oauth-protected-resource/route.ts | 13 +- src/app/api/mcp/connections/[id]/route.ts | 41 ++++ src/app/api/mcp/connections/route.ts | 26 +++ .../api/mcp/oauth/__tests__/authorize.test.ts | 91 ++++++++ .../api/mcp/oauth/__tests__/register.test.ts | 18 +- src/app/api/mcp/oauth/__tests__/token.test.ts | 131 +++++++++++- src/app/api/mcp/oauth/authorize/route.ts | 78 +++++++ src/app/api/mcp/oauth/register/route.ts | 38 ++-- src/app/api/mcp/oauth/token/route.ts | 128 +++++++++--- src/app/api/mcp/tokens/route.ts | 9 +- src/app/mcp/__tests__/route.test.ts | 19 ++ src/app/mcp/route.ts | 15 +- src/components/settings/mcp-section.tsx | 123 ++++++++++- src/lib/__tests__/require-auth-bearer.test.ts | 85 +++++++- src/lib/api-handler.ts | 46 ++++- src/lib/auth/issue-token.ts | 9 + src/lib/http/__tests__/read-capped.test.ts | 68 ++++++ src/lib/http/read-capped.ts | 62 ++++++ .../__tests__/mcp-token-cleanup-queue.test.ts | 58 ++++++ src/lib/jobs/mcp-token-cleanup.ts | 55 +++++ src/lib/jobs/reminder/cleanup-handlers.ts | 27 +++ src/lib/jobs/reminder/register-maintenance.ts | 23 +++ src/lib/logging/event-builder.ts | 5 + .../mcp/oauth/__tests__/connections.test.ts | 195 ++++++++++++++++++ .../mcp/oauth/__tests__/oauth-bridge.test.ts | 29 +++ src/lib/mcp/oauth/clients.ts | 38 +++- src/lib/mcp/oauth/config.ts | 53 ++++- src/lib/mcp/oauth/connections.ts | 190 +++++++++++++++++ src/lib/query-keys/integrations.ts | 4 + 39 files changed, 1826 insertions(+), 72 deletions(-) create mode 100644 prisma/migrations/0195_mcp_phase3_oauth_connection/migration.sql create mode 100644 src/app/.well-known/__tests__/oauth-discovery.test.ts create mode 100644 src/app/api/mcp/connections/[id]/route.ts create mode 100644 src/app/api/mcp/connections/route.ts create mode 100644 src/lib/http/__tests__/read-capped.test.ts create mode 100644 src/lib/http/read-capped.ts create mode 100644 src/lib/jobs/__tests__/mcp-token-cleanup-queue.test.ts create mode 100644 src/lib/jobs/mcp-token-cleanup.ts create mode 100644 src/lib/mcp/oauth/__tests__/connections.test.ts create mode 100644 src/lib/mcp/oauth/connections.ts diff --git a/messages/de.json b/messages/de.json index a41e7f47c..0e5669e2b 100644 --- a/messages/de.json +++ b/messages/de.json @@ -4907,7 +4907,16 @@ "connectDescription": "Füge diese URL in die Einrichtung des eigenen Connectors deines Assistenten ein und autorisiere dich mit dem Token unten oder melde dich bei Aufforderung an.", "tokensTitle": "Connector-Tokens", "tokensDescription": "Erzeuge ein schreibgeschütztes Token, um einen Assistenten zu verbinden. Das Token wird nur einmal angezeigt.", - "scopeNote": "Tokens sind auf health:read beschränkt — nur Lesezugriff auf deine eigenen Daten. Sie können niemals Daten schreiben oder Admin-Funktionen erreichen." + "scopeNote": "Tokens sind auf health:read beschränkt — nur Lesezugriff auf deine eigenen Daten. Sie können niemals Daten schreiben oder Admin-Funktionen erreichen.", + "connectionsTitle": "Verbundene Assistenten", + "connectionsDescription": "Assistenten, die du über die Anmeldung autorisiert hast. Das Widerrufen einer Verbindung beendet den Zugriff sofort, einschließlich der Möglichkeit zur Erneuerung.", + "noConnections": "Keine verbundenen Assistenten.", + "connectionConnected": "Verbunden", + "connectionLastUsed": "Zuletzt verwendet", + "connectionNeverUsed": "Nie verwendet", + "connectionRevoke": "Trennen", + "connectionRevokeDescription": "Dies beendet den Zugriff des Assistenten sofort und verhindert eine Erneuerung. Für eine erneute Verbindung ist eine neue Autorisierung nötig.", + "connectionRevokeFailed": "Assistent konnte nicht getrennt werden." } }, "admin": { diff --git a/messages/en.json b/messages/en.json index 0a94608d8..0b1200347 100644 --- a/messages/en.json +++ b/messages/en.json @@ -4907,7 +4907,16 @@ "connectDescription": "Paste this URL into your assistant's custom-connector setup, then authorize with the token below or sign in when prompted.", "tokensTitle": "Connector tokens", "tokensDescription": "Mint a read-only token to connect an assistant. The token is shown once.", - "scopeNote": "Tokens are scoped to health:read — read-only access to your own records. They can never write data or reach admin functions." + "scopeNote": "Tokens are scoped to health:read — read-only access to your own records. They can never write data or reach admin functions.", + "connectionsTitle": "Connected assistants", + "connectionsDescription": "Assistants you authorized through the sign-in flow. Revoking a connection ends its access immediately, including its ability to refresh.", + "noConnections": "No connected assistants.", + "connectionConnected": "Connected", + "connectionLastUsed": "Last used", + "connectionNeverUsed": "Never used", + "connectionRevoke": "Disconnect", + "connectionRevokeDescription": "This ends the assistant's access immediately and stops it from renewing. It will need to be re-authorized to reconnect.", + "connectionRevokeFailed": "Could not disconnect the assistant." } }, "admin": { diff --git a/messages/es.json b/messages/es.json index 19d8b2a1a..cac512da5 100644 --- a/messages/es.json +++ b/messages/es.json @@ -4907,7 +4907,16 @@ "connectDescription": "Pega esta URL en la configuración del conector personalizado de tu asistente y autoriza con el token de abajo o inicia sesión cuando se te solicite.", "tokensTitle": "Tokens del conector", "tokensDescription": "Genera un token de solo lectura para conectar un asistente. El token se muestra una sola vez.", - "scopeNote": "Los tokens se limitan a health:read: acceso de solo lectura a tus propios registros. Nunca pueden escribir datos ni acceder a funciones de administración." + "scopeNote": "Los tokens se limitan a health:read: acceso de solo lectura a tus propios registros. Nunca pueden escribir datos ni acceder a funciones de administración.", + "connectionsTitle": "Asistentes conectados", + "connectionsDescription": "Asistentes que autorizaste mediante el inicio de sesión. Revocar una conexión finaliza su acceso de inmediato, incluida su capacidad de renovación.", + "noConnections": "No hay asistentes conectados.", + "connectionConnected": "Conectado", + "connectionLastUsed": "Último uso", + "connectionNeverUsed": "Nunca usado", + "connectionRevoke": "Desconectar", + "connectionRevokeDescription": "Esto finaliza el acceso del asistente de inmediato e impide su renovación. Será necesario autorizarlo de nuevo para volver a conectarlo.", + "connectionRevokeFailed": "No se pudo desconectar el asistente." } }, "admin": { diff --git a/messages/fr.json b/messages/fr.json index 9861439ee..ff0d19256 100644 --- a/messages/fr.json +++ b/messages/fr.json @@ -4907,7 +4907,16 @@ "connectDescription": "Collez cette URL dans la configuration du connecteur personnalisé de votre assistant, puis autorisez avec le jeton ci-dessous ou connectez-vous lorsque vous y êtes invité.", "tokensTitle": "Jetons du connecteur", "tokensDescription": "Générez un jeton en lecture seule pour connecter un assistant. Le jeton n'est affiché qu'une seule fois.", - "scopeNote": "Les jetons sont limités à health:read : accès en lecture seule à vos propres données. Ils ne peuvent jamais écrire de données ni atteindre les fonctions d'administration." + "scopeNote": "Les jetons sont limités à health:read : accès en lecture seule à vos propres données. Ils ne peuvent jamais écrire de données ni atteindre les fonctions d'administration.", + "connectionsTitle": "Assistants connectés", + "connectionsDescription": "Assistants que vous avez autorisés via la connexion. Révoquer une connexion met fin à son accès immédiatement, y compris sa capacité à se renouveler.", + "noConnections": "Aucun assistant connecté.", + "connectionConnected": "Connecté", + "connectionLastUsed": "Dernière utilisation", + "connectionNeverUsed": "Jamais utilisé", + "connectionRevoke": "Déconnecter", + "connectionRevokeDescription": "Cela met fin immédiatement à l'accès de l'assistant et l'empêche de se renouveler. Une nouvelle autorisation sera nécessaire pour le reconnecter.", + "connectionRevokeFailed": "Impossible de déconnecter l'assistant." } }, "admin": { diff --git a/messages/it.json b/messages/it.json index 4ba9e434d..40fd73e45 100644 --- a/messages/it.json +++ b/messages/it.json @@ -4907,7 +4907,16 @@ "connectDescription": "Incolla questo URL nella configurazione del connettore personalizzato del tuo assistente e autorizza con il token qui sotto o accedi quando richiesto.", "tokensTitle": "Token del connettore", "tokensDescription": "Genera un token in sola lettura per collegare un assistente. Il token viene mostrato una sola volta.", - "scopeNote": "I token sono limitati a health:read: accesso in sola lettura ai tuoi dati. Non possono mai scrivere dati né raggiungere funzioni di amministrazione." + "scopeNote": "I token sono limitati a health:read: accesso in sola lettura ai tuoi dati. Non possono mai scrivere dati né raggiungere funzioni di amministrazione.", + "connectionsTitle": "Assistenti collegati", + "connectionsDescription": "Assistenti che hai autorizzato tramite l'accesso. Revocare una connessione ne termina subito l'accesso, inclusa la capacità di rinnovarsi.", + "noConnections": "Nessun assistente collegato.", + "connectionConnected": "Collegato", + "connectionLastUsed": "Ultimo utilizzo", + "connectionNeverUsed": "Mai usato", + "connectionRevoke": "Disconnetti", + "connectionRevokeDescription": "Questo termina immediatamente l'accesso dell'assistente e ne impedisce il rinnovo. Per riconnetterlo sarà necessaria una nuova autorizzazione.", + "connectionRevokeFailed": "Impossibile disconnettere l'assistente." } }, "admin": { diff --git a/messages/pl.json b/messages/pl.json index 735be10f4..cfbf4ef2b 100644 --- a/messages/pl.json +++ b/messages/pl.json @@ -4907,7 +4907,16 @@ "connectDescription": "Wklej ten adres URL w konfiguracji własnego łącznika asystenta, a następnie autoryzuj za pomocą poniższego tokenu lub zaloguj się po wyświetleniu monitu.", "tokensTitle": "Tokeny łącznika", "tokensDescription": "Wygeneruj token tylko do odczytu, aby połączyć asystenta. Token jest wyświetlany tylko raz.", - "scopeNote": "Tokeny są ograniczone do health:read — dostęp tylko do odczytu własnych danych. Nigdy nie mogą zapisywać danych ani uzyskać dostępu do funkcji administracyjnych." + "scopeNote": "Tokeny są ograniczone do health:read — dostęp tylko do odczytu własnych danych. Nigdy nie mogą zapisywać danych ani uzyskać dostępu do funkcji administracyjnych.", + "connectionsTitle": "Połączeni asystenci", + "connectionsDescription": "Asystenci autoryzowani podczas logowania. Cofnięcie połączenia natychmiast kończy dostęp, w tym możliwość odnawiania.", + "noConnections": "Brak połączonych asystentów.", + "connectionConnected": "Połączono", + "connectionLastUsed": "Ostatnio użyto", + "connectionNeverUsed": "Nigdy nie użyto", + "connectionRevoke": "Rozłącz", + "connectionRevokeDescription": "To natychmiast kończy dostęp asystenta i uniemożliwia odnawianie. Ponowne połączenie będzie wymagać nowej autoryzacji.", + "connectionRevokeFailed": "Nie udało się rozłączyć asystenta." } }, "admin": { diff --git a/prisma/migrations/0195_mcp_phase3_oauth_connection/migration.sql b/prisma/migrations/0195_mcp_phase3_oauth_connection/migration.sql new file mode 100644 index 000000000..8df886ee4 --- /dev/null +++ b/prisma/migrations/0195_mcp_phase3_oauth_connection/migration.sql @@ -0,0 +1,41 @@ +-- MCP Phase 3 security fix (H2) — revocable, theft-detectable refresh chains. +-- +-- The remote MCP OAuth bridge minted stateless `hlrt_` refresh tokens that +-- referenced no server row: they could not be revoked (the settings "revoke" +-- only killed the 60-minute access token) and a replayed, already-rotated +-- refresh token was not treated as theft. This migration adds the persistent +-- anchor the refresh artifact lacked. +-- +-- `mcp_oauth_connections` is the revocable unit: one row per connector +-- connection that requested `offline_access`. `current_jti` is the only refresh +-- `jti` valid at any moment — every rotation advances it, so a presented `jti` +-- that is not `current_jti` is a replay (theft signal) that revokes the row. +-- `revoked_at` is the kill switch the settings card flips; revoking the row +-- stops every future refresh. +CREATE TABLE "mcp_oauth_connections" ( + "id" TEXT NOT NULL, + "user_id" TEXT NOT NULL, + "client_id" TEXT NOT NULL, + "client_name" TEXT NOT NULL, + "scope" TEXT NOT NULL, + "resource" TEXT NOT NULL, + "current_jti" TEXT NOT NULL, + "created_at" TIMESTAMP(3) NOT NULL DEFAULT CURRENT_TIMESTAMP, + "last_used_at" TIMESTAMP(3), + "revoked_at" TIMESTAMP(3), + + CONSTRAINT "mcp_oauth_connections_pkey" PRIMARY KEY ("id") +); + +CREATE INDEX "mcp_oauth_connections_user_id_idx" ON "mcp_oauth_connections"("user_id"); + +ALTER TABLE "mcp_oauth_connections" ADD CONSTRAINT "mcp_oauth_connections_user_id_fkey" FOREIGN KEY ("user_id") REFERENCES "users"("id") ON DELETE CASCADE ON UPDATE CASCADE; + +-- Link each OAuth-minted access token back to its connection so a connection +-- revoke kills every access row it issued, and so the connector token list can +-- exclude the transient access rows (they would otherwise flood it — M2). +ALTER TABLE "api_tokens" ADD COLUMN "mcp_connection_id" TEXT; + +CREATE INDEX "api_tokens_mcp_connection_id_idx" ON "api_tokens"("mcp_connection_id"); + +ALTER TABLE "api_tokens" ADD CONSTRAINT "api_tokens_mcp_connection_id_fkey" FOREIGN KEY ("mcp_connection_id") REFERENCES "mcp_oauth_connections"("id") ON DELETE SET NULL ON UPDATE CASCADE; diff --git a/prisma/schema.prisma b/prisma/schema.prisma index b0c8bb68b..172aafc9d 100644 --- a/prisma/schema.prisma +++ b/prisma/schema.prisma @@ -536,6 +536,7 @@ model User { medications Medication[] intakeEvents MedicationIntakeEvent[] apiTokens ApiToken[] + mcpOAuthConnections McpOAuthConnection[] refreshTokens RefreshToken[] auditLogs AuditLog[] withingsConnection WithingsConnection? @@ -2369,13 +2370,52 @@ model ApiToken { expiresAt DateTime? @map("expires_at") createdAt DateTime @default(now()) @map("created_at") revoked Boolean @default(false) + // The MCP OAuth connection this access token belongs to, when it was minted + // through the `/api/mcp/oauth` bridge. Null for manually-minted tokens and + // native-client tokens. Lets a connection revoke kill every access row it + // ever issued (H2), and lets the connector token list exclude the transient + // 60-minute access rows so it does not flood (M2). + mcpConnectionId String? @map("mcp_connection_id") - user User @relation(fields: [userId], references: [id], onDelete: Cascade) + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + mcpConnection McpOAuthConnection? @relation(fields: [mcpConnectionId], references: [id], onDelete: SetNull) @@index([userId]) + @@index([mcpConnectionId]) @@map("api_tokens") } +// MCP OAuth connection — the revocable, theft-detectable anchor for a remote +// connector's refresh-token chain (Phase 3 security fix, H2). +// +// The stateless `hlrt_` refresh artifact carries this row's `id` (`cid`) plus +// the rotation `jti`; the row is the persistent server state the artifact alone +// lacks. `currentJti` is the only refresh `jti` valid for the connection at any +// moment — every rotation advances it. A presented refresh whose `jti` is not +// `currentJti` is a replay of an already-rotated token (theft signal) and +// revokes the connection (reuse-detection family revocation, mirroring +// `refresh-token.ts`). `revokedAt` is the kill switch the settings card flips, +// so revoking a connection stops every future refresh and (via the +// `ApiToken.mcpConnectionId` back-link) kills its outstanding access tokens. +model McpOAuthConnection { + id String @id @default(cuid()) + userId String @map("user_id") + clientId String @map("client_id") + clientName String @map("client_name") + scope String + resource String + currentJti String @map("current_jti") + createdAt DateTime @default(now()) @map("created_at") + lastUsedAt DateTime? @map("last_used_at") + revokedAt DateTime? @map("revoked_at") + + user User @relation(fields: [userId], references: [id], onDelete: Cascade) + accessTokens ApiToken[] + + @@index([userId]) + @@map("mcp_oauth_connections") +} + // v1.15.20 — registration invite tokens. When the operator has disabled // open registration (`AppSettings.registrationEnabled = false`), a valid, // unexpired invite token still admits a signup. The raw `hlv_` token diff --git a/src/app/.well-known/__tests__/oauth-discovery.test.ts b/src/app/.well-known/__tests__/oauth-discovery.test.ts new file mode 100644 index 000000000..52f9fd35e --- /dev/null +++ b/src/app/.well-known/__tests__/oauth-discovery.test.ts @@ -0,0 +1,59 @@ +/** + * OAuth discovery documents — M1 fail-closed + non-cacheable. + * + * Without a pinned origin the documents would derive the issuer / endpoints / + * audience from the Host header, so the routes 404. When configured they must + * not be `public`-cacheable (a host-derived document keyed on path alone could + * be served cross-tenant). + */ +import { describe, it, expect, beforeEach, afterEach } from "vitest"; + +const PRM_URL = "https://health.example/.well-known/oauth-protected-resource"; +const ASM_URL = "https://health.example/.well-known/oauth-authorization-server"; + +let GET_PRM: (r: Request) => Response; +let GET_ASM: (r: Request) => Response; + +beforeEach(async () => { + GET_PRM = (await import("../oauth-protected-resource/route")).GET; + GET_ASM = (await import("../oauth-authorization-server/route")).GET; +}); + +afterEach(() => { + delete process.env.APP_URL; + delete process.env.NEXT_PUBLIC_APP_URL; +}); + +describe("OAuth discovery — M1 origin config", () => { + it("404s the PRM when no origin is configured", () => { + delete process.env.APP_URL; + delete process.env.NEXT_PUBLIC_APP_URL; + const res = GET_PRM(new Request(PRM_URL)); + expect(res.status).toBe(404); + }); + + it("404s the AS metadata when no origin is configured", () => { + delete process.env.APP_URL; + delete process.env.NEXT_PUBLIC_APP_URL; + const res = GET_ASM(new Request(ASM_URL)); + expect(res.status).toBe(404); + }); + + it("serves the PRM with private/no-store when configured", async () => { + process.env.APP_URL = "https://health.example"; + const res = GET_PRM(new Request(PRM_URL)); + expect(res.status).toBe(200); + expect(res.headers.get("cache-control")).toBe("private, no-store"); + const body = await res.json(); + expect(body.resource).toBe("https://health.example/mcp"); + }); + + it("serves the AS metadata with private/no-store when configured", async () => { + process.env.APP_URL = "https://health.example"; + const res = GET_ASM(new Request(ASM_URL)); + expect(res.status).toBe(200); + expect(res.headers.get("cache-control")).toBe("private, no-store"); + const body = await res.json(); + expect(body.issuer).toBe("https://health.example"); + }); +}); diff --git a/src/app/.well-known/oauth-authorization-server/route.ts b/src/app/.well-known/oauth-authorization-server/route.ts index f52c79d27..9b22bb096 100644 --- a/src/app/.well-known/oauth-authorization-server/route.ts +++ b/src/app/.well-known/oauth-authorization-server/route.ts @@ -7,12 +7,23 @@ * Claude.ai / ChatGPT can self-register and complete the OAuth 2.1 + PKCE flow. */ import { authorizationServerMetadata } from "@/lib/mcp/oauth/metadata"; +import { isMcpOriginConfigured } from "@/lib/mcp/oauth/config"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; export function GET(request: Request): Response { + // M1 — fail closed when no origin is pinned; never derive the issuer / + // endpoint URLs from the attacker-influenceable Host header. + if (!isMcpOriginConfigured()) { + return Response.json( + { error: "not_found" }, + { status: 404, headers: { "Cache-Control": "no-store" } }, + ); + } return Response.json(authorizationServerMetadata(request.url), { - headers: { "Cache-Control": "public, max-age=300" }, + // M1 — `private, no-store`: the document is host-derived, so a shared cache + // keyed on path (ignoring Host) must never serve it cross-tenant. + headers: { "Cache-Control": "private, no-store" }, }); } diff --git a/src/app/.well-known/oauth-protected-resource/route.ts b/src/app/.well-known/oauth-protected-resource/route.ts index 46a4a79fb..b91c2910e 100644 --- a/src/app/.well-known/oauth-protected-resource/route.ts +++ b/src/app/.well-known/oauth-protected-resource/route.ts @@ -8,12 +8,23 @@ * pasted into Claude.ai / ChatGPT. */ import { protectedResourceMetadata } from "@/lib/mcp/oauth/metadata"; +import { isMcpOriginConfigured } from "@/lib/mcp/oauth/config"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; export function GET(request: Request): Response { + // M1 — fail closed when no origin is pinned; never derive the `resource` / + // authorization-server URLs from the attacker-influenceable Host header. + if (!isMcpOriginConfigured()) { + return Response.json( + { error: "not_found" }, + { status: 404, headers: { "Cache-Control": "no-store" } }, + ); + } return Response.json(protectedResourceMetadata(request.url), { - headers: { "Cache-Control": "public, max-age=300" }, + // M1 — `private, no-store`: the document is host-derived, so a shared cache + // keyed on path (ignoring Host) must never serve it cross-tenant. + headers: { "Cache-Control": "private, no-store" }, }); } diff --git a/src/app/api/mcp/connections/[id]/route.ts b/src/app/api/mcp/connections/[id]/route.ts new file mode 100644 index 000000000..63df749c4 --- /dev/null +++ b/src/app/api/mcp/connections/[id]/route.ts @@ -0,0 +1,41 @@ +/** + * Revoke an MCP OAuth connection (H2). + * + * Revoking a connection terminates its whole refresh chain: it stamps + * `revokedAt` (so every future refresh fails) and revokes every access token + * the connection ever issued. Ownership is enforced inside + * `revokeConnectionForUser`, so a connection id alone cannot revoke another + * user's connection. + */ +import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiError, apiSuccess } from "@/lib/api-response"; +import { annotate } from "@/lib/logging/context"; +import { auditLog } from "@/lib/auth/audit"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; +import { revokeConnectionForUser } from "@/lib/mcp/oauth/connections"; + +type RouteParams = { params: Promise<{ id: string }> }; + +export const DELETE = apiHandler( + async (_request: Request, { params }: RouteParams) => { + const { user } = await requireAuth(); + annotate({ action: { name: "mcp.connections.revoke" } }); + + if (!(await isApiGloballyEnabled())) { + return apiError("API is globally disabled", 403); + } + + const { id } = await params; + const revoked = await revokeConnectionForUser(user.id, id); + if (!revoked) { + return apiError("Connection not found", 404); + } + + await auditLog("mcp.connections.revoke", { + userId: user.id, + details: { connectionId: id }, + }); + + return apiSuccess({ revoked: true }); + }, +); diff --git a/src/app/api/mcp/connections/route.ts b/src/app/api/mcp/connections/route.ts new file mode 100644 index 000000000..3bd24e169 --- /dev/null +++ b/src/app/api/mcp/connections/route.ts @@ -0,0 +1,26 @@ +/** + * MCP OAuth connections — list (H2). + * + * A connection is the revocable unit for a remote connector authorized through + * the OAuth bridge (Claude.ai / ChatGPT / a third-party client). Unlike the + * transient 60-minute access-token rows, the connection persists across + * refreshes, so revoking it terminates the whole refresh chain. `userId` is + * always narrowed from the session — never a body field. + */ +import { apiHandler, requireAuth } from "@/lib/api-handler"; +import { apiError, apiSuccess } from "@/lib/api-response"; +import { annotate } from "@/lib/logging/context"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; +import { listConnectionsForUser } from "@/lib/mcp/oauth/connections"; + +export const GET = apiHandler(async () => { + const { user } = await requireAuth(); + annotate({ action: { name: "mcp.connections.list" } }); + + if (!(await isApiGloballyEnabled())) { + return apiError("API is globally disabled", 403); + } + + const connections = await listConnectionsForUser(user.id); + return apiSuccess(connections); +}); diff --git a/src/app/api/mcp/oauth/__tests__/authorize.test.ts b/src/app/api/mcp/oauth/__tests__/authorize.test.ts index fc65471c3..7079af430 100644 --- a/src/app/api/mcp/oauth/__tests__/authorize.test.ts +++ b/src/app/api/mcp/oauth/__tests__/authorize.test.ts @@ -21,9 +21,13 @@ vi.mock("@/lib/rate-limit", () => ({ })), rateLimitHeaders: vi.fn(() => ({})), })); +vi.mock("@/lib/app-settings", () => ({ + isApiGloballyEnabled: vi.fn(async () => true), +})); import { GET, POST } from "../authorize/route"; import { getSession } from "@/lib/auth/session"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; import { registerDcrClient } from "@/lib/mcp/oauth/clients"; import { s256Challenge } from "@/lib/mcp/oauth/pkce"; @@ -81,6 +85,7 @@ function signedIn() { beforeEach(() => { vi.clearAllMocks(); vi.mocked(getSession).mockResolvedValue(null); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); }); describe("GET /authorize — consent gate", () => { @@ -157,6 +162,92 @@ describe("GET /authorize — rejected requests", () => { }); }); +describe("GET /authorize — consent provenance (L1)", () => { + it("labels a DCR client as unverified (self-asserted name)", async () => { + signedIn(); + const res = await GET(getReq(authorizeUrl()) as never); + const body = await res.text(); + expect(body).toMatch(/Unverified application/); + }); +}); + +describe("authorize — surface availability (M1/M4)", () => { + it("GET returns 503 when the API is globally disabled", async () => { + signedIn(); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(false); + const res = await GET(getReq(authorizeUrl()) as never); + expect(res.status).toBe(503); + }); + + it("POST returns 503 when the API is globally disabled", async () => { + signedIn(); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(false); + const res = await POST( + postReq({ + response_type: "code", + client_id: CLIENT.clientId, + redirect_uri: REDIRECT, + code_challenge: CHALLENGE, + code_challenge_method: "S256", + scope: "health:read", + resource: RESOURCE, + decision: "allow", + }) as never, + ); + expect(res.status).toBe(503); + }); +}); + +describe("POST /authorize — cross-origin guard (L3)", () => { + it("rejects a cross-origin Origin with 403", async () => { + signedIn(); + const body = new URLSearchParams({ + response_type: "code", + client_id: CLIENT.clientId, + redirect_uri: REDIRECT, + code_challenge: CHALLENGE, + code_challenge_method: "S256", + scope: "health:read", + resource: RESOURCE, + decision: "allow", + }); + const req = new Request("https://health.example/api/mcp/oauth/authorize", { + method: "POST", + headers: { + "content-type": "application/x-www-form-urlencoded", + origin: "https://evil.example", + }, + body: body.toString(), + }); + const res = await POST(req as never); + expect(res.status).toBe(403); + }); + + it("rejects a cross-site Sec-Fetch-Site with 403", async () => { + signedIn(); + const body = new URLSearchParams({ + response_type: "code", + client_id: CLIENT.clientId, + redirect_uri: REDIRECT, + code_challenge: CHALLENGE, + code_challenge_method: "S256", + scope: "health:read", + resource: RESOURCE, + decision: "allow", + }); + const req = new Request("https://health.example/api/mcp/oauth/authorize", { + method: "POST", + headers: { + "content-type": "application/x-www-form-urlencoded", + "sec-fetch-site": "cross-site", + }, + body: body.toString(), + }); + const res = await POST(req as never); + expect(res.status).toBe(403); + }); +}); + describe("POST /authorize — decision", () => { it("requires a session (401 when unauthenticated)", async () => { const res = await POST( diff --git a/src/app/api/mcp/oauth/__tests__/register.test.ts b/src/app/api/mcp/oauth/__tests__/register.test.ts index bcf37c2be..9bacbea67 100644 --- a/src/app/api/mcp/oauth/__tests__/register.test.ts +++ b/src/app/api/mcp/oauth/__tests__/register.test.ts @@ -18,8 +18,12 @@ vi.mock("@/lib/rate-limit", () => ({ })), rateLimitHeaders: vi.fn(() => ({})), })); +vi.mock("@/lib/app-settings", () => ({ + isApiGloballyEnabled: vi.fn(async () => true), +})); import { POST } from "../register/route"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; function jsonReq(body: unknown): Request { return new Request("https://health.example/api/mcp/oauth/register", { @@ -29,7 +33,10 @@ function jsonReq(body: unknown): Request { }); } -beforeEach(() => vi.clearAllMocks()); +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); +}); describe("POST /register", () => { it("registers a public client and returns an hlc_ client_id", async () => { @@ -69,4 +76,13 @@ describe("POST /register", () => { ); expect(res.status).toBe(400); }); + + it("returns 503 when the API is globally disabled (M4)", async () => { + vi.mocked(isApiGloballyEnabled).mockResolvedValue(false); + const res = await POST( + jsonReq({ redirect_uris: ["https://chatgpt.com/cb"] }) as never, + ); + expect(res.status).toBe(503); + expect((await res.json()).error).toBe("temporarily_unavailable"); + }); }); diff --git a/src/app/api/mcp/oauth/__tests__/token.test.ts b/src/app/api/mcp/oauth/__tests__/token.test.ts index c48bf9872..76e3fbb07 100644 --- a/src/app/api/mcp/oauth/__tests__/token.test.ts +++ b/src/app/api/mcp/oauth/__tests__/token.test.ts @@ -34,11 +34,23 @@ vi.mock("@/lib/rate-limit", () => ({ })), rateLimitHeaders: vi.fn(() => ({})), })); +vi.mock("@/lib/app-settings", () => ({ + isApiGloballyEnabled: vi.fn(async () => true), +})); +vi.mock("@/lib/mcp/oauth/connections", () => ({ + createConnection: vi.fn(async () => "conn-1"), + rotateConnection: vi.fn(async () => ({ ok: true })), +})); import { randomUUID } from "node:crypto"; import { POST } from "../token/route"; import { issueApiToken } from "@/lib/auth/issue-token"; import { checkRateLimit } from "@/lib/rate-limit"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; +import { + createConnection, + rotateConnection, +} from "@/lib/mcp/oauth/connections"; import { signArtifact } from "@/lib/mcp/oauth/artifacts"; import { s256Challenge } from "@/lib/mcp/oauth/pkce"; @@ -80,8 +92,27 @@ beforeEach(() => { remaining: 0, resetAt: Date.now() + 60_000, }); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); + vi.mocked(createConnection).mockResolvedValue("conn-1"); + vi.mocked(rotateConnection).mockResolvedValue({ ok: true }); }); +function refreshToken(overrides: Record = {}): string { + return signArtifact( + "refreshToken", + { + jti: randomUUID(), + cid: "conn-1", + sub: "user-1", + client_id: CLIENT_ID, + scope: "health:read offline_access", + resource: RESOURCE, + ...overrides, + }, + 1_000_000, + ); +} + describe("authorization_code grant", () => { it("exchanges a valid code + verifier for an access token (+ refresh)", async () => { const res = await POST( @@ -104,6 +135,22 @@ describe("authorization_code grant", () => { ); }); + it("creates a revocable connection anchor and links the access token (H2)", async () => { + await POST( + tokenReq({ + grant_type: "authorization_code", + code: authCode(), + code_verifier: VERIFIER, + client_id: CLIENT_ID, + redirect_uri: REDIRECT, + }) as never, + ); + expect(createConnection).toHaveBeenCalledTimes(1); + expect(issueApiToken).toHaveBeenCalledWith( + expect.objectContaining({ mcpConnectionId: "conn-1" }), + ); + }); + it("rejects a wrong PKCE verifier with invalid_grant", async () => { const res = await POST( tokenReq({ @@ -169,8 +216,61 @@ describe("authorization_code grant", () => { }); describe("refresh_token grant", () => { - it("rotates: issues a new access + refresh pair", async () => { - const refresh = signArtifact( + it("rotates: issues a new access + refresh pair against a live connection", async () => { + const res = await POST( + tokenReq({ + grant_type: "refresh_token", + refresh_token: refreshToken(), + client_id: CLIENT_ID, + }) as never, + ); + expect(res.status).toBe(200); + const body = await res.json(); + expect(body.access_token).toMatch(/^hlk_/); + expect(body.refresh_token).toMatch(/^hlrt_/); + expect(rotateConnection).toHaveBeenCalledWith( + expect.objectContaining({ connectionId: "conn-1", clientId: CLIENT_ID }), + ); + }); + + it("rejects a refresh whose connection was revoked (H2 — settings revoke)", async () => { + vi.mocked(rotateConnection).mockResolvedValue({ + ok: false, + reason: "revoked", + }); + const res = await POST( + tokenReq({ + grant_type: "refresh_token", + refresh_token: refreshToken(), + client_id: CLIENT_ID, + }) as never, + ); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("invalid_grant"); + expect(issueApiToken).not.toHaveBeenCalled(); + }); + + it("rejects a replayed refresh and reports reuse detection (H2)", async () => { + vi.mocked(rotateConnection).mockResolvedValue({ + ok: false, + reason: "reuse_detected", + }); + const res = await POST( + tokenReq({ + grant_type: "refresh_token", + refresh_token: refreshToken(), + client_id: CLIENT_ID, + }) as never, + ); + expect(res.status).toBe(400); + const body = await res.json(); + expect(body.error).toBe("invalid_grant"); + expect(body.error_description).toMatch(/reuse/i); + expect(issueApiToken).not.toHaveBeenCalled(); + }); + + it("rejects a legacy refresh token with no connection binding", async () => { + const legacy = signArtifact( "refreshToken", { jti: randomUUID(), @@ -184,14 +284,31 @@ describe("refresh_token grant", () => { const res = await POST( tokenReq({ grant_type: "refresh_token", - refresh_token: refresh, + refresh_token: legacy, client_id: CLIENT_ID, }) as never, ); - expect(res.status).toBe(200); - const body = await res.json(); - expect(body.access_token).toMatch(/^hlk_/); - expect(body.refresh_token).toMatch(/^hlrt_/); + expect(res.status).toBe(400); + expect((await res.json()).error).toBe("invalid_grant"); + expect(rotateConnection).not.toHaveBeenCalled(); + }); +}); + +describe("operator kill-switch + origin config (M1/M4)", () => { + it("refuses to mint when the API is globally disabled", async () => { + vi.mocked(isApiGloballyEnabled).mockResolvedValue(false); + const res = await POST( + tokenReq({ + grant_type: "authorization_code", + code: authCode(), + code_verifier: VERIFIER, + client_id: CLIENT_ID, + redirect_uri: REDIRECT, + }) as never, + ); + expect(res.status).toBe(503); + expect((await res.json()).error).toBe("temporarily_unavailable"); + expect(issueApiToken).not.toHaveBeenCalled(); }); }); diff --git a/src/app/api/mcp/oauth/authorize/route.ts b/src/app/api/mcp/oauth/authorize/route.ts index 068fe7974..70288aa07 100644 --- a/src/app/api/mcp/oauth/authorize/route.ts +++ b/src/app/api/mcp/oauth/authorize/route.ts @@ -31,9 +31,12 @@ import { withBackgroundEvent } from "@/lib/logging/background"; import { auditLog } from "@/lib/auth/audit"; import { getSession } from "@/lib/auth/session"; import { checkAuthSurfaceRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; import { audienceMatches, AUTH_CODE_TTL_SECONDS, + isMcpOriginConfigured, + resolveBaseOrigin, SCOPE_HEALTH_READ, } from "@/lib/mcp/oauth/config"; import { redirectUriAllowed, resolveClient } from "@/lib/mcp/oauth/clients"; @@ -96,6 +99,10 @@ function grantScope(requested: string | undefined): string { interface ValidatedRequest { clientId: string; clientName: string; + /** The VERIFIED client origin shown on the consent screen (L1). */ + clientOrigin: string; + /** Registration provenance — drives the trust label on consent (L1). */ + clientSource: "cimd" | "dcr"; redirectUri: string; codeChallenge: string; scope: string; @@ -174,11 +181,28 @@ async function validate( }; } + // L1 — the VERIFIED origin. For CIMD the `client_id` IS the document URL, so + // its host is cryptographically tied to the fetched-and-matched document; for + // DCR there is no verified origin (anyone can self-register), so the consent + // screen labels it unverified rather than trusting the self-asserted name. + const clientOrigin = + resolved.client.source === "cimd" + ? (() => { + try { + return new URL(p.client_id).host; + } catch { + return p.client_id; + } + })() + : ""; + return { ok: true, v: { clientId: p.client_id, clientName: resolved.client.clientName, + clientOrigin, + clientSource: resolved.client.source, redirectUri: p.redirect_uri, codeChallenge: p.code_challenge as string, scope: grantScope(p.scope), @@ -188,6 +212,20 @@ async function validate( }; } +/** + * L3 — same-origin assertion for the consent POST. A non-browser caller sends + * neither `Origin` nor `Sec-Fetch-Site`, so their absence is allowed; a present + * `Origin` that is not our own origin, or a `Sec-Fetch-Site` that is not + * `same-origin`, is refused. Mirrors the `/mcp` DNS-rebinding posture. + */ +function consentOriginAllowed(request: NextRequest): boolean { + const origin = request.headers.get("origin"); + if (origin && origin !== resolveBaseOrigin(request.url)) return false; + const site = request.headers.get("sec-fetch-site"); + if (site && site !== "same-origin") return false; + return true; +} + function redirectBack( redirectUri: string, query: Record, @@ -197,8 +235,19 @@ function redirectBack( return Response.redirect(url.toString(), 302); } +/** Whether the surface is available (M1 origin pinned + M4 kill-switch off). */ +async function surfaceAvailable(): Promise { + return isMcpOriginConfigured() && (await isApiGloballyEnabled()); +} + export async function GET(request: NextRequest): Promise { return withBackgroundEvent("mcp.oauth.authorize", async () => { + if (!(await surfaceAvailable())) { + return Response.json( + { error: "temporarily_unavailable" }, + { status: 503 }, + ); + } const rl = await checkAuthSurfaceRateLimit( request, "mcp:oauth:authorize", @@ -244,10 +293,17 @@ export async function GET(request: NextRequest): Promise { // Consent screen — a plain POST form (no inline script/style, CSP-safe). const hidden = (name: string, value: string) => ``; + // L1 — show the VERIFIED origin (or an explicit "unverified" label for DCR) + // next to the self-asserted name so the user authorizes who, not just what. + const provenance = + v.clientSource === "cimd" + ? `

Verified origin: ${htmlEscape(v.clientOrigin)}

` + : `

Unverified application (dynamically registered — the name above is self-reported and not verified).

`; return html( `

Authorize access

${htmlEscape(v.clientName)} is requesting access to your HealthLog data.

+ ${provenance}

Scope: ${htmlEscape(v.scope)} — read-only access to your own health records.

${hidden("response_type", "code")} @@ -268,6 +324,25 @@ export async function GET(request: NextRequest): Promise { export async function POST(request: NextRequest): Promise { return withBackgroundEvent("mcp.oauth.authorize", async () => { + if (!(await surfaceAvailable())) { + return Response.json( + { error: "temporarily_unavailable" }, + { status: 503 }, + ); + } + + // L3 — defence-in-depth same-origin assertion on the consent decision. The + // session cookie is already SameSite=Lax (a cross-site POST carries none), + // but reject an explicit cross-origin Origin / Sec-Fetch-Site outright so a + // forged "allow" cannot ride a present session. + if (!consentOriginAllowed(request)) { + annotate({ action: { name: "mcp.oauth.authorize.cross_origin" } }); + return Response.json( + { error: "access_denied", error_description: "Cross-origin POST" }, + { status: 403 }, + ); + } + const rl = await checkAuthSurfaceRateLimit( request, "mcp:oauth:authorize", @@ -341,6 +416,9 @@ export async function POST(request: NextRequest): Promise { jti: randomUUID(), sub: session.user.id, client_id: v.clientId, + // Carried so the token endpoint can label the persistent connection + // (H2) without re-resolving the client. + client_name: v.clientName, redirect_uri: v.redirectUri, code_challenge: v.codeChallenge, scope: v.scope, diff --git a/src/app/api/mcp/oauth/register/route.ts b/src/app/api/mcp/oauth/register/route.ts index 9e49e8192..513bf1b85 100644 --- a/src/app/api/mcp/oauth/register/route.ts +++ b/src/app/api/mcp/oauth/register/route.ts @@ -18,7 +18,12 @@ import { z } from "zod/v4"; import { annotate } from "@/lib/logging/context"; import { withBackgroundEvent } from "@/lib/logging/background"; import { checkAuthSurfaceRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; -import { registerDcrClient } from "@/lib/mcp/oauth/clients"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; +import { readBodyCapped } from "@/lib/http/read-capped"; +import { + isAllowableRedirectUri, + registerDcrClient, +} from "@/lib/mcp/oauth/clients"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -27,24 +32,6 @@ const REGISTER_LIMIT = 20; const REGISTER_WINDOW_MS = 60 * 60 * 1000; const MAX_BODY_BYTES = 16 * 1024; -/** A redirect URI must be an absolute HTTPS URL or an http loopback URL. */ -function isAllowableRedirectUri(value: string): boolean { - let url: URL; - try { - url = new URL(value); - } catch { - return false; - } - if (url.protocol === "https:") return true; - if ( - url.protocol === "http:" && - ["localhost", "127.0.0.1", "::1", "[::1]"].includes(url.hostname) - ) { - return true; - } - return false; -} - const registrationSchema = z.object({ redirect_uris: z .array(z.string().min(1).max(2048)) @@ -96,13 +83,20 @@ export async function POST(request: NextRequest): Promise { ); } - const raw = await request.text(); - if (raw.length > MAX_BODY_BYTES) { + // M4 — the operator API kill-switch covers the OAuth surface too. + if (!(await isApiGloballyEnabled())) { + return oauthError("temporarily_unavailable", "API is disabled", 503); + } + + // M3 — enforce the cap WHILE reading the (unauthenticated) registration + // body so a hostile client cannot buffer an oversized payload first. + const read = await readBodyCapped(request, MAX_BODY_BYTES); + if (!read.ok) { return oauthError("invalid_client_metadata", "Payload too large", 400); } let body: unknown; try { - body = JSON.parse(raw); + body = JSON.parse(read.text); } catch { return oauthError("invalid_client_metadata", "Invalid JSON", 400); } diff --git a/src/app/api/mcp/oauth/token/route.ts b/src/app/api/mcp/oauth/token/route.ts index 0d8d628dd..749d2f917 100644 --- a/src/app/api/mcp/oauth/token/route.ts +++ b/src/app/api/mcp/oauth/token/route.ts @@ -30,6 +30,7 @@ import { annotate } from "@/lib/logging/context"; import { withBackgroundEvent } from "@/lib/logging/background"; import { auditLog } from "@/lib/auth/audit"; import { issueApiToken } from "@/lib/auth/issue-token"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkRateLimit, checkAuthSurfaceRateLimit, @@ -38,11 +39,16 @@ import { import { ACCESS_TOKEN_TTL_MINUTES, audienceMatches, + isMcpOriginConfigured, REFRESH_TOKEN_TTL_DAYS, SCOPE_HEALTH_READ, } from "@/lib/mcp/oauth/config"; import { signArtifact, verifyArtifact } from "@/lib/mcp/oauth/artifacts"; import { verifyPkceS256 } from "@/lib/mcp/oauth/pkce"; +import { + createConnection, + rotateConnection, +} from "@/lib/mcp/oauth/connections"; export const runtime = "nodejs"; export const dynamic = "force-dynamic"; @@ -82,6 +88,7 @@ interface AuthCodeClaims { jti: string; sub: string; client_id: string; + client_name?: string; redirect_uri: string; code_challenge: string; scope: string; @@ -90,40 +97,56 @@ interface AuthCodeClaims { interface RefreshClaims { jti: string; + /** Connection id — the persistent, revocable anchor for the chain (H2). */ + cid?: string; sub: string; client_id: string; scope: string; resource: string; } +/** + * Mint the access token (+ refresh artifact when `offline_access` is granted). + * + * `connectionId` is the persistent anchor for the refresh chain (H2): the access + * token is linked to it (so a connection revoke kills it) and the refresh + * artifact carries it (`cid`). `refreshJti` is the connection's new + * `currentJti` — for the `authorization_code` grant the caller has just created + * the connection seeded with it; for `refresh_token` the caller has just + * advanced the connection to it. + */ async function mintTokenPair(args: { userId: string; clientId: string; scope: string; resource: string; grant: "authorization_code" | "refresh_token"; + connectionId?: string; + refreshJti?: string; }): Promise { const access = await issueApiToken({ userId: args.userId, name: "MCP connector", permissions: [SCOPE_HEALTH_READ], expiresInMinutes: ACCESS_TOKEN_TTL_MINUTES, + ...(args.connectionId ? { mcpConnectionId: args.connectionId } : {}), }); - const includeRefresh = args.scope.split(/\s+/).includes("offline_access"); - const refresh = includeRefresh - ? signArtifact( - "refreshToken", - { - jti: randomUUID(), - sub: args.userId, - client_id: args.clientId, - scope: args.scope, - resource: args.resource, - }, - REFRESH_TOKEN_TTL_DAYS * 24 * 60 * 60 * 1000, - ) - : undefined; + const refresh = + args.connectionId && args.refreshJti + ? signArtifact( + "refreshToken", + { + jti: args.refreshJti, + cid: args.connectionId, + sub: args.userId, + client_id: args.clientId, + scope: args.scope, + resource: args.resource, + }, + REFRESH_TOKEN_TTL_DAYS * 24 * 60 * 60 * 1000, + ) + : undefined; await auditLog("mcp.oauth.token.issued", { userId: args.userId, @@ -145,6 +168,16 @@ async function mintTokenPair(args: { export async function POST(request: NextRequest): Promise { return withBackgroundEvent("mcp.oauth.token", async () => { + // M1 — fail closed without a pinned origin; M4 — honour the operator's + // global API kill-switch. Either off and the bridge mints no tokens. + if (!isMcpOriginConfigured() || !(await isApiGloballyEnabled())) { + return oauthError( + "temporarily_unavailable", + "The MCP OAuth surface is unavailable", + 503, + ); + } + const rl = await checkAuthSurfaceRateLimit( request, "mcp:oauth:token", @@ -242,12 +275,34 @@ export async function POST(request: NextRequest): Promise { ); } + // H2 — when offline_access is granted, create the persistent, revocable + // connection anchor seeded with the first refresh jti. Without it the + // refresh chain would be stateless and un-revocable. + const offlineGranted = claims.scope + .split(/\s+/) + .includes("offline_access"); + let connectionId: string | undefined; + let refreshJti: string | undefined; + if (offlineGranted) { + refreshJti = randomUUID(); + connectionId = await createConnection({ + userId: claims.sub, + clientId: claims.client_id, + clientName: claims.client_name ?? "MCP client", + scope: claims.scope, + resource: claims.resource, + jti: refreshJti, + }); + } + return mintTokenPair({ userId: claims.sub, clientId: claims.client_id, scope: claims.scope, resource: claims.resource, grant: "authorization_code", + connectionId, + refreshJti, }); } @@ -284,18 +339,41 @@ export async function POST(request: NextRequest): Promise { "Refresh token is bound to a different audience", ); } - // Rotation: claim the presented refresh jti so it cannot be reused. - if ( - !(await claimJti( - "refresh", - claims.jti, - REFRESH_TOKEN_TTL_DAYS * 24 * 60 * 60 * 1000, - )) - ) { - annotate({ action: { name: "mcp.oauth.token.refresh_replayed" } }); + + // H2 — rotation against the persistent connection anchor. The connection's + // `currentJti` is the single valid refresh jti; presenting any other (an + // already-rotated artifact) is a replay that revokes the whole connection + // (reuse-detection family revocation). A revoked connection always fails, + // so the settings "revoke" terminates the chain. + if (!claims.cid) { return oauthError( "invalid_grant", - "Refresh token has already been used", + "Refresh token is not bound to a connection", + ); + } + const newJti = randomUUID(); + const rotation = await rotateConnection({ + connectionId: claims.cid, + presentedJti: claims.jti, + newJti, + clientId: claims.client_id, + userId: claims.sub, + }); + if (!rotation.ok) { + annotate({ + action: { + name: + rotation.reason === "reuse_detected" + ? "mcp.oauth.token.refresh_reuse_detected" + : "mcp.oauth.token.refresh_rejected", + }, + meta: { reason: rotation.reason }, + }); + return oauthError( + "invalid_grant", + rotation.reason === "reuse_detected" + ? "Refresh token reuse detected; the connection was revoked" + : "Refresh token is invalid, revoked, or already used", ); } @@ -305,6 +383,8 @@ export async function POST(request: NextRequest): Promise { scope: claims.scope, resource: claims.resource, grant: "refresh_token", + connectionId: claims.cid, + refreshJti: newJti, }); } diff --git a/src/app/api/mcp/tokens/route.ts b/src/app/api/mcp/tokens/route.ts index c6f6542f9..5140a001d 100644 --- a/src/app/api/mcp/tokens/route.ts +++ b/src/app/api/mcp/tokens/route.ts @@ -42,8 +42,15 @@ export const GET = apiHandler(async () => { return apiError("API is globally disabled", 403); } + // M2 — list only manually-minted connector tokens. OAuth access rows + // (`mcpConnectionId != null`) are transient 60-minute credentials that would + // flood this list; they are surfaced (and revoked) as connections instead. const tokens = await prisma.apiToken.findMany({ - where: { userId: user.id, permissions: { has: SCOPE_HEALTH_READ } }, + where: { + userId: user.id, + permissions: { has: SCOPE_HEALTH_READ }, + mcpConnectionId: null, + }, select: { id: true, name: true, diff --git a/src/app/mcp/__tests__/route.test.ts b/src/app/mcp/__tests__/route.test.ts index b838cd9a8..e9115ba4b 100644 --- a/src/app/mcp/__tests__/route.test.ts +++ b/src/app/mcp/__tests__/route.test.ts @@ -21,7 +21,13 @@ */ import { describe, it, expect, vi, beforeEach } from "vitest"; +// The surface fails closed without a pinned origin (M1); pin it for the suite. +process.env.APP_URL = "http://localhost"; + vi.mock("@/lib/logging/transports", () => ({ emitIfSampled: vi.fn() })); +vi.mock("@/lib/app-settings", () => ({ + isApiGloballyEnabled: vi.fn(async () => true), +})); vi.mock("@/lib/auth/bearer", () => ({ resolveBearerToken: vi.fn(), BearerAuthError: class BearerAuthError extends Error {}, @@ -60,6 +66,7 @@ import { POST, GET } from "../route"; import { resolveBearerToken } from "@/lib/auth/bearer"; import { isModuleEnabled } from "@/lib/modules/gate"; import { checkMcpRateLimit } from "@/lib/rate-limit"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; const READ_TOOLS = [ "get_correlations", @@ -102,6 +109,7 @@ beforeEach(() => { headersGet.mockReturnValue(null); // Default happy-path: module on, under budget. vi.mocked(isModuleEnabled).mockResolvedValue(true); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(true); vi.mocked(checkMcpRateLimit).mockResolvedValue({ allowed: true, remaining: 119, @@ -174,6 +182,17 @@ describe("/mcp — Origin gate (DNS-rebinding defense, spec PR #1439)", () => { }); }); +describe("/mcp — operator kill-switch (M4)", () => { + it("hides the surface (404) when the API is globally disabled", async () => { + validToken(); + vi.mocked(isApiGloballyEnabled).mockResolvedValue(false); + const res = await POST(postRpc(TOOLS_LIST)); + expect(res.status).toBe(404); + // Fails closed before the token table is consulted. + expect(resolveBearerToken).not.toHaveBeenCalled(); + }); +}); + describe("/mcp — 401 carries the RFC 9728 resource_metadata pointer", () => { it("includes resource_metadata in the WWW-Authenticate challenge", async () => { const res = await POST( diff --git a/src/app/mcp/route.ts b/src/app/mcp/route.ts index 792182441..80bf1a323 100644 --- a/src/app/mcp/route.ts +++ b/src/app/mcp/route.ts @@ -46,8 +46,12 @@ import { createMcpServer, resolveMcpAuthContext } from "@/lib/mcp"; import { withBackgroundEvent } from "@/lib/logging/background"; import { annotate } from "@/lib/logging/context"; import { isModuleEnabled } from "@/lib/modules/gate"; +import { isApiGloballyEnabled } from "@/lib/app-settings"; import { checkMcpRateLimit, rateLimitHeaders } from "@/lib/rate-limit"; -import { resolveBaseOrigin } from "@/lib/mcp/oauth/config"; +import { + isMcpOriginConfigured, + resolveBaseOrigin, +} from "@/lib/mcp/oauth/config"; import { wwwAuthenticateChallenge } from "@/lib/mcp/oauth/metadata"; import type { ModuleKey } from "@/lib/modules/registry"; @@ -131,6 +135,15 @@ async function handleMcp(request: Request): Promise { return forbiddenOrigin(); } + // 0a. KILL-SWITCH + ORIGIN CONFIG — M1 fails closed without a pinned + // origin; M4 honours the operator's global API switch. Both answer the + // same invisible 404 as the module-off posture so a disabled instance + // leaks nothing. + if (!isMcpOriginConfigured() || !(await isApiGloballyEnabled())) { + annotate({ action: { name: "mcp.surface.unavailable" } }); + return moduleDisabled(); + } + // 1. AUTH — Bearer only; a cookie is never consulted (admin unreachable). const raw = readBearer(request); if (!raw) { diff --git a/src/components/settings/mcp-section.tsx b/src/components/settings/mcp-section.tsx index f645b398a..fdb8f58da 100644 --- a/src/components/settings/mcp-section.tsx +++ b/src/components/settings/mcp-section.tsx @@ -18,7 +18,7 @@ */ import { useState } from "react"; import { useMutation, useQuery, useQueryClient } from "@tanstack/react-query"; -import { Check, Copy, Key, Loader2, Plug, Trash2 } from "lucide-react"; +import { Check, Copy, Key, Link2, Loader2, Plug, Trash2 } from "lucide-react"; import { toast } from "sonner"; import { @@ -54,15 +54,136 @@ interface McpTokenInfo { revoked: boolean; } +interface McpConnectionInfo { + id: string; + clientName: string; + scope: string; + createdAt: string; + lastUsedAt: string | null; +} + export function McpSection() { return (
+
); } +function McpConnectionsCard() { + const { t } = useTranslations(); + const { isAuthenticated } = useAuth(); + const queryClient = useQueryClient(); + + const { data: connections } = useQuery({ + queryKey: queryKeys.mcpConnections(), + queryFn: async () => apiGet("/api/mcp/connections"), + enabled: isAuthenticated, + }); + + async function handleRevoke(connectionId: string) { + try { + const res = await apiFetchRaw(`/api/mcp/connections/${connectionId}`, { + method: "DELETE", + }); + if (!res.ok) { + toast.error(t("settings.mcp.connectionRevokeFailed")); + return; + } + queryClient.invalidateQueries({ queryKey: queryKeys.mcpConnections() }); + } catch { + toast.error(t("settings.mcp.connectionRevokeFailed")); + } + } + + const list = connections ?? []; + + return ( + + + +
+ {list.length === 0 ? ( +

+ {t("settings.mcp.noConnections")} +

+ ) : ( +
    + {list.map((conn) => ( +
  • +
    +

    + {conn.clientName} +

    + + {t("settings.mcp.connectionConnected")} + +
    +

    + + {t("settings.tokenTablePermissions")}: + {" "} + {conn.scope} +

    +

    + + {t("settings.mcp.connectionLastUsed")}: + {" "} + {conn.lastUsedAt + ? formatDateTime(conn.lastUsedAt) + : t("settings.mcp.connectionNeverUsed")} +

    + + + + + + + + {t("settings.mcp.connectionRevoke")} + + + {t("settings.mcp.connectionRevokeDescription")} + + + + + {t("common.cancel")} + + handleRevoke(conn.id)} + > + {t("settings.mcp.connectionRevoke")} + + + + +
    • + ))} +
+ )} +
+ + ); +} + function McpEnableCard() { const { t } = useTranslations(); const { user } = useAuth(); diff --git a/src/lib/__tests__/require-auth-bearer.test.ts b/src/lib/__tests__/require-auth-bearer.test.ts index a3c248933..f1688bba0 100644 --- a/src/lib/__tests__/require-auth-bearer.test.ts +++ b/src/lib/__tests__/require-auth-bearer.test.ts @@ -42,7 +42,7 @@ vi.mock("next/headers", () => ({ // --- Imports use the mocked modules above. --- -import { requireAuth, HttpError } from "../api-handler"; +import { requireAuth, HttpError, apiHandler } from "../api-handler"; import { prisma } from "@/lib/db"; import { getSession } from "@/lib/auth/session"; import { hashToken } from "@/lib/auth/hmac"; @@ -270,6 +270,89 @@ describe("requireAuth — Bearer token path", () => { }); }); +describe("requireAuth — H1 MCP health:read audience binding on REST", () => { + // Drive the FULL apiHandler path so the request method is in event context — + // the H1 check reads it to allow safe reads but block writes. + const route = apiHandler(async (_req: Request) => { + await requireAuth(); + return Response.json({ ok: true }); + }); + + function call(method: string): Promise { + setBearerHeader(`Bearer ${RAW_TOKEN}`); + return route( + new Request("https://health.example/api/measurements/batch", { + method, + }) as never, + ); + } + + beforeEach(() => { + vi.mocked(getSession).mockResolvedValue(null); + vi.mocked(prisma.user.findUnique).mockResolvedValue(FAKE_USER as never); + }); + + function mockHealthReadToken(): void { + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "mcp-token", + userId: "user-1", + permissions: ["health:read"], + revoked: false, + expiresAt: null, + } as never); + } + + it("accepts a health:read token on a GET read route", async () => { + mockHealthReadToken(); + const res = await call("GET"); + expect(res.status).toBe(200); + }); + + it("rejects a health:read token on POST with 403", async () => { + mockHealthReadToken(); + const res = await call("POST"); + expect(res.status).toBe(403); + expect(auditLog).toHaveBeenCalledWith( + "auth.bearer.failure", + expect.objectContaining({ + details: expect.objectContaining({ + reason: "mcp_audience_write_blocked", + }), + }), + ); + }); + + it("rejects a health:read token on DELETE with 403", async () => { + mockHealthReadToken(); + const res = await call("DELETE"); + expect(res.status).toBe(403); + }); + + it("does NOT restrict a wildcard token on POST (not MCP-audience)", async () => { + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "wild", + userId: "user-1", + permissions: ["*"], + revoked: false, + expiresAt: null, + } as never); + const res = await call("POST"); + expect(res.status).toBe(200); + }); + + it("does NOT restrict a health:read+write token on POST", async () => { + vi.mocked(prisma.apiToken.findUnique).mockResolvedValue({ + id: "rw", + userId: "user-1", + permissions: ["health:read", "health:write"], + revoked: false, + expiresAt: null, + } as never); + const res = await call("POST"); + expect(res.status).toBe(200); + }); +}); + describe("requireAuth — cookie path remains intact", () => { it("returns the session payload without consulting the Bearer header", async () => { const cookieSession = { diff --git a/src/lib/api-handler.ts b/src/lib/api-handler.ts index ba78ad92f..0e59c1c84 100644 --- a/src/lib/api-handler.ts +++ b/src/lib/api-handler.ts @@ -10,6 +10,27 @@ import { auditLog } from "./auth/audit"; import { resolveBearerToken, BearerAuthError } from "./auth/bearer"; import { AssistantDisabledError } from "./feature-flags"; import { ConsentRequiredError } from "./ai/consent-guard"; +import { SCOPE_HEALTH_READ } from "./mcp/oauth/config"; + +/** + * HTTP methods a read-only credential may use on the REST surface. A request + * with any other method (POST / PUT / PATCH / DELETE) is a write. + */ +const READ_HTTP_METHODS = new Set(["GET", "HEAD", "OPTIONS"]); + +/** + * Whether a token is MCP-audience-bound (H1). The MCP OAuth bridge and the + * connector settings card mint tokens whose ONLY grant is `health:read`. Such a + * token is bound to the read surface: the `/mcp` resolver accepts it, and so do + * safe (read) REST methods, but it must never reach a REST write/delete. A token + * carrying any broader or legacy grant (`*`, `medication:ingest`, an explicit + * `health:write`, …) is NOT MCP-audience-bound and keeps its existing reach. + */ +function isMcpAudienceToken(permissions: readonly string[]): boolean { + return ( + permissions.length > 0 && permissions.every((p) => p === SCOPE_HEALTH_READ) + ); +} /** * Custom error class for HTTP errors with status codes. @@ -331,7 +352,30 @@ async function authenticateBearer( throw err; } - const { user, tokenId, expiresAt } = resolution; + const { user, tokenId, expiresAt, permissions } = resolution; + + // H1 — audience binding at the resource server. A health:read-only token is + // the MCP read credential; it may reach `/mcp` (a separate resolver that + // never runs this edge) and safe REST reads, but a write/delete over REST is + // outside its audience and is refused. Fail closed when the method is unknown + // (no event context) since every real REST request runs inside apiHandler, + // which always sets the method — an unknown method means we cannot prove a + // read, so we deny. This is RFC 8707 audience binding on the credential the + // client actually holds, not only during the OAuth exchange. + if (isMcpAudienceToken(permissions)) { + const method = (getEvent()?.getHttpMethod() ?? "").toUpperCase(); + if (!READ_HTTP_METHODS.has(method)) { + auditLog("auth.bearer.failure", { + userId: user.id, + details: { + reason: "mcp_audience_write_blocked", + tokenId, + method: method || "unknown", + }, + }).catch(() => {}); + throw new HttpError(403, "Insufficient permissions"); + } + } auditLog("auth.bearer.success", { userId: user.id, diff --git a/src/lib/auth/issue-token.ts b/src/lib/auth/issue-token.ts index bf4375bf2..291b34342 100644 --- a/src/lib/auth/issue-token.ts +++ b/src/lib/auth/issue-token.ts @@ -29,6 +29,12 @@ export interface IssueTokenOptions { * carries continuity). Ignored when `expiresInDays` alone is supplied. */ expiresInMinutes?: number; + /** + * Links this access token to an MCP OAuth connection (H2). Set only by the + * `/api/mcp/oauth` bridge so a connection revoke can kill every access row it + * issued and the connector token list can exclude the transient access rows. + */ + mcpConnectionId?: string; } /** @@ -53,6 +59,9 @@ export async function issueApiToken( tokenHash: tokenHashValue, permissions: opts.permissions ?? ["*"], expiresAt, + ...(opts.mcpConnectionId + ? { mcpConnectionId: opts.mcpConnectionId } + : {}), }, select: { id: true }, }); diff --git a/src/lib/http/__tests__/read-capped.test.ts b/src/lib/http/__tests__/read-capped.test.ts new file mode 100644 index 000000000..97d0b525f --- /dev/null +++ b/src/lib/http/__tests__/read-capped.test.ts @@ -0,0 +1,68 @@ +/** + * Bounded body reader (M3) — reject oversized bodies WITHOUT buffering them. + */ +import { describe, it, expect } from "vitest"; + +import { readBodyCapped } from "../read-capped"; + +function streamOf(chunks: Uint8Array[]): ReadableStream { + let i = 0; + return new ReadableStream({ + pull(controller) { + if (i < chunks.length) controller.enqueue(chunks[i++]); + else controller.close(); + }, + }); +} + +const enc = (s: string) => new TextEncoder().encode(s); + +describe("readBodyCapped", () => { + it("returns the decoded text under the cap", async () => { + const res = await readBodyCapped( + { headers: new Headers(), body: streamOf([enc("hello "), enc("world")]) }, + 1024, + ); + expect(res).toEqual({ ok: true, text: "hello world" }); + }); + + it("rejects up front on an oversized Content-Length without consuming the body", async () => { + const body = streamOf([enc("x")]); + let getReaderCalled = false; + const original = body.getReader.bind(body); + body.getReader = ((...args: unknown[]) => { + getReaderCalled = true; + // @ts-expect-error — passthrough spy + return original(...args); + }) as typeof body.getReader; + const res = await readBodyCapped( + { headers: new Headers({ "content-length": "999999" }), body }, + 16, + ); + expect(res).toEqual({ ok: false, reason: "too_large" }); + expect(getReaderCalled).toBe(false); + }); + + it("aborts mid-stream the moment the running total exceeds the cap", async () => { + let chunksPulled = 0; + const body = new ReadableStream({ + pull(c) { + chunksPulled++; + // 10 bytes per chunk; cap is 16, so the 2nd chunk overflows. + c.enqueue(enc("0123456789")); + }, + }); + const res = await readBodyCapped({ headers: new Headers(), body }, 16); + expect(res).toEqual({ ok: false, reason: "too_large" }); + // It must stop pulling once it overflows, not drain the (infinite) stream. + expect(chunksPulled).toBeLessThanOrEqual(2); + }); + + it("treats a null body as empty", async () => { + const res = await readBodyCapped( + { headers: new Headers(), body: null }, + 16, + ); + expect(res).toEqual({ ok: true, text: "" }); + }); +}); diff --git a/src/lib/http/read-capped.ts b/src/lib/http/read-capped.ts new file mode 100644 index 000000000..9db36911a --- /dev/null +++ b/src/lib/http/read-capped.ts @@ -0,0 +1,62 @@ +/** + * Bounded body reader (M3). + * + * `await res.text()` / `await request.text()` buffer the WHOLE body before any + * size check runs, so a hostile peer can stream (or decompress) gigabytes into + * memory for the duration of the request timeout before a post-hoc + * `raw.length > MAX` rejection fires. This helper enforces the cap WHILE + * reading: it rejects up front on an oversized `Content-Length`, then streams + * the body and aborts the moment the running byte count exceeds the ceiling. + * + * Returns `{ ok: false }` on overflow (the caller maps it to its own error); + * returns the decoded UTF-8 text on success. + */ +export type CappedReadResult = + | { ok: true; text: string } + | { ok: false; reason: "too_large" }; + +export async function readBodyCapped( + source: { headers: Headers; body: ReadableStream | null }, + maxBytes: number, +): Promise { + // Up-front reject on a declared oversized length — cheap and stops the read + // before a single byte is buffered. + const declared = source.headers.get("content-length"); + if (declared) { + const n = Number(declared); + if (Number.isFinite(n) && n > maxBytes) { + return { ok: false, reason: "too_large" }; + } + } + + const body = source.body; + if (!body) return { ok: true, text: "" }; + + const reader = body.getReader(); + const chunks: Uint8Array[] = []; + let total = 0; + try { + for (;;) { + const { done, value } = await reader.read(); + if (done) break; + if (!value) continue; + total += value.byteLength; + if (total > maxBytes) { + // Stop pulling immediately — do not buffer past the ceiling. + await reader.cancel().catch(() => {}); + return { ok: false, reason: "too_large" }; + } + chunks.push(value); + } + } finally { + reader.releaseLock?.(); + } + + const merged = new Uint8Array(total); + let offset = 0; + for (const chunk of chunks) { + merged.set(chunk, offset); + offset += chunk.byteLength; + } + return { ok: true, text: new TextDecoder("utf-8").decode(merged) }; +} diff --git a/src/lib/jobs/__tests__/mcp-token-cleanup-queue.test.ts b/src/lib/jobs/__tests__/mcp-token-cleanup-queue.test.ts new file mode 100644 index 000000000..6215ec22f --- /dev/null +++ b/src/lib/jobs/__tests__/mcp-token-cleanup-queue.test.ts @@ -0,0 +1,58 @@ +/** + * MCP Phase 3 (M2) — mcp-token-cleanup queue registration guard. + * + * Source-text-grep guard like the other queue-wiring checks: assert the queue + * is in `allQueues`, scheduled, and wired to a `boss.work` handler. An + * unregistered queue silently never drains (the recurring v1.4.37 dead-queue + * bug), which would leave expired MCP connector access tokens growing unbounded. + */ +import { readFileSync } from "node:fs"; +import { join } from "node:path"; + +import { describe, expect, it } from "vitest"; + +const REGISTRAR_PATH = join( + __dirname, + "..", + "reminder", + "register-maintenance.ts", +); +const workerSource = + readFileSync(REGISTRAR_PATH, "utf8") + + readFileSync( + join(__dirname, "..", "reminder", "cleanup-handlers.ts"), + "utf8", + ); + +describe("reminder-worker — mcp-token-cleanup wiring", () => { + it("imports the cleanup handler from the cleanup-handlers module", () => { + expect(workerSource).toMatch(/\bhandleMcpTokenCleanup\b/); + expect(workerSource).toMatch(/\bMcpTokenCleanupPayload\b/); + }); + + it("registers the mcp-token-cleanup queue in the allQueues loop", () => { + const allQueuesMatch = workerSource.match( + /const allQueues\s*=\s*\[([\s\S]*?)\];/, + ); + expect(allQueuesMatch).not.toBeNull(); + expect(allQueuesMatch![1]).toMatch(/\bMCP_TOKEN_CLEANUP_QUEUE\b/); + }); + + it("schedules the mcp-token-cleanup cron", () => { + expect(workerSource).toMatch( + /\[MCP_TOKEN_CLEANUP_QUEUE,\s*MCP_TOKEN_CLEANUP_CRON\]/, + ); + }); + + it("registers a boss.work handler for the queue", () => { + expect(workerSource).toMatch( + /boss\.work[\s\S]{0,200}MCP_TOKEN_CLEANUP_QUEUE[\s\S]{0,200}handleMcpTokenCleanup/, + ); + }); + + it("prunes MCP tokens inside the handler", () => { + expect(workerSource).toMatch( + /handleMcpTokenCleanup[\s\S]{0,400}cleanupExpiredMcpTokens/, + ); + }); +}); diff --git a/src/lib/jobs/mcp-token-cleanup.ts b/src/lib/jobs/mcp-token-cleanup.ts new file mode 100644 index 000000000..d5d5869a9 --- /dev/null +++ b/src/lib/jobs/mcp-token-cleanup.ts @@ -0,0 +1,55 @@ +/** + * Daily cleanup for the MCP OAuth surface (M2). + * + * Every `authorization_code` exchange and every hourly refresh mints a fresh + * 60-minute `hlk_` access-token row through the bridge. Nothing deleted the + * expired/revoked rows, so a single connection refreshing hourly left ~720 dead + * rows per month, forever — unbounded table growth, and (before the connection + * model) a flooded connector list. + * + * This sweep hard-deletes MCP-connector access tokens that are past a short + * grace window after expiry, or revoked. It is scoped to `health:read`-only + * connector rows so it never touches native-client or wildcard tokens. Long- + * revoked connection anchors are pruned too once no access token references + * them, keeping the `mcp_oauth_connections` table bounded. + */ +import type { PrismaClient } from "@/generated/prisma/client"; +import { SCOPE_HEALTH_READ } from "@/lib/mcp/oauth/config"; + +/** Grace window kept after expiry before a dead access row is deleted. */ +const ACCESS_TOKEN_GRACE_MS = 24 * 60 * 60 * 1000; +/** Revoked connections older than this are pruned (kept briefly for audit). */ +const CONNECTION_PRUNE_MS = 30 * 24 * 60 * 60 * 1000; + +export interface McpTokenCleanupResult { + accessTokensDeleted: number; + connectionsDeleted: number; +} + +export async function cleanupExpiredMcpTokens( + prisma: PrismaClient, +): Promise { + const now = Date.now(); + const expiryCutoff = new Date(now - ACCESS_TOKEN_GRACE_MS); + const connectionCutoff = new Date(now - CONNECTION_PRUNE_MS); + + // Connector access tokens only: permissions is exactly `["health:read"]`. + const access = await prisma.apiToken.deleteMany({ + where: { + permissions: { equals: [SCOPE_HEALTH_READ] }, + OR: [{ revoked: true }, { expiresAt: { lt: expiryCutoff } }], + }, + }); + + // Prune long-revoked connection anchors. SetNull on the access-token FK means + // any lingering linked rows simply lose the pointer; the predicate keeps the + // window generous so an in-flight rotation never races a delete. + const connections = await prisma.mcpOAuthConnection.deleteMany({ + where: { revokedAt: { lt: connectionCutoff } }, + }); + + return { + accessTokensDeleted: access.count, + connectionsDeleted: connections.count, + }; +} diff --git a/src/lib/jobs/reminder/cleanup-handlers.ts b/src/lib/jobs/reminder/cleanup-handlers.ts index 9ae4568b3..e6530188b 100644 --- a/src/lib/jobs/reminder/cleanup-handlers.ts +++ b/src/lib/jobs/reminder/cleanup-handlers.ts @@ -11,6 +11,7 @@ import { cleanupExpiredWhoopOAuthStates, } from "@/lib/jobs/whoop-oauth-state-cleanup"; import { cleanupExpiredIdempotencyKeys } from "@/lib/jobs/idempotency-cleanup"; +import { cleanupExpiredMcpTokens } from "@/lib/jobs/mcp-token-cleanup"; import { cleanupOldAuditLogs } from "@/lib/jobs/audit-log-cleanup"; import { cleanupOldCoachMessages } from "@/lib/jobs/coach-message-cleanup"; import { cleanupExpiredWithingsOAuthStates } from "@/lib/jobs/withings-oauth-state-cleanup"; @@ -221,6 +222,32 @@ export async function handleCoachMessageCleanup( }); } +export interface McpTokenCleanupPayload { + triggeredAt: string; +} + +export async function handleMcpTokenCleanup( + jobs: Job[], +) { + void jobs; + await withBackgroundEvent("job.mcp_token_cleanup", async (evt) => { + const p = getWorkerPrisma(); + try { + const result = await cleanupExpiredMcpTokens(p); + evt.addMeta( + "mcp_token_cleanup_access_deleted", + result.accessTokensDeleted, + ); + evt.addMeta( + "mcp_token_cleanup_connections_deleted", + result.connectionsDeleted, + ); + } catch (err) { + evt.addWarning(`mcp-token-cleanup failed: ${err}`); + } + }); +} + export async function handleWithingsOAuthStateCleanup( jobs: Job[], ) { diff --git a/src/lib/jobs/reminder/register-maintenance.ts b/src/lib/jobs/reminder/register-maintenance.ts index e003d1d2a..ba765c5ea 100644 --- a/src/lib/jobs/reminder/register-maintenance.ts +++ b/src/lib/jobs/reminder/register-maintenance.ts @@ -79,6 +79,8 @@ import { handleAuditLogCleanup, CoachMessageCleanupPayload, handleCoachMessageCleanup, + McpTokenCleanupPayload, + handleMcpTokenCleanup, } from "./cleanup-handlers"; import { HostMetricSamplePayload, @@ -168,6 +170,14 @@ const MEASUREMENT_TOMBSTONE_CLEANUP_CRON = "40 3 * * *"; const COACH_MESSAGE_CLEANUP_QUEUE = "coach-message-cleanup"; const COACH_MESSAGE_CLEANUP_CRON = "50 3 * * *"; +// MCP Phase 3 (M2) — daily prune of expired/revoked MCP connector access +// tokens (every code exchange + hourly refresh mints a 60-minute row) and +// long-revoked OAuth connection anchors. Slots at 03:55 Europe/Berlin after +// the coach-message cleanup (03:50), inside the existing 03:xx window. + +const MCP_TOKEN_CLEANUP_QUEUE = "mcp-token-cleanup"; + +const MCP_TOKEN_CLEANUP_CRON = "55 3 * * *"; const allQueues = [ DATA_BACKUP_QUEUE, @@ -209,6 +219,10 @@ const allQueues = [ // the daily schedule silently no-ops and the encrypted coach_messages // table grows unbounded. COACH_MESSAGE_CLEANUP_QUEUE, + // MCP Phase 3 (M2) — expired/revoked MCP access-token + connection prune. + // Without this entry the daily schedule silently no-ops and the api_tokens + // table grows unbounded with dead 60-minute connector rows. + MCP_TOKEN_CLEANUP_QUEUE, ]; const schedules: ScheduleEntry[] = [ @@ -260,6 +274,8 @@ const schedules: ScheduleEntry[] = [ [MEASUREMENT_TOMBSTONE_CLEANUP_QUEUE, MEASUREMENT_TOMBSTONE_CLEANUP_CRON], // v1.18.7 — daily 03:50 Europe/Berlin prune for stale Coach history. [COACH_MESSAGE_CLEANUP_QUEUE, COACH_MESSAGE_CLEANUP_CRON], + // MCP Phase 3 (M2) — daily 03:55 Europe/Berlin prune for dead MCP tokens. + [MCP_TOKEN_CLEANUP_QUEUE, MCP_TOKEN_CLEANUP_CRON], ]; /** @@ -349,6 +365,13 @@ export async function registerMaintenanceQueues( { localConcurrency: 1 }, handleCoachMessageCleanup, ); + // MCP Phase 3 (M2) — daily prune of dead MCP connector access tokens + + // long-revoked connection anchors. Single-flight like every other cleanup. + await boss.work( + MCP_TOKEN_CLEANUP_QUEUE, + { localConcurrency: 1 }, + handleMcpTokenCleanup, + ); await boss.work( PR_DETECTION_QUEUE, { localConcurrency: PR_DETECTION_CONCURRENCY }, diff --git a/src/lib/logging/event-builder.ts b/src/lib/logging/event-builder.ts index 1b6204d62..26bb8b309 100644 --- a/src/lib/logging/event-builder.ts +++ b/src/lib/logging/event-builder.ts @@ -164,6 +164,11 @@ export class WideEventBuilder { return this.highestLevel; } + /** The HTTP method of the current request, if one was attached. */ + getHttpMethod(): string | undefined { + return this.event.http?.method; + } + getRequestId(): string { return this.event.request_id!; } diff --git a/src/lib/mcp/oauth/__tests__/connections.test.ts b/src/lib/mcp/oauth/__tests__/connections.test.ts new file mode 100644 index 000000000..c9223fa9d --- /dev/null +++ b/src/lib/mcp/oauth/__tests__/connections.test.ts @@ -0,0 +1,195 @@ +/** + * MCP OAuth connection anchor — H2 revocation + reuse-detection. + * + * These pin the security contract the stateless refresh artifact alone cannot + * provide: a revoked connection stops every future refresh, a replayed + * (already-rotated) jti revokes the whole connection, and a settings revoke + * terminates the chain and kills the connection's access tokens. + */ +import { describe, it, expect, vi, beforeEach } from "vitest"; + +vi.mock("@/lib/db", () => ({ + prisma: { + mcpOAuthConnection: { + create: vi.fn(), + findUnique: vi.fn(), + updateMany: vi.fn(), + findMany: vi.fn(), + }, + apiToken: { + updateMany: vi.fn(), + }, + }, +})); + +import { prisma } from "@/lib/db"; +import { + createConnection, + rotateConnection, + revokeConnectionForUser, +} from "../connections"; + +const CONN = { + id: "conn-1", + userId: "user-1", + clientId: "https://app.example/client.json", + currentJti: "jti-current", + revokedAt: null as Date | null, +}; + +beforeEach(() => { + vi.clearAllMocks(); + vi.mocked(prisma.apiToken.updateMany).mockResolvedValue({ + count: 0, + } as never); + vi.mocked(prisma.mcpOAuthConnection.updateMany).mockResolvedValue({ + count: 1, + } as never); +}); + +describe("createConnection", () => { + it("seeds currentJti with the first refresh jti", async () => { + vi.mocked(prisma.mcpOAuthConnection.create).mockResolvedValue({ + id: "conn-1", + } as never); + const id = await createConnection({ + userId: "user-1", + clientId: CONN.clientId, + clientName: "Claude", + scope: "health:read offline_access", + resource: "https://health.example/mcp", + jti: "jti-1", + }); + expect(id).toBe("conn-1"); + expect(prisma.mcpOAuthConnection.create).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ currentJti: "jti-1" }), + }), + ); + }); +}); + +describe("rotateConnection — happy path", () => { + it("advances currentJti and revokes prior access tokens (L4)", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + ...CONN, + } as never); + const out = await rotateConnection({ + connectionId: "conn-1", + presentedJti: "jti-current", + newJti: "jti-next", + clientId: CONN.clientId, + userId: "user-1", + }); + expect(out.ok).toBe(true); + expect(prisma.mcpOAuthConnection.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: expect.objectContaining({ currentJti: "jti-current" }), + data: expect.objectContaining({ currentJti: "jti-next" }), + }), + ); + // L4 — prior access tokens revoked on rotation. + expect(prisma.apiToken.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { mcpConnectionId: "conn-1", revoked: false }, + data: { revoked: true }, + }), + ); + }); +}); + +describe("rotateConnection — revoked connection stops refresh", () => { + it("rejects when the connection is revoked", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + ...CONN, + revokedAt: new Date(), + } as never); + const out = await rotateConnection({ + connectionId: "conn-1", + presentedJti: "jti-current", + newJti: "jti-next", + clientId: CONN.clientId, + userId: "user-1", + }); + expect(out).toEqual({ ok: false, reason: "revoked" }); + expect(prisma.mcpOAuthConnection.updateMany).not.toHaveBeenCalled(); + }); +}); + +describe("rotateConnection — reuse detection revokes the family", () => { + it("revokes the connection when a stale jti is presented", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + ...CONN, + currentJti: "jti-rotated-already", + } as never); + const out = await rotateConnection({ + connectionId: "conn-1", + presentedJti: "jti-old", // not the current jti → replay + newJti: "jti-next", + clientId: CONN.clientId, + userId: "user-1", + }); + expect(out).toEqual({ ok: false, reason: "reuse_detected" }); + // The whole connection is revoked + its access tokens killed. + expect(prisma.mcpOAuthConnection.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { id: "conn-1", revokedAt: null }, + data: expect.objectContaining({ revokedAt: expect.any(Date) }), + }), + ); + expect(prisma.apiToken.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { mcpConnectionId: "conn-1", revoked: false }, + }), + ); + }); + + it("rejects a connection owned by another user", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + ...CONN, + userId: "someone-else", + } as never); + const out = await rotateConnection({ + connectionId: "conn-1", + presentedJti: "jti-current", + newJti: "jti-next", + clientId: CONN.clientId, + userId: "user-1", + }); + expect(out).toEqual({ ok: false, reason: "not_found" }); + }); +}); + +describe("revokeConnectionForUser — settings revoke kills the chain", () => { + it("revokes a connection owned by the user and its access tokens", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + id: "conn-1", + userId: "user-1", + revokedAt: null, + } as never); + const ok = await revokeConnectionForUser("user-1", "conn-1"); + expect(ok).toBe(true); + expect(prisma.mcpOAuthConnection.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + data: expect.objectContaining({ revokedAt: expect.any(Date) }), + }), + ); + expect(prisma.apiToken.updateMany).toHaveBeenCalledWith( + expect.objectContaining({ + where: { mcpConnectionId: "conn-1", revoked: false }, + data: { revoked: true }, + }), + ); + }); + + it("refuses to revoke another user's connection", async () => { + vi.mocked(prisma.mcpOAuthConnection.findUnique).mockResolvedValue({ + id: "conn-1", + userId: "someone-else", + revokedAt: null, + } as never); + const ok = await revokeConnectionForUser("user-1", "conn-1"); + expect(ok).toBe(false); + expect(prisma.mcpOAuthConnection.updateMany).not.toHaveBeenCalled(); + }); +}); diff --git a/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts b/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts index e923dc7fa..ce94e51c3 100644 --- a/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts +++ b/src/lib/mcp/oauth/__tests__/oauth-bridge.test.ts @@ -149,6 +149,35 @@ describe("clients — CIMD via safeFetch (SSRF-safe)", () => { const res = await resolveClient("https://169.254.169.254/meta.json"); expect(res).toEqual({ ok: false, reason: "ssrf_blocked" }); }); + + it("rejects a CIMD doc with a non-https / non-loopback redirect (L2)", async () => { + const clientId = "https://app.example/mcp-client.json"; + vi.mocked(safeFetch).mockResolvedValue( + new Response( + JSON.stringify({ + client_id: clientId, + client_name: "Sketchy", + redirect_uris: ["http://evil.example/cb"], // non-loopback http + }), + { status: 200 }, + ), + ); + const res = await resolveClient(clientId); + expect(res).toEqual({ ok: false, reason: "invalid_metadata" }); + }); + + it("rejects an oversized CIMD body via the streaming cap (M3)", async () => { + const clientId = "https://app.example/mcp-client.json"; + // Declared Content-Length over the 16 KB cap → rejected before reading. + vi.mocked(safeFetch).mockResolvedValue( + new Response("x".repeat(64), { + status: 200, + headers: { "content-length": String(64 * 1024) }, + }), + ); + const res = await resolveClient(clientId); + expect(res).toEqual({ ok: false, reason: "invalid_metadata" }); + }); }); describe("clients — redirect URI matching", () => { diff --git a/src/lib/mcp/oauth/clients.ts b/src/lib/mcp/oauth/clients.ts index 74bcf2624..01717c1c1 100644 --- a/src/lib/mcp/oauth/clients.ts +++ b/src/lib/mcp/oauth/clients.ts @@ -23,6 +23,7 @@ * ordinary registered entry. */ import { safeFetch } from "@/lib/safe-fetch"; +import { readBodyCapped } from "@/lib/http/read-capped"; import { ARTIFACT_KINDS, signArtifact, verifyArtifact } from "./artifacts"; export interface ResolvedClient { @@ -55,6 +56,30 @@ function asStringArray(value: unknown): string[] | null { return out; } +const REDIRECT_LOOPBACK_HOSTS = ["localhost", "127.0.0.1", "::1", "[::1]"]; + +/** + * A redirect URI must be an absolute HTTPS URL or an `http` loopback URL. This + * is the single source of truth shared by DCR (the `register` route) and CIMD + * (L2): both registration paths enforce the identical https/loopback floor. + */ +export function isAllowableRedirectUri(value: string): boolean { + let url: URL; + try { + url = new URL(value); + } catch { + return false; + } + if (url.protocol === "https:") return true; + if ( + url.protocol === "http:" && + REDIRECT_LOOPBACK_HOSTS.includes(url.hostname) + ) { + return true; + } + return false; +} + /** A `client_id` that is an HTTPS URL is a CIMD client (SEP-991). */ export function isCimdClientId(clientId: string): boolean { try { @@ -131,13 +156,16 @@ async function resolveCimdClient(clientId: string): Promise { } if (!res.ok) return { ok: false, reason: "unknown_client" }; - const raw = await res.text(); - if (raw.length > CIMD_MAX_BYTES) { + // M3 — enforce the byte cap WHILE reading: reject up front on an oversized + // Content-Length and abort the stream the moment it overflows, so a hostile + // CIMD host cannot buffer gigabytes into memory before a post-hoc check. + const read = await readBodyCapped(res, CIMD_MAX_BYTES); + if (!read.ok) { return { ok: false, reason: "invalid_metadata" }; } let doc: Record; try { - doc = JSON.parse(raw); + doc = JSON.parse(read.text); } catch { return { ok: false, reason: "invalid_metadata" }; } @@ -148,6 +176,10 @@ async function resolveCimdClient(clientId: string): Promise { } const redirectUris = asStringArray(doc.redirect_uris); if (!redirectUris) return { ok: false, reason: "invalid_metadata" }; + // L2 — CIMD redirect URIs get the same https/loopback floor DCR enforces. + if (!redirectUris.every(isAllowableRedirectUri)) { + return { ok: false, reason: "invalid_metadata" }; + } return { ok: true, diff --git a/src/lib/mcp/oauth/config.ts b/src/lib/mcp/oauth/config.ts index b36eaec5b..d0597af59 100644 --- a/src/lib/mcp/oauth/config.ts +++ b/src/lib/mcp/oauth/config.ts @@ -15,9 +15,49 @@ * `resource` value matches the user-entered URL (a Claude.ai requirement). */ +/** + * Thrown when the MCP/OAuth surface is asked to resolve its canonical origin but + * neither `APP_URL` nor `NEXT_PUBLIC_APP_URL` is configured. The surface fails + * closed in that case (M1): without an operator-pinned origin the issuer, + * endpoint URLs, and RFC 8707 audience would be derived from the attacker- + * influenceable `Host` header, so we refuse to serve rather than trust it. + */ +export class McpOriginNotConfiguredError extends Error { + constructor() { + super( + "APP_URL (or NEXT_PUBLIC_APP_URL) must be set to serve the MCP/OAuth surface", + ); + this.name = "McpOriginNotConfiguredError"; + } +} + +/** The operator-configured origin, env-only — `Host` is never trusted. */ +function configuredOrigin(): string | null { + for (const value of [process.env.APP_URL, process.env.NEXT_PUBLIC_APP_URL]) { + const trimmed = value?.trim(); + if (!trimmed) continue; + try { + return new URL(trimmed).origin; + } catch { + // try the next candidate + } + } + return null; +} + +/** Whether the operator has pinned a canonical origin (M1 fail-closed gate). */ +export function isMcpOriginConfigured(): boolean { + return configuredOrigin() !== null; +} + /** * Resolve the public origin of this deployment, e.g. `https://health.example`. - * Operator config wins; the request URL is the last resort. + * + * Operator config (`APP_URL` → `NEXT_PUBLIC_APP_URL`) is authoritative. The + * request URL is a last resort retained ONLY for non-security-bearing callers; + * every MCP/OAuth entry point first asserts `isMcpOriginConfigured()` and fails + * closed (M1), so on those paths the env value is always what is returned and + * the `Host`-derived fallback is never reached. */ export function resolveBaseOrigin(requestUrl?: string): string { const candidates = [ @@ -38,6 +78,17 @@ export function resolveBaseOrigin(requestUrl?: string): string { return "http://localhost:3000"; } +/** + * Like `resolveBaseOrigin` but fails closed: throws `McpOriginNotConfiguredError` + * when no operator origin is set. Every MCP/OAuth surface calls this so a + * missing `APP_URL` refuses service instead of trusting the `Host` header (M1). + */ +export function requireBaseOrigin(): string { + const origin = configuredOrigin(); + if (!origin) throw new McpOriginNotConfiguredError(); + return origin; +} + /** * The canonical MCP resource identifier (RFC 8707 audience). This is the value * the AS binds every issued token to and the value a client MUST send as diff --git a/src/lib/mcp/oauth/connections.ts b/src/lib/mcp/oauth/connections.ts new file mode 100644 index 000000000..091143e01 --- /dev/null +++ b/src/lib/mcp/oauth/connections.ts @@ -0,0 +1,190 @@ +/** + * MCP OAuth connections — persistent, revocable, theft-detectable refresh + * state (Phase 3 security fix, H2). + * + * The bridge AS mints stateless `hlrt_` refresh artifacts (see `artifacts.ts`). + * On their own they reference no server row, so they could not be revoked and a + * replayed (already-rotated) artifact was not treated as theft. This module is + * the anchor the artifact lacked: one `McpOAuthConnection` row per connector + * connection that requested `offline_access`. + * + * - `createConnection` is called on the `authorization_code` exchange. It + * records the granted scope/audience and seeds `currentJti` with the jti of + * the first refresh artifact. + * - `rotateConnection` is called on every `refresh_token` exchange. It is the + * reuse-detection gate: a presented jti that is not the connection's + * `currentJti` is a replay of an already-rotated token → the connection is + * revoked (family revocation, mirroring `refresh-token.ts`) and the refresh + * fails. A revoked connection always fails. The happy path advances + * `currentJti` atomically (a `where currentJti = presented` guard so two + * concurrent refreshes cannot both win) and revokes the prior access tokens + * (L4). + * - `revokeConnection` is the kill switch the settings card flips: it stamps + * `revokedAt` and revokes every access token the connection ever issued, so + * a settings "revoke" terminates the whole refresh chain. + * + * The access tokens themselves stay ordinary `hlk_` `ApiToken` rows, linked + * back through `ApiToken.mcpConnectionId`. + */ +import { prisma } from "@/lib/db"; + +export interface ConnectionRotation { + /** The connection id (`cid`) embedded in the refresh artifact. */ + connectionId: string; + /** The jti presented in the refresh artifact being exchanged. */ + presentedJti: string; + /** The jti to advance the connection to (the new refresh artifact's jti). */ + newJti: string; + /** The client the refresh artifact claims — must match the stored client. */ + clientId: string; + /** The user the refresh artifact claims — must match the stored owner. */ + userId: string; +} + +export type RotationOutcome = + | { ok: true } + | { + ok: false; + reason: "not_found" | "revoked" | "reuse_detected" | "client_mismatch"; + }; + +/** Create the connection anchor for a freshly-granted `offline_access` flow. */ +export async function createConnection(args: { + userId: string; + clientId: string; + clientName: string; + scope: string; + resource: string; + jti: string; +}): Promise { + const row = await prisma.mcpOAuthConnection.create({ + data: { + userId: args.userId, + clientId: args.clientId, + clientName: args.clientName, + scope: args.scope, + resource: args.resource, + currentJti: args.jti, + }, + select: { id: true }, + }); + return row.id; +} + +/** + * Rotate a connection's refresh chain with reuse-detection. + * + * Returns `{ ok: true }` only when the presented jti is the connection's live + * `currentJti` and the connection is not revoked; on success `currentJti` is + * advanced to `newJti` and the connection's prior access tokens are revoked. + * A presented jti that does not match `currentJti` is a replay and revokes the + * connection before returning `reuse_detected`. + */ +export async function rotateConnection( + args: ConnectionRotation, +): Promise { + const connection = await prisma.mcpOAuthConnection.findUnique({ + where: { id: args.connectionId }, + select: { + id: true, + userId: true, + clientId: true, + currentJti: true, + revokedAt: true, + }, + }); + + if (!connection || connection.userId !== args.userId) { + return { ok: false, reason: "not_found" }; + } + if (connection.clientId !== args.clientId) { + return { ok: false, reason: "client_mismatch" }; + } + if (connection.revokedAt) { + return { ok: false, reason: "revoked" }; + } + if (connection.currentJti !== args.presentedJti) { + // Replay of an already-rotated refresh token — treat as theft and revoke + // the whole connection (reuse-detection family revocation). + await revokeConnectionRow(connection.id); + return { ok: false, reason: "reuse_detected" }; + } + + // Advance the chain atomically: only the caller presenting the current jti + // wins, so two concurrent refreshes cannot both rotate the same row. + const advanced = await prisma.mcpOAuthConnection.updateMany({ + where: { + id: connection.id, + currentJti: args.presentedJti, + revokedAt: null, + }, + data: { currentJti: args.newJti, lastUsedAt: new Date() }, + }); + if (advanced.count === 0) { + return { ok: false, reason: "reuse_detected" }; + } + + // L4 — revoke the access tokens minted before this rotation so a leaked + // 60-minute access token cannot outlive the refresh that spawned it. + await prisma.apiToken.updateMany({ + where: { mcpConnectionId: connection.id, revoked: false }, + data: { revoked: true }, + }); + + return { ok: true }; +} + +/** Internal — stamp `revokedAt` and revoke every linked access token. */ +async function revokeConnectionRow(connectionId: string): Promise { + await prisma.mcpOAuthConnection.updateMany({ + where: { id: connectionId, revokedAt: null }, + data: { revokedAt: new Date() }, + }); + await prisma.apiToken.updateMany({ + where: { mcpConnectionId: connectionId, revoked: false }, + data: { revoked: true }, + }); +} + +/** + * Revoke a connection on behalf of its owner (settings "revoke"). Ownership is + * enforced so a connection id alone cannot revoke another user's connection. + * Returns true when a live connection owned by the user was revoked. + */ +export async function revokeConnectionForUser( + userId: string, + connectionId: string, +): Promise { + const connection = await prisma.mcpOAuthConnection.findUnique({ + where: { id: connectionId }, + select: { id: true, userId: true, revokedAt: true }, + }); + if (!connection || connection.userId !== userId) return false; + await revokeConnectionRow(connection.id); + return true; +} + +export interface ConnectionSummary { + id: string; + clientName: string; + scope: string; + createdAt: Date; + lastUsedAt: Date | null; +} + +/** The user's live (non-revoked) connector connections for the settings list. */ +export async function listConnectionsForUser( + userId: string, +): Promise { + return prisma.mcpOAuthConnection.findMany({ + where: { userId, revokedAt: null }, + select: { + id: true, + clientName: true, + scope: true, + createdAt: true, + lastUsedAt: true, + }, + orderBy: { createdAt: "desc" }, + }); +} diff --git a/src/lib/query-keys/integrations.ts b/src/lib/query-keys/integrations.ts index 0434b52d4..361372960 100644 --- a/src/lib/query-keys/integrations.ts +++ b/src/lib/query-keys/integrations.ts @@ -9,6 +9,10 @@ export const integrationKeys = { // for the remote MCP endpoint). Distinct key from `tokens()` so the MCP // settings card's mint / revoke mutations invalidate only this list. mcpTokens: () => ["mcp-tokens"] as const, + // MCP Phase 3 — OAuth connector connections (the revocable unit for a remote + // client authorized through the OAuth bridge). Distinct from `mcpTokens()` so + // a connection revoke invalidates only the connection list. + mcpConnections: () => ["mcp-connections"] as const, telegram: () => ["telegram"] as const, telegramSettings: () => ["telegram", "settings"] as const, withings: () => ["withings"] as const,