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

@@ -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.

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((

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((

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;
 

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;
 

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<HashSet<Uuid>>,
+    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()))

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" <NUM> "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<HashSet<Uuid>>,
+    create_clients: bool,
     listen_addresses: Vec<String>,
 }
 
@@ -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::<String>("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;
 

server/src/lib.rs

@@ -24,12 +24,14 @@ impl WebServer {
     pub fn new<ST: Storage + 'static>(
         config: ServerConfig,
         client_id_allowlist: Option<HashSet<Uuid>>,
+        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;