From b093c8e7acaf1d5bd81f7d64d979cef3cdfbdc82 Mon Sep 17 00:00:00 2001 From: "@permadeath.com" Date: Tue, 15 Sep 2026 09:11:51 -0400 Subject: [PATCH] feat(serve): make the operator session lifetime configurable --operator-session-ttl and [operator] session_ttl_secs, resolved the way every other setting is. The section's doc carries the range worth staying inside: five minutes to a day, defaulting to twelve hours. Also drops a stale [estop] entry from plan/config.md, left behind when that section was removed. Co-Authored-By: Claude Opus 5 (1M context) --- crates/didbot-config/src/lib.rs | 6 +- crates/didbot-config/src/sections.rs | 24 +++++++ crates/didbot-serve/src/bin/didbot-pds.rs | 85 ++++++++++++++++++++++- crates/didbot-serve/src/dashboard.rs | 2 +- crates/didbot-serve/src/operator.rs | 27 +++++-- crates/didbot-serve/src/tests.rs | 1 + plan/config.md | 9 ++- 7 files changed, 142 insertions(+), 12 deletions(-) diff --git a/crates/didbot-config/src/lib.rs b/crates/didbot-config/src/lib.rs index e8d009ff..23749b11 100644 --- a/crates/didbot-config/src/lib.rs +++ b/crates/didbot-config/src/lib.rs @@ -88,7 +88,8 @@ use serde::{Deserialize, Serialize}; use sections::{ BlobsSection, CapacitySection, DisclosureSection, DnsSection, LimitsSection, NamesSection, - OAuthSection, PolicySection, RelaySection, ServerSection, TlsSection, ZoneSection, + OAuthSection, OperatorSection, PolicySection, RelaySection, ServerSection, TlsSection, + ZoneSection, }; /// The whole file, one optional table per section. @@ -127,6 +128,9 @@ pub struct Config { /// `[disclosure]` — which `bot.did.*` routes this run has closed. #[serde(default)] pub disclosure: Option, + /// `[operator]` — the operator sign-in's session lifetime. + #[serde(default)] + pub operator: Option, /// `[oauth]` — the scope ceiling and the pending-decision bounds. #[serde(default)] pub oauth: Option, diff --git a/crates/didbot-config/src/sections.rs b/crates/didbot-config/src/sections.rs index bc9fe9fe..ccd240db 100644 --- a/crates/didbot-config/src/sections.rs +++ b/crates/didbot-config/src/sections.rs @@ -190,6 +190,30 @@ pub struct LimitsSection { pub login_attempts: Option, } +/// The operator's own sign-in and the session it yields. +/// +/// Reload: needs a restart. The TTL is read once when the sign-in is built, +/// and a session already issued keeps the lifetime it was issued with. +#[derive(Debug, Clone, Default, PartialEq, Eq, Serialize, Deserialize)] +#[serde(deny_unknown_fields)] +pub struct OperatorSection { + /// How long a signed-in operator stays signed in, in seconds. Mirrors + /// `--operator-session-ttl`. + /// + /// **Reasonable range: 300 to 86_400** — five minutes to a day. + /// + /// Below five minutes an operator is signing in again mid-incident, + /// which is the one time the round trip to their own server is least + /// welcome. Above a day the window in which a borrowed browser stays + /// the operator outlives any sitting a person actually had. The + /// built-in default is twelve hours: one working day, so a sign-in in + /// the morning does not expire before the evening. + /// + /// Nothing enforces the range — it is documented for whoever sets this, + /// not checked — but a value outside it is worth a second thought. + pub session_ttl_secs: Option, +} + /// Which `bot.did.*` routes this deployment has closed to a public, /// unauthenticated caller. /// diff --git a/crates/didbot-serve/src/bin/didbot-pds.rs b/crates/didbot-serve/src/bin/didbot-pds.rs index 9f00cffc..5270e01f 100644 --- a/crates/didbot-serve/src/bin/didbot-pds.rs +++ b/crates/didbot-serve/src/bin/didbot-pds.rs @@ -182,6 +182,10 @@ struct Args { /// zone with a `--route53-zone-id` therefore has to be given one; see /// the refusal in `run`. record_target: Option, + /// `--operator-session-ttl`, in seconds, if given; resolved against + /// `[operator]` and the built-in default by + /// [`resolve_operator_session_ttl`]. + operator_session_ttl_secs: Option, /// The relay this server announces itself to when an operator asks. /// `None` — no flag, no `[relay]` section — leaves nothing to announce /// to; see `didbot_serve::relay`. @@ -236,6 +240,7 @@ impl Default for Args { port: DEFAULT_PORT, zones: vec![DEFAULT_ZONE.to_owned()], owner: DEFAULT_OWNER.to_owned(), + operator_session_ttl_secs: None, names: Names::default(), hold: None, log_budget: None, @@ -384,6 +389,10 @@ usage: didbot-pds [options] --admission-depth how many parent links a claim's chain may cross before it must reach a root this server trusts (default 4). + --operator-session-ttl + how long a signed-in operator stays signed in (default + 43200, twelve hours; 300 to 86400 is the range worth + staying inside -- see [operator] in the config docs). --close-disclosure close one or more disclosure routes: a comma-separated list drawn from `list-agents`, `list-agent-ledgers`, @@ -537,6 +546,25 @@ fn resolve_blobs(args: &Args, config: Option<&didbot_config::Config>) -> BlobLim } } +/// Resolves how long an operator's session lasts: command line, then the +/// config file's `[operator]` section, then this binary's own built-in +/// default -- `didbot_config::precedence`'s order, the same as +/// [`resolve_blobs`]. +fn resolve_operator_session_ttl( + args: &Args, + config: Option<&didbot_config::Config>, +) -> std::time::Duration { + didbot_config::precedence( + args.operator_session_ttl_secs, + config + .and_then(|c| c.operator.as_ref()) + .and_then(|operator| operator.session_ttl_secs), + None, + ) + .map(std::time::Duration::from_secs) + .unwrap_or(didbot_serve::operator::DEFAULT_SESSION_TTL) +} + /// Resolves the account cap this run applies: command line, then the config /// file's `[capacity]` section, then [`AuthState::default`]'s own built-in /// ceiling -- `didbot_config::precedence`'s order, the same as @@ -940,6 +968,12 @@ fn parse_args>(mut args: I) -> Result { })?; parsed.admission_depth = Some(edges); } + "--operator-session-ttl" => { + let raw = value()?; + parsed.operator_session_ttl_secs = Some(raw.parse().map_err(|_| { + format!("--operator-session-ttl: `{raw}` is not a whole number of seconds") + })?); + } "--close-disclosure" => { let raw = value()?; close_disclosure(&mut parsed.disclosure, &raw)?; @@ -1605,7 +1639,11 @@ async fn run(mut args: Args) -> Result<(), String> { let operator = if args.owner == DEFAULT_OWNER { None } else { - match didbot_serve::operator::OperatorAuth::new(&args.owner, &pds_endpoint) { + match didbot_serve::operator::OperatorAuth::new( + &args.owner, + &pds_endpoint, + resolve_operator_session_ttl(&args, config.as_ref()), + ) { Ok(operator) => Some(operator), Err(err) => { warn!(%err, "operator sign-in is not available on this run"); @@ -2719,6 +2757,51 @@ mod config_tests { assert_eq!(blobs.account_quota_bytes, 2048); } + /// The session lifetime follows the same precedence every other + /// setting does, and — the part worth a test rather than a reading — + /// the resolved value reaches the sign-in that issues sessions. + /// + /// `[oauth]`'s own defect is the precedent: a section that parsed and + /// then went nowhere, because the thing it configured was built with + /// `..Default::default()`. Asserting on `resolve_*` alone would not + /// have caught that, so this asserts on what `OperatorAuth` reports + /// holding. + #[test] + fn the_configured_session_lifetime_reaches_the_sign_in() { + let dir = TempDir::new("operator-ttl"); + let path = dir.write("didbot.toml", "[operator]\nsession_ttl_secs = 1800\n"); + let config = load_config(Some(&path)).unwrap(); + + // The file alone. + let from_file = resolve_operator_session_ttl(&Args::default(), config.as_ref()); + assert_eq!(from_file, std::time::Duration::from_secs(1800)); + + // The flag beats it. + let args = Args { + operator_session_ttl_secs: Some(600), + ..Args::default() + }; + let from_flag = resolve_operator_session_ttl(&args, config.as_ref()); + assert_eq!(from_flag, std::time::Duration::from_secs(600)); + + // Neither: the built-in default, not zero. + let fallback = resolve_operator_session_ttl(&Args::default(), None); + assert_eq!(fallback, didbot_serve::operator::DEFAULT_SESSION_TTL); + + // And it lands where sessions are actually issued from. + let operator = didbot_serve::operator::OperatorAuth::new( + "did:web:operator.pds.example", + "https://kestrel.example", + from_flag, + ) + .expect("the sign-in builds"); + assert_eq!( + operator.session_ttl(), + std::time::Duration::from_secs(600), + "a lifetime nothing reads is a setting that does not exist" + ); + } + /// **The e2e defect.** `[oauth]` parsed and then went nowhere: the /// router was built with `..OAuthState::default()`, which hardwires /// `GrantAnyScope`, so a ceiling an operator wrote down did not bind. diff --git a/crates/didbot-serve/src/dashboard.rs b/crates/didbot-serve/src/dashboard.rs index 814b51d3..e3528e10 100644 --- a/crates/didbot-serve/src/dashboard.rs +++ b/crates/didbot-serve/src/dashboard.rs @@ -353,7 +353,7 @@ async fn callback( let cookie = format!( "{SESSION_COOKIE}={}; HttpOnly; Secure; SameSite=Lax; Path=/dashboard; Max-Age={}", signed_in.token, - crate::operator::SESSION_TTL.as_secs(), + operator.session_ttl().as_secs(), ); // A command line started this one and is waiting on a loopback // address; hand it the session there rather than rendering it. diff --git a/crates/didbot-serve/src/operator.rs b/crates/didbot-serve/src/operator.rs index 7d81001a..f02ef8b4 100644 --- a/crates/didbot-serve/src/operator.rs +++ b/crates/didbot-serve/src/operator.rs @@ -22,7 +22,7 @@ //! //! What this server does issue is its own session, which says only "the //! browser holding this proved control of the owner DID at some point in -//! the last [`SESSION_TTL`]". It is minted here, it never leaves this +//! the last [`OperatorAuth::session_ttl`]". It is minted here, it never leaves this //! process, and it is not a credential any other party accepts. //! //! # `bot.did.operator` is a different question @@ -47,11 +47,13 @@ use jacquard_oauth::session::ClientData; use jacquard_oauth::types::{AuthorizeOptions, CallbackParams}; use smol_str::SmolStr; -/// How long a signed-in browser stays signed in. +/// How long a signed-in browser stays signed in, absent +/// `--operator-session-ttl` or an `[operator]` section. /// -/// Short, because the way back is cheap: signing in again is a redirect the -/// operator's own server answers, not a credential anybody has to find. -pub const SESSION_TTL: Duration = Duration::from_secs(12 * 60 * 60); +/// One working day, so a sign-in in the morning has not expired by the +/// evening. See `didbot_config::sections::OperatorSection` for the range +/// worth staying inside and why. +pub const DEFAULT_SESSION_TTL: Duration = Duration::from_secs(12 * 60 * 60); /// The one scope this server ever asks an operator's server for. const SIGN_IN_SCOPE: &str = "atproto"; @@ -118,6 +120,7 @@ pub enum SignInError { /// The operator sign-in for one deployment. pub struct OperatorAuth { owner_did: String, + session_ttl: Duration, client: OAuthClient, MemoryAuthStore>, sessions: Mutex>, /// Sign-ins started but not yet come back, keyed by the `state` this @@ -135,7 +138,11 @@ impl OperatorAuth { /// slash — the authorization server fetches the metadata document from /// it and checks the redirect against it, so it has to be the address a /// browser actually reached. - pub fn new(owner_did: impl Into, base_url: &str) -> Result, String> { + pub fn new( + owner_did: impl Into, + base_url: &str, + session_ttl: Duration, + ) -> Result, String> { let owner_did = owner_did.into(); let client_id = format!("{base_url}{CLIENT_METADATA_PATH}") .parse() @@ -153,6 +160,7 @@ impl OperatorAuth { ); Ok(Arc::new(Self { owner_did, + session_ttl, client, sessions: Mutex::new(HashMap::new()), pending: Mutex::new(HashMap::new()), @@ -164,6 +172,11 @@ impl OperatorAuth { &self.owner_did } + /// How long a session this issues stays good for. + pub fn session_ttl(&self) -> Duration { + self.session_ttl + } + /// The client metadata document, as the JSON served at /// [`CLIENT_METADATA_PATH`]. pub fn client_metadata_document(&self) -> Result { @@ -271,7 +284,7 @@ impl OperatorAuth { sessions.insert( token.clone(), Session { - expires: now + SESSION_TTL, + expires: now + self.session_ttl, csrf: csrf.clone(), }, ); diff --git a/crates/didbot-serve/src/tests.rs b/crates/didbot-serve/src/tests.rs index 94a9b39b..b315f6fc 100644 --- a/crates/didbot-serve/src/tests.rs +++ b/crates/didbot-serve/src/tests.rs @@ -7612,6 +7612,7 @@ mod dashboard_tests { crate::operator::OperatorAuth::new( "did:web:operator.pds.example", "https://kestrel.example", + crate::operator::DEFAULT_SESSION_TTL, ) .expect("the fixture's owner and base url build a client"), ), diff --git a/plan/config.md b/plan/config.md index a16b4a69..ebe4c3f8 100644 --- a/plan/config.md +++ b/plan/config.md @@ -83,7 +83,9 @@ and from `didbot-serve/src/routes.rs`'s constants rather than guessed: shape. - **`[disclosure]`** — which `bot.did.*` routes are closed; mirrors `auth::Disclosure`. Wired. -- **`[estop]`** — the file latch path and the admin socket path. +- **`[operator]`** — `session_ttl_secs`, how long a signed-in operator stays + signed in, against `--operator-session-ttl`. Wired, and the section's own + doc carries the range worth staying inside. - **`[policy]`** — which DID's records this server reads for [policy-store](policy-store.md), once that epic exists to read anything. @@ -144,7 +146,10 @@ pass: - [ ] `--avatar` (development-only; may never belong in a deployment's file at all — worth deciding when this list is next revisited rather than now) -- [ ] rate limit windows, once `rate_limit.rs` settles +- [ ] rate limit windows, once `rate_limit.rs` settles — including + `--dashboard/login`'s own per-address budget + (`routes::OPERATOR_LOGIN_LIMIT`), which is held as a constant for the + same reason its siblings are - [ ] DNS provider selection, once [dns-providers](dns-providers.md) has a second backend - [ ] TLS section, once `didbot-tls`'s configuration surface is stable -- 2.51.2