From e63d852847a5d4bb8507809dc38287c1128a37e8 Mon Sep 17 00:00:00 2001 From: Kristian Wind Date: Mon, 17 Aug 2026 14:00:39 +0200 Subject: [PATCH] feat(mcp): let the connector address carry its own key MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit verdande could not be added to Claude at all. The documented setup — POST /api/v1/mcp with an Authorization header — cannot be configured from Claude's custom-connector dialog, which asks for a name, a URL and OAuth client credentials and has no field for a bearer token. The instructions described something the interface does not offer. Worse, the obvious guess failed silently in the most misleading way available: /mcp was not a route, so the SPA fallback answered 200 with a page of HTML. The connector reported a successful connection and then failed to parse the response, which reads as a broken client rather than a missing endpoint. So /mcp exists now and takes its credential in the query string, exactly as the calendar feed does and for the same reason: a client that cannot be told to send a header means the address is the credential. The request logger records paths without their query strings, so the key does not reach the log. It is deliberately outside /api/v1 and accepts the key and nothing else. Reading a session cookie here would make it a state-changing POST outside the CSRF check — a cross-site request that acts as whoever is signed in — and there is a test that fails if that ever becomes true. The API path is unchanged for anything that can send a header. The tokens page now shows the connector address next to the token it belongs to, since that is the one moment the token exists. Co-Authored-By: Claude Opus 5 --- docs/mcp.md | 31 ++++- docs/openapi.yaml | 47 ++++++++ internal/httpapi/integrations_test.go | 109 ++++++++++++++++++ internal/httpapi/mcp_handlers.go | 38 ++++++ internal/httpapi/server.go | 12 ++ web/e2e/smoke.spec.js | 18 +++ .../routes/indstillinger/tokens/+page.svelte | 41 +++++++ 7 files changed, 292 insertions(+), 4 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index aff58a4..840de71 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -12,17 +12,40 @@ conversation. ## Connecting it -**Make a token.** **Settings → API tokens → New token**. It is shown once. +**Make a token** under **Settings → API tokens**. The page then shows the +connector address to go with it — token and all — which is the thing to copy: -**Add the connector.** In Claude, add a custom connector with: +``` +https://todo.example.dk/mcp?key=vrd_… +``` -- **URL**: `https://todo.example.dk/api/v1/mcp` -- **Authentication**: Bearer token, using the token you just made +**Add the connector** in Claude with that address, and nothing else. The dialog +asks for a name and a URL; leave the OAuth fields empty. + +!!! note "Why the key is in the address" + The dialog has no field for a bearer token — only OAuth client credentials — + so a header cannot be configured from it. The address *is* the credential + here, exactly as it is for the [calendar feed](caldav.md#subscribing-to-a-feed). + Treat it like a password: anyone holding it reaches what you reach. The + server logs request paths without their query strings, so it does not end up + in the log. !!! warning "It has to be reachable from the internet" Claude connects from Anthropic's side, not from your browser. An instance only reachable on your LAN or over a VPN cannot be used this way. +### With a header instead + +Anything that *can* send a header should, and use the API path: + +```bash +curl -H "Authorization: Bearer vrd_…" \ + -d '{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}' \ + https://todo.example.dk/api/v1/mcp +``` + +Both endpoints run the same server and reach the same things. + ## What Claude can do | Tool | What it does | diff --git a/docs/openapi.yaml b/docs/openapi.yaml index 916db81..dc738c6 100644 --- a/docs/openapi.yaml +++ b/docs/openapi.yaml @@ -1787,6 +1787,53 @@ paths: responses: "302": { description: "Back into the app, with the outcome as a query parameter." } + /mcp: + post: + tags: [integrations] + summary: The MCP endpoint, with the token in the query string. + description: | + The same server as `/api/v1/mcp`, for a client that can only be given a + URL. Claude's custom-connector dialog asks for a name, an address and + OAuth client credentials — there is no field for a bearer token — so the + API path cannot be configured from it. + + The key is the only credential accepted here. A session cookie is + ignored: this route sits outside the CSRF check, and honouring a cookie + would make it a cross-site request that acts as whoever is signed in. + security: [] + parameters: + - name: key + in: query + required: true + schema: { type: string, example: vrd_… } + description: A personal API token. Treat the whole address as a password. + requestBody: + required: true + content: + application/json: + schema: + type: object + required: [jsonrpc, method] + properties: + jsonrpc: { type: string, const: "2.0" } + id: { oneOf: [{ type: string }, { type: integer }] } + method: { type: string } + params: { type: object } + responses: + "200": + description: A JSON-RPC response. + content: + application/json: + schema: + type: object + properties: + jsonrpc: { type: string, const: "2.0" } + id: { oneOf: [{ type: string }, { type: integer }] } + result: { type: object } + error: { type: object } + "202": { description: A notification. No body. } + "401": { $ref: "#/components/responses/Unauthorized" } + /.well-known/caldav: get: tags: [integrations] diff --git a/internal/httpapi/integrations_test.go b/internal/httpapi/integrations_test.go index 37335e2..c3a663d 100644 --- a/internal/httpapi/integrations_test.go +++ b/internal/httpapi/integrations_test.go @@ -771,3 +771,112 @@ func TestUpdateNoticeIsForAdministratorsOnly(t *testing.T) { t.Error("the current version is hidden from members") } } + +// --- MCP with the key in the URL -------------------------------------------------- + +// Claude's custom-connector dialog takes a URL and nothing else — no bearer +// token — so this is the only shape that can be configured from it. +func TestMCPAcceptsAKeyInTheQuery(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + var userID string + err := ts.db.QueryRowContext(t.Context(), `SELECT id FROM users LIMIT 1`).Scan(&userID) + if err != nil { + t.Fatal(err) + } + key := ts.apiToken(t, userID) + + body := `{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}` + resp, err := http.Post(ts.URL+"/mcp?key="+key, "application/json", strings.NewReader(body)) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusOK { + t.Fatalf("status %d, want 200", resp.StatusCode) + } + var decoded map[string]any + if err := json.NewDecoder(resp.Body).Decode(&decoded); err != nil { + t.Fatalf("the response was not JSON-RPC: %v", err) + } + result, _ := decoded["result"].(map[string]any) + if result == nil || result["protocolVersion"] == nil { + t.Errorf("no protocol version was announced: %v", decoded) + } +} + +// The endpoint that broke it: /mcp used not to exist, so it fell through to the +// SPA fallback and answered 200 with the app shell. A connector pointed at it +// reported success and then failed to parse a page of HTML. +func TestMCPWithoutAKeyIsNotTheAppShell(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + for _, url := range []string{ts.URL + "/mcp", ts.URL + "/mcp?key=not-a-verdande-token"} { + resp, err := http.Post(url, "application/json", strings.NewReader(`{"jsonrpc":"2.0","id":1}`)) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("%s: status %d, want 401", url, resp.StatusCode) + } + if ct := resp.Header.Get("Content-Type"); !strings.HasPrefix(ct, "application/json") { + t.Errorf("%s: content-type %q — a connector cannot read that", url, ct) + } + } +} + +// A revoked key stops working, like any other. +func TestMCPKeyStopsWorkingWhenRevoked(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + var userID string + if err := ts.db.QueryRowContext(t.Context(), `SELECT id FROM users LIMIT 1`).Scan(&userID); err != nil { + t.Fatal(err) + } + key := ts.apiToken(t, userID) + + var id string + if err := ts.db.QueryRowContext(t.Context(), `SELECT id FROM api_tokens LIMIT 1`).Scan(&id); err != nil { + t.Fatal(err) + } + if err := ts.db.DeleteAPIToken(t.Context(), userID, id); err != nil { + t.Fatal(err) + } + + resp, err := http.Post(ts.URL+"/mcp?key="+key, "application/json", + strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}`)) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("a revoked key still works: status %d", resp.StatusCode) + } +} + +// The session cookie must NOT work here. This route sits outside the CSRF check, +// so a cookie-authenticated POST would be a cross-site request that acts as +// whoever is signed in — the exact hole the key-only rule exists to close. +func TestMCPWithKeyIgnoresTheSessionCookie(t *testing.T) { + ts := newTestServer(t) + ts.bootstrap(t) + + // ts.client holds the session from bootstrap. No key in the URL. + resp, err := ts.client.Post(ts.URL+"/mcp", "application/json", + strings.NewReader(`{"jsonrpc":"2.0","id":1,"method":"initialize","params":{}}`)) + if err != nil { + t.Fatal(err) + } + defer resp.Body.Close() + + if resp.StatusCode != http.StatusUnauthorized { + t.Errorf("a signed-in cookie authenticated a POST outside the CSRF check: status %d", + resp.StatusCode) + } +} diff --git a/internal/httpapi/mcp_handlers.go b/internal/httpapi/mcp_handlers.go index 88449cb..846aeae 100644 --- a/internal/httpapi/mcp_handlers.go +++ b/internal/httpapi/mcp_handlers.go @@ -3,11 +3,13 @@ package httpapi import ( "context" "encoding/json" + "errors" "io" "net/http" "strings" "time" + "github.com/kristianwind/verdande/internal/auth" "github.com/kristianwind/verdande/internal/mcp" "github.com/kristianwind/verdande/internal/quickadd" "github.com/kristianwind/verdande/internal/store" @@ -124,6 +126,42 @@ func (s *Server) handleMCP(w http.ResponseWriter, r *http.Request) { writeJSON(w, http.StatusOK, response) } +// handleMCPWithKey is the same endpoint for a client that can only be given a +// URL: `POST /mcp?key=vrd_…`. +// +// Claude's custom-connector dialog asks for a name, an address and — under +// advanced settings — an OAuth client id and secret. There is no field for a +// bearer token, so /api/v1/mcp cannot be configured from it however correct the +// address is. The calendar feed already carries its credential in the URL for +// the same reason, and the request logger records the path without the query, so +// the key does not end up in the log. +// +// The key is the *only* thing accepted here. Reading a session cookie instead +// would make this a state-changing POST outside the CSRF check — which is to say +// a cross-site request that acts as whoever is signed in. +func (s *Server) handleMCPWithKey(w http.ResponseWriter, r *http.Request) { + key := r.URL.Query().Get("key") + if key == "" || !auth.IsAPIToken(key) { + // 401 rather than 404: unlike a task, the existence of this endpoint is + // not a secret — it is written in the documentation. + writeError(w, http.StatusUnauthorized, CodeUnauthorized, + "add ?key= to the URL") + return + } + + user, err := s.db.UserByAPIToken(r.Context(), key) + if err != nil { + if !errors.Is(err, store.ErrNotFound) { + s.internal(w, "mcp key", err) + return + } + writeError(w, http.StatusUnauthorized, CodeUnauthorized, "that key is not valid") + return + } + + s.handleMCP(w, r.WithContext(withUser(r.Context(), user))) +} + // --- tool implementations ------------------------------------------------------------ func (s *Server) mcpListProjects(ctx context.Context, userID string, args json.RawMessage) (any, error) { diff --git a/internal/httpapi/server.go b/internal/httpapi/server.go index 8f47513..e8c2951 100644 --- a/internal/httpapi/server.go +++ b/internal/httpapi/server.go @@ -90,6 +90,18 @@ func New(cfg *config.Config, db *store.DB, log *slog.Logger, web fs.FS) *Server // authenticated by the token in the recipient address. r.Post("/inbound/mail", s.handleInboundMail) + // MCP with the token in the query string, for clients that cannot send a + // header. Claude's custom-connector dialog takes a URL and nothing else, so + // /api/v1/mcp — which wants Authorization: Bearer — cannot be configured from + // it at all. Same reasoning as the calendar feed above: a client that cannot + // be told to send a header means the token in the URL is the credential. + // + // Outside /api/v1 deliberately, because it must not be reachable with a + // session cookie. A cookie-authenticated POST with no CSRF check is a + // cross-site request waiting to happen; handleMCPWithKey accepts the key and + // nothing else. + r.Post("/mcp", s.handleMCPWithKey) + // Google sends the browser here after consent. Outside /api/v1 because it is a // top-level navigation rather than a fetch, but still behind the session — the // tokens it exchanges belong to whoever is signed in. diff --git a/web/e2e/smoke.spec.js b/web/e2e/smoke.spec.js index a3d9565..115eb3e 100644 --- a/web/e2e/smoke.spec.js +++ b/web/e2e/smoke.spec.js @@ -179,3 +179,21 @@ test('manifestet og service workeren findes, og ikonerne findes også', async ({ expect(worker.ok()).toBeTruthy(); expect(await worker.text()).toContain("addEventListener('push'"); }); + +/** + * The MCP connector address must not fall through to the app shell. + * + * It did: `/mcp` was not a route, so the SPA fallback answered 200 with a page + * of HTML. A connector pointed at it reported a successful connection and then + * failed to parse the response — which looks like a broken client rather than a + * missing route. A 401 in JSON is the honest answer. + */ +test('MCP-adressen svarer JSON, ikke app-skallen', async ({ request }) => { + const response = await request.post('/mcp', { + data: { jsonrpc: '2.0', id: 1, method: 'initialize' } + }); + + expect(response.status(), 'uden nøgle skal den afvise, ikke servere siden').toBe(401); + expect(response.headers()['content-type']).toContain('application/json'); + expect((await response.json()).code).toBe('unauthorized'); +}); diff --git a/web/src/routes/indstillinger/tokens/+page.svelte b/web/src/routes/indstillinger/tokens/+page.svelte index c3c887d..83b1ac1 100644 --- a/web/src/routes/indstillinger/tokens/+page.svelte +++ b/web/src/routes/indstillinger/tokens/+page.svelte @@ -20,6 +20,10 @@ /** The plaintext of the token just minted. Held only in this variable. */ let fresh = $state(null); let copied = $state(false); + let connectorCopied = $state(false); + + /** What Claude's connector dialog needs, which is a URL and nothing else. */ + let connectorURL = $derived(fresh ? `${location.origin}/mcp?key=${fresh.token}` : ''); $effect(() => { api @@ -59,6 +63,15 @@ } } + async function copyConnector() { + try { + await navigator.clipboard.writeText(connectorURL); + connectorCopied = true; + } catch { + app.toast('Kunne ikke kopiere — markér teksten i stedet.'); + } + } + async function revoke(token) { if (!confirm(`Tilbagekald "${token.name}"? Alt, der bruger den, holder op med at virke.`)) return; @@ -100,6 +113,22 @@ {#if copied}Kopieret.{/if} + + +
+

+ Skal du bruge den som MCP-forbindelse + i Claude? Så er det denne adresse, dialogen skal have: +

+

{connectorURL}

+
+ + {#if connectorCopied}Kopieret.{/if} +
+
{/if} @@ -194,6 +223,18 @@ user-select: all; } + .also { + display: flex; + flex-direction: column; + gap: var(--s2); + padding-top: var(--s3); + border-top: 1px solid var(--accent); + } + + .also a { + color: inherit; + } + .what { display: flex; flex-direction: column;