diff --git a/infra/packages/shared/src/gateway_priorities.ts b/infra/packages/shared/src/gateway_priorities.ts index 890ff3d3bb0..1ae2cb8de9d 100644 --- a/infra/packages/shared/src/gateway_priorities.ts +++ b/infra/packages/shared/src/gateway_priorities.ts @@ -15,6 +15,7 @@ export enum GatewayService { CONNECTION_GATEWAY = 'CONNECTION_GATEWAY', AUTHENTICATION_SERVICE = 'AUTHENTICATION_SERVICE', EMAIL_SERVICE = 'EMAIL_SERVICE', + MCP_SERVER = 'MCP_SERVER', DOCUMENT_COGNITION_SERVICE = 'DOCUMENT_COGNITION_SERVICE', } @@ -39,6 +40,7 @@ export const GATEWAY_PRIORITIES: GatewayPriorityMap = { [GatewayService.CONNECTION_GATEWAY]: 90, [GatewayService.AUTHENTICATION_SERVICE]: 100, [GatewayService.EMAIL_SERVICE]: 110, + [GatewayService.MCP_SERVER]: 130, [GatewayService.DOCUMENT_COGNITION_SERVICE]: 120, [GatewayService.CONVERT_SERVICE]: 3000, }; diff --git a/infra/packages/shared/src/service_urls.ts b/infra/packages/shared/src/service_urls.ts index d84c37281ce..a98c78b55b1 100644 --- a/infra/packages/shared/src/service_urls.ts +++ b/infra/packages/shared/src/service_urls.ts @@ -15,6 +15,7 @@ export enum ServiceUrl { LEXICAL_SERVICE_URL = 'LEXICAL_SERVICE_URL', UNFURL_SERVICE_URL = 'UNFURL_SERVICE_URL', AGENT_HARNESS_SERVICE_URL = 'AGENT_HARNESS_SERVICE_URL', + MCP_SERVER_URL = 'MCP_SERVER_URL', } /** @@ -47,6 +48,7 @@ const DEV_SERVICE_URLS: ServiceUrlMap = { [ServiceUrl.UNFURL_SERVICE_URL]: 'https://dev-gateway.macro.com/unfurl', [ServiceUrl.AGENT_HARNESS_SERVICE_URL]: 'https://dev-gateway.macro.com/agent-harness', + [ServiceUrl.MCP_SERVER_URL]: 'https://dev-gateway.macro.com/mcp', }; /** @@ -70,6 +72,7 @@ const PROD_SERVICE_URLS: ServiceUrlMap = { [ServiceUrl.UNFURL_SERVICE_URL]: 'https://gateway.macro.com/unfurl', [ServiceUrl.AGENT_HARNESS_SERVICE_URL]: 'https://gateway.macro.com/agent-harness', + [ServiceUrl.MCP_SERVER_URL]: 'https://gateway.macro.com/mcp', }; /** diff --git a/infra/stacks/fusionauth-instance/index.ts b/infra/stacks/fusionauth-instance/index.ts index 7cfe0b9c309..a818494e587 100644 --- a/infra/stacks/fusionauth-instance/index.ts +++ b/infra/stacks/fusionauth-instance/index.ts @@ -303,6 +303,9 @@ const macroApplication = new FusionAuthApplication( ] : []), `https://mcp-server${stack === 'prod' ? '' : `-${stack}`}.macro.com/oauth/callback`, + ...(stack === 'dev' || stack === 'prod' + ? [`${getServiceUrl(ServiceUrl.MCP_SERVER_URL)}/oauth/callback`] + : []), ...(stack === 'local' || stack === 'dev' ? ['http://localhost:8085/*', 'http://localhost:8085/oauth/*'] : []), diff --git a/infra/stacks/mcp-server/mcp-server.ts b/infra/stacks/mcp-server/mcp-server.ts index f4a810247b5..d555d514628 100644 --- a/infra/stacks/mcp-server/mcp-server.ts +++ b/infra/stacks/mcp-server/mcp-server.ts @@ -8,16 +8,21 @@ import { datadogAgentContainer, fargateLogRouterSidecarContainer, serviceLoadBalancer, + ServiceTargetGroup, } from '../../packages/resources'; import { EcrImage } from '../../packages/service'; import { BASE_DOMAIN, CLOUD_TRAIL_SNS_TOPIC_ARN, DopplerEcsEnvironment, + getGatewayAlb, getKafkaClusterPolicy, + GatewayService, stack, } from '../../packages/shared'; +const gatewayLoadBalancer = getGatewayAlb(); + const BASE_NAME = pulumi.getProject(); const REPO_ROOT = '../../..'; @@ -107,6 +112,22 @@ export class McpServer extends pulumi.ComponentResource { this.serviceAlbSg = sg.serviceAlbSg; this.serviceSg = sg.serviceSg; + const gatewayTargetGroup = new ServiceTargetGroup( + `${stack}-${BASE_NAME}`, + { + tags: this.tags, + listenerArn: gatewayLoadBalancer.httpsListenerArn, + vpcId: vpc.vpcId, + containerPort: serviceContainerPort, + service: GatewayService.MCP_SERVER, + healthCheckPath, + pathPatterns: ['/mcp', '/mcp/*'], + serviceSecurityGroupId: this.serviceSg.id, + albSecurityGroupId: gatewayLoadBalancer.albSecurityGroupId, + }, + { parent: this } + ); + // lb const { targetGroup, lb, listener } = serviceLoadBalancer(this, { serviceName: BASE_NAME, @@ -245,6 +266,18 @@ export class McpServer extends pulumi.ComponentResource { enable: true, rollback: true, }, + loadBalancers: [ + { + targetGroupArn: targetGroup.arn, + containerName: 'service', + containerPort: serviceContainerPort, + }, + { + targetGroupArn: gatewayTargetGroup.target_group.arn, + containerName: 'service', + containerPort: serviceContainerPort, + }, + ], taskDefinitionArgs: { taskRole: { roleArn: this.role.arn, @@ -299,6 +332,10 @@ export class McpServer extends pulumi.ComponentResource { }, { parent: this, + // ECS refuses a service whose target group is not yet associated with + // a load balancer. The dedicated HTTPS listener and the gateway + // listener rule each create that association for their target group. + dependsOn: [listener, gatewayTargetGroup.listener_rule], } ); diff --git a/services/mcp_auth_proxy/Cargo.toml b/services/mcp_auth_proxy/Cargo.toml index 0a30b408c82..4fa791d2aa3 100644 --- a/services/mcp_auth_proxy/Cargo.toml +++ b/services/mcp_auth_proxy/Cargo.toml @@ -37,4 +37,5 @@ uuid = { workspace = true } workspace-hack = { version = "0.1", path = "../../crates/workspace-hack" } [dev-dependencies] +macro_auth = { path = "../../crates/macro_auth", features = ["testing"] } tokio = { workspace = true } diff --git a/services/mcp_auth_proxy/src/domain/service.rs b/services/mcp_auth_proxy/src/domain/service.rs index c85ccbae0ee..b0d60d0a716 100644 --- a/services/mcp_auth_proxy/src/domain/service.rs +++ b/services/mcp_auth_proxy/src/domain/service.rs @@ -81,6 +81,20 @@ pub trait InflightAuthStore: Send + Sync { fn cleanup_expired(&self) -> impl Future> + Send; } +/// Streamable HTTP resource URL advertised in protected-resource metadata. +/// +/// MCP clients require a `resource` field. When `MCP_PUBLIC_URL` is an origin +/// (the current Doppler value), append `/mcp`. When it already ends in `/mcp` +/// (a later gateway cutover), do not append again. +fn mcp_resource_url(public_url: &str) -> String { + let base = public_url.trim_end_matches('/'); + if base.ends_with("/mcp") { + base.to_owned() + } else { + format!("{base}/mcp") + } +} + /// Domain service backing the MCP OAuth broker. pub struct McpAuthProxyServiceImpl { inflight_auth: Arc, @@ -205,6 +219,7 @@ where tracing::debug!("oauth-protected-resource metadata requested"); let base = &self.public_url; serde_json::json!({ + "resource": mcp_resource_url(base), "authorization_server": base, "authorization_servers": [base], }) diff --git a/services/mcp_auth_proxy/src/domain/service/test.rs b/services/mcp_auth_proxy/src/domain/service/test.rs index 0c16c7a73c4..901c73e9845 100644 --- a/services/mcp_auth_proxy/src/domain/service/test.rs +++ b/services/mcp_auth_proxy/src/domain/service/test.rs @@ -114,6 +114,38 @@ fn service(store: FakeInflightAuth) -> McpAuthProxyServiceImpl ) } +#[test] +fn protected_resource_metadata_includes_required_resource_url() { + let json = service(FakeInflightAuth::default()).protected_resource_metadata(); + assert_eq!( + json, + serde_json::json!({ + "resource": "https://mcp.example.com/mcp", + "authorization_server": "https://mcp.example.com", + "authorization_servers": ["https://mcp.example.com"], + }) + ); +} + +#[test] +fn protected_resource_metadata_does_not_double_the_mcp_path() { + let service = McpAuthProxyServiceImpl::new( + "https://gateway.macro.com/mcp".to_owned(), + Arc::new(FakeInflightAuth::default()), + Arc::new(FakeOAuthProvider { + expires_in: UPSTREAM_EXPIRES_IN, + }), + ); + assert_eq!( + service.protected_resource_metadata(), + serde_json::json!({ + "resource": "https://gateway.macro.com/mcp", + "authorization_server": "https://gateway.macro.com/mcp", + "authorization_servers": ["https://gateway.macro.com/mcp"], + }) + ); +} + fn issued_code(access_token_expires_at: Option) -> IssuedAuthorizationCode { IssuedAuthorizationCode { access_token: AccessToken::from("upstream-access"), diff --git a/services/mcp_auth_proxy/src/inbound.rs b/services/mcp_auth_proxy/src/inbound.rs index f76c2e5f6ba..e7e9e216911 100644 --- a/services/mcp_auth_proxy/src/inbound.rs +++ b/services/mcp_auth_proxy/src/inbound.rs @@ -1,5 +1,8 @@ //! Inbound adapters for the MCP OAuth broker. +/// Path the shared gateway forwards without stripping. +pub(crate) const GATEWAY_PATH_PREFIX: &str = "/mcp"; + /// Axum router for the MCP OAuth broker. pub mod axum_router; /// Bearer token middleware for the protected MCP endpoint. diff --git a/services/mcp_auth_proxy/src/inbound/axum_router.rs b/services/mcp_auth_proxy/src/inbound/axum_router.rs index 88ddbd1b764..4226a22aff0 100644 --- a/services/mcp_auth_proxy/src/inbound/axum_router.rs +++ b/services/mcp_auth_proxy/src/inbound/axum_router.rs @@ -1,5 +1,8 @@ //! Axum router for the MCP OAuth broker. +#[cfg(test)] +mod test; + use std::time::Duration; use axum::{ @@ -217,10 +220,6 @@ where "/.well-known/oauth-protected-resource/mcp", routing::get(protected_resource_metadata), ) - .route( - "/mcp/.well-known/oauth-protected-resource", - routing::get(protected_resource_metadata), - ) .route( "/.well-known/oauth-authorization-server", routing::get(authorization_server_metadata), @@ -229,10 +228,6 @@ where "/.well-known/oauth-authorization-server/mcp", routing::get(authorization_server_metadata), ) - .route( - "/mcp/.well-known/oauth-authorization-server", - routing::get(authorization_server_metadata), - ) .route("/authorize", routing::get(authorize)) .route("/register", routing::post(register)) .route("/oauth/callback", routing::get(oauth_callback)) @@ -247,7 +242,13 @@ where super::middleware::validate_bearer, )); - oauth_routes.merge(mcp_route).layer(mcp_cors_layer()) + mount_at_root_and_prefix(oauth_routes.merge(mcp_route)).layer(mcp_cors_layer()) +} + +fn mount_at_root_and_prefix(inner: Router) -> Router { + Router::new() + .merge(inner.clone()) + .nest(super::GATEWAY_PATH_PREFIX, inner) } /// CORS layer for the MCP router. diff --git a/services/mcp_auth_proxy/src/inbound/axum_router/test.rs b/services/mcp_auth_proxy/src/inbound/axum_router/test.rs new file mode 100644 index 00000000000..4a9c9d83055 --- /dev/null +++ b/services/mcp_auth_proxy/src/inbound/axum_router/test.rs @@ -0,0 +1,207 @@ +use std::sync::Arc; + +use super::{health, mcp_router, mount_at_root_and_prefix}; +use crate::domain::{ + models::{ + AccessToken, IssuedAuthorizationCode, PendingAuthorization, RefreshToken, UpstreamTokens, + }, + ports::OAuthProvider, + service::{InflightAuthStore, McpAuthProxyServiceImpl}, +}; +use axum::{ + Router, + body::Body, + http::{Request, StatusCode}, + routing::get, +}; +use macro_auth::middleware::decode_jwt::JwtValidationArgs; +use tower::ServiceExt; + +async fn ok() -> &'static str { + "ok" +} + +fn sample_app() -> Router { + mount_at_root_and_prefix( + Router::new() + .route("/health", get(ok)) + .route("/oauth/callback", get(ok)) + .route("/.well-known/oauth-protected-resource", get(ok)) + .route("/.well-known/oauth-protected-resource/mcp", get(ok)) + .route("/mcp", get(ok)), + ) +} + +async fn get_status(app: Router, path: &str) -> StatusCode { + app.oneshot( + Request::builder() + .uri(path) + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap() + .status() +} + +#[derive(Clone, Default)] +struct NoopInflightAuth; + +impl InflightAuthStore for NoopInflightAuth { + async fn insert_pending( + &self, + _session_id: &str, + _pending: PendingAuthorization, + ) -> anyhow::Result<()> { + Ok(()) + } + + async fn take_pending( + &self, + _session_id: &str, + ) -> anyhow::Result> { + Ok(None) + } + + async fn insert_issued( + &self, + _code: &str, + _issued: IssuedAuthorizationCode, + ) -> anyhow::Result<()> { + Ok(()) + } + + async fn take_issued(&self, _code: &str) -> anyhow::Result> { + Ok(None) + } + + async fn cleanup_expired(&self) -> anyhow::Result<()> { + Ok(()) + } +} + +struct NoopOAuthProvider; + +impl OAuthProvider for NoopOAuthProvider { + fn construct_authorize_url(&self, state: &str) -> anyhow::Result { + Ok(format!( + "https://upstream.example.com/authorize?state={state}" + )) + } + + fn exchange_authorization_code<'a>( + &'a self, + _code: &'a str, + ) -> crate::domain::ports::UpstreamTokensFuture<'a> { + Box::pin(async { + Ok(UpstreamTokens { + access_token: AccessToken::from("access"), + refresh_token: RefreshToken::from("refresh"), + expires_in: 3600, + }) + }) + } + + fn refresh_access_token<'a>( + &'a self, + _refresh_token: &'a RefreshToken, + ) -> crate::domain::ports::UpstreamTokensFuture<'a> { + Box::pin(async { + Ok(UpstreamTokens { + access_token: AccessToken::from("access"), + refresh_token: RefreshToken::from("refresh"), + expires_in: 3600, + }) + }) + } +} + +fn built_router() -> Router { + mcp_router( + McpAuthProxyServiceImpl::new( + "https://mcp.example.com".to_owned(), + Arc::new(NoopInflightAuth), + Arc::new(NoopOAuthProvider), + ), + JwtValidationArgs::new_testing(), + Router::new().route("/", get(ok)), + ) +} + +#[tokio::test] +async fn health_is_reachable_at_root_and_gateway_prefix() { + for path in ["/health", "/mcp/health"] { + let response = mount_at_root_and_prefix(Router::new().route("/health", get(health))) + .oneshot( + Request::builder() + .uri(path) + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::OK, "{path}"); + } +} + +#[tokio::test] +async fn existing_paths_stay_and_are_also_served_under_the_prefix() { + for path in [ + "/health", + "/mcp/health", + "/oauth/callback", + "/mcp/oauth/callback", + "/.well-known/oauth-protected-resource", + "/.well-known/oauth-protected-resource/mcp", + "/mcp/.well-known/oauth-protected-resource", + "/mcp/.well-known/oauth-protected-resource/mcp", + "/mcp", + "/mcp/mcp", + ] { + assert_eq!( + get_status(sample_app(), path).await, + StatusCode::OK, + "{path}" + ); + } +} + +#[tokio::test] +async fn mcp_router_builds_without_overlapping_routes() { + let app = built_router(); + for path in [ + "/health", + "/mcp/health", + "/.well-known/oauth-protected-resource", + "/.well-known/oauth-protected-resource/mcp", + "/mcp/.well-known/oauth-protected-resource", + "/mcp/.well-known/oauth-protected-resource/mcp", + "/.well-known/oauth-authorization-server", + "/mcp/.well-known/oauth-authorization-server", + ] { + assert_eq!( + get_status(app.clone(), path).await, + StatusCode::OK, + "{path}" + ); + } +} + +#[tokio::test] +async fn unprefixed_unknown_path_is_not_rewritten_onto_the_prefix() { + let response = mount_at_root_and_prefix(Router::new().route("/health", get(health))) + .oneshot( + Request::builder() + .uri("/missing") + .method("GET") + .body(Body::empty()) + .unwrap(), + ) + .await + .unwrap(); + + assert_eq!(response.status(), StatusCode::NOT_FOUND); +} diff --git a/services/mcp_auth_proxy/src/inbound/middleware.rs b/services/mcp_auth_proxy/src/inbound/middleware.rs index f006e54fc9b..dcb9bacfc4e 100644 --- a/services/mcp_auth_proxy/src/inbound/middleware.rs +++ b/services/mcp_auth_proxy/src/inbound/middleware.rs @@ -1,5 +1,8 @@ //! Bearer token middleware for the protected MCP endpoint. +#[cfg(test)] +mod test; + use axum::{ body::Body, http::{ @@ -14,7 +17,12 @@ use macro_user_id::user_id::MacroUserIdStr; #[derive(Clone)] pub struct JwtAccessToken(pub String); -const RESOURCE_METADATA_PATH: &str = "/.well-known/oauth-protected-resource/mcp"; +fn resource_metadata_path() -> String { + format!( + "{}/.well-known/oauth-protected-resource", + super::GATEWAY_PATH_PREFIX + ) +} fn absolute_resource_metadata_url(request: &Request) -> String { let scheme = request @@ -34,16 +42,17 @@ fn absolute_resource_metadata_url(request: &Request) -> String { }) .unwrap_or("localhost"); + let metadata_path = resource_metadata_path(); let mut uri = Uri::builder() .scheme(scheme) .authority(authority) - .path_and_query(RESOURCE_METADATA_PATH) + .path_and_query(metadata_path.as_str()) .build() .expect("valid resource metadata uri") .to_string(); if !uri.starts_with("http://") && !uri.starts_with("https://") { - uri = format!("{scheme}://{authority}{RESOURCE_METADATA_PATH}"); + uri = format!("{scheme}://{authority}{metadata_path}"); } uri diff --git a/services/mcp_auth_proxy/src/inbound/middleware/test.rs b/services/mcp_auth_proxy/src/inbound/middleware/test.rs new file mode 100644 index 00000000000..9296bec4cf6 --- /dev/null +++ b/services/mcp_auth_proxy/src/inbound/middleware/test.rs @@ -0,0 +1,43 @@ +use super::absolute_resource_metadata_url; +use axum::{ + body::Body, + http::{Request, header::HOST}, +}; + +fn request_with_host(host: &str, proto: Option<&str>) -> Request { + let mut builder = Request::builder() + .uri("/mcp") + .method("GET") + .header(HOST, host); + if let Some(proto) = proto { + builder = builder.header("x-forwarded-proto", proto); + } + builder.body(Body::empty()).unwrap() +} + +#[test] +fn resource_metadata_uses_path_style_well_known_on_the_gateway_host() { + let request = request_with_host("gateway.macro.com", Some("https")); + assert_eq!( + absolute_resource_metadata_url(&request), + "https://gateway.macro.com/mcp/.well-known/oauth-protected-resource" + ); +} + +#[test] +fn resource_metadata_uses_path_style_well_known_on_the_legacy_host() { + let request = request_with_host("mcp-server.macro.com", Some("https")); + assert_eq!( + absolute_resource_metadata_url(&request), + "https://mcp-server.macro.com/mcp/.well-known/oauth-protected-resource" + ); +} + +#[test] +fn resource_metadata_defaults_to_http_without_forwarded_proto() { + let request = request_with_host("dev-gateway.macro.com", None); + assert_eq!( + absolute_resource_metadata_url(&request), + "http://dev-gateway.macro.com/mcp/.well-known/oauth-protected-resource" + ); +} diff --git a/services/mcp_service/src/main.rs b/services/mcp_service/src/main.rs index 0571f75dcfc..4a2f5729405 100644 --- a/services/mcp_service/src/main.rs +++ b/services/mcp_service/src/main.rs @@ -55,6 +55,8 @@ async fn main() -> anyhow::Result<()> { context.mcp_public_host.clone(), "localhost".into(), "127.0.0.1".into(), + "gateway.macro.com".into(), + "dev-gateway.macro.com".into(), ]); config.stateful_mode = false; config.json_response = true;