diff --git a/Cargo.lock b/Cargo.lock index 2e0bf188..abcdcced 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4544,7 +4544,6 @@ version = "0.1.0" dependencies = [ "anyhow", "axum", - "hive-types", "reqwest", "serde", "serde_json", @@ -4559,7 +4558,6 @@ 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 cdfd5bef..5f86c108 100644 --- a/swarm-authelia-bridge-sock/Cargo.toml +++ b/swarm-authelia-bridge-sock/Cargo.toml @@ -5,7 +5,6 @@ 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 f635e478..d15c417b 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 -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 +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 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 3865aed4..c0fb0b60 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 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. +//! 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. -use hive_types::Ident; use serde::{Deserialize, Serialize}; -/// A request to the bridge — see the module doc for why this isn't a -/// file-replace API. +/// A request to the bridge. One variant today — 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,49 +47,12 @@ 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 a named outcome on the wire +/// external tag reads directly as one of three named outcomes 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)] @@ -98,36 +61,9 @@ pub enum BridgeResponse { /// The agent had no identity yet; one was minted and `users.yml` was /// rewritten. Created, - /// 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. + /// The agent already had an identity. No write happened — the + /// idempotent no-op path. 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 @@ -138,17 +74,11 @@ pub enum BridgeResponse { #[cfg(test)] mod tests { - use super::{AgentIdentity, BridgeRequest, BridgeResponse}; - use hive_types::Ident; + use super::{BridgeRequest, BridgeResponse}; /// Pins the external-tag wire shape — a reader off the wire (or a log - /// line) should be able to tell the outcomes apart without + /// line) should be able to tell the three 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(); @@ -157,24 +87,6 @@ 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(), }) @@ -187,46 +99,12 @@ 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 else { - panic!("round-tripped into a different variant: {back:?}"); - }; + let BridgeRequest::EnsureAgentIdentity { name } = 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 2d257cfa..cb177d95 100644 --- a/swarm-authelia-bridge/Cargo.toml +++ b/swarm-authelia-bridge/Cargo.toml @@ -9,7 +9,6 @@ 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 33e96a54..cb80a825 100644 --- a/swarm-authelia-bridge/README.md +++ b/swarm-authelia-bridge/README.md @@ -19,9 +19,8 @@ simply owns the file it writes. No elevated privilege anywhere. ## Shape - One endpoint, `POST /requests`, body = `swarm-authelia-bridge-sock`'s - `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). + `BridgeRequest` verbatim — one variant today (`EnsureAgentIdentity`, + idempotently ensure an agent exists as an authelia subject). - 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 3210edea..ccd4d6c9 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` touch a file it doesn't own": it doesn't — this +//! `swarm-controller` write 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: 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. +//! [`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. 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::{AgentIdentity, BridgeRequest, BridgeResponse}; +use swarm_authelia_bridge_sock::{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. 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. +/// 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. async fn handle_request( State(state): State>, headers: HeaderMap, @@ -138,17 +138,12 @@ async fn handle_request( if let Err(resp) = authorize(&state, &headers).await { return resp; } - let (op, result) = match req { - BridgeRequest::EnsureAgentIdentity { name } => { - ("ensure_identity", handle(&state, name).await) - } - BridgeRequest::ListAgentIdentities => ("list_identities", list(&state)), - }; - match result { + let BridgeRequest::EnsureAgentIdentity { name } = req; + match handle(&state, name).await { Ok(body) => (StatusCode::OK, Json(body)).into_response(), Err(e) => { let detail = format!("{e:#}"); - tracing::warn!(op, error = %detail, "request failed"); + tracing::warn!(error = %detail, "ensure_identity failed"); ( StatusCode::INTERNAL_SERVER_ERROR, Json(BridgeResponse::Error { message: detail }), @@ -274,26 +269,6 @@ 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?; @@ -303,7 +278,7 @@ async fn handle(state: &AppState, name: String) -> Result { displayname: name.clone(), password: generated, email: None, - groups: vec![store::AGENT_GROUP.to_owned()], + groups: Vec::new(), extra: std::collections::BTreeMap::new(), }, ); @@ -312,41 +287,6 @@ 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 284671c1..571ab23e 100644 --- a/swarm-authelia-bridge/src/store.rs +++ b/swarm-authelia-bridge/src/store.rs @@ -95,51 +95,6 @@ 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 @@ -333,23 +288,6 @@ 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. @@ -445,62 +383,6 @@ 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 f08cbb7a..fcc759ba 100644 --- a/swarm-controller/src/auth.rs +++ b/swarm-controller/src/auth.rs @@ -1,6 +1,6 @@ -//! 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). +//! 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). //! //! Authenticated with THIS daemon's own queue OIDC identity //! (`SWARM_CONTROLLER_OIDC_*`, the same one `swarm-queue-client` mints for @@ -13,12 +13,9 @@ //! //! `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 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. +//! from it simply has no identity-creation capability yet +//! (`SwarmNodeKind::CreateIdentity` fails such a job explicitly rather +//! than this module papering over the gap). use anyhow::{Context, Result}; use swarm_authelia_bridge_sock::{BridgeRequest, BridgeResponse}; @@ -62,38 +59,6 @@ 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 @@ -106,7 +71,9 @@ impl AuthBridge { .http .post(format!("{}/requests", self.base_url)) .bearer_auth(token) - .json(req) + .json(&BridgeRequest::EnsureAgentIdentity { + name: name.to_owned(), + }) .send() .await .context("calling swarm-authelia-bridge")?; diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index fed8555a..0e939810 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -36,7 +36,6 @@ 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}; @@ -151,19 +150,6 @@ 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:#}")), }, @@ -352,22 +338,14 @@ 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 @@ -429,45 +407,6 @@ 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 @@ -1049,7 +988,7 @@ async fn main() -> Result<()> { } }; let deps = WorkerDeps { - auth: auth.clone(), + auth, forge: forge_client.clone(), }; @@ -1092,7 +1031,6 @@ 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()) @@ -1106,7 +1044,6 @@ 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 @@ -1214,52 +1151,10 @@ 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