diff --git a/swarm-controller/src/matrix_account.rs b/swarm-controller/src/matrix_account.rs index 2e24303f..31843bda 100644 --- a/swarm-controller/src/matrix_account.rs +++ b/swarm-controller/src/matrix_account.rs @@ -45,7 +45,11 @@ fn default_mode() -> String { } /// The credential to store for one agent's external matrix account. -#[derive(Debug, Deserialize, ToSchema)] +/// +/// No `Debug` derive — matching `hive-c0re::dashboard::matrix_accounts`'s +/// analogous `MatrixLoginForm`, which also carries a password field and +/// deliberately omits it so nothing can `{:?}`-log this by accident. +#[derive(Deserialize, ToSchema)] pub struct PutMatrixAccountRequest { /// `"token"` (default, back-compat) or `"password"`. Token mode stores /// `token` as given; password mode logs into `homeserver` with `user_id` @@ -140,7 +144,7 @@ pub async fn put_matrix_account( // but would confuse anything that lists accounts by name. Same guard // hive-c0re's own `/api/matrix-account-login` applies, checked before any // mode-specific work (including a network login) runs. - if account == "main" { + if is_reserved_account(&account) { return Err(error_problem( StatusCode::BAD_REQUEST, "'main' is the hive-internal account, synthesized per agent from \ @@ -148,43 +152,9 @@ pub async fn put_matrix_account( )); } - // Resolve the mode to a concrete (token, homeserver, resolved user id) — - // password mode's network call happens here, before the store is + // Password mode's network call happens here, before the store is // touched, so a failed login leaves no partial state behind. - let (token, homeserver, user_id) = match req.mode.as_str() { - "token" => { - let Some(token) = req.token else { - return Err(error_problem( - StatusCode::BAD_REQUEST, - "token mode needs a token", - )); - }; - (token, req.homeserver, None) - } - "password" => { - let (Some(homeserver), Some(user_id), Some(password)) = ( - req.homeserver.as_deref(), - req.user_id.as_deref(), - req.password.as_deref(), - ) else { - return Err(error_problem( - StatusCode::BAD_REQUEST, - "password mode needs homeserver, user_id, and password", - )); - }; - let homeserver = homeserver.trim_end_matches('/').to_owned(); - let (token, resolved_user_id) = matrix_password_login(&homeserver, user_id, password) - .await - .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e))?; - (token, Some(homeserver), Some(resolved_user_id)) - } - other => { - return Err(error_problem( - StatusCode::BAD_REQUEST, - &format!("unknown mode {other:?} (want token|password)"), - )); - } - }; + let (token, homeserver, user_id) = resolve_credential(&req).await.map_err(|b| *b)?; let store = SecretStore::from_env(CERT_ROLE).await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); @@ -240,6 +210,92 @@ pub async fn put_matrix_account( Ok(Json(PutMatrixAccountResponse { user_id })) } +/// Whether `account` is the hive-internal name every hive synthesizes per +/// agent (`nix/agent-modules/matrix.nix`) — see the call site's own comment +/// for why this route must never write one. +fn is_reserved_account(account: &str) -> bool { + account == "main" +} + +/// Token-mode's only requirement: a token was actually given. Split out of +/// `resolve_credential` (with `password_fields` below) so each mode's +/// validation is unit-testable directly — only the actual network login in +/// `resolve_credential` itself needs an async runtime to exercise. Returns a +/// plain `&'static str` rather than a `ProblemDetails` — `clippy::result_large_err` +/// (this workspace runs `pedantic = deny`) flags a private fn returning one of +/// those directly; `put_matrix_account` itself is exempt only because it is +/// `pub` (clippy's `avoid-breaking-exported-api` default), which these +/// helpers are not. +fn token_credential( + req: &PutMatrixAccountRequest, +) -> Result<(String, Option), &'static str> { + let Some(token) = req.token.clone() else { + return Err("token mode needs a token"); + }; + Ok((token, req.homeserver.clone())) +} + +/// Password mode's required fields, validated present, with the +/// homeserver's trailing slash already trimmed — this is what actually +/// reaches `matrix_password_login`, not the caller's raw string, so the +/// login URL below never ends up with a doubled `//`. +struct PasswordFields<'a> { + homeserver: String, + user_id: &'a str, + password: &'a str, +} + +fn password_fields(req: &PutMatrixAccountRequest) -> Result, &'static str> { + let (Some(homeserver), Some(user_id), Some(password)) = ( + req.homeserver.as_deref(), + req.user_id.as_deref(), + req.password.as_deref(), + ) else { + return Err("password mode needs homeserver, user_id, and password"); + }; + Ok(PasswordFields { + homeserver: homeserver.trim_end_matches('/').to_owned(), + user_id, + password, + }) +} + +/// Resolve a request's credential mode to a concrete `(token, homeserver, +/// resolved user id)`. Extracted out of `put_matrix_account` to keep that +/// function under `clippy::too_many_lines`, and so the mode branching has +/// something to unit-test directly (via `token_credential`/`password_fields` +/// above) instead of only through the full route. +/// +/// `Box`ed error for the same `result_large_err` reason `token_credential`'s +/// doc explains — this fn is private too, so it does not get `put_matrix_account`'s +/// exported-API exemption. Unboxed at the one call site instead of changing +/// `put_matrix_account`'s own (exempt, and part of the route's documented +/// contract) return type. +async fn resolve_credential( + req: &PutMatrixAccountRequest, +) -> Result<(String, Option, Option), Box> { + match req.mode.as_str() { + "token" => { + let (token, homeserver) = token_credential(req) + .map_err(|e| Box::new(error_problem(StatusCode::BAD_REQUEST, e)))?; + Ok((token, homeserver, None)) + } + "password" => { + let fields = password_fields(req) + .map_err(|e| Box::new(error_problem(StatusCode::BAD_REQUEST, e)))?; + let (token, resolved_user_id) = + matrix_password_login(&fields.homeserver, fields.user_id, fields.password) + .await + .map_err(|e| Box::new(error_problem(StatusCode::BAD_REQUEST, &e)))?; + Ok((token, Some(fields.homeserver), Some(resolved_user_id))) + } + other => Err(Box::new(error_problem( + StatusCode::BAD_REQUEST, + &format!("unknown mode {other:?} (want token|password)"), + ))), + } +} + /// POST `m.login.password` to `/_matrix/client/v3/login`. /// Returns `(access_token, user_id)`. /// @@ -288,3 +344,101 @@ async fn matrix_password_login( .ok_or_else(|| "login response missing user_id".to_owned())?; Ok((token.to_owned(), uid.to_owned())) } + +#[cfg(test)] +mod tests { + use super::{ + PutMatrixAccountRequest, is_reserved_account, password_fields, resolve_credential, + token_credential, + }; + + fn request(mode: &str) -> PutMatrixAccountRequest { + PutMatrixAccountRequest { + mode: mode.to_owned(), + token: None, + user_id: None, + password: None, + homeserver: None, + } + } + + #[test] + fn main_is_reserved_but_nothing_else_is() { + assert!(is_reserved_account("main")); + assert!(!is_reserved_account("ops-relay")); + // Case-sensitive on purpose: `matrixAccounts` is a nix attrset, so + // `Main` is a distinct, legal key from the reserved `main`. + assert!(!is_reserved_account("Main")); + } + + #[test] + fn token_mode_needs_a_token() { + assert!(token_credential(&request("token")).is_err()); + } + + #[test] + fn token_mode_passes_through_token_and_homeserver_unchanged() { + let mut req = request("token"); + req.token = Some("t0k3n".to_owned()); + req.homeserver = Some("https://matrix.example.org".to_owned()); + let (token, homeserver) = token_credential(&req).expect("token + homeserver both given"); + assert_eq!(token, "t0k3n"); + assert_eq!(homeserver.as_deref(), Some("https://matrix.example.org")); + } + + #[test] + fn password_mode_needs_all_three_fields() { + let mut req = request("password"); + assert!(password_fields(&req).is_err(), "none given"); + req.homeserver = Some("https://matrix.example.org".to_owned()); + assert!( + password_fields(&req).is_err(), + "still missing user_id + password" + ); + req.user_id = Some("@a:matrix.example.org".to_owned()); + assert!(password_fields(&req).is_err(), "still missing password"); + req.password = Some("hunter2".to_owned()); + assert!(password_fields(&req).is_ok(), "now all three given"); + } + + #[test] + fn password_mode_trims_a_trailing_slash_off_the_homeserver() { + let mut req = request("password"); + req.homeserver = Some("https://matrix.example.org/".to_owned()); + req.user_id = Some("@a:matrix.example.org".to_owned()); + req.password = Some("hunter2".to_owned()); + let fields = password_fields(&req).expect("all three fields given"); + assert_eq!(fields.homeserver, "https://matrix.example.org"); + } + + #[test] + fn password_mode_leaves_a_homeserver_with_no_trailing_slash_unchanged() { + let mut req = request("password"); + req.homeserver = Some("https://matrix.example.org".to_owned()); + req.user_id = Some("@a:matrix.example.org".to_owned()); + req.password = Some("hunter2".to_owned()); + let fields = password_fields(&req).expect("all three fields given"); + assert_eq!(fields.homeserver, "https://matrix.example.org"); + } + + #[tokio::test] + async fn resolve_credential_token_mode_end_to_end() { + let mut req = request("token"); + req.token = Some("t0k3n".to_owned()); + let (token, homeserver, user_id) = resolve_credential(&req) + .await + .expect("token mode with a token given"); + assert_eq!(token, "t0k3n"); + assert_eq!(homeserver, None); + assert_eq!(user_id, None); + } + + #[tokio::test] + async fn resolve_credential_rejects_an_unknown_mode() { + assert!( + resolve_credential(&request("carrier-pigeon")) + .await + .is_err() + ); + } +}