Skip to content

Commit 87d1d02

Browse files
djmitchedjmitche
authored
Merge pull request #125 from djmitche/issue121
Allow disabling automatic creation of clients
2 parents 61b9293 + 67576fe commit 87d1d02

File tree

8 files changed

+181
-54
lines changed

8 files changed

+181
-54
lines changed

‎README.md‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -85,11 +85,20 @@ list of values.
8585
The `--data-dir` option specifies where the server should store its data. This
8686
value can be specified in the environment variable `DATA_DIR`.
8787

88-
By default, the server allows all client IDs. To limit the accepted client IDs,
89-
specify them in the environment variable `CLIENT_ID`, as a comma-separated list
90-
of UUIDs. Client IDs can be specified with `--allow-client-id`, but this should
91-
not be used on shared systems, as command line arguments are visible to all
92-
users on the system.
88+
By default, the server will allow all clients and create them in the database
89+
on first contact. There are two ways to limit the clients the server will
90+
interact with:
91+
92+
- To limit the accepted client IDs, specify them in the environment variable
93+
`CLIENT_ID`, as a comma-separated list of UUIDs. Client IDs can be specified
94+
with `--allow-client-id`, but this should not be used on shared systems, as
95+
command line arguments are visible to all users on the system. This convenient
96+
option is suitable for personal and small-scale deployments.
97+
98+
- To disable the automatic creation of clients, use the `--no-create-clients`
99+
flag or the `CREATE_CLIENTS=false` environment variable. You are now
100+
responsible for creating clients in the database manually, so this option is
101+
more suitable for large scale deployments.
93102

94103
The server only logs errors by default. To add additional logging output, set
95104
environment variable `RUST_LOG` to `info` to get a log message for every

