diff --git a/crates/didbot-agentd/fixtures/protocol/answer-pending-all.json b/crates/didbot-agentd/fixtures/protocol/answer-pending-all.json new file mode 100644 index 00000000..ae9f1bd5 --- /dev/null +++ b/crates/didbot-agentd/fixtures/protocol/answer-pending-all.json @@ -0,0 +1,38 @@ +{ + "pending": [ + { + "clientOrigin": "https://client.example", + "cut": [ + "repo:app.example.note" + ], + "expiresAt": "2026-09-22T12:34:56Z", + "firstTime": true, + "granted": [ + "atproto" + ], + "requested": [ + "atproto", + "repo:app.example.note" + ], + "rule": "scope-ceiling", + "token": "k-3f9a", + "verdict": "narrow" + } + ], + "unseen": [ + { + "clientOrigin": "https://stranger.example", + "expiresAt": "2026-09-22T12:35:10Z", + "firstTime": true, + "granted": [ + "atproto" + ], + "requested": [ + "atproto" + ], + "token": "k-7c21", + "verdict": "allow" + } + ], + "version": 2 +} diff --git a/crates/didbot-agentd/fixtures/protocol/ask-pending-all.json b/crates/didbot-agentd/fixtures/protocol/ask-pending-all.json new file mode 100644 index 00000000..f88ebeea --- /dev/null +++ b/crates/didbot-agentd/fixtures/protocol/ask-pending-all.json @@ -0,0 +1,5 @@ +{ + "all": true, + "asks": "pending", + "version": 2 +} diff --git a/crates/didbot-agentd/src/bin/didbot-oauth.rs b/crates/didbot-agentd/src/bin/didbot-oauth.rs index 04dd302e..adda809c 100644 --- a/crates/didbot-agentd/src/bin/didbot-oauth.rs +++ b/crates/didbot-agentd/src/bin/didbot-oauth.rs @@ -20,17 +20,18 @@ use didbot_agentd::cli::{ask, choose, flag, one_line, positional, printable, soc use didbot_agentd::decisions::Record; use didbot_agentd::direct::{Direct, Named, TokenFlag}; use didbot_agentd::protocol::{ - Approve, DecisionForAccount, Decline, Message, Pending, Show, VERSION, + Answer, Approve, DecisionForAccount, Decline, Message, Pending, Show, VERSION, }; const USAGE: &str = "\ -didbot oauth pending what has asked to sign in as you +didbot oauth pending sign-ins an agent here reported seeing +didbot oauth pending --all and the ones none did, each marked 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: +When a sign-in has not reached you -- no hook saw the client print its URL -- +name it by that URL instead: didbot oauth show look one up and print it didbot oauth approve --url let that one in @@ -42,9 +43,10 @@ own. A token names one request: it is single use and it is not an account. With no daemon on this host -- a CI job that provisioned itself, or a host handed a token by an operator -- set DIDBOT_PDS and DIDBOT_ACCOUNT_TOKEN (or DIDBOT_ACCOUNT_TOKEN_FILE) and the same commands talk to that server as that one -account. --direct insists on it rather than trying the socket first. Run the -binary by name, didbot-oauth, for that: `didbot oauth` hands nothing from the -environment's credentials to what it runs. +account, where pending lists every sign-in waiting for it. --direct insists on +it rather than trying the socket first. Run the binary by name, didbot-oauth, +for that: `didbot oauth` hands nothing from the environment's credentials to +what it runs. An account whose document names an OpenID Connect identity signs in with a token from that issuer instead, at the server DIDBOT_PDS names: @@ -129,7 +131,7 @@ fn main() -> ExitCode { } match command { - Some("pending") => pending(), + Some("pending") => pending(&args[1..]), Some("show") => show(&args[1..]), Some("approve") => approve(&args[1..]), Some("decline") => decline(&args[1..]), @@ -148,15 +150,21 @@ fn main() -> ExitCode { } } -fn pending() -> ExitCode { - match ask(&Message::Pending(Pending { version: VERSION })) { +/// What has asked to sign in: the sign-ins an agent here reported seeing, +/// and with `--all` the rest. +fn pending(args: &[String]) -> ExitCode { + let all = args.iter().any(|arg| arg == "--all"); + match ask(&Message::Pending(Pending { + version: VERSION, + all, + })) { Ok(answer) => { - if let Some(trouble) = answer.trouble { + if let Some(trouble) = &answer.trouble { eprintln!("didbot oauth pending: {trouble}"); return ExitCode::FAILURE; } - for decision in answer.pending.unwrap_or_default() { - println!("{}", one_line(&decision)); + for line in pending_lines(&answer) { + println!("{line}"); } ExitCode::SUCCESS } @@ -167,6 +175,22 @@ fn pending() -> ExitCode { } } +/// What starts the line of a sign-in no agent here reported seeing. +const UNSEEN: &str = "not-seen-by-any-agent-here"; + +/// The lines `pending` prints: the sign-ins an agent here reported seeing, +/// then the ones none did, each marked first so it is read before anything +/// else on its line. +fn pending_lines(answer: &Answer) -> Vec { + let seen = answer.pending.iter().flatten().map(one_line); + let unseen = answer + .unseen + .iter() + .flatten() + .map(|decision| format!("{UNSEEN} {}", one_line(decision))); + seen.chain(unseen).collect() +} + /// Look one decision up by the URL a client printed, and print it. fn show(args: &[String]) -> ExitCode { let named = positional(args) @@ -435,12 +459,43 @@ mod tests { "{command} is not in the usage" ); } + assert!(USAGE.contains("didbot oauth pending --all"), "{USAGE}"); // And nothing here asks for an account, which is the property the // whole design turns on. assert!(!USAGE.contains("--as"), "{USAGE}"); assert!(!USAGE.contains("did:"), "{USAGE}"); } + /// A sign-in no agent here reported comes after the rest, marked first + /// on its line, with its token still last to copy. + #[test] + fn a_sign_in_no_agent_here_reported_is_marked_first_on_its_line() { + let decision = |origin: &str, token: &str| DecisionForAccount { + token: Some(token.into()), + client_origin: origin.into(), + first_time: true, + requested: vec!["atproto".into()], + granted: vec!["atproto".into()], + cut: Vec::new(), + rule: None, + reason: None, + verdict: "allow".into(), + expires_at: "2026-09-23T12:04:00Z".into(), + }; + let answer = Answer::quiet() + .and_pending(vec![decision("https://app.example", "k1")]) + .and_unseen(vec![decision("https://stranger.example", "k2")]); + + let lines = pending_lines(&answer); + assert_eq!(lines.len(), 2, "{lines:?}"); + assert!(lines[0].starts_with("https://app.example "), "{lines:?}"); + assert!( + lines[1].starts_with(&format!("{UNSEEN} https://stranger.example ")), + "{lines:?}" + ); + assert!(lines[1].ends_with(" token=k2"), "{lines:?}"); + } + #[test] fn a_decline_reads_its_token_and_its_reason_from_one_line() { // The shape `didbot oauth decline --reason WHY` produces, and diff --git a/crates/didbot-agentd/src/protocol.rs b/crates/didbot-agentd/src/protocol.rs index 0f68787e..ad9c0505 100644 --- a/crates/didbot-agentd/src/protocol.rs +++ b/crates/didbot-agentd/src/protocol.rs @@ -26,6 +26,9 @@ use serde::{Deserialize, Serialize}; /// 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. +/// +/// A field added within a version is optional and left off when empty, so a +/// component that predates it reads the message as it always did. pub const VERSION: u32 = 2; /// The longest request line the daemon reads, in bytes, newline included. @@ -210,15 +213,20 @@ pub struct Show { /// A caller asking what the daemon is holding. /// -/// Answered with everything, not with one context's share. A `report` names -/// the context it is about and is answered with that context's decisions -/// only; this arrives from a command line, which names nothing the daemon -/// can check — and the ceiling on that is the user account, as -/// [`crate::socket`] already sets out. +/// Answered for every context, not one. A `report` names the context it is +/// about and is answered with that context's decisions only; this arrives +/// from a command line, which names nothing the daemon can check — and the +/// ceiling on that is the user account, as [`crate::socket`] already sets +/// out. [`Answer::pending`] carries the sign-ins their context reported +/// seeing, and [`Pending::all`] asks for the rest in [`Answer::unseen`]. #[derive(Debug, Clone, Serialize, Deserialize)] pub struct Pending { /// The wire version this message was written against. pub version: u32, + /// Whether to answer with the sign-ins their context has not reported + /// seeing, too. + #[serde(default, skip_serializing_if = "std::ops::Not::not")] + pub all: bool, } /// One sign-in decision, as an agent is shown it. @@ -286,9 +294,15 @@ pub struct Answer { /// Why there is nothing to report, when there should have been. #[serde(default, skip_serializing_if = "Option::is_none")] pub trouble: Option, - /// Sign-ins waiting on a decision, for the context that asked. + /// Sign-ins waiting on a decision: a reporting context's own that it + /// reported seeing, every one its context reported for a [`Pending`] ask, + /// or the one a [`Show`] named. #[serde(default, skip_serializing_if = "Option::is_none")] pub pending: Option>, + /// Sign-ins waiting that their context has not reported seeing, carried + /// only for a [`Pending`] ask that sets [`Pending::all`]. + #[serde(default, skip_serializing_if = "Option::is_none")] + pub unseen: Option>, /// What the approved login may do now: the request as the ceiling grants /// it, which is less than was asked for while the ceiling narrows it. #[serde(default, skip_serializing_if = "Option::is_none")] @@ -307,6 +321,7 @@ impl Answer { done: None, trouble: None, pending: None, + unseen: None, granted: None, requested: None, } @@ -349,6 +364,16 @@ impl Answer { self } + /// Carry the sign-ins their context has not reported seeing, left off + /// when there are none, as [`Answer::and_pending`] does. + #[must_use] + pub fn and_unseen(mut self, unseen: Vec) -> Self { + if !unseen.is_empty() { + self.unseen = Some(unseen); + } + self + } + /// Carry what an approval's client requested and what it was granted. #[must_use] pub fn and_granted(mut self, requested: Vec, granted: Vec) -> Self { @@ -499,6 +524,10 @@ mod tests { let Message::Pending(pending) = message else { panic!("a question"); }; + assert!( + !pending.all, + "a line without the flag asks for what was seen" + ); assert_eq!( serde_json::to_string(&Message::Pending(pending)).unwrap(), line diff --git a/crates/didbot-agentd/src/serve.rs b/crates/didbot-agentd/src/serve.rs index 87e5aa89..9d9343e4 100644 --- a/crates/didbot-agentd/src/serve.rs +++ b/crates/didbot-agentd/src/serve.rs @@ -178,15 +178,25 @@ impl Daemon { } } - /// Everything this daemon is holding, for a caller that cannot say which - /// context it is. See [`crate::protocol::Pending`]. + /// What this daemon is holding, for a caller that cannot say which + /// context it is: the sign-ins their context reported seeing, and the + /// rest only when asked for all. See [`crate::protocol::Pending`]. pub async fn pending(&self, pending: crate::protocol::Pending) -> Answer { if let Some(refusal) = too_new(pending.version) { return refusal; } - self.held.lock().await.sweep(OffsetDateTime::now_utc()); - let held = self.held.lock().await.all(); - Answer::quiet().and_pending(held.iter().map(DecisionForAccount::from).collect()) + let mut held = self.held.lock().await; + held.sweep(OffsetDateTime::now_utc()); + let seen = held.seen(); + let unseen = if pending.all { + held.unseen() + } else { + Vec::new() + }; + drop(held); + Answer::quiet() + .and_pending(seen.iter().map(DecisionForAccount::from).collect()) + .and_unseen(unseen.iter().map(DecisionForAccount::from).collect()) } /// Redeem an approval token and hand the client its code. @@ -1163,6 +1173,7 @@ mod tests { let mut stream = UnixStream::connect(&path).await.unwrap(); let mut line = serde_json::to_vec(&Message::Pending(crate::protocol::Pending { version: VERSION, + all: false, })) .unwrap(); line.push(b'\n'); @@ -1860,13 +1871,51 @@ mod tests { double.state.offer(offered(&double, "r1", "k1")); let (daemon, _scratch) = daemon_with(&double).await; until!("the poll found it", !daemon.holding().await.is_empty()); + let mut seen = report(Observed::Noted, Some("a-1")); + seen.seen_request_uris = vec!["r1".into()]; + daemon.consider(seen).await; let answer = daemon - .pending(crate::protocol::Pending { version: VERSION }) + .pending(crate::protocol::Pending { + version: VERSION, + all: false, + }) .await; let pending = answer.pending.expect("the decision"); assert_eq!(pending.len(), 1); assert_eq!(pending[0].client_origin, double.origin); + assert!(answer.unseen.is_none()); + } + + /// The command line names no context, so it is answered with what some + /// context here reported. A sign-in nobody here reported is listed only + /// when asked for all, and apart from the rest. + #[tokio::test] + async fn asking_what_is_held_leaves_out_what_no_context_reported() { + let double = double::start().await; + double.state.offer(offered(&double, "r1", "k1")); + let (daemon, _scratch) = daemon_with(&double).await; + until!("the poll found it", !daemon.holding().await.is_empty()); + + let seen_only = daemon + .pending(crate::protocol::Pending { + version: VERSION, + all: false, + }) + .await; + assert!(seen_only.pending.is_none(), "{seen_only:?}"); + assert!(seen_only.unseen.is_none(), "{seen_only:?}"); + + let all = daemon + .pending(crate::protocol::Pending { + version: VERSION, + all: true, + }) + .await; + assert!(all.pending.is_none(), "{all:?}"); + let unseen = all.unseen.expect("the unseen decision"); + assert_eq!(unseen.len(), 1); + assert_eq!(unseen[0].token.as_deref(), Some("k1")); } /// Whatever the harness invents, the name is a DNS label the server diff --git a/crates/didbot-agentd/tests/fixtures.rs b/crates/didbot-agentd/tests/fixtures.rs index cd08dcd5..04b3e7bc 100644 --- a/crates/didbot-agentd/tests/fixtures.rs +++ b/crates/didbot-agentd/tests/fixtures.rs @@ -85,6 +85,22 @@ fn narrowed_decision() -> DecisionForAccount { } } +/// One that no context reported seeing, as an ask for all lists it. +fn unreported_decision() -> DecisionForAccount { + DecisionForAccount { + token: Some("k-7c21".into()), + client_origin: "https://stranger.example".into(), + first_time: true, + requested: vec!["atproto".into()], + granted: vec!["atproto".into()], + cut: Vec::new(), + rule: None, + reason: None, + verdict: "allow".into(), + expires_at: "2026-09-22T12:35:10Z".into(), + } +} + /// Every message and every answer this crate can put on the wire. /// /// A variant with two legal ways of naming what it acts on gets one of each, @@ -134,7 +150,20 @@ fn everything() -> BTreeMap<&'static str, Value> { reason: Some("not something I asked for".into()), }), ), - ("ask-pending", Message::Pending(Pending { version: VERSION })), + ( + "ask-pending", + Message::Pending(Pending { + version: VERSION, + all: false, + }), + ), + ( + "ask-pending-all", + Message::Pending(Pending { + version: VERSION, + all: true, + }), + ), ( "ask-show", Message::Show(Show { @@ -157,6 +186,12 @@ fn everything() -> BTreeMap<&'static str, Value> { "answer-pending", Answer::quiet().and_pending(vec![narrowed_decision()]), ), + ( + "answer-pending-all", + Answer::quiet() + .and_pending(vec![narrowed_decision()]) + .and_unseen(vec![unreported_decision()]), + ), ( "answer-done", Answer::done("signed in to https://client.example as did:web:explorer-3f9a1c2d4e5f6071.pds.example")