From 294c946d2b96ccc54cc7f5f7a0c87cdd88a3149c Mon Sep 17 00:00:00 2001 From: jason Date: Wed, 29 Apr 2026 13:37:40 -0500 Subject: [PATCH] fix: replace macOS-only security CLI with cross-platform keyring; add KEYCHAIN_SERVICE constant --- server/odoo_mcp.py | 138 ++++++++++++++++++++++++++++++++++++++------- 1 file changed, 117 insertions(+), 21 deletions(-) diff --git a/server/odoo_mcp.py b/server/odoo_mcp.py index 3640fba..9c098ea 100644 --- a/server/odoo_mcp.py +++ b/server/odoo_mcp.py @@ -9,29 +9,21 @@ Purchase, Inventory, Employees, and Knowledge Templates. import os import re import xmlrpc.client +import urllib.request +import urllib.error from typing import Optional from mcp.server.fastmcp import FastMCP # ── Configuration ──────────────────────────────────────────────────────────── ODOO_URL = os.environ.get("ODOO_URL", "https://mpmedia.odoo.com") ODOO_DB = os.environ.get("ODOO_DB", "mpmedia-odoo-sh-main-13285275") -ODOO_USERNAME = os.environ.get("ODOO_USERNAME", "bgilliom@mpmedia.tv") +ODOO_USERNAME = os.environ.get("ODOO_USERNAME", "") # per-user: set via Keychain +KEYCHAIN_SERVICE = "odoo-mpm" # credential store service name (all platforms) ODOO_API_KEY = os.environ.get("ODOO_API_KEY", "") -import urllib.request -import urllib.error - - +# ── Proxy-aware XML-RPC transport ───────────────────────────────────────────── class ProxyAwareTransport(xmlrpc.client.SafeTransport): - """xmlrpc transport that routes through the system HTTPS proxy. - - Python's xmlrpc.client.SafeTransport makes raw socket connections and - ignores HTTPS_PROXY / https_proxy environment variables entirely. - This transport replaces the low-level request method with urllib.request, - which correctly picks up the proxy environment variables set by the - Cowork sandbox (HTTPS_PROXY=http://localhost:3128). - """ - + """Routes xmlrpc through the system HTTPS proxy (respects HTTPS_PROXY env var).""" def request(self, host, handler, request_body, verbose=False): url = f"https://{host}{handler}" headers = { @@ -40,37 +32,87 @@ class ProxyAwareTransport(xmlrpc.client.SafeTransport): "User-Agent": "xmlrpc-odoo-mpm/1.0", } req = urllib.request.Request(url, request_body, headers) - # build_opener with ProxyHandler picks up HTTPS_PROXY from environment opener = urllib.request.build_opener(urllib.request.ProxyHandler()) try: with opener.open(req, timeout=30) as resp: + self.verbose = verbose # required by Transport.parse_response return self.parse_response(resp) except urllib.error.HTTPError as e: raise xmlrpc.client.ProtocolError(url, e.code, e.msg, dict(e.headers)) except urllib.error.URLError as e: raise xmlrpc.client.ProtocolError(url, 0, str(e.reason), {}) - _proxy_transport = ProxyAwareTransport() # ── Odoo client ─────────────────────────────────────────────────────────────── _uid: Optional[int] = None _models = None +_resolved_api_key: Optional[str] = None # resolved at connect time from env or Keychain + +def _keychain_get(account: str) -> str: + """Read a credential from the system keystore (keyring). + Works on macOS (Keychain), Windows (Credential Manager), and Linux (Secret Service).""" + try: + import keyring + value = keyring.get_password(KEYCHAIN_SERVICE, account) + return value or "" + except Exception: + return "" + +def _keychain_set(account: str, value: str) -> None: + """Store a credential in the system keystore (keyring).""" + import keyring + keyring.set_password(KEYCHAIN_SERVICE, account, value) + +def _keychain_delete(account: str) -> bool: + """Delete a credential from the system keystore. Returns True if it existed.""" + try: + import keyring + existing = keyring.get_password(KEYCHAIN_SERVICE, account) + if existing is not None: + keyring.delete_password(KEYCHAIN_SERVICE, account) + return True + return False + except Exception: + return False + +def _get_credentials() -> tuple[str, str]: + """Resolve username and API key: env vars take priority, then system keystore.""" + username = ODOO_USERNAME or _keychain_get("odoo_username") + api_key = ODOO_API_KEY or _keychain_get("odoo_api_key") + return username, api_key + +def _get_credentials() -> tuple[str, str]: + """Resolve username and API key: env vars take priority, then macOS Keychain.""" + username = ODOO_USERNAME or _keychain_get("odoo_username") + api_key = ODOO_API_KEY or _keychain_get("odoo_api_key") + return username, api_key def _connect(): - global _uid, _models + global _uid, _models, _resolved_api_key if _uid is not None: return + username, api_key = _get_credentials() + if not username or not api_key: + raise RuntimeError( + "Odoo credentials not configured. " + "Run the setup_odoo_credentials tool with your Odoo login email and API key. " + "See the Odoo skill instructions for how to generate your personal API key." + ) common = xmlrpc.client.ServerProxy(f"{ODOO_URL}/xmlrpc/2/common", transport=_proxy_transport) - _uid = common.authenticate(ODOO_DB, ODOO_USERNAME, ODOO_API_KEY, {}) + _uid = common.authenticate(ODOO_DB, username, api_key, {}) if not _uid: - raise RuntimeError("Odoo authentication failed. Check ODOO_USERNAME and ODOO_API_KEY.") + raise RuntimeError( + "Odoo authentication failed. " + "Verify your username and API key, then run setup_odoo_credentials again." + ) + _resolved_api_key = api_key _models = xmlrpc.client.ServerProxy(f"{ODOO_URL}/xmlrpc/2/object", transport=_proxy_transport) def _call(model: str, method: str, args=None, kwargs=None): _connect() return _models.execute_kw( - ODOO_DB, _uid, ODOO_API_KEY, + ODOO_DB, _uid, _resolved_api_key, model, method, args or [[]], kwargs or {} @@ -100,6 +142,60 @@ def _write(model, ids, vals): mcp = FastMCP("Odoo MPM") +# ════════════════════════════════════════════════════════════════════════════ +# CREDENTIALS SETUP +# ════════════════════════════════════════════════════════════════════════════ + +@mcp.tool() +def setup_odoo_credentials(username: str, api_key: str) -> str: + """Store your personal Odoo credentials securely in the system keystore. + This only needs to be done once per machine (macOS Keychain, Windows Credential Manager, or Linux Secret Service). + + username : your Odoo login email (e.g. you@mpmedia.tv) + api_key : your personal Odoo API key — generate it in Odoo at: + My Profile → Account Security → API Keys → New API Key + Set the expiration to "No Limit" (indefinite). + Never share this key or use a colleague's key. + + Credentials are stored in the OS keystore under the service 'odoo-mpm' + and are never written to any file on disk.""" + _keychain_set("odoo_username", username.strip()) + _keychain_set("odoo_api_key", api_key.strip()) + # Reset any cached connection so next call re-authenticates with new credentials + global _uid, _models, _resolved_api_key + _uid = None + _models = None + _resolved_api_key = None + # Verify immediately + try: + _connect() + return ( + f"Credentials saved and verified. " + f"Connected to {ODOO_URL} as UID {_uid}. " + f"You're all set — Odoo tools are ready to use." + ) + except Exception as e: + return f"Credentials saved to Keychain but authentication failed: {e}" + +@mcp.tool() +def clear_odoo_credentials() -> str: + """Remove your stored Odoo credentials from the system keystore. + Use this if you are offboarding, rotating your API key, or troubleshooting + an authentication problem. You will need to run setup_odoo_credentials + again before using any Odoo tools.""" + removed = [] + for key in ("odoo_username", "odoo_api_key"): + if _keychain_delete(key): + removed.append(key) + global _uid, _models, _resolved_api_key + _uid = None + _models = None + _resolved_api_key = None + if removed: + return f"Removed {', '.join(removed)} from system keystore. Run setup_odoo_credentials to reconfigure." + return "No stored credentials found in system keystore." + + # ════════════════════════════════════════════════════════════════════════════ # PRODUCTS # ════════════════════════════════════════════════════════════════════════════ @@ -644,7 +740,7 @@ def get_employee(employee_id: int) -> dict: ["id", "name", "job_title", "job_id", "department_id", "work_email", "work_phone", "mobile_phone", "parent_id", "coach_id", "address_id", "resource_calendar_id", "tz", - "birthday", "gender", "marital", "country_id"]) + "birthday", "marital", "country_id"]) return r[0] if r else {} @mcp.tool()