From c4384256b2f3ec680e755fc9788048ece447fac4 Mon Sep 17 00:00:00 2001 From: Omar McIver Date: Fri, 11 Sep 2026 11:49:52 -0500 Subject: [PATCH] fix(mcp): discover OAuth endpoints per spec and support dynamic client registration MCP OAuth login failed with an opaque HTTP 500 ("Internal server error.") for public servers such as monday.com and Microsoft 365. Two defects: 1. Discovery appended `.well-known/...` to the full server URL, so `https://mcp.monday.com/sse` was probed at `https://mcp.monday.com/sse/.well-known/oauth-authorization-server`, which those servers answer with 401. RFC 8414 places the well-known segment between the origin and the resource path (`/.well-known/oauth-authorization-server/sse`). Discovery now follows the RFC 9728 protected-resource pointer first, then probes the origin-relative RFC 8414 and OIDC locations, keeping the previous appended spelling as a last-resort fallback. 2. The client ID was hardcoded to "aionui". Servers that require RFC 7591 dynamic client registration reject it, so login could not complete even once discovery succeeded. Clients are now registered dynamically when the server advertises a registration endpoint, and the resulting credentials are reused for the token exchange and refresh. Discovery failures also no longer map to a 500: they are caused by the remote server or the configured URL, so they now return 502 with the actual reason instead of being scrubbed to "Internal server error." Verified against the live monday.com MCP server: the two URLs the old code probed return 401, while the new discovery chain resolves the endpoints and reports the advertised registration endpoint. --- Cargo.lock | 1 + crates/aionui-mcp/Cargo.toml | 1 + crates/aionui-mcp/src/error.rs | 6 + crates/aionui-mcp/src/lib.rs | 1 + crates/aionui-mcp/src/oauth_discovery.rs | 272 ++++++++++++++++++ crates/aionui-mcp/src/oauth_service.rs | 146 ++++++---- crates/aionui-mcp/src/routes.rs | 17 ++ .../aionui-mcp/tests/oauth_discovery_live.rs | 35 +++ 8 files changed, 427 insertions(+), 52 deletions(-) create mode 100644 crates/aionui-mcp/src/oauth_discovery.rs create mode 100644 crates/aionui-mcp/tests/oauth_discovery_live.rs diff --git a/Cargo.lock b/Cargo.lock index 94ed1f9fb..7559f123d 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -740,6 +740,7 @@ dependencies = [ "toml 0.8.23", "tracing", "tracing-subscriber", + "url", ] [[package]] diff --git a/crates/aionui-mcp/Cargo.toml b/crates/aionui-mcp/Cargo.toml index b9381f165..53fe0da17 100644 --- a/crates/aionui-mcp/Cargo.toml +++ b/crates/aionui-mcp/Cargo.toml @@ -19,6 +19,7 @@ thiserror.workspace = true oauth2.workspace = true open.workspace = true toml.workspace = true +url.workspace = true tokio.workspace = true tracing.workspace = true reqwest.workspace = true diff --git a/crates/aionui-mcp/src/error.rs b/crates/aionui-mcp/src/error.rs index 675f40514..1cced6583 100644 --- a/crates/aionui-mcp/src/error.rs +++ b/crates/aionui-mcp/src/error.rs @@ -25,6 +25,12 @@ pub enum McpError { #[error("OAuth error: {0}")] OAuth(String), + /// OAuth endpoint discovery failed — the server did not publish usable + /// authorization metadata. Caused by remote/config state, not an internal + /// fault, so it must not surface as a 500. + #[error("{0}")] + OAuthDiscovery(String), + #[error("{0}")] Database(#[from] aionui_db::DbError), diff --git a/crates/aionui-mcp/src/lib.rs b/crates/aionui-mcp/src/lib.rs index 6ea9d814b..3efae5893 100644 --- a/crates/aionui-mcp/src/lib.rs +++ b/crates/aionui-mcp/src/lib.rs @@ -5,6 +5,7 @@ pub mod adapter; pub mod adapters; pub mod connection_test; pub mod error; +pub mod oauth_discovery; pub mod oauth_service; pub mod routes; pub mod service; diff --git a/crates/aionui-mcp/src/oauth_discovery.rs b/crates/aionui-mcp/src/oauth_discovery.rs new file mode 100644 index 000000000..14170909e --- /dev/null +++ b/crates/aionui-mcp/src/oauth_discovery.rs @@ -0,0 +1,272 @@ +//! OAuth endpoint discovery and dynamic client registration for MCP servers. +//! +//! Implements the discovery chain the MCP authorization spec requires: +//! +//! 1. RFC 9728 protected-resource metadata to locate the authorization server +//! 2. RFC 8414 / OIDC authorization-server metadata for the endpoints +//! 3. RFC 7591 dynamic client registration when the server advertises it +//! +//! The well-known probe order matters: RFC 8414 inserts `.well-known/...` +//! between the origin and the resource path (`/sse` -> `/.well-known/x/sse`), +//! which is *not* the same as appending it to the full URL. Appending to the +//! full path is what many public MCP servers reject, so both spellings are +//! tried, origin-relative first. + +use serde::Deserialize; +use url::Url; + +use crate::error::McpError; + +/// OAuth Authorization Server Metadata (RFC 8414) — subset of fields we need. +#[derive(Debug, Clone, Deserialize)] +pub struct OAuthServerMetadata { + pub authorization_endpoint: String, + pub token_endpoint: String, + /// RFC 7591 dynamic client registration endpoint, when supported. + #[serde(default)] + pub registration_endpoint: Option, +} + +/// OAuth Protected Resource Metadata (RFC 9728) — subset of fields we need. +#[derive(Debug, Deserialize)] +struct ProtectedResourceMetadata { + #[serde(default)] + authorization_servers: Vec, +} + +/// Credentials for the OAuth client used against one MCP server. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct ClientCredentials { + pub client_id: String, + pub client_secret: Option, +} + +/// Successful RFC 7591 registration response — subset of fields we need. +#[derive(Debug, Deserialize)] +struct RegistrationResponse { + client_id: String, + #[serde(default)] + client_secret: Option, +} + +/// Build the candidate well-known URLs for a metadata document, in probe order. +/// +/// For `https://host/sse` and suffix `oauth-authorization-server` this yields: +/// +/// 1. `https://host/.well-known/oauth-authorization-server/sse` (RFC 8414) +/// 2. `https://host/.well-known/oauth-authorization-server` (path-less form) +/// 3. `https://host/sse/.well-known/oauth-authorization-server` (legacy) +/// +/// Returns an empty vec when `server_url` cannot be parsed as a URL. +pub fn well_known_candidates(server_url: &str, suffix: &str) -> Vec { + let Ok(url) = Url::parse(server_url) else { + return Vec::new(); + }; + let Some(origin) = url_origin(&url) else { + return Vec::new(); + }; + + let path = url.path().trim_end_matches('/'); + let mut candidates = Vec::new(); + + if !path.is_empty() { + candidates.push(format!("{origin}/.well-known/{suffix}{path}")); + } + candidates.push(format!("{origin}/.well-known/{suffix}")); + if !path.is_empty() { + candidates.push(format!("{origin}{path}/.well-known/{suffix}")); + } + + candidates +} + +/// Serialize a URL's origin as `scheme://host[:port]`. +/// +/// `Url::origin()` is not used because it opaquefies non-special schemes; we +/// only ever probe http(s) MCP endpoints and want a plain string. +fn url_origin(url: &Url) -> Option { + let scheme = url.scheme(); + let host = url.host_str()?; + match url.port() { + Some(port) => Some(format!("{scheme}://{host}:{port}")), + None => Some(format!("{scheme}://{host}")), + } +} + +/// Discovery and registration over a shared HTTP client. +pub struct OAuthDiscovery<'a> { + http_client: &'a reqwest::Client, +} + +impl<'a> OAuthDiscovery<'a> { + pub fn new(http_client: &'a reqwest::Client) -> Self { + Self { http_client } + } + + /// Discover the authorization server metadata for an MCP server URL. + /// + /// Follows the RFC 9728 pointer when present, then probes RFC 8414 and + /// OIDC well-known locations. Returns the first document that parses. + pub async fn discover(&self, server_url: &str) -> Result { + // RFC 9728: the resource may name its authorization server(s). + let mut issuers: Vec = Vec::new(); + for candidate in well_known_candidates(server_url, "oauth-protected-resource") { + if let Some(resource) = self.fetch::(&candidate).await { + issuers = resource.authorization_servers; + break; + } + } + + // Probe the advertised issuers first, then the resource URL itself. + let mut probe_bases: Vec = issuers; + probe_bases.push(server_url.to_string()); + + for base in &probe_bases { + for suffix in ["oauth-authorization-server", "openid-configuration"] { + for candidate in well_known_candidates(base, suffix) { + if let Some(metadata) = self.fetch::(&candidate).await { + return Ok(metadata); + } + } + } + } + + Err(McpError::OAuthDiscovery(format!( + "Could not discover OAuth endpoints for '{server_url}'. \ + The server did not publish RFC 8414 authorization-server metadata, \ + OpenID configuration, or RFC 9728 protected-resource metadata at any \ + well-known location." + ))) + } + + /// Register a public OAuth client via RFC 7591 dynamic client registration. + pub async fn register_client( + &self, + registration_endpoint: &str, + redirect_uri: &str, + ) -> Result { + let body = serde_json::json!({ + "client_name": "AionUi", + "redirect_uris": [redirect_uri], + "grant_types": ["authorization_code", "refresh_token"], + "response_types": ["code"], + "token_endpoint_auth_method": "none", + }); + + let resp = self + .http_client + .post(registration_endpoint) + .json(&body) + .send() + .await + .map_err(|e| McpError::OAuth(format!("Client registration request failed: {e}")))?; + + let status = resp.status(); + if !status.is_success() { + return Err(McpError::OAuth(format!( + "Client registration at '{registration_endpoint}' returned {status}" + ))); + } + + let registered: RegistrationResponse = resp + .json() + .await + .map_err(|e| McpError::OAuth(format!("Failed to parse client registration response: {e}")))?; + + Ok(ClientCredentials { + client_id: registered.client_id, + client_secret: registered.client_secret, + }) + } + + /// GET and deserialize a metadata document, returning `None` on any failure. + async fn fetch(&self, url: &str) -> Option { + let resp = self.http_client.get(url).send().await.ok()?; + if !resp.status().is_success() { + return None; + } + resp.json::().await.ok() + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn candidates_put_origin_relative_path_suffix_first() { + let candidates = well_known_candidates("https://mcp.monday.com/sse", "oauth-authorization-server"); + assert_eq!( + candidates, + vec![ + "https://mcp.monday.com/.well-known/oauth-authorization-server/sse", + "https://mcp.monday.com/.well-known/oauth-authorization-server", + "https://mcp.monday.com/sse/.well-known/oauth-authorization-server", + ] + ); + } + + /// Regression guard for the reported bug: the legacy "append to full path" + /// spelling must not be the only thing we try, because public MCP servers + /// answer 401 there. + #[test] + fn candidates_are_not_only_the_legacy_appended_form() { + let legacy = "https://mcp.monday.com/sse/.well-known/oauth-authorization-server"; + let candidates = well_known_candidates("https://mcp.monday.com/sse", "oauth-authorization-server"); + assert!(candidates.len() > 1); + assert_ne!(candidates[0], legacy); + assert!(candidates.contains(&legacy.to_string())); + } + + #[test] + fn candidates_for_root_url_have_no_duplicate_path_forms() { + let candidates = well_known_candidates("https://example.com/", "openid-configuration"); + assert_eq!(candidates, vec!["https://example.com/.well-known/openid-configuration"]); + } + + #[test] + fn candidates_preserve_non_default_port() { + let candidates = well_known_candidates("http://127.0.0.1:8931/mcp", "oauth-authorization-server"); + assert_eq!( + candidates[0], + "http://127.0.0.1:8931/.well-known/oauth-authorization-server/mcp" + ); + } + + #[test] + fn candidates_for_nested_path() { + let candidates = well_known_candidates("https://example.com/api/v1/mcp", "oauth-protected-resource"); + assert_eq!( + candidates[0], + "https://example.com/.well-known/oauth-protected-resource/api/v1/mcp" + ); + } + + #[test] + fn candidates_empty_for_unparseable_url() { + assert!(well_known_candidates("not a url", "oauth-authorization-server").is_empty()); + } + + #[test] + fn metadata_parses_without_registration_endpoint() { + let metadata: OAuthServerMetadata = serde_json::from_str( + r#"{"authorization_endpoint":"https://a.example/auth","token_endpoint":"https://a.example/token"}"#, + ) + .unwrap(); + assert_eq!(metadata.registration_endpoint, None); + } + + #[test] + fn metadata_parses_with_registration_endpoint_and_extra_fields() { + let metadata: OAuthServerMetadata = serde_json::from_str( + r#"{"issuer":"https://a.example","authorization_endpoint":"https://a.example/auth", + "token_endpoint":"https://a.example/token", + "registration_endpoint":"https://a.example/register"}"#, + ) + .unwrap(); + assert_eq!( + metadata.registration_endpoint.as_deref(), + Some("https://a.example/register") + ); + } +} diff --git a/crates/aionui-mcp/src/oauth_service.rs b/crates/aionui-mcp/src/oauth_service.rs index dddce6ed0..f6b7f26e3 100644 --- a/crates/aionui-mcp/src/oauth_service.rs +++ b/crates/aionui-mcp/src/oauth_service.rs @@ -7,16 +7,16 @@ use aionui_common::{TimestampMs, now_ms}; use aionui_db::{IOAuthTokenRepository, UpsertOAuthTokenParams}; use oauth2::basic::BasicClient; use oauth2::{ - AuthUrl, AuthorizationCode, ClientId, CsrfToken, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, RefreshToken, - TokenResponse, TokenUrl, + AuthUrl, AuthorizationCode, ClientId, ClientSecret, CsrfToken, PkceCodeChallenge, PkceCodeVerifier, RedirectUrl, + RefreshToken, TokenResponse, TokenUrl, }; -use serde::Deserialize; use tokio::io::{AsyncReadExt, AsyncWriteExt}; use tokio::net::TcpListener; use tokio::sync::Mutex; use tracing::{debug, warn}; use crate::error::McpError; +use crate::oauth_discovery::{ClientCredentials, OAuthDiscovery, OAuthServerMetadata}; // --------------------------------------------------------------------------- // Constants @@ -31,17 +31,6 @@ const DEFAULT_CLIENT_ID: &str = "aionui"; /// Token expiry safety margin (refresh 5 minutes before expiration). const EXPIRY_MARGIN_MS: i64 = 5 * 60 * 1000; -// --------------------------------------------------------------------------- -// Discovery response -// --------------------------------------------------------------------------- - -/// OAuth Authorization Server Metadata (RFC 8414) — subset of fields we need. -#[derive(Debug, Deserialize)] -struct OAuthServerMetadata { - authorization_endpoint: String, - token_endpoint: String, -} - // --------------------------------------------------------------------------- // Pending login state // --------------------------------------------------------------------------- @@ -56,6 +45,10 @@ struct PendingLogin { auth_url: String, token_url: String, redirect_url: String, + /// Credentials the authorize request was made with. The token exchange must + /// reuse exactly these — a dynamically registered client_id differs per + /// server and per registration. + credentials: ClientCredentials, } // --------------------------------------------------------------------------- @@ -72,6 +65,16 @@ pub struct McpOAuthService { http_client: reqwest::Client, /// Mutex protecting pending login state by (user_id, oauth_state). pending: Arc>>, + /// Dynamically registered client credentials, keyed by server URL. + /// + /// Cached in-process so the authorize/exchange/refresh legs of one login + /// share a client_id without re-registering on every call. + /// + /// Not persisted: after a restart the cache is empty, so a refresh against + /// a registration-only server falls back to the static client ID and fails, + /// requiring a fresh login. Persisting the registration alongside the token + /// row would remove that re-login and is the natural follow-up. + registered_clients: Arc>>, } impl McpOAuthService { @@ -80,6 +83,7 @@ impl McpOAuthService { token_repo, http_client, pending: Arc::new(Mutex::new(HashMap::new())), + registered_clients: Arc::new(Mutex::new(HashMap::new())), } } @@ -206,10 +210,10 @@ impl McpOAuthService { let auth_url_str = metadata.authorization_endpoint.clone(); let token_url_str = metadata.token_endpoint.clone(); - let auth_url = AuthUrl::new(metadata.authorization_endpoint) + let auth_url = AuthUrl::new(metadata.authorization_endpoint.clone()) .map_err(|e| McpError::OAuth(format!("Invalid auth URL: {e}")))?; - let token_url = - TokenUrl::new(metadata.token_endpoint).map_err(|e| McpError::OAuth(format!("Invalid token URL: {e}")))?; + let token_url = TokenUrl::new(metadata.token_endpoint.clone()) + .map_err(|e| McpError::OAuth(format!("Invalid token URL: {e}")))?; let listener = TcpListener::bind("127.0.0.1:0") .await @@ -223,10 +227,17 @@ impl McpOAuthService { let redirect = RedirectUrl::new(redirect_url_str.clone()) .map_err(|e| McpError::OAuth(format!("Invalid redirect URL: {e}")))?; - let client = BasicClient::new(ClientId::new(DEFAULT_CLIENT_ID.to_string())) + let credentials = self + .resolve_client_credentials(server_url, &metadata, &redirect_url_str) + .await?; + + let mut client = BasicClient::new(ClientId::new(credentials.client_id.clone())) .set_auth_uri(auth_url) .set_token_uri(token_url) .set_redirect_uri(redirect); + if let Some(ref secret) = credentials.client_secret { + client = client.set_client_secret(ClientSecret::new(secret.clone())); + } let (pkce_challenge, pkce_verifier) = PkceCodeChallenge::new_random_sha256(); @@ -246,6 +257,7 @@ impl McpOAuthService { auth_url: auth_url_str, token_url: token_url_str, redirect_url: redirect_url_str, + credentials, }, ); } @@ -269,48 +281,66 @@ impl McpOAuthService { Ok(true) } - /// Discover OAuth authorization server metadata. + /// Discover OAuth authorization server metadata for an MCP server URL. /// - /// Tries `.well-known/oauth-authorization-server` first, - /// falls back to `.well-known/openid-configuration`. + /// See [`crate::oauth_discovery`] for the probe order; the important part + /// is that `.well-known` paths are resolved against the server's *origin* + /// with the resource path as a suffix (RFC 8414), not appended to the full + /// URL — appending is rejected by many public MCP servers. async fn discover_endpoints(&self, server_url: &str) -> Result { - let base = server_url.trim_end_matches('/'); + OAuthDiscovery::new(&self.http_client).discover(server_url).await + } - let well_known_url = format!("{base}/.well-known/oauth-authorization-server"); - if let Ok(metadata) = self.fetch_metadata(&well_known_url).await { - debug!(server_url, "Discovered OAuth metadata via RFC 8414"); - return Ok(metadata); + /// Resolve the OAuth client to use for a server. + /// + /// Prefers a previously registered client, then RFC 7591 dynamic client + /// registration when the server advertises a registration endpoint, and + /// finally the static fallback client ID for servers that pre-provision it. + async fn resolve_client_credentials( + &self, + server_url: &str, + metadata: &OAuthServerMetadata, + redirect_uri: &str, + ) -> Result { + if let Some(existing) = self.registered_clients.lock().await.get(server_url) { + return Ok(existing.clone()); } - let oidc_url = format!("{base}/.well-known/openid-configuration"); - if let Ok(metadata) = self.fetch_metadata(&oidc_url).await { - debug!(server_url, "Discovered OAuth metadata via OIDC"); - return Ok(metadata); - } + let Some(registration_endpoint) = metadata.registration_endpoint.as_deref() else { + debug!( + server_url, + "No registration endpoint advertised; using static client ID" + ); + return Ok(ClientCredentials { + client_id: DEFAULT_CLIENT_ID.to_string(), + client_secret: None, + }); + }; - Err(McpError::OAuth(format!( - "Failed to discover OAuth endpoints for '{server_url}': \ - no .well-known/oauth-authorization-server or \ - .well-known/openid-configuration found" - ))) - } + let credentials = OAuthDiscovery::new(&self.http_client) + .register_client(registration_endpoint, redirect_uri) + .await?; + debug!(server_url, "Registered OAuth client via dynamic client registration"); - /// Fetch and parse OAuth server metadata from a URL. - async fn fetch_metadata(&self, url: &str) -> Result { - let resp = self - .http_client - .get(url) - .send() + self.registered_clients + .lock() .await - .map_err(|e| McpError::OAuth(format!("HTTP request failed: {e}")))?; + .insert(server_url.to_string(), credentials.clone()); - if !resp.status().is_success() { - return Err(McpError::OAuth(format!("Metadata endpoint returned {}", resp.status()))); - } + Ok(credentials) + } - resp.json() + /// Return the registered client for a server, or the static fallback. + async fn cached_client_credentials(&self, server_url: &str) -> ClientCredentials { + self.registered_clients + .lock() .await - .map_err(|e| McpError::OAuth(format!("Failed to parse metadata: {e}"))) + .get(server_url) + .cloned() + .unwrap_or_else(|| ClientCredentials { + client_id: DEFAULT_CLIENT_ID.to_string(), + client_secret: None, + }) } /// Wait for the OAuth callback redirect on the given listener. @@ -392,7 +422,7 @@ impl McpOAuthService { code: String, state: String, ) -> Result<(), McpError> { - let (auth_url_str, token_url_str, redirect_url_str, pkce_verifier) = { + let (auth_url_str, token_url_str, redirect_url_str, pkce_verifier, credentials) = { let mut guard = self.pending.lock().await; let pending = guard .remove(&(user_id.to_string(), state)) @@ -402,6 +432,7 @@ impl McpOAuthService { pending.token_url, pending.redirect_url, pending.pkce_verifier, + pending.credentials, ) }; @@ -410,10 +441,13 @@ impl McpOAuthService { let redirect = RedirectUrl::new(redirect_url_str).map_err(|e| McpError::OAuth(format!("Invalid redirect URL: {e}")))?; - let client = BasicClient::new(ClientId::new(DEFAULT_CLIENT_ID.to_string())) + let mut client = BasicClient::new(ClientId::new(credentials.client_id)) .set_auth_uri(auth_url) .set_token_uri(token_url) .set_redirect_uri(redirect); + if let Some(secret) = credentials.client_secret { + client = client.set_client_secret(ClientSecret::new(secret)); + } let http_client = Self::build_no_redirect_client()?; @@ -440,7 +474,11 @@ impl McpOAuthService { let token_url = TokenUrl::new(metadata.token_endpoint).map_err(|e| McpError::OAuth(format!("Invalid token URL: {e}")))?; - let client = BasicClient::new(ClientId::new(DEFAULT_CLIENT_ID.to_string())).set_token_uri(token_url); + let credentials = self.cached_client_credentials(server_url).await; + let mut client = BasicClient::new(ClientId::new(credentials.client_id)).set_token_uri(token_url); + if let Some(secret) = credentials.client_secret { + client = client.set_client_secret(ClientSecret::new(secret)); + } let http_client = Self::build_no_redirect_client()?; @@ -706,6 +744,10 @@ mod tests { auth_url: "https://auth.example.com/authorize".to_string(), token_url: "https://auth.example.com/token".to_string(), redirect_url: "http://127.0.0.1/callback".to_string(), + credentials: ClientCredentials { + client_id: DEFAULT_CLIENT_ID.to_string(), + client_secret: None, + }, }, ); } diff --git a/crates/aionui-mcp/src/routes.rs b/crates/aionui-mcp/src/routes.rs index 671818ab1..2f8a051ce 100644 --- a/crates/aionui-mcp/src/routes.rs +++ b/crates/aionui-mcp/src/routes.rs @@ -33,6 +33,13 @@ impl From for ApiError { McpError::AgentOperationFailed(msg) => ApiError::Internal(msg), McpError::ConnectionFailed(msg) => ApiError::BadGateway(msg), McpError::OAuth(msg) => ApiError::Internal(format!("OAuth error: {msg}")), + // Discovery failures are caused by the remote server or the + // configured URL, not by an internal fault. Use a coded error so + // the actionable reason reaches the user instead of the generic + // "Internal server error." / "Upstream service unavailable." + McpError::OAuthDiscovery(msg) => { + ApiError::coded(StatusCode::BAD_GATEWAY, "MCP_OAUTH_DISCOVERY_FAILED", msg, None) + } McpError::Database(db_err) => ApiError::Internal(db_err.to_string()), McpError::Json(e) => ApiError::Internal(format!("JSON error: {e}")), } @@ -370,6 +377,16 @@ mod error_mapping_tests { assert!(matches!(err, ApiError::BadRequest(_))); } + /// Discovery failures must not be laundered into an opaque 500 — the user + /// needs to see which server/URL failed to publish OAuth metadata. + #[test] + fn oauth_discovery_maps_to_bad_gateway_and_keeps_message() { + let err = ApiError::from(McpError::OAuthDiscovery("no metadata at example.com".into())); + assert_eq!(err.status_code(), StatusCode::BAD_GATEWAY); + assert_eq!(err.error_code(), "MCP_OAUTH_DISCOVERY_FAILED"); + assert!(err.public_message().contains("no metadata at example.com")); + } + #[test] fn agent_operation_failed_maps_to_internal() { let err = ApiError::from(McpError::AgentOperationFailed("exit code 1".into())); diff --git a/crates/aionui-mcp/tests/oauth_discovery_live.rs b/crates/aionui-mcp/tests/oauth_discovery_live.rs new file mode 100644 index 000000000..316e760cd --- /dev/null +++ b/crates/aionui-mcp/tests/oauth_discovery_live.rs @@ -0,0 +1,35 @@ +//! Live discovery checks against real public MCP servers. +//! +//! Ignored by default (network-dependent). Run with: +//! `cargo test -p aionui-mcp --test oauth_discovery_live -- --ignored` +//! +//! These guard the exact regression from the bug report: the old code appended +//! `.well-known/...` to the full URL path, which these servers answer with 401. + +use aionui_mcp::oauth_discovery::OAuthDiscovery; + +#[tokio::test] +#[ignore = "requires network access to mcp.monday.com"] +async fn discovers_monday_sse_endpoints() { + let client = reqwest::Client::new(); + let metadata = OAuthDiscovery::new(&client) + .discover("https://mcp.monday.com/sse") + .await + .expect("discovery must succeed for monday.com MCP server"); + + assert!( + metadata.authorization_endpoint.starts_with("https://"), + "unexpected authorization_endpoint: {}", + metadata.authorization_endpoint + ); + assert!( + metadata.token_endpoint.starts_with("https://"), + "unexpected token_endpoint: {}", + metadata.token_endpoint + ); + // monday requires dynamic client registration. + assert!( + metadata.registration_endpoint.is_some(), + "expected a registration endpoint to be advertised" + ); +}