From 8aa31b9f00d5e34d4dd53778c0bf434ff15f3125 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9=20Delion?= Date: Mon, 13 Apr 2026 14:52:29 +0200 Subject: [PATCH 1/3] chore(oauth2): Claim Protected Resource protocol and proxy some path to overcome this issue on Claude Web side : https://github.com/anthropics/claude-ai-mcp/issues/82 --- .../src/developer_mcp_server/http_app.py | 1 + .../gg_api_core/src/gg_api_core/client.py | 83 +---- .../gg_api_core/src/gg_api_core/mcp_server.py | 62 +++- .../src/gg_api_core/oauth_proxy_auth.py | 338 ++++++++++++++++++ .../gg_api_core/src/gg_api_core/settings.py | 18 +- packages/gg_api_core/src/gg_api_core/urls.py | 62 ++++ .../src/secops_mcp_server/http_app.py | 1 + tests/test_client.py | 23 ++ tests/test_oauth_proxy_middleware.py | 67 ++++ 9 files changed, 583 insertions(+), 72 deletions(-) create mode 100644 packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py create mode 100644 packages/gg_api_core/src/gg_api_core/urls.py create mode 100644 tests/test_oauth_proxy_middleware.py diff --git a/packages/developer_mcp_server/src/developer_mcp_server/http_app.py b/packages/developer_mcp_server/src/developer_mcp_server/http_app.py index d30501e1..42813a1f 100644 --- a/packages/developer_mcp_server/src/developer_mcp_server/http_app.py +++ b/packages/developer_mcp_server/src/developer_mcp_server/http_app.py @@ -14,6 +14,7 @@ http_app = create_streamable_http_app( server=mcp, streamable_http_path="/mcp", + auth=mcp.auth, json_response=True, stateless_http=True, ) diff --git a/packages/gg_api_core/src/gg_api_core/client.py b/packages/gg_api_core/src/gg_api_core/client.py index ab87f092..27fdc30a 100644 --- a/packages/gg_api_core/src/gg_api_core/client.py +++ b/packages/gg_api_core/src/gg_api_core/client.py @@ -10,13 +10,20 @@ import httpx -from gg_api_core.host import is_self_hosted_instance from gg_api_core.settings import get_settings # Setup logger logger = logging.getLogger(__name__) +class DownstreamUnauthorizedError(Exception): + """Raised when the downstream GitGuardian API returns 401. + + Bridged to an HTTP 401 + ``WWW-Authenticate`` response by middleware so + the MCP client can re-run the OAuth flow. + """ + + class IncidentSeverity(str, Enum): """Enum for incident severity levels.""" @@ -277,10 +284,12 @@ def __init__( self._token_info: Any | None = None def _init_urls(self, gitguardian_url: str | None = None): + from .urls import derive_public_api_url + # Use provided raw URL or get from environment with default fallback raw_url = gitguardian_url or get_settings().gitguardian_url - self.public_api_url = self._normalize_api_url(raw_url) + self.public_api_url = derive_public_api_url(raw_url) logger.info(f"Using API URL: {self.public_api_url}") # Extract the base URL for dashboard (needed for OAuth) @@ -289,72 +298,6 @@ def _init_urls(self, gitguardian_url: str | None = None): self.private_api_url = f"{self.dashboard_url}/api/v1" logger.info(f"Using private API URL: {self.private_api_url}") - def _normalize_api_url(self, api_url: str) -> str: - """ - Normalize the API URL for different GitGuardian instance types. - - Args: - api_url: Raw API URL or base URL - - Returns: - str: Normalized API URL - """ - from urllib.parse import urlparse - - # Strip trailing slashes - api_url = api_url.rstrip("/") - - try: - parsed = urlparse(api_url) - - # Special handling for localhost and 127.0.0.1 - always treat as self-hosted - # regardless of SAAS_HOSTNAMES list (used for local development) - is_localhost = parsed.netloc.startswith("localhost") or parsed.netloc.startswith("127.0.0.1") - - # Check if this is a SaaS URL (dashboard or API) - if not is_localhost and not is_self_hosted_instance(api_url): - # Convert dashboard URLs to API URLs with /v1 suffix - if "dashboard" in parsed.netloc: - api_netloc = parsed.netloc.replace("dashboard", "api") - normalized_url = f"{parsed.scheme}://{api_netloc}/v1" - logger.debug(f"Normalized SaaS dashboard URL: {api_url} -> {normalized_url}") - return normalized_url - # For API URLs, ensure they have /v1 suffix - elif not parsed.path.endswith("/v1"): - normalized_url = f"{api_url}/v1" - logger.debug(f"Normalized SaaS API URL: {api_url} -> {normalized_url}") - return normalized_url - else: - logger.debug(f"SaaS API URL already has /v1: {api_url}") - return api_url - - # Check if this already has the API path structure - path = parsed.path.lower() - if path.endswith("/v1") or path.endswith("/exposed/v1"): - logger.debug(f"API URL already has API path: {api_url}") - return api_url - - # This appears to be a self-hosted base URL - append the API path - if not path or path == "/" or not path.startswith("/exposed"): - normalized_url = f"{api_url}/exposed/v1" - logger.info(f"Normalized self-hosted base URL: {api_url} -> {normalized_url}") - return normalized_url - - # If it has /exposed but no /v1, append /v1 - if path.startswith("/exposed") and not path.endswith("/v1"): - normalized_url = f"{api_url}/v1" - logger.info(f"Normalized self-hosted API URL: {api_url} -> {normalized_url}") - return normalized_url - - # Default: return as-is - logger.debug(f"Using API URL as provided: {api_url}") - return api_url - - except Exception as e: - logger.warning(f"Failed to parse API URL '{api_url}': {e}") - logger.warning("Using API URL as provided") - return api_url - def _get_dashboard_url(self) -> str: """ Get the GitGuardian dashboard URL by deriving it from the API URL. @@ -603,6 +546,10 @@ async def _request(self, method: str, endpoint: str, **kwargs) -> Any: logger.exception(f"HTTP error occurred: {e.response.status_code} - {e.response.reason_phrase}") logger.debug(f"Error response content: {e.response.text}") logger.debug(f"Failed URL: {url}") + if e.response.status_code == 401: + raise DownstreamUnauthorizedError( + f"GitGuardian API returned 401 for {url}" + ) from e raise except httpx.RequestError as e: logger.exception(f"Request error occurred: {str(e)}") diff --git a/packages/gg_api_core/src/gg_api_core/mcp_server.py b/packages/gg_api_core/src/gg_api_core/mcp_server.py index 65afc957..260d0b0b 100644 --- a/packages/gg_api_core/src/gg_api_core/mcp_server.py +++ b/packages/gg_api_core/src/gg_api_core/mcp_server.py @@ -9,15 +9,22 @@ from fastmcp import FastMCP from fastmcp.exceptions import ValidationError -from fastmcp.server.dependencies import get_http_headers +from fastmcp.server.dependencies import get_access_token, get_http_headers from fastmcp.server.middleware import Middleware from fastmcp.tools import Tool -from gg_api_core.client import GitGuardianClient +from gg_api_core.client import DownstreamUnauthorizedError, GitGuardianClient from gg_api_core.icons import get_gitguardian_icons +from gg_api_core.oauth_proxy_auth import ( + PassThroughTokenVerifier, + create_oauth_proxy, + mark_downstream_unauthorized, +) +from gg_api_core.oauth_proxy_auth import create_oauth_proxy from gg_api_core.settings import get_settings from gg_api_core.utils import get_client + # Configure logger logger = logging.getLogger(__name__) @@ -31,6 +38,9 @@ class AuthenticationMode(Enum): PERSONAL_ACCESS_TOKEN_ENV_VAR = "PERSONAL_ACCESS_TOKEN_ENV_VAR" # Use per-request Authorization header AUTHORIZATION_HEADER = "AUTHORIZATION_HEADER" + # Use FastMCP OAuthProxy : declares the MCP server as a Protected Resource (RFC 9728) + # whose Authorization Server is api.gitguardian.com + OAUTH_PROXY = "OAUTH_PROXY" class CachedTokenInfoMixin: @@ -92,6 +102,22 @@ async def get_token_info(self) -> dict[str, Any]: return self._token_info +class DownstreamUnauthorizedMiddleware(Middleware): + """Flag the request when a tool surfaces a downstream 401. + + The exception still propagates so FastMCP serializes a JSON-RPC error + body for clients that ignore the HTTP status. The ASGI middleware + rewrites the status to 401 based on the flag set here. + """ + + async def on_message(self, context, call_next): + try: + return await call_next(context) + except DownstreamUnauthorizedError: + mark_downstream_unauthorized() + raise + + class ScopeFilteringMiddleware(Middleware): """Middleware to filter tools based on token scopes.""" @@ -143,6 +169,7 @@ def __init__(self, *args, default_scopes: list[str] | None = None, **kwargs): self._tool_scopes: dict[str, set[str]] = {} self.add_middleware(ScopeFilteringMiddleware(self)) + self.add_middleware(DownstreamUnauthorizedMiddleware()) def clear_cache(self) -> None: """Clear cached data. Override in subclasses that cache.""" @@ -371,10 +398,41 @@ async def get_token_info(self) -> dict[str, Any]: return await self._fetch_token_info_from_api() +class GitGuardianOAuthProxyMCP(AbstractGitGuardianFastMCP): + """GitGuardian MCP server with thin OAuth proxy to the GG dashboard. + + Same-origin OAuth endpoints proxy auth requests to the GG dashboard. + The MCP client gets the real GG PAT directly as Bearer token. + """ + + authentication_mode = AuthenticationMode.OAUTH_PROXY + + def get_personal_access_token(self) -> str: + # TODO(TIM): Why different than GitGuardianAuthorizationHeaderMCP ? + access_token = get_access_token() + + if not access_token: + raise ValidationError("No access token available - OAuth proxy authentication required") + return access_token.token + + async def get_token_info(self) -> dict[str, Any]: + return await self._fetch_token_info_from_api() + + def get_mcp_server(*args, **kwargs) -> AbstractGitGuardianFastMCP: kwargs.setdefault("icons", get_gitguardian_icons()) settings = get_settings() + + if settings.is_oauth_proxy_enabled: + oauth_proxy = create_oauth_proxy( + base_url=settings.mcp_base_url, + gg_url=settings.gitguardian_url, + gg_api_url=settings.gitguardian_api_url, + advertised_scopes=settings.effective_scopes, + ) + return GitGuardianOAuthProxyMCP(*args, auth=oauth_proxy, **kwargs) + if settings.is_oauth_enabled: return GitGuardianLocalOAuthMCP(*args, **kwargs) diff --git a/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py b/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py new file mode 100644 index 00000000..ef80443f --- /dev/null +++ b/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py @@ -0,0 +1,338 @@ +"""Thin OAuth proxy for GitGuardian MCP Server. + +Routes same-origin OAuth endpoints to the GG dashboard to work around +Claude.ai issue #82 (requires OAuth endpoints on same origin as MCP server). + +Architecture: single OAuth loop, no JWT issuance, no server-side token storage. +The MCP client gets the real GG PAT directly and sends it as Bearer token. + + MCP Client ──Bearer PAT──► MCP Server ──PAT──► GG API + │ + /authorize ─────┼──► 302 to GG dashboard /auth/login + /token ─────────┼──► proxy to GG API /oauth/token + /register ──────┼──► proxy to GG API /oauth/register +""" + +import logging +import os +from contextvars import ContextVar +from urllib.parse import urlencode + +import httpx +from fastmcp.server.auth.auth import AccessToken, TokenVerifier +from starlette.middleware import Middleware +from starlette.requests import Request +from starlette.responses import JSONResponse, RedirectResponse +from starlette.routing import Route +from starlette.types import ASGIApp, Message, Receive, Scope, Send + +logger = logging.getLogger(__name__) + + +class AdvertiseAuthorizationServerMetadataMiddleware: + """Append the AS metadata URL to ``WWW-Authenticate`` on 401 responses. + + Claude.ai does not follow ``authorization_servers`` from RFC 9728 protected + resource metadata (https://github.com/anthropics/claude-ai-mcp/issues/82), + so we expose the AS metadata location directly via an ``as_metadata`` + parameter alongside the existing ``resource_metadata`` one. + """ + + def __init__(self, app: ASGIApp, as_metadata_url: str): + self.app = app + self._param = f'as_metadata="{as_metadata_url}"' + self._param_bytes = self._param.encode() + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + async def send_wrapper(message: Message) -> None: + if message["type"] == "http.response.start" and message.get("status") == 401: + headers = list(message.get("headers", [])) + rewritten = [] + found = False + for name, value in headers: + if name.lower() == b"www-authenticate": + found = True + rewritten.append((name, value + b", " + self._param_bytes)) + else: + rewritten.append((name, value)) + if not found: + rewritten.append((b"www-authenticate", f"Bearer {self._param}".encode())) + message["headers"] = rewritten + await send(message) + + await self.app(scope, receive, send_wrapper) + + +class GitGuardianOAuthThinProxy(TokenVerifier): + """Thin OAuth proxy that routes auth requests to the GG dashboard. + + Serves same-origin OAuth endpoints so MCP clients that require + same-origin OAuth (Claude.ai as per https://github.com/anthropics/claude-ai-mcp/issues/82) can authenticate + with the GG dashboard. The client receives the real GG PAT — no intermediate JWTs or storage. + """ + + def __init__( + self, + gg_authorize_url: str, + gg_token_url: str, + gg_register_url: str, + gg_api_url: str, + gg_client_id: str = "ggshield_oauth", + advertised_scopes: list[str] | None = None, + **kwargs, + ): + super().__init__(**kwargs) + self.gg_authorize_url = gg_authorize_url + self.gg_token_url = gg_token_url + self.gg_register_url = gg_register_url + self.gg_api_url = gg_api_url.rstrip("/") + self.gg_client_id = gg_client_id + self._advertised_scopes = advertised_scopes + + @property + def scopes_supported(self) -> list[str]: + return self._advertised_scopes or ["scan"] + + # TODO(TIM): Should we implement this method or let actual calls fail with 401 ? + async def verify_token(self, token: str) -> AccessToken | None: + """Validate a GG PAT by calling /api_tokens/self.""" + try: + async with httpx.AsyncClient(timeout=10) as client: + response = await client.get( + f"{self.gg_api_url}/api_tokens/self", + headers={"Authorization": f"Token {token}"}, + ) + + if response.status_code != 200: + logger.warning(f"Token verification failed: HTTP {response.status_code}") + return None + + token_info = response.json() + scopes = token_info.get("scopes", []) + + return AccessToken( + token=token, + client_id=token_info.get("id", "unknown"), + scopes=scopes, + ) + except Exception: + logger.exception("Error verifying upstream token") + return None + + def get_middleware(self) -> list: + """Add downstream-401 handling and WWW-Authenticate advertising. + + Order matters. ``TranslateDownstreamUnauthorizedMiddleware`` runs + first so a flagged response becomes a 401; then + ``AdvertiseAuthorizationServerMetadataMiddleware`` appends the + ``as_metadata`` parameter to the resulting ``WWW-Authenticate`` + header. + """ + middleware = super().get_middleware() + middleware.append(Middleware(TranslateDownstreamUnauthorizedMiddleware)) + base = str(self.base_url).rstrip("/") if self.base_url else "" + if base: + middleware.append( + Middleware( + AdvertiseAuthorizationServerMetadataMiddleware, + as_metadata_url=f"{base}/.well-known/oauth-authorization-server", + ) + ) + return middleware + + def get_routes(self, mcp_path: str | None = None) -> list[Route]: + """Return OAuth proxy routes alongside discovery metadata.""" + self.set_mcp_path(mcp_path) + + return [ + Route( + f"/.well-known/oauth-protected-resource{mcp_path or ''}", + self._handle_resource_metadata, + methods=["GET"], + ), + # The following routes are served to workaround Claude.ai issue regarding + # Protected Resource metadata (RFC 9728) : https://github.com/anthropics/claude-ai-mcp/issues/82 + Route( + "/.well-known/oauth-authorization-server", + self._handle_authorization_server_metadata, + methods=["GET"], + ), + Route("/authorize", self._handle_authorize, methods=["GET"]), + Route("/token", self._handle_token, methods=["POST"]), + Route("/register", self._handle_register, methods=["POST"]), + ] + + async def _handle_authorization_server_metadata(self, request: Request) -> JSONResponse: + """Serve OAuth Authorization Server metadata (RFC 8414).""" + base = str(self.base_url).rstrip("/") + return JSONResponse( + { + "issuer": f"{base}/", + "authorization_endpoint": f"{base}/authorize", + "token_endpoint": f"{base}/token", + "registration_endpoint": f"{base}/register", + "scopes_supported": self.scopes_supported, + "response_types_supported": ["code"], + "grant_types_supported": ["authorization_code"], + "token_endpoint_auth_methods_supported": [ + "client_secret_post", + "client_secret_basic", + ], + "code_challenge_methods_supported": ["S256"], + }, + headers={"Cache-Control": "public, max-age=3600"}, + ) + + async def _handle_resource_metadata(self, request: Request) -> JSONResponse: + """Serve OAuth Protected Resource metadata (RFC 9728).""" + base = str(self.base_url).rstrip("/") + return JSONResponse( + { + "resource": str(self._resource_url), + "authorization_servers": [f"{base}/"], + "scopes_supported": self.scopes_supported, + "bearer_methods_supported": ["header"], + }, + headers={"Cache-Control": "public, max-age=3600"}, + ) + + async def _handle_register(self, request: Request) -> JSONResponse: + """Proxy Dynamic Client Registration (DCR) to the GG dashboard.""" + body = await request.body() + + async with httpx.AsyncClient(timeout=30) as client: + response = await client.post( + self.gg_register_url, + content=body, + headers={"Content-Type": "application/json"}, + ) + + return JSONResponse( + response.json(), + status_code=response.status_code, + ) + + async def _handle_authorize(self, request: Request) -> RedirectResponse: + """Redirect to GG dashboard authorization endpoint. + + Passes through all query params, including the client's own + ``client_id`` so RFC 7591 (DCR) clients reach the GG backend with + their registered identity. Falls back to ``self.gg_client_id`` only + when the request omits ``client_id``. + """ + params = dict(request.query_params) + params.setdefault("client_id", self.gg_client_id) + + # Add GG-specific params + params.setdefault("auth_mode", "ggshield_login") + params.setdefault("utm_source", "mcp") + params.setdefault("utm_medium", "oauth_proxy") + + separator = "&" if "?" in self.gg_authorize_url else "?" + url = f"{self.gg_authorize_url}{separator}{urlencode(params)}" + return RedirectResponse(url=url, status_code=302) + + async def _handle_token(self, request: Request) -> JSONResponse: + """Proxy token exchange to GG dashboard, transforming the response. + + Forwards the request to the GG token endpoint, passing through the + client's own ``client_id`` (and ``client_secret`` when present) so + RFC 7591 (DCR) clients can complete the code exchange with their + registered identity. Transforms ``{key: "..."}`` → + ``{access_token: "..."}`` in the response. + """ + body = await request.body() + form_data = dict(request.query_params) + + # Parse form body + for pair in body.decode().split("&"): + if "=" in pair: + k, v = pair.split("=", 1) + from urllib.parse import unquote_plus + + form_data[k] = unquote_plus(v) + + # Fall back to the configured client only when none was sent + form_data.setdefault("client_id", self.gg_client_id) + + # Add token name + token_name = os.environ.get("MCP_OAUTH_TOKEN_NAME", "MCP server token (OAuth Proxy)") + form_data.setdefault("name", token_name) + + # Add token lifetime + token_lifetime = os.environ.get("GITGUARDIAN_TOKEN_LIFETIME") + if token_lifetime and token_lifetime.lower() != "never": + form_data.setdefault("lifetime", token_lifetime) + + async with httpx.AsyncClient(timeout=30) as client: + response = await client.post( + self.gg_token_url, + data=form_data, + headers={"Content-Type": "application/x-www-form-urlencoded"}, + ) + + if response.status_code != 200: + return JSONResponse( + response.json() + if response.headers.get("content-type", "").startswith("application/json") + else {"error": response.text}, + status_code=response.status_code, + ) + + token_data = response.json() + + # Transform GG's non-standard response to OAuth standard + access_token = token_data.get("access_token") or token_data.get("key") + if not access_token: + return JSONResponse( + {"error": "server_error", "error_description": "No access token in upstream response"}, + status_code=500, + ) + + return JSONResponse( + { + "access_token": access_token, + "token_type": "Bearer", + "scope": " ".join(token_data.get("scope", ["scan"])) + if isinstance(token_data.get("scope"), list) + else token_data.get("scope", "scan"), + } + ) + + +def create_oauth_proxy( + base_url: str, + gg_url: str = "https://dashboard.gitguardian.com", + gg_api_url: str | None = None, + gg_client_id: str = "ggshield_oauth", + advertised_scopes: list[str] | None = None, +) -> GitGuardianOAuthThinProxy: + """Create a thin OAuth proxy that routes auth to the GG dashboard. + + Args: + base_url: Public URL of this MCP server. + gg_url: GG dashboard URL. + gg_api_url: GG API URL. If None, derived from gg_url. + gg_client_id: OAuth client ID registered on the GG dashboard. + advertised_scopes: Scopes advertised in AS / protected-resource + metadata so DCR clients (Claude.ai, Cursor, …) request them + at registration / authorize time. Defaults to ``["scan"]``. + """ + if gg_api_url is None: + from gg_api_core.urls import derive_public_api_url + + gg_api_url = derive_public_api_url(gg_url) # e.g. https://api.gitguardian.com/v1 + + return GitGuardianOAuthThinProxy( + gg_authorize_url=f"{gg_url}/auth/login", + gg_token_url=f"{gg_api_url}/oauth/token", + gg_register_url=f"{gg_api_url}/oauth/register", + gg_api_url=gg_api_url, + advertised_scopes=advertised_scopes, + gg_client_id=gg_client_id, + base_url=base_url, + ) diff --git a/packages/gg_api_core/src/gg_api_core/settings.py b/packages/gg_api_core/src/gg_api_core/settings.py index e006121f..db7f13a9 100644 --- a/packages/gg_api_core/src/gg_api_core/settings.py +++ b/packages/gg_api_core/src/gg_api_core/settings.py @@ -19,6 +19,7 @@ from .scopes import ServerProfile + TRUTHY_ENV_VALUES = frozenset( { "true", @@ -28,7 +29,9 @@ ) -def string_env_to_bool(value: str) -> bool: +def string_env_to_bool(value: str | None) -> bool: + if value is None: + return False return value.lower() in TRUTHY_ENV_VALUES @@ -42,6 +45,7 @@ class Settings(BaseSettings): # --- GitGuardian core --- gitguardian_url: str = "https://dashboard.gitguardian.com" + gitguardian_api_url: str | None = None gitguardian_personal_access_token: str | None = None # GITGUARDIAN_REQUESTED_SCOPES is the legacy name kept for backward compat. @@ -62,6 +66,10 @@ class Settings(BaseSettings): # None ⇒ unset (default: True). Empty/anything-but-"true" ⇒ False. enable_local_oauth: str | None = None + # --- OAuth proxy --- + mcp_oauth_proxy_enabled: str | None = None + mcp_base_url: str = "http://localhost:8000" + # --- Server profile --- # Set by each server entry-point (e.g. developer_mcp_server/server.py) to # signal which scope-set the OAuth flow may request. ``None`` means no @@ -95,6 +103,10 @@ def is_multi_tenant(self) -> bool: def use_dashboard_authenticated_page(self) -> bool: return string_env_to_bool(self.gitguardian_use_dashboard_authenticated_page) + @property + def is_oauth_proxy_enabled(self) -> bool: + return string_env_to_bool(self.mcp_oauth_proxy_enabled) + @property def requested_scopes(self) -> list[str]: """Scopes the user asked for via env, parsed and validated. @@ -135,7 +147,9 @@ def effective_scopes(self) -> list[str]: if self.server_profile is None: return self.requested_scopes - restricted = is_self_hosted_instance(self.gitguardian_url) and not is_local_instance(self.gitguardian_url) + restricted = is_self_hosted_instance( + self.gitguardian_url + ) and not is_local_instance(self.gitguardian_url) allowed = set(self.server_profile.max_scopes(restricted=restricted)) requested = set(self.requested_scopes) return sorted(allowed & requested if requested else allowed) diff --git a/packages/gg_api_core/src/gg_api_core/urls.py b/packages/gg_api_core/src/gg_api_core/urls.py new file mode 100644 index 00000000..f3bd48b1 --- /dev/null +++ b/packages/gg_api_core/src/gg_api_core/urls.py @@ -0,0 +1,62 @@ +"""GitGuardian URL helpers. + +Maps a GitGuardian dashboard URL (or already-normalized API URL) to its +public API URL, so callers can derive ``GITGUARDIAN_API_URL`` from +``GITGUARDIAN_URL`` without instantiating a :class:`GitGuardianClient`. +""" + +import logging +from urllib.parse import urlparse + +from .host import is_self_hosted_instance + +logger = logging.getLogger(__name__) + + +def derive_public_api_url(gitguardian_url: str) -> str: + """Normalize a GitGuardian dashboard or API URL to its public API URL. + + Handles: + * SaaS dashboard URL → corresponding ``api.*`` host with ``/v1`` suffix + * SaaS API URL → ensures ``/v1`` suffix + * Self-hosted / localhost base URL → appends ``/exposed/v1`` + (or ``/v1`` if the path already starts with ``/exposed``) + """ + api_url = gitguardian_url.rstrip("/") + + try: + parsed = urlparse(api_url) + # localhost is always treated as self-hosted, regardless of SAAS_HOSTNAMES + is_localhost = parsed.netloc.startswith("localhost") or parsed.netloc.startswith("127.0.0.1") + + # SaaS path + if not is_localhost and not is_self_hosted_instance(api_url): + if "dashboard" in parsed.netloc: + api_netloc = parsed.netloc.replace("dashboard", "api") + normalized = f"{parsed.scheme}://{api_netloc}/v1" + logger.debug(f"Normalized SaaS dashboard URL: {api_url} -> {normalized}") + return normalized + if not parsed.path.endswith("/v1"): + normalized = f"{api_url}/v1" + logger.debug(f"Normalized SaaS API URL: {api_url} -> {normalized}") + return normalized + return api_url + + # Self-hosted / localhost path + path = parsed.path.lower() + if path.endswith("/v1") or path.endswith("/exposed/v1"): + return api_url + + if not path or path == "/" or not path.startswith("/exposed"): + normalized = f"{api_url}/exposed/v1" + logger.info(f"Normalized self-hosted base URL: {api_url} -> {normalized}") + return normalized + + # path starts with /exposed but lacks /v1 + normalized = f"{api_url}/v1" + logger.info(f"Normalized self-hosted API URL: {api_url} -> {normalized}") + return normalized + + except Exception as e: + logger.warning(f"Failed to parse API URL '{api_url}': {e}; using as provided") + return api_url diff --git a/packages/secops_mcp_server/src/secops_mcp_server/http_app.py b/packages/secops_mcp_server/src/secops_mcp_server/http_app.py index 4e41be1e..52f77317 100644 --- a/packages/secops_mcp_server/src/secops_mcp_server/http_app.py +++ b/packages/secops_mcp_server/src/secops_mcp_server/http_app.py @@ -27,6 +27,7 @@ app = create_streamable_http_app( server=mcp, streamable_http_path="/mcp", + auth=mcp.auth, json_response=True, stateless_http=True, ) diff --git a/tests/test_client.py b/tests/test_client.py index ff0e48b2..88591102 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -124,6 +124,29 @@ async def test_request_error(self, client, mock_httpx_client): with pytest.raises(httpx.HTTPStatusError): await client._request_get("/test") + @pytest.mark.asyncio + async def test_request_401_raises_downstream_unauthorized(self, client, mock_httpx_client): + """Downstream 401 should surface as DownstreamUnauthorizedError so middleware can rewrite to HTTP 401.""" + from gg_api_core.client import DownstreamUnauthorizedError + + mock_request = MagicMock() + mock_response = MagicMock() + mock_response.status_code = 401 + mock_response.reason_phrase = "Unauthorized" + mock_response.text = "Unauthorized" + mock_response.json.return_value = {"detail": "Invalid API key."} + mock_response.raise_for_status.side_effect = httpx.HTTPStatusError( + "401", request=mock_request, response=mock_response + ) + + async_client_instance = AsyncMock() + async_client_instance.__aenter__.return_value = mock_httpx_client + mock_httpx_client.request = AsyncMock(return_value=mock_response) + + with patch("httpx.AsyncClient", return_value=async_client_instance): + with pytest.raises(DownstreamUnauthorizedError): + await client._request_get("/test") + @pytest.mark.asyncio async def test_create_honeytoken(self, client): """Test create_honeytoken method.""" diff --git a/tests/test_oauth_proxy_middleware.py b/tests/test_oauth_proxy_middleware.py new file mode 100644 index 00000000..e0edc61a --- /dev/null +++ b/tests/test_oauth_proxy_middleware.py @@ -0,0 +1,67 @@ +"""Tests for the OAuth-proxy ASGI middleware that converts downstream 401s.""" + +import pytest +from gg_api_core.oauth_proxy_auth import ( + AdvertiseAuthorizationServerMetadataMiddleware, + TranslateDownstreamUnauthorizedMiddleware, + mark_downstream_unauthorized, +) + + +async def _drive(app): + """Run an ASGI app once and return (status, headers).""" + captured = {} + + async def receive(): + return {"type": "http.request", "body": b"", "more_body": False} + + async def send(message): + if message["type"] == "http.response.start": + captured["status"] = message["status"] + captured["headers"] = message.get("headers", []) + + scope = {"type": "http", "path": "/mcp", "method": "POST", "headers": []} + await app(scope, receive, send) + return captured.get("status"), captured.get("headers", []) + + +@pytest.mark.asyncio +async def test_translate_rewrites_status_when_flag_set(): + async def inner_app(scope, receive, send): + mark_downstream_unauthorized() + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b""}) + + middleware = TranslateDownstreamUnauthorizedMiddleware(inner_app) + status, _ = await _drive(middleware) + assert status == 401 + + +@pytest.mark.asyncio +async def test_translate_passthrough_when_flag_unset(): + async def inner_app(scope, receive, send): + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b""}) + + middleware = TranslateDownstreamUnauthorizedMiddleware(inner_app) + status, _ = await _drive(middleware) + assert status == 200 + + +@pytest.mark.asyncio +async def test_advertise_adds_as_metadata_param_to_401(): + async def inner_app(scope, receive, send): + mark_downstream_unauthorized() + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b""}) + + # Stack as the proxy would: Translate (innermost) → Advertise (outermost). + stacked = AdvertiseAuthorizationServerMetadataMiddleware( + TranslateDownstreamUnauthorizedMiddleware(inner_app), + as_metadata_url="https://mcp.example.com/.well-known/oauth-authorization-server", + ) + status, headers = await _drive(stacked) + assert status == 401 + header_map = {name.decode(): value.decode() for name, value in headers} + assert "www-authenticate" in header_map + assert "as_metadata=" in header_map["www-authenticate"] From 20bc4ee9caea7f29a9066707f2fb747c0226f52c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9=20Delion?= Date: Tue, 12 May 2026 12:27:11 +0200 Subject: [PATCH 2/3] refactor(oauth2): unify bearer-token handling across header & proxy modes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both modes now read the bearer token from the request scope via FastMCP's `get_access_token()`. `GitGuardianOAuthThinProxy.verify_token` no longer round-trips to `/api_tokens/self` per request — a new `PassThroughTokenVerifier` trusts the token and lets downstream calls surface 401s. `GitGuardianAuthorizationHeaderMCP` reuses the same verifier so the manual `Authorization` header parser is gone. Issue: APPAI-524 --- .../gg_api_core/src/gg_api_core/client.py | 4 +- .../gg_api_core/src/gg_api_core/mcp_server.py | 72 +++++----------- .../src/gg_api_core/oauth_proxy_auth.py | 85 +++++++++++++------ .../gg_api_core/src/gg_api_core/settings.py | 5 +- tests/test_mcp_server.py | 77 +++-------------- 5 files changed, 91 insertions(+), 152 deletions(-) diff --git a/packages/gg_api_core/src/gg_api_core/client.py b/packages/gg_api_core/src/gg_api_core/client.py index 27fdc30a..6e37f87b 100644 --- a/packages/gg_api_core/src/gg_api_core/client.py +++ b/packages/gg_api_core/src/gg_api_core/client.py @@ -547,9 +547,7 @@ async def _request(self, method: str, endpoint: str, **kwargs) -> Any: logger.debug(f"Error response content: {e.response.text}") logger.debug(f"Failed URL: {url}") if e.response.status_code == 401: - raise DownstreamUnauthorizedError( - f"GitGuardian API returned 401 for {url}" - ) from e + raise DownstreamUnauthorizedError(f"GitGuardian API returned 401 for {url}") from e raise except httpx.RequestError as e: logger.exception(f"Request error occurred: {str(e)}") diff --git a/packages/gg_api_core/src/gg_api_core/mcp_server.py b/packages/gg_api_core/src/gg_api_core/mcp_server.py index 260d0b0b..0bb21d74 100644 --- a/packages/gg_api_core/src/gg_api_core/mcp_server.py +++ b/packages/gg_api_core/src/gg_api_core/mcp_server.py @@ -9,7 +9,7 @@ from fastmcp import FastMCP from fastmcp.exceptions import ValidationError -from fastmcp.server.dependencies import get_access_token, get_http_headers +from fastmcp.server.dependencies import get_access_token from fastmcp.server.middleware import Middleware from fastmcp.tools import Tool @@ -20,11 +20,9 @@ create_oauth_proxy, mark_downstream_unauthorized, ) -from gg_api_core.oauth_proxy_auth import create_oauth_proxy from gg_api_core.settings import get_settings from gg_api_core.utils import get_client - # Configure logger logger = logging.getLogger(__name__) @@ -352,53 +350,32 @@ def get_personal_access_token(self) -> str: return self.personal_access_token -class GitGuardianAuthorizationHeaderMCP(AbstractGitGuardianFastMCP): - """GitGuardian MCP server using per-request Authorization header (HTTP/SSE mode).""" +class _BearerTokenMCP(AbstractGitGuardianFastMCP): + """Base for modes where the bearer token is installed in the request scope. - authentication_mode = AuthenticationMode.AUTHORIZATION_HEADER + Requires an ``auth=`` provider that populates an ``AccessToken`` on the + request — :class:`PassThroughTokenVerifier` for raw header mode, or + :class:`GitGuardianOAuthThinProxy` for OAuth proxy mode. Both classes + below differ only in :attr:`authentication_mode`. + """ def get_personal_access_token(self) -> str: - headers = get_http_headers(include={"authorization"}) - if not headers: - raise ValidationError("No HTTP headers available - Authorization header required") - - auth_header = headers.get("authorization") - if not auth_header: - raise ValidationError("Missing Authorization header") - - token = self._default_extract_token(auth_header) - if not token: - raise ValidationError("Invalid Authorization header format") - - return token - - @staticmethod - def _default_extract_token(auth_header: str) -> str | None: - """Extract token from Authorization header. - - Supports formats: - - Bearer - - Token - - (raw) - """ - auth_header = auth_header.strip() - - if auth_header.lower().startswith("bearer "): - return auth_header[7:].strip() + access_token = get_access_token() + if not access_token: + raise ValidationError("No access token available - bearer authentication required") + return access_token.token - if auth_header.lower().startswith("token "): - return auth_header[6:].strip() + async def get_token_info(self) -> dict[str, Any]: + return await self._fetch_token_info_from_api() - if auth_header: - return auth_header - return None +class GitGuardianAuthorizationHeaderMCP(_BearerTokenMCP): + """GitGuardian MCP server using per-request Authorization header (HTTP/SSE mode).""" - async def get_token_info(self) -> dict[str, Any]: - return await self._fetch_token_info_from_api() + authentication_mode = AuthenticationMode.AUTHORIZATION_HEADER -class GitGuardianOAuthProxyMCP(AbstractGitGuardianFastMCP): +class GitGuardianOAuthProxyMCP(_BearerTokenMCP): """GitGuardian MCP server with thin OAuth proxy to the GG dashboard. Same-origin OAuth endpoints proxy auth requests to the GG dashboard. @@ -407,17 +384,6 @@ class GitGuardianOAuthProxyMCP(AbstractGitGuardianFastMCP): authentication_mode = AuthenticationMode.OAUTH_PROXY - def get_personal_access_token(self) -> str: - # TODO(TIM): Why different than GitGuardianAuthorizationHeaderMCP ? - access_token = get_access_token() - - if not access_token: - raise ValidationError("No access token available - OAuth proxy authentication required") - return access_token.token - - async def get_token_info(self) -> dict[str, Any]: - return await self._fetch_token_info_from_api() - def get_mcp_server(*args, **kwargs) -> AbstractGitGuardianFastMCP: kwargs.setdefault("icons", get_gitguardian_icons()) @@ -439,4 +405,4 @@ def get_mcp_server(*args, **kwargs) -> AbstractGitGuardianFastMCP: if personal_access_token := settings.gitguardian_personal_access_token: return GitGuardianPATEnvMCP(*args, personal_access_token=personal_access_token, **kwargs) - return GitGuardianAuthorizationHeaderMCP(*args, **kwargs) + return GitGuardianAuthorizationHeaderMCP(*args, auth=PassThroughTokenVerifier(), **kwargs) diff --git a/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py b/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py index ef80443f..0686488d 100644 --- a/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py +++ b/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py @@ -29,6 +29,63 @@ logger = logging.getLogger(__name__) +_downstream_unauthorized: ContextVar[bool] = ContextVar("downstream_unauthorized", default=False) + + +def mark_downstream_unauthorized() -> None: + """Signal that the downstream GG API rejected the current request as unauthorized. + + Call this from anywhere inside the request task (typically a FastMCP + middleware catching :class:`DownstreamUnauthorizedError`). The outgoing + response will then carry HTTP 401 instead of the JSON-RPC error envelope + FastMCP would otherwise serialize, letting the MCP client re-run the + OAuth flow. + """ + _downstream_unauthorized.set(True) + + +class PassThroughTokenVerifier(TokenVerifier): + """Token verifier that trusts the bearer token without contacting the IdP. + + Used by MCP modes where the GG API is the source of truth: the verifier + accepts the token so it lands in the request scope, and downstream calls + fail with a real 401 if the token is invalid. + """ + + async def verify_token(self, token: str) -> AccessToken | None: + return AccessToken(token=token, client_id="unknown", scopes=[]) + + +class TranslateDownstreamUnauthorizedMiddleware: + """Convert flagged tool responses into HTTP 401. + + Reads the request-scoped flag set by :func:`mark_downstream_unauthorized` + and rewrites the outgoing status to 401 when set. Pairs with + :class:`AdvertiseAuthorizationServerMetadataMiddleware` which then + ensures ``WWW-Authenticate`` is present. + """ + + def __init__(self, app: ASGIApp): + self.app = app + + async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: + if scope["type"] != "http": + await self.app(scope, receive, send) + return + + token = _downstream_unauthorized.set(False) + + async def send_wrapper(message: Message) -> None: + if message["type"] == "http.response.start" and _downstream_unauthorized.get(): + message["status"] = 401 + await send(message) + + try: + await self.app(scope, receive, send_wrapper) + finally: + _downstream_unauthorized.reset(token) + + class AdvertiseAuthorizationServerMetadataMiddleware: """Append the AS metadata URL to ``WWW-Authenticate`` on 401 responses. @@ -67,7 +124,7 @@ async def send_wrapper(message: Message) -> None: await self.app(scope, receive, send_wrapper) -class GitGuardianOAuthThinProxy(TokenVerifier): +class GitGuardianOAuthThinProxy(PassThroughTokenVerifier): """Thin OAuth proxy that routes auth requests to the GG dashboard. Serves same-origin OAuth endpoints so MCP clients that require @@ -97,32 +154,6 @@ def __init__( def scopes_supported(self) -> list[str]: return self._advertised_scopes or ["scan"] - # TODO(TIM): Should we implement this method or let actual calls fail with 401 ? - async def verify_token(self, token: str) -> AccessToken | None: - """Validate a GG PAT by calling /api_tokens/self.""" - try: - async with httpx.AsyncClient(timeout=10) as client: - response = await client.get( - f"{self.gg_api_url}/api_tokens/self", - headers={"Authorization": f"Token {token}"}, - ) - - if response.status_code != 200: - logger.warning(f"Token verification failed: HTTP {response.status_code}") - return None - - token_info = response.json() - scopes = token_info.get("scopes", []) - - return AccessToken( - token=token, - client_id=token_info.get("id", "unknown"), - scopes=scopes, - ) - except Exception: - logger.exception("Error verifying upstream token") - return None - def get_middleware(self) -> list: """Add downstream-401 handling and WWW-Authenticate advertising. diff --git a/packages/gg_api_core/src/gg_api_core/settings.py b/packages/gg_api_core/src/gg_api_core/settings.py index db7f13a9..28781add 100644 --- a/packages/gg_api_core/src/gg_api_core/settings.py +++ b/packages/gg_api_core/src/gg_api_core/settings.py @@ -19,7 +19,6 @@ from .scopes import ServerProfile - TRUTHY_ENV_VALUES = frozenset( { "true", @@ -147,9 +146,7 @@ def effective_scopes(self) -> list[str]: if self.server_profile is None: return self.requested_scopes - restricted = is_self_hosted_instance( - self.gitguardian_url - ) and not is_local_instance(self.gitguardian_url) + restricted = is_self_hosted_instance(self.gitguardian_url) and not is_local_instance(self.gitguardian_url) allowed = set(self.server_profile.max_scopes(restricted=restricted)) requested = set(self.requested_scopes) return sorted(allowed & requested if requested else allowed) diff --git a/tests/test_mcp_server.py b/tests/test_mcp_server.py index bfa64871..81a23485 100644 --- a/tests/test_mcp_server.py +++ b/tests/test_mcp_server.py @@ -1,3 +1,4 @@ +from types import SimpleNamespace from unittest.mock import AsyncMock, MagicMock, patch import pytest @@ -192,81 +193,27 @@ async def tool_with_teams_write(): # The teams:write tool should be excluded since the required scope is missing assert "tool_with_teams_write" not in tool_names - def test_extract_token_from_header(self): - """Test extracting tokens from various Authorization header formats.""" + @patch("gg_api_core.mcp_server.get_access_token") + def test_get_personal_access_token_returns_scope_token(self, mock_get_access_token): + """get_personal_access_token() returns the bearer token installed in the request scope.""" from gg_api_core.mcp_server import GitGuardianAuthorizationHeaderMCP - # Test Bearer format - token = GitGuardianAuthorizationHeaderMCP._default_extract_token("Bearer test-token-123") - assert token == "test-token-123" + access_token = SimpleNamespace(token="test-pat-token-123") + mock_get_access_token.return_value = access_token - # Test Token format - token = GitGuardianAuthorizationHeaderMCP._default_extract_token("Token another-token-456") - assert token == "another-token-456" - - # Test raw token (no prefix) - token = GitGuardianAuthorizationHeaderMCP._default_extract_token("raw-token-789") - assert token == "raw-token-789" - - # Test case insensitivity - token = GitGuardianAuthorizationHeaderMCP._default_extract_token("bearer lowercase-token") - assert token == "lowercase-token" - - # Test with extra whitespace - token = GitGuardianAuthorizationHeaderMCP._default_extract_token("Bearer token-with-spaces ") - assert token == "token-with-spaces" - - # Test empty string - token = GitGuardianAuthorizationHeaderMCP._default_extract_token("") - assert token is None - - @patch("gg_api_core.mcp_server.get_http_headers") - @patch("gg_api_core.mcp_server.get_client") - def test_get_client_with_authorization_header(self, mock_get_client, mock_get_http_headers): - """Test that get_personal_access_token extracts token from Authorization header.""" - from gg_api_core.mcp_server import GitGuardianAuthorizationHeaderMCP - - # Mock HTTP headers with Authorization header - mock_get_http_headers.return_value = {"authorization": "Bearer test-pat-token-123"} - - # Create MCP with Authorization header mode mcp = GitGuardianAuthorizationHeaderMCP("test_server") - # Call get_personal_access_token - token = mcp.get_personal_access_token() + assert mcp.get_personal_access_token() == "test-pat-token-123" - # Verify token was extracted correctly - assert token == "test-pat-token-123" - - @patch("gg_api_core.mcp_server.get_http_headers") - @patch("gg_api_core.mcp_server.get_client") - def test_get_client_without_authorization_header(self, mock_get_client, mock_get_http_headers): - """Test that get_personal_access_token raises ValidationError when no Authorization header.""" + @patch("gg_api_core.mcp_server.get_access_token") + def test_get_personal_access_token_raises_without_scope_token(self, mock_get_access_token): + """get_personal_access_token() raises ValidationError when no AccessToken is in the request scope.""" from fastmcp.exceptions import ValidationError from gg_api_core.mcp_server import GitGuardianAuthorizationHeaderMCP - # Mock HTTP headers without Authorization header - mock_get_http_headers.return_value = {} - - # Create MCP with Authorization header mode - mcp = GitGuardianAuthorizationHeaderMCP("test_server") - - # Call get_personal_access_token - should raise ValidationError - with pytest.raises(ValidationError, match="Authorization header required"): - mcp.get_personal_access_token() - - @patch("gg_api_core.mcp_server.get_http_headers") - @patch("gg_api_core.mcp_server.get_client") - def test_get_client_no_http_context(self, mock_get_client, mock_get_http_headers): - """Test that get_personal_access_token propagates RuntimeError when no HTTP context.""" - from gg_api_core.mcp_server import GitGuardianAuthorizationHeaderMCP - - # Mock get_http_headers to raise exception (no HTTP context) - mock_get_http_headers.side_effect = RuntimeError("No HTTP context") + mock_get_access_token.return_value = None - # Create MCP with Authorization header mode mcp = GitGuardianAuthorizationHeaderMCP("test_server") - # Call get_personal_access_token - should raise RuntimeError - with pytest.raises(RuntimeError, match="No HTTP context"): + with pytest.raises(ValidationError, match="No access token available"): mcp.get_personal_access_token() From 67493f5ea059bf5088bb3c3d5d8c141194e849f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Timoth=C3=A9=20Delion?= Date: Wed, 13 May 2026 08:35:26 +0200 Subject: [PATCH 3/3] feat(oauth2): emit resource_metadata in synthesized WWW-Authenticate MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When `TranslateDownstreamUnauthorizedMiddleware` flips a 200 into a 401, the response has no `WWW-Authenticate` header for the advertise middleware to augment, so it synthesizes one. Previously it emitted only `as_metadata` — the Claude.ai-specific shortcut — leaving spec-compliant MCP clients (Cursor, MCP Inspector) without the standard `resource_metadata` discovery pointer required by RFC 9728. `resource_metadata_url_provider` is resolved lazily because FastMCP calls `get_middleware()` before `set_mcp_path()`, so the resource URL is not known at middleware construction time. Issue: APPAI-524 --- .../src/gg_api_core/oauth_proxy_auth.py | 83 ++++++++++++++----- tests/test_oauth_proxy_middleware.py | 67 ++++++++++++++- 2 files changed, 125 insertions(+), 25 deletions(-) diff --git a/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py b/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py index 0686488d..1bd46e3c 100644 --- a/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py +++ b/packages/gg_api_core/src/gg_api_core/oauth_proxy_auth.py @@ -15,6 +15,7 @@ import logging import os +from collections.abc import Callable from contextvars import ContextVar from urllib.parse import urlencode @@ -87,18 +88,36 @@ async def send_wrapper(message: Message) -> None: class AdvertiseAuthorizationServerMetadataMiddleware: - """Append the AS metadata URL to ``WWW-Authenticate`` on 401 responses. - - Claude.ai does not follow ``authorization_servers`` from RFC 9728 protected - resource metadata (https://github.com/anthropics/claude-ai-mcp/issues/82), - so we expose the AS metadata location directly via an ``as_metadata`` - parameter alongside the existing ``resource_metadata`` one. + """Ensure 401 responses advertise both ``resource_metadata`` and ``as_metadata``. + + * **Augment branch.** When FastMCP's auth backend already emitted a 401 + with ``WWW-Authenticate: Bearer resource_metadata="..."``, append + ``, as_metadata="..."`` so Claude.ai (which does not follow RFC 9728 + ``authorization_servers``, see + https://github.com/anthropics/claude-ai-mcp/issues/82) can locate the + AS metadata directly. + * **Synthesize branch.** When a 401 carries no ``WWW-Authenticate`` at + all (the case after :class:`TranslateDownstreamUnauthorizedMiddleware` + flips a 200 into a 401), build one with **both** ``resource_metadata`` + (so spec-compliant clients can discover via the standard chain) and + ``as_metadata`` (so Claude.ai can shortcut). + + ``resource_metadata_url`` is resolved lazily on each request because + FastMCP constructs the middleware before calling ``set_mcp_path()`` on + the auth provider — the resource URL isn't known at middleware + construction time. """ - def __init__(self, app: ASGIApp, as_metadata_url: str): + def __init__( + self, + app: ASGIApp, + as_metadata_url: str, + resource_metadata_url_provider: Callable[[], str | None] | None = None, + ): self.app = app - self._param = f'as_metadata="{as_metadata_url}"' - self._param_bytes = self._param.encode() + self._as_param = f'as_metadata="{as_metadata_url}"' + self._append_suffix = f", {self._as_param}".encode() + self._resource_metadata_url_provider = resource_metadata_url_provider async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: if scope["type"] != "http": @@ -108,21 +127,26 @@ async def __call__(self, scope: Scope, receive: Receive, send: Send) -> None: async def send_wrapper(message: Message) -> None: if message["type"] == "http.response.start" and message.get("status") == 401: headers = list(message.get("headers", [])) - rewritten = [] - found = False - for name, value in headers: + for i, (name, value) in enumerate(headers): if name.lower() == b"www-authenticate": - found = True - rewritten.append((name, value + b", " + self._param_bytes)) - else: - rewritten.append((name, value)) - if not found: - rewritten.append((b"www-authenticate", f"Bearer {self._param}".encode())) - message["headers"] = rewritten + headers[i] = (name, value + self._append_suffix) + break + else: + headers.append((b"www-authenticate", self._build_synth_header())) + message["headers"] = headers await send(message) await self.app(scope, receive, send_wrapper) + def _build_synth_header(self) -> bytes: + parts: list[str] = [] + if self._resource_metadata_url_provider is not None: + rm_url = self._resource_metadata_url_provider() + if rm_url: + parts.append(f'resource_metadata="{rm_url}"') + parts.append(self._as_param) + return f"Bearer {', '.join(parts)}".encode() + class GitGuardianOAuthThinProxy(PassThroughTokenVerifier): """Thin OAuth proxy that routes auth requests to the GG dashboard. @@ -159,9 +183,9 @@ def get_middleware(self) -> list: Order matters. ``TranslateDownstreamUnauthorizedMiddleware`` runs first so a flagged response becomes a 401; then - ``AdvertiseAuthorizationServerMetadataMiddleware`` appends the - ``as_metadata`` parameter to the resulting ``WWW-Authenticate`` - header. + ``AdvertiseAuthorizationServerMetadataMiddleware`` enriches the + resulting ``WWW-Authenticate`` header with ``resource_metadata`` + and ``as_metadata`` parameters. """ middleware = super().get_middleware() middleware.append(Middleware(TranslateDownstreamUnauthorizedMiddleware)) @@ -171,10 +195,25 @@ def get_middleware(self) -> list: Middleware( AdvertiseAuthorizationServerMetadataMiddleware, as_metadata_url=f"{base}/.well-known/oauth-authorization-server", + resource_metadata_url_provider=self._build_resource_metadata_url, ) ) return middleware + def _build_resource_metadata_url(self) -> str | None: + """Resolve the RFC 9728 protected-resource metadata URL. + + Called lazily per request because ``set_mcp_path()`` runs after + ``get_middleware()`` during FastMCP's app construction — at + middleware-construction time ``self._resource_url`` is still ``None``. + """ + from mcp.server.auth.routes import build_resource_metadata_url + + resource_url = getattr(self, "_resource_url", None) + if not resource_url: + return None + return str(build_resource_metadata_url(resource_url)) + def get_routes(self, mcp_path: str | None = None) -> list[Route]: """Return OAuth proxy routes alongside discovery metadata.""" self.set_mcp_path(mcp_path) diff --git a/tests/test_oauth_proxy_middleware.py b/tests/test_oauth_proxy_middleware.py index e0edc61a..9118499a 100644 --- a/tests/test_oauth_proxy_middleware.py +++ b/tests/test_oauth_proxy_middleware.py @@ -49,7 +49,9 @@ async def inner_app(scope, receive, send): @pytest.mark.asyncio -async def test_advertise_adds_as_metadata_param_to_401(): +async def test_advertise_synthesizes_header_with_both_params(): + """Bridged 401 (no existing WWW-Authenticate) gets resource_metadata + as_metadata.""" + async def inner_app(scope, receive, send): mark_downstream_unauthorized() await send({"type": "http.response.start", "status": 200, "headers": []}) @@ -59,9 +61,68 @@ async def inner_app(scope, receive, send): stacked = AdvertiseAuthorizationServerMetadataMiddleware( TranslateDownstreamUnauthorizedMiddleware(inner_app), as_metadata_url="https://mcp.example.com/.well-known/oauth-authorization-server", + resource_metadata_url_provider=lambda: "https://mcp.example.com/.well-known/oauth-protected-resource/mcp", + ) + status, headers = await _drive(stacked) + assert status == 401 + header_map = {name.decode(): value.decode() for name, value in headers} + www_auth = header_map["www-authenticate"] + assert www_auth.startswith("Bearer ") + assert 'resource_metadata="https://mcp.example.com/.well-known/oauth-protected-resource/mcp"' in www_auth + assert 'as_metadata="https://mcp.example.com/.well-known/oauth-authorization-server"' in www_auth + + +@pytest.mark.asyncio +async def test_advertise_synthesizes_header_without_resource_metadata_when_unavailable(): + """If the resource URL isn't set yet, fall back to as_metadata-only.""" + + async def inner_app(scope, receive, send): + mark_downstream_unauthorized() + await send({"type": "http.response.start", "status": 200, "headers": []}) + await send({"type": "http.response.body", "body": b""}) + + stacked = AdvertiseAuthorizationServerMetadataMiddleware( + TranslateDownstreamUnauthorizedMiddleware(inner_app), + as_metadata_url="https://mcp.example.com/.well-known/oauth-authorization-server", + resource_metadata_url_provider=lambda: None, + ) + status, headers = await _drive(stacked) + assert status == 401 + header_map = {name.decode(): value.decode() for name, value in headers} + www_auth = header_map["www-authenticate"] + assert "resource_metadata=" not in www_auth + assert "as_metadata=" in www_auth + + +@pytest.mark.asyncio +async def test_advertise_appends_as_metadata_to_existing_header(): + """When FastMCP's auth already emitted WWW-Authenticate, just append as_metadata.""" + + async def inner_app(scope, receive, send): + await send( + { + "type": "http.response.start", + "status": 401, + "headers": [ + ( + b"www-authenticate", + b'Bearer resource_metadata="https://mcp.example.com/.well-known/oauth-protected-resource/mcp"', + ) + ], + } + ) + await send({"type": "http.response.body", "body": b""}) + + stacked = AdvertiseAuthorizationServerMetadataMiddleware( + inner_app, + as_metadata_url="https://mcp.example.com/.well-known/oauth-authorization-server", + resource_metadata_url_provider=lambda: "https://should-not-be-used.example.com", ) status, headers = await _drive(stacked) assert status == 401 header_map = {name.decode(): value.decode() for name, value in headers} - assert "www-authenticate" in header_map - assert "as_metadata=" in header_map["www-authenticate"] + www_auth = header_map["www-authenticate"] + # The original resource_metadata is preserved; only as_metadata is appended. + assert www_auth.count("resource_metadata=") == 1 + assert "should-not-be-used" not in www_auth + assert ", as_metadata=" in www_auth