From 5cb25bc2927981ecdd728cade69c34333953d603 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 20:39:02 +0000 Subject: [PATCH 1/3] fix: authenticate resource owner at OAuth authorize consent The /oauth/authorize consent step issued an authorization code to anyone who clicked "Authorize", with no owner authentication. Because the OAuth endpoints are public, any party who knew the server URL could add it as a custom connector, complete consent, and obtain a token with full access to the single user's memories. Require MEM0_API_KEY at the consent step: the form now has an API-key field and POST /oauth/authorize validates it (constant-time) before issuing a code, re-rendering the form with a 401 on failure. Add tests for the wrong/missing password cases and document the security model. https://claude.ai/code/session_01U3EtN3puoZRq2t7nedcnHY --- CLAUDE.md | 1 + app/oauth.py | 73 +++++++++++++++++++++++++++++++---------- docs/DEVELOPER_GUIDE.md | 5 ++- docs/USER_GUIDE.md | 15 ++++++--- tests/test_oauth.py | 45 +++++++++++++++++++++++-- 5 files changed, 113 insertions(+), 26 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9a40dd8..a5c0674 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -33,6 +33,7 @@ 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) diff --git a/app/oauth.py b/app/oauth.py index 6c1889c..6e695f8 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,42 +161,66 @@ 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. Constant-time compare; an empty + # configured key must never authenticate. + key = get_settings().mem0_api_key + return bool(key) and secrets.compare_digest(password, key) + + +@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") @@ -206,10 +230,23 @@ def authorize_submit( 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") + # 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..c133cff 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -182,13 +182,20 @@ transport). Restart Claude Desktop to pick it up. 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_oauth.py b/tests/test_oauth.py index 8df661d..31b1527 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,41 @@ 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_form_escapes_state(oauth_client): client_id = _register(oauth_client) _, challenge = _pkce() @@ -185,7 +221,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 +264,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 +285,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] From 52a503a744064f1d973b4efe83178b094ac0bdd8 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 21:02:23 +0000 Subject: [PATCH 2/3] fix: make MCP memory reads span all agents (shared store) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The MCP search_memories/list_memories tools filtered by agent_id when the caller supplied it. Clients' models populate agent_id (observed: a memory tagged agent_id="claude-web"), so each agent effectively read only its own writes — Claude Code and Codex appeared to have separate memories even though the store is physically shared under one user_id. Remove agent_id from the read tools so search/list always span the whole user store. Keep agent_id on add_memory as an optional write-only provenance tag. The REST API still supports agent_id read filters for explicit scripted use. https://claude.ai/code/session_01U3EtN3puoZRq2t7nedcnHY --- CLAUDE.md | 2 +- app/mcp_server.py | 25 +++++++++++++------------ docs/USER_GUIDE.md | 5 ++++- tests/test_mcp.py | 17 +++++++++++++---- 4 files changed, 31 insertions(+), 18 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index a5c0674..a114790 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -39,7 +39,7 @@ Phase 1 (MVP) = static bearer-token auth. Phase 2 adds OAuth 2.1 + PKCE + DCR en - `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/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index c133cff..9e00e7f 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -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. 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: From 8723f7ec6dd1368e8f747672934a7d918beb051a Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 23 May 2026 21:10:42 +0000 Subject: [PATCH 3/3] fix: enforce PKCE on authorize POST, hash consent compare, doc URL consistency Address review feedback on PR #45: - POST /oauth/authorize now rejects an empty code_challenge (400) so a direct POST can't mint a code that could never be exchanged - _owner_authenticated hashes both the submitted password and the configured key to fixed-length digests before secrets.compare_digest, so the comparison is genuinely constant-time (no length leak) - standardize the Claude Code/Desktop example URLs on the canonical /mcp (no trailing slash) to match the OAuth section https://claude.ai/code/session_01U3EtN3puoZRq2t7nedcnHY --- app/oauth.py | 17 +++++++++++++---- docs/USER_GUIDE.md | 9 +++++---- tests/test_oauth.py | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+), 8 deletions(-) diff --git a/app/oauth.py b/app/oauth.py index 6e695f8..2e7fb33 100644 --- a/app/oauth.py +++ b/app/oauth.py @@ -195,10 +195,15 @@ def _consent_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. Constant-time compare; an empty - # configured key must never authenticate. + # 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 - return bool(key) and secrets.compare_digest(password, 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") @@ -227,7 +232,7 @@ def authorize_form( 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"), password: str = Form(""), @@ -235,6 +240,10 @@ def authorize_submit( 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". diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 9e00e7f..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. @@ -168,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" ``` @@ -178,8 +178,9 @@ 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) diff --git a/tests/test_oauth.py b/tests/test_oauth.py index 31b1527..3259ea7 100644 --- a/tests/test_oauth.py +++ b/tests/test_oauth.py @@ -194,6 +194,24 @@ def test_authorize_rejects_missing_password(oauth_client): 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()