From f446c9c304fc9e137dda87fa5f642903982b4d78 Mon Sep 17 00:00:00 2001 From: unforcedagi Date: Wed, 29 Jul 2026 01:02:45 -0600 Subject: [PATCH 1/2] hive mcp: own MCP servers and their credentials MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Buzz cannot configure an HTTP MCP server. `buzz-acp`'s McpServer is {name, command, args, env} — stdio only, no url, no headers — and the backend-provider deploy payload carries no MCP field at all. That is a boundary in the surface, not a gap to route around, so hive owns MCP configuration outright. Until now the only way to attach one was two flat fields on the desktop provider config, which could express exactly ONE server and hardcoded its name to "mcp". hive-spec has always had `mcp: Vec`; the format was never the limit. hive mcp add parachute --url https://vault/mcp --agent uni hive mcp login parachute --agent uni hive mcp list --agent uni Specs are edited with toml_edit rather than regenerated. The README calls generated specs "safe to edit and to commit", and any generator that rewrites the document silently deletes comments, ordering, and blocks it does not know about. `login` walks the flow the MCP specification defines: unauthenticated probe -> WWW-Authenticate resource_metadata -> protected-resource metadata (RFC 9728) -> authorization-server metadata (RFC 8414) -> dynamic client registration (RFC 7591) -> PKCE S256 -> authorization code -> token, bound to the resource with RFC 8707. Why an interactive flow rather than `secret put` with a hand-minted token: a minted token carries the minter's authority and expires without warning. Verified against a live Parachute vault, whose access tokens last 900 SECONDS — an agent on a static token would fail its first tool call fifteen minutes after setup, and the error would read as a broken server. The refresh token is stored alongside, and matters only because the broker serves credentials PER CONNECTION rather than injecting them once at container start; a design that baked credentials into the environment could not renew them without recreating the container. `refresh` reads through the broker's grant-checked path under a named `hive-cli` grant rather than reaching around it, so a manual renewal appears in the audit log exactly like an agent's own fetch. No new runtime dependency: HTTP goes through curl for the same reasons hive_core::docker drives Docker through its CLI, and base64url is forty lines rather than a crate. Known limitation: the loopback redirect needs a browser that can reach the host's 127.0.0.1, so a headless box needs someone at its screen. RFC 8628 device grant would fix that; Parachute advertises only authorization_code and refresh_token today. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Gw5rgVt1CQ8EYrLpZtQjdM --- Cargo.lock | 2 + crates/hive-cli/Cargo.toml | 2 + crates/hive-cli/src/main.rs | 84 ++++++ crates/hive-cli/src/mcp.rs | 372 +++++++++++++++++++++++++ crates/hive-cli/src/oauth.rs | 518 +++++++++++++++++++++++++++++++++++ 5 files changed, 978 insertions(+) create mode 100644 crates/hive-cli/src/mcp.rs create mode 100644 crates/hive-cli/src/oauth.rs diff --git a/Cargo.lock b/Cargo.lock index 92c9b71..02551f3 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -252,6 +252,8 @@ dependencies = [ "hive-core", "hive-spec", "serde_json", + "sha2", + "toml_edit", ] [[package]] diff --git a/crates/hive-cli/Cargo.toml b/crates/hive-cli/Cargo.toml index a81fb23..511ac7e 100644 --- a/crates/hive-cli/Cargo.toml +++ b/crates/hive-cli/Cargo.toml @@ -19,3 +19,5 @@ hive-broker.workspace = true serde_json.workspace = true clap.workspace = true anyhow.workspace = true +toml_edit.workspace = true +sha2.workspace = true diff --git a/crates/hive-cli/src/main.rs b/crates/hive-cli/src/main.rs index 1806c74..2069b16 100644 --- a/crates/hive-cli/src/main.rs +++ b/crates/hive-cli/src/main.rs @@ -19,6 +19,9 @@ use hive_core::harness::CATALOG; use hive_core::{agent, network}; use hive_spec::AgentSpec; +mod mcp; +mod oauth; + #[derive(Parser)] #[command(name = "hive", version, about = "Run persistent ACP agents in isolated containers")] struct Cli { @@ -82,6 +85,13 @@ enum Command { /// Manage credentials. #[command(subcommand)] Secret(SecretCmd), + /// MCP servers on an agent, and the credentials they need. + /// + /// Buzz cannot configure an HTTP MCP server — its `McpServer` is stdio-only + /// and the provider deploy payload carries no MCP field at all — so this is + /// where they live. + #[command(subcommand)] + Mcp(McpCmd), /// Print the firewall rules for hive's whole subnet pool, without applying them. /// /// Run this ONCE at setup. Every agent's network is allocated from the pool, @@ -103,6 +113,64 @@ enum Command { }, } +#[derive(Subcommand)] +enum McpCmd { + /// Show the MCP servers configured on an agent. + List { + #[arg(long)] + agent: String, + }, + /// Attach an HTTP MCP server. Re-running with the same name updates it + /// rather than adding a second block the harness would resolve arbitrarily. + Add { + name: String, + #[arg(long)] + url: String, + #[arg(long)] + agent: String, + /// Broker key holding its credential. Defaults to `mcp/`. + #[arg(long)] + credential: Option, + /// Restrict the agent to these tools. Empty means all of them. + #[arg(long)] + tool: Vec, + }, + /// Detach an MCP server. Its stored credential is left alone. + Rm { + name: String, + #[arg(long)] + agent: String, + }, + /// Walk the OAuth flow for an attached server and store the result. + /// + /// Prefer this over `secret put` with a hand-minted token: the credential is + /// scoped to what the resource advertises and comes with a refresh token, so + /// it does not lapse mid-conversation. + Login { + name: String, + #[arg(long)] + agent: String, + /// Comma- or space-separated. Defaults to every scope the resource + /// advertises — narrowing is deliberate, because a token that silently + /// lacks write fails at the first write tool call rather than here. + #[arg(long)] + scope: Option, + /// Print the URL without trying to launch a browser. + #[arg(long)] + no_browser: bool, + }, + /// Exchange a stored refresh token for a fresh access token. + /// + /// Rarely needed by hand — it exists so an expired credential can be + /// renewed without walking the browser flow again, and so the same code + /// path is exercised outside the broker. + Refresh { + name: String, + #[arg(long)] + agent: String, + }, +} + #[derive(Subcommand)] enum SecretCmd { /// Store a credential. Value is read from stdin so it never lands in shell @@ -126,6 +194,22 @@ fn main() -> Result<()> { Command::Restart { agent } => restart(agent), Command::Doctor => doctor(&cli), Command::Secret(c) => secret(&cli.secrets_dir, c), + Command::Mcp(c) => match c { + McpCmd::List { agent } => mcp::list(&cli.spec_dir, &agent), + McpCmd::Add { name, url, agent, credential, tool } => { + mcp::add(&cli.spec_dir, &agent, &name, &url, credential.as_deref(), &tool) + } + McpCmd::Rm { name, agent } => mcp::rm(&cli.spec_dir, &agent, &name), + McpCmd::Refresh { name, agent } => mcp::refresh(&cli.spec_dir, &cli.secrets_dir, &agent, &name), + McpCmd::Login { name, agent, scope, no_browser } => mcp::login( + &cli.spec_dir, + &cli.secrets_dir, + &agent, + &name, + scope.as_deref(), + !no_browser, + ), + }, Command::Firewall { agent, host_addr, published } => { firewall(agent.as_deref(), host_addr, *published) } diff --git a/crates/hive-cli/src/mcp.rs b/crates/hive-cli/src/mcp.rs new file mode 100644 index 0000000..961d002 --- /dev/null +++ b/crates/hive-cli/src/mcp.rs @@ -0,0 +1,372 @@ +//! `hive mcp` — MCP servers on an agent, and the credentials they need. +//! +//! # Why this lives in hive rather than in the desktop +//! +//! Buzz cannot express an HTTP MCP server. Its `McpServer` is +//! `{name, command, args, env}` — stdio only — and the backend-provider deploy +//! payload has no MCP field at all. That is a boundary, not a gap to route +//! around, so hive owns MCP configuration outright. +//! +//! # Why the spec is edited in place +//! +//! Specs are meant to be hand-editable and committable — the README says so. +//! Anything that regenerates a whole spec destroys comments, ordering, and any +//! block the generator does not know about. `toml_edit` preserves the document, +//! so `hive mcp add` and a human editor can share one file. + +use std::path::Path; + +use anyhow::{bail, Context, Result}; +use hive_broker::Broker; +use hive_core::credential::CredentialKey; +use toml_edit::{value, Array, DocumentMut, Item, Table}; + +use crate::oauth; + +/// Read a spec, or explain which agent names do exist. A typo'd agent name is +/// the most common way to reach this, and "no such file" does not help. +fn load(spec_dir: &Path, agent: &str) -> Result<(std::path::PathBuf, DocumentMut)> { + let path = oauth::spec_path(spec_dir, agent); + let text = std::fs::read_to_string(&path).map_err(|e| { + let known: Vec = std::fs::read_dir(spec_dir) + .map(|rd| { + rd.filter_map(|e| e.ok()) + .filter_map(|e| { + let n = e.file_name().to_string_lossy().to_string(); + n.strip_suffix(".toml").map(String::from) + }) + .collect() + }) + .unwrap_or_default(); + if known.is_empty() { + anyhow::anyhow!("no agent specs in {} ({e})", spec_dir.display()) + } else { + anyhow::anyhow!("no agent {agent:?} in {} — have: {}", spec_dir.display(), known.join(", ")) + } + })?; + let doc: DocumentMut = text + .parse() + .with_context(|| format!("{} is not valid TOML", path.display()))?; + Ok((path, doc)) +} + +fn mcp_array<'a>(doc: &'a mut DocumentMut) -> &'a mut toml_edit::ArrayOfTables { + if !doc.contains_key("mcp") { + doc["mcp"] = Item::ArrayOfTables(toml_edit::ArrayOfTables::new()); + } + doc["mcp"] + .as_array_of_tables_mut() + .expect("mcp is an array of tables") +} + +pub fn list(spec_dir: &Path, agent: &str) -> Result<()> { + let (_, mut doc) = load(spec_dir, agent)?; + let arr = mcp_array(&mut doc); + if arr.is_empty() { + println!("no MCP servers configured for {agent}"); + return Ok(()); + } + println!("{:<16} {:<10} {:<44} {}", "NAME", "TRANSPORT", "URL", "CREDENTIAL"); + for t in arr.iter() { + println!( + "{:<16} {:<10} {:<44} {}", + t.get("name").and_then(|v| v.as_str()).unwrap_or("?"), + t.get("transport").and_then(|v| v.as_str()).unwrap_or("http"), + t.get("url").and_then(|v| v.as_str()).unwrap_or(""), + t.get("credential").and_then(|v| v.as_str()).unwrap_or("-"), + ); + } + Ok(()) +} + +/// Add (or update) one server. `credential` defaults to `mcp/` so the +/// common case needs no flag and the broker key is predictable from the spec. +pub fn add( + spec_dir: &Path, + agent: &str, + name: &str, + url: &str, + credential: Option<&str>, + tools: &[String], +) -> Result<()> { + if name.is_empty() { + bail!("an MCP server needs a name — it is how the harness refers to it"); + } + let credential = credential.map(String::from).unwrap_or_else(|| format!("mcp/{name}")); + let (path, mut doc) = load(spec_dir, agent)?; + let arr = mcp_array(&mut doc); + + // Replace by name rather than appending: two blocks with one name is a + // config the harness resolves arbitrarily, and `add` twice is a normal + // thing to do while getting a URL right. + let existing = arr.iter().position(|t| t.get("name").and_then(|v| v.as_str()) == Some(name)); + + let mut t = Table::new(); + t["name"] = value(name); + t["transport"] = value("http"); + t["url"] = value(url); + t["credential"] = value(&credential); + if !tools.is_empty() { + let mut a = Array::new(); + for tool in tools { + a.push(tool.as_str()); + } + t["tools"] = value(a); + } + + match existing { + Some(i) => { + *arr.get_mut(i).expect("index from position") = t; + println!("updated {name} on {agent}"); + } + None => { + arr.push(t); + println!("added {name} to {agent}"); + } + } + std::fs::write(&path, doc.to_string()).with_context(|| format!("writing {}", path.display()))?; + println!(" credential: {credential}"); + println!(" next: hive mcp login {name} --agent {agent}"); + Ok(()) +} + +pub fn rm(spec_dir: &Path, agent: &str, name: &str) -> Result<()> { + let (path, mut doc) = load(spec_dir, agent)?; + let arr = mcp_array(&mut doc); + let before = arr.len(); + arr.retain(|t| t.get("name").and_then(|v| v.as_str()) != Some(name)); + if arr.len() == before { + bail!("{agent} has no MCP server named {name:?}"); + } + std::fs::write(&path, doc.to_string()).with_context(|| format!("writing {}", path.display()))?; + println!("removed {name} from {agent}"); + println!("note: its credential is still stored — `hive secret rm mcp/{name}` to drop it"); + Ok(()) +} + +/// Walk the OAuth flow for a server already present in the spec, and store the +/// result in the broker. +/// +/// The URL is read from the spec rather than taken as an argument: logging in +/// against a different URL than the agent will use produces a token bound to +/// the wrong resource, and that failure surfaces much later as a 401 from the +/// MCP server. +pub fn login( + spec_dir: &Path, + secrets_dir: &Path, + agent: &str, + name: &str, + scopes: Option<&str>, + open_browser: bool, +) -> Result<()> { + let (_, mut doc) = load(spec_dir, agent)?; + let arr = mcp_array(&mut doc); + let entry = arr + .iter() + .find(|t| t.get("name").and_then(|v| v.as_str()) == Some(name)) + .with_context(|| format!("{agent} has no MCP server named {name:?} — add it first"))?; + let url = entry + .get("url") + .and_then(|v| v.as_str()) + .with_context(|| format!("{name} has no url"))? + .to_string(); + let credential = entry + .get("credential") + .and_then(|v| v.as_str()) + .map(String::from) + .unwrap_or_else(|| format!("mcp/{name}")); + + println!("discovering {url} …"); + let auth = oauth::discover(&url)?; + println!(" authorization server: {}", auth.issuer); + println!(" resource: {}", auth.resource); + + // Requested scopes default to everything the resource advertises. Narrowing + // is a deliberate act (--scope), because a token that silently lacks write + // fails at the first create-note rather than at login. + let requested: Vec = match scopes { + Some(s) => s.split(&[',', ' '][..]).filter(|p| !p.is_empty()).map(String::from).collect(), + None => auth.scopes_supported.clone(), + }; + if !requested.is_empty() { + println!(" scopes: {}", requested.join(" ")); + } + + // Bind BEFORE registering. The redirect URI carries the port, the server + // records it at registration time, and it must match exactly at the + // authorization step — registering a placeholder port and hoping produces + // an "invalid redirect_uri" that points at nothing obvious. + let (listener, redirect_uri) = oauth::bind_callback()?; + let client_id = oauth::register_client(&auth, &redirect_uri)?; + + let (code, bundle) = + oauth::authorize(listener, &redirect_uri, &auth, &client_id, &requested, open_browser)?; + let tokens = oauth::exchange(&auth, &client_id, &code, &bundle)?; + + let broker = Broker::open(secrets_dir)?; + broker.put(&CredentialKey::new(&credential), tokens.access_token.as_bytes())?; + if let Some(rt) = tokens.refresh_token.as_deref() { + broker.put(&CredentialKey::new(&oauth::refresh_key(&credential)), rt.as_bytes())?; + } + broker.put( + &CredentialKey::new(&oauth::meta_key(&credential)), + oauth::meta_json(&auth, &client_id).as_bytes(), + )?; + + println!("\nstored {credential}"); + match tokens.expires_in { + // Stated plainly because it is the difference between "this works" and + // "this works until Thursday". A refresh token makes the expiry a + // detail; without one it is the whole story. + Some(s) if tokens.refresh_token.is_some() => { + println!(" expires in {s}s, refresh token stored — the broker can renew it") + } + Some(s) => println!(" expires in {s}s and NO refresh token was issued — login again when it lapses"), + None => println!(" no expiry advertised"), + } + if let Some(sc) = tokens.scope { + println!(" granted scopes: {sc}"); + } + println!("\n hive restart {agent} # so the harness picks it up"); + Ok(()) +} + +/// Renew an access token from the stored refresh token. +/// +/// Reads the endpoint and client id from the metadata saved at login rather +/// than re-running discovery: a server that has since moved its endpoints +/// should fail loudly here, not silently mint against a different issuer. +pub fn refresh(spec_dir: &Path, secrets_dir: &Path, agent: &str, name: &str) -> Result<()> { + let (_, mut doc) = load(spec_dir, agent)?; + let arr = mcp_array(&mut doc); + let credential = arr + .iter() + .find(|t| t.get("name").and_then(|v| v.as_str()) == Some(name)) + .and_then(|t| t.get("credential").and_then(|v| v.as_str()).map(String::from)) + .unwrap_or_else(|| format!("mcp/{name}")); + + let broker = Broker::open(secrets_dir)?; + // The broker has no ungated read: every fetch is checked against a grant + // and written to the audit log. Rather than reach around it, the operator + // gets a named grant for exactly these two keys — so a manual refresh is + // as visible in the audit trail as an agent's own fetch would be. + let meta_k = oauth::meta_key(&credential); + let refresh_k = oauth::refresh_key(&credential); + let grant = hive_broker::Grant::new("hive-cli", [meta_k.clone(), refresh_k.clone()]); + + let meta_secret = broker + .fetch_for(&grant, &CredentialKey::new(&meta_k)) + .context("no stored OAuth metadata — run `hive mcp login` first")?; + let meta: serde_json::Value = serde_json::from_slice(meta_secret.expose()) + .context("stored OAuth metadata is not JSON")?; + let rt = broker + .fetch_for(&grant, &CredentialKey::new(&refresh_k)) + .context("no refresh token stored — this server issued none, so log in again")?; + + let tokens = oauth::refresh( + meta["token_endpoint"].as_str().context("metadata has no token_endpoint")?, + meta["client_id"].as_str().context("metadata has no client_id")?, + rt.as_str()?.trim(), + meta["resource"].as_str().unwrap_or_default(), + )?; + + broker.put(&CredentialKey::new(&credential), tokens.access_token.as_bytes())?; + // Only overwrite when the server rotated it. Clearing a still-valid refresh + // token because this response omitted one would force a browser login. + if let Some(new_rt) = tokens.refresh_token.as_deref() { + broker.put(&CredentialKey::new(&oauth::refresh_key(&credential)), new_rt.as_bytes())?; + } + println!("refreshed {credential}"); + if let Some(s) = tokens.expires_in { + println!(" expires in {s}s"); + } + Ok(()) +} + +#[cfg(test)] +mod tests { + use super::*; + + /// Same shape as `hive_broker`'s test helper: a per-process directory under + /// the system temp dir, wiped on entry. No dev-dependency — this crate + /// graph deliberately has none. + fn spec_with(tag: &str, body: &str) -> (std::path::PathBuf, std::path::PathBuf) { + let d = std::env::temp_dir().join(format!("hive-mcp-{}-{tag}", std::process::id())); + let _ = std::fs::remove_dir_all(&d); + std::fs::create_dir_all(&d).expect("create tempdir"); + let p = d.join("uni.toml"); + std::fs::write(&p, body).expect("write"); + (d, p) + } + + #[test] + fn adding_a_server_preserves_comments_and_unrelated_blocks() { + // The whole reason this uses toml_edit. A generator that rewrites the + // document drops the operator's comments and any block it does not know + // about — which is how the old desktop shim silently erased hand-added + // MCP config on every redeploy. + let (d, p) = spec_with( + "preserve", + "# hand-written, keep me\n[identity]\npubkey = \"aa\"\n\n[[volume]]\nname = \"shared\"\ntarget = \"/home/agent/work\"\n", + ); + add(&d, "uni", "parachute", "https://x/mcp", None, &[]).expect("add"); + let out = std::fs::read_to_string(&p).expect("read"); + assert!(out.contains("# hand-written, keep me"), "comment was dropped:\n{out}"); + assert!(out.contains("[[volume]]"), "unrelated block was dropped:\n{out}"); + assert!(out.contains("credential = \"mcp/parachute\""), "{out}"); + } + + #[test] + fn adding_the_same_name_twice_updates_rather_than_duplicating() { + // Two blocks with one name is a config the harness resolves + // arbitrarily, and re-running `add` while correcting a URL is normal. + let (d, p) = spec_with("update", "[identity]\npubkey = \"aa\"\n"); + add(&d, "uni", "parachute", "https://old/mcp", None, &[]).expect("first"); + add(&d, "uni", "parachute", "https://new/mcp", None, &[]).expect("second"); + let out = std::fs::read_to_string(&p).expect("read"); + assert_eq!(out.matches("name = \"parachute\"").count(), 1, "{out}"); + assert!(out.contains("https://new/mcp"), "{out}"); + assert!(!out.contains("https://old/mcp"), "{out}"); + } + + #[test] + fn several_servers_coexist_on_one_agent() { + // The limit that made this necessary: the desktop shim could express + // exactly one MCP server, hardcoded to the name "mcp". + let (d, p) = spec_with("several", "[identity]\npubkey = \"aa\"\n"); + add(&d, "uni", "parachute", "https://a/mcp", None, &[]).expect("a"); + add(&d, "uni", "github", "https://b/mcp", None, &[]).expect("b"); + let out = std::fs::read_to_string(&p).expect("read"); + assert_eq!(out.matches("[[mcp]]").count(), 2, "{out}"); + assert!(out.contains("mcp/parachute") && out.contains("mcp/github"), "{out}"); + } + + #[test] + fn removing_a_server_leaves_the_others() { + let (d, p) = spec_with("removeone", "[identity]\npubkey = \"aa\"\n"); + add(&d, "uni", "a", "https://a/mcp", None, &[]).expect("a"); + add(&d, "uni", "b", "https://b/mcp", None, &[]).expect("b"); + rm(&d, "uni", "a").expect("rm"); + let out = std::fs::read_to_string(&p).expect("read"); + assert!(!out.contains("name = \"a\""), "{out}"); + assert!(out.contains("name = \"b\""), "{out}"); + } + + #[test] + fn removing_a_name_that_is_not_there_is_an_error_not_a_silent_noop() { + // A typo'd name that reports success leaves the operator believing an + // MCP server was detached when it is still in the spec. + let (d, _) = spec_with("rmmissing", "[identity]\npubkey = \"aa\"\n"); + add(&d, "uni", "a", "https://a/mcp", None, &[]).expect("a"); + assert!(rm(&d, "uni", "typo").is_err()); + } + + #[test] + fn an_unknown_agent_lists_the_ones_that_exist() { + // "No such file" sends people to look at permissions; the agent list + // sends them to look at their spelling. + let (d, _) = spec_with("unknown", "[identity]\npubkey = \"aa\"\n"); + let e = add(&d, "nope", "x", "https://x", None, &[]).unwrap_err().to_string(); + assert!(e.contains("uni"), "error should name known agents: {e}"); + } +} diff --git a/crates/hive-cli/src/oauth.rs b/crates/hive-cli/src/oauth.rs new file mode 100644 index 0000000..caca8ef --- /dev/null +++ b/crates/hive-cli/src/oauth.rs @@ -0,0 +1,518 @@ +//! The MCP authorization-code flow, for `hive mcp login`. +//! +//! # Why hive does this at all +//! +//! Buzz cannot. Its `McpServer` is `{name, command, args, env}` — stdio only, +//! no `url`, no `headers` — and the backend-provider deploy payload carries no +//! MCP field whatsoever. So an HTTP MCP server can never be configured from the +//! desktop, in either extension seam. If agents are to reach one, hive has to +//! own the credential. +//! +//! # Why an interactive flow rather than a static token +//! +//! A hand-minted bearer token is derived from whoever minted it, carries that +//! person's authority, and expires without warning — at which point the agent +//! starts failing tool calls and the error reads as a broken server. The +//! authorization-code flow gets a token scoped to the *agent's* grant, plus a +//! refresh token. +//! +//! Refresh is only useful because of an existing design choice: the broker +//! serves credentials **per connection** rather than injecting them once at +//! container start. So a token can be refreshed between one tool call and the +//! next without recreating the container. A design that baked credentials into +//! the environment could not do this. +//! +//! # Why curl instead of an HTTP crate +//! +//! Same reasoning `hive_core::docker` gives for driving Docker through its CLI: +//! no large dependency tracking a moving API, and every request is a command +//! that can be logged verbatim and re-run by hand when a server misbehaves. +//! Token values are the one thing never logged. + +use std::collections::BTreeMap; +use std::io::{BufRead, BufReader, Write}; +use std::net::TcpListener; +use std::path::PathBuf; +use std::process::Command; + +use anyhow::{bail, Context, Result}; +use serde_json::Value; +use sha2::{Digest, Sha256}; + +/// Everything discovery has to produce before an authorization request can be +/// built. Discovered, never configured: an authorization server that moves its +/// endpoints would otherwise silently break every stored config. +#[derive(Debug, Clone)] +pub struct AuthServer { + pub issuer: String, + pub authorization_endpoint: String, + pub token_endpoint: String, + pub registration_endpoint: Option, + /// The canonical resource identifier the token must be bound to (RFC 8707). + /// Taken from the protected-resource metadata rather than from the URL the + /// user typed, so a token cannot be minted for the wrong audience because + /// someone reached the same server by a different hostname. + pub resource: String, + pub scopes_supported: Vec, +} + +/// What a successful flow yields. `refresh_token` is optional because not every +/// server issues one; when absent the credential simply expires and login must +/// be repeated. +#[derive(Debug, Clone)] +pub struct Tokens { + pub access_token: String, + pub refresh_token: Option, + pub expires_in: Option, + pub scope: Option, +} + +/// GET a URL and parse JSON. Non-2xx is an error carrying the body, because an +/// OAuth server's error body is the only useful diagnostic it gives you. +fn get_json(url: &str) -> Result { + let out = Command::new(curl()?) + .args(["-sS", "-L", "--max-time", "20", "-w", "\n%{http_code}", url]) + .output() + .with_context(|| format!("GET {url}"))?; + finish(url, &out.stdout, &out.stderr) +} + +fn post_form(url: &str, fields: &BTreeMap<&str, String>) -> Result { + let mut cmd = Command::new(curl()?); + cmd.args(["-sS", "--max-time", "20", "-w", "\n%{http_code}", "-X", "POST"]); + // --data-urlencode, not --data: a code_verifier is base64url and a scope + // contains spaces; both corrupt silently if sent raw, and the server's + // complaint ("invalid_grant") points at the code rather than the encoding. + for (k, v) in fields { + cmd.arg("--data-urlencode").arg(format!("{k}={v}")); + } + cmd.arg(url); + let out = cmd.output().with_context(|| format!("POST {url}"))?; + finish(url, &out.stdout, &out.stderr) +} + +fn post_json(url: &str, body: &Value) -> Result { + let out = Command::new(curl()?) + .args([ + "-sS", "--max-time", "20", "-w", "\n%{http_code}", + "-X", "POST", "-H", "Content-Type: application/json", + "--data-binary", "@-", + ]) + .arg(url) + .stdin(std::process::Stdio::piped()) + .stdout(std::process::Stdio::piped()) + .stderr(std::process::Stdio::piped()) + .spawn() + .and_then(|mut c| { + c.stdin.take().expect("piped").write_all(body.to_string().as_bytes())?; + c.wait_with_output() + }) + .with_context(|| format!("POST {url}"))?; + finish(url, &out.stdout, &out.stderr) +} + +/// Split curl's `-w "\n%{http_code}"` suffix off the body, then parse. +fn finish(url: &str, stdout: &[u8], stderr: &[u8]) -> Result { + let text = String::from_utf8_lossy(stdout); + let (body, code) = text.rsplit_once('\n').unwrap_or(("", text.as_ref())); + let code: u16 = code.trim().parse().unwrap_or(0); + if code == 0 { + bail!("{url}: no response ({})", String::from_utf8_lossy(stderr).trim()); + } + if !(200..300).contains(&code) { + bail!("{url}: HTTP {code} {}", body.trim()); + } + serde_json::from_str(body).with_context(|| format!("{url}: response was not JSON: {}", body.trim())) +} + +fn curl() -> Result { + for p in ["/usr/bin/curl", "/opt/homebrew/bin/curl", "/usr/local/bin/curl"] { + if std::path::Path::new(p).is_file() { + return Ok(p.to_string()); + } + } + Ok("curl".to_string()) +} + +/// Walk the discovery chain the MCP specification defines: +/// unauthenticated request → `WWW-Authenticate: Bearer resource_metadata=…` +/// → protected-resource metadata (RFC 9728) → authorization-server metadata +/// (RFC 8414). +/// +/// The `resource_metadata` pointer is read from the challenge rather than +/// guessed from a well-known path: a vault served under a path prefix +/// (`/vault//mcp`) advertises its metadata under that same prefix, and +/// the origin-root well-known path 404s. +pub fn discover(mcp_url: &str) -> Result { + let out = Command::new(curl()?) + .args([ + "-sS", "-i", "--max-time", "20", "-X", "POST", + "-H", "Content-Type: application/json", "-d", "{}", + ]) + .arg(mcp_url) + .output() + .with_context(|| format!("probing {mcp_url}"))?; + let head = String::from_utf8_lossy(&out.stdout); + + let meta_url = head + .lines() + .find(|l| l.to_ascii_lowercase().starts_with("www-authenticate:")) + .and_then(|l| l.split_once("resource_metadata=")) + .map(|(_, v)| v.trim().trim_matches('"').to_string()) + .with_context(|| { + format!( + "{mcp_url} did not advertise resource_metadata in a WWW-Authenticate header. \ + Either it is not an OAuth-protected MCP server, or it accepted the \ + unauthenticated probe — check whether it needs credentials at all." + ) + })?; + + let prm = get_json(&meta_url)?; + let resource = prm["resource"].as_str().unwrap_or(mcp_url).to_string(); + let issuer = prm["authorization_servers"] + .as_array() + .and_then(|a| a.first()) + .and_then(Value::as_str) + .context("protected-resource metadata listed no authorization_servers")? + .to_string(); + let scopes_supported = prm["scopes_supported"] + .as_array() + .map(|a| a.iter().filter_map(|v| v.as_str().map(String::from)).collect()) + .unwrap_or_default(); + + let asm = get_json(&format!("{}/.well-known/oauth-authorization-server", issuer.trim_end_matches('/')))?; + + Ok(AuthServer { + authorization_endpoint: asm["authorization_endpoint"] + .as_str() + .context("authorization server declared no authorization_endpoint")? + .to_string(), + token_endpoint: asm["token_endpoint"] + .as_str() + .context("authorization server declared no token_endpoint")? + .to_string(), + registration_endpoint: asm["registration_endpoint"].as_str().map(String::from), + issuer, + resource, + scopes_supported, + }) +} + +/// Register a client on the fly (RFC 7591). +/// +/// hive has no pre-registered client id and should not need one: an agent host +/// that required manual client registration per vault would make adding an MCP +/// server a support ticket rather than a command. +pub fn register_client(auth: &AuthServer, redirect_uri: &str) -> Result { + let Some(endpoint) = auth.registration_endpoint.as_deref() else { + bail!( + "{} supports no dynamic client registration; register a client manually \ + and pass --client-id", + auth.issuer + ); + }; + let body = serde_json::json!({ + "client_name": "hive", + "redirect_uris": [redirect_uri], + "grant_types": ["authorization_code", "refresh_token"], + "response_types": ["code"], + // `none` because this is a public client: hive runs on a box the user + // controls and cannot keep a client secret meaningfully. PKCE is what + // actually protects the exchange. + "token_endpoint_auth_method": "none", + }); + let resp = post_json(endpoint, &body)?; + resp["client_id"] + .as_str() + .map(String::from) + .context("registration response contained no client_id") +} + +/// RFC 7636 S256 verifier/challenge pair. +fn pkce() -> (String, String) { + // 32 bytes of entropy from the OS, via getrandom(2) through /dev/urandom. + // Not a PRNG seeded from the clock: two agents enrolled in the same second + // must not derive the same verifier. + let mut raw = [0u8; 32]; + let mut f = std::fs::File::open("/dev/urandom").expect("/dev/urandom"); + use std::io::Read; + f.read_exact(&mut raw).expect("read /dev/urandom"); + let verifier = b64url(&raw); + let challenge = b64url(&Sha256::digest(verifier.as_bytes())); + (verifier, challenge) +} + +/// base64url without padding (RFC 4648 §5). Hand-rolled to avoid a dependency +/// for forty lines of table lookup; `=` padding is omitted because RFC 7636 +/// requires it to be. +fn b64url(bytes: &[u8]) -> String { + const T: &[u8; 64] = b"ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz0123456789-_"; + let mut out = String::with_capacity(bytes.len().div_ceil(3) * 4); + for c in bytes.chunks(3) { + let b = [c[0], *c.get(1).unwrap_or(&0), *c.get(2).unwrap_or(&0)]; + let n = u32::from(b[0]) << 16 | u32::from(b[1]) << 8 | u32::from(b[2]); + out.push(T[(n >> 18 & 63) as usize] as char); + out.push(T[(n >> 12 & 63) as usize] as char); + if c.len() > 1 { + out.push(T[(n >> 6 & 63) as usize] as char); + } + if c.len() > 2 { + out.push(T[(n & 63) as usize] as char); + } + } + out +} + +fn urlenc(s: &str) -> String { + let mut out = String::with_capacity(s.len()); + for b in s.bytes() { + match b { + b'A'..=b'Z' | b'a'..=b'z' | b'0'..=b'9' | b'-' | b'_' | b'.' | b'~' => out.push(b as char), + _ => out.push_str(&format!("%{b:02X}")), + } + } + out +} + +/// Run the interactive half: bind a loopback listener, print (and try to open) +/// the authorization URL, and block until the browser redirects back. +/// +/// Loopback rather than a fixed port: the port is part of the redirect URI that +/// was registered moments earlier, so nothing is hardcoded and two concurrent +/// logins cannot collide. +pub fn bind_callback() -> Result<(TcpListener, String)> { + let listener = TcpListener::bind("127.0.0.1:0").context("binding a loopback callback listener")?; + let port = listener.local_addr()?.port(); + Ok((listener, format!("http://127.0.0.1:{port}/callback"))) +} + +pub fn authorize( + listener: TcpListener, + redirect_uri: &str, + auth: &AuthServer, + client_id: &str, + scopes: &[String], + open_browser: bool, +) -> Result<(String, String)> { + let port = listener.local_addr()?.port(); + let (verifier, challenge) = pkce(); + let state = b64url(&Sha256::digest(format!("{port}{challenge}").as_bytes()))[..16].to_string(); + + let mut url = format!( + "{}?response_type=code&client_id={}&redirect_uri={}&code_challenge={}&code_challenge_method=S256&state={}&resource={}", + auth.authorization_endpoint, + urlenc(client_id), + urlenc(redirect_uri), + urlenc(&challenge), + urlenc(&state), + urlenc(&auth.resource), + ); + if !scopes.is_empty() { + url.push_str(&format!("&scope={}", urlenc(&scopes.join(" ")))); + } + + println!("\nAuthorize hive in your browser:\n\n {url}\n"); + if open_browser { + // Best-effort. A headless box has no browser and that is not an error — + // the URL above is printed first precisely so it stays usable there. + let _ = Command::new("open").arg(&url).stdout(std::process::Stdio::null()).stderr(std::process::Stdio::null()).status(); + } + println!("Waiting for the redirect on 127.0.0.1:{port} …"); + + let (mut sock, _) = listener.accept().context("waiting for the OAuth redirect")?; + let mut reader = BufReader::new(sock.try_clone()?); + let mut request_line = String::new(); + reader.read_line(&mut request_line)?; + + let query = request_line + .split_whitespace() + .nth(1) + .and_then(|p| p.split_once('?').map(|(_, q)| q.to_string())) + .unwrap_or_default(); + let params: BTreeMap<&str, &str> = + query.split('&').filter_map(|kv| kv.split_once('=')).collect(); + + let reply = |sock: &mut std::net::TcpStream, msg: &str| { + let _ = write!( + sock, + "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nConnection: close\r\n\r\n

