diff --git a/Cargo.lock b/Cargo.lock index abcdcced..2e0bf188 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4544,6 +4544,7 @@ version = "0.1.0" dependencies = [ "anyhow", "axum", + "hive-types", "reqwest", "serde", "serde_json", @@ -4558,6 +4559,7 @@ dependencies = [ name = "swarm-authelia-bridge-sock" version = "0.1.0" dependencies = [ + "hive-types", "serde", "serde_json", ] diff --git a/swarm-authelia-bridge-sock/Cargo.toml b/swarm-authelia-bridge-sock/Cargo.toml index 5f86c108..cdfd5bef 100644 --- a/swarm-authelia-bridge-sock/Cargo.toml +++ b/swarm-authelia-bridge-sock/Cargo.toml @@ -5,6 +5,7 @@ edition.workspace = true readme = "README.md" [dependencies] +hive-types.workspace = true serde.workspace = true [dev-dependencies] diff --git a/swarm-authelia-bridge-sock/README.md b/swarm-authelia-bridge-sock/README.md index d15c417b..f635e478 100644 --- a/swarm-authelia-bridge-sock/README.md +++ b/swarm-authelia-bridge-sock/README.md @@ -16,8 +16,8 @@ client logic here, only the request/response shapes both sides import. ## Shape -One operation today: idempotently ensure an agent exists as an authelia -subject. Deliberately **not** a wholesale-replace-the-file API — the bridge -owns both `users.json` (canonical) and rendering `users.yml` internally; a +Two operations: idempotently ensure an agent exists as an authelia subject, +and list the ones that do. Deliberately **not** a wholesale-replace-the-file API — the bridge +reads `users.yml`, changes what the request named, and writes it back; a caller only ever asks for one user to exist, never sends rendered YAML or a file blob. See `swarm-authelia-bridge/README.md` for the helper itself. diff --git a/swarm-authelia-bridge-sock/src/lib.rs b/swarm-authelia-bridge-sock/src/lib.rs index c0fb0b60..3865aed4 100644 --- a/swarm-authelia-bridge-sock/src/lib.rs +++ b/swarm-authelia-bridge-sock/src/lib.rs @@ -20,16 +20,16 @@ //! the file it writes. Fully ordinary permissions, no capabilities, no root. //! //! **Per-operation, not wholesale-replace.** [`BridgeRequest::EnsureAgentIdentity`] -//! asks for one user to exist; the bridge owns both `users.json` (canonical) -//! and rendering `users.yml` internally. A caller never sends rendered YAML -//! or a file blob — that would invite a last-writer-wins race between -//! independent callers and duplicate the rendering logic on both sides of -//! the wire. +//! asks for one user to exist; the bridge reads `users.yml`, changes what the +//! request named, and writes it back. A caller never sends rendered YAML or a +//! file blob — that would invite a last-writer-wins race between independent +//! callers and duplicate the rendering logic on both sides of the wire. +use hive_types::Ident; use serde::{Deserialize, Serialize}; -/// A request to the bridge. One variant today — see the module doc for why -/// this isn't a file-replace API. +/// A request to the bridge — see the module doc for why this isn't a +/// file-replace API. #[derive(Debug, Clone, Serialize, Deserialize)] pub enum BridgeRequest { /// Idempotently ensure `name` exists as an authelia subject — @@ -47,12 +47,49 @@ pub enum BridgeRequest { /// crate carries the wire shape only, not the validation rule. name: String, }, + /// Every agent identity the swarm holds — the roster. + /// + /// Reads the same file [`BridgeRequest::EnsureAgentIdentity`] writes, + /// through the same process, for the same reason: the store is owned by + /// a uid `swarm-controller` does not have. A reader that opened the file + /// itself would also be a second parser of a format this bridge owns. + /// + /// Answers *"which agents exist"*, never *"which are healthy"* — those + /// are separate sets on purpose, and an agent in the roster that has + /// never reported is not the same as one that does not exist. + ListAgentIdentities, +} + +/// One roster entry. +/// +/// A record rather than a bare name because the roster is the set other +/// swarm-level views are *complete against*, so an entry is a thing later +/// facts attach to. It carries what identifies an agent and nothing else — +/// notably not the user's groups (an authorisation detail this answer has no +/// reason to publish) and never the password digest. +#[derive(Debug, Clone, PartialEq, Eq, Serialize, Deserialize)] +pub struct AgentIdentity { + /// The authelia username, which is the agent's name. + /// + /// [`Ident`] rather than a bare `String`, so a consumer gets the same + /// serde-checked parsing at the socket boundary that every other + /// agent-name field in the crate suite does. + /// + /// ⚠️ **It is deliberately NARROWER than what the store accepts.** + /// `Ident` is `[a-z0-9-]`; the users database also admits `.`, `_` and + /// uppercase, because it holds humans too. Every *agent* name is a valid + /// `Ident` by construction — agent creation parses one before the job is + /// queued — so the narrowing costs nothing for real agents and refuses to + /// describe a human who was hand-added to the agent group as though they + /// were one. The reader skips such an entry and logs it; see the bridge's + /// `list`. + pub name: Ident, } /// The bridge's answer to a [`BridgeRequest`]. /// /// `#[serde(tag = "status")]` rather than a bare `Result`-shaped wrapper: an -/// external tag reads directly as one of three named outcomes on the wire +/// external tag reads directly as a named outcome on the wire /// (`{"status":"created",...}`), with no separate "was this an error" /// boolean to keep in sync with which variant it is. #[derive(Debug, Clone, Serialize, Deserialize)] @@ -61,9 +98,36 @@ pub enum BridgeResponse { /// The agent had no identity yet; one was minted and `users.yml` was /// rewritten. Created, - /// The agent already had an identity. No write happened — the - /// idempotent no-op path. + /// The agent already had an identity **and already carried the agent + /// marker**, so nothing was minted and nothing was written. + /// + /// The uneventful case: re-running agent creation for an agent that is + /// already an agent. AlreadyExists, + /// A subject with this name existed **without** the agent marker, and + /// this call added it. + /// + /// ⚠️ **Its own variant, not a flag on [`Self::AlreadyExists`], because + /// the two cases carry different risk and a `bool` field is ignorable.** + /// Adding a variant makes every existing `match` fail to compile until + /// its author decides what to do about a heal; a field would let the + /// dangerous case keep travelling as the safe one, which is how it went + /// unreported before. + /// + /// Why the risk differs: the store cannot tell a pre-marker *agent* + /// identity from a *human* operator account created without a group — + /// both are simply `groups: []`. So this outcome is either the intended + /// migration (re-running creation is the whole of that story) or an + /// agent quietly joining a person's live SSO account. The caller is the + /// only place with the context to tell those apart, so it has to be + /// told the write happened. + Healed, + /// The roster, in answer to [`BridgeRequest::ListAgentIdentities`]. + Agents { + /// Sorted by name — the store is a `BTreeMap`, and a stable order + /// means a consumer diffing two answers sees real changes only. + agents: Vec, + }, /// The request was rejected or the write failed. Carries a message /// for the caller to log/propagate, not a typed error enum: the /// failure modes here (bad username, authelia binary failed, disk @@ -74,11 +138,17 @@ pub enum BridgeResponse { #[cfg(test)] mod tests { - use super::{BridgeRequest, BridgeResponse}; + use super::{AgentIdentity, BridgeRequest, BridgeResponse}; + use hive_types::Ident; /// Pins the external-tag wire shape — a reader off the wire (or a log - /// line) should be able to tell the three outcomes apart without + /// line) should be able to tell the outcomes apart without /// cross-referencing this crate's source. + /// + /// **Every variant, deliberately.** A test that claims to pin the wire + /// shape and covers all but one is worse than a narrower one: the next + /// variant gets added with nothing to remind its author that this is + /// where the shape is settled. #[test] fn response_variants_tag_on_status() { let created = serde_json::to_value(BridgeResponse::Created).unwrap(); @@ -87,6 +157,24 @@ mod tests { let exists = serde_json::to_value(BridgeResponse::AlreadyExists).unwrap(); assert_eq!(exists, serde_json::json!({"status": "already_exists"})); + // Distinct on the wire from `already_exists`, which is the whole + // point of it being a separate variant: a reader tailing these has + // to be able to see a heal without knowing the Rust type. + let healed = serde_json::to_value(BridgeResponse::Healed).unwrap(); + assert_eq!(healed, serde_json::json!({"status": "healed"})); + assert_ne!(healed, exists); + + let agents = serde_json::to_value(BridgeResponse::Agents { + agents: vec![AgentIdentity { + name: Ident::parse("atlas").expect("valid ident"), + }], + }) + .unwrap(); + assert_eq!( + agents, + serde_json::json!({"status": "agents", "agents": [{"name": "atlas"}]}) + ); + let err = serde_json::to_value(BridgeResponse::Error { message: "boom".to_owned(), }) @@ -99,12 +187,46 @@ mod tests { #[test] fn request_round_trips() { + // Still a `String`: the REQUEST carries whatever the caller asked for + // and the bridge validates it server-side, which is what lets an + // illegal name be refused with a message rather than failing to + // deserialise. Only the ROSTER ENTRY is an `Ident`, because that one + // describes something the store already accepted. let req = BridgeRequest::EnsureAgentIdentity { name: "atlas".to_owned(), }; let json = serde_json::to_string(&req).unwrap(); let back: BridgeRequest = serde_json::from_str(&json).unwrap(); - let BridgeRequest::EnsureAgentIdentity { name } = back; + let BridgeRequest::EnsureAgentIdentity { name } = back else { + panic!("round-tripped into a different variant: {back:?}"); + }; assert_eq!(name, "atlas"); + + let json = serde_json::to_string(&BridgeRequest::ListAgentIdentities).unwrap(); + let back: BridgeRequest = serde_json::from_str(&json).unwrap(); + assert!(matches!(back, BridgeRequest::ListAgentIdentities)); + } + + /// An empty roster must be a roster, not an absence: `{"agents":[]}` + /// says "no agents exist", and a consumer that cannot tell that from a + /// missing field will render a swarm it failed to read as an empty one. + #[test] + fn an_empty_roster_still_serialises_its_list() { + let empty = serde_json::to_value(BridgeResponse::Agents { agents: vec![] }).unwrap(); + assert_eq!(empty, serde_json::json!({"status": "agents", "agents": []})); + } + + /// The roster entry carries the name and nothing else. Asserted on the + /// serialised keys rather than on the struct, because the risk is a + /// future field being added here and reaching the wire unnoticed — this + /// answer is derived from a file of password digests. + #[test] + fn a_roster_entry_carries_only_the_name() { + let entry = serde_json::to_value(AgentIdentity { + name: Ident::parse("atlas").expect("valid ident"), + }) + .unwrap(); + let keys: Vec<&String> = entry.as_object().expect("an object").keys().collect(); + assert_eq!(keys, ["name"], "the roster names agents, nothing more"); } } diff --git a/swarm-authelia-bridge/Cargo.toml b/swarm-authelia-bridge/Cargo.toml index cb177d95..2d257cfa 100644 --- a/swarm-authelia-bridge/Cargo.toml +++ b/swarm-authelia-bridge/Cargo.toml @@ -9,6 +9,7 @@ name = "swarm-authelia-bridge" path = "src/main.rs" [dependencies] +hive-types.workspace = true anyhow.workspace = true axum.workspace = true reqwest.workspace = true diff --git a/swarm-authelia-bridge/README.md b/swarm-authelia-bridge/README.md index cb80a825..33e96a54 100644 --- a/swarm-authelia-bridge/README.md +++ b/swarm-authelia-bridge/README.md @@ -19,8 +19,9 @@ simply owns the file it writes. No elevated privilege anywhere. ## Shape - One endpoint, `POST /requests`, body = `swarm-authelia-bridge-sock`'s - `BridgeRequest` verbatim — one variant today (`EnsureAgentIdentity`, - idempotently ensure an agent exists as an authelia subject). + `BridgeRequest` verbatim — `EnsureAgentIdentity` (idempotently ensure an + agent exists as an authelia subject) and `ListAgentIdentities` (the + roster: the subjects carrying the agent marker group, names only). - Bearer-authenticated via authelia's own OIDC token introspection (RFC 7662), against `swarm-controller`'s **existing** machine-client identity (already minted for the queue connection) — no new credential. diff --git a/swarm-authelia-bridge/src/main.rs b/swarm-authelia-bridge/src/main.rs index ccd4d6c9..3210edea 100644 --- a/swarm-authelia-bridge/src/main.rs +++ b/swarm-authelia-bridge/src/main.rs @@ -6,7 +6,7 @@ //! `swarm-authelia.nix`'s `unitName` already derives) — the same user //! authelia's own instance runs as, and therefore the file's actual //! owner. That is the whole answer to "how does an unprivileged -//! `swarm-controller` write a file it doesn't own": it doesn't — this +//! `swarm-controller` touch a file it doesn't own": it doesn't — this //! process does, sidestepping the uid boundary instead of bridging it //! with root/`CAP_CHOWN`/a shared group (all examined and rejected — see //! `swarmctl/README.md`'s own identical analysis of this same file). @@ -23,10 +23,10 @@ //! this process runs right next to the authelia it asks. //! //! One endpoint, `POST /requests`, body = -//! [`swarm_authelia_bridge_sock::BridgeRequest`] verbatim — one variant -//! today (`EnsureAgentIdentity`, idempotently ensure an agent exists as -//! an authelia subject). Not a wholesale-replace-the-file API — this -//! process owns rendering `users.yml` internally; see `store` module. +//! [`swarm_authelia_bridge_sock::BridgeRequest`] verbatim: ensure an agent +//! exists as an authelia subject, and list the ones that do. Not a +//! wholesale-replace-the-file API — this process owns rendering +//! `users.yml` internally; see `store` module. mod introspect; mod store; @@ -39,7 +39,7 @@ use axum::http::{HeaderMap, StatusCode}; use axum::response::{IntoResponse, Response}; use axum::routing::post; use axum::{Json, Router}; -use swarm_authelia_bridge_sock::{BridgeRequest, BridgeResponse}; +use swarm_authelia_bridge_sock::{AgentIdentity, BridgeRequest, BridgeResponse}; /// Where the introspecting bearer token is authenticated *to* — this /// bridge's own authelia machine client, distinct from @@ -127,9 +127,9 @@ async fn main() -> Result<()> { } /// The single endpoint — body is the wire-crate's [`BridgeRequest`] -/// verbatim (JSON), not a per-operation REST route. One variant today, -/// but this is what keeps a second operation from needing a new route + -/// a new extractor shape: it just becomes a new match arm below. +/// verbatim (JSON), not a per-operation REST route. That is what let the +/// second operation arrive as a match arm rather than as another route +/// with its own extractor and its own copy of the authorize call. async fn handle_request( State(state): State>, headers: HeaderMap, @@ -138,12 +138,17 @@ async fn handle_request( if let Err(resp) = authorize(&state, &headers).await { return resp; } - let BridgeRequest::EnsureAgentIdentity { name } = req; - match handle(&state, name).await { + let (op, result) = match req { + BridgeRequest::EnsureAgentIdentity { name } => { + ("ensure_identity", handle(&state, name).await) + } + BridgeRequest::ListAgentIdentities => ("list_identities", list(&state)), + }; + match result { Ok(body) => (StatusCode::OK, Json(body)).into_response(), Err(e) => { let detail = format!("{e:#}"); - tracing::warn!(error = %detail, "ensure_identity failed"); + tracing::warn!(op, error = %detail, "request failed"); ( StatusCode::INTERNAL_SERVER_ERROR, Json(BridgeResponse::Error { message: detail }), @@ -269,6 +274,26 @@ async fn handle(state: &AppState, name: String) -> Result { let mut user_store = store::load_store(&cfg.users_file)?; if user_store.users.contains_key(&name) { + // The subject is there, so no password is minted. Whether it gained + // the marker here is the caller's business, not an implementation + // detail: an unmarked subject is either a pre-marker agent (the + // intended migration) or a human created without a group, and this + // process cannot tell them apart. So the two outcomes are reported + // as different variants rather than folded into one. + if store::mark_as_agent(&mut user_store, &name) { + store::publish(&cfg.users_file, &mut user_store)?; + // `warn`, not `info`: the uneventful path is silent, so anything + // logged here is a subject that was *not* an agent a moment ago. + // Names the account because "which one" is the entire question + // an operator will have. + tracing::warn!( + subject = %name, + "added the agent marker to an EXISTING identity — intended if this \ + predates the marker, but the store cannot distinguish that from a \ + human account created without a group" + ); + return Ok(BridgeResponse::Healed); + } return Ok(BridgeResponse::AlreadyExists); } let generated = generate_password(&cfg.authelia_bin).await?; @@ -278,7 +303,7 @@ async fn handle(state: &AppState, name: String) -> Result { displayname: name.clone(), password: generated, email: None, - groups: Vec::new(), + groups: vec![store::AGENT_GROUP.to_owned()], extra: std::collections::BTreeMap::new(), }, ); @@ -287,6 +312,41 @@ async fn handle(state: &AppState, name: String) -> Result { Ok(BridgeResponse::Created) } +/// The roster: every subject carrying the agent marker. +/// +/// Takes no write lock. The store is replaced by an atomic rename, so a +/// reader sees either the whole old file or the whole new one — and holding +/// the lock would make a read wait behind a password mint, which is an +/// external process. +fn list(state: &AppState) -> Result { + let user_store = store::load_store(&state.config.users_file)?; + Ok(BridgeResponse::Agents { + agents: store::agent_names(&user_store) + .into_iter() + .filter_map(|name| match hive_types::Ident::parse(&name) { + Ok(ident) => Some(AgentIdentity { name: ident }), + // The store's charset is wider than an agent name's, because + // it holds humans too. Every agent this bridge creates is a + // legal `Ident` by construction, so reaching here means a + // subject was hand-added to the agent group that cannot be an + // agent. Skipped rather than failing the whole roster — one + // bad row must not make every other agent invisible — but + // logged, because silently shrinking an answer is how a + // roster starts lying about who is missing. + Err(reason) => { + tracing::warn!( + subject = %name, + %reason, + "subject carries the agent group but is not a legal agent name; \ + omitting it from the roster" + ); + None + } + }) + .collect(), + }) +} + /// Mint a password digest via the **configured** authelia — the argon2 /// parameters baked into a hash have to match the verifier's, same /// reasoning `swarmctl::generate_password` documents (this is diff --git a/swarm-authelia-bridge/src/store.rs b/swarm-authelia-bridge/src/store.rs index 571ab23e..284671c1 100644 --- a/swarm-authelia-bridge/src/store.rs +++ b/swarm-authelia-bridge/src/store.rs @@ -95,6 +95,51 @@ pub fn reject_reserved_name(name: &str) -> Result<()> { Ok(()) } +/// The group that marks a subject as an agent. +/// +/// One file holds humans and agents, and nothing else in it says which is +/// which — so the roster needs a predicate, and a positive mark is the only +/// one that fails safe. The alternative, *"everyone who is not an +/// operator"*, misfiles a **human**: an account created without a group is +/// an operator who cannot log in, a documented mistake, and it would read as +/// an agent. +/// +/// A constant rather than an option, for the reason `swarm-authelia.nix` +/// already gives about the operator group it mirrors: a configurable name is +/// one more way for the rule and the account to disagree silently. +pub const AGENT_GROUP: &str = "agents"; + +/// Add the agent marker to an existing subject if it is missing; reports +/// whether anything changed, so the caller knows whether a write is owed. +/// +/// Exists because an identity created before the marker did is otherwise +/// invisible to the roster forever. The agreed migration for those is *"run +/// agent creation again with the same name"*, and this is what makes that +/// heal rather than no-op. +pub fn mark_as_agent(store: &mut UserStore, name: &str) -> bool { + let Some(user) = store.users.get_mut(name) else { + return false; + }; + if user.groups.iter().any(|g| g == AGENT_GROUP) { + return false; + } + user.groups.push(AGENT_GROUP.to_owned()); + true +} + +/// The agent names in the store, sorted. +/// +/// Sorted because [`UserStore::users`] is a `BTreeMap`, so this is free and +/// a consumer diffing two answers sees real changes rather than reordering. +pub fn agent_names(store: &UserStore) -> Vec { + store + .users + .iter() + .filter(|(_, user)| user.groups.iter().any(|g| g == AGENT_GROUP)) + .map(|(name, _)| name.clone()) + .collect() +} + /// Same conservative charset `swarmctl::users::validate_username` already /// enforces — usernames are YAML map keys, log lines, and access-control /// subjects, so keeping them to plain ASCII means nothing downstream ever @@ -288,6 +333,23 @@ mod tests { } } + fn agent(password: &str) -> User { + User { + groups: vec![AGENT_GROUP.to_owned()], + ..user(password) + } + } + + /// The group an operator carries — a literal here rather than a shared + /// constant, because this binary does not read the operator group and a + /// test that imported one would imply it did. + fn operator(password: &str) -> User { + User { + groups: vec!["admins".to_owned()], + ..user(password) + } + } + /// The document `swarm-authelia`'s first-boot unit writes must load as /// an empty store with no special case — otherwise a fresh hive's very /// first `EnsureAgentIdentity` fails on a file we wrote ourselves. @@ -383,6 +445,62 @@ some_future_top_level_key: 7 ); } + /// The roster's whole job: one file holds humans and agents, and only + /// the marker separates them. Asserted with an operator present, because + /// a filter that returned everyone would pass a fixture of agents alone. + #[test] + fn the_roster_is_the_marked_subjects_and_not_the_file() { + let mut store = UserStore::default(); + store.users.insert("atlas".to_owned(), agent("$argon2id$a")); + store + .users + .insert("mara".to_owned(), operator("$argon2id$b")); + // An account with no group at all: the documented mistake of adding + // an operator without `--group`. It must NOT read as an agent. + store + .users + .insert("ungrouped".to_owned(), user("$argon2id$c")); + + assert_eq!(agent_names(&store), ["atlas"]); + } + + /// An identity that predates the marker is invisible to the roster until + /// agent creation runs again — which is the agreed migration, so it has + /// to actually change something. + #[test] + fn marking_an_existing_identity_is_a_change_once_and_never_again() { + let mut store = UserStore::default(); + store.users.insert("atlas".to_owned(), user("$argon2id$a")); + assert!(agent_names(&store).is_empty(), "fixture starts unmarked"); + + assert!(mark_as_agent(&mut store, "atlas"), "the first mark writes"); + assert_eq!(agent_names(&store), ["atlas"]); + assert!( + !mark_as_agent(&mut store, "atlas"), + "a second call must report no change — otherwise every ensure \ + rewrites the file and wakes authelia's watcher" + ); + + assert!( + !mark_as_agent(&mut store, "nobody"), + "marking a subject that does not exist creates nothing" + ); + assert!(!store.users.contains_key("nobody")); + } + + /// The marker is a group like any other, so it must survive the + /// round-trip that every write goes through — a marker that renders but + /// does not re-parse would make the roster empty after one restart. + #[test] + fn the_marker_survives_a_round_trip() { + let mut store = UserStore::default(); + store.users.insert("atlas".to_owned(), agent("$argon2id$a")); + + let out = render_yaml(&store).expect("renders"); + let back: UserStore = serde_norway::from_str(&out).expect("reparses"); + assert_eq!(agent_names(&back), ["atlas"]); + } + #[test] fn usernames_outside_the_conservative_set_are_refused() { for bad in ["", "-leading", "has space", "quote\"d", "sla/sh"] { diff --git a/swarm-controller/src/auth.rs b/swarm-controller/src/auth.rs index fcc759ba..f08cbb7a 100644 --- a/swarm-controller/src/auth.rs +++ b/swarm-controller/src/auth.rs @@ -1,6 +1,6 @@ -//! Client for `swarm-authelia-bridge` — the only writer of the swarm's -//! authelia users database (see that crate's README for why this daemon -//! cannot write it directly). +//! Client for `swarm-authelia-bridge` — this daemon's only access to the +//! swarm's authelia users database, in either direction (see that crate's +//! README for why the file cannot be touched from here). //! //! Authenticated with THIS daemon's own queue OIDC identity //! (`SWARM_CONTROLLER_OIDC_*`, the same one `swarm-queue-client` mints for @@ -13,9 +13,12 @@ //! //! `None` when this deployment did not wire a bridge up — the bridge only //! exists on hosts that also run `swarm-authelia`, so a controller split -//! from it simply has no identity-creation capability yet -//! (`SwarmNodeKind::CreateIdentity` fails such a job explicitly rather -//! than this module papering over the gap). +//! from it can neither create an identity nor read the roster. +//! +//! Each caller answers that absence in its own terms rather than this +//! module inventing a shared one: `SwarmNodeKind::CreateIdentity` fails +//! the job explicitly, and the roster endpoint refuses. Neither substitutes +//! an empty answer, which is the failure mode a default here would create. use anyhow::{Context, Result}; use swarm_authelia_bridge_sock::{BridgeRequest, BridgeResponse}; @@ -59,6 +62,38 @@ impl AuthBridge { /// Idempotently ensure `name` exists as an authelia subject. pub async fn ensure_agent_identity(&self, name: &str) -> Result { + self.request(&BridgeRequest::EnsureAgentIdentity { + name: name.to_owned(), + }) + .await + } + + /// The swarm's agent roster, straight from the identity store. + /// + /// Returns the names rather than the whole [`BridgeResponse`]: every + /// other variant is a protocol error for this request, and a caller that + /// had to match them would be free to treat one as an empty roster. An + /// empty roster and a bridge that answered something else are different + /// facts, and only one of them is a valid render. + pub async fn list_agent_identities(&self) -> Result> { + match self.request(&BridgeRequest::ListAgentIdentities).await? { + BridgeResponse::Agents { agents } => Ok(agents + .into_iter() + .map(|entry| entry.name.into_string()) + .collect()), + other => { + anyhow::bail!("swarm-authelia-bridge answered a roster request with {other:?}") + } + } + } + + /// One authenticated round-trip to the bridge. + /// + /// A fresh token per call, deliberately — see the module doc. Shared by + /// every operation so the auth and the error handling cannot drift + /// between them; a second copy is how one path ends up treating a 500 as + /// an answer. + async fn request(&self, req: &BridgeRequest) -> Result { // `mint_token_for` builds its own HTTP client (trusting the queue's // configured CA, if any) — deliberately not `self.http`, which is // the bridge's own client and has nothing to do with authelia's @@ -71,9 +106,7 @@ impl AuthBridge { .http .post(format!("{}/requests", self.base_url)) .bearer_auth(token) - .json(&BridgeRequest::EnsureAgentIdentity { - name: name.to_owned(), - }) + .json(req) .send() .await .context("calling swarm-authelia-bridge")?; diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 0e939810..fed8555a 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -36,6 +36,7 @@ use axum::{ }; use hive_jobq_wire::GraphWire as _; use serde::{Deserialize, Serialize}; +use swarm_authelia_bridge_sock::BridgeResponse; use utoipa::{OpenApi, ToSchema}; use utoipa_axum::{router::OpenApiRouter, routes}; @@ -150,6 +151,19 @@ async fn run_swarm_node( Outcome::Failed("no swarm-authelia-bridge is configured on this host".to_owned()) } Some(bridge) => match bridge.ensure_agent_identity(&agent).await { + // Matched rather than discarded with `Ok(_)`: a heal writes + // to a subject this job did not create, and that has to + // reach a human. `Outcome` has no success-carrying variant, + // so the controller's own log is the only channel a + // succeeding node has today. + Ok(BridgeResponse::Healed) => { + tracing::warn!( + %agent, + "swarm jobq: create_identity marked an EXISTING identity as an \ + agent — verify this name did not belong to a person" + ); + Outcome::Done + } Ok(_) => Outcome::Done, Err(e) => Outcome::Failed(format!("{e:#}")), }, @@ -338,14 +352,22 @@ struct AppState { /// is synchronous (no `.await` while held). Always present, never /// gated on the swarm queue: this is process state, not something /// read over the network. - /// **Not** consulted by `POST /api/agents` — that endpoint only ever - /// queues the job (see [`create_agent`]); whether a bridge is - /// configured is `run_swarm_node`'s concern (it holds its own clone, - /// handed to it by `spawn_jobq_worker`), not this handler's. A request - /// still queues cleanly on a bridge-less host, then fails loud once - /// claimed — same "queue now, fail per-job" shape as an unreachable - /// swarm queue. jobq: Arc>>, + /// The identity store's only reader, for `GET /api/agents`. + /// + /// `None` when no bridge is wired up, which is the one state in which + /// the roster cannot be answered at all — the same shape as `status` + /// above, and for the same reason: an empty roster and an unreadable + /// one are different facts. + /// + /// ⚠️ The two `/api/agents` verbs treat this differently on purpose. + /// **POST does not consult it**: creation only queues a job, and + /// whether a bridge exists is `run_swarm_node`'s concern (it holds its + /// own clone from `spawn_jobq_worker`), so a request still queues + /// cleanly on a bridge-less host and fails loud once claimed. **GET + /// has nowhere to defer to** — there is no job, only an answer it + /// either has or does not. + auth: Option>, /// HMAC secret for swarm-wide forge webhooks, loaded once at startup. /// `None` when it could not be read or created — the webhook endpoint /// then refuses every delivery with 503 rather than admitting one it @@ -407,6 +429,45 @@ async fn get_hives(State(state): State) -> Json> { Json((*state.hives).clone()) } +/// The swarm's agent roster — every agent the swarm holds an identity for. +/// +/// The identity store **is** the roster rather than one source to assemble +/// one from: an agent without a swarm identity is not a swarm agent, so +/// there is no hive-side list to merge in and no reconciliation to do. +/// +/// Deliberately says nothing about health. A roster is the set other views +/// are *complete against* — it is what makes "this agent has never reported" +/// expressible at all, and that distinction only survives while the declared +/// set and the reported set stay separate. +/// +/// A hive-less swarm answers `[]`; a swarm with no bridge answers 503. Those +/// are different facts and collapsing them would render an unreadable store +/// as an empty one. +#[utoipa::path( + get, + path = "/api/agents", + responses( + (status = 200, description = "every agent the swarm holds an identity for", body = Vec), + (status = 503, description = "no identity bridge is configured here, or its store could not be read", body = String), + ), + tag = "agents" +)] +async fn get_agents(State(state): State) -> Result>, StatusUnavailable> { + let Some(bridge) = state.auth.as_ref() else { + return Err(StatusUnavailable( + "no identity bridge is configured on this host".to_owned(), + )); + }; + match bridge.list_agent_identities().await { + Ok(agents) => Ok(Json(agents)), + Err(e) => { + let detail = format!("{e:#}"); + tracing::warn!(error = %detail, "reading the agent roster failed"); + Err(StatusUnavailable(detail)) + } + } +} + /// One quick link to a swarm-wide service (authelia, matrix, forge, this /// daemon's own swagger UI, …). Deliberately generic rather than named /// fields per service: each service's own nix module contributes its own @@ -988,7 +1049,7 @@ async fn main() -> Result<()> { } }; let deps = WorkerDeps { - auth, + auth: auth.clone(), forge: forge_client.clone(), }; @@ -1031,6 +1092,7 @@ async fn main() -> Result<()> { webhook_secret, config_prs, swarm_name: load_swarm_name().map(Arc::from), + auth, }; let (router, api) = OpenApiRouter::::with_openapi(ApiDoc::openapi()) @@ -1044,6 +1106,7 @@ async fn main() -> Result<()> { .routes(routes!(get_agent_config_pr)) .routes(routes!(get_config_prs)) .routes(routes!(create_agent)) + .routes(routes!(get_agents)) .routes(routes!(webhook::post_webhook_forge)) .split_for_parts(); // Just the JSON, not the UI — Swagger UI itself is nginx-hosted from @@ -1151,10 +1214,52 @@ mod tests { webhook_secret: None, config_prs: None, swarm_name: None, + // No bridge: these tests drive agent *creation*, which queues a + // job and never consults one. The roster read is the verb that + // needs it, and it has its own test below. + auth: None, }; (state, sched) } + /// A swarm with no identity bridge cannot answer the roster, and must + /// say so rather than answer `[]`. + /// + /// The distinction is the whole reason this endpoint exists: an empty + /// roster means *no agents*, and a UI that renders "no agents" for a + /// store it could not read is showing a fact nobody established. 503 + /// rather than 500 for the same reason the status route uses it — a + /// bridge is wired up per deployment, so a caller retrying is right. + #[tokio::test] + async fn a_roster_with_no_bridge_refuses_rather_than_answering_empty() { + use axum::response::IntoResponse as _; + + let (state, _sched) = state_with_roster(); + assert!(state.auth.is_none(), "the fixture must have no bridge"); + + let err = super::get_agents(axum::extract::State(state)) + .await + .expect_err("a bridge-less controller cannot produce a roster"); + let resp = err.into_response(); + assert_eq!(resp.status(), axum::http::StatusCode::SERVICE_UNAVAILABLE); + + let bytes = axum::body::to_bytes(resp.into_body(), 64 * 1024) + .await + .expect("body reads"); + let v: serde_json::Value = serde_json::from_slice(&bytes).expect("problem+json parses"); + // Asserted on the rendered body rather than the error value: what a + // consumer can distinguish is what matters, and `[]` and this share + // a type on the Rust side. + assert_eq!(v["status"], 503); + assert!( + v["detail"] + .as_str() + .is_some_and(|d| d.contains("identity bridge")), + "the cause must name what is missing, got {:?}", + v["detail"] + ); + } + /// The roster check is the half that makes the recorded hive worth /// having, so assert it by EFFECT rather than by the message: a hive /// that is not in this swarm must be refused **before anything is