feat(mcp): let the connector address carry its own key
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 <noreply@anthropic.com>
This commit is contained in:
committed by
Kristian Wind
co-authored by
Claude Opus 5
parent
d7e250f860
commit
e63d852847
@@ -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)
|
||||
}
|
||||
}
|
||||
|
||||
@@ -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=<api token> 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) {
|
||||
|
||||
@@ -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.
|
||||
|
||||
Reference in New Issue
Block a user