From 18b3cfcb60746d54e5d90bb105d363d7ace1d254 Mon Sep 17 00:00:00 2001 From: Steven Lee Date: Tue, 16 Jun 2026 20:18:32 +0000 Subject: [PATCH] Add MCP OAuth callback URL command --- codex-rs/cli/src/mcp_cmd.rs | 158 ++++++++++- codex-rs/cli/src/mcp_cmd_tests.rs | 15 ++ codex-rs/cli/tests/mcp_add_remove.rs | 52 ++++ codex-rs/cli/tests/mcp_callback_url.rs | 65 +++++ codex-rs/rmcp-client/src/lib.rs | 2 + .../rmcp-client/src/perform_oauth_login.rs | 246 ++++++++++++++++-- 6 files changed, 508 insertions(+), 30 deletions(-) create mode 100644 codex-rs/cli/src/mcp_cmd_tests.rs create mode 100644 codex-rs/cli/tests/mcp_callback_url.rs diff --git a/codex-rs/cli/src/mcp_cmd.rs b/codex-rs/cli/src/mcp_cmd.rs index 1466f10394..b01b6d6bcf 100644 --- a/codex-rs/cli/src/mcp_cmd.rs +++ b/codex-rs/cli/src/mcp_cmd.rs @@ -28,6 +28,8 @@ use codex_mcp::should_retry_without_scopes; use codex_protocol::protocol::McpAuthStatus; use codex_rmcp_client::delete_oauth_tokens; use codex_rmcp_client::perform_oauth_login; +use codex_rmcp_client::perform_oauth_login_print_callback_url; +use codex_rmcp_client::resolve_mcp_oauth_callback_url; use codex_utils_cli::CliConfigOverrides; use codex_utils_cli::format_env_display; @@ -38,6 +40,7 @@ use codex_utils_cli::format_env_display; /// - `remove` — delete a server entry /// - `login` — authenticate with MCP server using OAuth /// - `logout` — remove OAuth credentials for MCP server +/// - `callback-url` — print the OAuth callback URL for MCP server setup #[derive(Debug, clap::Parser)] pub struct McpCli { #[clap(flatten)] @@ -55,6 +58,7 @@ pub enum McpSubcommand { Remove(RemoveArgs), Login(LoginArgs), Logout(LogoutArgs), + CallbackUrl(CallbackUrlArgs), } #[derive(Debug, clap::Parser)] @@ -167,6 +171,12 @@ pub struct LogoutArgs { pub name: String, } +#[derive(Debug, clap::Parser)] +pub struct CallbackUrlArgs { + /// Name of the MCP server whose OAuth callback URL should be printed. + pub name: String, +} + impl McpCli { pub async fn run(self, loader_overrides: LoaderOverrides) -> Result<()> { let McpCli { @@ -197,12 +207,21 @@ impl McpCli { McpSubcommand::Logout(args) => { run_logout(&config_overrides, args).await?; } + McpSubcommand::CallbackUrl(args) => { + run_callback_url(&config_overrides, args).await?; + } } Ok(()) } } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum OAuthCallbackUrlOutput { + Print, + Suppress, +} + /// Preserve compatibility with servers that still expect the legacy empty-scope /// OAuth request. If a discovered-scope request is rejected by the provider, /// retry the login flow once without scopes. @@ -219,8 +238,9 @@ async fn perform_oauth_login_retry_without_scopes( oauth_resource: Option<&str>, callback_port: Option, callback_url: Option<&str>, + callback_url_output: OAuthCallbackUrlOutput, ) -> Result<()> { - match perform_oauth_login( + match run_oauth_login( name, url, store_mode, @@ -232,13 +252,14 @@ async fn perform_oauth_login_retry_without_scopes( oauth_resource, callback_port, callback_url, + callback_url_output, ) .await { Ok(()) => Ok(()), Err(err) if should_retry_without_scopes(resolved_scopes, &err) => { println!("OAuth provider rejected discovered scopes. Retrying without scopes…"); - perform_oauth_login( + run_oauth_login( name, url, store_mode, @@ -250,6 +271,7 @@ async fn perform_oauth_login_retry_without_scopes( oauth_resource, callback_port, callback_url, + callback_url_output, ) .await } @@ -257,6 +279,57 @@ async fn perform_oauth_login_retry_without_scopes( } } +#[allow(clippy::too_many_arguments)] +async fn run_oauth_login( + name: &str, + url: &str, + store_mode: codex_config::types::OAuthCredentialsStoreMode, + keyring_backend_kind: codex_config::types::AuthKeyringBackendKind, + http_headers: Option>, + env_http_headers: Option>, + scopes: &[String], + oauth_client_id: Option<&str>, + oauth_resource: Option<&str>, + callback_port: Option, + callback_url: Option<&str>, + callback_url_output: OAuthCallbackUrlOutput, +) -> Result<()> { + match callback_url_output { + OAuthCallbackUrlOutput::Print => { + perform_oauth_login_print_callback_url( + name, + url, + store_mode, + keyring_backend_kind, + http_headers, + env_http_headers, + scopes, + oauth_client_id, + oauth_resource, + callback_port, + callback_url, + ) + .await + } + OAuthCallbackUrlOutput::Suppress => { + perform_oauth_login( + name, + url, + store_mode, + keyring_backend_kind, + http_headers, + env_http_headers, + scopes, + oauth_client_id, + oauth_resource, + callback_port, + callback_url, + ) + .await + } + } +} + async fn validate_profile_v2_migration( config_overrides: &CliConfigOverrides, loader_overrides: LoaderOverrides, @@ -365,6 +438,28 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re tools: HashMap::new(), }; + let callback_url = match &transport { + McpServerTransportConfig::StreamableHttp { url, .. } => { + match resolve_mcp_oauth_callback_url( + url, + config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), + ) { + Ok(callback_url) => Some(Ok(callback_url)), + Err(err) + if config.mcp_oauth_callback_port.is_none() + && config.mcp_oauth_callback_url.is_none() => + { + Some(Err(err)) + } + Err(err) => { + return Err(err).context("failed to resolve MCP OAuth callback URL"); + } + } + } + _ => None, + }; + servers.insert(name.clone(), new_entry); ConfigEditsBuilder::new(&codex_home) @@ -374,7 +469,6 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re .with_context(|| format!("failed to write MCP servers to {}", codex_home.display()))?; println!("Added global MCP server '{name}'."); - match oauth_login_support(&transport).await { McpOAuthLoginSupport::Supported(oauth_config) => { println!("Detected OAuth support. Starting OAuth flow…"); @@ -395,19 +489,29 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), + OAuthCallbackUrlOutput::Print, ) .await?; println!("Successfully logged in."); } - McpOAuthLoginSupport::Unsupported => {} - McpOAuthLoginSupport::Unknown(_) => println!( - "MCP server may or may not require login. Run `codex mcp login {name}` to login." - ), + McpOAuthLoginSupport::Unsupported => print_precomputed_oauth_callback_url(&callback_url), + McpOAuthLoginSupport::Unknown(_) => { + print_precomputed_oauth_callback_url(&callback_url); + println!( + "MCP server may or may not require login. Run `codex mcp login {name}` to login." + ); + } } Ok(()) } +fn print_precomputed_oauth_callback_url(callback_url: &Option>) { + if let Some(Ok(callback_url)) = callback_url { + println!("OAuth callback URL: {callback_url}"); + } +} + async fn run_remove(config_overrides: &CliConfigOverrides, remove_args: RemoveArgs) -> Result<()> { config_overrides .parse_overrides() @@ -490,12 +594,48 @@ async fn run_login(config_overrides: &CliConfigOverrides, login_args: LoginArgs) server.oauth_resource.as_deref(), config.mcp_oauth_callback_port, config.mcp_oauth_callback_url.as_deref(), + OAuthCallbackUrlOutput::Suppress, ) .await?; println!("Successfully logged in to MCP server '{name}'."); Ok(()) } +async fn run_callback_url( + config_overrides: &CliConfigOverrides, + callback_url_args: CallbackUrlArgs, +) -> Result<()> { + let overrides = config_overrides + .parse_overrides() + .map_err(anyhow::Error::msg)?; + let config = Config::load_with_cli_overrides(overrides) + .await + .context("failed to load configuration")?; + let mcp_manager = McpManager::new(Arc::new(PluginsManager::new( + config.codex_home.to_path_buf(), + ))); + let mcp_servers = mcp_manager.configured_servers(&config).await; + + let CallbackUrlArgs { name } = callback_url_args; + + let Some(server) = mcp_servers.get(&name) else { + bail!("No MCP server named '{name}' found."); + }; + + let url = match &server.transport { + McpServerTransportConfig::StreamableHttp { url, .. } => url, + _ => bail!("OAuth callback URL is only available for streamable HTTP servers."), + }; + + let callback_url = resolve_mcp_oauth_callback_url( + url, + config.mcp_oauth_callback_port, + config.mcp_oauth_callback_url.as_deref(), + )?; + println!("{callback_url}"); + Ok(()) +} + async fn run_logout(config_overrides: &CliConfigOverrides, logout_args: LogoutArgs) -> Result<()> { let overrides = config_overrides .parse_overrides() @@ -998,3 +1138,7 @@ fn format_mcp_status(config: &McpServerConfig) -> String { "disabled".to_string() } } + +#[cfg(test)] +#[path = "mcp_cmd_tests.rs"] +mod tests; diff --git a/codex-rs/cli/src/mcp_cmd_tests.rs b/codex-rs/cli/src/mcp_cmd_tests.rs new file mode 100644 index 0000000000..b00018008c --- /dev/null +++ b/codex-rs/cli/src/mcp_cmd_tests.rs @@ -0,0 +1,15 @@ +use clap::Parser; + +use super::McpCli; +use super::McpSubcommand; + +#[test] +fn parses_callback_url_subcommand() { + let cli = McpCli::try_parse_from(["codex", "callback-url", "outlookmcp"]) + .expect("callback-url command should parse"); + + assert!(matches!( + cli.subcommand, + McpSubcommand::CallbackUrl(args) if args.name == "outlookmcp" + )); +} diff --git a/codex-rs/cli/tests/mcp_add_remove.rs b/codex-rs/cli/tests/mcp_add_remove.rs index d0fc5f327d..0ddcf416dc 100644 --- a/codex-rs/cli/tests/mcp_add_remove.rs +++ b/codex-rs/cli/tests/mcp_add_remove.rs @@ -3,6 +3,8 @@ use std::path::Path; use anyhow::Result; use codex_config::types::McpServerTransportConfig; use codex_core::config::load_global_mcp_servers; +use codex_rmcp_client::resolve_mcp_oauth_callback_url; +use predicates::prelude::PredicateBooleanExt; use predicates::str::contains; use pretty_assertions::assert_eq; use tempfile::TempDir; @@ -234,6 +236,56 @@ async fn add_streamable_http_with_oauth_options() -> Result<()> { Ok(()) } +#[tokio::test] +async fn add_streamable_http_prints_configured_oauth_callback_url() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"mcp_oauth_callback_url = "https://callbacks.example.com/oauth/callback" +"#, + )?; + let expected = resolve_mcp_oauth_callback_url( + "https://example.com/mcp", + /*callback_port*/ None, + Some("https://callbacks.example.com/oauth/callback"), + )?; + + let mut add_cmd = codex_command(codex_home.path())?; + add_cmd + .args([ + "mcp", + "add", + "oauth-server", + "--url", + "https://example.com/mcp", + ]) + .assert() + .success() + .stdout(contains(format!("OAuth callback URL: {expected}"))); + + Ok(()) +} + +#[tokio::test] +async fn add_streamable_http_does_not_print_ephemeral_callback_port_hint() -> Result<()> { + let codex_home = TempDir::new()?; + + let mut add_cmd = codex_command(codex_home.path())?; + add_cmd + .args([ + "mcp", + "add", + "oauth-server", + "--url", + "https://example.com/mcp", + ]) + .assert() + .success() + .stdout(contains("OAuth callback URL: unavailable").not()); + + Ok(()) +} + #[tokio::test] async fn add_streamable_http_rejects_removed_flag() -> Result<()> { let codex_home = TempDir::new()?; diff --git a/codex-rs/cli/tests/mcp_callback_url.rs b/codex-rs/cli/tests/mcp_callback_url.rs new file mode 100644 index 0000000000..41d529dea2 --- /dev/null +++ b/codex-rs/cli/tests/mcp_callback_url.rs @@ -0,0 +1,65 @@ +use std::path::Path; + +use anyhow::Result; +use codex_rmcp_client::resolve_mcp_oauth_callback_url; +use pretty_assertions::assert_eq; +use tempfile::TempDir; + +fn codex_command(codex_home: &Path) -> Result { + let mut cmd = assert_cmd::Command::new(codex_utils_cargo_bin::cargo_bin("codex")?); + cmd.env("CODEX_HOME", codex_home); + Ok(cmd) +} + +#[test] +fn callback_url_prints_configured_final_url() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"mcp_oauth_callback_url = "https://callbacks.example.com/oauth/callback" + +[mcp_servers.github] +url = "https://example.com/mcp" +"#, + )?; + + let expected = resolve_mcp_oauth_callback_url( + "https://example.com/mcp", + /*callback_port*/ None, + Some("https://callbacks.example.com/oauth/callback"), + )?; + + let mut cmd = codex_command(codex_home.path())?; + let output = cmd.args(["mcp", "callback-url", "github"]).output()?; + + assert!(output.status.success()); + assert_eq!(String::from_utf8(output.stdout)?, format!("{expected}\n")); + assert_eq!(String::from_utf8(output.stderr)?, ""); + assert!(!codex_home.path().join(".credentials.json").exists()); + + Ok(()) +} + +#[test] +fn callback_url_errors_when_default_port_is_ephemeral() -> Result<()> { + let codex_home = TempDir::new()?; + std::fs::write( + codex_home.path().join("config.toml"), + r#"[mcp_servers.github] +url = "https://example.com/mcp" +"#, + )?; + + let mut cmd = codex_command(codex_home.path())?; + let output = cmd.args(["mcp", "callback-url", "github"]).output()?; + + assert!(!output.status.success()); + assert_eq!(String::from_utf8(output.stdout)?, ""); + assert!( + String::from_utf8(output.stderr)? + .contains("set `mcp_oauth_callback_port` or `mcp_oauth_callback_url`") + ); + assert!(!codex_home.path().join(".credentials.json").exists()); + + Ok(()) +} diff --git a/codex-rs/rmcp-client/src/lib.rs b/codex-rs/rmcp-client/src/lib.rs index e1ee18c753..1680bb577a 100644 --- a/codex-rs/rmcp-client/src/lib.rs +++ b/codex-rs/rmcp-client/src/lib.rs @@ -25,8 +25,10 @@ pub use oauth::save_oauth_tokens; pub use perform_oauth_login::OAuthProviderError; pub use perform_oauth_login::OauthLoginHandle; pub use perform_oauth_login::perform_oauth_login; +pub use perform_oauth_login::perform_oauth_login_print_callback_url; pub use perform_oauth_login::perform_oauth_login_return_url; pub use perform_oauth_login::perform_oauth_login_silent; +pub use perform_oauth_login::resolve_mcp_oauth_callback_url; pub use rmcp::model::ElicitationAction; pub use rmcp_client::Elicitation; pub use rmcp_client::ElicitationResponse; diff --git a/codex-rs/rmcp-client/src/perform_oauth_login.rs b/codex-rs/rmcp-client/src/perform_oauth_login.rs index 8c72f320bc..310ea15bd2 100644 --- a/codex-rs/rmcp-client/src/perform_oauth_login.rs +++ b/codex-rs/rmcp-client/src/perform_oauth_login.rs @@ -104,6 +104,7 @@ pub async fn perform_oauth_login( callback_port, callback_url, /*emit_browser_url*/ true, + CallbackUrlOutput::Suppress, ) .await } @@ -135,6 +136,39 @@ pub async fn perform_oauth_login_silent( callback_port, callback_url, /*emit_browser_url*/ false, + CallbackUrlOutput::Suppress, + ) + .await +} + +#[allow(clippy::too_many_arguments)] +pub async fn perform_oauth_login_print_callback_url( + server_name: &str, + server_url: &str, + store_mode: OAuthCredentialsStoreMode, + keyring_backend_kind: AuthKeyringBackendKind, + http_headers: Option>, + env_http_headers: Option>, + scopes: &[String], + oauth_client_id: Option<&str>, + oauth_resource: Option<&str>, + callback_port: Option, + callback_url: Option<&str>, +) -> Result<()> { + perform_oauth_login_with_browser_output( + server_name, + server_url, + store_mode, + keyring_backend_kind, + http_headers, + env_http_headers, + scopes, + oauth_client_id, + oauth_resource, + callback_port, + callback_url, + /*emit_browser_url*/ true, + CallbackUrlOutput::Print, ) .await } @@ -153,6 +187,7 @@ async fn perform_oauth_login_with_browser_output( callback_port: Option, callback_url: Option<&str>, emit_browser_url: bool, + callback_url_output: CallbackUrlOutput, ) -> Result<()> { let headers = OauthHeaders { http_headers, @@ -173,7 +208,7 @@ async fn perform_oauth_login_with_browser_output( /*timeout_secs*/ None, ) .await? - .finish(emit_browser_url) + .finish(emit_browser_url, callback_url_output) .await } @@ -275,6 +310,12 @@ enum CallbackResult { Error(OAuthProviderError), } +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum CallbackUrlOutput { + Print, + Suppress, +} + #[derive(Debug, PartialEq, Eq)] enum CallbackOutcome { Success(OauthCallbackResult), @@ -353,6 +394,7 @@ impl OauthLoginHandle { struct OauthLoginFlow { auth_url: String, + redirect_uri: String, oauth_state: OAuthState, rx: oneshot::Receiver, guard: CallbackServerGuard, @@ -394,15 +436,51 @@ fn local_redirect_uri(server: &Server) -> Result { } } -fn resolve_redirect_uri(server: &Server, callback_url: Option<&str>) -> Result { - let Some(callback_url) = callback_url else { - return local_redirect_uri(server); - }; +fn configured_redirect_uri(callback_url: &str) -> Result { Url::parse(callback_url) .with_context(|| format!("invalid MCP OAuth callback URL `{callback_url}`"))?; Ok(callback_url.to_string()) } +fn local_redirect_uri_from_port(callback_port: Option) -> Result { + let Some(port) = callback_port else { + bail!( + "cannot determine MCP OAuth callback URL with an ephemeral callback port; set `mcp_oauth_callback_port` or `mcp_oauth_callback_url`" + ); + }; + Ok(format!("http://127.0.0.1:{port}/callback")) +} + +/// Resolve the final OAuth redirect URI Codex will use for an MCP server. +/// +/// The returned URI includes the deterministic callback ID suffix derived from +/// the normalized MCP server URL. This helper is read-only and does not start an +/// OAuth flow, bind a callback listener, or perform network I/O. +pub fn resolve_mcp_oauth_callback_url( + server_url: &str, + callback_port: Option, + callback_url: Option<&str>, +) -> Result { + let callback_port = resolve_callback_port(callback_port)?; + let redirect_uri = match callback_url { + Some(callback_url) => configured_redirect_uri(callback_url)?, + None => local_redirect_uri_from_port(callback_port)?, + }; + append_callback_id_to_redirect_uri(server_url, &redirect_uri) +} + +fn resolve_redirect_uri( + server: &Server, + server_url: &str, + callback_url: Option<&str>, +) -> Result { + let redirect_uri = match callback_url { + Some(callback_url) => configured_redirect_uri(callback_url)?, + None => local_redirect_uri(server)?, + }; + append_callback_id_to_redirect_uri(server_url, &redirect_uri) +} + fn callback_id_from_server_url(server_url: &str) -> Result { let mut parsed = Url::parse(server_url).with_context(|| format!("invalid MCP server URL `{server_url}`"))?; @@ -415,7 +493,8 @@ fn callback_id_from_server_url(server_url: &str) -> Result { Ok(URL_SAFE_NO_PAD.encode(&digest[..9])) } -fn append_callback_id_to_redirect_uri(redirect_uri: &str, callback_id: &str) -> Result { +fn append_callback_id_to_redirect_uri(server_url: &str, redirect_uri: &str) -> Result { + let callback_id = callback_id_from_server_url(server_url)?; let mut parsed = Url::parse(redirect_uri) .with_context(|| format!("invalid redirect URI `{redirect_uri}`"))?; let path = parsed.path(); @@ -479,9 +558,7 @@ impl OauthLoginFlow { server: Arc::clone(&server), }; - let redirect_uri = resolve_redirect_uri(&server, callback_url)?; - let callback_id = callback_id_from_server_url(server_url)?; - let redirect_uri = append_callback_id_to_redirect_uri(&redirect_uri, &callback_id)?; + let redirect_uri = resolve_redirect_uri(&server, server_url, callback_url)?; let callback_path = callback_path_from_redirect_uri(&redirect_uri)?; let (tx, rx) = oneshot::channel(); @@ -513,6 +590,7 @@ impl OauthLoginFlow { Ok(Self { auth_url, + redirect_uri, oauth_state, rx, guard, @@ -529,10 +607,18 @@ impl OauthLoginFlow { self.auth_url.clone() } - async fn finish(mut self, emit_browser_url: bool) -> Result<()> { + async fn finish( + mut self, + emit_browser_url: bool, + callback_url_output: CallbackUrlOutput, + ) -> Result<()> { if self.launch_browser { let server_name = &self.server_name; let auth_url = &self.auth_url; + if callback_url_output == CallbackUrlOutput::Print { + let redirect_uri = &self.redirect_uri; + println!("OAuth callback URL: {redirect_uri}"); + } if emit_browser_url { println!( "Authorize `{server_name}` by opening this URL in your browser:\n{auth_url}\n" @@ -603,7 +689,9 @@ impl OauthLoginFlow { let (tx, rx) = oneshot::channel(); tokio::spawn(async move { - let result = self.finish(/*emit_browser_url*/ false).await; + let result = self + .finish(/*emit_browser_url*/ false, CallbackUrlOutput::Suppress) + .await; if let Err(err) = &result { eprintln!( @@ -671,6 +759,8 @@ mod tests { use axum::Json; use axum::Router; use axum::routing::get; + use codex_config::types::AuthKeyringBackendKind; + use codex_config::types::OAuthCredentialsStoreMode; use pretty_assertions::assert_eq; use reqwest::Url; use serde_json::json; @@ -678,11 +768,14 @@ mod tests { use super::CallbackOutcome; use super::OAuthProviderError; + use super::OauthHeaders; + use super::OauthLoginFlow; use super::append_callback_id_to_redirect_uri; use super::append_query_param; use super::callback_id_from_server_url; use super::callback_path_from_redirect_uri; use super::parse_oauth_callback; + use super::resolve_mcp_oauth_callback_url; use super::start_authorization; async fn spawn_oauth_metadata_server() -> String { @@ -748,6 +841,44 @@ mod tests { assert_eq!(client_id.as_deref(), Some("eci-prd-pub-codex-123")); } + #[tokio::test] + async fn oauth_flow_redirect_uri_matches_authorization_url() { + let base_url = spawn_oauth_metadata_server().await; + let server_url = format!("{base_url}/mcp"); + + let flow = OauthLoginFlow::new( + "test-server", + &server_url, + OAuthCredentialsStoreMode::File, + AuthKeyringBackendKind::default(), + OauthHeaders { + http_headers: None, + env_http_headers: None, + }, + &[], + Some("eci-prd-pub-codex-123"), + /*oauth_resource*/ None, + /*launch_browser*/ false, + /*callback_port*/ None, + /*callback_url*/ None, + Some(1), + ) + .await + .expect("start oauth flow"); + + let authorization_url = flow.authorization_url(); + let auth_url = Url::parse(&authorization_url).expect("authorization url should parse"); + let redirect_uri = auth_url + .query_pairs() + .find(|(key, _)| key == "redirect_uri") + .map(|(_, value)| value.into_owned()) + .expect("authorization url should include redirect_uri"); + + assert_eq!(redirect_uri, flow.redirect_uri); + assert!(redirect_uri.starts_with("http://127.0.0.1:")); + assert!(redirect_uri.contains("/callback/")); + } + #[test] fn parse_oauth_callback_accepts_default_path() { let parsed = parse_oauth_callback("/callback?code=abc&state=xyz", "/callback"); @@ -802,6 +933,66 @@ mod tests { assert_eq!(path, "/oauth/callback"); } + #[test] + fn precomputed_configured_callback_url_includes_callback_id() { + let server_url = "https://mcp.example.com/mcp"; + let callback_id = callback_id_from_server_url(server_url).expect("derive callback ID"); + + let redirect_uri = resolve_mcp_oauth_callback_url( + server_url, + /*callback_port*/ None, + Some("https://callbacks.example.com/oauth/callback"), + ) + .expect("resolve redirect URI"); + + assert_eq!( + redirect_uri, + format!("https://callbacks.example.com/oauth/callback/{callback_id}") + ); + } + + #[test] + fn precomputed_local_callback_url_includes_callback_id() { + let server_url = "https://mcp.example.com/mcp"; + let callback_id = callback_id_from_server_url(server_url).expect("derive callback ID"); + + let redirect_uri = + resolve_mcp_oauth_callback_url(server_url, Some(4321), /*callback_url*/ None) + .expect("resolve redirect URI"); + + assert_eq!( + redirect_uri, + format!("http://127.0.0.1:4321/callback/{callback_id}") + ); + } + + #[test] + fn precomputed_local_callback_url_rejects_ephemeral_port() { + let err = resolve_mcp_oauth_callback_url( + "https://mcp.example.com/mcp", + /*callback_port*/ None, + /*callback_url*/ None, + ) + .expect_err("ephemeral callback port should not resolve"); + + assert!( + err.to_string() + .contains("set `mcp_oauth_callback_port` or `mcp_oauth_callback_url`") + ); + } + + #[test] + fn precomputed_callback_url_rejects_invalid_callback_port() { + let err = resolve_mcp_oauth_callback_url( + "https://mcp.example.com/mcp", + Some(0), + Some("https://callbacks.example.com/oauth/callback"), + ) + .expect_err("invalid callback port should not resolve"); + + assert!(err.to_string().contains("port must be between 1 and 65535")); + } + #[test] fn callback_id_is_bound_to_server_url() { let callback_id = callback_id_from_server_url("https://mcp.example.com/mcp?tenant=one") @@ -830,24 +1021,33 @@ mod tests { #[test] fn callback_id_is_appended_to_redirect_uri_path() { - let redirect_uri = - append_callback_id_to_redirect_uri("http://127.0.0.1:1234/callback", "abc123") - .expect("redirect URI should parse"); - - assert_eq!(redirect_uri, "http://127.0.0.1:1234/callback/abc123"); - } - - #[test] - fn callback_id_is_appended_before_redirect_uri_query() { + let callback_id = + callback_id_from_server_url("https://mcp.example.com/mcp").expect("derive callback ID"); let redirect_uri = append_callback_id_to_redirect_uri( - "https://callbacks.example.com/oauth/callback?provider=github", - "abc123", + "https://mcp.example.com/mcp", + "http://127.0.0.1:1234/callback", ) .expect("redirect URI should parse"); assert_eq!( redirect_uri, - "https://callbacks.example.com/oauth/callback/abc123?provider=github" + format!("http://127.0.0.1:1234/callback/{callback_id}") + ); + } + + #[test] + fn callback_id_is_appended_before_redirect_uri_query() { + let callback_id = + callback_id_from_server_url("https://mcp.example.com/mcp").expect("derive callback ID"); + let redirect_uri = append_callback_id_to_redirect_uri( + "https://mcp.example.com/mcp", + "https://callbacks.example.com/oauth/callback?provider=github", + ) + .expect("redirect URI should parse"); + + assert_eq!( + redirect_uri, + format!("https://callbacks.example.com/oauth/callback/{callback_id}?provider=github") ); }