fix: replace macOS-only security CLI with cross-platform keyring; add KEYCHAIN_SERVICE constant

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
+# ── Proxy-aware XML-RPC transport ─────────────────────────────────────────────
-import urllib.error
-
-
 class ProxyAwareTransport(xmlrpc.client.SafeTransport):
-    """xmlrpc transport that routes through the system HTTPS proxy.
+    """Routes xmlrpc through the system HTTPS proxy (respects HTTPS_PROXY env var)."""
-
-    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).
-    """
-
     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()