From 3a794341ceea82fb401922c0c1f79c7b900ce889 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Thu, 10 Jul 2025 21:49:57 -0400 Subject: [PATCH 1/3] Allow disabling automatic creation of clients This may be useful in multi-user deployment scenarios where some external administrative tools are used to create new clients. --- README.md | 5 ++ server/src/api/add_snapshot.rs | 16 +++---- server/src/api/add_version.rs | 50 +++++++++++++++----- server/src/api/get_child_version.rs | 6 +-- server/src/api/get_snapshot.rs | 4 +- server/src/api/mod.rs | 3 ++ server/src/bin/taskchampion-sync-server.rs | 55 +++++++++++++++++++++- server/src/lib.rs | 4 +- 8 files changed, 117 insertions(+), 26 deletions(-) diff --git a/README.md b/README.md index 0c4d0bc..e1763c8 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,11 @@ of UUIDs. Client IDs can be specified with `--allow-client-id`, but this should not be used on shared systems, as command line arguments are visible to all users on the system. +By default, the server will create clients on first contact, so it is easy to +start from an empty database. If you are managing clients in the database +through some other means, disable this behavior with `--no-create-clients` or +`CREATE_CLIENTS=false`. + The server only logs errors by default. To add additional logging output, set environment variable `RUST_LOG` to `info` to get a log message for every request, or to `debug` to get more verbose debugging output. diff --git a/server/src/api/add_snapshot.rs b/server/src/api/add_snapshot.rs index 745a5a9..fb088e1 100644 --- a/server/src/api/add_snapshot.rs +++ b/server/src/api/add_snapshot.rs @@ -76,11 +76,11 @@ mod test { txn.commit()?; } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-snapshot/{}", version_id); + let uri = format!("/v1/client/add-snapshot/{version_id}"); let req = test::TestRequest::post() .uri(&uri) .insert_header(("Content-Type", "application/vnd.taskchampion.snapshot")) @@ -119,12 +119,12 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; // add a snapshot for a nonexistent version - let uri = format!("/v1/client/add-snapshot/{}", version_id); + let uri = format!("/v1/client/add-snapshot/{version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(("Content-Type", "application/vnd.taskchampion.snapshot")) @@ -149,11 +149,11 @@ mod test { let client_id = Uuid::new_v4(); let version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-snapshot/{}", version_id); + let uri = format!("/v1/client/add-snapshot/{version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(("Content-Type", "not/correct")) @@ -169,11 +169,11 @@ mod test { let client_id = Uuid::new_v4(); let version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-snapshot/{}", version_id); + let uri = format!("/v1/client/add-snapshot/{version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(( diff --git a/server/src/api/add_version.rs b/server/src/api/add_version.rs index 32cfad3..c07035d 100644 --- a/server/src/api/add_version.rs +++ b/server/src/api/add_version.rs @@ -80,7 +80,7 @@ pub(crate) async fn service( rb.append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string())); Ok(rb.finish()) } - Err(ServerError::NoSuchClient) => { + Err(ServerError::NoSuchClient) if server_state.create_clients => { // Create a new client and repeat the `add_version` call. let mut txn = server_state .server @@ -118,11 +118,11 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-version/{}", parent_version_id); + let uri = format!("/v1/client/add-version/{parent_version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(( @@ -152,11 +152,11 @@ mod test { let client_id = Uuid::new_v4(); let version_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); - let server = WebServer::new(Default::default(), None, InMemoryStorage::new()); + let server = WebServer::new(Default::default(), None, true, InMemoryStorage::new()); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-version/{}", parent_version_id); + let uri = format!("/v1/client/add-version/{parent_version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(( @@ -190,6 +190,34 @@ mod test { } } + #[actix_rt::test] + async fn test_auto_add_client_disabled() { + let client_id = Uuid::new_v4(); + let parent_version_id = Uuid::new_v4(); + let server = WebServer::new( + Default::default(), + None, + /*create_clients=*/ false, + InMemoryStorage::new(), + ); + let app = App::new().configure(|sc| server.config(sc)); + let app = test::init_service(app).await; + + let uri = format!("/v1/client/add-version/{parent_version_id}"); + let req = test::TestRequest::post() + .uri(&uri) + .append_header(( + "Content-Type", + "application/vnd.taskchampion.history-segment", + )) + .append_header((CLIENT_ID_HEADER, client_id.to_string())) + .set_payload(b"abcd".to_vec()) + .to_request(); + let resp = test::call_service(&app, req).await; + // Client is not added, and returns 404. + assert_eq!(resp.status(), StatusCode::NOT_FOUND); + } + #[actix_rt::test] async fn test_conflict() { let client_id = Uuid::new_v4(); @@ -204,11 +232,11 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-version/{}", parent_version_id); + let uri = format!("/v1/client/add-version/{parent_version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(( @@ -232,11 +260,11 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-version/{}", parent_version_id); + let uri = format!("/v1/client/add-version/{parent_version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(("Content-Type", "not/correct")) @@ -252,11 +280,11 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/add-version/{}", parent_version_id); + let uri = format!("/v1/client/add-version/{parent_version_id}"); let req = test::TestRequest::post() .uri(&uri) .append_header(( diff --git a/server/src/api/get_child_version.rs b/server/src/api/get_child_version.rs index a845617..2341582 100644 --- a/server/src/api/get_child_version.rs +++ b/server/src/api/get_child_version.rs @@ -71,7 +71,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -105,7 +105,7 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -134,7 +134,7 @@ mod test { .unwrap(); txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; diff --git a/server/src/api/get_snapshot.rs b/server/src/api/get_snapshot.rs index 0a1f030..24d616b 100644 --- a/server/src/api/get_snapshot.rs +++ b/server/src/api/get_snapshot.rs @@ -53,7 +53,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -89,7 +89,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, storage); + let server = WebServer::new(Default::default(), None, true, storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs index 5ffb18e..672e795 100644 --- a/server/src/api/mod.rs +++ b/server/src/api/mod.rs @@ -32,6 +32,7 @@ pub(crate) const SNAPSHOT_REQUEST_HEADER: &str = "X-Snapshot-Request"; pub(crate) struct ServerState { pub(crate) server: Server, pub(crate) client_id_allowlist: Option>, + pub(crate) create_clients: bool, } impl ServerState { @@ -87,6 +88,7 @@ mod test { let state = ServerState { server: Server::new(Default::default(), InMemoryStorage::new()), client_id_allowlist: None, + create_clients: true, }; let req = actix_web::test::TestRequest::default() .insert_header((CLIENT_ID_HEADER, client_id.to_string())) @@ -101,6 +103,7 @@ mod test { let state = ServerState { server: Server::new(Default::default(), InMemoryStorage::new()), client_id_allowlist: Some([client_id_ok].into()), + create_clients: true, }; let req = actix_web::test::TestRequest::default() .insert_header((CLIENT_ID_HEADER, client_id_ok.to_string())) diff --git a/server/src/bin/taskchampion-sync-server.rs b/server/src/bin/taskchampion-sync-server.rs index cea68d3..fc3b3de 100644 --- a/server/src/bin/taskchampion-sync-server.rs +++ b/server/src/bin/taskchampion-sync-server.rs @@ -43,6 +43,13 @@ fn command() -> Command { .action(ArgAction::Append) .required(false), ) + .arg( + arg!("create-clients": --"no-create-clients" "If a client does not exist in the database, do not create it") + .env("CREATE_CLIENTS") + .default_value("true") + .action(ArgAction::SetFalse) + .required(false), + ) .arg( arg!(--"snapshot-versions" "Target number of versions between snapshots") .value_parser(value_parser!(u32)) @@ -69,6 +76,7 @@ struct ServerArgs { snapshot_versions: u32, snapshot_days: i64, client_id_allowlist: Option>, + create_clients: bool, listen_addresses: Vec, } @@ -81,6 +89,7 @@ impl ServerArgs { client_id_allowlist: matches .get_many("allow-client-id") .map(|ids| ids.copied().collect()), + create_clients: matches.get_one("create-clients").copied().unwrap_or(true), listen_addresses: matches .get_many::("listen") .unwrap() @@ -103,6 +112,7 @@ async fn main() -> anyhow::Result<()> { let server = WebServer::new( config, server_args.client_id_allowlist, + server_args.create_clients, SqliteStorage::new(server_args.data_dir)?, ); @@ -122,6 +132,8 @@ async fn main() -> anyhow::Result<()> { #[cfg(test)] mod test { + #![allow(clippy::bool_assert_comparison)] + use super::*; use actix_web::{self, App}; use clap::ArgMatches; @@ -309,9 +321,50 @@ mod test { ); } + #[test] + fn command_create_clients_default() { + with_var_unset("CREATE_CLIENTS", || { + let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]); + let server_args = ServerArgs::new(matches); + assert_eq!(server_args.create_clients, true); + }); + } + + #[test] + fn command_create_clients_cmdline() { + with_var_unset("CREATE_CLIENTS", || { + let matches = command().get_matches_from([ + "tss", + "--listen", + "localhost:8080", + "--no-create-clients", + ]); + let server_args = ServerArgs::new(matches); + assert_eq!(server_args.create_clients, false); + }); + } + + #[test] + fn command_create_clients_env_true() { + with_vars([("CREATE_CLIENTS", Some("true"))], || { + let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]); + let server_args = ServerArgs::new(matches); + assert_eq!(server_args.create_clients, true); + }); + } + + #[test] + fn command_create_clients_env_false() { + with_vars([("CREATE_CLIENTS", Some("false"))], || { + let matches = command().get_matches_from(["tss", "--listen", "localhost:8080"]); + let server_args = ServerArgs::new(matches); + assert_eq!(server_args.create_clients, false); + }); + } + #[actix_rt::test] async fn test_index_get() { - let server = WebServer::new(Default::default(), None, InMemoryStorage::new()); + let server = WebServer::new(Default::default(), None, true, InMemoryStorage::new()); let app = App::new().configure(|sc| server.config(sc)); let app = actix_web::test::init_service(app).await; diff --git a/server/src/lib.rs b/server/src/lib.rs index 13827a5..20aa55d 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -24,12 +24,14 @@ impl WebServer { pub fn new( config: ServerConfig, client_id_allowlist: Option>, + create_clients: bool, storage: ST, ) -> Self { Self { server_state: Arc::new(ServerState { server: Server::new(config, storage), client_id_allowlist, + create_clients, }), } } @@ -57,7 +59,7 @@ mod test { #[actix_rt::test] async fn test_cache_control() { - let server = WebServer::new(Default::default(), None, InMemoryStorage::new()); + let server = WebServer::new(Default::default(), None, true, InMemoryStorage::new()); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; From 0a71cce2d19eab067090fab13df5f5ea584db589 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Thu, 10 Jul 2025 22:10:33 -0400 Subject: [PATCH 2/3] use a struct for arguments to WebServer::new --- server/src/api/add_snapshot.rs | 12 +++++----- server/src/api/add_version.rs | 28 +++++++++++++--------- server/src/api/get_child_version.rs | 18 +++++++------- server/src/api/get_snapshot.rs | 8 +++---- server/src/api/mod.rs | 23 ++++++++++-------- server/src/bin/taskchampion-sync-server.rs | 14 +++++++---- server/src/lib.rs | 27 +++++++++++++++++---- 7 files changed, 81 insertions(+), 49 deletions(-) diff --git a/server/src/api/add_snapshot.rs b/server/src/api/add_snapshot.rs index fb088e1..da6b654 100644 --- a/server/src/api/add_snapshot.rs +++ b/server/src/api/add_snapshot.rs @@ -55,11 +55,11 @@ pub(crate) async fn service( #[cfg(test)] mod test { - use crate::api::CLIENT_ID_HEADER; use crate::WebServer; + use crate::{api::CLIENT_ID_HEADER, WebConfig}; use actix_web::{http::StatusCode, test, App}; use pretty_assertions::assert_eq; - use taskchampion_sync_server_core::{InMemoryStorage, Storage, NIL_VERSION_ID}; + use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage, NIL_VERSION_ID}; use uuid::Uuid; #[actix_rt::test] @@ -76,7 +76,7 @@ mod test { txn.commit()?; } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -119,7 +119,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -149,7 +149,7 @@ mod test { let client_id = Uuid::new_v4(); let version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -169,7 +169,7 @@ mod test { let client_id = Uuid::new_v4(); let version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; diff --git a/server/src/api/add_version.rs b/server/src/api/add_version.rs index c07035d..7f7e57b 100644 --- a/server/src/api/add_version.rs +++ b/server/src/api/add_version.rs @@ -80,7 +80,7 @@ pub(crate) async fn service( rb.append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string())); Ok(rb.finish()) } - Err(ServerError::NoSuchClient) if server_state.create_clients => { + Err(ServerError::NoSuchClient) if server_state.web_config.create_clients => { // Create a new client and repeat the `add_version` call. let mut txn = server_state .server @@ -97,11 +97,11 @@ pub(crate) async fn service( #[cfg(test)] mod test { - use crate::api::CLIENT_ID_HEADER; use crate::WebServer; + use crate::{api::CLIENT_ID_HEADER, WebConfig}; use actix_web::{http::StatusCode, test, App}; use pretty_assertions::assert_eq; - use taskchampion_sync_server_core::{InMemoryStorage, Storage}; + use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage}; use uuid::Uuid; #[actix_rt::test] @@ -118,7 +118,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -152,7 +152,11 @@ mod test { let client_id = Uuid::new_v4(); let version_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); - let server = WebServer::new(Default::default(), None, true, InMemoryStorage::new()); + let server = WebServer::new( + ServerConfig::default(), + WebConfig::default(), + InMemoryStorage::new(), + ); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -195,9 +199,11 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let server = WebServer::new( - Default::default(), - None, - /*create_clients=*/ false, + ServerConfig::default(), + WebConfig { + create_clients: false, + ..WebConfig::default() + }, InMemoryStorage::new(), ); let app = App::new().configure(|sc| server.config(sc)); @@ -232,7 +238,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -260,7 +266,7 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -280,7 +286,7 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; diff --git a/server/src/api/get_child_version.rs b/server/src/api/get_child_version.rs index 2341582..9f21b0f 100644 --- a/server/src/api/get_child_version.rs +++ b/server/src/api/get_child_version.rs @@ -48,11 +48,11 @@ pub(crate) async fn service( #[cfg(test)] mod test { - use crate::api::CLIENT_ID_HEADER; use crate::WebServer; + use crate::{api::CLIENT_ID_HEADER, WebConfig}; use actix_web::{http::StatusCode, test, App}; use pretty_assertions::assert_eq; - use taskchampion_sync_server_core::{InMemoryStorage, Storage, NIL_VERSION_ID}; + use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage, NIL_VERSION_ID}; use uuid::Uuid; #[actix_rt::test] @@ -71,11 +71,11 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/get-child-version/{}", parent_version_id); + let uri = format!("/v1/client/get-child-version/{parent_version_id}"); let req = test::TestRequest::get() .uri(&uri) .append_header((CLIENT_ID_HEADER, client_id.to_string())) @@ -105,11 +105,11 @@ mod test { let client_id = Uuid::new_v4(); let parent_version_id = Uuid::new_v4(); let storage = InMemoryStorage::new(); - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; - let uri = format!("/v1/client/get-child-version/{}", parent_version_id); + let uri = format!("/v1/client/get-child-version/{parent_version_id}"); let req = test::TestRequest::get() .uri(&uri) .append_header((CLIENT_ID_HEADER, client_id.to_string())) @@ -134,12 +134,12 @@ mod test { .unwrap(); txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; // the child of the nil version is the added version - let uri = format!("/v1/client/get-child-version/{}", NIL_VERSION_ID); + let uri = format!("/v1/client/get-child-version/{NIL_VERSION_ID}"); let req = test::TestRequest::get() .uri(&uri) .append_header((CLIENT_ID_HEADER, client_id.to_string())) @@ -168,7 +168,7 @@ mod test { // The child of the latest version is NOT_FOUND. The tests in crate::server test more // corner cases. - let uri = format!("/v1/client/get-child-version/{}", test_version_id); + let uri = format!("/v1/client/get-child-version/{test_version_id}"); let req = test::TestRequest::get() .uri(&uri) .append_header((CLIENT_ID_HEADER, client_id.to_string())) diff --git a/server/src/api/get_snapshot.rs b/server/src/api/get_snapshot.rs index 24d616b..d7aabdb 100644 --- a/server/src/api/get_snapshot.rs +++ b/server/src/api/get_snapshot.rs @@ -33,12 +33,12 @@ pub(crate) async fn service( #[cfg(test)] mod test { - use crate::api::CLIENT_ID_HEADER; use crate::WebServer; + use crate::{api::CLIENT_ID_HEADER, WebConfig}; use actix_web::{http::StatusCode, test, App}; use chrono::{TimeZone, Utc}; use pretty_assertions::assert_eq; - use taskchampion_sync_server_core::{InMemoryStorage, Snapshot, Storage}; + use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Snapshot, Storage}; use uuid::Uuid; #[actix_rt::test] @@ -53,7 +53,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; @@ -89,7 +89,7 @@ mod test { txn.commit().unwrap(); } - let server = WebServer::new(Default::default(), None, true, storage); + let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; diff --git a/server/src/api/mod.rs b/server/src/api/mod.rs index 672e795..ec19a5b 100644 --- a/server/src/api/mod.rs +++ b/server/src/api/mod.rs @@ -1,8 +1,7 @@ -use std::collections::HashSet; - use actix_web::{error, web, HttpRequest, Result, Scope}; use taskchampion_sync_server_core::{ClientId, Server, ServerError}; -use uuid::Uuid; + +use crate::WebConfig; mod add_snapshot; mod add_version; @@ -31,8 +30,7 @@ pub(crate) const SNAPSHOT_REQUEST_HEADER: &str = "X-Snapshot-Request"; /// The type containing a reference to the persistent state for the server pub(crate) struct ServerState { pub(crate) server: Server, - pub(crate) client_id_allowlist: Option>, - pub(crate) create_clients: bool, + pub(crate) web_config: WebConfig, } impl ServerState { @@ -44,7 +42,7 @@ impl ServerState { if let Some(client_id_hdr) = req.headers().get(CLIENT_ID_HEADER) { let client_id = client_id_hdr.to_str().map_err(|_| badrequest())?; let client_id = ClientId::parse_str(client_id).map_err(|_| badrequest())?; - if let Some(allow_list) = &self.client_id_allowlist { + if let Some(allow_list) = &self.web_config.client_id_allowlist { if !allow_list.contains(&client_id) { return Err(error::ErrorForbidden("unknown x-client-id")); } @@ -81,14 +79,17 @@ fn server_error_to_actix(err: ServerError) -> actix_web::Error { mod test { use super::*; use taskchampion_sync_server_core::InMemoryStorage; + use uuid::Uuid; #[test] fn client_id_header_allow_all() { let client_id = Uuid::new_v4(); let state = ServerState { server: Server::new(Default::default(), InMemoryStorage::new()), - client_id_allowlist: None, - create_clients: true, + web_config: WebConfig { + client_id_allowlist: None, + create_clients: true, + }, }; let req = actix_web::test::TestRequest::default() .insert_header((CLIENT_ID_HEADER, client_id.to_string())) @@ -102,8 +103,10 @@ mod test { let client_id_disallowed = Uuid::new_v4(); let state = ServerState { server: Server::new(Default::default(), InMemoryStorage::new()), - client_id_allowlist: Some([client_id_ok].into()), - create_clients: true, + web_config: WebConfig { + client_id_allowlist: Some([client_id_ok].into()), + create_clients: true, + }, }; let req = actix_web::test::TestRequest::default() .insert_header((CLIENT_ID_HEADER, client_id_ok.to_string())) diff --git a/server/src/bin/taskchampion-sync-server.rs b/server/src/bin/taskchampion-sync-server.rs index fc3b3de..bc59d01 100644 --- a/server/src/bin/taskchampion-sync-server.rs +++ b/server/src/bin/taskchampion-sync-server.rs @@ -8,7 +8,7 @@ use actix_web::{ }; use clap::{arg, builder::ValueParser, value_parser, ArgAction, Command}; use std::{collections::HashSet, ffi::OsString}; -use taskchampion_sync_server::WebServer; +use taskchampion_sync_server::{WebConfig, WebServer}; use taskchampion_sync_server_core::ServerConfig; use taskchampion_sync_server_storage_sqlite::SqliteStorage; use uuid::Uuid; @@ -111,8 +111,10 @@ async fn main() -> anyhow::Result<()> { }; let server = WebServer::new( config, - server_args.client_id_allowlist, - server_args.create_clients, + WebConfig { + client_id_allowlist: server_args.client_id_allowlist, + create_clients: server_args.create_clients, + }, SqliteStorage::new(server_args.data_dir)?, ); @@ -364,7 +366,11 @@ mod test { #[actix_rt::test] async fn test_index_get() { - let server = WebServer::new(Default::default(), None, true, InMemoryStorage::new()); + let server = WebServer::new( + ServerConfig::default(), + WebConfig::default(), + InMemoryStorage::new(), + ); let app = App::new().configure(|sc| server.config(sc)); let app = actix_web::test::init_service(app).await; diff --git a/server/src/lib.rs b/server/src/lib.rs index 20aa55d..46cded5 100644 --- a/server/src/lib.rs +++ b/server/src/lib.rs @@ -19,19 +19,32 @@ pub struct WebServer { server_state: Arc, } +/// Configuration for WebServer (as distinct from [`ServerConfig`]). +pub struct WebConfig { + pub client_id_allowlist: Option>, + pub create_clients: bool, +} + +impl Default for WebConfig { + fn default() -> Self { + Self { + client_id_allowlist: Default::default(), + create_clients: true, + } + } +} + impl WebServer { /// Create a new sync server with the given storage implementation. pub fn new( config: ServerConfig, - client_id_allowlist: Option>, - create_clients: bool, + web_config: WebConfig, storage: ST, ) -> Self { Self { server_state: Arc::new(ServerState { server: Server::new(config, storage), - client_id_allowlist, - create_clients, + web_config, }), } } @@ -59,7 +72,11 @@ mod test { #[actix_rt::test] async fn test_cache_control() { - let server = WebServer::new(Default::default(), None, true, InMemoryStorage::new()); + let server = WebServer::new( + ServerConfig::default(), + WebConfig::default(), + InMemoryStorage::new(), + ); let app = App::new().configure(|sc| server.config(sc)); let app = test::init_service(app).await; From 67576fe382c61217c36894f7c5371729f0b21b48 Mon Sep 17 00:00:00 2001 From: "Dustin J. Mitchell" Date: Sat, 12 Jul 2025 09:48:15 -0400 Subject: [PATCH 3/3] suggestion from @ryneeverett --- README.md | 22 +++++++++++++--------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/README.md b/README.md index e1763c8..3af91f0 100644 --- a/README.md +++ b/README.md @@ -85,16 +85,20 @@ list of values. The `--data-dir` option specifies where the server should store its data. This value can be specified in the environment variable `DATA_DIR`. -By default, the server allows all client IDs. To limit the accepted client IDs, -specify them in the environment variable `CLIENT_ID`, as a comma-separated list -of UUIDs. Client IDs can be specified with `--allow-client-id`, but this should -not be used on shared systems, as command line arguments are visible to all -users on the system. +By default, the server will allow all clients and create them in the database +on first contact. There are two ways to limit the clients the server will +interact with: -By default, the server will create clients on first contact, so it is easy to -start from an empty database. If you are managing clients in the database -through some other means, disable this behavior with `--no-create-clients` or -`CREATE_CLIENTS=false`. +- To limit the accepted client IDs, specify them in the environment variable +`CLIENT_ID`, as a comma-separated list of UUIDs. Client IDs can be specified +with `--allow-client-id`, but this should not be used on shared systems, as +command line arguments are visible to all users on the system. This convenient +option is suitable for personal and small-scale deployments. + +- To disable the automatic creation of clients, use the `--no-create-clients` +flag or the `CREATE_CLIENTS=false` environment variable. You are now +responsible for creating clients in the database manually, so this option is +more suitable for large scale deployments. The server only logs errors by default. To add additional logging output, set environment variable `RUST_LOG` to `info` to get a log message for every