diff --git a/src/mt_odoo_mcp/api/routes.py b/src/mt_odoo_mcp/api/routes.py index 333d744..ba20472 100644 --- a/src/mt_odoo_mcp/api/routes.py +++ b/src/mt_odoo_mcp/api/routes.py @@ -56,6 +56,7 @@ class ResetPasswordRequest(BaseModel): class CreateApiKeyRequest(BaseModel): name: str + instance_name: Optional[str] = None # ─── Auth dependency ────────────────────────────────────────────────────────── @@ -195,7 +196,8 @@ def api_list_keys(authorization: Optional[str] = Header(None)): "keys": [ {"id": k.id, "name": k.name, "key_prefix": k.key_prefix, "created_at": k.created_at, "last_used_at": k.last_used_at, - "revoked_at": k.revoked_at, "is_active": k.is_active} + "revoked_at": k.revoked_at, "is_active": k.is_active, + "instance_name": k.instance_name} for k in keys ], "total": len(keys), @@ -204,12 +206,23 @@ def api_list_keys(authorization: Optional[str] = Header(None)): @router.post("/keys") def api_create_key(body: CreateApiKeyRequest, authorization: Optional[str] = Header(None)): - """Create a new named API key. Shown once.""" + """Create a new named API key. Shown once. If instance_name is set, the + key is locked to that one Odoo connection — must be one the user + actually owns, checked here to avoid scoping a key to a typo'd name + that would make it permanently unusable.""" user = _require_auth(authorization) - key_id, full_key = create_api_key(user.id, body.name) + if body.instance_name: + owned_names = {c.instance_name for c in list_odoo_credentials(user.id)} + if body.instance_name not in owned_names: + raise HTTPException( + status_code=400, + detail=f"No Odoo connection named '{body.instance_name}'. Add it first via POST /api/credentials.", + ) + key_id, full_key = create_api_key(user.id, body.name, body.instance_name) return { "id": key_id, "name": body.name, + "instance_name": body.instance_name, "api_key": full_key, "warning": "Save this key — it will not be shown again.", "mcp_connection": { diff --git a/src/mt_odoo_mcp/auth/models.py b/src/mt_odoo_mcp/auth/models.py index 8803f16..a119a9b 100644 --- a/src/mt_odoo_mcp/auth/models.py +++ b/src/mt_odoo_mcp/auth/models.py @@ -34,3 +34,4 @@ class ApiKeyInfo: last_used_at: Optional[str] revoked_at: Optional[str] is_active: bool + instance_name: Optional[str] diff --git a/src/mt_odoo_mcp/auth/service.py b/src/mt_odoo_mcp/auth/service.py index 7f865ab..cbbe940 100644 --- a/src/mt_odoo_mcp/auth/service.py +++ b/src/mt_odoo_mcp/auth/service.py @@ -74,15 +74,19 @@ def login(email: str, password: str) -> User: def get_user_by_api_key(api_key: str) -> Optional[User]: """Look up a user by their API key. Returns None if not found/invalid. - The legacy users.api_key_hash path is checked first for backward - compatibility, but if that same key has since been backfilled into - api_keys (via list_api_keys) and explicitly revoked there, it must be - rejected here too — otherwise "revoking" the legacy key in the UI would - silently do nothing since this function never looked at api_keys before. + Checks two places: the legacy users.api_key_hash column (kept for + backward compatibility), and the api_keys table for named/scoped keys + created via POST /api/keys — a named key is a real credential and must + authenticate here too, not just show up in the Tokens list. + + If the legacy key matches but has since been backfilled into api_keys + (via list_api_keys) and explicitly revoked there, it's rejected — + otherwise "revoking" the legacy key in the UI would silently do nothing. """ if not api_key or not api_key.startswith("mtom_"): return None prefix = api_key[:12] + rows = db_fetchall("SELECT * FROM users WHERE api_key_prefix = ? AND is_active = 1", (prefix,)) for row in rows: if verify_api_key(api_key, row["api_key_hash"]): @@ -93,6 +97,16 @@ def get_user_by_api_key(api_key: str) -> Optional[User]: if revoked: return None return _row_to_user(row) + + key_rows = db_fetchall( + "SELECT * FROM api_keys WHERE key_prefix = ? AND is_active = 1 AND revoked_at IS NULL", + (prefix,), + ) + for key_row in key_rows: + if verify_api_key(api_key, key_row["key_hash"]): + user_row = db_fetchone("SELECT * FROM users WHERE id = ? AND is_active = 1", (key_row["user_id"],)) + if user_row: + return _row_to_user(user_row) return None @@ -148,12 +162,14 @@ def delete_user(user_id: int) -> bool: # ─── Multiple API Keys ────────────────────────────────────────────────────── -def create_api_key(user_id: int, name: str) -> tuple[int, str]: - """Create a new named API key. Returns (key_id, full_key) — full_key shown once.""" +def create_api_key(user_id: int, name: str, instance_name: Optional[str] = None) -> tuple[int, str]: + """Create a new named API key. If instance_name is set, this key is + scoped to that one Odoo connection only — see server.py's call_tool + wrapper for enforcement. Returns (key_id, full_key) — full_key shown once.""" full_key, key_hash, key_prefix = generate_api_key() cur = db_execute( - "INSERT INTO api_keys (user_id, name, key_hash, key_prefix) VALUES (?, ?, ?, ?)", - (user_id, name, key_hash, key_prefix), + "INSERT INTO api_keys (user_id, name, key_hash, key_prefix, instance_name) VALUES (?, ?, ?, ?, ?)", + (user_id, name, key_hash, key_prefix, instance_name), ) db_commit() return cur.lastrowid, full_key @@ -191,28 +207,29 @@ def revoke_api_key(user_id: int, key_id: int) -> bool: return cur.rowcount > 0 -def get_api_key_id_for_mcp(api_key: str) -> Optional[int]: - """Resolve which api_keys row (if any) this raw key belongs to, for - analytics attribution at MCP-session-start. Returns the row id whether - it's a named key or a backfilled 'Default (legacy)' row (see - list_api_keys). Returns None only if the key has never been backfilled - into api_keys at all — harmless, just means analytics rows for that - session carry api_key_id=NULL until the user's first /api/keys view - triggers the backfill. The caller already validated the key via +def get_api_key_scope_for_mcp(api_key: str) -> tuple[Optional[int], Optional[str]]: + """Resolve which api_keys row (if any) this raw key belongs to, plus its + instance scope (None = unscoped, can access any of the user's Odoo + connections). Used at MCP-session-start for analytics attribution + (api_key_id) and per-call instance enforcement (instance_name) — see + server.py's call_tool wrapper. Returns (None, None) only if the key has + never been backfilled into api_keys at all (harmless — analytics rows + for that session carry api_key_id=NULL until the user's first /api/keys + view triggers the backfill). The caller already validated the key via get_user_by_api_key before calling this.""" if not api_key or not api_key.startswith("mtom_"): - return None + return None, None prefix = api_key[:12] rows = db_fetchall( - "SELECT id, key_hash FROM api_keys WHERE key_prefix = ? AND is_active = 1 AND revoked_at IS NULL", + "SELECT id, key_hash, instance_name FROM api_keys WHERE key_prefix = ? AND is_active = 1 AND revoked_at IS NULL", (prefix,), ) for row in rows: if verify_api_key(api_key, row["key_hash"]): db_execute("UPDATE api_keys SET last_used_at = datetime('now') WHERE id = ?", (row["id"],)) db_commit() - return row["id"] - return None + return row["id"], row["instance_name"] + return None, None def _row_to_api_key(row) -> ApiKeyInfo: @@ -224,6 +241,7 @@ def _row_to_api_key(row) -> ApiKeyInfo: last_used_at=row["last_used_at"], revoked_at=row["revoked_at"], is_active=bool(row["is_active"]), + instance_name=row["instance_name"], ) diff --git a/src/mt_odoo_mcp/context.py b/src/mt_odoo_mcp/context.py index 9398186..3503a9c 100644 --- a/src/mt_odoo_mcp/context.py +++ b/src/mt_odoo_mcp/context.py @@ -12,6 +12,7 @@ _user_email: ContextVar[str] = ContextVar("user_email") _conn_manager: ContextVar["ConnectionManager"] = ContextVar("conn_manager") _confirm_manager: ContextVar["ConfirmationManager"] = ContextVar("confirm_manager") _api_key_id: ContextVar[Optional[int]] = ContextVar("api_key_id", default=None) +_key_instance_scope: ContextVar[Optional[str]] = ContextVar("key_instance_scope", default=None) def get_user_id() -> int: @@ -34,14 +35,25 @@ def get_api_key_id() -> Optional[int]: return _api_key_id.get() -def set_user_context(user_id: int, user_email: str, conn_mgr, confirm_mgr, api_key_id: Optional[int] = None): +def get_key_instance_scope() -> Optional[str]: + """None means the current session's API key can access any of the + user's Odoo connections. If set, tool calls are locked to that one + connection — see server.py's call_tool wrapper for enforcement.""" + return _key_instance_scope.get() + + +def set_user_context( + user_id: int, user_email: str, conn_mgr, confirm_mgr, + api_key_id: Optional[int] = None, key_instance_scope: Optional[str] = None, +): """Set all context vars for the current MCP session. Returns tokens for reset.""" t1 = _user_id.set(user_id) t2 = _user_email.set(user_email) t3 = _conn_manager.set(conn_mgr) t4 = _confirm_manager.set(confirm_mgr) t5 = _api_key_id.set(api_key_id) - return (t1, t2, t3, t4, t5) + t6 = _key_instance_scope.set(key_instance_scope) + return (t1, t2, t3, t4, t5, t6) def reset_user_context(tokens): @@ -50,3 +62,4 @@ def reset_user_context(tokens): _conn_manager.reset(tokens[2]) _confirm_manager.reset(tokens[3]) _api_key_id.reset(tokens[4]) + _key_instance_scope.reset(tokens[5]) diff --git a/src/mt_odoo_mcp/database.py b/src/mt_odoo_mcp/database.py index b414eb7..5fb3ce8 100644 --- a/src/mt_odoo_mcp/database.py +++ b/src/mt_odoo_mcp/database.py @@ -50,12 +50,16 @@ def init_db(): -- Multiple named, revocable API keys. users.api_key_hash/api_key_prefix -- stay untouched — that's the legacy key and keeps working as-is. + -- instance_name (nullable): if set, this key can only be used + -- against that one Odoo connection (see migration below for + -- adding this column to already-deployed databases). CREATE TABLE IF NOT EXISTS api_keys ( id INTEGER PRIMARY KEY AUTOINCREMENT, user_id INTEGER NOT NULL REFERENCES users(id) ON DELETE CASCADE, name TEXT NOT NULL, key_hash TEXT NOT NULL, key_prefix TEXT NOT NULL, + instance_name TEXT, created_at TEXT NOT NULL DEFAULT (datetime('now')), last_used_at TEXT, revoked_at TEXT, @@ -92,6 +96,17 @@ def init_db(): CREATE INDEX IF NOT EXISTS idx_tce_user_tool ON tool_call_events(user_id, tool_name); """) db.commit() + _migrate(db) + + +def _migrate(db: sqlite3.Connection) -> None: + """CREATE TABLE IF NOT EXISTS above doesn't alter already-existing tables + (e.g. a production api_keys table created before a new column existed), + so new columns on existing tables are added here, idempotently.""" + cols = {row["name"] for row in db.execute("PRAGMA table_info(api_keys)")} + if "instance_name" not in cols: + db.execute("ALTER TABLE api_keys ADD COLUMN instance_name TEXT") + db.commit() def db_execute(sql: str, params: tuple = ()) -> sqlite3.Cursor: diff --git a/src/mt_odoo_mcp/server.py b/src/mt_odoo_mcp/server.py index 3d26679..d83c638 100644 --- a/src/mt_odoo_mcp/server.py +++ b/src/mt_odoo_mcp/server.py @@ -19,9 +19,11 @@ from mcp.types import Tool, TextContent from .config import settings from .database import init_db -from .auth.service import get_user_by_api_key, get_api_key_id_for_mcp +from .auth.service import get_user_by_api_key, get_api_key_scope_for_mcp from .registry import get_conn_manager, get_confirm_manager -from .context import set_user_context, reset_user_context, get_user_id, get_api_key_id +from .context import ( + set_user_context, reset_user_context, get_user_id, get_api_key_id, get_key_instance_scope, +) from .api.routes import router as api_router from .admin.routes import router as admin_router @@ -592,11 +594,29 @@ async def call_tool(name: str, arguments: dict) -> list[TextContent]: if not entry: return [TextContent(type="text", text=json.dumps({"error": f"Unknown tool: '{name}'"}))] - _, _, fn, _ = entry + _, _, fn, schema = entry instance = arguments.get("instance", "default") started = time.monotonic() started_at_iso = datetime.now(timezone.utc).isoformat() + # If this session's API key is scoped to one Odoo instance, enforce it + # for every tool that takes an `instance` argument: silently default to + # the scoped instance when the caller didn't specify one (or specified + # the schema's literal "default"), or reject outright if they explicitly + # asked for a different instance. Tools with no `instance` param (e.g. + # list_instances) are untouched — nothing to scope there. + key_scope = get_key_instance_scope() + if key_scope and "instance" in (schema.get("properties") or {}): + if "instance" not in arguments or arguments["instance"] == "default": + arguments = {**arguments, "instance": key_scope} + instance = key_scope + elif arguments["instance"] != key_scope: + error_text = ( + f"This API key is scoped to Odoo instance '{key_scope}' and cannot access '{arguments['instance']}'." + ) + _record_tool_call(name, arguments["instance"], started, started_at_iso, False, error_text) + return [TextContent(type="text", text=json.dumps({"error": error_text}))] + # Auto-connect instance (skip for tools that explicitly handle connection errors) from .context import get_conn_manager as _gcm try: @@ -759,7 +779,7 @@ class _MCPEndpoint: resp = _JSON({"error": "Invalid API key."}, status_code=401) await resp(scope, receive, send) return - api_key_id = get_api_key_id_for_mcp(api_key) + api_key_id, key_instance_scope = get_api_key_scope_for_mcp(api_key) # Session ID must be visible ASCII chars; hex satisfies that new_session_id = _secrets.token_hex(32) @@ -774,9 +794,9 @@ class _MCPEndpoint: async def _run_session( _t=transport, _u=user, _cm=conn_mgr, _fm=confirm_mgr, - _ev=ready, _sid=new_session_id, _kid=api_key_id, + _ev=ready, _sid=new_session_id, _kid=api_key_id, _scope=key_instance_scope, ): - tokens = set_user_context(_u.id, _u.email, _cm, _fm, _kid) + tokens = set_user_context(_u.id, _u.email, _cm, _fm, _kid, _scope) try: async with _t.connect() as (read_stream, write_stream): _ev.set()