diff --git a/swarm-authelia-bridge/src/main.rs b/swarm-authelia-bridge/src/main.rs index 043daf6c..1561ca7a 100644 --- a/swarm-authelia-bridge/src/main.rs +++ b/swarm-authelia-bridge/src/main.rs @@ -278,7 +278,7 @@ async fn handle(state: &AppState, name: String) -> Result { extra: std::collections::BTreeMap::new(), }, ); - store::publish(&cfg.users_file, &user_store)?; + store::publish(&cfg.users_file, &mut user_store)?; tracing::info!(agent = %name, "created authelia identity"); Ok(BridgeResponse::Created) } diff --git a/swarm-authelia-bridge/src/store.rs b/swarm-authelia-bridge/src/store.rs index ffc0a6c3..5f19f4b8 100644 --- a/swarm-authelia-bridge/src/store.rs +++ b/swarm-authelia-bridge/src/store.rs @@ -169,7 +169,8 @@ pub fn load_store(users_file: &Path) -> Result { /// against the pinned 4.39.20 build during this design work — a /// restart would drop every active SSO session, which mara ruled out for /// agent creation specifically (not a rare, human-initiated event). -pub fn publish(users_file: &Path, store: &UserStore) -> Result<()> { +pub fn publish(users_file: &Path, store: &mut UserStore) -> Result<()> { + fill_missing_emails(store); // One file, so there is no longer an ordering question between two // writes — the old version wrote the JSON store first so a crash // between them left something to repair from. That whole failure mode @@ -178,6 +179,30 @@ pub fn publish(users_file: &Path, store: &UserStore) -> Result<()> { write_atomic(users_file, &rendered) } +/// Domain for an address this bridge invents — the same one +/// `swarmctl::users` uses, and the same one `hive-c0re` already gives every +/// agent's forge account. Never routable, deliberately: nothing sends mail +/// here, the address exists so a relying party asking for an `email` claim +/// gets one. +const SYNTHETIC_EMAIL_DOMAIN: &str = "hyperhive.local"; + +/// Give every user an address before the file is written. +/// +/// 🩸 **This bridge did not do it, and `swarmctl` did.** #3414 fixed the +/// missing-email defect in `swarmctl` only, so every identity created *here* +/// landed in `users.yml` with no `email` — and a relying party that asks for +/// the claim does not degrade, it fails (grafana's OIDC login is the +/// measured case, #3393). Two writers of one file disagreeing about a +/// required field is not a difference worth keeping, so the rule now lives +/// on both write paths. +fn fill_missing_emails(store: &mut UserStore) { + for (name, user) in &mut store.users { + if user.email.is_none() { + user.email = Some(format!("{name}@{SYNTHETIC_EMAIL_DOMAIN}")); + } + } +} + /// Replace `path`'s contents atomically. Unlike `swarmctl::write_atomic`, /// this does **not** need to preserve a foreign owner via `chown` — this /// process runs as `users_file`'s own owning user (see the module doc: @@ -317,6 +342,38 @@ some_future_top_level_key: 7 /// `write_atomic` round-trips through a real temp dir — the property /// under test is the rename-into-place, not just the render. + /// 🩸 The asymmetry this fixes: #3414 gave `swarmctl`-created humans a + /// synthetic address and left bridge-created **agents** with none, so + /// two writers of one file disagreed about a field a relying party + /// treats as required — grafana's OIDC login fails outright without it + /// (#3393) rather than degrading. + /// + /// Asserted through the real write path, because that is where the rule + /// lives: `handle` inserts a user with `email: None` and nothing between + /// there and the file would notice. + #[test] + fn an_agent_identity_reaches_the_file_with_an_email() { + let dir = tempdir(); + let users_file = dir.join("users.yml"); + + let mut store = UserStore::default(); + store.users.insert("atlas".to_owned(), user("$argon2id$x")); + assert!( + store.users["atlas"].email.is_none(), + "the fixture must start with the state `handle` creates" + ); + publish(&users_file, &mut store).expect("publish"); + + let reloaded = load_store(&users_file).expect("reload"); + assert_eq!( + reloaded.users["atlas"].email.as_deref(), + Some("atlas@hyperhive.local"), + "an agent must not land in the file without an email" + ); + + fs::remove_dir_all(&dir).ok(); + } + #[test] fn publish_then_load_round_trips_through_the_real_file() { let dir = tempdir(); @@ -325,7 +382,7 @@ some_future_top_level_key: 7 let mut store = UserStore::default(); store.users.insert("atlas".to_owned(), user("$argon2id$x")); - publish(&users_file, &store).expect("publish"); + publish(&users_file, &mut store).expect("publish"); let reloaded = load_store(&users_file).expect("reload"); assert!(reloaded.users.contains_key("atlas")); @@ -353,7 +410,7 @@ some_future_top_level_key: 7 store .users .insert("atlas".to_owned(), user("$argon2id$second")); - publish(&users_file, &store).expect("publish"); + publish(&users_file, &mut store).expect("publish"); let reloaded = load_store(&users_file).expect("reload"); assert!(reloaded.users.contains_key("mara"), "existing user lost"); assert!(reloaded.users.contains_key("atlas"));