diff --git a/swarm-authelia-bridge-sock/README.md b/swarm-authelia-bridge-sock/README.md index 69aa3035..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 +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 963c5765..890669f9 100644 --- a/swarm-authelia-bridge-sock/src/lib.rs +++ b/swarm-authelia-bridge-sock/src/lib.rs @@ -27,8 +27,8 @@ 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 — @@ -46,6 +46,30 @@ 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. + pub name: String, } /// The bridge's answer to a [`BridgeRequest`]. @@ -60,9 +84,20 @@ 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, so no password was minted. + /// + /// ⚠️ Says nothing about whether the file was written. An identity that + /// exists but is missing the marker group gains it here, because + /// re-running agent creation is the whole of the migration story for + /// identities that predate that marker — so "already exists" has to mean + /// *the subject was there*, not *nothing changed on disk*. AlreadyExists, + /// 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 @@ -73,7 +108,7 @@ pub enum BridgeResponse { #[cfg(test)] mod tests { - use super::{BridgeRequest, BridgeResponse}; + use super::{AgentIdentity, BridgeRequest, BridgeResponse}; /// 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 @@ -103,7 +138,36 @@ mod tests { }; 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: "atlas".to_owned(), + }) + .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/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..cfbcbf53 100644 --- a/swarm-authelia-bridge/src/main.rs +++ b/swarm-authelia-bridge/src/main.rs @@ -23,10 +23,15 @@ //! 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. +//! +//! The read is here rather than in its caller for the same reason the +//! write is: the store is owned by a uid `swarm-controller` does not have, +//! and a caller that opened the file itself would be a second parser of a +//! format this process owns. mod introspect; mod store; @@ -39,7 +44,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 +132,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 +143,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 +279,15 @@ 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 — but it may + // predate the marker that puts it in the roster, and running agent + // creation again is the whole of the migration for those. Reported + // as `AlreadyExists` either way: the wire answer is about the + // subject existing, not about whether bytes moved. + if store::mark_as_agent(&mut user_store, &name) { + store::publish(&cfg.users_file, &mut user_store)?; + tracing::info!(agent = %name, "marked an existing identity as an agent"); + } return Ok(BridgeResponse::AlreadyExists); } let generated = generate_password(&cfg.authelia_bin).await?; @@ -278,7 +297,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 +306,22 @@ 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() + .map(|name| AgentIdentity { name }) + .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"] {