diff --git a/docs/src/sync-protocol.md b/docs/src/sync-protocol.md index 1a4799630..e96f7cc83 100644 --- a/docs/src/sync-protocol.md +++ b/docs/src/sync-protocol.md @@ -69,10 +69,11 @@ The transactions above are realized for an HTTP server at `` using the H The `origin` *should* be an HTTPS endpoint on general principle, but nothing in the functonality or security of the protocol depends on connection encryption. The replica identifies itself to the server using a `clientKey` in the form of a UUID. +This value is passed with every request in the `X-Client-Id` header, in its dashed-hex format. ### AddVersion -The request is a `POST` to `/client//add-version/`. +The request is a `POST` to `/client/add-version/`. The request body contains the history segment, optionally encoded using any encoding supported by actix-web. The content-type must be `application/vnd.taskchampion.history-segment`. @@ -86,7 +87,7 @@ Other error responses (4xx or 5xx) may be returned and should be treated appropr ### GetChildVersion -The request is a `GET` to `/client//get-child-version/`. +The request is a `GET` to `/client/get-child-version/`. The response is 404 NOT FOUND if no such version exists. Otherwise, the response is a 200 OK. The version's history segment is returned in the response body, with content-type `application/vnd.taskchampion.history-segment`. diff --git a/sync-server/src/api/add_version.rs b/sync-server/src/api/add_version.rs index fcb45fb10..4ca7cbdb1 100644 --- a/sync-server/src/api/add_version.rs +++ b/sync-server/src/api/add_version.rs @@ -1,8 +1,8 @@ use crate::api::{ - failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, - VERSION_ID_HEADER, + client_key_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE, + PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER, }; -use crate::server::{add_version, AddVersionResult, ClientKey, VersionId, NO_VERSION_ID}; +use crate::server::{add_version, AddVersionResult, VersionId, NO_VERSION_ID}; use actix_web::{error, post, web, HttpMessage, HttpRequest, HttpResponse, Result}; use futures::StreamExt; @@ -19,11 +19,11 @@ const MAX_SIZE: usize = 100 * 1024 * 1024; /// parent version ID in the `X-Parent-Version-Id` header. /// /// Returns other 4xx or 5xx responses on other errors. -#[post("/client/{client_key}/add-version/{parent_version_id}")] +#[post("/client/add-version/{parent_version_id}")] pub(crate) async fn service( req: HttpRequest, server_state: web::Data, - web::Path((client_key, parent_version_id)): web::Path<(ClientKey, VersionId)>, + web::Path((parent_version_id,)): web::Path<(VersionId,)>, mut payload: web::Payload, ) -> Result { // check content-type @@ -31,6 +31,8 @@ pub(crate) async fn service( return Err(error::ErrorBadRequest("Bad content-type")); } + let client_key = client_key_header(&req)?; + // read the body in its entirety let mut body = web::BytesMut::new(); while let Some(chunk) = payload.next().await { @@ -97,13 +99,14 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!("/client/{}/add-version/{}", client_key, parent_version_id); + let uri = format!("/client/add-version/{}", parent_version_id); let req = test::TestRequest::post() .uri(&uri) .header( "Content-Type", "application/vnd.taskchampion.history-segment", ) + .header("X-Client-Key", client_key.to_string()) .set_payload(b"abcd".to_vec()) .to_request(); let resp = test::call_service(&mut app, req).await; @@ -133,13 +136,14 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!("/client/{}/add-version/{}", client_key, parent_version_id); + let uri = format!("/client/add-version/{}", parent_version_id); let req = test::TestRequest::post() .uri(&uri) .header( "Content-Type", "application/vnd.taskchampion.history-segment", ) + .header("X-Client-Key", client_key.to_string()) .set_payload(b"abcd".to_vec()) .to_request(); let resp = test::call_service(&mut app, req).await; @@ -159,10 +163,11 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!("/client/{}/add-version/{}", client_key, parent_version_id); + let uri = format!("/client/add-version/{}", parent_version_id); let req = test::TestRequest::post() .uri(&uri) .header("Content-Type", "not/correct") + .header("X-Client-Key", client_key.to_string()) .set_payload(b"abcd".to_vec()) .to_request(); let resp = test::call_service(&mut app, req).await; @@ -177,13 +182,14 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!("/client/{}/add-version/{}", client_key, parent_version_id); + let uri = format!("/client/add-version/{}", parent_version_id); let req = test::TestRequest::post() .uri(&uri) .header( "Content-Type", "application/vnd.taskchampion.history-segment", ) + .header("X-Client-Key", client_key.to_string()) .to_request(); let resp = test::call_service(&mut app, req).await; assert_eq!(resp.status(), StatusCode::BAD_REQUEST); diff --git a/sync-server/src/api/get_child_version.rs b/sync-server/src/api/get_child_version.rs index 84a9fafa3..0bf04d96a 100644 --- a/sync-server/src/api/get_child_version.rs +++ b/sync-server/src/api/get_child_version.rs @@ -1,9 +1,9 @@ use crate::api::{ - failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE, PARENT_VERSION_ID_HEADER, - VERSION_ID_HEADER, + client_key_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE, + PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER, }; -use crate::server::{get_child_version, ClientKey, VersionId}; -use actix_web::{error, get, web, HttpResponse, Result}; +use crate::server::{get_child_version, VersionId}; +use actix_web::{error, get, web, HttpRequest, HttpResponse, Result}; /// Get a child version. /// @@ -13,13 +13,16 @@ use actix_web::{error, get, web, HttpResponse, Result}; /// /// If no such child exists, returns a 404 with no content. /// Returns other 4xx or 5xx responses on other errors. -#[get("/client/{client_key}/get-child-version/{parent_version_id}")] +#[get("/client/get-child-version/{parent_version_id}")] pub(crate) async fn service( + req: HttpRequest, server_state: web::Data, - web::Path((client_key, parent_version_id)): web::Path<(ClientKey, VersionId)>, + web::Path((parent_version_id,)): web::Path<(VersionId,)>, ) -> Result { let mut txn = server_state.txn().map_err(failure_to_ise)?; + let client_key = client_key_header(&req)?; + txn.get_client(client_key) .map_err(failure_to_ise)? .ok_or_else(|| error::ErrorNotFound("no such client"))?; @@ -65,11 +68,11 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!( - "/client/{}/get-child-version/{}", - client_key, parent_version_id - ); - let req = test::TestRequest::get().uri(&uri).to_request(); + let uri = format!("/client/get-child-version/{}", parent_version_id); + let req = test::TestRequest::get() + .uri(&uri) + .header("X-Client-Key", client_key.to_string()) + .to_request(); let mut resp = test::call_service(&mut app, req).await; assert_eq!(resp.status(), StatusCode::OK); assert_eq!( @@ -98,11 +101,11 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!( - "/client/{}/get-child-version/{}", - client_key, parent_version_id - ); - let req = test::TestRequest::get().uri(&uri).to_request(); + let uri = format!("/client/get-child-version/{}", parent_version_id); + let req = test::TestRequest::get() + .uri(&uri) + .header("X-Client-Key", client_key.to_string()) + .to_request(); let resp = test::call_service(&mut app, req).await; assert_eq!(resp.status(), StatusCode::NOT_FOUND); assert_eq!(resp.headers().get("X-Version-Id"), None); @@ -123,11 +126,11 @@ mod test { let server_state = ServerState::new(server_box); let mut app = test::init_service(App::new().service(app_scope(server_state))).await; - let uri = format!( - "/client/{}/get-child-version/{}", - client_key, parent_version_id - ); - let req = test::TestRequest::get().uri(&uri).to_request(); + let uri = format!("/client/get-child-version/{}", parent_version_id); + let req = test::TestRequest::get() + .uri(&uri) + .header("X-Client-Key", client_key.to_string()) + .to_request(); let resp = test::call_service(&mut app, req).await; assert_eq!(resp.status(), StatusCode::NOT_FOUND); assert_eq!(resp.headers().get("X-Version-Id"), None); diff --git a/sync-server/src/api/mod.rs b/sync-server/src/api/mod.rs index cddcab59c..5929f28c1 100644 --- a/sync-server/src/api/mod.rs +++ b/sync-server/src/api/mod.rs @@ -1,5 +1,6 @@ +use crate::server::ClientKey; use crate::storage::Storage; -use actix_web::{error, http::StatusCode, web, Scope}; +use actix_web::{error, http::StatusCode, web, HttpRequest, Result, Scope}; use std::sync::Arc; mod add_version; @@ -9,10 +10,13 @@ mod get_child_version; pub(crate) const HISTORY_SEGMENT_CONTENT_TYPE: &str = "application/vnd.taskchampion.history-segment"; -/// The header names for version ID +/// The header name for version ID pub(crate) const VERSION_ID_HEADER: &str = "X-Version-Id"; -/// The header names for parent version ID +/// The header name for client key +pub(crate) const CLIENT_KEY_HEADER: &str = "X-Client-Key"; + +/// The header name for parent version ID pub(crate) const PARENT_VERSION_ID_HEADER: &str = "X-Parent-Version-Id"; /// The type containing a reference to the Storage object in the Actix state. @@ -28,3 +32,17 @@ pub(crate) fn api_scope() -> Scope { fn failure_to_ise(err: failure::Error) -> impl actix_web::ResponseError { error::InternalError::new(err, StatusCode::INTERNAL_SERVER_ERROR) } + +/// Get the client key +fn client_key_header(req: &HttpRequest) -> Result { + fn badrequest() -> error::Error { + error::ErrorBadRequest("bad x-client-id") + } + if let Some(client_key_hdr) = req.headers().get(CLIENT_KEY_HEADER) { + let client_key = client_key_hdr.to_str().map_err(|_| badrequest())?; + let client_key = ClientKey::parse_str(client_key).map_err(|_| badrequest())?; + Ok(client_key) + } else { + Err(badrequest()) + } +} diff --git a/taskchampion/src/server/remote/mod.rs b/taskchampion/src/server/remote/mod.rs index 44b713b20..cd20618ec 100644 --- a/taskchampion/src/server/remote/mod.rs +++ b/taskchampion/src/server/remote/mod.rs @@ -56,10 +56,7 @@ impl Server for RemoteServer { parent_version_id: VersionId, history_segment: HistorySegment, ) -> Fallible { - let url = format!( - "{}/client/{}/add-version/{}", - self.origin, self.client_key, parent_version_id - ); + let url = format!("{}/client/add-version/{}", self.origin, parent_version_id); let history_cleartext = HistoryCleartext { parent_version_id, history_segment, @@ -74,6 +71,7 @@ impl Server for RemoteServer { "Content-Type", "application/vnd.taskchampion.history-segment", ) + .set("X-Client-Key", &self.client_key.to_string()) .send_bytes(history_ciphertext.as_ref()); if resp.ok() { let version_id = get_uuid_header(&resp, "X-Version-Id")?; @@ -88,14 +86,15 @@ impl Server for RemoteServer { fn get_child_version(&mut self, parent_version_id: VersionId) -> Fallible { let url = format!( - "{}/client/{}/get-child-version/{}", - self.origin, self.client_key, parent_version_id + "{}/client/get-child-version/{}", + self.origin, parent_version_id ); let resp = self .agent .get(&url) .timeout_connect(10_000) .timeout_read(60_000) + .set("X-Client-Key", &self.client_key.to_string()) .call(); if resp.ok() {