From 3c9757ffdecbbdbb0b1ed9cf8c09f88cca9e56ef Mon Sep 17 00:00:00 2001 From: Pa04rth Date: Fri, 2 Oct 2026 12:25:37 +0530 Subject: [PATCH 1/2] MCP V2: main code files --- .gitignore | 6 + application/database/db.py | 87 ++++++ application/mcp/__main__.py | 28 +- application/mcp/auth.py | 196 ++++++++++++ application/mcp/catalog.py | 50 ++- application/mcp/cli.py | 66 ++++ application/mcp/openapi_loader.py | 35 ++- application/mcp/rest_client.py | 115 ++++++- application/mcp/server.py | 13 +- application/mcp/token_store.py | 106 +++++++ application/tests/conftest.py | 35 +++ application/tests/mcp_auth_test.py | 135 +++++++++ application/tests/mcp_grants_test.py | 285 ++++++++++++++++++ application/tests/web_identity_test.py | 245 +++++++++++++++ application/web/identity.py | 179 +++++++++++ application/web/openapi_registry.py | 89 ++++++ application/web/web_main.py | 67 +++- docs/api/mcp.md | 96 +++++- docs/api/openapi.yaml | 109 +++++++ .../versions/f3a91c07d24b_add_mcp_grants.py | 40 +++ requirements-dev.txt | 1 + scripts/device_flow_check.py | 124 ++++++++ 22 files changed, 2068 insertions(+), 39 deletions(-) create mode 100644 application/mcp/auth.py create mode 100644 application/mcp/cli.py create mode 100644 application/mcp/token_store.py create mode 100644 application/tests/conftest.py create mode 100644 application/tests/mcp_auth_test.py create mode 100644 application/tests/mcp_grants_test.py create mode 100644 application/tests/web_identity_test.py create mode 100644 application/web/identity.py create mode 100644 migrations/versions/f3a91c07d24b_add_mcp_grants.py create mode 100644 scripts/device_flow_check.py diff --git a/.gitignore b/.gitignore index bfd3699d3..df9752a0d 100644 --- a/.gitignore +++ b/.gitignore @@ -97,3 +97,9 @@ cres/* project management scripts/ .harvester_cache/ + +### Local MCP client config (machine-specific paths) +.mcp.json + +### MCP v2 local dev credentials (never commit) +.mcp_tokens.json diff --git a/application/database/db.py b/application/database/db.py index 18e5d2bf0..22e7358be 100644 --- a/application/database/db.py +++ b/application/database/db.py @@ -306,6 +306,27 @@ class UserResourceSelection(BaseModel): # type: ignore ) +class McpGrants(BaseModel): # type: ignore + """One OpenCRE scope granted to a user's MCP client. + + Google's device flow only carries ``openid email profile``, so OpenCRE + permissions cannot live in the token and are recorded here instead. + """ + + __tablename__ = "mcp_grants" + id = sqla.Column(sqla.String, primary_key=True, default=generate_uuid) + user_id = sqla.Column( + sqla.String, + sqla.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ) + scope = sqla.Column(sqla.String, nullable=False) + created_at = sqla.Column(sqla.DateTime, nullable=False) + revoked_at = sqla.Column(sqla.DateTime, nullable=True) + + __table_args__ = (sqla.UniqueConstraint(user_id, scope, name="uq_mcp_grants"),) + + class ArtifactIngestEvent(BaseModel): # type: ignore """Tracks one harvested artifact persisted per import run.""" @@ -1393,6 +1414,72 @@ def _apply_profile(u: User) -> None: def get_user_by_sub(self, google_sub: str) -> Optional[User]: return self.session.query(User).filter(User.google_sub == google_sub).first() + def get_mcp_grants(self, user_id: str) -> List[str]: + """Return the scopes granted to this user's MCP client, newest last.""" + rows = ( + self.session.query(McpGrants) + .filter(McpGrants.user_id == user_id) + .filter(McpGrants.revoked_at.is_(None)) + .order_by(McpGrants.scope) + .all() + ) + return [row.scope for row in rows] + + def add_mcp_grant(self, user_id: str, scope: str) -> List[str]: + """Grant ``scope`` to this user's MCP client, reviving a revoked row.""" + from datetime import datetime, timezone + + now = datetime.now(timezone.utc) + existing = ( + self.session.query(McpGrants) + .filter(McpGrants.user_id == user_id) + .filter(McpGrants.scope == scope) + .first() + ) + if existing is None: + self.session.add(McpGrants(user_id=user_id, scope=scope, created_at=now)) + else: + existing.revoked_at = None + self.session.commit() + return self.get_mcp_grants(user_id) + + def bootstrap_mcp_grants(self, user_id: str, scopes: List[str]) -> List[str]: + """Grant ``scopes`` the first time an MCP client is seen for a user. + + Applies only when the user has no rows at all. Someone who revoked a + scope must never have it silently restored, so a revoked row counts as + history and suppresses the bootstrap. + """ + from datetime import datetime, timezone + + seen = ( + self.session.query(McpGrants).filter(McpGrants.user_id == user_id).count() + ) + if seen: + return self.get_mcp_grants(user_id) + + now = datetime.now(timezone.utc) + for scope in scopes: + self.session.add(McpGrants(user_id=user_id, scope=scope, created_at=now)) + try: + self.session.commit() + except IntegrityError: + # A concurrent first call inserted the same rows; theirs stand. + self.session.rollback() + return self.get_mcp_grants(user_id) + + def revoke_mcp_grant(self, user_id: str, scope: str) -> List[str]: + """Revoke ``scope`` without deleting the row, so the change is auditable.""" + from datetime import datetime, timezone + + self.session.query(McpGrants).filter(McpGrants.user_id == user_id).filter( + McpGrants.scope == scope + ).filter(McpGrants.revoked_at.is_(None)).update( + {"revoked_at": datetime.now(timezone.utc)} + ) + self.session.commit() + return self.get_mcp_grants(user_id) + def get_user_resource_selection(self, user_id: str) -> List[str]: """Return the standard names a user has selected, ordered by name.""" rows = ( diff --git a/application/mcp/__main__.py b/application/mcp/__main__.py index cd32b7d3e..e5afc2b69 100644 --- a/application/mcp/__main__.py +++ b/application/mcp/__main__.py @@ -1,4 +1,4 @@ -"""Entry point: python -m application.mcp""" +"""Entry point: python -m application.mcp [login|logout|status]""" from __future__ import annotations @@ -7,9 +7,35 @@ logger = get_logger(__name__) import asyncio +import os +import pathlib +import sys + + +def _load_repo_env() -> None: + """Fall back to the repo's .env for anything the MCP client did not set. + + MCP clients cannot set a working directory, so the path is resolved from + this file rather than the cwd. Values already present in the environment + win, keeping the client config authoritative. + """ + try: + from dotenv import load_dotenv + except ImportError: + return + env_file = pathlib.Path(__file__).resolve().parents[2] / ".env" + if env_file.is_file(): + load_dotenv(env_file, override=False) def main() -> None: + _load_repo_env() + command = sys.argv[1:] + if command: + from application.mcp.cli import parse_and_run_command + + raise SystemExit(parse_and_run_command(command)) + logger.info("starting OpenCRE MCP stdio server") from application.mcp.server import run_stdio diff --git a/application/mcp/auth.py b/application/mcp/auth.py new file mode 100644 index 000000000..4a5af8990 --- /dev/null +++ b/application/mcp/auth.py @@ -0,0 +1,196 @@ +"""Google device-flow credentials for the MCP client. + +The only module that knows Google exists. Callers ask for an id_token and get +one, refreshed silently when needed. Never writes to stdout: that stream +carries JSON-RPC when the stdio server is running. +""" + +from __future__ import annotations + +from cre_logging import get_logger + +logger = get_logger(__name__) + +import base64 +import json +import os +import time +from typing import Any, Callable, Dict, Optional, Tuple + +import requests + +from application.mcp import token_store + +DEVICE_CODE_URL = "https://oauth2.googleapis.com/device/code" +TOKEN_URL = "https://oauth2.googleapis.com/token" +DEVICE_GRANT = "urn:ietf:params:oauth:grant-type:device_code" + +# Device flow supports only this set, and staying inside it keeps refresh +# tokens exempt from the 7-day expiry Google applies to Testing-mode apps. +SCOPES = "openid email profile" + +REQUEST_TIMEOUT_SECONDS = 30.0 +EXPIRY_SKEW_SECONDS = 60 + +_cached_id_token: Optional[str] = None + + +class AuthError(Exception): + """Base error for MCP credential failures.""" + + +class NotAuthenticated(AuthError): + """No usable credential. The caller should run the login command.""" + + +def _client_credentials() -> Tuple[str, str]: + """Read client config lazily so importing this module never fails.""" + client_id = os.environ.get("GOOGLE_MCP_CLIENT_ID") + client_secret = os.environ.get("GOOGLE_MCP_CLIENT_SECRET") + if not client_id or not client_secret: + raise AuthError("GOOGLE_MCP_CLIENT_ID / GOOGLE_MCP_CLIENT_SECRET are not set.") + return client_id, client_secret + + +def _decode_claims(id_token: str) -> Dict[str, Any]: + """Read claims without verifying. Display only; REST verifies signatures.""" + try: + payload = id_token.split(".")[1] + raw = base64.urlsafe_b64decode(payload + "=" * (-len(payload) % 4)) + return json.loads(raw) + except (IndexError, ValueError): + return {} + + +def _is_fresh(id_token: str) -> bool: + exp = _decode_claims(id_token).get("exp") + if not isinstance(exp, (int, float)): + return False + return time.time() < exp - EXPIRY_SKEW_SECONDS + + +def _refresh(refresh_token: str) -> Optional[Dict[str, Any]]: + """Trade a refresh token for fresh tokens. None if it is no longer valid.""" + client_id, client_secret = _client_credentials() + response = requests.post( + TOKEN_URL, + data={ + "client_id": client_id, + "client_secret": client_secret, + "refresh_token": refresh_token, + "grant_type": "refresh_token", + }, + timeout=REQUEST_TIMEOUT_SECONDS, + ) + if response.status_code == 200: + return response.json() + logger.info("refresh token rejected with status %s", response.status_code) + return None + + +def login(on_code: Callable[[str, str], None]) -> Dict[str, Any]: + """Run the device flow. `on_code(verification_url, user_code)` displays the + prompt before polling begins. Returns the id_token claims on success.""" + client_id, client_secret = _client_credentials() + + response = requests.post( + DEVICE_CODE_URL, + data={"client_id": client_id, "scope": SCOPES}, + timeout=REQUEST_TIMEOUT_SECONDS, + ) + if response.status_code != 200: + raise AuthError(f"device/code failed [{response.status_code}]: {response.text}") + device = response.json() + + # Google spells it verification_url; RFC 8628 says verification_uri. + url = device.get("verification_url") or device.get("verification_uri") + on_code(url, device["user_code"]) + + interval = device.get("interval", 5) + deadline = time.time() + device["expires_in"] + while time.time() < deadline: + time.sleep(interval) + polled = requests.post( + TOKEN_URL, + data={ + "client_id": client_id, + "client_secret": client_secret, + "device_code": device["device_code"], + "grant_type": DEVICE_GRANT, + }, + timeout=REQUEST_TIMEOUT_SECONDS, + ) + body = polled.json() + if polled.status_code == 200: + return _store(body) + error = body.get("error") + if error == "authorization_pending": + continue + if error == "slow_down": + interval += 5 + continue + raise AuthError(f"authorization failed: {error or body}") + raise AuthError("timed out: the code expired before it was approved") + + +def _store(tokens: Dict[str, Any]) -> Dict[str, Any]: + """Persist the refresh token and cache the id_token. Returns its claims.""" + global _cached_id_token + + # A refresh response omits refresh_token; keep the one already held. + refresh_token = tokens.get("refresh_token") or token_store.load() + if refresh_token: + token_store.save(refresh_token) + + _cached_id_token = tokens.get("id_token") + return _decode_claims(_cached_id_token) if _cached_id_token else {} + + +def current_id_token() -> str: + """A valid id_token, refreshing if needed.""" + global _cached_id_token + + if _cached_id_token and _is_fresh(_cached_id_token): + return _cached_id_token + + refresh_token = token_store.load() + if not refresh_token: + raise NotAuthenticated("Not logged in. Run: python -m application.mcp login") + + tokens = _refresh(refresh_token) + if tokens is None: + _cached_id_token = None + raise NotAuthenticated( + "Stored credential is no longer valid. " + "Run: python -m application.mcp login" + ) + _store(tokens) + if not _cached_id_token: + raise AuthError("token endpoint returned no id_token") + return _cached_id_token + + +def logout() -> bool: + global _cached_id_token + + _cached_id_token = None + return token_store.clear() + + +def status() -> Dict[str, Any]: + """Describe the stored credential without raising when absent.""" + client_id = os.environ.get("GOOGLE_MCP_CLIENT_ID") + if not token_store.load(): + return {"logged_in": False, "client_id": client_id} + try: + claims = _decode_claims(current_id_token()) + except AuthError as exc: + return {"logged_in": False, "client_id": client_id, "error": str(exc)} + return { + "logged_in": True, + "client_id": client_id, + "email": claims.get("email"), + "sub": claims.get("sub"), + "audience": claims.get("aud"), + "expires_in": int(claims.get("exp", 0) - time.time()), + } diff --git a/application/mcp/catalog.py b/application/mcp/catalog.py index ebbf04273..4e079d677 100644 --- a/application/mcp/catalog.py +++ b/application/mcp/catalog.py @@ -1,4 +1,4 @@ -"""Static security allowlist for OpenCRE MCP v1 public-read tools. +"""Static security allowlist for OpenCRE MCP tools. Exposure is determined only by this allowlist — never by enumerating OpenAPI. """ @@ -9,18 +9,27 @@ logger = get_logger(__name__) -from dataclasses import dataclass +import os +from dataclasses import dataclass, field from typing import Dict, List, Tuple +# Authenticated tools stay hidden unless a deployment opts in, so the public +# surface is identical whether or not anyone has logged in on this machine. +AUTH_TOOLS_ENV = "OPENCRE_MCP_AUTH_TOOLS" +_TRUE_VALUES = {"1", "true", "yes"} + @dataclass(frozen=True) class ToolSpec: - """One MCP tool bound to a fixed public REST GET operation.""" + """One MCP tool bound to a fixed REST operation.""" name: str method: str path_template: str summary: str + auth: bool = False + read_only: bool = True + scopes: Tuple[str, ...] = field(default=()) @property def openapi_identity(self) -> Tuple[str, str]: @@ -86,7 +95,33 @@ def openapi_identity(self) -> Tuple[str, str]: ), ) -_TOOLS_BY_NAME: Dict[str, ToolSpec] = {tool.name: tool for tool in PUBLIC_TOOLS} +AUTHENTICATED_TOOLS: Tuple[ToolSpec, ...] = ( + ToolSpec( + name="get_my_resources", + method="GET", + path_template="/rest/v1/user/resources", + summary="Get your saved standards selection", + auth=True, + scopes=("myopencre:read",), + ), + ToolSpec( + name="set_my_resources", + method="PUT", + path_template="/rest/v1/user/resources", + summary="Replace your saved standards selection", + auth=True, + read_only=False, + scopes=("myopencre:write",), + ), +) + +_TOOLS_BY_NAME: Dict[str, ToolSpec] = { + tool.name: tool for tool in PUBLIC_TOOLS + AUTHENTICATED_TOOLS +} + + +def auth_tools_enabled() -> bool: + return os.environ.get(AUTH_TOOLS_ENV, "").strip().lower() in _TRUE_VALUES def list_tool_names() -> List[str]: @@ -98,3 +133,10 @@ def get_tool(name: str) -> ToolSpec: return _TOOLS_BY_NAME[name] except KeyError as exc: raise KeyError(f"Unknown MCP tool: {name}") from exc + + +def active_tools() -> Tuple[ToolSpec, ...]: + """Tools this process serves: public always, authenticated on opt-in.""" + if auth_tools_enabled(): + return PUBLIC_TOOLS + AUTHENTICATED_TOOLS + return PUBLIC_TOOLS diff --git a/application/mcp/cli.py b/application/mcp/cli.py new file mode 100644 index 000000000..794d44ec8 --- /dev/null +++ b/application/mcp/cli.py @@ -0,0 +1,66 @@ +"""Human-facing commands for the MCP client: login, logout, status. + +Owns all printing. Everything is written to stderr so stdout stays clean for +JSON-RPC when the stdio server runs. +""" + +from __future__ import annotations + +from cre_logging import get_logger + +logger = get_logger(__name__) + +import sys +from typing import List + +from application.mcp import auth + +COMMANDS = ("login", "logout", "status") + + +def _say(message: str = "") -> None: + print(message, file=sys.stderr) + + +def _login() -> int: + def show(url: str, code: str) -> None: + _say(f"\n Open: {url}\n Code: {code}\n") + _say(" Waiting for approval...") + + try: + claims = auth.login(show) + except auth.AuthError as exc: + _say(f"Login failed: {exc}") + return 1 + _say(f"Logged in as {claims.get('email') or claims.get('sub')}") + return 0 + + +def _logout() -> int: + if auth.logout(): + _say("Logged out.") + else: + _say("No stored credential.") + return 0 + + +def _status() -> int: + info = auth.status() + if not info["logged_in"]: + _say("Not logged in.") + if info.get("error"): + _say(f" {info['error']}") + return 1 + _say(f"Logged in as : {info.get('email')}") + _say(f"Subject : {info.get('sub')}") + _say(f"Audience : {info.get('audience')}") + _say(f"Token expires: {info.get('expires_in')}s") + return 0 + + +def parse_and_run_command(command: List[str]) -> int: + handlers = {"login": _login, "logout": _logout, "status": _status} + if len(command) != 1 or command[0] not in handlers: + _say(f"Usage: python -m application.mcp [{'|'.join(COMMANDS)}]") + return 2 + return handlers[command[0]]() diff --git a/application/mcp/openapi_loader.py b/application/mcp/openapi_loader.py index c881f6d94..27e785b44 100644 --- a/application/mcp/openapi_loader.py +++ b/application/mcp/openapi_loader.py @@ -13,7 +13,7 @@ import copy import os from functools import lru_cache -from typing import Any, Dict, List, Optional, Set +from typing import Any, Dict, Iterable, List, Optional, Set import yaml @@ -22,6 +22,9 @@ # Response-format switchers that can leave JSON; omit from MCP v1 inputs. _JSON_UNSAFE_QUERY_PARAMS: Set[str] = {"format"} +# Argument name carrying an operation's JSON request body. +BODY_ARG = "body" + REPO_ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), "..", "..")) DEFAULT_OPENAPI_PATH = os.path.join(REPO_ROOT, "docs", "api", "openapi.yaml") @@ -132,6 +135,27 @@ def operation_input_schema( if param.get("required"): required.append(name) + request_body = _resolve_ref(document, operation.get("requestBody")) + if isinstance(request_body, dict): + content = request_body.get("content") or {} + json_content = content.get("application/json") + + if isinstance(json_content, dict): + body_schema = json_content.get("schema") + if body_schema is not None: + resolved_body_schema = _resolve_ref(document, body_schema) + + if not isinstance(resolved_body_schema, dict): + raise OpenAPILookupError( + f"Request body schema must be an object on " + f"{method.upper()} {path}" + ) + + properties[BODY_ARG] = copy.deepcopy(resolved_body_schema) + + if request_body.get("required"): + required.append(BODY_ARG) + schema: Dict[str, Any] = { "type": "object", "properties": properties, @@ -156,9 +180,14 @@ def input_schema_for_tool(name: str) -> Dict[str, Any]: def all_tool_input_schemas() -> Dict[str, Dict[str, Any]]: - """Resolve input schemas for every allowlisted tool (fails if OpenAPI drifts).""" + """Resolve input schemas for every public tool (fails if OpenAPI drifts).""" + return input_schemas_for(PUBLIC_TOOLS) + + +def input_schemas_for(tools: Iterable[ToolSpec]) -> Dict[str, Dict[str, Any]]: + """Resolve input schemas for the given tools against one loaded spec.""" spec = load_openapi_spec() - return {tool.name: operation_input_schema(tool, spec=spec) for tool in PUBLIC_TOOLS} + return {tool.name: operation_input_schema(tool, spec=spec) for tool in tools} def openapi_has_operation(method: str, path: str) -> bool: diff --git a/application/mcp/rest_client.py b/application/mcp/rest_client.py index 5826d7075..1e9cc4b3f 100644 --- a/application/mcp/rest_client.py +++ b/application/mcp/rest_client.py @@ -14,7 +14,16 @@ import os import re from dataclasses import dataclass -from typing import Any, Dict, Mapping, MutableMapping, Optional, Sequence, Set +from typing import ( + Any, + Dict, + Mapping, + MutableMapping, + Optional, + Sequence, + Set, + Tuple, +) from urllib.parse import quote, urljoin import requests @@ -23,12 +32,15 @@ from application.defs import cre_defs as defs from application.mcp.catalog import ToolSpec, get_tool -from application.mcp.openapi_loader import operation_input_schema +from application.mcp.openapi_loader import BODY_ARG, operation_input_schema DEFAULT_BASE_URL = "http://127.0.0.1:5000" DEFAULT_TIMEOUT_SECONDS = 30.0 +# Writes are allowed only for tools whose ToolSpec declares them. +_ALLOWED_METHODS = frozenset({"GET", "PUT", "POST"}) + # Path params must be single URL segments — no separators that alter routing. _UNSAFE_PATH_PARAM = re.compile(r"[/?#]") # CRE names may contain "/" ("MFA/OTP"); REST serves them via a path: converter. @@ -104,7 +116,7 @@ def _new_session() -> requests.Session: class RestClient: - """Execute allowlisted GET templates against OPENCRE_BASE_URL.""" + """Execute allowlisted GET , POST and PUT templates against OPENCRE_BASE_URL.""" def __init__( self, @@ -132,8 +144,9 @@ def call_tool( return self.call_spec(tool, arguments or {}) def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult: - if tool.method.upper() != "GET": - raise RestRequestError(f"Only GET is supported (got {tool.method})") + method = tool.method.upper() + if method not in _ALLOWED_METHODS: + raise RestRequestError(f"Method not allowed for {tool.name}: {tool.method}") schema = operation_input_schema(tool) path_param_names = _path_param_names(tool.path_template) @@ -154,6 +167,8 @@ def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult: # Enforce the exact OpenAPI-derived JSON Schema (types, enums, etc.). _validate_against_input_schema(tool.name, schema, args) + body = args.pop(BODY_ARG, None) + path_params: Dict[str, str] = {} query_params: Dict[str, Any] = {} for key, value in args.items(): @@ -171,12 +186,10 @@ def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult: _validate_node_type_param(path_params) path = _render_path(tool.path_template, path_params) url = urljoin(self._base_url + "/", path.lstrip("/")) - response = self._session.get( - url, - params=_encode_query(query_params), - timeout=self._timeout, - allow_redirects=False, - ) + + # Credentials come from the environment only, never from arguments. + headers, token = self._auth_headers(tool) + response = self._send(method, url, query_params, body, headers) # Never retain cookies across tool calls (no credential forwarding). if hasattr(self._session, "cookies"): try: @@ -188,11 +201,65 @@ def call_spec(self, tool: ToolSpec, arguments: Mapping[str, Any]) -> RestResult: "Failed to clear REST session cookies after tool call", exc_info=True, ) - return _parse_response(response) + try: + return _parse_response(response) + except RestResponseError as exc: + raise RestResponseError( + exc.status_code, _scrub(str(exc), token), body=exc.body + ) from None + + def _send( + self, + method: str, + url: str, + query_params: Mapping[str, Any], + body: Any, + headers: Mapping[str, str], + ) -> Any: + """Dispatch the request, tolerating sessions that only implement get().""" + request = getattr(self._session, "request", None) + if request is None: + if method != "GET": + raise RestClientError( + f"Injected session cannot perform {method} requests" + ) + return self._session.get( + url, + params=_encode_query(query_params), + timeout=self._timeout, + allow_redirects=False, + ) + return request( + method, + url, + params=_encode_query(query_params), + json=body, + headers=dict(headers), + timeout=self._timeout, + allow_redirects=False, + ) + + def _auth_headers(self, tool: ToolSpec) -> Tuple[Dict[str, str], Optional[str]]: + """Bearer header for authenticated tools, plus the token to redact.""" + if not getattr(tool, "auth", False): + return {}, None + + from application.mcp import auth + + try: + token = auth.current_id_token() + except auth.AuthError as exc: + raise RestRequestError(f"{tool.name}: {exc}") from None + return {"Authorization": f"Bearer {token}"}, token + + +def _scrub(text: str, token: Optional[str]) -> str: + """Keep credentials out of anything that can reach an MCP client.""" + return text.replace(token, "") if token else text def flask_test_session(flask_client: Any) -> "_FlaskTestSession": - """Adapt Flask's test_client to the requests Session.get interface.""" + """Adapt Flask's test_client to the requests Session interface.""" return _FlaskTestSession(flask_client) @@ -220,6 +287,19 @@ def get( params: Optional[Any] = None, timeout: Optional[float] = None, allow_redirects: bool = False, + **kwargs: Any, + ) -> _FlaskTestResponse: + return self.request("GET", url, params=params, **kwargs) + + def request( + self, + method: str, + url: str, + params: Optional[Any] = None, + json: Optional[Any] = None, + headers: Optional[Mapping[str, str]] = None, + timeout: Optional[float] = None, + allow_redirects: bool = False, **_kwargs: Any, ) -> _FlaskTestResponse: del timeout, allow_redirects # parity with requests; unused by test client @@ -227,7 +307,14 @@ def get( parsed = urlparse(url) path = parsed.path or "/" - response = self._client.get(path, query_string=params) + kwargs: Dict[str, Any] = { + "method": method.upper(), + "query_string": params, + "headers": dict(headers or {}), + } + if json is not None: + kwargs["json"] = json + response = self._client.open(path, **kwargs) return _FlaskTestResponse(response) diff --git a/application/mcp/server.py b/application/mcp/server.py index 15b529527..c9630f242 100644 --- a/application/mcp/server.py +++ b/application/mcp/server.py @@ -13,8 +13,8 @@ from mcp.server.lowlevel import Server from mcp.server.stdio import stdio_server -from application.mcp.catalog import PUBLIC_TOOLS -from application.mcp.openapi_loader import all_tool_input_schemas +from application.mcp.catalog import active_tools +from application.mcp.openapi_loader import input_schemas_for from application.mcp.rest_client import ( RestClient, RestClientError, @@ -34,8 +34,9 @@ def build_server(rest_client: Optional[RestClient] = None) -> Server[Any]: """Create an MCP Server with OpenAPI-derived tool schemas and REST dispatch.""" client = rest_client or RestClient() + served = active_tools() # Resolve schemas once at startup so OpenAPI drift fails before serving. - schemas = all_tool_input_schemas() + schemas = input_schemas_for(served) async def on_list_tools( ctx: Any, params: types.PaginatedRequestParams | None @@ -47,13 +48,13 @@ async def on_list_tools( description=tool.summary, input_schema=schemas[tool.name], annotations=types.ToolAnnotations( - read_only_hint=True, - destructive_hint=False, + read_only_hint=tool.read_only, + destructive_hint=not tool.read_only, idempotent_hint=True, open_world_hint=True, ), ) - for tool in PUBLIC_TOOLS + for tool in served ] return types.ListToolsResult(tools=tools) diff --git a/application/mcp/token_store.py b/application/mcp/token_store.py new file mode 100644 index 000000000..c922dbe52 --- /dev/null +++ b/application/mcp/token_store.py @@ -0,0 +1,106 @@ +"""Local storage for the MCP refresh token. + +OS keyring when one is usable, otherwise a 0600 file. Never writes to stdout: +that stream carries JSON-RPC when the stdio server is running. +""" + +from __future__ import annotations + +from cre_logging import get_logger + +logger = get_logger(__name__) + +import json +import os +import pathlib +import time +from typing import Any, Optional + +SERVICE_NAME = "opencre-mcp" +ACCOUNT_NAME = "refresh_token" + +DEFAULT_TOKEN_FILE = pathlib.Path.home() / ".config" / "opencre-mcp" / "tokens.json" + + +def token_file() -> pathlib.Path: + override = os.environ.get("OPENCRE_MCP_TOKEN_FILE") + return pathlib.Path(override) if override else DEFAULT_TOKEN_FILE + + +def _keyring() -> Optional[Any]: + """Return a working keyring module, or None to use the file backend.""" + if os.environ.get("OPENCRE_MCP_TOKEN_FILE"): + return None + try: + import keyring + from keyring.errors import KeyringError + except ImportError: + return None + try: + backend = keyring.get_keyring() + if backend is None or "fail" in type(backend).__name__.lower(): + return None + except Exception: + logger.debug("keyring unavailable, using file backend", exc_info=True) + return None + return keyring + + +def save(refresh_token: str) -> None: + ring = _keyring() + if ring is not None: + try: + ring.set_password(SERVICE_NAME, ACCOUNT_NAME, refresh_token) + return + except Exception: + logger.debug("keyring write failed, using file backend", exc_info=True) + + path = token_file() + path.parent.mkdir(parents=True, exist_ok=True) + path.write_text( + json.dumps({"refresh_token": refresh_token, "saved_at": time.time()}) + ) + try: + os.chmod(path, 0o600) + except OSError: + logger.debug("could not chmod token file", exc_info=True) + + +def load() -> Optional[str]: + ring = _keyring() + if ring is not None: + try: + stored = ring.get_password(SERVICE_NAME, ACCOUNT_NAME) + if stored: + return stored + except Exception: + logger.debug("keyring read failed, using file backend", exc_info=True) + + path = token_file() + if not path.exists(): + return None + try: + return json.loads(path.read_text()).get("refresh_token") or None + except (ValueError, OSError): + logger.debug("token file unreadable", exc_info=True) + return None + + +def clear() -> bool: + """Remove the stored token. True if anything was removed.""" + removed = False + + ring = _keyring() + if ring is not None: + try: + if ring.get_password(SERVICE_NAME, ACCOUNT_NAME): + ring.delete_password(SERVICE_NAME, ACCOUNT_NAME) + removed = True + except Exception: + logger.debug("keyring delete failed", exc_info=True) + + path = token_file() + if path.exists(): + path.unlink() + removed = True + return removed diff --git a/application/tests/conftest.py b/application/tests/conftest.py new file mode 100644 index 000000000..bbc65eeae --- /dev/null +++ b/application/tests/conftest.py @@ -0,0 +1,35 @@ +"""Shared pytest setup for the OpenCRE test suite. + +``application.feature_flags`` calls ``load_dotenv()`` at import, so a +contributor's local ``.env`` lands in ``os.environ`` for the whole test run. +Values that change what the code under test does must not leak in that way: +they make results depend on whose machine the suite runs on, and a stray +``NO_LOGIN`` would make every authorization test pass without testing anything. +""" + +from __future__ import annotations + +import os + +import pytest + +# Cleared before every test. Any test that needs one sets it explicitly. +AMBIENT_ENV_VARS = ( + "NO_LOGIN", + "NO_LOAD_GRAPH_DB", + "INSECURE_REQUESTS", + "CRE_ENABLE_LOGIN", + "CRE_ENABLE_MYOPENCRE", + "OPENCRE_MCP_AUTH_TOOLS", + "OPENCRE_MCP_TOKEN_FILE", + "OPENCRE_BASE_URL", + "GOOGLE_CLIENT_ID", + "GOOGLE_MCP_CLIENT_ID", + "GOOGLE_MCP_CLIENT_SECRET", +) + + +@pytest.fixture(autouse=True) +def _isolate_ambient_env(monkeypatch: pytest.MonkeyPatch) -> None: + for name in AMBIENT_ENV_VARS: + monkeypatch.delenv(name, raising=False) diff --git a/application/tests/mcp_auth_test.py b/application/tests/mcp_auth_test.py new file mode 100644 index 000000000..68edc8f3a --- /dev/null +++ b/application/tests/mcp_auth_test.py @@ -0,0 +1,135 @@ +"""Tests for OpenCRE MCP v2 authenticated tools.""" + +from __future__ import annotations + +import os +import unittest +from unittest.mock import patch + +from application.mcp import auth, catalog +from application.mcp.openapi_loader import input_schemas_for +from application.mcp.rest_client import ( + RestClient, + RestRequestError, + RestResponseError, +) + + +class _FakeResponse: + """The three attributes rest_client reads off a response.""" + + def __init__(self, status_code: int, text: str) -> None: + self.status_code = status_code + self.text = text + + def json(self): + raise ValueError("not json") + + +class McpAuthToolsTest(unittest.TestCase): + def test_authenticated_tools_are_hidden_by_default(self) -> None: + with patch.dict(os.environ, {}, clear=False): + os.environ.pop(catalog.AUTH_TOOLS_ENV, None) + served = catalog.active_tools() + self.assertEqual(served, catalog.PUBLIC_TOOLS) + self.assertEqual(len(served), 9) + + def test_authenticated_tools_appear_when_enabled(self) -> None: + with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): + served = catalog.active_tools() + names = [tool.name for tool in served] + self.assertIn("get_my_resources", names) + self.assertIn("set_my_resources", names) + self.assertEqual(len(served), 11) + + def test_no_tool_schema_can_carry_a_credential(self) -> None: + """Credentials come from the environment, never from tool arguments.""" + forbidden = {"token", "authorization", "cookie", "bearer", "password"} + schemas = input_schemas_for(catalog.PUBLIC_TOOLS + catalog.AUTHENTICATED_TOOLS) + + for name, schema in schemas.items(): + self.assertFalse( + schema.get("additionalProperties", True), + f"{name} allows unexpected arguments", + ) + for prop in schema.get("properties") or {}: + self.assertNotIn(prop.lower(), forbidden, f"{name}.{prop}") + + def test_write_tool_is_not_marked_read_only(self) -> None: + """A wrong hint here makes MCP clients auto-approve writes.""" + by_name = {tool.name: tool for tool in catalog.AUTHENTICATED_TOOLS} + self.assertTrue(by_name["get_my_resources"].read_only) + self.assertFalse(by_name["set_my_resources"].read_only) + + def test_authenticated_tool_without_login_makes_no_http_call(self) -> None: + class _NoHttpSession: + cookies = None + + def request(self, *args, **kwargs): + raise AssertionError("HTTP must not be called without a credential") + + client = RestClient(session=_NoHttpSession()) + with patch.object( + auth, "current_id_token", side_effect=auth.NotAuthenticated("run login") + ): + with self.assertRaises(RestRequestError) as ctx: + client.call_tool("get_my_resources", {}) + self.assertIn("run login", str(ctx.exception)) + + def test_token_is_redacted_from_error_text(self) -> None: + """Error text reaches the LLM, so it must never carry the credential.""" + token = "SECRET_ID_TOKEN_DO_NOT_LEAK" + + class _EchoSession: + cookies = None + + def request(self, method, url, **kwargs): + sent = kwargs["headers"]["Authorization"] + return _FakeResponse(500, f"upstream rejected {sent}") + + client = RestClient(session=_EchoSession()) + with patch.object(auth, "current_id_token", return_value=token): + with self.assertRaises(RestResponseError) as ctx: + client.call_tool("get_my_resources", {}) + + message = str(ctx.exception) + self.assertNotIn(token, message) + self.assertIn("", message) + + def test_authenticated_tool_sends_a_bearer_header(self) -> None: + token = "an-id-token" + seen = {} + + class _CapturingSession: + cookies = None + + def request(self, method, url, **kwargs): + seen["method"] = method + seen["url"] = url + seen["headers"] = kwargs["headers"] + return _FakeResponse(200, "[]") + + client = RestClient(session=_CapturingSession()) + with patch.object(auth, "current_id_token", return_value=token): + client.call_tool("get_my_resources", {}) + + self.assertEqual(seen["method"], "GET") + self.assertTrue(seen["url"].endswith("/rest/v1/user/resources")) + self.assertEqual(seen["headers"]["Authorization"], f"Bearer {token}") + + def test_public_tool_sends_no_credential(self) -> None: + seen = {} + + class _CapturingSession: + cookies = None + + def request(self, method, url, **kwargs): + seen["headers"] = kwargs["headers"] + return _FakeResponse(200, "[]") + + RestClient(session=_CapturingSession()).call_tool("list_standards", {}) + self.assertNotIn("Authorization", seen["headers"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/mcp_grants_test.py b/application/tests/mcp_grants_test.py new file mode 100644 index 000000000..7c772292e --- /dev/null +++ b/application/tests/mcp_grants_test.py @@ -0,0 +1,285 @@ +"""Tests for MCP grants (issue #1003 v2). + +Google's device flow only carries ``openid email profile``, so OpenCRE scopes +live in the ``mcp_grants`` table. These cover the storage helpers, the +management endpoints, and the scope check on MyOpenCRE. +""" + +from __future__ import annotations + +import json +import os +import unittest +from typing import Any, Dict +from unittest.mock import patch + +from application import create_app, sqla +from application.database import db +from application.web import identity + +WEB_CLIENT_ID = "web-client.apps.googleusercontent.com" +MCP_CLIENT_ID = "mcp-client.apps.googleusercontent.com" +GOOGLE_SUB = "1076915035000615071" +GRANTS_URL = "/rest/v1/user/mcp_grants" +RESOURCES_URL = "/rest/v1/user/resources" + + +class McpGrantsTest(unittest.TestCase): + def setUp(self) -> None: + # One patcher owns every variable. Mixing manual os.environ edits with + # patch.dict leaks: cleanups run after tearDown and re-apply the + # snapshot taken at start(). + self._env = patch.dict(os.environ, self._flags()) + self._env.start() + self.addCleanup(self._env.stop) + os.environ.pop("NO_LOGIN", None) + + self.app = create_app(mode="test") + self.app.secret_key = "test-secret" + self.app_context = self.app.app_context() + self.app_context.push() + sqla.create_all() + self.collection = db.Node_collection() + + def tearDown(self) -> None: + sqla.session.remove() + sqla.drop_all() + self.app_context.pop() + + @staticmethod + def _flags() -> Dict[str, str]: + return { + "NO_LOAD_GRAPH_DB": "1", + "CRE_ENABLE_LOGIN": "1", + "CRE_ENABLE_MYOPENCRE": "1", + "INSECURE_REQUESTS": "1", + "GOOGLE_CLIENT_ID": WEB_CLIENT_ID, + "GOOGLE_MCP_CLIENT_ID": MCP_CLIENT_ID, + } + + # --- helpers --- + + def _seed_user(self): + return self.collection.upsert_user( + google_sub=GOOGLE_SUB, email="a@x.com", display_name="U" + ) + + def _login(self, client: Any, user: Any) -> None: + with client.session_transaction() as sess: + sess["user_id"] = user.id + + def _as_token(self, sub: str = GOOGLE_SUB): + claims = {"sub": sub, "aud": MCP_CLIENT_ID, "email": "a@x.com"} + return patch.object(identity, "verify_google_token", return_value=claims) + + @staticmethod + def _bearer() -> Dict[str, str]: + return {"Authorization": "Bearer any-token"} + + # --- A. storage helpers --- + + def test_a_new_user_has_no_grants(self) -> None: + user = self._seed_user() + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + def test_bootstrap_grants_defaults_only_once(self) -> None: + user = self._seed_user() + first = self.collection.bootstrap_mcp_grants( + user.id, [identity.SCOPE_MYOPENCRE_READ] + ) + self.assertEqual(first, [identity.SCOPE_MYOPENCRE_READ]) + + self.collection.revoke_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + second = self.collection.bootstrap_mcp_grants( + user.id, [identity.SCOPE_MYOPENCRE_READ] + ) + self.assertEqual(second, []) + + def test_add_then_get_returns_the_scope(self) -> None: + user = self._seed_user() + granted = self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + self.assertEqual(granted, [identity.SCOPE_MYOPENCRE_READ]) + + def test_adding_the_same_scope_twice_is_idempotent(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + granted = self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + self.assertEqual(granted, [identity.SCOPE_MYOPENCRE_READ]) + + def test_revoke_removes_only_that_scope(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_WRITE) + granted = self.collection.revoke_mcp_grant( + user.id, identity.SCOPE_MYOPENCRE_WRITE + ) + self.assertEqual(granted, [identity.SCOPE_MYOPENCRE_READ]) + + def test_a_revoked_scope_can_be_granted_again(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + self.collection.revoke_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + granted = self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + self.assertEqual(granted, [identity.SCOPE_MYOPENCRE_READ]) + + def test_grants_do_not_leak_between_users(self) -> None: + first = self._seed_user() + second = self.collection.upsert_user( + google_sub="another-sub", email="b@x.com", display_name="B" + ) + self.collection.add_mcp_grant(first.id, identity.SCOPE_MYOPENCRE_READ) + self.assertEqual(self.collection.get_mcp_grants(second.id), []) + + # --- B. the management endpoints --- + + def test_grants_round_trip_over_http(self) -> None: + user = self._seed_user() + with self.app.test_client() as client: + self._login(client, user) + + empty = client.get(GRANTS_URL) + self.assertEqual(empty.status_code, 200) + self.assertEqual(empty.get_json(), {"granted": []}) + + added = client.put( + GRANTS_URL, json={"scope": identity.SCOPE_MYOPENCRE_READ} + ) + self.assertEqual(added.status_code, 200) + self.assertEqual( + added.get_json(), {"granted": [identity.SCOPE_MYOPENCRE_READ]} + ) + + removed = client.delete( + GRANTS_URL, json={"scope": identity.SCOPE_MYOPENCRE_READ} + ) + self.assertEqual(removed.status_code, 200) + self.assertEqual(removed.get_json(), {"granted": []}) + + def test_an_unknown_scope_is_rejected(self) -> None: + user = self._seed_user() + with self.app.test_client() as client: + self._login(client, user) + response = client.put(GRANTS_URL, json={"scope": "admin:everything"}) + self.assertEqual(response.status_code, 400) + + def test_a_missing_scope_field_is_rejected(self) -> None: + user = self._seed_user() + with self.app.test_client() as client: + self._login(client, user) + response = client.put(GRANTS_URL, json={}) + self.assertEqual(response.status_code, 400) + + def test_anonymous_callers_cannot_read_grants(self) -> None: + self._seed_user() + with self.app.test_client() as client: + response = client.get(GRANTS_URL) + self.assertEqual(response.status_code, 401) + + def test_a_token_caller_cannot_manage_grants(self) -> None: + """A leaked MCP token must not be able to widen its own grants.""" + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + + with self._as_token(): + with self.app.test_client() as client: + read = client.get(GRANTS_URL, headers=self._bearer()) + escalate = client.put( + GRANTS_URL, + json={"scope": identity.SCOPE_MYOPENCRE_WRITE}, + headers=self._bearer(), + ) + + self.assertEqual(read.status_code, 403) + self.assertEqual(escalate.status_code, 403) + self.assertEqual( + self.collection.get_mcp_grants(user.id), + [identity.SCOPE_MYOPENCRE_READ], + ) + + # --- C. scopes gate MyOpenCRE --- + + def test_a_read_grant_cannot_write(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + + with self._as_token(): + with self.app.test_client() as client: + read = client.get(RESOURCES_URL, headers=self._bearer()) + write = client.put( + RESOURCES_URL, + json={"selected": ["ASVS"]}, + headers=self._bearer(), + ) + + self.assertEqual(read.status_code, 200) + self.assertEqual(write.status_code, 403) + + def test_a_write_grant_can_write(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_WRITE) + + with self._as_token(): + with self.app.test_client() as client: + write = client.put( + RESOURCES_URL, + json={"selected": ["ASVS"]}, + headers=self._bearer(), + ) + + self.assertEqual(write.status_code, 200) + self.assertIn("ASVS", write.get_json()["selected"]) + + def test_a_first_time_token_can_read_but_not_write(self) -> None: + """A fresh MCP client is bootstrapped with the default read scope.""" + user = self._seed_user() + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + with self._as_token(): + with self.app.test_client() as client: + read = client.get(RESOURCES_URL, headers=self._bearer()) + write = client.put( + RESOURCES_URL, + json={"selected": ["ASVS"]}, + headers=self._bearer(), + ) + + self.assertEqual(read.status_code, 200) + self.assertEqual(write.status_code, 403) + self.assertEqual( + self.collection.get_mcp_grants(user.id), + [identity.SCOPE_MYOPENCRE_READ], + ) + + def test_a_revoked_scope_stays_revoked_over_http(self) -> None: + """Revoking from the browser must survive the next MCP call.""" + user = self._seed_user() + with self._as_token(): + with self.app.test_client() as client: + client.get(RESOURCES_URL, headers=self._bearer()) + + with self.app.test_client() as client: + self._login(client, user) + client.delete(GRANTS_URL, json={"scope": identity.SCOPE_MYOPENCRE_READ}) + + with self._as_token(): + with self.app.test_client() as client: + response = client.get(RESOURCES_URL, headers=self._bearer()) + + self.assertEqual(response.status_code, 403) + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + def test_a_browser_session_keeps_full_access(self) -> None: + """Adding scopes must not restrict the human.""" + user = self._seed_user() + with self.app.test_client() as client: + self._login(client, user) + write = client.put(RESOURCES_URL, json={"selected": ["ASVS"]}) + read = client.get(RESOURCES_URL) + + self.assertEqual(write.status_code, 200) + self.assertEqual(read.status_code, 200) + self.assertIn("ASVS", read.get_json()["selected"]) + + +if __name__ == "__main__": + unittest.main() diff --git a/application/tests/web_identity_test.py b/application/tests/web_identity_test.py new file mode 100644 index 000000000..a8db09314 --- /dev/null +++ b/application/tests/web_identity_test.py @@ -0,0 +1,245 @@ +"""Tests for OpenCRE identity resolution (issue #1003 v2). + +One user, two doors: a browser session and a Google ID token issued to the +MCP client resolve to the same ``users`` row, but carry different scopes. +""" + +from __future__ import annotations + +import os +import unittest +from unittest.mock import patch + +from flask import g, session +from werkzeug.exceptions import Forbidden + +from application import create_app, sqla +from application.database import db +from application.web import identity + +WEB_CLIENT_ID = "web-client.apps.googleusercontent.com" +MCP_CLIENT_ID = "mcp-client.apps.googleusercontent.com" +OTHER_CLIENT_ID = "someone-else.apps.googleusercontent.com" +GOOGLE_SUB = "1076915035000615071" + + +class IdentityTest(unittest.TestCase): + def setUp(self) -> None: + # One patcher owns every variable. Mixing manual os.environ edits with + # patch.dict leaks: cleanups run after tearDown and re-apply the + # snapshot taken at start(). + self._env = patch.dict( + os.environ, + { + "NO_LOAD_GRAPH_DB": "1", + "GOOGLE_CLIENT_ID": WEB_CLIENT_ID, + "GOOGLE_MCP_CLIENT_ID": MCP_CLIENT_ID, + }, + ) + self._env.start() + self.addCleanup(self._env.stop) + # NO_LOGIN short-circuits every guard; a stray value would hide failures. + os.environ.pop("NO_LOGIN", None) + + self.app = create_app(mode="test") + self.app.secret_key = "test-secret" + self.app_context = self.app.app_context() + self.app_context.push() + sqla.create_all() + self.collection = db.Node_collection() + + def tearDown(self) -> None: + sqla.session.remove() + sqla.drop_all() + self.app_context.pop() + + # --- helpers --- + + def _seed_user(self, google_sub: str = GOOGLE_SUB): + return self.collection.upsert_user( + google_sub=google_sub, email="a@x.com", display_name="U" + ) + + def _claims(self, aud: str = MCP_CLIENT_ID, sub: str = GOOGLE_SUB): + """A Google ID token as it looks once verified and decoded.""" + return {"sub": sub, "aud": aud, "email": "a@x.com"} + + def _as_token(self, claims=None, side_effect=None): + """Patch the verification seam; returns a context manager.""" + kwargs = ( + {"side_effect": side_effect} + if side_effect is not None + else {"return_value": claims if claims is not None else self._claims()} + ) + return patch.object(identity, "verify_google_token", **kwargs) + + def _token_request(self): + return self.app.test_request_context(headers={"Authorization": "Bearer x"}) + + def _reset_identity_cache(self) -> None: + """``g`` lives on the app context, and setUp keeps one pushed for the + whole test, so the per-request cache survives between request contexts + here in a way it never would in a real request.""" + g.pop(identity._IDENTITY_KEY, None) + + # --- A. one user, two doors --- + + def test_mcp_token_resolves_to_the_same_user_as_a_session(self) -> None: + user = self._seed_user() + + with self._as_token(): + with self._token_request(): + token_user, token_channel = identity.current_identity() + + self._reset_identity_cache() + with self.app.test_request_context(): + session["user_id"] = user.id + session_user, session_channel = identity.current_identity() + + self.assertEqual(token_user.id, user.id) + self.assertEqual(session_user.id, user.id) + self.assertEqual(token_channel, identity.CHANNEL_MCP) + self.assertEqual(session_channel, identity.CHANNEL_WEB) + + def test_web_audience_token_is_the_web_channel(self) -> None: + self._seed_user() + with self._as_token(self._claims(aud=WEB_CLIENT_ID)): + with self._token_request(): + _, channel = identity.current_identity() + self.assertEqual(channel, identity.CHANNEL_WEB) + + # --- B. tokens that must be refused --- + + def test_token_with_unknown_audience_is_rejected(self) -> None: + self._seed_user() + with self._as_token(self._claims(aud=OTHER_CLIENT_ID)): + with self._token_request(): + self.assertIsNone(identity.current_identity()) + + def test_token_that_fails_verification_is_rejected(self) -> None: + self._seed_user() + with self._as_token(side_effect=ValueError("signature mismatch")): + with self._token_request(): + self.assertIsNone(identity.current_identity()) + + def test_token_for_unknown_subject_resolves_to_nobody(self) -> None: + """A token must never create an account.""" + with self._as_token(self._claims(sub="nobody-has-this-sub")): + with self._token_request(): + self.assertIsNone(identity.current_identity()) + self.assertIsNone(self.collection.get_user_by_sub("nobody-has-this-sub")) + + def test_request_without_a_credential_is_nobody(self) -> None: + self._seed_user() + with self.app.test_request_context(): + self.assertIsNone(identity.current_identity()) + + # --- C. scopes --- + + def test_web_channel_gets_all_scopes(self) -> None: + user = self._seed_user() + with self.app.test_request_context(): + session["user_id"] = user.id + self.assertEqual(identity.current_scopes(), identity.ALL_SCOPES) + + def test_mcp_channel_gets_only_granted_scopes(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + + with self._as_token(): + with self._token_request(): + scopes = identity.current_scopes() + + self.assertIn(identity.SCOPE_MYOPENCRE_READ, scopes) + self.assertNotIn(identity.SCOPE_MYOPENCRE_WRITE, scopes) + + def test_first_mcp_call_grants_the_default_scopes(self) -> None: + """Logging in is enough to read; writing stays an explicit choice.""" + user = self._seed_user() + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + with self._as_token(): + with self._token_request(): + scopes = identity.current_scopes() + + self.assertEqual(scopes, identity.DEFAULT_MCP_SCOPES) + self.assertIn(identity.SCOPE_MYOPENCRE_READ, scopes) + self.assertNotIn(identity.SCOPE_MYOPENCRE_WRITE, scopes) + + def test_a_revoked_scope_is_never_restored(self) -> None: + """The bootstrap must not undo a deliberate revocation.""" + user = self._seed_user() + + with self._as_token(): + with self._token_request(): + identity.current_scopes() + + self.collection.revoke_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + self._reset_identity_cache() + with self._as_token(): + with self._token_request(): + scopes = identity.current_scopes() + + self.assertEqual(scopes, ()) + + def test_the_session_channel_creates_no_grants(self) -> None: + user = self._seed_user() + with self.app.test_request_context(): + session["user_id"] = user.id + identity.current_scopes() + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + def test_revoking_a_grant_removes_the_scope(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_WRITE) + self.collection.revoke_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_WRITE) + + with self._as_token(): + with self._token_request(): + self.assertNotIn( + identity.SCOPE_MYOPENCRE_WRITE, identity.current_scopes() + ) + + # --- D. the guards --- + + def test_require_scopes_allows_a_holder_and_blocks_others(self) -> None: + user = self._seed_user() + self.collection.add_mcp_grant(user.id, identity.SCOPE_MYOPENCRE_READ) + + @identity.require_scopes(identity.SCOPE_MYOPENCRE_READ) + def _read_view(): + return "ok" + + @identity.require_scopes(identity.SCOPE_MYOPENCRE_WRITE) + def _write_view(): + return "ok" + + with self._as_token(): + with self._token_request(): + self.assertEqual(_read_view(), "ok") + with self.assertRaises(Forbidden): + _write_view() + + def test_session_only_rejects_a_token_caller(self) -> None: + """A leaked MCP token must not be able to widen its own grants.""" + user = self._seed_user() + + @identity.session_only + def _grant_view(): + return "ok" + + with self._as_token(): + with self._token_request(): + with self.assertRaises(Forbidden): + _grant_view() + + self._reset_identity_cache() + with self.app.test_request_context(): + session["user_id"] = user.id + self.assertEqual(_grant_view(), "ok") + + +if __name__ == "__main__": + unittest.main() diff --git a/application/web/identity.py b/application/web/identity.py new file mode 100644 index 000000000..b201c673f --- /dev/null +++ b/application/web/identity.py @@ -0,0 +1,179 @@ +"""Who is making this request, and through which door. + +A request authenticates either with the browser session cookie or with a +Google ID token issued to one of OpenCRE's registered clients. Both resolve to +the same ``users`` row; the channel decides how much that user may do here. +""" + +from __future__ import annotations + +from cre_logging import get_logger + +logger = get_logger(__name__) + +import os +import re +from functools import wraps +from typing import Any, Dict, Optional, Tuple + +from flask import abort, g, request +from google.auth.transport import requests as google_requests +from google.oauth2 import id_token as google_id_token + +from application.database import db + +CHANNEL_WEB = "web" +CHANNEL_MCP = "mcp" + +SCOPE_MYOPENCRE_READ = "myopencre:read" +SCOPE_MYOPENCRE_WRITE = "myopencre:write" + +# A browser session is the user acting directly, so it carries every scope. +ALL_SCOPES: Tuple[str, ...] = (SCOPE_MYOPENCRE_READ, SCOPE_MYOPENCRE_WRITE) + +# Granted the first time a user's MCP client calls, so logging in is enough to +# read. Writing stays an explicit choice, and a revoked scope is never restored. +DEFAULT_MCP_SCOPES: Tuple[str, ...] = (SCOPE_MYOPENCRE_READ,) + +_BEARER_RE = re.compile(r"^Bearer\s+(\S+)$", re.IGNORECASE) +_IDENTITY_KEY = "cre_identity" + + +def allowed_audiences() -> Dict[str, str]: + """Map each registered Google client id to the channel it represents.""" + audiences: Dict[str, str] = {} + web = os.environ.get("GOOGLE_CLIENT_ID") + mcp = os.environ.get("GOOGLE_MCP_CLIENT_ID") + if web: + audiences[web] = CHANNEL_WEB + if mcp: + audiences[mcp] = CHANNEL_MCP + return audiences + + +def verify_google_token(token: str) -> Dict[str, Any]: + """Verify signature, issuer and expiry. The seam tests patch.""" + return google_id_token.verify_oauth2_token( + token, google_requests.Request(), audience=None + ) + + +def bearer_token() -> Optional[str]: + match = _BEARER_RE.match(request.headers.get("Authorization", "")) + return match.group(1) if match else None + + +def _identity_from_token(database: Any) -> Optional[Tuple[Any, str]]: + token = bearer_token() + if not token: + return None + try: + claims = verify_google_token(token) + except ValueError: + logger.info("rejected a bearer token that failed verification") + return None + + channel = allowed_audiences().get(claims.get("aud", "")) + if channel is None: + logger.info("rejected a bearer token issued for another audience") + return None + + subject = claims.get("sub") + if not subject: + return None + user = database.get_user_by_sub(subject) + if user is None: + return None + if channel == CHANNEL_MCP: + database.bootstrap_mcp_grants(user.id, list(DEFAULT_MCP_SCOPES)) + return user, channel + + +def _identity_from_session(database: Any) -> Optional[Tuple[Any, str]]: + """A browser session is authenticated by ``session['user_id']`` alone. + + That is the pre-#1003 predicate and must not change: a session carrying only + ``google_id`` stays anonymous, and a session whose row is missing is still a + session (the view decides what to do about the absent user). + """ + from flask import session + + if "user_id" not in session: + return None + from application.web.web_main import _resolve_current_user + + return _resolve_current_user(database), CHANNEL_WEB + + +def current_identity() -> Optional[Tuple[Any, str]]: + """``(User, channel)`` for this request, or None. Cached per request.""" + if _IDENTITY_KEY not in g: + database = db.Node_collection() + identity = _identity_from_session(database) + if identity is None: + identity = _identity_from_token(database) + setattr(g, _IDENTITY_KEY, identity) + return getattr(g, _IDENTITY_KEY) + + +def current_user() -> Optional[Any]: + identity = current_identity() + return identity[0] if identity else None + + +def current_channel() -> Optional[str]: + identity = current_identity() + return identity[1] if identity else None + + +def is_session_authenticated() -> bool: + """True only for a browser session, never for a token.""" + from flask import session + + return "user_id" in session + + +def current_scopes() -> Tuple[str, ...]: + """What this caller may do. Google never carries OpenCRE scopes, so for + the MCP channel they come from the ``mcp_grants`` table.""" + identity = current_identity() + if identity is None: + return () + user, channel = identity + if channel == CHANNEL_WEB: + return ALL_SCOPES + return tuple(db.Node_collection().get_mcp_grants(user.id)) + + +def require_scopes(*needed: str) -> Any: + """Deny a caller whose channel does not carry every required scope.""" + + def decorator(view: Any) -> Any: + @wraps(view) + def wrapper(*args: Any, **kwargs: Any) -> Any: + if os.environ.get("NO_LOGIN") == "1": + return view(*args, **kwargs) + held = set(current_scopes()) + missing = [scope for scope in needed if scope not in held] + if missing: + abort(403, description=f"Missing scope(s): {', '.join(missing)}") + return view(*args, **kwargs) + + return wrapper + + return decorator + + +def session_only(view: Any) -> Any: + """Restrict a view to browser sessions so a token cannot widen its own + grants.""" + + @wraps(view) + def wrapper(*args: Any, **kwargs: Any) -> Any: + if os.environ.get("NO_LOGIN") == "1": + return view(*args, **kwargs) + if not is_session_authenticated(): + abort(403, description="This endpoint requires a browser session") + return view(*args, **kwargs) + + return wrapper diff --git a/application/web/openapi_registry.py b/application/web/openapi_registry.py index 7b8b39efc..1ea9d72d7 100644 --- a/application/web/openapi_registry.py +++ b/application/web/openapi_registry.py @@ -105,6 +105,47 @@ def __init__( self.parameters = parameters +_MCP_GRANT_SCOPES = ["myopencre:read", "myopencre:write"] + +_MCP_GRANT_REQUEST_BODY: Dict[str, Any] = { + "required": True, + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["scope"], + "properties": { + "scope": { + "type": "string", + "enum": _MCP_GRANT_SCOPES, + "description": "The OpenCRE scope to grant or revoke.", + } + }, + } + } + }, +} + +_MCP_GRANTS_RESPONSE: Dict[str, Any] = { + "200": { + "description": "The scopes currently granted", + "content": { + "application/json": { + "schema": { + "type": "object", + "required": ["granted"], + "properties": { + "granted": { + "type": "array", + "items": {"type": "string", "enum": _MCP_GRANT_SCOPES}, + } + }, + } + } + }, + } +} + OPENAPI_PATHS: List[PathSpec] = [ PathSpec( "/rest/v1/id/{creid}", @@ -437,6 +478,54 @@ def __init__( } }, ), + PathSpec( + "/rest/v1/user/mcp_grants", + "get_user_mcp_grants", + method="get", + tags=["User"], + summary="Scopes granted to the current user's MCP client", + description=( + "Requires a browser session. Google's device flow cannot carry " + "OpenCRE scopes, so an MCP client's permissions are recorded here " + "instead of in its token." + ), + not_found=False, + extra_responses={ + "401": {"description": "Not authenticated"}, + "403": {"description": "Requires a browser session"}, + }, + response_override=_MCP_GRANTS_RESPONSE, + ), + PathSpec( + "/rest/v1/user/mcp_grants", + "put_user_mcp_grant", + method="put", + tags=["User"], + summary="Grant one scope to the current user's MCP client", + not_found=False, + extra_responses={ + "400": {"description": "Unknown or missing scope"}, + "401": {"description": "Not authenticated"}, + "403": {"description": "Requires a browser session"}, + }, + request_body=_MCP_GRANT_REQUEST_BODY, + response_override=_MCP_GRANTS_RESPONSE, + ), + PathSpec( + "/rest/v1/user/mcp_grants", + "delete_user_mcp_grant", + method="delete", + tags=["User"], + summary="Revoke one scope from the current user's MCP client", + not_found=False, + extra_responses={ + "400": {"description": "Unknown or missing scope"}, + "401": {"description": "Not authenticated"}, + "403": {"description": "Requires a browser session"}, + }, + request_body=_MCP_GRANT_REQUEST_BODY, + response_override=_MCP_GRANTS_RESPONSE, + ), ] diff --git a/application/web/web_main.py b/application/web/web_main.py index 7b5b99e76..3fb7ad6e9 100644 --- a/application/web/web_main.py +++ b/application/web/web_main.py @@ -28,7 +28,7 @@ is_login_enabled, is_myopencre_enabled, ) - +from application.web import identity from application.utils import spreadsheet as sheet_utils from application.utils import mdutils, redirectors, gap_analysis from application.web.openapi_registry import openapi_documented @@ -919,8 +919,11 @@ def _is_logged_in() -> bool: Keyed on ``session['user_id']`` (recorded by the login flow since #980) — not ``google_id``/``name``. Both ``login_required`` and its content-negotiation branch route through here, so the predicate lives in exactly one place. + + Since #1003 v2 a Google ID token issued to a registered client also counts, + so the MCP client authenticates the same way the browser does. """ - return "user_id" in session + return identity.current_identity() is not None def _auth_challenge(): @@ -1456,10 +1459,11 @@ def logout(): lambda: jsonify({"selected": []}), ) @login_required +@identity.require_scopes(identity.SCOPE_MYOPENCRE_READ) def get_user_resources() -> Any: """Return the standard names the current user has selected.""" database = db.Node_collection() - user = _resolve_current_user(database) + user = identity.current_user() or _resolve_current_user(database) if user is None: abort(401, description="Not authenticated") return jsonify({"selected": database.get_user_resource_selection(user.id)}) @@ -1472,6 +1476,7 @@ def get_user_resources() -> Any: lambda: jsonify({"selected": []}), ) @login_required +@identity.require_scopes(identity.SCOPE_MYOPENCRE_WRITE) def put_user_resources() -> Any: """Replace the current user's selected standards.""" body = request.get_json(silent=True) @@ -1489,7 +1494,7 @@ def put_user_resources() -> Any: if selected and OPENCRE_STANDARD_NAME not in selected: selected.append(OPENCRE_STANDARD_NAME) database = db.Node_collection() - user = _resolve_current_user(database) + user = identity.current_user() or _resolve_current_user(database) if user is None: abort(401, description="Not authenticated") stored = database.set_user_resource_selection(user.id, selected) @@ -1698,6 +1703,60 @@ def import_from_cre_csv() -> Any: ) +def _requested_scope() -> str: + body = request.get_json(silent=True) + if not isinstance(body, dict) or "scope" not in body: + abort(400, description="Body must be a JSON object with a 'scope' field") + scope = body["scope"] + if not isinstance(scope, str) or scope.strip() not in identity.ALL_SCOPES: + abort( + 400, + description=f"'scope' must be one of: {', '.join(identity.ALL_SCOPES)}", + ) + return scope.strip() + + +@openapi_documented("get_user_mcp_grants") +@app.route("/rest/v1/user/mcp_grants", methods=["GET"]) +@login_required +@identity.session_only +def get_user_mcp_grants() -> Any: + """Return the scopes granted to the current user's MCP client.""" + database = db.Node_collection() + user = identity.current_user() + if user is None: + abort(401, description="Not authenticated") + return jsonify({"granted": database.get_mcp_grants(user.id)}) + + +@openapi_documented("put_user_mcp_grant") +@app.route("/rest/v1/user/mcp_grants", methods=["PUT"]) +@login_required +@identity.session_only +def put_user_mcp_grant() -> Any: + """Grant one scope to the current user's MCP client.""" + scope = _requested_scope() + database = db.Node_collection() + user = identity.current_user() + if user is None: + abort(401, description="Not authenticated") + return jsonify({"granted": database.add_mcp_grant(user.id, scope)}) + + +@openapi_documented("delete_user_mcp_grant") +@app.route("/rest/v1/user/mcp_grants", methods=["DELETE"]) +@login_required +@identity.session_only +def delete_user_mcp_grant() -> Any: + """Revoke one scope from the current user's MCP client.""" + scope = _requested_scope() + database = db.Node_collection() + user = identity.current_user() + if user is None: + abort(401, description="Not authenticated") + return jsonify({"granted": database.revoke_mcp_grant(user.id, scope)}) + + # /End Importing Handlers diff --git a/docs/api/mcp.md b/docs/api/mcp.md index a10985f16..8cf871c41 100644 --- a/docs/api/mcp.md +++ b/docs/api/mcp.md @@ -36,8 +36,13 @@ Or point the MCP server at a hosted instance such as `https://opencre.org`. | Variable | Meaning | Default | |----------|---------|---------| | `OPENCRE_BASE_URL` | OpenCRE origin used for REST calls (server-side only) | `http://127.0.0.1:5000` | +| `OPENCRE_MCP_AUTH_TOOLS` | Expose the v2 authenticated tools (`1`/`true`/`yes`) | off | +| `GOOGLE_MCP_CLIENT_ID` | Google client of type *TVs and Limited Input devices* | — | +| `GOOGLE_MCP_CLIENT_SECRET` | Secret for that client | — | +| `OPENCRE_MCP_TOKEN_FILE` | Override the credential path (skips the OS keyring) | `~/.config/opencre-mcp/tokens.json` | -Tool arguments cannot override the base URL. +Tool arguments cannot override the base URL, and no credential can be passed +as a tool argument. ## Start the MCP server @@ -82,7 +87,79 @@ Add an MCP server entry that runs the module with your venv interpreter. Example Parameter names, types, requiredness, and descriptions for these tools are derived from the matching OpenAPI operations. The `format` query parameter is intentionally omitted so MCP tools stay JSON-oriented. -## Deliberate first-PR parity gaps +Authenticated tools (v2, only when `OPENCRE_MCP_AUTH_TOOLS` is set): + +| MCP tool | HTTP method | REST path | Required scope | +|----------|-------------|-----------|----------------| +| `get_my_resources` | GET | `/rest/v1/user/resources` | `myopencre:read` | +| `set_my_resources` | PUT | `/rest/v1/user/resources` | `myopencre:write` | + +`set_my_resources` takes its JSON request body as a single `body` argument, +derived from the OpenAPI `requestBody`, and is annotated as a write so MCP +clients prompt before calling it. + +## Authenticated tools (v2) + +OpenCRE registers **two** Google OAuth clients: a *Web application* for the +browser and a *TVs and Limited Input devices* client for MCP. Both issue ID +tokens for the same Google account, so both resolve to the same `users` row — +the `aud` claim tells REST which door a request came through. + +Google's device flow supports only `openid email profile` and will never carry +an OpenCRE scope, so permissions live in the `mcp_grants` table instead. Google +answers *who you are*; OpenCRE answers *what you may do*. + +### Log in + +```bash +export GOOGLE_MCP_CLIENT_ID=...apps.googleusercontent.com +export GOOGLE_MCP_CLIENT_SECRET=... +python -m application.mcp login # prints a code to enter at google.com/device +python -m application.mcp status +python -m application.mcp logout +``` + +The refresh token is stored in the OS keyring, falling back to a `0600` file. +It is never written to stdout, never passed as a tool argument, and is redacted +from any error text an MCP client can see. + +Staying inside `openid email profile` also keeps refresh tokens exempt from the +7-day expiry Google applies to apps whose consent screen is still in *Testing*. + +### Scopes and grants + +The first request from a user's MCP client is granted `myopencre:read` +automatically, so logging in is enough to read. **Writing is never granted +automatically** — `set_my_resources` returns 403 until `myopencre:write` is +granted deliberately from a browser. + +Manage grants from a browser session: + +``` +GET /rest/v1/user/mcp_grants → {"granted": [...]} +PUT /rest/v1/user/mcp_grants {"scope": "..."} → grant one scope +DELETE /rest/v1/user/mcp_grants {"scope": "..."} → revoke one scope +``` + +These endpoints accept **browser sessions only**. A request authenticated with +an MCP token receives 403, so a leaked token cannot widen its own grants. + +Revoking is permanent until re-granted: the automatic grant applies only to a +user with no grant history at all, so a revoked scope is never silently +restored on the next call. + +### Why write access is exposed at all + +`set_my_resources` is the one write in the allowlist, and it only changes the +caller's own saved standards selection — it cannot touch CREs, mappings, or +another user's data. It stays unreachable unless four things are all true: + +1. the deployment sets `OPENCRE_MCP_AUTH_TOOLS` +2. the user has completed the device-flow login +3. the user granted `myopencre:write` from a browser +4. the MCP client honours the write annotation and the user approves the call + +## Deliberate parity gaps Not exposed yet (tracked for follow-up, not implied as MCP-complete): @@ -94,13 +171,18 @@ Not exposed yet (tracked for follow-up, not implied as MCP-complete): - Deeplink redirect routes - `GET /rest/v1/openapi.yaml` as a tool - `/api/capabilities` -- Authenticated MyOpenCRE / chat / admin surfaces +- Chat (`POST /rest/v1/completion`) and admin/import surfaces +- Remote (HTTP) transport — stdio only, so credentials come from the + environment as the MCP specification directs for stdio servers ## Security boundary -- Fixed public GET allowlist in `application/mcp/catalog.py` — OpenAPI membership alone does **not** expose a tool -- No credentials forwarded -- No MCP bypass secret -- Callers cannot supply arbitrary methods, paths, or base URLs +- Fixed allowlist in `application/mcp/catalog.py` — OpenAPI membership alone does **not** expose a tool +- Authenticated tools stay hidden unless `OPENCRE_MCP_AUTH_TOOLS` is set, so the public surface is unchanged by default +- No MCP bypass secret; the MCP client authenticates as the user, with fewer scopes than the user +- Credentials come from the environment only — they cannot be expressed as tool arguments (`header` and `cookie` parameters are excluded from every schema, and every schema sets `additionalProperties: false`) +- The credential is redacted from all error text returned to an MCP client +- Callers cannot supply arbitrary methods, paths, or base URLs; only `GET`/`PUT`/`POST` declared by a `ToolSpec` are dispatched, and redirects are never followed - `get_node` only accepts Credoctypes for `ntype` (blocks collisions such as `/rest/v1/user/resources`) +- Grant management requires a browser session, so a token cannot escalate itself - REST remains the backend execution and authorization boundary diff --git a/docs/api/openapi.yaml b/docs/api/openapi.yaml index 3708aa0f4..9a9499534 100644 --- a/docs/api/openapi.yaml +++ b/docs/api/openapi.yaml @@ -505,6 +505,115 @@ paths: description: Not found '400': description: Missing text parameter + /rest/v1/user/mcp_grants: + get: + tags: + - User + summary: Scopes granted to the current user's MCP client + description: Requires a browser session. Google's device flow cannot carry OpenCRE + scopes, so an MCP client's permissions are recorded here instead of in its + token. + responses: + '200': + description: The scopes currently granted + content: + application/json: + schema: + type: object + required: + - granted + properties: + granted: + type: array + items: + type: string + enum: + - myopencre:read + - myopencre:write + '401': + description: Not authenticated + '403': + description: Requires a browser session + put: + tags: + - User + summary: Grant one scope to the current user's MCP client + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - scope + properties: + scope: + type: string + enum: &id001 + - myopencre:read + - myopencre:write + description: The OpenCRE scope to grant or revoke. + responses: + '200': + description: The scopes currently granted + content: + application/json: + schema: + type: object + required: + - granted + properties: + granted: + type: array + items: + type: string + enum: *id001 + '400': + description: Unknown or missing scope + '401': + description: Not authenticated + '403': + description: Requires a browser session + delete: + tags: + - User + summary: Revoke one scope from the current user's MCP client + requestBody: + required: true + content: + application/json: + schema: + type: object + required: + - scope + properties: + scope: + type: string + enum: &id002 + - myopencre:read + - myopencre:write + description: The OpenCRE scope to grant or revoke. + responses: + '200': + description: The scopes currently granted + content: + application/json: + schema: + type: object + required: + - granted + properties: + granted: + type: array + items: + type: string + enum: *id002 + '400': + description: Unknown or missing scope + '401': + description: Not authenticated + '403': + description: Requires a browser session /rest/v1/user/resources: get: tags: diff --git a/migrations/versions/f3a91c07d24b_add_mcp_grants.py b/migrations/versions/f3a91c07d24b_add_mcp_grants.py new file mode 100644 index 000000000..dafc4c7c7 --- /dev/null +++ b/migrations/versions/f3a91c07d24b_add_mcp_grants.py @@ -0,0 +1,40 @@ +"""add mcp_grants table for MCP v2 authenticated tools (issue #1003) + +Google's device flow only carries ``openid email profile``, so OpenCRE scopes +cannot live in the token and are recorded per user here instead. + +Revision ID: f3a91c07d24b +Revises: 967016ee10fa +Create Date: 2026-09-29 + +""" + +from alembic import op +import sqlalchemy as sa + + +revision = "f3a91c07d24b" +down_revision = "967016ee10fa" +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + "mcp_grants", + sa.Column("id", sa.String, primary_key=True), + sa.Column( + "user_id", + sa.String, + sa.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("scope", sa.String, nullable=False), + sa.Column("created_at", sa.DateTime, nullable=False), + sa.Column("revoked_at", sa.DateTime, nullable=True), + sa.UniqueConstraint("user_id", "scope", name="uq_mcp_grants"), + ) + + +def downgrade(): + op.drop_table("mcp_grants") diff --git a/requirements-dev.txt b/requirements-dev.txt index 7121de20e..7bf1f2bdb 100644 --- a/requirements-dev.txt +++ b/requirements-dev.txt @@ -89,3 +89,4 @@ pytest-playwright # Local stdio MCP server (issue #1003 v1) — not needed on Heroku slug mcp==2.0.0 +keyring diff --git a/scripts/device_flow_check.py b/scripts/device_flow_check.py new file mode 100644 index 000000000..4357635c2 --- /dev/null +++ b/scripts/device_flow_check.py @@ -0,0 +1,124 @@ +"""Standalone Google device-flow check. No OpenCRE involved. + +First run : full device flow, saves the refresh token to .mcp_tokens.json +Later runs : refreshes silently, no browser, no re-approval + +This mirrors what application/mcp/auth.py will do in v2, with a file +standing in for the OS keyring. +""" + +import base64 +import json +import os +import pathlib +import sys +import time + +import requests + +try: + from dotenv import load_dotenv + + load_dotenv() # pull GOOGLE_MCP_* out of .env at the repo root +except ImportError: + pass + +DEVICE_CODE_URL = "https://oauth2.googleapis.com/device/code" +TOKEN_URL = "https://oauth2.googleapis.com/token" +DEVICE_GRANT = "urn:ietf:params:oauth:grant-type:device_code" +SCOPES = "openid email profile" # NEVER add a fourth - see docs/api/mcp.md +TOKEN_FILE = pathlib.Path(__file__).parent / ".mcp_tokens.json" + +CLIENT_ID = os.environ.get("GOOGLE_MCP_CLIENT_ID") +CLIENT_SECRET = os.environ.get("GOOGLE_MCP_CLIENT_SECRET") +if not CLIENT_ID or not CLIENT_SECRET: + sys.exit( + "GOOGLE_MCP_CLIENT_ID / GOOGLE_MCP_CLIENT_SECRET not found.\n" + "Put them in .env at the repo root, or export them in this shell." + ) + + +def claims_of(id_token: str) -> dict: + """Decode WITHOUT verifying - display only. Real code verifies the signature.""" + payload = id_token.split(".")[1] + return json.loads(base64.urlsafe_b64decode(payload + "=" * (-len(payload) % 4))) + + +def refresh(refresh_token: str) -> dict | None: + """Trade a refresh token for fresh tokens. None if it is no longer valid.""" + r = requests.post( + TOKEN_URL, + data={ + "client_id": CLIENT_ID, + "client_secret": CLIENT_SECRET, + "refresh_token": refresh_token, + "grant_type": "refresh_token", + }, + timeout=30, + ) + if r.status_code == 200: + return r.json() + print(f" refresh rejected [{r.status_code}] - falling back to device flow") + return None + + +def device_flow() -> dict: + """Full approval flow: print a code, wait for the human, return tokens.""" + r = requests.post( + DEVICE_CODE_URL, + data={"client_id": CLIENT_ID, "scope": SCOPES}, + timeout=30, + ) + if r.status_code != 200: + # Google's body says WHY (wrong client type, consent screen incomplete...). + sys.exit(f"device/code failed [{r.status_code}]: {r.text}") + d = r.json() + + # Google spells it verification_url; RFC 8628 says verification_uri. + url = d.get("verification_url") or d["verification_uri"] + print(f"\n Open: {url}\n Code: {d['user_code']}\n") + + interval = d.get("interval", 5) + deadline = time.time() + d["expires_in"] + while time.time() < deadline: + time.sleep(interval) + t = requests.post( + TOKEN_URL, + data={ + "client_id": CLIENT_ID, + "client_secret": CLIENT_SECRET, + "device_code": d["device_code"], + "grant_type": DEVICE_GRANT, + }, + timeout=30, + ) + body = t.json() + if t.status_code == 200: + return body + err = body.get("error") + if err == "authorization_pending": + print(" waiting...") + continue + if err == "slow_down": + interval += 5 + continue + sys.exit(f"failed: {body}") + sys.exit("timed out - the code expired before it was approved") + + +saved = json.loads(TOKEN_FILE.read_text()) if TOKEN_FILE.exists() else {} +tokens = refresh(saved["refresh_token"]) if saved.get("refresh_token") else None +if tokens is None: + tokens = device_flow() + +# A refresh response omits refresh_token - keep the one we already hold. +keep = tokens.get("refresh_token") or saved.get("refresh_token") +TOKEN_FILE.write_text(json.dumps({"refresh_token": keep, "saved_at": time.time()})) + +claims = claims_of(tokens["id_token"]) +print("refresh token held :", bool(keep)) +print("sub :", claims["sub"]) +print("aud :", claims["aud"]) # must be your MCP client id +print("email :", claims.get("email")) +print("id_token expires :", int(claims["exp"] - time.time()), "seconds") +print(f"\nsaved to {TOKEN_FILE.name} - re-run to confirm it refreshes with no browser") From 8eff369bcc09163654ad6ef11cc41cd6941eb1c8 Mon Sep 17 00:00:00 2001 From: Spyros Date: Wed, 14 Oct 2026 09:30:29 +0100 Subject: [PATCH 2/2] fix(mcp): narrow bearer to scoped MyOpenCRE routes Keep login_required session-only; admit MCP bearer only on MyOpenCRE routes that also require scopes. Reject web-audience bearers and harden token file permissions. --- .gitignore | 2 - application/mcp/auth.py | 20 +++- application/mcp/catalog.py | 28 +++--- application/mcp/server.py | 29 ++++-- application/mcp/token_store.py | 23 +++-- application/tests/conftest.py | 35 ------- application/tests/mcp_auth_test.py | 92 +++++++++++++++--- application/tests/mcp_grants_test.py | 74 ++++++++++++++- application/tests/web_identity_test.py | 10 +- application/web/identity.py | 10 +- application/web/web_main.py | 31 +++++-- scripts/device_flow_check.py | 124 ------------------------- 12 files changed, 260 insertions(+), 218 deletions(-) delete mode 100644 application/tests/conftest.py delete mode 100644 scripts/device_flow_check.py diff --git a/.gitignore b/.gitignore index df9752a0d..9efaa738d 100644 --- a/.gitignore +++ b/.gitignore @@ -101,5 +101,3 @@ project management scripts/ ### Local MCP client config (machine-specific paths) .mcp.json -### MCP v2 local dev credentials (never commit) -.mcp_tokens.json diff --git a/application/mcp/auth.py b/application/mcp/auth.py index 4a5af8990..c543f3d7d 100644 --- a/application/mcp/auth.py +++ b/application/mcp/auth.py @@ -120,10 +120,16 @@ def login(on_code: Callable[[str, str], None]) -> Dict[str, Any]: }, timeout=REQUEST_TIMEOUT_SECONDS, ) - body = polled.json() + try: + body = polled.json() + except ValueError: + raise AuthError( + f"authorization failed: non-JSON token response " + f"[{polled.status_code}]" + ) if polled.status_code == 200: return _store(body) - error = body.get("error") + error = body.get("error") if isinstance(body, dict) else None if error == "authorization_pending": continue if error == "slow_down": @@ -138,9 +144,15 @@ def _store(tokens: Dict[str, Any]) -> Dict[str, Any]: global _cached_id_token # A refresh response omits refresh_token; keep the one already held. + # Login must not succeed with only an in-memory id_token — without a + # durable refresh token the next process cannot continue the session. refresh_token = tokens.get("refresh_token") or token_store.load() - if refresh_token: - token_store.save(refresh_token) + if not refresh_token: + raise AuthError( + "token endpoint returned no refresh_token and none is stored; " + "cannot persist MCP login" + ) + token_store.save(refresh_token) _cached_id_token = tokens.get("id_token") return _decode_claims(_cached_id_token) if _cached_id_token else {} diff --git a/application/mcp/catalog.py b/application/mcp/catalog.py index 4e079d677..dc5f7eae0 100644 --- a/application/mcp/catalog.py +++ b/application/mcp/catalog.py @@ -115,28 +115,30 @@ def openapi_identity(self) -> Tuple[str, str]: ), ) -_TOOLS_BY_NAME: Dict[str, ToolSpec] = { - tool.name: tool for tool in PUBLIC_TOOLS + AUTHENTICATED_TOOLS -} - def auth_tools_enabled() -> bool: return os.environ.get(AUTH_TOOLS_ENV, "").strip().lower() in _TRUE_VALUES +def active_tools() -> Tuple[ToolSpec, ...]: + """Tools this process serves: public always, authenticated on opt-in.""" + if auth_tools_enabled(): + return PUBLIC_TOOLS + AUTHENTICATED_TOOLS + return PUBLIC_TOOLS + + def list_tool_names() -> List[str]: - return [tool.name for tool in PUBLIC_TOOLS] + return [tool.name for tool in active_tools()] def get_tool(name: str) -> ToolSpec: + """Resolve a tool that is active in this process. + + Hidden authenticated tools must not dispatch when ``OPENCRE_MCP_AUTH_TOOLS`` + is off — lookup is built from ``active_tools()``, not the full catalog. + """ + by_name: Dict[str, ToolSpec] = {tool.name: tool for tool in active_tools()} try: - return _TOOLS_BY_NAME[name] + return by_name[name] except KeyError as exc: raise KeyError(f"Unknown MCP tool: {name}") from exc - - -def active_tools() -> Tuple[ToolSpec, ...]: - """Tools this process serves: public always, authenticated on opt-in.""" - if auth_tools_enabled(): - return PUBLIC_TOOLS + AUTHENTICATED_TOOLS - return PUBLIC_TOOLS diff --git a/application/mcp/server.py b/application/mcp/server.py index c9630f242..7b03b6b17 100644 --- a/application/mcp/server.py +++ b/application/mcp/server.py @@ -13,7 +13,7 @@ from mcp.server.lowlevel import Server from mcp.server.stdio import stdio_server -from application.mcp.catalog import active_tools +from application.mcp.catalog import active_tools, auth_tools_enabled from application.mcp.openapi_loader import input_schemas_for from application.mcp.rest_client import ( RestClient, @@ -25,10 +25,27 @@ SERVER_NAME = "opencre" SERVER_VERSION = "0.1.0" -SERVER_INSTRUCTIONS = ( - "OpenCRE MCP v1 exposes public JSON REST reads only. " - "Authenticated MyOpenCRE, chat, and admin tools are out of scope." -) + + +def server_instructions() -> str: + """Describe the live tool surface (v1 public-only vs v2 with auth tools).""" + if not auth_tools_enabled(): + return ( + "OpenCRE MCP v1 exposes public JSON REST reads only. " + "Authenticated MyOpenCRE, chat, and admin tools are out of scope." + ) + served = active_tools() + has_write = any(not tool.read_only for tool in served) + if has_write: + return ( + "OpenCRE MCP v2 exposes public JSON REST reads plus authenticated " + "MyOpenCRE tools (including set_my_resources for writes). " + "Chat and admin tools remain out of scope." + ) + return ( + "OpenCRE MCP v2 exposes public JSON REST reads plus authenticated " + "MyOpenCRE read tools. Chat and admin tools remain out of scope." + ) def build_server(rest_client: Optional[RestClient] = None) -> Server[Any]: @@ -110,7 +127,7 @@ async def on_call_tool( return Server( SERVER_NAME, version=SERVER_VERSION, - instructions=SERVER_INSTRUCTIONS, + instructions=server_instructions(), on_list_tools=on_list_tools, on_call_tool=on_call_tool, ) diff --git a/application/mcp/token_store.py b/application/mcp/token_store.py index c922dbe52..2310912d0 100644 --- a/application/mcp/token_store.py +++ b/application/mcp/token_store.py @@ -57,13 +57,24 @@ def save(refresh_token: str) -> None: path = token_file() path.parent.mkdir(parents=True, exist_ok=True) - path.write_text( - json.dumps({"refresh_token": refresh_token, "saved_at": time.time()}) - ) + payload = json.dumps({"refresh_token": refresh_token, "saved_at": time.time()}) + # Create/truncate with mode 0600 up front — never write under a looser umask + # and chmod afterwards. Fail closed if confidentiality cannot be guaranteed. try: - os.chmod(path, 0o600) - except OSError: - logger.debug("could not chmod token file", exc_info=True) + fd = os.open( + path, + os.O_WRONLY | os.O_CREAT | os.O_TRUNC, + 0o600, + ) + except OSError as exc: + raise OSError(f"cannot create confidential token file {path}") from exc + try: + os.fchmod(fd, 0o600) + except (AttributeError, OSError) as exc: + os.close(fd) + raise OSError(f"cannot set confidential mode on token file {path}") from exc + with os.fdopen(fd, "w") as handle: + handle.write(payload) def load() -> Optional[str]: diff --git a/application/tests/conftest.py b/application/tests/conftest.py deleted file mode 100644 index bbc65eeae..000000000 --- a/application/tests/conftest.py +++ /dev/null @@ -1,35 +0,0 @@ -"""Shared pytest setup for the OpenCRE test suite. - -``application.feature_flags`` calls ``load_dotenv()`` at import, so a -contributor's local ``.env`` lands in ``os.environ`` for the whole test run. -Values that change what the code under test does must not leak in that way: -they make results depend on whose machine the suite runs on, and a stray -``NO_LOGIN`` would make every authorization test pass without testing anything. -""" - -from __future__ import annotations - -import os - -import pytest - -# Cleared before every test. Any test that needs one sets it explicitly. -AMBIENT_ENV_VARS = ( - "NO_LOGIN", - "NO_LOAD_GRAPH_DB", - "INSECURE_REQUESTS", - "CRE_ENABLE_LOGIN", - "CRE_ENABLE_MYOPENCRE", - "OPENCRE_MCP_AUTH_TOOLS", - "OPENCRE_MCP_TOKEN_FILE", - "OPENCRE_BASE_URL", - "GOOGLE_CLIENT_ID", - "GOOGLE_MCP_CLIENT_ID", - "GOOGLE_MCP_CLIENT_SECRET", -) - - -@pytest.fixture(autouse=True) -def _isolate_ambient_env(monkeypatch: pytest.MonkeyPatch) -> None: - for name in AMBIENT_ENV_VARS: - monkeypatch.delenv(name, raising=False) diff --git a/application/tests/mcp_auth_test.py b/application/tests/mcp_auth_test.py index 68edc8f3a..a440a59fb 100644 --- a/application/tests/mcp_auth_test.py +++ b/application/tests/mcp_auth_test.py @@ -3,16 +3,19 @@ from __future__ import annotations import os +import tempfile import unittest +from pathlib import Path from unittest.mock import patch -from application.mcp import auth, catalog +from application.mcp import auth, catalog, token_store from application.mcp.openapi_loader import input_schemas_for from application.mcp.rest_client import ( RestClient, RestRequestError, RestResponseError, ) +from application.mcp.server import server_instructions class _FakeResponse: @@ -31,16 +34,45 @@ def test_authenticated_tools_are_hidden_by_default(self) -> None: with patch.dict(os.environ, {}, clear=False): os.environ.pop(catalog.AUTH_TOOLS_ENV, None) served = catalog.active_tools() + names = catalog.list_tool_names() self.assertEqual(served, catalog.PUBLIC_TOOLS) self.assertEqual(len(served), 9) + self.assertNotIn("get_my_resources", names) + with self.assertRaises(KeyError): + catalog.get_tool("get_my_resources") def test_authenticated_tools_appear_when_enabled(self) -> None: with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): served = catalog.active_tools() - names = [tool.name for tool in served] + names = catalog.list_tool_names() + get_my = catalog.get_tool("get_my_resources") self.assertIn("get_my_resources", names) self.assertIn("set_my_resources", names) self.assertEqual(len(served), 11) + self.assertEqual(get_my.name, "get_my_resources") + + def test_get_tool_and_list_names_respect_auth_flag(self) -> None: + with patch.dict(os.environ, {}, clear=False): + os.environ.pop(catalog.AUTH_TOOLS_ENV, None) + self.assertEqual( + catalog.list_tool_names(), [t.name for t in catalog.PUBLIC_TOOLS] + ) + with self.assertRaises(KeyError): + catalog.get_tool("set_my_resources") + with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): + self.assertIn("set_my_resources", catalog.list_tool_names()) + self.assertFalse(catalog.get_tool("set_my_resources").read_only) + + def test_server_instructions_mention_write_when_auth_tools_on(self) -> None: + with patch.dict(os.environ, {}, clear=False): + os.environ.pop(catalog.AUTH_TOOLS_ENV, None) + v1 = server_instructions() + self.assertIn("v1", v1) + self.assertNotIn("set_my_resources", v1) + with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): + v2 = server_instructions() + self.assertIn("v2", v2) + self.assertIn("set_my_resources", v2) def test_no_tool_schema_can_carry_a_credential(self) -> None: """Credentials come from the environment, never from tool arguments.""" @@ -69,11 +101,12 @@ def request(self, *args, **kwargs): raise AssertionError("HTTP must not be called without a credential") client = RestClient(session=_NoHttpSession()) - with patch.object( - auth, "current_id_token", side_effect=auth.NotAuthenticated("run login") - ): - with self.assertRaises(RestRequestError) as ctx: - client.call_tool("get_my_resources", {}) + with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): + with patch.object( + auth, "current_id_token", side_effect=auth.NotAuthenticated("run login") + ): + with self.assertRaises(RestRequestError) as ctx: + client.call_tool("get_my_resources", {}) self.assertIn("run login", str(ctx.exception)) def test_token_is_redacted_from_error_text(self) -> None: @@ -88,9 +121,10 @@ def request(self, method, url, **kwargs): return _FakeResponse(500, f"upstream rejected {sent}") client = RestClient(session=_EchoSession()) - with patch.object(auth, "current_id_token", return_value=token): - with self.assertRaises(RestResponseError) as ctx: - client.call_tool("get_my_resources", {}) + with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): + with patch.object(auth, "current_id_token", return_value=token): + with self.assertRaises(RestResponseError) as ctx: + client.call_tool("get_my_resources", {}) message = str(ctx.exception) self.assertNotIn(token, message) @@ -110,8 +144,9 @@ def request(self, method, url, **kwargs): return _FakeResponse(200, "[]") client = RestClient(session=_CapturingSession()) - with patch.object(auth, "current_id_token", return_value=token): - client.call_tool("get_my_resources", {}) + with patch.dict(os.environ, {catalog.AUTH_TOOLS_ENV: "1"}): + with patch.object(auth, "current_id_token", return_value=token): + client.call_tool("get_my_resources", {}) self.assertEqual(seen["method"], "GET") self.assertTrue(seen["url"].endswith("/rest/v1/user/resources")) @@ -130,6 +165,39 @@ def request(self, method, url, **kwargs): RestClient(session=_CapturingSession()).call_tool("list_standards", {}) self.assertNotIn("Authorization", seen["headers"]) + def test_store_requires_a_refresh_token(self) -> None: + """Login must not succeed with only an in-memory id_token.""" + auth._cached_id_token = None + with patch.object(token_store, "load", return_value=None): + with patch.object(token_store, "save") as save: + with self.assertRaises(auth.AuthError) as ctx: + auth._store({"id_token": "header.payload.sig"}) + self.assertIn("refresh_token", str(ctx.exception)) + save.assert_not_called() + self.assertIsNone(auth._cached_id_token) + + def test_store_reuses_existing_refresh_token_on_refresh_response(self) -> None: + auth._cached_id_token = None + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "tokens.json" + with patch.dict(os.environ, {"OPENCRE_MCP_TOKEN_FILE": str(path)}): + token_store.save("existing-refresh") + claims = auth._store( + { + "id_token": "eyJhbGciOiJub25lIn0.eyJzdWIiOiIxIn0.", + } + ) + self.assertEqual(auth._cached_id_token, "eyJhbGciOiJub25lIn0.eyJzdWIiOiIxIn0.") + self.assertIsInstance(claims, dict) + + def test_token_file_is_created_mode_0600(self) -> None: + with tempfile.TemporaryDirectory() as tmp: + path = Path(tmp) / "tokens.json" + with patch.dict(os.environ, {"OPENCRE_MCP_TOKEN_FILE": str(path)}): + token_store.save("refresh-secret") + mode = path.stat().st_mode & 0o777 + self.assertEqual(mode, 0o600) + if __name__ == "__main__": unittest.main() diff --git a/application/tests/mcp_grants_test.py b/application/tests/mcp_grants_test.py index 7c772292e..8a0808ac0 100644 --- a/application/tests/mcp_grants_test.py +++ b/application/tests/mcp_grants_test.py @@ -189,8 +189,10 @@ def test_a_token_caller_cannot_manage_grants(self) -> None: headers=self._bearer(), ) - self.assertEqual(read.status_code, 403) - self.assertEqual(escalate.status_code, 403) + # login_required is session-only, so a bearer is challenged with 401 + # before session_only's 403; either denial is correct. + self.assertIn(read.status_code, (401, 403)) + self.assertIn(escalate.status_code, (401, 403)) self.assertEqual( self.collection.get_mcp_grants(user.id), [identity.SCOPE_MYOPENCRE_READ], @@ -280,6 +282,74 @@ def test_a_browser_session_keeps_full_access(self) -> None: self.assertEqual(read.status_code, 200) self.assertIn("ASVS", read.get_json()["selected"]) + # --- D. bearer must not unlock general login_required routes --- + + def test_mcp_bearer_cannot_call_completion(self) -> None: + """A leaked MCP token must not unlock the chatbot.""" + self._seed_user() + with self._as_token(): + with self.app.test_client() as client: + response = client.post( + "/rest/v1/completion", + json={"prompt": "hi"}, + headers=self._bearer(), + content_type="application/json", + ) + self.assertIn(response.status_code, (401, 403)) + + def test_mcp_bearer_cannot_apply_admin_import(self) -> None: + """A leaked MCP token must not unlock admin import apply.""" + self._seed_user() + with patch.dict(os.environ, {"CRE_ALLOW_IMPORT": "1"}): + with self._as_token(): + with self.app.test_client() as client: + response = client.post( + "/admin/imports/runs/not-a-real-run/apply", + headers=self._bearer(), + ) + self.assertIn(response.status_code, (401, 403)) + + def test_revoked_grants_block_resources_and_completion(self) -> None: + """All grants revoked → resources 403; completion still denied.""" + user = self._seed_user() + with self._as_token(): + with self.app.test_client() as client: + client.get(RESOURCES_URL, headers=self._bearer()) + + for scope in list(self.collection.get_mcp_grants(user.id)): + self.collection.revoke_mcp_grant(user.id, scope) + self.assertEqual(self.collection.get_mcp_grants(user.id), []) + + with self._as_token(): + with self.app.test_client() as client: + resources = client.get(RESOURCES_URL, headers=self._bearer()) + completion = client.post( + "/rest/v1/completion", + json={"prompt": "hi"}, + headers=self._bearer(), + content_type="application/json", + ) + self.assertEqual(resources.status_code, 403) + self.assertIn(completion.status_code, (401, 403)) + + def test_web_audience_bearer_is_rejected_on_resources(self) -> None: + """A web client ID token must not authenticate as CHANNEL_WEB via bearer.""" + self._seed_user() + claims = {"sub": GOOGLE_SUB, "aud": WEB_CLIENT_ID, "email": "a@x.com"} + with patch.object(identity, "verify_google_token", return_value=claims): + with self.app.test_client() as client: + response = client.get(RESOURCES_URL, headers=self._bearer()) + self.assertEqual(response.status_code, 401) + + def test_mcp_grants_responses_are_no_store(self) -> None: + user = self._seed_user() + with self.app.test_client() as client: + self._login(client, user) + response = client.get(GRANTS_URL) + self.assertEqual(response.status_code, 200) + cache_control = response.headers.get("Cache-Control", "") + self.assertIn("no-store", cache_control.lower()) + if __name__ == "__main__": unittest.main() diff --git a/application/tests/web_identity_test.py b/application/tests/web_identity_test.py index a8db09314..feb3cefb5 100644 --- a/application/tests/web_identity_test.py +++ b/application/tests/web_identity_test.py @@ -101,12 +101,16 @@ def test_mcp_token_resolves_to_the_same_user_as_a_session(self) -> None: self.assertEqual(token_channel, identity.CHANNEL_MCP) self.assertEqual(session_channel, identity.CHANNEL_WEB) - def test_web_audience_token_is_the_web_channel(self) -> None: + def test_web_audience_token_is_rejected(self) -> None: + """A web Google client ID token must not authenticate via bearer. + + Browser traffic uses the session cookie; a leaked web ID token must not + become CHANNEL_WEB with ALL_SCOPES. + """ self._seed_user() with self._as_token(self._claims(aud=WEB_CLIENT_ID)): with self._token_request(): - _, channel = identity.current_identity() - self.assertEqual(channel, identity.CHANNEL_WEB) + self.assertIsNone(identity.current_identity()) # --- B. tokens that must be refused --- diff --git a/application/web/identity.py b/application/web/identity.py index b201c673f..123d66033 100644 --- a/application/web/identity.py +++ b/application/web/identity.py @@ -40,12 +40,14 @@ def allowed_audiences() -> Dict[str, str]: - """Map each registered Google client id to the channel it represents.""" + """Map Google client ids accepted on the bearer door to their channel. + + Only the MCP client id is admitted here. The web client id is never a + bearer audience: browser traffic authenticates via the session cookie, and + a leaked web ID token must not become CHANNEL_WEB with ALL_SCOPES. + """ audiences: Dict[str, str] = {} - web = os.environ.get("GOOGLE_CLIENT_ID") mcp = os.environ.get("GOOGLE_MCP_CLIENT_ID") - if web: - audiences[web] = CHANNEL_WEB if mcp: audiences[mcp] = CHANNEL_MCP return audiences diff --git a/application/web/web_main.py b/application/web/web_main.py index 3fb7ad6e9..094f818e8 100644 --- a/application/web/web_main.py +++ b/application/web/web_main.py @@ -901,9 +901,10 @@ def add_header(response): # Per-user endpoints must never be shared-cached; no-store wins over the # default max-age for them (this hook runs after the view). Covers the # resource selection AND the identity endpoint (returns the user's email) - # plus its deprecated alias. + # plus its deprecated alias, and MCP grant management. if request.path in ( "/rest/v1/user/resources", + "/rest/v1/user/mcp_grants", "/rest/v1/auth/user", "/rest/v1/user", ): @@ -914,16 +915,18 @@ def add_header(response): def _is_logged_in() -> bool: - """Single source of truth for session presence. + """Single source of truth for browser-session presence. Keyed on ``session['user_id']`` (recorded by the login flow since #980) — not ``google_id``/``name``. Both ``login_required`` and its content-negotiation branch route through here, so the predicate lives in exactly one place. - Since #1003 v2 a Google ID token issued to a registered client also counts, - so the MCP client authenticates the same way the browser does. + An MCP bearer token is deliberately *not* logged-in here: ``login_required`` + stays session-only so a leaked MCP token cannot unlock chat, admin, or other + general authenticated routes. MyOpenCRE resources use + ``login_or_mcp_required`` instead. """ - return identity.current_identity() is not None + return identity.is_session_authenticated() def _auth_challenge(): @@ -963,6 +966,20 @@ def login_r(*args, **kwargs): return login_r +def login_or_mcp_required(f): + """Admit a browser session or a verified MCP bearer (not general login).""" + + @wraps(f) + def login_or_mcp_r(*args, **kwargs): + if os.environ.get("NO_LOGIN") == "1": + return f(*args, **kwargs) + if identity.current_identity() is None: + return _auth_challenge() + return f(*args, **kwargs) + + return login_or_mcp_r + + def feature_enabled_or_default(is_enabled: Any, default_factory: Any) -> Any: """Gate a view on a feature predicate. @@ -1458,7 +1475,7 @@ def logout(): lambda: is_login_enabled() and is_myopencre_enabled(), lambda: jsonify({"selected": []}), ) -@login_required +@login_or_mcp_required @identity.require_scopes(identity.SCOPE_MYOPENCRE_READ) def get_user_resources() -> Any: """Return the standard names the current user has selected.""" @@ -1475,7 +1492,7 @@ def get_user_resources() -> Any: lambda: is_login_enabled() and is_myopencre_enabled(), lambda: jsonify({"selected": []}), ) -@login_required +@login_or_mcp_required @identity.require_scopes(identity.SCOPE_MYOPENCRE_WRITE) def put_user_resources() -> Any: """Replace the current user's selected standards.""" diff --git a/scripts/device_flow_check.py b/scripts/device_flow_check.py deleted file mode 100644 index 4357635c2..000000000 --- a/scripts/device_flow_check.py +++ /dev/null @@ -1,124 +0,0 @@ -"""Standalone Google device-flow check. No OpenCRE involved. - -First run : full device flow, saves the refresh token to .mcp_tokens.json -Later runs : refreshes silently, no browser, no re-approval - -This mirrors what application/mcp/auth.py will do in v2, with a file -standing in for the OS keyring. -""" - -import base64 -import json -import os -import pathlib -import sys -import time - -import requests - -try: - from dotenv import load_dotenv - - load_dotenv() # pull GOOGLE_MCP_* out of .env at the repo root -except ImportError: - pass - -DEVICE_CODE_URL = "https://oauth2.googleapis.com/device/code" -TOKEN_URL = "https://oauth2.googleapis.com/token" -DEVICE_GRANT = "urn:ietf:params:oauth:grant-type:device_code" -SCOPES = "openid email profile" # NEVER add a fourth - see docs/api/mcp.md -TOKEN_FILE = pathlib.Path(__file__).parent / ".mcp_tokens.json" - -CLIENT_ID = os.environ.get("GOOGLE_MCP_CLIENT_ID") -CLIENT_SECRET = os.environ.get("GOOGLE_MCP_CLIENT_SECRET") -if not CLIENT_ID or not CLIENT_SECRET: - sys.exit( - "GOOGLE_MCP_CLIENT_ID / GOOGLE_MCP_CLIENT_SECRET not found.\n" - "Put them in .env at the repo root, or export them in this shell." - ) - - -def claims_of(id_token: str) -> dict: - """Decode WITHOUT verifying - display only. Real code verifies the signature.""" - payload = id_token.split(".")[1] - return json.loads(base64.urlsafe_b64decode(payload + "=" * (-len(payload) % 4))) - - -def refresh(refresh_token: str) -> dict | None: - """Trade a refresh token for fresh tokens. None if it is no longer valid.""" - r = requests.post( - TOKEN_URL, - data={ - "client_id": CLIENT_ID, - "client_secret": CLIENT_SECRET, - "refresh_token": refresh_token, - "grant_type": "refresh_token", - }, - timeout=30, - ) - if r.status_code == 200: - return r.json() - print(f" refresh rejected [{r.status_code}] - falling back to device flow") - return None - - -def device_flow() -> dict: - """Full approval flow: print a code, wait for the human, return tokens.""" - r = requests.post( - DEVICE_CODE_URL, - data={"client_id": CLIENT_ID, "scope": SCOPES}, - timeout=30, - ) - if r.status_code != 200: - # Google's body says WHY (wrong client type, consent screen incomplete...). - sys.exit(f"device/code failed [{r.status_code}]: {r.text}") - d = r.json() - - # Google spells it verification_url; RFC 8628 says verification_uri. - url = d.get("verification_url") or d["verification_uri"] - print(f"\n Open: {url}\n Code: {d['user_code']}\n") - - interval = d.get("interval", 5) - deadline = time.time() + d["expires_in"] - while time.time() < deadline: - time.sleep(interval) - t = requests.post( - TOKEN_URL, - data={ - "client_id": CLIENT_ID, - "client_secret": CLIENT_SECRET, - "device_code": d["device_code"], - "grant_type": DEVICE_GRANT, - }, - timeout=30, - ) - body = t.json() - if t.status_code == 200: - return body - err = body.get("error") - if err == "authorization_pending": - print(" waiting...") - continue - if err == "slow_down": - interval += 5 - continue - sys.exit(f"failed: {body}") - sys.exit("timed out - the code expired before it was approved") - - -saved = json.loads(TOKEN_FILE.read_text()) if TOKEN_FILE.exists() else {} -tokens = refresh(saved["refresh_token"]) if saved.get("refresh_token") else None -if tokens is None: - tokens = device_flow() - -# A refresh response omits refresh_token - keep the one we already hold. -keep = tokens.get("refresh_token") or saved.get("refresh_token") -TOKEN_FILE.write_text(json.dumps({"refresh_token": keep, "saved_at": time.time()})) - -claims = claims_of(tokens["id_token"]) -print("refresh token held :", bool(keep)) -print("sub :", claims["sub"]) -print("aud :", claims["aud"]) # must be your MCP client id -print("email :", claims.get("email")) -print("id_token expires :", int(claims["exp"] - time.time()), "seconds") -print(f"\nsaved to {TOKEN_FILE.name} - re-run to confirm it refreshes with no browser")