diff --git a/deployments/charts/service/ci/validate-mcp-chart.sh b/deployments/charts/service/ci/validate-mcp-chart.sh index 6c71caafc..7e3531593 100644 --- a/deployments/charts/service/ci/validate-mcp-chart.sh +++ b/deployments/charts/service/ci/validate-mcp-chart.sh @@ -180,45 +180,43 @@ for expected in \ assert_file_contains "$PROXY_MCP_MANIFEST" "$expected" done +# FastMCP's OAuth endpoints are published under /mcp, so the gateway needs one +# prefix route rather than an entry per endpoint name. for route in \ mcp-protected-resource-metadata \ - mcp-oauth-authorization-server-metadata \ - mcp-oauth-authorize-get \ - mcp-oauth-authorize-post \ - mcp-oauth-oidc-callback \ - mcp-oauth-register \ - mcp-oauth-token \ - mcp-oauth-consent-get \ - mcp-oauth-consent-post \ - mcp-oauth-authorization-server-metadata-options \ - mcp-oauth-authorize-options \ - mcp-oauth-oidc-callback-options \ - mcp-oauth-register-options \ - mcp-oauth-token-options \ - mcp-oauth-consent-options; do + mcp-oauth \ + mcp-authorization-server-metadata; do assert_route_contains "$PROXY_RENDERED_MANIFEST" "$route" 'cluster: osmo-mcp' assert_route_contains "$PROXY_RENDERED_MANIFEST" "$route" 'envoy.filters.http.jwt_authn:' assert_route_contains "$PROXY_RENDERED_MANIFEST" "$route" 'envoy.filters.http.ext_authz:' done +# Scope the rewrite assertions to their routes: a file-wide grep for +# 'prefix_rewrite: /' also matches the well-known route's longer value. +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-oauth 'prefix: /mcp/' +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-oauth 'prefix_rewrite: /' +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-authorization-server-metadata \ + 'path: /.well-known/oauth-authorization-server/mcp' +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-authorization-server-metadata \ + 'prefix_rewrite: /.well-known/oauth-authorization-server' + +# The health carve-out must answer 404 itself. Without the auth filters +# disabled, jwt_authn answers 401 first and the carve-out is dead code that +# only looks like it works. +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-health-not-public 'status: 404' +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-health-not-public \ + 'envoy.filters.http.jwt_authn:' +assert_route_contains "$PROXY_RENDERED_MANIFEST" mcp-health-not-public \ + 'envoy.filters.http.ext_authz:' assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_ENABLED true assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_ISSUER_URL https://osmo.example.com assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_RESOURCE_URL https://osmo.example.com/mcp assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_SCOPE https://osmo.example.com/mcp/access_as_user assert_env_value "$PROXY_MCP_MANIFEST" OSMO_MCP_AUTH_REDIS_URL rediss://proxy-redis.example.internal:6380/14 -for expected in \ - 'path: /.well-known/oauth-authorization-server' \ - 'path: /authorize' \ - 'path: /auth/callback' \ - 'path: /register' \ - 'path: /token' \ - 'path: /consent' \ - 'name: mcp-protected-resource-metadata' \ - 'name: osmo-mcp' \ - 'cluster: osmo-mcp' \ - 'path: "%PATH(NQ:ORIG_OR_PATH)%"'; do - assert_file_contains "$PROXY_RENDERED_MANIFEST" "$expected" -done +# FastMCP advertises its endpoints under /mcp; the gateway publishes that +# prefix and rewrites it off before forwarding to the root paths the MCP SDK +# actually registers. +assert_file_contains "$PROXY_RENDERED_MANIFEST" 'path: "%PATH(NQ:ORIG_OR_PATH)%"' for forbidden in \ 'name: osmo-mcp-oauth' \ diff --git a/deployments/charts/service/templates/_gateway-envoy-config.tpl b/deployments/charts/service/templates/_gateway-envoy-config.tpl index 080c11707..6cde4f644 100644 --- a/deployments/charts/service/templates/_gateway-envoy-config.tpl +++ b/deployments/charts/service/templates/_gateway-envoy-config.tpl @@ -37,24 +37,6 @@ setting detects this rotation and triggers Envoy to reload. {{- $mcpOidcProxyEnabled := and $mcpEnabled ($mcpOidcProxy.enabled | default false) }} {{- $mcpPath := "/mcp" }} {{- $mcpMetadataPath := "/.well-known/oauth-protected-resource/mcp" }} -{{- $mcpOidcProxyRoutes := list - (dict "name" "mcp-oauth-authorization-server-metadata" "path" "/.well-known/oauth-authorization-server" "pathRegex" "^/[.]well-known/oauth-authorization-server([?].*)?$" "method" "GET") - (dict "name" "mcp-oauth-authorize-get" "path" "/authorize" "pathRegex" "^/authorize([?].*)?$" "method" "GET") - (dict "name" "mcp-oauth-authorize-post" "path" "/authorize" "pathRegex" "^/authorize([?].*)?$" "method" "POST") - (dict "name" "mcp-oauth-oidc-callback" "path" "/auth/callback" "pathRegex" "^/auth/callback([?].*)?$" "method" "GET" "timeout" "45s") - (dict "name" "mcp-oauth-register" "path" "/register" "pathRegex" "^/register([?].*)?$" "method" "POST") - (dict "name" "mcp-oauth-token" "path" "/token" "pathRegex" "^/token([?].*)?$" "method" "POST" "timeout" "45s") - (dict "name" "mcp-oauth-consent-get" "path" "/consent" "pathRegex" "^/consent([?].*)?$" "method" "GET") - (dict "name" "mcp-oauth-consent-post" "path" "/consent" "pathRegex" "^/consent([?].*)?$" "method" "POST") - }} -{{- $mcpOidcProxyOptionRoutes := list - (dict "name" "mcp-oauth-authorization-server-metadata-options" "path" "/.well-known/oauth-authorization-server" "pathRegex" "^/[.]well-known/oauth-authorization-server([?].*)?$") - (dict "name" "mcp-oauth-authorize-options" "path" "/authorize" "pathRegex" "^/authorize([?].*)?$") - (dict "name" "mcp-oauth-oidc-callback-options" "path" "/auth/callback" "pathRegex" "^/auth/callback([?].*)?$" "timeout" "45s") - (dict "name" "mcp-oauth-register-options" "path" "/register" "pathRegex" "^/register([?].*)?$") - (dict "name" "mcp-oauth-token-options" "path" "/token" "pathRegex" "^/token([?].*)?$") - (dict "name" "mcp-oauth-consent-options" "path" "/consent" "pathRegex" "^/consent([?].*)?$") - }} {{- $mcpResourceUrl := "" }} {{- $mcpMetadataUrl := "" }} {{- $mcpAuthorizationServers := $mcp.authorizationServers }} @@ -102,26 +84,12 @@ setting detects this rotation and triggers Envoy to reload. {{- if or (lt (int $mcp.port) 1) (gt (int $mcp.port) 65535) }} {{- fail "services.mcp.port must be between 1 and 65535" }} {{- end }} -{{- if not (kindIs "slice" $mcp.allowedOrigins) }} -{{- fail "services.mcp.allowedOrigins must be a list" }} -{{- end }} -{{- range $origin := $mcp.allowedOrigins }} -{{- if not (regexMatch "^https?://[^/?#]+$" $origin) }} -{{- fail (printf "services.mcp.allowedOrigins entry %q must be an exact HTTP(S) Origin without a path" $origin) }} -{{- end }} -{{- end }} {{- range $skipPath := $skipAuthPaths }} {{- $overlapsMcpPath := or (hasPrefix $skipPath $mcpPath) (hasPrefix $mcpPath $skipPath) }} {{- $overlapsMcpMetadataPath := or (hasPrefix $skipPath $mcpMetadataPath) (hasPrefix $mcpMetadataPath $skipPath) }} -{{- $overlapsMcpOidcProxyPath := false }} -{{- if $mcpOidcProxyEnabled }} -{{- range $route := $mcpOidcProxyRoutes }} -{{- if or (hasPrefix $skipPath $route.path) (hasPrefix $route.path $skipPath) }} -{{- $overlapsMcpOidcProxyPath = true }} -{{- end }} -{{- end }} -{{- end }} -{{- if or $overlapsMcpPath $overlapsMcpMetadataPath $overlapsMcpOidcProxyPath }} +{{- $mcpAuthServerMetadataPath := "/.well-known/oauth-authorization-server/mcp" }} +{{- $overlapsMcpAuthServerPath := and $mcpOidcProxyEnabled (or (hasPrefix $skipPath $mcpAuthServerMetadataPath) (hasPrefix $mcpAuthServerMetadataPath $skipPath)) }} +{{- if or $overlapsMcpPath $overlapsMcpMetadataPath $overlapsMcpAuthServerPath }} {{- fail (printf "gateway auth bypass prefix %q overlaps a protected MCP path" $skipPath) }} {{- end }} {{- end }} @@ -353,54 +321,55 @@ data: value: "*" append_action: OVERWRITE_IF_EXISTS_OR_ADD typed_per_filter_config: - envoy.filters.http.jwt_authn: - "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.PerRouteConfig - disabled: true - envoy.filters.http.ext_authz: - "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute - disabled: true + {{- include "osmo.gateway-auth-filters-disabled" . | nindent 20 }} {{- if $mcpOidcProxyEnabled }} - # FastMCP owns this complete public OAuth surface in the same - # process as /mcp. Every route remains method/path exact. - {{- range $route := $mcpOidcProxyRoutes }} - - name: {{ $route.name }} + # FastMCP owns the whole OAuth surface for this deployment. The + # MCP SDK registers its handlers at fixed root paths, so the + # gateway publishes them under /mcp -- matching the endpoint + # URLs FastMCP advertises in its metadata -- and rewrites the + # prefix back off before forwarding. One prefix route keeps the + # gateway from having to enumerate FastMCP's endpoint names. + # The prefix route below publishes the container's whole root + # namespace, so anything registered at the app root becomes + # public. Keep the health endpoints off the internet; any new + # non-OAuth root route must be carved out here too. The auth + # filters are disabled so this route answers 404 itself rather + # than letting jwt_authn answer 401 first -- the carve-out has + # to be the thing that rejects, not a side effect of another + # filter that a later change could switch off. + - name: mcp-health-not-public match: - path: {{ $route.path }} - headers: - - name: ":method" - string_match: - exact: {{ $route.method }} + prefix: /mcp/health + direct_response: + status: 404 + typed_per_filter_config: + {{- include "osmo.gateway-auth-filters-disabled" . | nindent 20 }} + - name: mcp-oauth + match: + prefix: /mcp/ route: cluster: osmo-mcp - timeout: {{ default "15s" $route.timeout }} + prefix_rewrite: / + timeout: 45s typed_per_filter_config: - envoy.filters.http.jwt_authn: - "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.PerRouteConfig - disabled: true - envoy.filters.http.ext_authz: - "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute - disabled: true - {{- end }} - {{- range $route := $mcpOidcProxyOptionRoutes }} - - name: {{ $route.name }} + {{- include "osmo.gateway-auth-filters-disabled" . | nindent 20 }} + # RFC 8414 locates a path-scoped issuer's metadata under the + # well-known prefix; FastMCP serves the document at the root + # path, so rewrite onto it. + - name: mcp-authorization-server-metadata match: - path: {{ $route.path }} + path: /.well-known/oauth-authorization-server/mcp headers: - name: ":method" string_match: - exact: OPTIONS + exact: GET route: cluster: osmo-mcp - timeout: {{ default "15s" $route.timeout }} + prefix_rewrite: /.well-known/oauth-authorization-server + timeout: 15s typed_per_filter_config: - envoy.filters.http.jwt_authn: - "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.PerRouteConfig - disabled: true - envoy.filters.http.ext_authz: - "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute - disabled: true - {{- end }} + {{- include "osmo.gateway-auth-filters-disabled" . | nindent 20 }} {{- end }} # In direct mode Gateway validates and authorizes /mcp. With @@ -414,12 +383,7 @@ data: timeout: 0s {{- if $mcpOidcProxyEnabled }} typed_per_filter_config: - envoy.filters.http.jwt_authn: - "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.PerRouteConfig - disabled: true - envoy.filters.http.ext_authz: - "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute - disabled: true + {{- include "osmo.gateway-auth-filters-disabled" . | nindent 20 }} {{- end }} {{- end }} @@ -582,38 +546,6 @@ data: return end end - {{- if $mcpEnabled }} - - name: envoy.filters.http.lua.mcp-origin - typed_config: - "@type": type.googleapis.com/envoy.extensions.filters.http.lua.v3.Lua - default_source_code: - inline_string: | - function envoy_on_request(request_handle) - local raw_path = request_handle:headers():get(":path") or "" - local request_path = string.match(raw_path, "^[^?]*") or raw_path - if request_path ~= {{ $mcpPath | quote }} then - return - end - - -- Native MCP clients omit Origin. A present Origin must - -- exactly match the deployment's explicit allowlist. - local origin = request_handle:headers():get("origin") - if origin == nil then - return - end - - local allowed_origins = {} - {{ range $origin := $mcp.allowedOrigins }} - allowed_origins[{{ $origin | quote }}] = true - {{ end }} - if not allowed_origins[origin] then - request_handle:respond( - {[":status"] = "403", ["content-type"] = "text/plain"}, - "Origin is not allowed" - ) - end - end - {{- end }} {{- if $authnSkipPaths }} {{- /* Authn skip paths bypass both authn and authz. */}} # set_metadata has no path matcher of its own, so wrap it and @@ -746,52 +678,17 @@ data: value_match: exact: "GET" {{- if $mcpOidcProxyEnabled }} - # Skip the browser-session proxy only for the exact - # method/path pairs owned by FastMCP's OIDC proxy. - {{- range $route := $mcpOidcProxyRoutes }} - - and_matcher: - predicate: - - single_predicate: - input: - name: request-headers - typed_config: - "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput - header_name: ":path" - value_match: - safe_regex: - google_re2: {} - regex: {{ $route.pathRegex | quote }} - - single_predicate: - input: - name: request-headers - typed_config: - "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput - header_name: ":method" - value_match: - exact: {{ $route.method | quote }} - {{- end }} - {{- range $route := $mcpOidcProxyOptionRoutes }} - - and_matcher: - predicate: - - single_predicate: - input: - name: request-headers - typed_config: - "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput - header_name: ":path" - value_match: - safe_regex: - google_re2: {} - regex: {{ $route.pathRegex | quote }} - - single_predicate: - input: - name: request-headers - typed_config: - "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput - header_name: ":method" - value_match: - exact: "OPTIONS" - {{- end }} + # FastMCP is authoritative for its own OAuth surface. + - single_predicate: + input: + name: request-headers + typed_config: + "@type": type.googleapis.com/envoy.type.matcher.v3.HttpRequestHeaderMatchInput + header_name: ":path" + value_match: + safe_regex: + google_re2: {} + regex: "^(/mcp/.*|/[.]well-known/oauth-authorization-server/mcp([?].*)?)$" {{- end }} {{- end }} {{- if $authnSkipPaths }} diff --git a/deployments/charts/service/templates/_gateway-helpers.tpl b/deployments/charts/service/templates/_gateway-helpers.tpl index 49b255498..9e3ecaf16 100644 --- a/deployments/charts/service/templates/_gateway-helpers.tpl +++ b/deployments/charts/service/templates/_gateway-helpers.tpl @@ -93,3 +93,16 @@ Use: {{- end }} {{- toYaml $probe }} {{- end }} + +{{/* +Per-route config that turns off gateway authentication and authorization. +Used by the MCP routes, where FastMCP is authoritative for the request. +*/}} +{{- define "osmo.gateway-auth-filters-disabled" -}} +envoy.filters.http.jwt_authn: + "@type": type.googleapis.com/envoy.extensions.filters.http.jwt_authn.v3.PerRouteConfig + disabled: true +envoy.filters.http.ext_authz: + "@type": type.googleapis.com/envoy.extensions.filters.http.ext_authz.v3.ExtAuthzPerRoute + disabled: true +{{- end -}} diff --git a/deployments/charts/service/templates/mcp-service.yaml b/deployments/charts/service/templates/mcp-service.yaml index b405e89bd..beb26978a 100644 --- a/deployments/charts/service/templates/mcp-service.yaml +++ b/deployments/charts/service/templates/mcp-service.yaml @@ -149,11 +149,20 @@ {{- $redisScheme := ternary "rediss" "redis" $oidcProxy.redis.tlsEnabled }} {{- $redisUrl = printf "%s://%s:%v/%v" $redisScheme $redisHost $redisPort $oidcProxy.redis.dbNumber }} {{- end }} +{{- if not (kindIs "slice" $mcp.allowedOrigins) }} +{{- fail "services.mcp.allowedOrigins must be a list" }} +{{- end }} +{{- range $origin := $mcp.allowedOrigins }} +{{- if not (regexMatch "^https?://[^/?#]+$" $origin) }} +{{- fail (printf "services.mcp.allowedOrigins entry %q must be an exact HTTP(S) Origin without a path" $origin) }} +{{- end }} +{{- end }} {{- $reservedEnvNames := list "OSMO_MCP_HOST" "OSMO_MCP_PORT" "OSMO_GATEWAY_URL" "OSMO_MCP_REQUEST_TIMEOUT_SECONDS" + "OSMO_MCP_ALLOWED_ORIGINS" "OSMO_MCP_AUTH_ENABLED" "OSMO_MCP_AUTH_ISSUER_URL" "OSMO_MCP_AUTH_RESOURCE_URL" @@ -244,6 +253,8 @@ spec: value: {{ $gatewayUrl | quote }} - name: OSMO_MCP_REQUEST_TIMEOUT_SECONDS value: {{ $requestTimeoutSeconds | quote }} + - name: OSMO_MCP_ALLOWED_ORIGINS + value: {{ join "," $mcp.allowedOrigins | quote }} - name: OSMO_MCP_AUTH_ENABLED value: {{ $oidcProxyEnabled | quote }} {{- if $oidcProxyEnabled }} diff --git a/docs/deployment_guide/advanced_config/mcp.rst b/docs/deployment_guide/advanced_config/mcp.rst index 09d86ba12..b760142fd 100644 --- a/docs/deployment_guide/advanced_config/mcp.rst +++ b/docs/deployment_guide/advanced_config/mcp.rst @@ -86,7 +86,7 @@ Register the Upstream Application Configure one confidential application in the identity provider with: -* The exact redirect URL ``https:///auth/callback``. +* The exact redirect URL ``https:///mcp/auth/callback``. * Authorization code flow and the ``client_secret_post`` token authentication method. * A delegated API scope whose full URI is @@ -110,7 +110,7 @@ compatibility before using another provider. .. important:: - Do not confuse the fixed upstream ``/auth/callback`` URL with a native MCP + Do not confuse the fixed upstream ``/mcp/auth/callback`` URL with a native MCP client's temporary localhost callback. FastMCP accepts native loopback callbacks automatically. Browser-hosted clients require separately configured HTTPS redirect origins. @@ -209,7 +209,7 @@ FastMCP serves OAuth and MCP from the same process: #. The client uses CIMD or falls back to DCR through ``POST /register``. #. FastMCP obtains user consent, runs authorization code flow with Proof Key for Code Exchange (PKCE), and sends the user to the upstream OIDC provider. -#. The provider returns to the fixed ``/auth/callback`` URL. +#. The provider returns to the fixed ``/mcp/auth/callback`` URL. #. FastMCP exchanges the upstream authorization code for tokens, stores the resulting token state encrypted in Redis, and redirects the browser to the MCP client with a FastMCP authorization code. @@ -302,7 +302,7 @@ In OIDC proxy mode, also verify authorization-server metadata: .. code-block:: bash $ curl --fail --silent --show-error \ - https://osmo.example.com/.well-known/oauth-authorization-server + https://osmo.example.com/.well-known/oauth-authorization-server/mcp Confirm the exact resource URL and full delegated scope. Proxy metadata must also contain ``client_id_metadata_document_supported`` set to ``true`` and a @@ -338,11 +338,11 @@ Operate OIDC Proxy Safely * Keep the MCP ingress NetworkPolicy. It is additive, so audit other policies that select the same pod. -The public proxy surface is limited to exact protected-resource and -authorization-server metadata, ``/authorize``, ``/auth/callback``, -``/register``, ``/token``, and ``/consent`` routes. Gateway bypasses its own JWT -and semantic authorization filters only for these OAuth routes and exact -``/mcp`` in proxy mode. FastMCP authenticates ``/mcp``; all ``/api`` calls keep +The public proxy surface is the protected-resource and authorization-server +metadata documents plus everything under ``/mcp``, where FastMCP serves +``authorize``, ``token``, ``register``, ``consent`` and the callback. Gateway +bypasses its own JWT and semantic authorization filters only for that prefix +and the two metadata documents in proxy mode. FastMCP authenticates ``/mcp``; all ``/api`` calls keep normal Gateway validation and authorization. .. _mcp_deployment_troubleshooting: @@ -366,7 +366,7 @@ Troubleshooting secret and optional Redis password must exist at the configured absolute paths. * - Browser reports a redirect mismatch or no reply address - - Register exact ``https:///auth/callback`` on the confidential + - Register exact ``https:///mcp/auth/callback`` on the confidential upstream application. For Entra, use the Web platform for this server-side client. * - Browser reports ``Approval required`` diff --git a/docs/user_guide/getting_started/mcp.rst b/docs/user_guide/getting_started/mcp.rst index 036638b85..61161d0f1 100644 --- a/docs/user_guide/getting_started/mcp.rst +++ b/docs/user_guide/getting_started/mcp.rst @@ -77,7 +77,7 @@ either CIMD client identification or DCR registration. .. note:: The identity provider returns to the deployment's fixed - ``https:///auth/callback`` URL. After FastMCP completes that + ``https:///mcp/auth/callback`` URL. After FastMCP completes that exchange, the browser redirects to a temporary loopback URL owned by the MCP client. The administrator registers only the fixed upstream callback with the identity provider. @@ -169,7 +169,7 @@ Troubleshooting administrator approval. This is not an MCP client configuration error. * - The browser reports ``AADSTS50011`` or ``AADSTS900971`` - Ask the administrator to verify that the confidential upstream - application has the exact ``https:///auth/callback`` redirect + application has the exact ``https:///mcp/auth/callback`` redirect registered for the correct application type. * - MCP initialization returns ``HTTP 401`` - Run ``codex mcp logout osmo`` and ``codex mcp login osmo``. If the error diff --git a/src/service/mcp/auth.py b/src/service/mcp/auth.py index b88f70e27..3a0166158 100644 --- a/src/service/mcp/auth.py +++ b/src/service/mcp/auth.py @@ -264,6 +264,7 @@ def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime: required_scopes=[config.oidc_access_token_required_scope], http_client=http_client, ) + mcp_url = cast(str, config.resource_url) requested_scope = cast(str, config.auth_scope) upstream_scope = ' '.join((requested_scope, *_UPSTREAM_OIDC_SCOPES)) provider = OIDCProxy( @@ -271,9 +272,19 @@ def create_auth_runtime(config: MCPAuthConfig) -> MCPAuthRuntime: client_id=cast(str, config.oidc_client_id), client_secret=client_secret, token_verifier=verifier, - base_url=cast(str, config.issuer_url), - resource_base_url=cast(str, config.issuer_url), - issuer_url=cast(str, config.issuer_url), + # FastMCP builds its operational OAuth endpoints from base_url and its + # RFC 9728 resource identity from resource_base_url plus the MCP path. + # Publishing base_url at the MCP URL therefore keeps authorize, token, + # register, consent and the callback under /mcp instead of on the + # shared gateway root, while the origin keeps the resource named /mcp + # rather than /mcp/mcp. The path-scoped issuer is what the + # protected-resource document advertises in authorization_servers, + # which is what points clients at RFC 8414 path-aware discovery; the + # gateway serves that path, since FastMCP registers the document at + # the root. + base_url=mcp_url, + resource_base_url=mcp_url.removesuffix('/mcp'), + issuer_url=mcp_url, redirect_path='/auth/callback', allowed_client_redirect_uris=config.allowed_client_redirect_uris, client_storage=encrypted_store, diff --git a/src/service/mcp/server.py b/src/service/mcp/server.py index cbe018c1b..3f182c298 100644 --- a/src/service/mcp/server.py +++ b/src/service/mcp/server.py @@ -70,6 +70,19 @@ class MCPServiceConfig( le=60, description='Total timeout for each OSMO Gateway request.', json_schema_extra={'env': 'OSMO_MCP_REQUEST_TIMEOUT_SECONDS'}) + allowed_origins: list[str] = pydantic.Field( + default_factory=list, + description=( + 'Browser Origins permitted to call the MCP endpoint. Native MCP ' + 'clients send no Origin and stay allowed. An empty list rejects ' + 'every browser Origin.'), + json_schema_extra={'env': 'OSMO_MCP_ALLOWED_ORIGINS'}) + + @pydantic.field_validator('allowed_origins', mode='after') + @classmethod + def _discard_blank_origins(cls, value: list[str]) -> list[str]: + """Drop the empty entry an unset comma-separated variable produces.""" + return [origin.strip() for origin in value if origin.strip()] @pydantic.model_validator(mode='after') def _validate_gateway_url(self) -> 'MCPServiceConfig': @@ -103,13 +116,21 @@ async def health_ready(request: Request) -> JSONResponse: # pylint: disable=unu return server -def create_application(protocol_server: FastMCP) -> Starlette: +def create_application( + protocol_server: FastMCP, + allowed_origins: list[str] | None = None, +) -> Starlette: """Create the ASGI application for an MCP protocol server.""" application = protocol_server.http_app( path='/mcp', transport='streamable-http', stateless_http=True, json_response=True, + # FastMCP's own guard replaces the Origin allowlist the gateway used to + # implement in templated Lua. Passing allowed_origins alone is inert: + # the guard is only installed when host_origin_protection is not False. + host_origin_protection='auto', + allowed_origins=allowed_origins, ) application.add_middleware( request_body.RequestBodyLimitMiddleware, @@ -145,7 +166,16 @@ def create_runtime_application( protocol_server = create_mcp_server( auth_runtime.provider if auth_runtime is not None else None, ) - application = create_application(protocol_server) + # FastMCP serves its browser consent page on this deployment's own origin, + # and a same-origin form POST still carries an Origin header. Supplying any + # explicit allowlist turns off FastMCP's same-origin fallback for non- + # loopback hosts (fastmcp/server/http.py:297-306), so the deployment origin + # has to be listed explicitly or consent is rejected. + browser_origins = list(dict.fromkeys([ + str(config.gateway_url).rstrip('/'), + *config.allowed_origins, + ])) + application = create_application(protocol_server, browser_origins) protocol_lifespan = application.router.lifespan_context @contextlib.asynccontextmanager diff --git a/src/service/mcp/tests/test_auth.py b/src/service/mcp/tests/test_auth.py index 8c35b3c59..2867a586a 100644 --- a/src/service/mcp/tests/test_auth.py +++ b/src/service/mcp/tests/test_auth.py @@ -173,6 +173,11 @@ async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> N for route in application.routes if hasattr(route, 'path') } + # The MCP SDK registers OAuth handlers at fixed root paths + # (mcp/server/auth/routes.py) regardless of base_url, so the + # in-process paths stay at the root. The gateway publishes them + # under /mcp and rewrites the prefix back off; the advertised + # metadata below is the contract clients actually follow. self.assertIn('/authorize', route_paths) self.assertIn('/token', route_paths) self.assertIn('/register', route_paths) @@ -200,9 +205,21 @@ async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> N metadata_body['scopes_supported'], ['https://osmo.example/mcp/access_as_user'], ) + self.assertEqual( + metadata_body['issuer'], + 'https://osmo.example/mcp', + ) + self.assertEqual( + metadata_body['authorization_endpoint'], + 'https://osmo.example/mcp/authorize', + ) + self.assertEqual( + metadata_body['token_endpoint'], + 'https://osmo.example/mcp/token', + ) self.assertEqual( metadata_body['registration_endpoint'], - 'https://osmo.example/register', + 'https://osmo.example/mcp/register', ) self.assertTrue( metadata_body['client_id_metadata_document_supported'] @@ -212,6 +229,9 @@ async def test_factory_uses_plain_oidc_proxy_and_split_scope_contract(self) -> N protected.json()['scopes_supported'], ['https://osmo.example/mcp/access_as_user'], ) + # resource_base_url keeps the RFC 9728 identity at /mcp even + # though base_url moved there too; without it the advertised + # resource would become /mcp/mcp. self.assertEqual( protected.json()['resource'], 'https://osmo.example/mcp', diff --git a/src/service/mcp/tests/test_server.py b/src/service/mcp/tests/test_server.py index 48383ad8e..9c148b831 100644 --- a/src/service/mcp/tests/test_server.py +++ b/src/service/mcp/tests/test_server.py @@ -59,6 +59,8 @@ def test_create_application_direct_mode_uses_header_middleware(self) -> None: transport='streamable-http', stateless_http=True, json_response=True, + host_origin_protection='auto', + allowed_origins=None, ) self.assertEqual(application.add_middleware.call_args_list, [ mock.call( @@ -690,6 +692,40 @@ async def create_app_context( self.assertFalse(hasattr(application.state, 'mcp_app_context')) self.assertEqual(lifecycle_events, ['entered', 'exited']) + def test_deployment_origin_is_always_a_permitted_browser_origin(self) -> None: + """FastMCP's consent page POSTs same-origin, so the origin must be listed. + + Supplying any explicit allowlist disables FastMCP's same-origin + fallback for non-loopback hosts, which would otherwise reject consent. + """ + config = server.MCPServiceConfig( + gateway_url='https://gateway.test', + allowed_origins=['http://localhost:6274'], + ) + with mock.patch.object(server, 'create_application') as create: + server.create_runtime_application(config) + self.assertEqual( + create.call_args.args[1], + ['https://gateway.test', 'http://localhost:6274'], + ) + + def test_allowed_origins_drops_blank_entries(self) -> None: + """An unset comma-separated variable must not become one blank origin.""" + self.assertEqual( + server.MCPServiceConfig( + gateway_url='https://gateway.test', + allowed_origins=[''], + ).allowed_origins, + [], + ) + self.assertEqual( + server.MCPServiceConfig( + gateway_url='https://gateway.test', + allowed_origins=[' http://localhost:6274 ', 'http://[::1]:6274'], + ).allowed_origins, + ['http://localhost:6274', 'http://[::1]:6274'], + ) + async def test_runtime_application_cleans_up_in_dependency_order(self) -> None: config = server.MCPServiceConfig(gateway_url='https://gateway.test') app_context = gateway.AppContext(gateway=mock.Mock())