diff --git a/README.md b/README.md index 7a7388d..5b95f7d 100644 --- a/README.md +++ b/README.md @@ -326,7 +326,7 @@ Every setting is an environment variable. Required ones in **bold**. #### Choosing a transport opik-mcp performs **no local credential validation** on HTTP transport: any -well-formed `Authorization: Bearer …` (an Opik API key or an `opik_at_…` +well-formed `Authorization: Bearer …` (an Opik API key or an `opik_mcp_at_…` OAuth access token) is forwarded verbatim to opik-backend, which is the single point of auth enforcement. Pick the transport by deployment shape: diff --git a/src/opik_mcp/analytics/client.py b/src/opik_mcp/analytics/client.py index 0d96a14..e2f961c 100644 --- a/src/opik_mcp/analytics/client.py +++ b/src/opik_mcp/analytics/client.py @@ -260,9 +260,9 @@ def _per_request_props(self) -> dict[str, str]: ``auth_mode`` is ALWAYS set so stdio / no-auth events are not a dark cohort. PRIVACY: the raw bearer token never enters the result — only its - sha256 digest, and only for ``opik_at_`` OAuth tokens. ``request_workspace`` - mirrors the existing plaintext ``workspace`` posture (workspace names are - used as ``user_id`` in ``resolve_anonymous_id``). + sha256 digest, and only for ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed OAuth + tokens. ``request_workspace`` mirrors the existing plaintext ``workspace`` + posture (workspace names are used as ``user_id`` in ``resolve_anonymous_id``). """ props: dict[str, str] = {} try: diff --git a/src/opik_mcp/auth_context.py b/src/opik_mcp/auth_context.py index 4bfec29..b82db10 100644 --- a/src/opik_mcp/auth_context.py +++ b/src/opik_mcp/auth_context.py @@ -1,7 +1,8 @@ """Per-request inbound-auth propagation for OAuth-passthrough mode. When opik-mcp runs over HTTP transport with OAuth, the MCP host attaches -`Authorization: Bearer opik_at_…` per RFC 6750 and opik-mcp's job is to +`Authorization: Bearer …` (an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed token) +per RFC 6750 and opik-mcp's job is to forward that bearer onward to opik-backend's data API verbatim. Permission enforcement lives at the data API endpoint via `@RequiredPermissions` annotations; opik-mcp performs no local validation and makes no separate @@ -20,8 +21,14 @@ from contextvars import ContextVar -# Full inbound ``Authorization`` header value (e.g. ``"Bearer opik_at_…"``), -# forwarded verbatim on outbound calls to opik-backend's data API. ``None`` +# Access-token prefix minted by opik-backend (McpOAuthTokenUtils.ACCESS_PREFIX). +# OAuth-passthrough detection MUST match the issuer: a mismatch makes a real +# OAuth bearer fall through to the API-key path, which then forwards a stale +# Comet-Workspace header that opik-backend rejects with 403. +OAUTH_ACCESS_TOKEN_PREFIX = "opik_mcp_at_" + +# Full inbound ``Authorization`` header value (an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed +# bearer), forwarded verbatim on outbound calls to opik-backend's data API. ``None`` # means "no inbound bearer; fall back to settings.opik_api_key". inbound_authorization: ContextVar[str | None] = ContextVar("inbound_authorization", default=None) @@ -36,20 +43,20 @@ def classify_bearer(auth_header: str) -> tuple[str, str]: """Classify a non-empty inbound ``Authorization`` header for BI analytics. Returns ``(auth_mode, oauth_token)``: - - ``("oauth", "")`` for an OAuth bearer — the token is returned - ONLY so the caller can hash it; it is never stored or emitted raw. + - ``("oauth", token)`` for an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed bearer — the + token is returned ONLY so the caller can hash it; never stored or emitted raw. - ``("api_key", "")`` for any other forwarded credential (the token is NOT returned — api-key-shaped credentials are not hashed here). Mirrors ``opik_client.resolve_opik_config``'s OAuth detection - (``partition(" ")`` + ``lstrip`` + ``opik_at_`` prefix) so BI's ``auth_mode`` - / ``token_sha256`` agree with the credential actually forwarded outbound. - Single source of truth shared by ``analytics.client._build_event`` and - ``server.AuthRejectionMiddleware`` so the two cannot drift. + (``partition(" ")`` + ``lstrip`` + ``OAUTH_ACCESS_TOKEN_PREFIX``) so BI's + ``auth_mode`` / ``token_sha256`` agree with the credential actually forwarded + outbound. Single source of truth shared by ``analytics.client._build_event`` + and ``server.AuthRejectionMiddleware`` so the two cannot drift. """ scheme, _, token_raw = auth_header.partition(" ") token = token_raw.lstrip() - if scheme.lower() == "bearer" and token.startswith("opik_at_"): + if scheme.lower() == "bearer" and token.startswith(OAUTH_ACCESS_TOKEN_PREFIX): return "oauth", token return "api_key", "" diff --git a/src/opik_mcp/opik_client.py b/src/opik_mcp/opik_client.py index 19183c3..26726ff 100644 --- a/src/opik_mcp/opik_client.py +++ b/src/opik_mcp/opik_client.py @@ -20,7 +20,11 @@ import httpx -from opik_mcp.auth_context import inbound_authorization, inbound_workspace +from opik_mcp.auth_context import ( + OAUTH_ACCESS_TOKEN_PREFIX, + inbound_authorization, + inbound_workspace, +) from opik_mcp.config import DEFAULT_WORKSPACE, MissingConfigError, Settings from opik_mcp.error_kinds import ErrorKind @@ -628,8 +632,9 @@ def resolve_opik_config(settings: Settings) -> tuple[str, str, str | None]: header (OAuth-passthrough mode), the middleware populates :mod:`opik_mcp.auth_context` ContextVars and we prefer those over the env-bound ``OPIK_API_KEY`` / ``COMET_WORKSPACE``. opik-backend's - ``AuthFilter`` accepts both shapes (API key and ``Bearer opik_at_…``) - and enforces ``@RequiredPermissions`` per endpoint, so opik-mcp is a + ``AuthFilter`` accepts both shapes (API key and an + ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed ``Bearer``) and enforces + ``@RequiredPermissions`` per endpoint, so opik-mcp is a thin forwarder either way. """ inbound_auth = inbound_authorization.get() @@ -641,12 +646,14 @@ def resolve_opik_config(settings: Settings) -> tuple[str, str, str | None]: ) # OAuth access tokens carry their workspace server-side (opik-backend # derives it from the token row). Identified by token prefix — mirrors - # the backend's McpOAuthTokens.isMcpOAuthToken — so an API key that + # the backend's McpOAuthTokenUtils.isMcpOAuthToken — so an API key that # merely contains the marker can't skip the workspace requirement. oauth_passthrough = False if inbound_auth is not None: scheme, _, token = inbound_auth.partition(" ") - oauth_passthrough = scheme.lower() == "bearer" and token.lstrip().startswith("opik_at_") + oauth_passthrough = scheme.lower() == "bearer" and token.lstrip().startswith( + OAUTH_ACCESS_TOKEN_PREFIX + ) if oauth_passthrough: # Workspace is derived from the token server-side; may be None here. workspace = inbound_ws diff --git a/src/opik_mcp/server.py b/src/opik_mcp/server.py index 29c32ed..fc401b1 100644 --- a/src/opik_mcp/server.py +++ b/src/opik_mcp/server.py @@ -592,7 +592,7 @@ class BearerAuthMiddleware(BaseHTTPMiddleware): captured into a ContextVar and forwarded verbatim on the outbound call to opik-backend (see :mod:`opik_mcp.auth_context`). opik-backend's ``AuthFilter`` is the single point of auth enforcement — it validates the - bearer (API key or ``opik_at_…`` OAuth token) and enforces + bearer (API key or an ``OAUTH_ACCESS_TOKEN_PREFIX``-prefixed OAuth token) and enforces ``@RequiredPermissions`` on the data API endpoint. Deployments where the backend enforces auth are protected end-to-end; OSS installs without backend auth are as open via MCP as via their own REST API. diff --git a/tests/test_analytics_auth_rejected.py b/tests/test_analytics_auth_rejected.py index 3abc82c..ac31ff7 100644 --- a/tests/test_analytics_auth_rejected.py +++ b/tests/test_analytics_auth_rejected.py @@ -14,10 +14,11 @@ import pytest +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX from opik_mcp.config import Settings # Unique bearer token canary — must never appear raw in the emitted event. -RAW_TOKEN_CANARY = "opik_at_AUTHREJECT-CANARY-TOKEN-3f9a2b1c" +RAW_TOKEN_CANARY = f"{OAUTH_ACCESS_TOKEN_PREFIX}AUTHREJECT-CANARY-TOKEN-3f9a2b1c" class _Recorder: @@ -94,19 +95,19 @@ def test_empty_bearer_token_reason(monkeypatch: pytest.MonkeyPatch) -> None: def test_421_is_host_rejected(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 421) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_x") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x".encode()) assert _only(recorder)["rejection_reason"] == "host_rejected" def test_403_is_origin_rejected(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 403) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_x") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x".encode()) assert _only(recorder)["rejection_reason"] == "origin_rejected" def test_success_emits_no_event(monkeypatch: pytest.MonkeyPatch) -> None: recorder, mw = _make(monkeypatch, 200) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_x") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x".encode()) assert not [p for et, p in recorder.events if et == "opik_mcp_auth_rejected"] @@ -177,7 +178,7 @@ def test_auth_mode_reflects_rejected_oauth_bearer(monkeypatch: pytest.MonkeyPatc # auth_mode="oauth" — derived from the header, NOT the already-reset # ContextVar (which would yield the settings fallback). recorder, mw = _make(monkeypatch, 421) - _drive(mw, path="/mcp", auth=b"Bearer opik_at_valid-token") + _drive(mw, path="/mcp", auth=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}valid-token".encode()) props = _only(recorder) assert props["rejection_reason"] == "host_rejected" assert props["auth_mode"] == "oauth" diff --git a/tests/test_analytics_client_build_event.py b/tests/test_analytics_client_build_event.py index b6c34bc..0189da2 100644 --- a/tests/test_analytics_client_build_event.py +++ b/tests/test_analytics_client_build_event.py @@ -21,11 +21,15 @@ import pytest from opik_mcp.analytics.client import AnalyticsClient -from opik_mcp.auth_context import inbound_authorization, inbound_workspace +from opik_mcp.auth_context import ( + OAUTH_ACCESS_TOKEN_PREFIX, + inbound_authorization, + inbound_workspace, +) from opik_mcp.config import Settings # Canaries: unique, greppable values that must never appear raw in an event. -RAW_OAUTH_TOKEN = "opik_at_BEARER-CANARY-TOKEN-UNIQUE-7a3b2c1d" +RAW_OAUTH_TOKEN = f"{OAUTH_ACCESS_TOKEN_PREFIX}BEARER-CANARY-TOKEN-UNIQUE-7a3b2c1d" RAW_WORKSPACE = "WORKSPACE-CANARY-NAME-MUST-NOT-LEAK-9f4e5a6b" @@ -80,7 +84,7 @@ def test_non_oauth_bearer_is_api_key_mode_without_token_hash(make_client: Any) - with _inbound(auth="Bearer some-non-oauth-static-key"): props = client._build_event("opik_mcp_tool_called", {})["event_properties"] assert props["auth_mode"] == "api_key" - assert "token_sha256" not in props # only opik_at_ bearers are hashed + assert "token_sha256" not in props # only OAUTH_ACCESS_TOKEN_PREFIX-prefixed bearers are hashed def test_request_workspace_plaintext_present(make_client: Any) -> None: diff --git a/tests/test_analytics_privacy.py b/tests/test_analytics_privacy.py index 23036e7..e7153a6 100644 --- a/tests/test_analytics_privacy.py +++ b/tests/test_analytics_privacy.py @@ -23,6 +23,7 @@ import pytest +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX from opik_mcp.comet_client import PodDiscovery from opik_mcp.ollie_client import OnTick, SSEEvent @@ -50,7 +51,7 @@ "FORBIDDEN-CANARY-home-path-5e6f7a8b", # OAuth bearer token canary — auth_rejected derives its reason from the # header SHAPE only; the raw token must never reach the event. - "opik_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c", + f"{OAUTH_ACCESS_TOKEN_PREFIX}FORBIDDEN-CANARY-oauth-token-2f8e1d3c", ] @@ -879,11 +880,12 @@ def hi() -> str: ) # Drive the emit path directly with a canary-laden bearer token; the # event must carry only the bucketed reason/auth_mode, never the token. + bearer = f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}FORBIDDEN-CANARY-oauth-token-2f8e1d3c" scope = { "type": "http", "path": "/mcp", "headers": [ - (b"authorization", b"Bearer opik_at_FORBIDDEN-CANARY-oauth-token-2f8e1d3c"), + (b"authorization", bearer.encode()), ], } mw._emit_rejection(scope, 401) diff --git a/tests/test_http_auth.py b/tests/test_http_auth.py index bb09d55..ef3b495 100644 --- a/tests/test_http_auth.py +++ b/tests/test_http_auth.py @@ -10,6 +10,8 @@ import httpx import pytest +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX + INITIALIZE = { "jsonrpc": "2.0", "id": 1, @@ -48,7 +50,7 @@ async def test_any_bearer_initializes(http_client: httpx.AsyncClient) -> None: "/mcp", json=INITIALIZE, headers={ - "Authorization": "Bearer opik_at_anything", + "Authorization": f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}anything", "Accept": "application/json, text/event-stream", }, ) diff --git a/tests/test_oauth_passthrough_mode.py b/tests/test_oauth_passthrough_mode.py index 77efcd4..44aea12 100644 --- a/tests/test_oauth_passthrough_mode.py +++ b/tests/test_oauth_passthrough_mode.py @@ -11,7 +11,11 @@ from starlette.requests import Request from starlette.responses import JSONResponse, Response -from opik_mcp.auth_context import inbound_authorization, inbound_workspace +from opik_mcp.auth_context import ( + OAUTH_ACCESS_TOKEN_PREFIX, + inbound_authorization, + inbound_workspace, +) from opik_mcp.server import BearerAuthMiddleware @@ -51,7 +55,7 @@ async def test_passthrough_accepts_any_well_formed_bearer() -> None: opik-backend's AuthFilter validates the token; opik-mcp is a thin pipe. """ mw = _build_middleware() - request = _make_request({"authorization": "Bearer opik_at_abc123"}) + request = _make_request({"authorization": f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123"}) captured: dict[str, str | None] = {} @@ -64,7 +68,7 @@ async def call_next(_r: Request) -> Response: assert resp.status_code == 200 # Bearer captured exactly as inbound so outbound forwarding preserves it. - assert captured["auth"] == "Bearer opik_at_abc123" + assert captured["auth"] == f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123" # No workspace header on this request → ContextVar reads as None. assert captured["workspace"] is None # ContextVar is reset after the request returns — no leakage to the @@ -78,7 +82,7 @@ async def test_passthrough_captures_comet_workspace_header() -> None: mw = _build_middleware() request = _make_request( { - "authorization": "Bearer opik_at_abc", + "authorization": f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc", "comet-workspace": "my-team", } ) diff --git a/tests/test_opik_client.py b/tests/test_opik_client.py index 3371a9c..75b620d 100644 --- a/tests/test_opik_client.py +++ b/tests/test_opik_client.py @@ -2,6 +2,7 @@ import pytest import respx +from opik_mcp.auth_context import OAUTH_ACCESS_TOKEN_PREFIX from opik_mcp.opik_client import ( FeedbackScore, OpikAuthError, @@ -363,13 +364,13 @@ def test_resolve_opik_config_oauth_token_makes_workspace_optional() -> None: from opik_mcp.opik_client import resolve_opik_config s = Settings(opik_api_key=None, comet_workspace=None, opik_url="https://opik.example.com") - token = inbound_authorization.set("Bearer opik_at_abc123") + token = inbound_authorization.set(f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123") try: _base, api_key, workspace = resolve_opik_config(s) finally: inbound_authorization.reset(token) - assert api_key == "Bearer opik_at_abc123" + assert api_key == f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}abc123" assert workspace is None @@ -383,7 +384,7 @@ def test_resolve_opik_config_oauth_detection_is_prefix_not_substring() -> None: from opik_mcp.opik_client import resolve_opik_config s = Settings(opik_api_key=None, comet_workspace=None, opik_url="https://opik.example.com") - token = inbound_authorization.set("Bearer sk-xopik_at_y") + token = inbound_authorization.set(f"Bearer sk-x{OAUTH_ACCESS_TOKEN_PREFIX}y") try: _base, _api_key, workspace = resolve_opik_config(s) finally: @@ -395,11 +396,13 @@ def test_oauth_client_omits_workspace_header() -> None: from opik_mcp.opik_client import OpikClient client = OpikClient( - base_url="https://opik.example.com", api_key="Bearer opik_at_x", workspace=None + base_url="https://opik.example.com", + api_key=f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x", + workspace=None, ) headers = client._headers() assert "Comet-Workspace" not in headers - assert headers["Authorization"] == "Bearer opik_at_x" + assert headers["Authorization"] == f"Bearer {OAUTH_ACCESS_TOKEN_PREFIX}x" # --- execute_experiment ------------------------------------------------------- #