mirror of
https://github.com/openai/codex.git
synced 2026-10-29 19:08:43 +00:00
Add manual callback input to MCP OAuth login (#44629)
## Why Allow MCP authentication to complete when the browser cannot reach the callback page by accepting the full redirect URL copied from its address bar. ## What changed - Add `codex mcp login <name> --no-browser` to print the authorization URL and accept a pasted callback without launching a browser. HTTP callbacks remain supported while waiting for input. - Validate pasted redirect URLs and OAuth responses before exchanging tokens, hide terminal input, bound input size, and avoid echoing callback values in diagnostics. - Preserve manual input across discovered-scope retries and support cancellation during callback input and token exchange. ## Testing Add unit and CLI integration tests for callback validation, bounded input, credential storage, HTTP callback completion with stdin open, scope retries, and cancellation during token exchange. GitOrigin-RevId: 16ea9c3221f72d93e6e3e5acdc6b9465293a0a65
This commit is contained in:
committed by
copyberry
parent
9b033b4642
commit
f8ab57359d
@@ -67,6 +67,7 @@ mod exec_server_auth;
|
||||
mod exec_server_telemetry;
|
||||
mod marketplace_cmd;
|
||||
mod mcp_cmd;
|
||||
mod mcp_login;
|
||||
mod migrate_rollouts;
|
||||
mod plugin_cmd;
|
||||
mod queue_cmd;
|
||||
|
||||
+13
-66
@@ -24,26 +24,25 @@ use codex_exec_server::RouteAwareHttpClient;
|
||||
use codex_login::AuthManager;
|
||||
use codex_mcp::McpOAuthLoginSupport;
|
||||
use codex_mcp::McpRuntimeContext;
|
||||
use codex_mcp::ResolvedMcpOAuthScopes;
|
||||
use codex_mcp::apply_http_headers_helper;
|
||||
use codex_mcp::compute_auth_statuses;
|
||||
use codex_mcp::discover_supported_scopes;
|
||||
use codex_mcp::oauth_login_support;
|
||||
use codex_mcp::resolve_oauth_callback;
|
||||
use codex_mcp::resolve_oauth_scopes;
|
||||
use codex_mcp::should_retry_without_scopes;
|
||||
use codex_protocol::protocol::McpAuthStatus;
|
||||
use codex_rmcp_client::McpOAuthCallbackMode;
|
||||
use codex_rmcp_client::McpOAuthClientRegistration;
|
||||
use codex_rmcp_client::OAuthDiscoveryTimeout;
|
||||
use codex_rmcp_client::StreamableHttpRedirectMode;
|
||||
use codex_rmcp_client::delete_oauth_tokens;
|
||||
use codex_rmcp_client::perform_oauth_login;
|
||||
use codex_rmcp_client::resolve_mcp_oauth_callback_url;
|
||||
use codex_utils_cli::CliConfigOverrides;
|
||||
use codex_utils_cli::format_env_display;
|
||||
|
||||
use crate::cloud_config;
|
||||
use crate::mcp_login::McpLoginMode;
|
||||
use crate::mcp_login::perform_oauth_login_retry_without_scopes;
|
||||
use crate::plugin_cmd::load_cli_auth_manager;
|
||||
|
||||
/// Subcommands:
|
||||
@@ -197,6 +196,10 @@ pub struct LoginArgs {
|
||||
/// Name of the MCP server to authenticate with oauth.
|
||||
pub name: String,
|
||||
|
||||
/// Print the authorization URL and accept the callback URL without opening a browser.
|
||||
#[arg(long)]
|
||||
pub no_browser: bool,
|
||||
|
||||
/// Comma-separated list of OAuth scopes to request.
|
||||
#[arg(long, value_delimiter = ',', value_name = "SCOPE,SCOPE")]
|
||||
pub scopes: Vec<String>,
|
||||
@@ -256,69 +259,6 @@ impl McpCli {
|
||||
}
|
||||
}
|
||||
|
||||
/// 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.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
async fn perform_oauth_login_retry_without_scopes(
|
||||
name: &str,
|
||||
url: &str,
|
||||
store_mode: codex_config::types::OAuthCredentialsStoreMode,
|
||||
keyring_backend_kind: codex_config::types::AuthKeyringBackendKind,
|
||||
http_headers: Option<HashMap<String, String>>,
|
||||
env_http_headers: Option<HashMap<String, String>>,
|
||||
resolved_scopes: &ResolvedMcpOAuthScopes,
|
||||
oauth_client_id: Option<&str>,
|
||||
client_registration: McpOAuthClientRegistration,
|
||||
oauth_resource: Option<&str>,
|
||||
callback_port: Option<u16>,
|
||||
callback_url: Option<&str>,
|
||||
global_callback_url: Option<&str>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
) -> Result<()> {
|
||||
match perform_oauth_login(
|
||||
name,
|
||||
url,
|
||||
store_mode,
|
||||
keyring_backend_kind,
|
||||
http_headers.clone(),
|
||||
env_http_headers.clone(),
|
||||
&resolved_scopes.scopes,
|
||||
oauth_client_id,
|
||||
client_registration,
|
||||
oauth_resource,
|
||||
callback_port,
|
||||
callback_url,
|
||||
global_callback_url,
|
||||
Arc::clone(&http_client),
|
||||
)
|
||||
.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(
|
||||
name,
|
||||
url,
|
||||
store_mode,
|
||||
keyring_backend_kind,
|
||||
http_headers,
|
||||
env_http_headers,
|
||||
&[],
|
||||
oauth_client_id,
|
||||
client_registration,
|
||||
oauth_resource,
|
||||
callback_port,
|
||||
callback_url,
|
||||
global_callback_url,
|
||||
http_client,
|
||||
)
|
||||
.await
|
||||
}
|
||||
Err(err) => Err(err),
|
||||
}
|
||||
}
|
||||
|
||||
async fn validate_profile_v2_migration(
|
||||
config_overrides: &CliConfigOverrides,
|
||||
loader_overrides: LoaderOverrides,
|
||||
@@ -512,6 +452,7 @@ async fn run_add(config_overrides: &CliConfigOverrides, add_args: AddArgs) -> Re
|
||||
.or(config.mcp_oauth_callback_url.as_deref()),
|
||||
config.mcp_oauth_callback_url.as_deref(),
|
||||
http_client,
|
||||
McpLoginMode::Browser,
|
||||
)
|
||||
.await?;
|
||||
println!("Successfully logged in.");
|
||||
@@ -572,6 +513,7 @@ async fn run_login(config: &Config, login_args: LoginArgs) -> Result<()> {
|
||||
|
||||
let LoginArgs {
|
||||
name,
|
||||
no_browser,
|
||||
scopes,
|
||||
oauth_client_registration,
|
||||
} = login_args;
|
||||
@@ -633,6 +575,11 @@ async fn run_login(config: &Config, login_args: LoginArgs) -> Result<()> {
|
||||
callback_url.as_deref(),
|
||||
config.mcp_oauth_callback_url.as_deref(),
|
||||
http_client,
|
||||
if no_browser {
|
||||
McpLoginMode::PasteCallback
|
||||
} else {
|
||||
McpLoginMode::Browser
|
||||
},
|
||||
)
|
||||
.await?;
|
||||
println!("Successfully logged in to MCP server '{name}'.");
|
||||
|
||||
@@ -0,0 +1,265 @@
|
||||
//! CLI MCP OAuth login modes and bounded, hidden callback input.
|
||||
//!
|
||||
//! Manual input remains cancellable while the OAuth flow waits for its HTTP
|
||||
//! callback, and dropping terminal input restores the terminal immediately.
|
||||
//! Ctrl-C also cancels token exchange and scope retries in manual login mode.
|
||||
|
||||
use std::collections::HashMap;
|
||||
use std::io;
|
||||
use std::io::BufRead;
|
||||
use std::io::IsTerminal;
|
||||
use std::io::Write;
|
||||
use std::sync::Arc;
|
||||
|
||||
use anyhow::Context;
|
||||
use anyhow::Result;
|
||||
use anyhow::bail;
|
||||
use codex_config::types::AuthKeyringBackendKind;
|
||||
use codex_config::types::OAuthCredentialsStoreMode;
|
||||
use codex_exec_server::HttpClient;
|
||||
use codex_mcp::ResolvedMcpOAuthScopes;
|
||||
use codex_mcp::should_retry_without_scopes;
|
||||
use codex_rmcp_client::McpOAuthClientRegistration;
|
||||
use codex_rmcp_client::perform_oauth_login;
|
||||
use codex_rmcp_client::perform_oauth_login_with_callback_input;
|
||||
use crossterm::event::Event;
|
||||
use crossterm::event::EventStream;
|
||||
use crossterm::event::KeyCode;
|
||||
use crossterm::event::KeyEventKind;
|
||||
use crossterm::event::KeyModifiers;
|
||||
use crossterm::terminal;
|
||||
use futures::StreamExt;
|
||||
use tokio::sync::oneshot;
|
||||
|
||||
const MAX_CALLBACK_BYTES: usize = 64 * 1024;
|
||||
|
||||
#[derive(Clone, Copy)]
|
||||
pub(crate) enum McpLoginMode {
|
||||
Browser,
|
||||
PasteCallback,
|
||||
}
|
||||
|
||||
/// Retry provider-rejected discovered scopes once, retaining the selected input mode.
|
||||
#[allow(clippy::too_many_arguments)]
|
||||
pub(crate) async fn perform_oauth_login_retry_without_scopes(
|
||||
name: &str,
|
||||
url: &str,
|
||||
store_mode: OAuthCredentialsStoreMode,
|
||||
keyring_backend_kind: AuthKeyringBackendKind,
|
||||
http_headers: Option<HashMap<String, String>>,
|
||||
env_http_headers: Option<HashMap<String, String>>,
|
||||
resolved_scopes: &ResolvedMcpOAuthScopes,
|
||||
oauth_client_id: Option<&str>,
|
||||
client_registration: McpOAuthClientRegistration,
|
||||
oauth_resource: Option<&str>,
|
||||
callback_port: Option<u16>,
|
||||
callback_url: Option<&str>,
|
||||
global_callback_url: Option<&str>,
|
||||
http_client: Arc<dyn HttpClient>,
|
||||
mode: McpLoginMode,
|
||||
) -> Result<()> {
|
||||
let mut pending_input = None;
|
||||
// Tokio retains its process-wide signal handler, so keep listening throughout
|
||||
// token exchange and scope retries after the callback input reader is dropped.
|
||||
let ctrl_c = tokio::signal::ctrl_c();
|
||||
tokio::pin!(ctrl_c);
|
||||
for (attempt, scopes) in [resolved_scopes.scopes.as_slice(), &[]]
|
||||
.into_iter()
|
||||
.enumerate()
|
||||
{
|
||||
let login = async {
|
||||
match mode {
|
||||
McpLoginMode::Browser => {
|
||||
perform_oauth_login(
|
||||
name,
|
||||
url,
|
||||
store_mode,
|
||||
keyring_backend_kind,
|
||||
http_headers.clone(),
|
||||
env_http_headers.clone(),
|
||||
scopes,
|
||||
oauth_client_id,
|
||||
client_registration,
|
||||
oauth_resource,
|
||||
callback_port,
|
||||
callback_url,
|
||||
global_callback_url,
|
||||
Arc::clone(&http_client),
|
||||
)
|
||||
.await
|
||||
}
|
||||
McpLoginMode::PasteCallback => {
|
||||
let input = &mut pending_input;
|
||||
perform_oauth_login_with_callback_input(
|
||||
name,
|
||||
url,
|
||||
store_mode,
|
||||
keyring_backend_kind,
|
||||
http_headers.clone(),
|
||||
env_http_headers.clone(),
|
||||
scopes,
|
||||
oauth_client_id,
|
||||
client_registration,
|
||||
oauth_resource,
|
||||
callback_port,
|
||||
callback_url,
|
||||
global_callback_url,
|
||||
Arc::clone(&http_client),
|
||||
move |authorization_url| read_callback(authorization_url, input),
|
||||
)
|
||||
.await
|
||||
}
|
||||
}
|
||||
};
|
||||
let result = tokio::select! {
|
||||
result = login => result,
|
||||
result = &mut ctrl_c, if matches!(mode, McpLoginMode::PasteCallback) => {
|
||||
result.context("failed to listen for Ctrl-C")?;
|
||||
bail!("OAuth login cancelled");
|
||||
}
|
||||
};
|
||||
match result {
|
||||
Err(error) if attempt == 0 && should_retry_without_scopes(resolved_scopes, &error) => {
|
||||
println!("OAuth provider rejected discovered scopes. Retrying without scopes…");
|
||||
}
|
||||
result => return result,
|
||||
}
|
||||
}
|
||||
unreachable!("the empty-scope attempt always returns")
|
||||
}
|
||||
|
||||
async fn read_callback(
|
||||
authorization_url: String,
|
||||
pending_input: &mut Option<oneshot::Receiver<Result<String>>>,
|
||||
) -> Result<String> {
|
||||
println!(
|
||||
"Authorize the MCP server by opening this URL in your browser:\n{authorization_url}\n"
|
||||
);
|
||||
println!(
|
||||
"After signing in, copy the full URL from your browser's address bar.\n\
|
||||
If the callback page cannot load, paste that URL here anyway."
|
||||
);
|
||||
if io::stdin().is_terminal() {
|
||||
return read_terminal_callback().await;
|
||||
}
|
||||
print_callback_prompt()?;
|
||||
|
||||
// A pipe may stay open after the HTTP callback wins. A detached thread avoids
|
||||
// making runtime shutdown wait for an uncancellable blocking stdin read.
|
||||
// Retain an unfinished read if an HTTP provider error triggers a scope retry.
|
||||
// Otherwise its detached reader could consume the next attempt's pasted URL.
|
||||
if pending_input.is_none() {
|
||||
let (tx, rx) = oneshot::channel();
|
||||
std::thread::Builder::new()
|
||||
.name("mcp-oauth-callback-input".to_string())
|
||||
.spawn(move || {
|
||||
let result = read_callback_line(io::stdin().lock());
|
||||
let _ = tx.send(result);
|
||||
})
|
||||
.context("failed to start callback input reader")?;
|
||||
*pending_input = Some(rx);
|
||||
}
|
||||
let reader = pending_input
|
||||
.as_mut()
|
||||
.context("callback input reader was unavailable")?;
|
||||
let result = reader.await;
|
||||
*pending_input = None;
|
||||
result.context("callback input reader stopped")?
|
||||
}
|
||||
|
||||
fn read_callback_line(reader: impl BufRead) -> Result<String> {
|
||||
let mut bytes = Vec::new();
|
||||
reader
|
||||
.take((MAX_CALLBACK_BYTES + 3) as u64)
|
||||
.read_until(b'\n', &mut bytes)
|
||||
.context("failed to read callback URL")?;
|
||||
if bytes.ends_with(b"\n") {
|
||||
bytes.pop();
|
||||
}
|
||||
if bytes.ends_with(b"\r") {
|
||||
bytes.pop();
|
||||
}
|
||||
if bytes.len() > MAX_CALLBACK_BYTES {
|
||||
bail!("OAuth callback URL exceeds 64 KiB");
|
||||
}
|
||||
if bytes.is_empty() {
|
||||
bail!("No OAuth callback URL received before input closed");
|
||||
}
|
||||
String::from_utf8(bytes).map_err(|_| anyhow::anyhow!("OAuth callback URL must be valid UTF-8"))
|
||||
}
|
||||
|
||||
struct TerminalInputGuard;
|
||||
|
||||
impl Drop for TerminalInputGuard {
|
||||
fn drop(&mut self) {
|
||||
let _ = terminal::disable_raw_mode();
|
||||
eprintln!();
|
||||
}
|
||||
}
|
||||
|
||||
fn print_callback_prompt() -> Result<()> {
|
||||
eprint!("Callback URL (input hidden): ");
|
||||
io::stderr()
|
||||
.flush()
|
||||
.context("failed to display callback prompt")
|
||||
}
|
||||
|
||||
async fn read_terminal_callback() -> Result<String> {
|
||||
terminal::enable_raw_mode().context("failed to hide callback input")?;
|
||||
let _guard = TerminalInputGuard;
|
||||
print_callback_prompt()?;
|
||||
let mut events = EventStream::new();
|
||||
let mut input = String::new();
|
||||
loop {
|
||||
let event = events
|
||||
.next()
|
||||
.await
|
||||
.context("callback input stream closed")?
|
||||
.context("failed to read callback input")?;
|
||||
match event {
|
||||
Event::Key(key) if key.kind != KeyEventKind::Release => match key.code {
|
||||
KeyCode::Enter => {
|
||||
if input.is_empty() {
|
||||
bail!("No OAuth callback URL entered");
|
||||
}
|
||||
return Ok(input);
|
||||
}
|
||||
KeyCode::Char('c') if key.modifiers.contains(KeyModifiers::CONTROL) => {
|
||||
bail!("OAuth login cancelled");
|
||||
}
|
||||
KeyCode::Char('d') if key.modifiers.contains(KeyModifiers::CONTROL) => {
|
||||
bail!("No OAuth callback URL received before input closed");
|
||||
}
|
||||
KeyCode::Char('u') if key.modifiers.contains(KeyModifiers::CONTROL) => {
|
||||
input.clear()
|
||||
}
|
||||
KeyCode::Backspace => {
|
||||
input.pop();
|
||||
}
|
||||
KeyCode::Char(character)
|
||||
if !character.is_control()
|
||||
&& !key.modifiers.contains(KeyModifiers::CONTROL) =>
|
||||
{
|
||||
if input.len() + character.len_utf8() > MAX_CALLBACK_BYTES {
|
||||
bail!("OAuth callback URL exceeds 64 KiB");
|
||||
}
|
||||
input.push(character);
|
||||
}
|
||||
_ => {}
|
||||
},
|
||||
Event::Paste(paste) => {
|
||||
if input.len() + paste.len() > MAX_CALLBACK_BYTES {
|
||||
bail!("OAuth callback URL exceeds 64 KiB");
|
||||
}
|
||||
input.push_str(&paste);
|
||||
}
|
||||
_ => {}
|
||||
}
|
||||
// Yield even with queued input so callback completion and timeout stay responsive.
|
||||
tokio::task::yield_now().await;
|
||||
}
|
||||
}
|
||||
|
||||
#[cfg(test)]
|
||||
#[path = "mcp_login_tests.rs"]
|
||||
mod tests;
|
||||
@@ -0,0 +1,46 @@
|
||||
//! Checks bounded callback input independently of terminal state.
|
||||
|
||||
use super::MAX_CALLBACK_BYTES;
|
||||
use super::read_callback_line;
|
||||
use pretty_assertions::assert_eq;
|
||||
use std::io::Cursor;
|
||||
|
||||
#[test]
|
||||
fn callback_line_accepts_crlf_and_eof_after_input() {
|
||||
for input in [
|
||||
"https://example.test/callback?code=abc\r\n",
|
||||
"https://example.test/callback?code=abc",
|
||||
] {
|
||||
assert_eq!(
|
||||
read_callback_line(Cursor::new(input)).expect("read callback URL"),
|
||||
"https://example.test/callback?code=abc"
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn callback_line_is_bounded_and_does_not_expose_input_in_errors() {
|
||||
let maximum_input = "s".repeat(MAX_CALLBACK_BYTES);
|
||||
assert_eq!(
|
||||
read_callback_line(Cursor::new(format!("{maximum_input}\r\n"))).expect("bounded input"),
|
||||
maximum_input
|
||||
);
|
||||
let input = "s".repeat(MAX_CALLBACK_BYTES + 1);
|
||||
assert_eq!(
|
||||
read_callback_line(Cursor::new(input))
|
||||
.unwrap_err()
|
||||
.to_string(),
|
||||
"OAuth callback URL exceeds 64 KiB"
|
||||
);
|
||||
assert_eq!(
|
||||
read_callback_line(Cursor::new("")).unwrap_err().to_string(),
|
||||
"No OAuth callback URL received before input closed"
|
||||
);
|
||||
assert_eq!(
|
||||
format!(
|
||||
"{:?}",
|
||||
read_callback_line(Cursor::new(b"sensitive\xff")).unwrap_err()
|
||||
),
|
||||
"OAuth callback URL must be valid UTF-8"
|
||||
);
|
||||
}
|
||||
Reference in New Issue
Block a user