diff --git a/crates/didbot-agentd/src/bin/didbot-oauth.rs b/crates/didbot-agentd/src/bin/didbot-oauth.rs index 8533c0f3..3b27a0fd 100644 --- a/crates/didbot-agentd/src/bin/didbot-oauth.rs +++ b/crates/didbot-agentd/src/bin/didbot-oauth.rs @@ -16,7 +16,7 @@ use std::process::ExitCode; use didbot_agentd::cli::{ask, flag, list, one_line, positional}; -use didbot_agentd::protocol::{Approve, Decline, Message, Pending, VERSION}; +use didbot_agentd::protocol::{Approve, Decline, Message, Pending, Show, VERSION}; const USAGE: &str = "\ didbot-oauth pending what has asked to sign in as you @@ -24,6 +24,13 @@ didbot-oauth approve let one of them in didbot-oauth decline [--reason WHY] turn one of them down +When a sign-in has not reached you -- no hook saw the client print its URL, +or the poll has not come back yet -- name it by that URL instead: + +didbot-oauth show look one up and print it +didbot-oauth approve --url let that one in +didbot-oauth decline --url [--reason WHY] + Nothing signs in until you choose, and a request nobody answers expires on its own. A token names one request: it is single use and it is not an account. "; @@ -32,6 +39,7 @@ fn main() -> ExitCode { let args: Vec = std::env::args().skip(1).collect(); match args.first().map(String::as_str) { Some("pending") => pending(), + Some("show") => show(&args[1..]), Some("approve") => approve(&args[1..]), Some("decline") => decline(&args[1..]), Some("--help" | "-h") | None => { @@ -64,15 +72,56 @@ fn pending() -> ExitCode { } } -fn approve(args: &[String]) -> ExitCode { - let Some(token) = positional(args) else { - eprintln!("didbot-oauth approve: needs the token from `didbot-oauth pending`"); +/// Look one decision up by the URL a client printed, and print it. +fn show(args: &[String]) -> ExitCode { + let named = positional(args) + .cloned() + .or_else(|| flag(args, "url").map(str::to_owned)); + let Some(named) = named else { + eprintln!("didbot-oauth show: needs the authorize URL a client printed"); return ExitCode::FAILURE; }; + match ask(&Message::Show(Show { + version: VERSION, + url: named, + })) { + Ok(answer) => { + if let Some(trouble) = answer.trouble { + eprintln!("didbot-oauth show: {trouble}"); + return ExitCode::FAILURE; + } + let found = answer.pending.unwrap_or_default(); + if found.is_empty() { + eprintln!("didbot-oauth show: no sign-in there"); + return ExitCode::FAILURE; + } + for decision in found { + println!("{}", one_line(&decision)); + } + ExitCode::SUCCESS + } + Err(err) => { + eprintln!("didbot-oauth show: {err}"); + ExitCode::FAILURE + } + } +} + +fn approve(args: &[String]) -> ExitCode { + let (token, url) = named(args); + if token.is_none() && url.is_none() { + eprintln!( + "didbot-oauth approve: needs the token from `didbot-oauth pending`, or --url with \ + the authorize URL a client printed" + ); + return ExitCode::FAILURE; + } + match ask(&Message::Approve(Approve { version: VERSION, - token: token.clone(), + token, + url, })) { Ok(answer) => { if let Some(trouble) = answer.trouble { @@ -95,15 +144,28 @@ fn approve(args: &[String]) -> ExitCode { } } +/// Which decision a command line names: a bare token, or `--url`. +fn named(args: &[String]) -> (Option, Option) { + ( + positional(args).cloned(), + flag(args, "url").map(str::to_owned), + ) +} + fn decline(args: &[String]) -> ExitCode { - let Some(token) = positional(args) else { - eprintln!("didbot-oauth decline: needs the token from `didbot-oauth pending`"); + let (token, url) = named(args); + if token.is_none() && url.is_none() { + eprintln!( + "didbot-oauth decline: needs the token from `didbot-oauth pending`, or --url with \ + the authorize URL a client printed" + ); return ExitCode::FAILURE; - }; + } match ask(&Message::Decline(Decline { version: VERSION, - token: token.clone(), + token, + url, reason: flag(args, "reason").map(str::to_owned), })) { Ok(answer) => { @@ -156,6 +218,38 @@ mod tests { assert_eq!(flag(&reversed, "reason"), Some("not something I asked for")); } + #[test] + fn a_decision_may_be_named_by_url_instead_of_by_token() { + let by_url = args(&[ + "--url", + "https://pds.example/oauth/authorize?request_uri=r1", + ]); + let (token, url) = named(&by_url); + assert!(token.is_none()); + assert_eq!( + url.as_deref(), + Some("https://pds.example/oauth/authorize?request_uri=r1") + ); + + // And `show` takes it either way round, since an agent pasting a URL + // should not have to remember whether it is a flag here. + assert_eq!( + positional(&args(&["urn:ietf:params:oauth:request_uri:r1"])).map(String::as_str), + Some("urn:ietf:params:oauth:request_uri:r1") + ); + } + + #[test] + fn every_way_of_naming_one_is_in_the_usage() { + for line in [ + "didbot-oauth show ", + "didbot-oauth approve --url ", + "didbot-oauth decline --url ", + ] { + assert!(USAGE.contains(line), "{line} is not in the usage"); + } + } + #[test] fn an_approve_with_nothing_to_approve_is_not_a_token() { // `didbot-oauth approve` with no argument must not send a call: there diff --git a/crates/didbot-agentd/src/decisions.rs b/crates/didbot-agentd/src/decisions.rs index 264eedf8..df19e1a6 100644 --- a/crates/didbot-agentd/src/decisions.rs +++ b/crates/didbot-agentd/src/decisions.rs @@ -133,6 +133,17 @@ impl Record { self.state != "pending" } + /// The rule that refused this, and the sentence with it, if it was. + /// + /// For the one place both belong on a single line: telling a caller why + /// the decision it named has no token to approve with. + pub fn rule_and_reason(&self) -> Option { + match &self.verdict { + Verdict::Deny { reason, rule } => Some(format!("{rule}: {reason}")), + _ => None, + } + } + /// The scopes that would actually be granted. pub fn granted(&self) -> Vec { match &self.verdict { @@ -306,6 +317,14 @@ impl Pds { self.read(self.http.post(url).json(&body), account).await } + /// The origin this daemon reaches the server on. + /// + /// Read so that an authorize URL a caller hands over can be checked + /// against it before an account's credential is sent anywhere. + pub fn base(&self) -> &str { + &self.base + } + /// Hand a code to the client waiting for it on this machine. /// /// Bounded to loopback, with no chains: see [`crate::loopback`] for why diff --git a/crates/didbot-agentd/src/double.rs b/crates/didbot-agentd/src/double.rs index 732129df..f58298d6 100644 --- a/crates/didbot-agentd/src/double.rs +++ b/crates/didbot-agentd/src/double.rs @@ -36,6 +36,8 @@ pub struct Shared { delivered: Mutex>, /// How many polls have been answered. polls: AtomicUsize, + /// Which account each credential authenticates as, when a test says so. + accounts: Mutex>, /// What was approved and declined. approved: Mutex>, declined: Mutex)>>, @@ -60,6 +62,18 @@ impl Shared { .insert(record.request_uri.clone(), record); } + /// Say which account a credential authenticates as. + /// + /// Left unset, the double hands any record to any caller, which is enough + /// for a test about something else. Set, it behaves the way the real + /// routes do: a record goes only to the account it names. + pub fn authenticates(&self, token: &str, account: &str) { + self.accounts + .lock() + .unwrap() + .insert(token.to_owned(), account.to_owned()); + } + /// Make the next approval fail with this error body. pub fn refuse(&self, status: u16, error: &str, message: &str) { *self.refusal.lock().unwrap() = Some((status, error.to_owned(), message.to_owned())); @@ -179,15 +193,29 @@ async fn one( Query(query): Query>, headers: HeaderMap, ) -> Result, (StatusCode, Json)> { - presented(&state, &headers).ok_or_else(unauthenticated)?; + let presented_as = presented(&state, &headers).ok_or_else(unauthenticated)?; let wanted = query.get("requestUri").cloned().unwrap_or_default(); - match state.known.lock().unwrap().get(&wanted) { - Some(record) => Ok(Json(serde_json::to_value(record).unwrap())), - None => Err(( + let found = state.known.lock().unwrap().get(&wanted).cloned(); + let Some(record) = found else { + return Err(( StatusCode::BAD_REQUEST, Json(json!({ "error": "NotFound", "message": "no such request" })), - )), + )); + }; + + // A record goes only to the account it names, which is what makes trying + // each held account safe: at most one of them can succeed. + let accounts = state.accounts.lock().unwrap(); + if !accounts.is_empty() && accounts.get(&presented_as) != Some(&record.account) { + return Err(( + StatusCode::BAD_REQUEST, + Json(json!({ + "error": "NotFound", + "message": "no such request for this account", + })), + )); } + Ok(Json(serde_json::to_value(&record).unwrap())) } async fn approve( diff --git a/crates/didbot-agentd/src/protocol.rs b/crates/didbot-agentd/src/protocol.rs index 4459d323..a6f4d52d 100644 --- a/crates/didbot-agentd/src/protocol.rs +++ b/crates/didbot-agentd/src/protocol.rs @@ -22,7 +22,7 @@ use serde::{Deserialize, Serialize}; /// /// Version 2 added the sign-in decisions: [`Answer::pending`], /// [`Report::seen_request_uris`], and the [`Message::Approve`], -/// [`Message::Decline`] and [`Message::Pending`] asks. Everything version 1 +/// [`Message::Decline`], [`Message::Pending`] and [`Message::Show`] asks. Everything version 1 /// sent is still read and answered — see `serve::Daemon::consider`, which /// refuses a version newer than its own and nothing else — and an adapter /// still on version 1 finds the fields it knows where they were. @@ -46,6 +46,8 @@ pub enum Message { Decline(Decline), /// A caller asking what the daemon is holding. Pending(Pending), + /// A caller asking about one decision by name. + Show(Show), } impl<'de> Deserialize<'de> for Message { @@ -65,6 +67,9 @@ impl<'de> Deserialize<'de> for Message { Some("pending") => serde_json::from_value(value) .map(Message::Pending) .map_err(D::Error::custom), + Some("show") => serde_json::from_value(value) + .map(Message::Show) + .map_err(D::Error::custom), Some(other) => Err(D::Error::custom(format!( "this daemon does not know how to `{other}`" ))), @@ -136,18 +141,25 @@ pub struct Report { /// An agent approving a sign-in the daemon put in front of it. /// -/// One field, and it is not an account. The token was minted by the server -/// against one pushed request for one account, and the daemon knows which -/// because it is holding the record the token came in. So there is nothing -/// here for a caller to name wrongly: a token this daemon is not holding is -/// refused, and one it is holding is approved as the account that record -/// says, never as an account the caller asked for. +/// Names a decision, and never an account. Either the one-time `token` from a +/// decision the daemon already showed it, or the `url` of an authorize page +/// for one it has not -- the manual path, for a request no poll had delivered +/// and no tool output was seen to carry. Both end in the same place: the +/// daemon resolves them to a record it holds and acts as the account that +/// record names, never as an account the caller asked for. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Approve { /// The wire version this message was written against. pub version: u32, /// The one-time approval token from the decision. - pub token: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub token: Option, + /// An authorize URL, or a bare `request_uri`, naming the decision. + /// + /// Exactly one of this and [`Approve::token`]: a message carrying neither + /// names nothing, and one carrying both is refused rather than guessed at. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub url: Option, } /// An agent refusing one. @@ -160,12 +172,36 @@ pub struct Decline { /// The wire version this message was written against. pub version: u32, /// The one-time approval token from the decision. - pub token: String, + #[serde(default, skip_serializing_if = "Option::is_none")] + pub token: Option, + /// An authorize URL, or a bare `request_uri`, naming the decision. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub url: Option, /// Why, in the agent's own words, for the record. #[serde(default, skip_serializing_if = "Option::is_none")] pub reason: Option, } +/// A caller asking about one decision by name, rather than waiting to be told. +/// +/// The daemon hears about a sign-in by polling, and an adapter can hand it a +/// `request_uri` it saw go past in tool output. Both can miss: a client that +/// printed its URL where no hook read it, or a poll that has not come back +/// yet. This is that same lookup made deliberate -- asked for by an agent +/// holding the URL, rather than by the daemon noticing it. +/// +/// It is a fetch, not a way around anything. The record still comes from +/// `bot.did.getAuthorization` under an account's own credential, and the +/// server hands a record only to the account it belongs to, so a URL naming +/// somebody else's sign-in resolves to nothing here. +#[derive(Debug, Clone, Serialize, Deserialize)] +pub struct Show { + /// The wire version this message was written against. + pub version: u32, + /// An authorize URL, or a bare `request_uri`. + pub url: String, +} + /// A caller asking what the daemon is holding. /// /// Answered with everything, not with one context's share. A `report` names @@ -374,13 +410,44 @@ mod tests { let Message::Approve(approve) = message else { panic!("an approval"); }; - assert_eq!(approve.token, "k7f3"); + assert_eq!(approve.token.as_deref(), Some("k7f3")); + assert!(approve.url.is_none()); assert_eq!( serde_json::to_string(&Message::Approve(approve)).unwrap(), line ); } + #[test] + fn or_the_url_of_one_it_has_not_been_shown() { + let line = r#"{"asks":"approve","version":2,"url":"https://pds.example/oauth/authorize?request_uri=r1"}"#; + let Message::Approve(approve) = serde_json::from_str::(line).unwrap() else { + panic!("an approval"); + }; + assert!(approve.token.is_none()); + assert_eq!( + approve.url.as_deref(), + Some("https://pds.example/oauth/authorize?request_uri=r1") + ); + // Still no account anywhere on the wire, which is the property that + // survives adding a second way to name a decision. + assert!(!line.contains("did:")); + assert_eq!( + serde_json::to_string(&Message::Approve(approve)).unwrap(), + line + ); + } + + #[test] + fn asking_about_one_by_name_carries_the_url_and_a_version() { + let line = r#"{"asks":"show","version":2,"url":"urn:ietf:params:oauth:request_uri:r1"}"#; + let Message::Show(show) = serde_json::from_str::(line).unwrap() else { + panic!("a lookup"); + }; + assert_eq!(show.url, "urn:ietf:params:oauth:request_uri:r1"); + assert_eq!(serde_json::to_string(&Message::Show(show)).unwrap(), line); + } + #[test] fn a_refusal_may_say_why_and_need_not() { let bare = r#"{"asks":"decline","version":2,"token":"k7f3"}"#; @@ -388,6 +455,7 @@ mod tests { panic!("a refusal"); }; assert!(decline.reason.is_none()); + assert_eq!(decline.token.as_deref(), Some("k7f3")); assert_eq!( serde_json::to_string(&Message::Decline(decline)).unwrap(), bare diff --git a/crates/didbot-agentd/src/serve.rs b/crates/didbot-agentd/src/serve.rs index 747c71d3..44a41306 100644 --- a/crates/didbot-agentd/src/serve.rs +++ b/crates/didbot-agentd/src/serve.rs @@ -68,7 +68,60 @@ impl Daemon { if let Some(refusal) = too_new(approve.version) { return refusal; } - self.redeem(&approve.token).await + match self.token_named(approve.token, approve.url).await { + Ok(token) => self.redeem(&token).await, + Err(why) => Answer::trouble(why), + } + } + + /// Show one decision by name, fetching it if no poll has delivered it. + pub async fn show(&self, show: crate::protocol::Show) -> Answer { + if let Some(refusal) = too_new(show.version) { + return refusal; + } + match self.find(&show.url).await { + Ok(holding) => { + info!(request_uri = %holding.record.request_uri, "showed a decision"); + Answer::quiet().and_pending(vec![DecisionForAgent::from(&holding.record)]) + } + Err(why) => Answer::trouble(why), + } + } + + /// The approval token a caller named, however they named it. + /// + /// Exactly one of the two. Both is refused rather than resolved and + /// checked for agreement: a caller that sent both does not know which it + /// meant, and picking one for them would hide that. + async fn token_named( + &self, + token: Option, + url: Option, + ) -> Result { + match (token, url) { + (Some(_), Some(_)) => { + Err("name a decision by its token or by its URL, not both".to_owned()) + } + (Some(token), None) => Ok(token), + (None, Some(url)) => { + let holding = self.find(&url).await?; + holding.record.token.clone().ok_or_else(|| { + format!( + "that request was refused by policy, so there is no token to approve \ + or decline with: {}", + holding + .record + .rule_and_reason() + .unwrap_or_else(|| "no reason given".to_owned()) + ) + }) + } + (None, None) => Err( + "name a decision: a token from `didbot-oauth pending`, or --url with the \ + authorize URL a client printed" + .to_owned(), + ), + } } /// Decline one, so the refusal is recorded rather than left to expire. @@ -76,12 +129,16 @@ impl Daemon { if let Some(refusal) = too_new(decline.version) { return refusal; } - let Some((pds, account, holding)) = self.holder_of(&decline.token).await else { - return Answer::trouble(unknown_token(&decline.token)); + let token = match self.token_named(decline.token, decline.url).await { + Ok(token) => token, + Err(why) => return Answer::trouble(why), + }; + let Some((pds, account, holding)) = self.holder_of(&token).await else { + return Answer::trouble(unknown_token(&token)); }; match pds - .decline(&account, &decline.token, decline.reason.as_deref()) + .decline(&account, &token, decline.reason.as_deref()) .await { Ok(declined) => { @@ -207,6 +264,7 @@ impl Daemon { Ok(Message::Approve(approve)) => self.approve(approve).await, Ok(Message::Decline(decline)) => self.decline(decline).await, Ok(Message::Pending(pending)) => self.pending(pending).await, + Ok(Message::Show(show)) => self.show(show).await, Err(err) => { warn!(error = %err, "unreadable message"); Answer::trouble(format!("unreadable message: {err}")) @@ -306,9 +364,9 @@ impl Daemon { if seen.is_empty() { return; } - let Some(pds) = self.decisions.as_ref() else { + if self.decisions.is_none() { return; - }; + } let Some(did) = self .contexts .lock() @@ -323,22 +381,135 @@ impl Daemon { }; for request_uri in seen { - if self.held.lock().await.by_request_uri(request_uri).is_some() { - continue; + if let Err(err) = self.fetch_one(key, &account, request_uri).await { + // Not the caller's trouble: the report itself succeeded, and + // a decision that could not be fetched arrives on the next + // poll or not at all. + debug!(%request_uri, error = %err, "could not fetch a request the adapter saw"); } - match pds.get(&account, request_uri).await { - Ok(record) => { - let now = OffsetDateTime::now_utc(); - self.held.lock().await.keep(key.clone(), record, now); - } - Err(err) => { - // Not the caller's trouble: the report itself succeeded, - // and a decision that could not be fetched arrives on the - // next poll or not at all. - debug!(%request_uri, error = %err, "could not fetch a request the adapter saw"); + } + } + + /// Fetch one record as one account and hold it for that account's context. + /// + /// The whole of the fast path, and of the manual one behind + /// [`crate::protocol::Show`]: the two differ only in how they arrive at + /// an account to ask as. A record already held costs nothing. + async fn fetch_one( + &self, + key: &Key, + account: &Account, + request_uri: &str, + ) -> Result<(), crate::decisions::Trouble> { + if self.held.lock().await.by_request_uri(request_uri).is_some() { + return Ok(()); + } + let Some(pds) = self.decisions.as_ref() else { + return Err(crate::decisions::Trouble::Unreachable( + "this daemon has no server to ask".to_owned(), + )); + }; + let record = pds.get(account, request_uri).await?; + let now = OffsetDateTime::now_utc(); + self.held.lock().await.keep(key.clone(), record, now); + Ok(()) + } + + /// The `request_uri` a caller named, from a URL or from itself. + /// + /// An authorize URL has to be this deployment's own. That check is not + /// about trust in the record -- the server decides that, and hands one + /// only to the account it belongs to -- but about where this daemon is + /// willing to send an account's credential. A bare `request_uri` reaches + /// no origin at all and needs no such check. + fn request_uri_in(&self, named: &str) -> Result { + let Ok(url) = url::Url::parse(named) else { + // Not a URL of any kind: take it for the identifier it claims to + // be and let the server say whether it knows one. + return Ok(named.to_owned()); + }; + if !matches!(url.scheme(), "http" | "https") { + // `urn:ietf:params:oauth:request_uri:...` parses, and is the + // identifier itself rather than somewhere to go. + return Ok(named.to_owned()); + } + + let Some(pds) = self.decisions.as_ref() else { + return Err("this daemon has no server to ask".to_owned()); + }; + let ours = url::Url::parse(pds.base()) + .map_err(|err| format!("this daemon's own server is unusable: {err}"))?; + if url.origin() != ours.origin() { + return Err(format!( + "that URL is at {}, and this daemon signs in only at {}", + url.origin().ascii_serialization(), + ours.origin().ascii_serialization() + )); + } + + url.query_pairs() + .find(|(name, _)| name == "request_uri") + .map(|(_, value)| value.into_owned()) + .ok_or_else(|| format!("{url} carries no request_uri")) + } + + /// The record a caller named, fetched if this daemon is not holding it. + /// + /// Which account to ask as is not the caller's to say. If the daemon is + /// already holding the record, the account is the one it was held for; + /// otherwise every account this daemon has is tried, and the server + /// answers for exactly the one the record belongs to -- so the account is + /// still decided by the record, and still never by the caller. + async fn find(&self, named: &str) -> Result { + let request_uri = self.request_uri_in(named)?; + + if let Some(holding) = self.held.lock().await.by_request_uri(&request_uri).cloned() { + return Ok(holding); + } + + let mine: Vec = self + .contexts + .lock() + .await + .all() + .into_iter() + .cloned() + .collect(); + let mut asked = 0; + let mut unreachable = None; + for context in mine { + let (Some(did), Some(token)) = (context.did.clone(), context.token.clone()) else { + continue; + }; + asked += 1; + let account = Account { did, token }; + match self.fetch_one(&context.key, &account, &request_uri).await { + Ok(()) => { + if let Some(holding) = + self.held.lock().await.by_request_uri(&request_uri).cloned() + { + return Ok(holding); + } } + // A refusal is the ordinary answer for every account but the + // one the record names, so it is not worth reporting on its + // own. A server that could not be reached at all is. + Err(crate::decisions::Trouble::Unreachable(why)) => unreachable = Some(why), + Err(err) => debug!(%request_uri, error = %err, "not this account's sign-in"), } } + + if let Some(why) = unreachable { + return Err(format!("could not reach the server: {why}")); + } + if asked == 0 { + return Err( + "this daemon holds no account yet, so there is nothing to ask as".to_owned(), + ); + } + Err(format!( + "no account this daemon holds has a sign-in for {request_uri}" + )) } /// Start following one account's sign-ins, outbound. @@ -692,7 +863,8 @@ mod tests { let answer = daemon .approve(Approve { version: VERSION, - token: "k1".into(), + token: Some("k1".into()), + url: None, }) .await; @@ -716,7 +888,8 @@ mod tests { let answer = daemon .approve(Approve { version: VERSION, - token: "never-issued".into(), + token: Some("never-issued".into()), + url: None, }) .await; @@ -739,7 +912,8 @@ mod tests { let answer = daemon .approve(Approve { version: VERSION, - token: "k1".into(), + token: Some("k1".into()), + url: None, }) .await; @@ -759,7 +933,8 @@ mod tests { let answer = daemon .decline(Decline { version: VERSION, - token: "k1".into(), + token: Some("k1".into()), + url: None, reason: Some("not something I asked for".into()), }) .await; @@ -791,6 +966,197 @@ mod tests { assert_eq!(pending[0].token.as_deref(), Some("k1")); } + #[tokio::test] + async fn a_sign_in_no_poll_delivered_can_be_looked_up_by_its_url() { + let double = double::start().await; + // Known to the server but never offered to a poll: the case where the + // hook saw nothing and the long poll has not come back. + double.state.know(offered(&double, "r1", "k1")); + let daemon = daemon_with(&double).await; + assert!(daemon.holding().await.is_empty()); + + let url = format!("{}/oauth/authorize?request_uri=r1", double.origin); + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: url.clone(), + }) + .await; + + assert!(shown.trouble.is_none(), "{shown:?}"); + let found = shown.pending.expect("the decision"); + assert_eq!(found.len(), 1); + assert_eq!(found[0].token.as_deref(), Some("k1")); + // Looking it up holds it, so it is answerable from then on. + assert_eq!(daemon.holding().await.len(), 1); + + // And it can be approved by that same URL, without the agent ever + // having been handed the token. + let answer = daemon + .approve(Approve { + version: VERSION, + token: None, + url: Some(url), + }) + .await; + assert!(answer.trouble.is_none(), "{answer:?}"); + assert_eq!(double.state.approved(), vec!["k1".to_owned()]); + until!("the code arrived", !double.state.delivered().is_empty()); + } + + #[tokio::test] + async fn a_bare_request_uri_needs_no_url_at_all() { + let double = double::start().await; + double.state.know(offered( + &double, + "urn:ietf:params:oauth:request_uri:r1", + "k1", + )); + let daemon = daemon_with(&double).await; + + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: "urn:ietf:params:oauth:request_uri:r1".into(), + }) + .await; + + assert!(shown.trouble.is_none(), "{shown:?}"); + assert_eq!(shown.pending.expect("the decision").len(), 1); + } + + #[tokio::test] + async fn a_url_somewhere_else_is_refused_before_a_credential_leaves() { + let double = double::start().await; + double.state.know(offered(&double, "r1", "k1")); + let daemon = daemon_with(&double).await; + until!("the poll happened", double.state.polls() > 0); + let before = double.state.presented().len(); + + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: "https://not-ours.example/oauth/authorize?request_uri=r1".into(), + }) + .await; + + let trouble = shown.trouble.expect("a refusal"); + assert!(trouble.contains("not-ours.example"), "{trouble}"); + assert!(trouble.contains(&double.origin), "{trouble}"); + // Nothing was sent anywhere: the account's credential does not go to + // an origin this daemon does not serve. + assert_eq!(double.state.presented().len(), before); + } + + #[tokio::test] + async fn a_sign_in_for_an_account_this_daemon_does_not_hold_resolves_to_nothing() { + let double = double::start().await; + // The record exists and names an account that is not this daemon's, + // and the double enforces what the real route does: a record goes + // only to the account it names. + double.state.authenticates("agent-token", MINTED); + let mut theirs = double::record( + "r-theirs", + "did:web:somebody.else", + Some("k-theirs"), + &double::later(), + ); + theirs.client.origin = double.origin.clone(); + double.state.know(theirs); + let daemon = daemon_with(&double).await; + + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: format!("{}/oauth/authorize?request_uri=r-theirs", double.origin), + }) + .await; + + let trouble = shown.trouble.expect("a refusal"); + assert!(trouble.contains("r-theirs"), "{trouble}"); + // Not held, and no token from it reached this daemon to be spent. + assert!(daemon.holding().await.is_empty()); + assert!(double.state.approved().is_empty()); + } + + #[tokio::test] + async fn and_one_for_an_account_it_does_hold_is_found_among_several() { + let double = double::start().await; + double.state.authenticates("agent-token", MINTED); + double.state.know(offered(&double, "r-mine", "k-mine")); + let daemon = daemon_with(&double).await; + // A second context, so `find` has more than one account to try. + daemon.consider(report(Observed::Began, Some("a-2"))).await; + + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: format!("{}/oauth/authorize?request_uri=r-mine", double.origin), + }) + .await; + + assert!(shown.trouble.is_none(), "{shown:?}"); + assert_eq!( + shown.pending.expect("the decision")[0].token.as_deref(), + Some("k-mine") + ); + } + + #[tokio::test] + async fn a_url_with_no_request_uri_on_it_is_said_to_carry_none() { + let double = double::start().await; + let daemon = daemon_with(&double).await; + + let shown = daemon + .show(crate::protocol::Show { + version: VERSION, + url: format!("{}/oauth/authorize", double.origin), + }) + .await; + + assert!( + shown.trouble.expect("a refusal").contains("no request_uri"), + "should say what is missing" + ); + } + + #[tokio::test] + async fn naming_a_decision_twice_over_is_refused_rather_than_guessed_at() { + let double = double::start().await; + double.state.offer(offered(&double, "r1", "k1")); + let daemon = daemon_with(&double).await; + until!("the poll found it", !daemon.holding().await.is_empty()); + + let answer = daemon + .approve(Approve { + version: VERSION, + token: Some("k1".into()), + url: Some(format!("{}/oauth/authorize?request_uri=r1", double.origin)), + }) + .await; + + assert!(answer.trouble.is_some(), "{answer:?}"); + assert!(double.state.approved().is_empty()); + } + + #[tokio::test] + async fn naming_none_is_refused_with_both_ways_of_naming_one() { + let double = double::start().await; + let daemon = daemon_with(&double).await; + + let trouble = daemon + .approve(Approve { + version: VERSION, + token: None, + url: None, + }) + .await + .trouble + .expect("a refusal"); + assert!(trouble.contains("token"), "{trouble}"); + assert!(trouble.contains("--url"), "{trouble}"); + } + #[tokio::test] async fn asking_what_is_held_answers_from_memory() { let double = double::start().await; diff --git a/docs/agentd.md b/docs/agentd.md index 724cc969..53c21760 100644 --- a/docs/agentd.md +++ b/docs/agentd.md @@ -203,8 +203,15 @@ loopback on this host — so it answers with the redirect and the daemon fetches it. That fetch is bounded to loopback, with redirects turned off, as is every request the daemon makes on something the model influenced. -`didbot-oauth` is the only agent-facing command, and every command in it names -a token. There is no `--as ` anywhere in it, and nothing left that takes +When neither path reached the agent — no hook saw the client print its URL, +and the poll has not come back — `didbot-oauth show ` looks one up by +that URL, and `approve --url` and `decline --url` answer it; the daemon +refuses a URL on any origin but this deployment's, then fetches the record +through `bot.did.getAuthorization` as whichever of its accounts the record +names, which is the same authenticated fetch the fast path makes. + +`didbot-oauth` is the only agent-facing command, and everything in it names a +decision — a token, or the URL of one. There is no `--as ` anywhere in it, and nothing left that takes an account from its caller: the daemon reads the account off the record it is holding the token in. That is what closes the gap the socket section above describes — reaching the socket is still the authorization, and anything diff --git a/plan/oauth.md b/plan/oauth.md index 85203f65..17b63c72 100644 --- a/plan/oauth.md +++ b/plan/oauth.md @@ -49,9 +49,14 @@ Two tool calls, and no browser anywhere: code. 4. The client exchanges it and holds tokens bound to its own key. -The agent never handles a URL and never names its account. `didbot-oauth` is -the only agent-facing command, and there is no `--as ` in it: the daemon -reads the account off the record it holds the token in. The `didbot confirm` +An agent that does hold a URL for a request neither path delivered can name it +directly — `didbot-oauth show `, and `approve --url` — which is the same +authenticated `getAuthorization` fetch, asked for rather than noticed. + +On the ordinary path the agent never handles a URL, and on neither path does +it name its account. `didbot-oauth` is the only agent-facing command, and +there is no `--as ` in it: the daemon reads the account off the record, +whether the agent named that record by token or by URL. The `didbot confirm` command that used to take one, and the `confirm` ask on the socket it sent, are both gone.