{msg}

You can close this tab.

" + ); + }; + + if let Some(err) = params.get("error") { + reply(&mut sock, "Authorization failed"); + bail!("authorization server refused: {err}"); + } + // Compare state before touching the code: without this a redirect from an + // unrelated flow would be accepted and exchanged. + if params.get("state").map(|s| *s != state).unwrap_or(true) { + reply(&mut sock, "Authorization failed"); + bail!("state mismatch — the redirect did not belong to this login attempt"); + } + let code = params + .get("code") + .map(|c| percent_decode(c)) + .context("redirect carried no authorization code")?; + reply(&mut sock, "hive is authorized"); + + Ok((code, format!("{verifier}\u{1}{redirect_uri}"))) +} + +fn percent_decode(s: &str) -> String { + let b = s.as_bytes(); + let mut out = Vec::with_capacity(b.len()); + let mut i = 0; + while i < b.len() { + if b[i] == b'%' && i + 2 < b.len() { + if let Ok(v) = u8::from_str_radix(&s[i + 1..i + 3], 16) { + out.push(v); + i += 3; + continue; + } + } + out.push(if b[i] == b'+' { b' ' } else { b[i] }); + i += 1; + } + String::from_utf8_lossy(&out).into_owned() +} + +/// Exchange the authorization code for tokens. +pub fn exchange(auth: &AuthServer, client_id: &str, code: &str, verifier_and_redirect: &str) -> Result { + let (verifier, redirect_uri) = verifier_and_redirect + .split_once('\u{1}') + .context("internal: malformed verifier bundle")?; + let mut f = BTreeMap::new(); + f.insert("grant_type", "authorization_code".to_string()); + f.insert("code", code.to_string()); + f.insert("redirect_uri", redirect_uri.to_string()); + f.insert("client_id", client_id.to_string()); + f.insert("code_verifier", verifier.to_string()); + // RFC 8707. Without it a server that issues audience-bound tokens returns + // one the resource will reject, and the failure appears later as a 401 from + // the MCP server rather than here. + f.insert("resource", auth.resource.clone()); + + let resp = post_form(&auth.token_endpoint, &f)?; + Ok(Tokens { + access_token: resp["access_token"] + .as_str() + .context("token response contained no access_token")? + .to_string(), + refresh_token: resp["refresh_token"].as_str().map(String::from), + expires_in: resp["expires_in"].as_u64(), + scope: resp["scope"].as_str().map(String::from), + }) +} + +/// Where a refresh token is parked so the broker can find it later. Kept beside +/// the access token under a `+refresh` suffix rather than in a sidecar file, so +/// `hive secret rm` on the credential removes both halves and cannot leave a +/// refresh token behind that still mints access. +pub fn refresh_key(credential: &str) -> String { + format!("{credential}+refresh") +} + +/// Metadata a later `refresh` needs, stored next to the credential. +pub fn meta_key(credential: &str) -> String { + format!("{credential}+oauth") +} + +pub fn meta_json(auth: &AuthServer, client_id: &str) -> String { + serde_json::json!({ + "token_endpoint": auth.token_endpoint, + "client_id": client_id, + "resource": auth.resource, + "issuer": auth.issuer, + }) + .to_string() +} + +/// Not used by the CLI directly — exposed so the broker can refresh on the +/// per-connection path without duplicating the token-endpoint contract. +pub fn refresh(token_endpoint: &str, client_id: &str, refresh_token: &str, resource: &str) -> Result { + let mut f = BTreeMap::new(); + f.insert("grant_type", "refresh_token".to_string()); + f.insert("refresh_token", refresh_token.to_string()); + f.insert("client_id", client_id.to_string()); + f.insert("resource", resource.to_string()); + let resp = post_form(token_endpoint, &f)?; + Ok(Tokens { + access_token: resp["access_token"] + .as_str() + .context("refresh response contained no access_token")? + .to_string(), + // A server that rotates refresh tokens returns a new one; when it does + // not, the old one stays valid and must be kept rather than cleared. + refresh_token: resp["refresh_token"].as_str().map(String::from), + expires_in: resp["expires_in"].as_u64(), + scope: resp["scope"].as_str().map(String::from), + }) +} + +/// Spec directory helper shared with the `mcp` subcommand. +pub fn spec_path(spec_dir: &std::path::Path, agent: &str) -> PathBuf { + spec_dir.join(format!("{agent}.toml")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn base64url_is_unpadded_and_uses_the_url_alphabet() { + // RFC 7636 requires unpadded base64url. Standard base64 would emit '+' + // and '/', which a server percent-decodes into different bytes — and + // the failure surfaces as "invalid_grant" against a verifier that looks + // correct in the logs. + assert_eq!(b64url(b""), ""); + assert_eq!(b64url(b"f"), "Zg"); + assert_eq!(b64url(b"fo"), "Zm8"); + assert_eq!(b64url(b"foo"), "Zm9v"); + assert_eq!(b64url(b"foobar"), "Zm9vYmFy"); + let all = b64url(&(0u8..=255).collect::>()); + assert!(!all.contains('+') && !all.contains('/') && !all.contains('=')); + } + + #[test] + fn a_verifier_is_not_reused_between_logins() { + // Two agents enrolled against the same server must not share a verifier; + // if they did, one agent's redirect could be exchanged by the other. + let (a, _) = pkce(); + let (b, _) = pkce(); + assert_ne!(a, b); + assert!(a.len() >= 43, "RFC 7636 requires at least 43 characters"); + } + + #[test] + fn the_challenge_is_the_hash_of_the_verifier_not_the_raw_entropy() { + // S256 hashes the ASCII verifier string. Hashing the pre-encoding bytes + // produces a challenge the server cannot reproduce, and the exchange + // fails with a generic invalid_grant. + let (v, c) = pkce(); + assert_eq!(c, b64url(&Sha256::digest(v.as_bytes()))); + } + + #[test] + fn refresh_and_metadata_keys_hang_off_the_credential_name() { + // `hive secret rm mcp/parachute` must be able to find and remove every + // derived key; a refresh token left behind still mints access tokens. + assert_eq!(refresh_key("mcp/parachute"), "mcp/parachute+refresh"); + assert_eq!(meta_key("mcp/parachute"), "mcp/parachute+oauth"); + } + + #[test] + fn percent_decoding_handles_the_plus_and_escape_forms() { + assert_eq!(percent_decode("a%2Fb"), "a/b"); + assert_eq!(percent_decode("a+b"), "a b"); + assert_eq!(percent_decode("plain"), "plain"); + } + + #[test] + fn urlencoding_escapes_everything_outside_the_unreserved_set() { + // A scope contains spaces and a resource is a URL; both corrupt the + // authorization request if passed through raw. + assert_eq!(urlenc("a b"), "a%20b"); + assert_eq!(urlenc("https://x/y"), "https%3A%2F%2Fx%2Fy"); + assert_eq!(urlenc("-_.~"), "-_.~"); + } +} From c40f74adcff0db2e8a07690011865405f5f2b4fa Mon Sep 17 00:00:00 2001 From: unforcedagi Date: Wed, 29 Jul 2026 01:27:09 -0600 Subject: [PATCH 2/2] mcp login: accept a pasted callback URL as well as the redirect MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The loopback redirect only resolves on the machine running the command, and an agent host is exactly the machine nobody is sitting at. Authorizing from a laptop left the browser on a dead address while the box waited forever — hit on the first real login. Now the listener and stdin race, whichever arrives first. Paste the whole URL or just its query string; both are percent-decoded. This is the same escape hatch Claude Code offers for MCP auth. It costs nothing in security: the code is single-use, PKCE-bound and state-checked on both paths, so accepting it over the terminal is no weaker than accepting it over loopback. A callback belonging to a different login attempt is refused rather than exchanged. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01Gw5rgVt1CQ8EYrLpZtQjdM --- crates/hive-cli/src/oauth.rs | 136 +++++++++++++++++++++++++++-------- 1 file changed, 108 insertions(+), 28 deletions(-) diff --git a/crates/hive-cli/src/oauth.rs b/crates/hive-cli/src/oauth.rs index caca8ef..cfef8bb 100644 --- a/crates/hive-cli/src/oauth.rs +++ b/crates/hive-cli/src/oauth.rs @@ -317,47 +317,92 @@ pub fn authorize( // the URL above is printed first precisely so it stays usable there. let _ = Command::new("open").arg(&url).stdout(std::process::Stdio::null()).stderr(std::process::Stdio::null()).status(); } - println!("Waiting for the redirect on 127.0.0.1:{port} …"); - - let (mut sock, _) = listener.accept().context("waiting for the OAuth redirect")?; - let mut reader = BufReader::new(sock.try_clone()?); - let mut request_line = String::new(); - reader.read_line(&mut request_line)?; + println!("Waiting for the redirect on 127.0.0.1:{port}"); + println!("…or paste the callback URL here and press enter:"); + + // Two ways in, whichever arrives first. + // + // The listener alone is not enough. `redirect_uri` is loopback, so it only + // resolves on the machine running this command — and an agent host is + // exactly the machine nobody is sitting at. Authorizing from a laptop then + // leaves a browser stuck on a dead address while the box waits forever. + // + // Pasting the URL is the same escape hatch Claude Code offers for MCP + // auth, and it costs nothing: the code is single-use, PKCE-bound and + // state-checked either way, so accepting it over the terminal is no + // weaker than accepting it over loopback. + let (tx, rx) = std::sync::mpsc::channel::>>(); + + let tx_sock = tx.clone(); + std::thread::spawn(move || { + let got = (|| -> Result<(BTreeMap, std::net::TcpStream)> { + let (sock, _) = listener.accept().context("waiting for the OAuth redirect")?; + let mut reader = BufReader::new(sock.try_clone()?); + let mut line = String::new(); + reader.read_line(&mut line)?; + let query = line + .split_whitespace() + .nth(1) + .and_then(|p| p.split_once('?').map(|(_, q)| q.to_string())) + .unwrap_or_default(); + Ok((parse_query(&query), sock)) + })(); + match got { + Ok((params, mut sock)) => { + let ok = params.contains_key("code"); + let msg = if ok { "hive is authorized" } else { "Authorization failed" }; + let _ = write!( + sock, + "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nConnection: close\r\n\r\n

