diff --git a/CLAUDE.md b/CLAUDE.md index 9a40dd8..a114790 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,12 +33,13 @@ Phase 1 (MVP) = static bearer-token auth. Phase 2 adds OAuth 2.1 + PKCE + DCR en - **`MEM0_EMBED_DIMS` must match the embedder's real output dimension** (3-small=1536, 3-large=3072). A mismatch causes *silent* search failures, not errors. Changing embedding models requires dropping and recreating the Qdrant collection. - **FastMCP = the PrefectHQ `fastmcp` PyPI package**, imported `from fastmcp import FastMCP`. It is NOT the older `mcp.server.fastmcp` module. - **Same `MEM0_API_KEY` protects both** the REST endpoints (`require_bearer` dependency) and the MCP endpoint (`StaticTokenVerifier` in Phase 1). +- **The Phase 2 OAuth `/oauth/authorize` consent step authenticates the resource owner** by requiring `MEM0_API_KEY` (constant-time compare) before issuing a code. Don't remove this — the OAuth endpoints are public, so without it anyone reaching the consent screen could mint a token for the single user's memories. ## Planned layout (per PRD §3) - `app/config.py` — `Settings` (pydantic-settings); single source of config truth, reject startup on missing required vars. - `app/memory.py` — mem0 wrapper / `_build_config`; **the most tweak-prone file**. -- `app/mcp_server.py` — the six MCP tools (add/search/list/get/update/delete), each thinly wrapping a mem0 op with `user_id` defaulted. +- `app/mcp_server.py` — the six MCP tools (add/search/list/get/update/delete), each thinly wrapping a mem0 op with `user_id` defaulted. MCP reads (`search`/`list`) must NOT filter by `agent_id` — all agents share one memory; `agent_id` is a write-only provenance tag on `add`. (REST may still filter reads by `agent_id` for explicit scripted use.) - `app/rest.py` — REST router under `/api/v1`. - `app/auth.py` — `require_bearer` + `build_verifier()` (Phase 1); composite bearer-or-JWT verifier (Phase 2). - `app/oauth.py` / `app/oauth_store.py` — Phase 2 OAuth AS endpoints + SQLite store at `/app/data/oauth.db` (CapRover persistent volume). diff --git a/app/mcp_server.py b/app/mcp_server.py index 99cf705..b8f45a5 100644 --- a/app/mcp_server.py +++ b/app/mcp_server.py @@ -17,6 +17,10 @@ def add_memory(content: str, agent_id: str | None = None, metadata: dict | None Use when the user shares preferences, project context, decisions, or anything they may want recalled in future conversations. + + agent_id is an optional provenance tag recording which agent wrote the + memory. It does NOT partition the store — search and list always span + every memory for the user, so all connected agents share one memory. """ kwargs: dict = {"user_id": default_user} if agent_id: @@ -26,20 +30,17 @@ def add_memory(content: str, agent_id: str | None = None, metadata: dict | None return memory.add(content, **kwargs) @mcp.tool - def search_memories(query: str, agent_id: str | None = None, limit: int = 10) -> dict: - """Search long-term memory by semantic similarity.""" - filters: dict = {"user_id": default_user} - if agent_id: - filters["agent_id"] = agent_id - return memory.search(query=query, filters=filters, top_k=limit) + def search_memories(query: str, limit: int = 10) -> dict: + """Search long-term memory by semantic similarity. + + Searches the single shared memory store for the user, across all agents. + """ + return memory.search(query=query, filters={"user_id": default_user}, top_k=limit) @mcp.tool - def list_memories(agent_id: str | None = None) -> dict: - """List all stored memories for the current user.""" - filters: dict = {"user_id": default_user} - if agent_id: - filters["agent_id"] = agent_id - return memory.get_all(filters=filters) + def list_memories() -> dict: + """List all stored memories for the user (shared across all agents).""" + return memory.get_all(filters={"user_id": default_user}) @mcp.tool def get_memory(memory_id: str) -> dict: diff --git a/app/oauth.py b/app/oauth.py index 6c1889c..2e7fb33 100644 --- a/app/oauth.py +++ b/app/oauth.py @@ -10,7 +10,7 @@ from cryptography.hazmat.primitives import serialization from cryptography.hazmat.primitives.asymmetric.rsa import RSAPublicNumbers from fastapi import APIRouter, Form, HTTPException, Request -from fastapi.responses import HTMLResponse, JSONResponse, RedirectResponse +from fastapi.responses import HTMLResponse, JSONResponse, RedirectResponse, Response from app import oauth_store from app.config import get_settings @@ -161,55 +161,101 @@ async def register(request: Request) -> JSONResponse: ) -@router.get("/oauth/authorize") -def authorize_form( +def _consent_page( client_id: str, redirect_uri: str, code_challenge: str, - code_challenge_method: str = "S256", - response_type: str = "code", - state: str = "", - scope: str = "read write", -) -> HTMLResponse: - if response_type != "code": - raise HTTPException(status_code=400, detail="unsupported response_type") - if code_challenge_method != "S256" or not code_challenge: - raise HTTPException(status_code=400, detail="PKCE S256 required") - client = oauth_store.get_client(client_id) - if not client or redirect_uri not in client["redirect_uris"]: - raise HTTPException(status_code=400, detail="invalid client or redirect_uri") + state: str, + scope: str, + error: str = "", +) -> str: e_client_id = html.escape(client_id, quote=True) e_redirect_uri = html.escape(redirect_uri, quote=True) e_code_challenge = html.escape(code_challenge, quote=True) e_state = html.escape(state, quote=True) e_scope = html.escape(scope, quote=True) - page = f""" + error_html = f'

