diff --git a/swarm-controller/src/forge_account.rs b/swarm-controller/src/forge_account.rs index d6558f98..8cdead36 100644 --- a/swarm-controller/src/forge_account.rs +++ b/swarm-controller/src/forge_account.rs @@ -72,7 +72,7 @@ pub async fn put_forge_account( .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; let account = account(req).map_err(|e| error_problem(StatusCode::BAD_REQUEST, e))?; - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -108,6 +108,9 @@ fn account(req: PutForgeAccountRequest) -> Result #[cfg(test)] mod tests { + use std::sync::Arc; + + use super::super::linked_accounts::{Accounts, tests::FakeStore}; use super::{PutForgeAccountRequest, account}; fn request(url: &str, token: &str) -> PutForgeAccountRequest { @@ -138,7 +141,7 @@ mod tests { } /// Bare-minimum `AppState`, as `matrix_account`'s tests build it. - fn state() -> super::super::AppState { + fn state(store: &Arc) -> super::super::AppState { super::super::AppState { hives: std::sync::Arc::new(vec![super::super::HiveEntry { name: "pr1ma".to_owned(), @@ -159,53 +162,49 @@ mod tests { auth: None, forge: None, create_gate: std::sync::Arc::default(), + accounts: Accounts::Fake(Arc::clone(store)), } } - async fn put(label: &str) -> problem_details::ProblemDetails { + async fn put( + store: &Arc, + label: &str, + ) -> Result, problem_details::ProblemDetails> { super::put_forge_account( - axum::extract::State(state()), + axum::extract::State(state(store)), axum::extract::Path(("pr1ma".to_owned(), "atlas".to_owned(), label.to_owned())), axum::Json(request("https://codeberg.org", "t0k3n")), ) .await - .expect_err("no store is configured in a test") } - /// A label `hive-forge -f` could not name is refused before the store: with - /// `BAO_*` unset a store connect would answer 500. + /// A label `hive-forge -f` could not name is refused before the store. #[tokio::test] async fn a_label_hive_forge_cannot_name_is_refused_before_the_store() { - for var in ["BAO_ADDR", "BAO_CLIENT_CERT", "BAO_CLIENT_KEY"] { - assert!( - std::env::var(var).is_err(), - "{var} must be unset for this test to prove anything" - ); - } + let store = Arc::default(); for bad in ["Codeberg", "code_berg", "../x"] { - let problem = put(bad).await; + let problem = put(&store, bad).await.expect_err("refused"); assert_eq!( problem.status, Some(axum::http::StatusCode::BAD_REQUEST), "{bad}: {problem:?}" ); } + assert!(store.written().is_empty()); } - /// The control: a plain label reaches the store connect. + /// The control: a plain label is written. #[tokio::test] async fn a_plain_label_reaches_the_store() { - let problem = put("codeberg").await; - assert_eq!( - problem.status, - Some(axum::http::StatusCode::INTERNAL_SERVER_ERROR), - "{problem:?}" - ); + let store = Arc::default(); + let stored = put(&store, "codeberg").await.expect("nothing is stored"); + assert_eq!(stored.url, "https://codeberg.org"); + assert_eq!(store.written(), ["swarm/agents/atlas/forge/codeberg"]); } #[tokio::test] async fn linking_a_label_twice_is_a_409_and_the_first_account_stays() { - use super::super::linked_accounts::{AccountStore, link, tests::FakeStore}; + use super::super::linked_accounts::{AccountStore, link}; use swarm_secret_client::forge; let store = FakeStore::default(); diff --git a/swarm-controller/src/github_account.rs b/swarm-controller/src/github_account.rs index 998419e1..c143fcad 100644 --- a/swarm-controller/src/github_account.rs +++ b/swarm-controller/src/github_account.rs @@ -55,7 +55,7 @@ pub async fn put_github_account( .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; let credential = credential(&req).map_err(|e| error_problem(StatusCode::BAD_REQUEST, e))?; - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -87,6 +87,9 @@ fn credential(req: &PutGithubAccountRequest) -> Result PutGithubAccountRequest { @@ -108,7 +111,7 @@ mod tests { } /// Bare-minimum `AppState`, as `forge_account`'s tests build it. - fn state() -> super::super::AppState { + fn state(store: &Arc) -> super::super::AppState { super::super::AppState { hives: std::sync::Arc::new(vec![super::super::HiveEntry { name: "pr1ma".to_owned(), @@ -129,58 +132,61 @@ mod tests { auth: None, forge: None, create_gate: std::sync::Arc::default(), + accounts: Accounts::Fake(Arc::clone(store)), } } - async fn put(agent: &str, token: &str) -> problem_details::ProblemDetails { + async fn put( + store: &Arc, + agent: &str, + token: &str, + ) -> Result { super::put_github_account( - axum::extract::State(state()), + axum::extract::State(state(store)), axum::extract::Path(("pr1ma".to_owned(), agent.to_owned())), axum::Json(request(token)), ) .await - .expect_err("no store is configured in a test") } - fn assert_store_unset() { - for var in ["BAO_ADDR", "BAO_CLIENT_CERT", "BAO_CLIENT_KEY"] { - assert!( - std::env::var(var).is_err(), - "{var} must be unset for this test to prove anything" - ); - } - } - - /// An agent name or an empty token is refused before the store: with - /// `BAO_*` unset a store connect would answer 500. + /// An agent name or an empty token is refused before the store. #[tokio::test] async fn a_bad_agent_or_an_empty_token_is_refused_before_the_store() { - assert_store_unset(); + let store = Arc::default(); for (agent, token) in [("Atlas", "t0k3n"), ("../x", "t0k3n"), ("atlas", " ")] { - let problem = put(agent, token).await; + let problem = put(&store, agent, token).await.expect_err("refused"); assert_eq!( problem.status, Some(axum::http::StatusCode::BAD_REQUEST), "{agent:?}: {problem:?}" ); } + assert!(store.written().is_empty()); } - /// The control: a plain agent and a token reach the store connect. + /// The control: a plain agent and a token are written. #[tokio::test] async fn a_plain_request_reaches_the_store() { - assert_store_unset(); - let problem = put("atlas", "t0k3n").await; - assert_eq!( - problem.status, - Some(axum::http::StatusCode::INTERNAL_SERVER_ERROR), - "{problem:?}" - ); + use super::super::linked_accounts::AccountStore; + use swarm_secret_client::github; + + let store = Arc::default(); + let status = put(&store, "atlas", "t0k3n") + .await + .expect("nothing is stored"); + assert_eq!(status, axum::http::StatusCode::NO_CONTENT); + assert_eq!(store.written(), ["swarm/agents/atlas/github-token"]); + let stored: github::Credential = store + .read_optional("swarm/agents/atlas/github-token") + .await + .expect("store answers") + .expect("stored"); + assert_eq!(stored.value, "t0k3n"); } #[tokio::test] async fn linking_a_second_token_is_a_409_and_the_first_stays() { - use super::super::linked_accounts::{AccountStore, link, tests::FakeStore}; + use super::super::linked_accounts::{AccountStore, link}; use swarm_secret_client::github; let store = FakeStore::default(); diff --git a/swarm-controller/src/linked_accounts.rs b/swarm-controller/src/linked_accounts.rs index 99ca8f14..5debff72 100644 --- a/swarm-controller/src/linked_accounts.rs +++ b/swarm-controller/src/linked_accounts.rs @@ -104,6 +104,78 @@ impl AccountStore for SecretStore { } } +/// The store the link, delete and list routes use, as [`AppState`] holds it. +#[derive(Clone)] +pub(crate) enum Accounts { + /// The swarm's secret store, logged in to afresh on every request: a + /// [`SecretStore`] holds a token it never renews, and a store that is down + /// fails only the requests made while it is down. + Live, + /// An in-memory store a test reads back. + #[cfg(test)] + Fake(std::sync::Arc), +} + +impl Accounts { + /// A store to make one request's calls on. + /// + /// # Errors + /// Whatever [`crate::store::connect`] raises. + pub(crate) async fn open(&self) -> Result { + match self { + Self::Live => crate::store::connect() + .await + .map(|s| OpenAccounts::Live(Box::new(s))), + #[cfg(test)] + Self::Fake(store) => Ok(OpenAccounts::Fake(store.clone())), + } + } +} + +/// One request's store, from [`Accounts::open`]. +pub(crate) enum OpenAccounts { + Live(Box), + #[cfg(test)] + Fake(std::sync::Arc), +} + +impl AccountStore for OpenAccounts { + async fn list(&self, dir: &str) -> Result, Error> { + match self { + Self::Live(s) => s.list(dir).await, + #[cfg(test)] + Self::Fake(s) => s.list(dir).await, + } + } + + async fn read_optional( + &self, + path: &str, + ) -> Result, Error> { + match self { + Self::Live(s) => s.read_optional(path).await, + #[cfg(test)] + Self::Fake(s) => s.read_optional(path).await, + } + } + + async fn write(&self, path: &str, value: &T) -> Result<(), Error> { + match self { + Self::Live(s) => s.write(path, value).await, + #[cfg(test)] + Self::Fake(s) => s.write(path, value).await, + } + } + + async fn delete_all_versions(&self, path: &str) -> Result<(), Error> { + match self { + Self::Live(s) => s.delete_all_versions(path).await, + #[cfg(test)] + Self::Fake(s) => s.delete_all_versions(path).await, + } + } +} + /// The object names directly under `dir`. A key ending in `/` is a directory /// below it, not an account. async fn objects(store: &impl AccountStore, dir: &str) -> Result, Error> { @@ -192,7 +264,7 @@ pub async fn get_linked_accounts( .map_err(|reason| error_problem(StatusCode::BAD_REQUEST, reason))? .into_string(); - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -383,7 +455,7 @@ pub async fn delete_matrix_account( )); } - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -433,7 +505,7 @@ pub async fn delete_forge_account( let path = forge::account_path(&agent, &label) .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -475,7 +547,7 @@ pub async fn delete_github_account( let path = github::account_path(&agent) .map_err(|e| error_problem(StatusCode::BAD_REQUEST, &e.to_string()))?; - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -493,7 +565,7 @@ pub async fn delete_github_account( #[cfg(test)] pub(crate) mod tests { use std::collections::BTreeMap; - use std::sync::Mutex; + use std::sync::{Arc, Mutex}; use serde::Serialize; use serde::de::DeserializeOwned; @@ -874,7 +946,7 @@ pub(crate) mod tests { } /// Bare-minimum `AppState`, as `matrix_account`'s tests build it. - fn state() -> super::super::AppState { + fn state(store: &Arc) -> super::super::AppState { super::super::AppState { hives: std::sync::Arc::new(vec![super::super::HiveEntry { name: "pr1ma".to_owned(), @@ -895,30 +967,29 @@ pub(crate) mod tests { auth: None, forge: None, create_gate: std::sync::Arc::default(), + accounts: super::Accounts::Fake(Arc::clone(store)), } } - async fn delete_matrix(account: &str) -> problem_details::ProblemDetails { + async fn delete_matrix( + store: &Arc, + account: &str, + revoke: bool, + ) -> Result { super::delete_matrix_account( - axum::extract::State(state()), + axum::extract::State(state(store)), axum::extract::Path(("pr1ma".to_owned(), "atlas".to_owned(), account.to_owned())), - axum::extract::Query(super::DeleteMatrixAccountQuery { revoke: true }), + axum::extract::Query(super::DeleteMatrixAccountQuery { revoke }), ) .await - .expect_err("no store is configured in a test") } - /// Refused before the store, and before any revoke: with `BAO_*` unset a - /// store connect would answer 500. + /// Refused before the store, and before any revoke. #[tokio::test] async fn the_agents_own_matrix_account_cannot_be_deleted() { - for var in ["BAO_ADDR", "BAO_CLIENT_CERT", "BAO_CLIENT_KEY"] { - assert!( - std::env::var(var).is_err(), - "{var} must be unset for this test to prove anything" - ); - } - let problem = delete_matrix("main").await; + let problem = delete_matrix(&Arc::default(), "main", true) + .await + .expect_err("'main' is reserved"); assert_eq!( problem.status, Some(axum::http::StatusCode::BAD_REQUEST), @@ -926,14 +997,14 @@ pub(crate) mod tests { ); } - /// The control: any other account name reaches the store connect. + /// The control: any other account name is deleted. #[tokio::test] async fn another_matrix_account_reaches_the_store() { - let problem = delete_matrix("catgirl").await; - assert_eq!( - problem.status, - Some(axum::http::StatusCode::INTERNAL_SERVER_ERROR), - "{problem:?}" - ); + let store = Arc::new(every_kind()); + let status = delete_matrix(&store, "catgirl", false) + .await + .expect("stored"); + assert_eq!(status, axum::http::StatusCode::NO_CONTENT); + assert_eq!(deleted(&store), ["swarm/agents/atlas/matrix/catgirl"]); } } diff --git a/swarm-controller/src/main.rs b/swarm-controller/src/main.rs index 64209c94..51b2f384 100644 --- a/swarm-controller/src/main.rs +++ b/swarm-controller/src/main.rs @@ -733,6 +733,8 @@ struct AppState { /// find it unplaced. `tokio`'s mutex, unlike `jobq`'s: the read awaits /// the queue. create_gate: Arc>, + /// The store the linked-account routes read and write. + accounts: linked_accounts::Accounts, } /// Env var the controller's NixOS module sets from @@ -2957,6 +2959,7 @@ async fn main() -> Result<()> { auth, forge: state_forge, create_gate: Arc::default(), + accounts: linked_accounts::Accounts::Live, }; let app = build_app(state); @@ -3129,6 +3132,7 @@ mod tests { auth: None, forge: None, create_gate: std::sync::Arc::default(), + accounts: super::linked_accounts::Accounts::Fake(std::sync::Arc::default()), }; (state, sched) } diff --git a/swarm-controller/src/matrix_account.rs b/swarm-controller/src/matrix_account.rs index f53b06ac..17aa7c77 100644 --- a/swarm-controller/src/matrix_account.rs +++ b/swarm-controller/src/matrix_account.rs @@ -166,7 +166,7 @@ pub async fn put_matrix_account( )); } - let store = crate::store::connect().await.map_err(|e| { + let store = state.accounts.open().await.map_err(|e| { tracing::warn!(error = %e, "connecting to the swarm secret store failed"); error_problem(StatusCode::INTERNAL_SERVER_ERROR, &e.to_string()) })?; @@ -425,6 +425,9 @@ pub(crate) async fn matrix_logout(homeserver: &str, token: &str) -> Result<(), S #[cfg(test)] mod tests { + use std::sync::Arc; + + use super::super::linked_accounts::{Accounts, tests::FakeStore}; use super::{ PutMatrixAccountRequest, homeserver_or_configured_default, is_reserved_account, matrix_logout, password_fields, resolve_credential, token_credential, @@ -547,9 +550,9 @@ mod tests { } /// Bare-minimum `AppState` for a handler test: one hive, nothing - /// queue-backed, and an empty in-memory job graph the endpoint under test - /// never touches. - fn state() -> super::super::AppState { + /// queue-backed, an empty in-memory job graph the endpoint under test + /// never touches, and `store` as the account store. + fn state(store: &Arc) -> super::super::AppState { super::super::AppState { hives: std::sync::Arc::new(vec![super::super::HiveEntry { name: "pr1ma".to_owned(), @@ -570,21 +573,16 @@ mod tests { auth: None, forge: None, create_gate: std::sync::Arc::default(), + accounts: Accounts::Fake(Arc::clone(store)), } } - /// Refused before the store is reached: with `BAO_*` unset a store - /// connect would answer 500, so a 400 is the reserved-name check. + /// Refused before the store is reached. #[tokio::test] async fn a_reserved_account_name_is_refused_before_the_store() { - for var in ["BAO_ADDR", "BAO_CLIENT_CERT", "BAO_CLIENT_KEY"] { - assert!( - std::env::var(var).is_err(), - "{var} must be unset for this test to prove anything" - ); - } + let store = Arc::default(); let result = super::put_matrix_account( - axum::extract::State(state()), + axum::extract::State(state(&store)), axum::extract::Path(("pr1ma".to_owned(), "atlas".to_owned(), "main".to_owned())), axum::Json(PutMatrixAccountRequest { mode: "token".to_owned(), @@ -602,15 +600,19 @@ mod tests { Some(axum::http::StatusCode::BAD_REQUEST), "{problem:?}" ); + assert!(store.written().is_empty()); } /// The control for the test above: an ordinary name gets past every check - /// and fails at the store connect, so the 400 above is not what every call - /// answers. + /// and is written, so the 400 above is not what every call answers. #[tokio::test] async fn an_ordinary_account_name_reaches_the_store() { + use super::super::linked_accounts::AccountStore; + use swarm_secret_client::matrix; + + let store = Arc::default(); let result = super::put_matrix_account( - axum::extract::State(state()), + axum::extract::State(state(&store)), axum::extract::Path(( "pr1ma".to_owned(), "atlas".to_owned(), @@ -626,19 +628,23 @@ mod tests { ) .await; - let problem = result.expect_err("no store is configured in a test"); - assert_eq!( - problem.status, - Some(axum::http::StatusCode::INTERNAL_SERVER_ERROR), - "{problem:?}" - ); + let response = result.expect("nothing is stored"); + assert_eq!(response.user_id, None); + let path = "swarm/agents/atlas/matrix/workaccount"; + assert_eq!(store.written(), [path]); + let stored: matrix::Credential = store + .read_optional(path) + .await + .expect("store answers") + .expect("stored"); + assert_eq!(stored.value, "t0k3n"); } /// The second link is password mode against a port nothing listens on: /// a login attempt would answer 400, so the 409 is the check running first. #[tokio::test] async fn linking_a_name_twice_is_a_409_and_the_first_account_stays() { - use super::super::linked_accounts::{AccountStore, tests::FakeStore}; + use super::super::linked_accounts::AccountStore; use swarm_secret_client::matrix; let store = FakeStore::default();