‎server/src/api/add_snapshot.rs‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,11 @@ pub(crate) async fn service(
5555

5656
#[cfg(test)]
5757
mod test {
58-
use crate::api::CLIENT_ID_HEADER;
5958
use crate::WebServer;
59+
use crate::{api::CLIENT_ID_HEADER, WebConfig};
6060
use actix_web::{http::StatusCode, test, App};
6161
use pretty_assertions::assert_eq;
62-
use taskchampion_sync_server_core::{InMemoryStorage, Storage, NIL_VERSION_ID};
62+
use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage, NIL_VERSION_ID};
6363
use uuid::Uuid;
6464

6565
#[actix_rt::test]
@@ -76,11 +76,11 @@ mod test {
7676
txn.commit()?;
7777
}
7878

79-
let server = WebServer::new(Default::default(), None, storage);
79+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
8080
let app = App::new().configure(|sc| server.config(sc));
8181
let app = test::init_service(app).await;
8282

83-
let uri = format!("/v1/client/add-snapshot/{}", version_id);
83+
let uri = format!("/v1/client/add-snapshot/{version_id}");
8484
let req = test::TestRequest::post()
8585
.uri(&uri)
8686
.insert_header(("Content-Type", "application/vnd.taskchampion.snapshot"))
@@ -119,12 +119,12 @@ mod test {
119119
txn.commit().unwrap();
120120
}
121121

122-
let server = WebServer::new(Default::default(), None, storage);
122+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
123123
let app = App::new().configure(|sc| server.config(sc));
124124
let app = test::init_service(app).await;
125125

126126
// add a snapshot for a nonexistent version
127-
let uri = format!("/v1/client/add-snapshot/{}", version_id);
127+
let uri = format!("/v1/client/add-snapshot/{version_id}");
128128
let req = test::TestRequest::post()
129129
.uri(&uri)
130130
.append_header(("Content-Type", "application/vnd.taskchampion.snapshot"))
@@ -149,11 +149,11 @@ mod test {
149149
let client_id = Uuid::new_v4();
150150
let version_id = Uuid::new_v4();
151151
let storage = InMemoryStorage::new();
152-
let server = WebServer::new(Default::default(), None, storage);
152+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
153153
let app = App::new().configure(|sc| server.config(sc));
154154
let app = test::init_service(app).await;
155155

156-
let uri = format!("/v1/client/add-snapshot/{}", version_id);
156+
let uri = format!("/v1/client/add-snapshot/{version_id}");
157157
let req = test::TestRequest::post()
158158
.uri(&uri)
159159
.append_header(("Content-Type", "not/correct"))
@@ -169,11 +169,11 @@ mod test {
169169
let client_id = Uuid::new_v4();
170170
let version_id = Uuid::new_v4();
171171
let storage = InMemoryStorage::new();
172-
let server = WebServer::new(Default::default(), None, storage);
172+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
173173
let app = App::new().configure(|sc| server.config(sc));
174174
let app = test::init_service(app).await;
175175

176-
let uri = format!("/v1/client/add-snapshot/{}", version_id);
176+
let uri = format!("/v1/client/add-snapshot/{version_id}");
177177
let req = test::TestRequest::post()
178178
.uri(&uri)
179179
.append_header((

‎server/src/api/add_version.rs‎

Lines changed: 47 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,7 @@ pub(crate) async fn service(
8080
rb.append_header((PARENT_VERSION_ID_HEADER, parent_version_id.to_string()));
8181
Ok(rb.finish())
8282
}
83-
Err(ServerError::NoSuchClient) => {
83+
Err(ServerError::NoSuchClient) if server_state.web_config.create_clients => {
8484
// Create a new client and repeat the `add_version` call.
8585
let mut txn = server_state
8686
.server
@@ -97,11 +97,11 @@ pub(crate) async fn service(
9797

9898
#[cfg(test)]
9999
mod test {
100-
use crate::api::CLIENT_ID_HEADER;
101100
use crate::WebServer;
101+
use crate::{api::CLIENT_ID_HEADER, WebConfig};
102102
use actix_web::{http::StatusCode, test, App};
103103
use pretty_assertions::assert_eq;
104-
use taskchampion_sync_server_core::{InMemoryStorage, Storage};
104+
use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage};
105105
use uuid::Uuid;
106106

107107
#[actix_rt::test]
@@ -118,11 +118,11 @@ mod test {
118118
txn.commit().unwrap();
119119
}
120120

121-
let server = WebServer::new(Default::default(), None, storage);
121+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
122122
let app = App::new().configure(|sc| server.config(sc));
123123
let app = test::init_service(app).await;
124124

125-
let uri = format!("/v1/client/add-version/{}", parent_version_id);
125+
let uri = format!("/v1/client/add-version/{parent_version_id}");
126126
let req = test::TestRequest::post()
127127
.uri(&uri)
128128
.append_header((
@@ -152,11 +152,15 @@ mod test {
152152
let client_id = Uuid::new_v4();
153153
let version_id = Uuid::new_v4();
154154
let parent_version_id = Uuid::new_v4();
155-
let server = WebServer::new(Default::default(), None, InMemoryStorage::new());
155+
let server = WebServer::new(
156+
ServerConfig::default(),
157+
WebConfig::default(),
158+
InMemoryStorage::new(),
159+
);
156160
let app = App::new().configure(|sc| server.config(sc));
157161
let app = test::init_service(app).await;
158162

159-
let uri = format!("/v1/client/add-version/{}", parent_version_id);
163+
let uri = format!("/v1/client/add-version/{parent_version_id}");
160164
let req = test::TestRequest::post()
161165
.uri(&uri)
162166
.append_header((
@@ -190,6 +194,36 @@ mod test {
190194
}
191195
}
192196

197+
#[actix_rt::test]
198+
async fn test_auto_add_client_disabled() {
199+
let client_id = Uuid::new_v4();
200+
let parent_version_id = Uuid::new_v4();
201+
let server = WebServer::new(
202+
ServerConfig::default(),
203+
WebConfig {
204+
create_clients: false,
205+
..WebConfig::default()
206+
},
207+
InMemoryStorage::new(),
208+
);
209+
let app = App::new().configure(|sc| server.config(sc));
210+
let app = test::init_service(app).await;
211+
212+
let uri = format!("/v1/client/add-version/{parent_version_id}");
213+
let req = test::TestRequest::post()
214+
.uri(&uri)
215+
.append_header((
216+
"Content-Type",
217+
"application/vnd.taskchampion.history-segment",
218+
))
219+
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
220+
.set_payload(b"abcd".to_vec())
221+
.to_request();
222+
let resp = test::call_service(&app, req).await;
223+
// Client is not added, and returns 404.
224+
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
225+
}
226+
193227
#[actix_rt::test]
194228
async fn test_conflict() {
195229
let client_id = Uuid::new_v4();
@@ -204,11 +238,11 @@ mod test {
204238
txn.commit().unwrap();
205239
}
206240

207-
let server = WebServer::new(Default::default(), None, storage);
241+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
208242
let app = App::new().configure(|sc| server.config(sc));
209243
let app = test::init_service(app).await;
210244

211-
let uri = format!("/v1/client/add-version/{}", parent_version_id);
245+
let uri = format!("/v1/client/add-version/{parent_version_id}");
212246
let req = test::TestRequest::post()
213247
.uri(&uri)
214248
.append_header((
@@ -232,11 +266,11 @@ mod test {
232266
let client_id = Uuid::new_v4();
233267
let parent_version_id = Uuid::new_v4();
234268
let storage = InMemoryStorage::new();
235-
let server = WebServer::new(Default::default(), None, storage);
269+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
236270
let app = App::new().configure(|sc| server.config(sc));
237271
let app = test::init_service(app).await;
238272

239-
let uri = format!("/v1/client/add-version/{}", parent_version_id);
273+
let uri = format!("/v1/client/add-version/{parent_version_id}");
240274
let req = test::TestRequest::post()
241275
.uri(&uri)
242276
.append_header(("Content-Type", "not/correct"))
@@ -252,11 +286,11 @@ mod test {
252286
let client_id = Uuid::new_v4();
253287
let parent_version_id = Uuid::new_v4();
254288
let storage = InMemoryStorage::new();
255-
let server = WebServer::new(Default::default(), None, storage);
289+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
256290
let app = App::new().configure(|sc| server.config(sc));
257291
let app = test::init_service(app).await;
258292

259-
let uri = format!("/v1/client/add-version/{}", parent_version_id);
293+
let uri = format!("/v1/client/add-version/{parent_version_id}");
260294
let req = test::TestRequest::post()
261295
.uri(&uri)
262296
.append_header((

‎server/src/api/get_child_version.rs‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -48,11 +48,11 @@ pub(crate) async fn service(
4848

4949
#[cfg(test)]
5050
mod test {
51-
use crate::api::CLIENT_ID_HEADER;
5251
use crate::WebServer;
52+
use crate::{api::CLIENT_ID_HEADER, WebConfig};
5353
use actix_web::{http::StatusCode, test, App};
5454
use pretty_assertions::assert_eq;
55-
use taskchampion_sync_server_core::{InMemoryStorage, Storage, NIL_VERSION_ID};
55+
use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Storage, NIL_VERSION_ID};
5656
use uuid::Uuid;
5757

5858
#[actix_rt::test]
@@ -71,11 +71,11 @@ mod test {
7171
txn.commit().unwrap();
7272
}
7373

74-
let server = WebServer::new(Default::default(), None, storage);
74+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
7575
let app = App::new().configure(|sc| server.config(sc));
7676
let app = test::init_service(app).await;
7777

78-
let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
78+
let uri = format!("/v1/client/get-child-version/{parent_version_id}");
7979
let req = test::TestRequest::get()
8080
.uri(&uri)
8181
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
@@ -105,11 +105,11 @@ mod test {
105105
let client_id = Uuid::new_v4();
106106
let parent_version_id = Uuid::new_v4();
107107
let storage = InMemoryStorage::new();
108-
let server = WebServer::new(Default::default(), None, storage);
108+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
109109
let app = App::new().configure(|sc| server.config(sc));
110110
let app = test::init_service(app).await;
111111

112-
let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
112+
let uri = format!("/v1/client/get-child-version/{parent_version_id}");
113113
let req = test::TestRequest::get()
114114
.uri(&uri)
115115
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
@@ -134,12 +134,12 @@ mod test {
134134
.unwrap();
135135
txn.commit().unwrap();
136136
}
137-
let server = WebServer::new(Default::default(), None, storage);
137+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
138138
let app = App::new().configure(|sc| server.config(sc));
139139
let app = test::init_service(app).await;
140140

141141
// the child of the nil version is the added version
142-
let uri = format!("/v1/client/get-child-version/{}", NIL_VERSION_ID);
142+
let uri = format!("/v1/client/get-child-version/{NIL_VERSION_ID}");
143143
let req = test::TestRequest::get()
144144
.uri(&uri)
145145
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
@@ -168,7 +168,7 @@ mod test {
168168

169169
// The child of the latest version is NOT_FOUND. The tests in crate::server test more
170170
// corner cases.
171-
let uri = format!("/v1/client/get-child-version/{}", test_version_id);
171+
let uri = format!("/v1/client/get-child-version/{test_version_id}");
172172
let req = test::TestRequest::get()
173173
.uri(&uri)
174174
.append_header((CLIENT_ID_HEADER, client_id.to_string()))

‎server/src/api/get_snapshot.rs‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -33,12 +33,12 @@ pub(crate) async fn service(
3333

3434
#[cfg(test)]
3535
mod test {
36-
use crate::api::CLIENT_ID_HEADER;
3736
use crate::WebServer;
37+
use crate::{api::CLIENT_ID_HEADER, WebConfig};
3838
use actix_web::{http::StatusCode, test, App};
3939
use chrono::{TimeZone, Utc};
4040
use pretty_assertions::assert_eq;
41-
use taskchampion_sync_server_core::{InMemoryStorage, Snapshot, Storage};
41+
use taskchampion_sync_server_core::{InMemoryStorage, ServerConfig, Snapshot, Storage};
4242
use uuid::Uuid;
4343

4444
#[actix_rt::test]
@@ -53,7 +53,7 @@ mod test {
5353
txn.commit().unwrap();
5454
}
5555

56-
let server = WebServer::new(Default::default(), None, storage);
56+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
5757
let app = App::new().configure(|sc| server.config(sc));
5858
let app = test::init_service(app).await;
5959

@@ -89,7 +89,7 @@ mod test {
8989
txn.commit().unwrap();
9090
}
9191

92-
let server = WebServer::new(Default::default(), None, storage);
92+
let server = WebServer::new(ServerConfig::default(), WebConfig::default(), storage);
9393
let app = App::new().configure(|sc| server.config(sc));
9494
let app = test::init_service(app).await;
9595

‎server/src/api/mod.rs‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
1-
use std::collections::HashSet;
2-
31
use actix_web::{error, web, HttpRequest, Result, Scope};
42
use taskchampion_sync_server_core::{ClientId, Server, ServerError};
5-
use uuid::Uuid;
3+
4+
use crate::WebConfig;
65

76
mod add_snapshot;
87
mod add_version;
@@ -31,7 +30,7 @@ pub(crate) const SNAPSHOT_REQUEST_HEADER: &str = "X-Snapshot-Request";
3130
/// The type containing a reference to the persistent state for the server
3231
pub(crate) struct ServerState {
3332
pub(crate) server: Server,
34-
pub(crate) client_id_allowlist: Option<HashSet<Uuid>>,
33+
pub(crate) web_config: WebConfig,
3534
}
3635

3736
impl ServerState {
@@ -43,7 +42,7 @@ impl ServerState {
4342
if let Some(client_id_hdr) = req.headers().get(CLIENT_ID_HEADER) {
4443
let client_id = client_id_hdr.to_str().map_err(|_| badrequest())?;
4544
let client_id = ClientId::parse_str(client_id).map_err(|_| badrequest())?;
46-
if let Some(allow_list) = &self.client_id_allowlist {
45+
if let Some(allow_list) = &self.web_config.client_id_allowlist {
4746
if !allow_list.contains(&client_id) {
4847
return Err(error::ErrorForbidden("unknown x-client-id"));
4948
}
@@ -80,13 +79,17 @@ fn server_error_to_actix(err: ServerError) -> actix_web::Error {
8079
mod test {
8180
use super::*;
8281
use taskchampion_sync_server_core::InMemoryStorage;
82+
use uuid::Uuid;
8383

8484
#[test]
8585
fn client_id_header_allow_all() {
8686
let client_id = Uuid::new_v4();
8787
let state = ServerState {
8888
server: Server::new(Default::default(), InMemoryStorage::new()),
89-
client_id_allowlist: None,
89+
web_config: WebConfig {
90+
client_id_allowlist: None,
91+
create_clients: true,
92+
},
9093
};
9194
let req = actix_web::test::TestRequest::default()
9295
.insert_header((CLIENT_ID_HEADER, client_id.to_string()))
@@ -100,7 +103,10 @@ mod test {
100103
let client_id_disallowed = Uuid::new_v4();
101104
let state = ServerState {
102105
server: Server::new(Default::default(), InMemoryStorage::new()),
103-
client_id_allowlist: Some([client_id_ok].into()),
106+
web_config: WebConfig {
107+
client_id_allowlist: Some([client_id_ok].into()),
108+
create_clients: true,
109+
},
104110
};
105111
let req = actix_web::test::TestRequest::default()
106112
.insert_header((CLIENT_ID_HEADER, client_id_ok.to_string()))

0 commit comments

Comments
 (0)