diff --git a/swarm-authelia-bridge-sock/src/lib.rs b/swarm-authelia-bridge-sock/src/lib.rs index 0f3a2fa5..3865aed4 100644 --- a/swarm-authelia-bridge-sock/src/lib.rs +++ b/swarm-authelia-bridge-sock/src/lib.rs @@ -98,14 +98,30 @@ pub enum BridgeResponse { /// The agent had no identity yet; one was minted and `users.yml` was /// rewritten. Created, - /// The agent already had an identity, so no password was minted. + /// The agent already had an identity **and already carried the agent + /// marker**, so nothing was minted and nothing was written. /// - /// ⚠️ 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*. + /// 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 @@ -141,6 +157,13 @@ 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"), diff --git a/swarm-authelia-bridge/src/main.rs b/swarm-authelia-bridge/src/main.rs index c65b0a41..3210edea 100644 --- a/swarm-authelia-bridge/src/main.rs +++ b/swarm-authelia-bridge/src/main.rs @@ -274,14 +274,25 @@ 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. + // 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)?; - tracing::info!(agent = %name, "marked an existing identity as an agent"); + // `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); } diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index e3d95e69..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:#}")), },