{msg}

You can close this tab.

" + ); + let _ = tx_sock.send(Ok(params)); + } + Err(e) => { + let _ = tx_sock.send(Err(e)); + } + } + }); - let query = request_line - .split_whitespace() - .nth(1) - .and_then(|p| p.split_once('?').map(|(_, q)| q.to_string())) - .unwrap_or_default(); - let params: BTreeMap<&str, &str> = - query.split('&').filter_map(|kv| kv.split_once('=')).collect(); + std::thread::spawn(move || { + let mut line = String::new(); + if std::io::stdin().read_line(&mut line).is_ok() { + let t = line.trim(); + if !t.is_empty() { + // Accept a whole URL or a bare query string, since what people + // copy out of an address bar is inconsistent. + let q = t.split_once('?').map(|(_, q)| q).unwrap_or(t); + let _ = tx.send(Ok(parse_query(q))); + } + } + }); - let reply = |sock: &mut std::net::TcpStream, msg: &str| { - let _ = write!( - sock, - "HTTP/1.1 200 OK\r\nContent-Type: text/html\r\nConnection: close\r\n\r\n

{msg}

You can close this tab.

" - ); - }; + let params = rx.recv().context("no callback received")??; if let Some(err) = params.get("error") { - reply(&mut sock, "Authorization failed"); bail!("authorization server refused: {err}"); } - // Compare state before touching the code: without this a redirect from an - // unrelated flow would be accepted and exchanged. - if params.get("state").map(|s| *s != state).unwrap_or(true) { - reply(&mut sock, "Authorization failed"); - bail!("state mismatch — the redirect did not belong to this login attempt"); + // Check state before touching the code: without this a redirect belonging + // to an unrelated flow would be accepted and exchanged. + if params.get("state").map(|s| s != &state).unwrap_or(true) { + bail!("state mismatch — that callback did not belong to this login attempt"); } let code = params .get("code") - .map(|c| percent_decode(c)) - .context("redirect carried no authorization code")?; - reply(&mut sock, "hive is authorized"); + .cloned() + .context("callback carried no authorization code")?; Ok((code, format!("{verifier}\u{1}{redirect_uri}"))) } +/// Percent-decoded query pairs. Owned rather than borrowed because the two +/// arrival paths (socket, stdin) own their buffers in different threads. +fn parse_query(q: &str) -> BTreeMap { + q.split('&') + .filter_map(|kv| kv.split_once('=')) + .map(|(k, v)| (percent_decode(k), percent_decode(v))) + .collect() +} + fn percent_decode(s: &str) -> String { let b = s.as_bytes(); let mut out = Vec::with_capacity(b.len()); @@ -500,6 +545,41 @@ mod tests { assert_eq!(meta_key("mcp/parachute"), "mcp/parachute+oauth"); } + #[test] + fn a_pasted_callback_parses_as_a_whole_url_or_a_bare_query() { + // People copy inconsistent things out of an address bar, and a paste + // that silently yields no code sends them looking at the server. + let want = |m: &BTreeMap| { + assert_eq!(m.get("code").map(String::as_str), Some("abc123")); + assert_eq!(m.get("state").map(String::as_str), Some("xyz")); + }; + want(&parse_query("code=abc123&state=xyz")); + want(&parse_query( + "http://127.0.0.1:51940/callback?code=abc123&state=xyz" + .split_once('?') + .unwrap() + .1, + )); + } + + #[test] + fn a_pasted_callback_is_percent_decoded() { + // Authorization codes routinely contain '/' and '+', which arrive + // escaped. Exchanging the raw form fails with "invalid_grant" against + // a code that looks correct on screen. + let m = parse_query("code=a%2Fb%2Bc&state=s"); + assert_eq!(m.get("code").map(String::as_str), Some("a/b+c")); + } + + #[test] + fn an_error_callback_is_distinguishable_from_a_successful_one() { + // The server redirects with ?error=... rather than a code; treating a + // missing code as "still waiting" would hang instead of reporting the + // refusal. + let m = parse_query("error=access_denied&state=s"); + assert!(m.contains_key("error") && !m.contains_key("code")); + } + #[test] fn percent_decoding_handles_the_plus_and_escape_forms() { assert_eq!(percent_decode("a%2Fb"), "a/b");