{html.escape(error, quote=True)}

' if error else "" + return f"""

Authorize mem0

+ {error_html} +

Enter your mem0 API key to grant this client access to your memories.

+
""" - return HTMLResponse(page) + + +def _owner_authenticated(password: str) -> bool: + # Single-user: the resource owner proves ownership at the consent step with + # the same MEM0_API_KEY that protects the API. An empty configured key must + # never authenticate. Hash both sides to fixed-length digests so the compare + # is genuinely constant-time (compare_digest can leak length for raw strings). + key = get_settings().mem0_api_key + if not key: + return False + provided = hashlib.sha256(password.encode()).digest() + expected = hashlib.sha256(key.encode()).digest() + return secrets.compare_digest(provided, expected) + + +@router.get("/oauth/authorize") +def authorize_form( + client_id: str, + redirect_uri: str, + code_challenge: str, + code_challenge_method: str = "S256", + response_type: str = "code", + state: str = "", + scope: str = "read write", +) -> HTMLResponse: + if response_type != "code": + raise HTTPException(status_code=400, detail="unsupported response_type") + if code_challenge_method != "S256" or not code_challenge: + raise HTTPException(status_code=400, detail="PKCE S256 required") + client = oauth_store.get_client(client_id) + if not client or redirect_uri not in client["redirect_uris"]: + raise HTTPException(status_code=400, detail="invalid client or redirect_uri") + return HTMLResponse( + _consent_page(client_id, redirect_uri, code_challenge, state, scope) + ) @router.post("/oauth/authorize") def authorize_submit( client_id: str = Form(...), redirect_uri: str = Form(...), - code_challenge: str = Form(...), + code_challenge: str = Form(""), state: str = Form(""), scope: str = Form("read write"), -) -> RedirectResponse: + password: str = Form(""), +) -> Response: client = oauth_store.get_client(client_id) if not client or redirect_uri not in client["redirect_uris"]: raise HTTPException(status_code=400, detail="invalid client or redirect_uri") + # Enforce PKCE here too (the GET form does), so a direct POST can't mint a + # code that could never be exchanged. + if not code_challenge: + raise HTTPException(status_code=400, detail="PKCE S256 required") + # Authenticate the resource owner before issuing a code. Without this, anyone + # who reaches the consent screen could obtain a token for the single user's + # memories just by clicking "Authorize". + if not _owner_authenticated(password): + _log.warning("oauth_consent_rejected", client_id=client_id) + return HTMLResponse( + _consent_page( + client_id, redirect_uri, code_challenge, state, scope, + error="Invalid API key.", + ), + status_code=401, + ) code = secrets.token_urlsafe(32) oauth_store.save_code(code, client_id, redirect_uri, code_challenge) sep = "&" if "?" in redirect_uri else "?" diff --git a/docs/DEVELOPER_GUIDE.md b/docs/DEVELOPER_GUIDE.md index 9e9e82b..de145a5 100644 --- a/docs/DEVELOPER_GUIDE.md +++ b/docs/DEVELOPER_GUIDE.md @@ -121,7 +121,10 @@ MCP tools default `user_id` to the single configured user and expose a narrower explicitly to `/mcp` so the advertised resource is correct. The OAuth flow (`app/oauth.py`) is OAuth 2.1 with PKCE (S256 required) and public clients only — no -client secrets are issued. Endpoints: `/oauth/register` (DCR), `/oauth/authorize` (GET form + POST +client secrets are issued. The `/oauth/authorize` consent step **authenticates the resource owner**: +the POST handler requires the `MEM0_API_KEY` (constant-time compared) before issuing a code. Without +this gate, anyone who reached the public consent screen could mint a token for the single user's +memories just by clicking "Authorize". Endpoints: `/oauth/register` (DCR), `/oauth/authorize` (GET form + POST consent), `/oauth/token` (authorization_code + refresh_token grants), `/.well-known/jwks.json`, and the RFC 8414 / RFC 9728 metadata documents. Tokens live 24h; refresh tokens are single-use and rotated. `oauth_store.py` persists clients/codes/refresh tokens in SQLite, hashing refresh tokens diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 335a198..3d5933f 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -28,7 +28,7 @@ a single service. It gives AI agents and scripts a shared, persistent long-term two ways from one process: - **REST API** under `/api/v1/memories…` — for scripts, n8n, curl, and any HTTP client. -- **Streamable HTTP MCP** under `/mcp/` — for Claude Code, Claude Desktop, Claude.ai web, and Cowork. +- **Streamable HTTP MCP** under `/mcp` — for Claude Code, Claude Desktop, Claude.ai web, and Cowork. Both protocols read and write the **same** memory store, so a fact you save from Claude Code is searchable from a curl script and vice versa. @@ -45,7 +45,10 @@ keyword matches. Memories can optionally be tagged with: -- `agent_id` — which agent/tool wrote it (e.g. `n8n-flow`, `claude-code`), so you can filter later. +- `agent_id` — a provenance tag for which agent/tool wrote it (e.g. `n8n-flow`, `claude-code`). + Over MCP it is **write-only**: the `search`/`list` tools always span the whole store, so every + connected agent (Claude Code, Codex, Claude.ai web, …) shares one memory. The REST API can still + filter reads by `agent_id` for scripts that explicitly want a slice. - `run_id` — a session or workflow run identifier. - `metadata` — arbitrary JSON you attach to a memory. @@ -165,7 +168,7 @@ and Cowork, which use OAuth (Phase 2). ```bash claude mcp add --scope user --transport http mem0-remote \ - https://mem0.your-domain.com/mcp/ \ + https://mem0.your-domain.com/mcp \ --header "Authorization: Bearer $MEM0_API_KEY" ``` @@ -175,20 +178,28 @@ Code. ### Claude Desktop Add an entry under the MCP servers section of Claude Desktop's config, pointing at -`https://mem0.your-domain.com/mcp/` with an `Authorization: Bearer ` header (Streamable HTTP -transport). Restart Claude Desktop to pick it up. +`https://mem0.your-domain.com/mcp` with an `Authorization: Bearer ` header (Streamable HTTP +transport). Restart Claude Desktop to pick it up. Both `/mcp` and `/mcp/` work; `/mcp` is the +canonical form. ### Claude.ai web / Cowork (OAuth) This requires **Phase 2** (`OAUTH_SIGNING_KEY` set). In the client's connector settings: -1. Add a **custom connector** pointing at `https://mem0.your-domain.com/mcp/`. +1. Add a **custom connector** pointing at `https://mem0.your-domain.com/mcp`. 2. Leave the client ID and secret **blank** — the server supports Dynamic Client Registration, so the client registers itself automatically. -3. Complete the consent screen (click **Authorize**) and the redirect back to the client. +3. On the consent screen, **enter your `MEM0_API_KEY`** in the API key field and click + **Authorize**, then let the redirect complete. -The server only allows redirect URIs listed in `OAUTH_ALLOWED_REDIRECT_URIS`, which defaults to the -official claude.ai and Cowork callback URLs. +**Why the API key prompt matters (security):** this server is single-user and the consent step +authenticates *you* as the owner. Because the OAuth endpoints are public, anyone who knows the URL +could otherwise reach the consent screen; requiring `MEM0_API_KEY` at authorization ensures only the +holder of that key can mint an access token to your memories. Treat `MEM0_API_KEY` as the master +credential — anyone with it has full access via either the bearer header or the OAuth flow. + +The server also only allows redirect URIs listed in `OAUTH_ALLOWED_REDIRECT_URIS`, which defaults to +the official claude.ai and Cowork callback URLs. ### REST / curl / n8n diff --git a/tests/test_mcp.py b/tests/test_mcp.py index f078b03..811ab67 100644 --- a/tests/test_mcp.py +++ b/tests/test_mcp.py @@ -38,14 +38,23 @@ async def test_add_memory_tool(mcp, mem): async def test_search_memories_tool(mcp, mem): mem.search.return_value = {"results": []} async with Client(mcp) as client: - await client.call_tool( - "search_memories", {"query": "what", "agent_id": "cc", "limit": 7} - ) + await client.call_tool("search_memories", {"query": "what", "limit": 7}) _, kwargs = mem.search.call_args - assert kwargs["filters"] == {"user_id": "ian", "agent_id": "cc"} + # Reads are never scoped by agent_id: the store is shared across agents. + assert kwargs["filters"] == {"user_id": "ian"} assert kwargs["top_k"] == 7 +async def test_read_tools_do_not_expose_agent_id(mcp): + # Reads must not be scopable by agent_id, or a client's model can partition + # the shared store and break cross-agent memory. + async with Client(mcp) as client: + tools = {t.name: t for t in await client.list_tools()} + for name in ("search_memories", "list_memories"): + props = (tools[name].inputSchema or {}).get("properties", {}) + assert "agent_id" not in props, name + + async def test_list_memories_tool(mcp, mem): mem.get_all.return_value = {"results": []} async with Client(mcp) as client: diff --git a/tests/test_oauth.py b/tests/test_oauth.py index 8df661d..3259ea7 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -131,6 +131,7 @@ def test_full_authorize_token_flow(oauth_client): "client_id": client_id, "redirect_uri": ALLOWED_URI, "code_challenge": challenge, + "password": "test-bearer-token", }, follow_redirects=False, ) @@ -158,6 +159,59 @@ def test_full_authorize_token_flow(oauth_client): assert payload["scope"] == "read write" +def test_authorize_rejects_wrong_password(oauth_client): + # The consent step must authenticate the resource owner: a wrong API key + # must not yield an authorization code. + client_id = _register(oauth_client) + _, challenge = _pkce() + resp = oauth_client.post( + "/oauth/authorize", + data={ + "client_id": client_id, + "redirect_uri": ALLOWED_URI, + "code_challenge": challenge, + "password": "not-the-key", + }, + follow_redirects=False, + ) + assert resp.status_code == 401 + assert "location" not in resp.headers + + +def test_authorize_rejects_missing_password(oauth_client): + client_id = _register(oauth_client) + _, challenge = _pkce() + resp = oauth_client.post( + "/oauth/authorize", + data={ + "client_id": client_id, + "redirect_uri": ALLOWED_URI, + "code_challenge": challenge, + }, + follow_redirects=False, + ) + assert resp.status_code == 401 + assert "location" not in resp.headers + + +def test_authorize_rejects_empty_pkce(oauth_client): + # A direct POST with an empty code_challenge must not mint a code that could + # never be exchanged (PKCE is enforced on POST, not just the GET form). + client_id = _register(oauth_client) + resp = oauth_client.post( + "/oauth/authorize", + data={ + "client_id": client_id, + "redirect_uri": ALLOWED_URI, + "code_challenge": "", + "password": "test-bearer-token", + }, + follow_redirects=False, + ) + assert resp.status_code == 400 + assert "location" not in resp.headers + + def test_authorize_form_escapes_state(oauth_client): client_id = _register(oauth_client) _, challenge = _pkce() @@ -185,7 +239,8 @@ def test_pkce_mismatch_rejected(oauth_client): _, challenge = _pkce() resp = oauth_client.post( "/oauth/authorize", - data={"client_id": client_id, "redirect_uri": ALLOWED_URI, "code_challenge": challenge}, + data={"client_id": client_id, "redirect_uri": ALLOWED_URI, + "code_challenge": challenge, "password": "test-bearer-token"}, follow_redirects=False, ) code = resp.headers["location"].split("code=")[1].split("&")[0] @@ -227,7 +282,8 @@ def test_code_is_single_use(oauth_client): verifier, challenge = _pkce() resp = oauth_client.post( "/oauth/authorize", - data={"client_id": client_id, "redirect_uri": ALLOWED_URI, "code_challenge": challenge}, + data={"client_id": client_id, "redirect_uri": ALLOWED_URI, + "code_challenge": challenge, "password": "test-bearer-token"}, follow_redirects=False, ) code = resp.headers["location"].split("code=")[1].split("&")[0] @@ -247,7 +303,8 @@ def _obtain_tokens(oauth_client) -> dict: verifier, challenge = _pkce() resp = oauth_client.post( "/oauth/authorize", - data={"client_id": client_id, "redirect_uri": ALLOWED_URI, "code_challenge": challenge}, + data={"client_id": client_id, "redirect_uri": ALLOWED_URI, + "code_challenge": challenge, "password": "test-bearer-token"}, follow_redirects=False, ) code = resp.headers["location"].split("code=")[1].split("&")[0]