gergely/taskwarrior-2.x
1
0
Fork 0
Code Issues Actions Packages Projects Releases 1 Wiki Activity

Change "client key" to "client id" (#3130)

Browse Source 
In #3118 @ryneeverett mentioned that "key" suggests that this is a
secret, when in truth it's just a user identifier. So "ID" is a better
word for it than "key".
This commit is contained in:
Dustin J. Mitchell
2023-07-11 22:13:53 -04:00
committed by GitHub
parent 8097e28318
commit 7f68441916
32 changed files with 387 additions and 388 deletions
Download Patch File Download Diff File Expand all files Collapse all files

39
taskchampion/sync-server/src/api/add_snapshot.rs
View File

@@ -1,4 +1,4 @@
use crate::api::{client_key_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE};
use crate::api::{client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE};
use crate::server::{add_snapshot, VersionId, NIL_VERSION_ID};
use actix_web::{error, post, web, HttpMessage, HttpRequest, HttpResponse, Result};
use futures::StreamExt;
@@ -29,7 +29,7 @@ pub(crate) async fn service(
return Err(error::ErrorBadRequest("Bad content-type"));
}
let client_key = client_key_header(&req)?;
let client_id = client_id_header(&req)?;
// read the body in its entirety
let mut body = web::BytesMut::new();
@@ -52,19 +52,19 @@ pub(crate) async fn service(
let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
// get, or create, the client
let client = match txn.get_client(client_key).map_err(failure_to_ise)? {
let client = match txn.get_client(client_id).map_err(failure_to_ise)? {
Some(client) => client,
None => {
txn.new_client(client_key, NIL_VERSION_ID)
txn.new_client(client_id, NIL_VERSION_ID)
.map_err(failure_to_ise)?;
txn.get_client(client_key).map_err(failure_to_ise)?.unwrap()
txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
}
};
add_snapshot(
txn,
&server_state.config,
client_key,
client_id,
client,
version_id,
body.to_vec(),
@@ -76,6 +76,7 @@ pub(crate) async fn service(
#[cfg(test)]
mod test {
use super::*;
use crate::api::CLIENT_ID_HEADER;
use crate::storage::{InMemoryStorage, Storage};
use crate::Server;
use actix_web::{http::StatusCode, test, App};
@@ -84,15 +85,15 @@ mod test {
#[actix_rt::test]
async fn test_success() -> anyhow::Result<()> {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, version_id).unwrap();
txn.add_version(client_key, version_id, NIL_VERSION_ID, vec![])?;
txn.new_client(client_id, version_id).unwrap();
txn.add_version(client_id, version_id, NIL_VERSION_ID, vec![])?;
}
let server = Server::new(Default::default(), storage);
@@ -103,7 +104,7 @@ mod test {
let req = test::TestRequest::post()
.uri(&uri)
.insert_header(("Content-Type", "application/vnd.taskchampion.snapshot"))
.insert_header(("X-Client-Key", client_key.to_string()))
.insert_header((CLIENT_ID_HEADER, client_id.to_string()))
.set_payload(b"abcd".to_vec())
.to_request();
let resp = test::call_service(&mut app, req).await;
@@ -113,7 +114,7 @@ mod test {
let uri = "/v1/client/snapshot";
let req = test::TestRequest::get()
.uri(uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::OK);
@@ -127,14 +128,14 @@ mod test {
#[actix_rt::test]
async fn test_not_added_200() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, NIL_VERSION_ID).unwrap();
txn.new_client(client_id, NIL_VERSION_ID).unwrap();
}
let server = Server::new(Default::default(), storage);
@@ -146,7 +147,7 @@ mod test {
let req = test::TestRequest::post()
.uri(&uri)
.append_header(("Content-Type", "application/vnd.taskchampion.snapshot"))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.set_payload(b"abcd".to_vec())
.to_request();
let resp = test::call_service(&mut app, req).await;
@@ -156,7 +157,7 @@ mod test {
let uri = "/v1/client/snapshot";
let req = test::TestRequest::get()
.uri(uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
@@ -164,7 +165,7 @@ mod test {
#[actix_rt::test]
async fn test_bad_content_type() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
let server = Server::new(Default::default(), storage);
@@ -175,7 +176,7 @@ mod test {
let req = test::TestRequest::post()
.uri(&uri)
.append_header(("Content-Type", "not/correct"))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.set_payload(b"abcd".to_vec())
.to_request();
let resp = test::call_service(&mut app, req).await;
@@ -184,7 +185,7 @@ mod test {
#[actix_rt::test]
async fn test_empty_body() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
let server = Server::new(Default::default(), storage);
@@ -198,7 +199,7 @@ mod test {
"Content-Type",
"application/vnd.taskchampion.history-segment",
))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);

33
taskchampion/sync-server/src/api/add_version.rs
View File

@@ -1,5 +1,5 @@
use crate::api::{
client_key_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
client_id_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
PARENT_VERSION_ID_HEADER, SNAPSHOT_REQUEST_HEADER, VERSION_ID_HEADER,
};
use crate::server::{add_version, AddVersionResult, SnapshotUrgency, VersionId, NIL_VERSION_ID};
@@ -37,7 +37,7 @@ pub(crate) async fn service(
return Err(error::ErrorBadRequest("Bad content-type"));
}
let client_key = client_key_header(&req)?;
let client_id = client_id_header(&req)?;
// read the body in its entirety
let mut body = web::BytesMut::new();
@@ -60,19 +60,19 @@ pub(crate) async fn service(
let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
// get, or create, the client
let client = match txn.get_client(client_key).map_err(failure_to_ise)? {
let client = match txn.get_client(client_id).map_err(failure_to_ise)? {
Some(client) => client,
None => {
txn.new_client(client_key, NIL_VERSION_ID)
txn.new_client(client_id, NIL_VERSION_ID)
.map_err(failure_to_ise)?;
txn.get_client(client_key).map_err(failure_to_ise)?.unwrap()
txn.get_client(client_id).map_err(failure_to_ise)?.unwrap()
}
};
let (result, snap_urgency) = add_version(
txn,
&server_state.config,
client_key,
client_id,
client,
parent_version_id,
body.to_vec(),
@@ -104,6 +104,7 @@ pub(crate) async fn service(
#[cfg(test)]
mod test {
use crate::api::CLIENT_ID_HEADER;
use crate::storage::{InMemoryStorage, Storage};
use crate::Server;
use actix_web::{http::StatusCode, test, App};
@@ -112,7 +113,7 @@ mod test {
#[actix_rt::test]
async fn test_success() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
@@ -120,7 +121,7 @@ mod test {
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, Uuid::nil()).unwrap();
txn.new_client(client_id, Uuid::nil()).unwrap();
}
let server = Server::new(Default::default(), storage);
@@ -134,7 +135,7 @@ mod test {
"Content-Type",
"application/vnd.taskchampion.history-segment",
))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.set_payload(b"abcd".to_vec())
.to_request();
let resp = test::call_service(&mut app, req).await;
@@ -154,7 +155,7 @@ mod test {
#[actix_rt::test]
async fn test_conflict() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
@@ -162,7 +163,7 @@ mod test {
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, version_id).unwrap();
txn.new_client(client_id, version_id).unwrap();
}
let server = Server::new(Default::default(), storage);
@@ -176,7 +177,7 @@ mod test {
"Content-Type",
"application/vnd.taskchampion.history-segment",
))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.set_payload(b"abcd".to_vec())
.to_request();
let resp = test::call_service(&mut app, req).await;
@@ -190,7 +191,7 @@ mod test {
#[actix_rt::test]
async fn test_bad_content_type() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
let server = Server::new(Default::default(), storage);
@@ -201,7 +202,7 @@ mod test {
let req = test::TestRequest::post()
.uri(&uri)
.append_header(("Content-Type", "not/correct"))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.set_payload(b"abcd".to_vec())
.to_request();
let resp = test::call_service(&mut app, req).await;
@@ -210,7 +211,7 @@ mod test {
#[actix_rt::test]
async fn test_empty_body() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
let server = Server::new(Default::default(), storage);
@@ -224,7 +225,7 @@ mod test {
"Content-Type",
"application/vnd.taskchampion.history-segment",
))
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::BAD_REQUEST);

29
taskchampion/sync-server/src/api/get_child_version.rs
View File

@@ -1,5 +1,5 @@
use crate::api::{
client_key_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
client_id_header, failure_to_ise, ServerState, HISTORY_SEGMENT_CONTENT_TYPE,
PARENT_VERSION_ID_HEADER, VERSION_ID_HEADER,
};
use crate::server::{get_child_version, GetVersionResult, VersionId};
@@ -24,17 +24,17 @@ pub(crate) async fn service(
let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
let client_key = client_key_header(&req)?;
let client_id = client_id_header(&req)?;
let client = txn
.get_client(client_key)
.get_client(client_id)
.map_err(failure_to_ise)?
.ok_or_else(|| error::ErrorNotFound("no such client"))?;
return match get_child_version(
txn,
&server_state.config,
client_key,
client_id,
client,
parent_version_id,
)
@@ -56,6 +56,7 @@ pub(crate) async fn service(
#[cfg(test)]
mod test {
use crate::api::CLIENT_ID_HEADER;
use crate::server::NIL_VERSION_ID;
use crate::storage::{InMemoryStorage, Storage};
use crate::Server;
@@ -65,7 +66,7 @@ mod test {
#[actix_rt::test]
async fn test_success() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
@@ -73,8 +74,8 @@ mod test {
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, Uuid::new_v4()).unwrap();
txn.add_version(client_key, version_id, parent_version_id, b"abcd".to_vec())
txn.new_client(client_id, Uuid::new_v4()).unwrap();
txn.add_version(client_id, version_id, parent_version_id, b"abcd".to_vec())
.unwrap();
}
@@ -85,7 +86,7 @@ mod test {
let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
let req = test::TestRequest::get()
.uri(&uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::OK);
@@ -109,7 +110,7 @@ mod test {
#[actix_rt::test]
async fn test_client_not_found() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
let server = Server::new(Default::default(), storage);
@@ -119,7 +120,7 @@ mod test {
let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
let req = test::TestRequest::get()
.uri(&uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
@@ -129,14 +130,14 @@ mod test {
#[actix_rt::test]
async fn test_version_not_found_and_gone() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let parent_version_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
// create the client, but not the version
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, Uuid::new_v4()).unwrap();
txn.new_client(client_id, Uuid::new_v4()).unwrap();
}
let server = Server::new(Default::default(), storage);
let app = App::new().configure(|sc| server.config(sc));
@@ -146,7 +147,7 @@ mod test {
let uri = format!("/v1/client/get-child-version/{}", parent_version_id);
let req = test::TestRequest::get()
.uri(&uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::GONE);
@@ -159,7 +160,7 @@ mod test {
let uri = format!("/v1/client/get-child-version/{}", NIL_VERSION_ID);
let req = test::TestRequest::get()
.uri(&uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::NOT_FOUND);

23
taskchampion/sync-server/src/api/get_snapshot.rs
View File

@@ -1,5 +1,5 @@
use crate::api::{
client_key_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE, VERSION_ID_HEADER,
client_id_header, failure_to_ise, ServerState, SNAPSHOT_CONTENT_TYPE, VERSION_ID_HEADER,
};
use crate::server::get_snapshot;
use actix_web::{error, get, web, HttpRequest, HttpResponse, Result};
@@ -20,15 +20,15 @@ pub(crate) async fn service(
) -> Result<HttpResponse> {
let mut txn = server_state.storage.txn().map_err(failure_to_ise)?;
let client_key = client_key_header(&req)?;
let client_id = client_id_header(&req)?;
let client = txn
.get_client(client_key)
.get_client(client_id)
.map_err(failure_to_ise)?
.ok_or_else(|| error::ErrorNotFound("no such client"))?;
if let Some((version_id, data)) =
get_snapshot(txn, &server_state.config, client_key, client).map_err(failure_to_ise)?
get_snapshot(txn, &server_state.config, client_id, client).map_err(failure_to_ise)?
{
Ok(HttpResponse::Ok()
.content_type(SNAPSHOT_CONTENT_TYPE)
@@ -41,6 +41,7 @@ pub(crate) async fn service(
#[cfg(test)]
mod test {
use crate::api::CLIENT_ID_HEADER;
use crate::storage::{InMemoryStorage, Snapshot, Storage};
use crate::Server;
use actix_web::{http::StatusCode, test, App};
@@ -50,13 +51,13 @@ mod test {
#[actix_rt::test]
async fn test_not_found() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, Uuid::new_v4()).unwrap();
txn.new_client(client_id, Uuid::new_v4()).unwrap();
}
let server = Server::new(Default::default(), storage);
@@ -66,7 +67,7 @@ mod test {
let uri = "/v1/client/snapshot";
let req = test::TestRequest::get()
.uri(uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::NOT_FOUND);
@@ -74,7 +75,7 @@ mod test {
#[actix_rt::test]
async fn test_success() {
let client_key = Uuid::new_v4();
let client_id = Uuid::new_v4();
let version_id = Uuid::new_v4();
let snapshot_data = vec![1, 2, 3, 4];
let storage: Box<dyn Storage> = Box::new(InMemoryStorage::new());
@@ -82,9 +83,9 @@ mod test {
// set up the storage contents..
{
let mut txn = storage.txn().unwrap();
txn.new_client(client_key, Uuid::new_v4()).unwrap();
txn.new_client(client_id, Uuid::new_v4()).unwrap();
txn.set_snapshot(
client_key,
client_id,
Snapshot {
version_id,
versions_since: 3,
@@ -102,7 +103,7 @@ mod test {
let uri = "/v1/client/snapshot";
let req = test::TestRequest::get()
.uri(uri)
.append_header(("X-Client-Key", client_key.to_string()))
.append_header((CLIENT_ID_HEADER, client_id.to_string()))
.to_request();
let resp = test::call_service(&mut app, req).await;
assert_eq!(resp.status(), StatusCode::OK);

18
taskchampion/sync-server/src/api/mod.rs
View File

@@ -1,4 +1,4 @@
use crate::server::ClientKey;
use crate::server::ClientId;
use crate::storage::Storage;
use crate::ServerConfig;
use actix_web::{error, http::StatusCode, web, HttpRequest, Result, Scope};
@@ -18,8 +18,8 @@ pub(crate) const SNAPSHOT_CONTENT_TYPE: &str = "application/vnd.taskchampion.sna
/// The header name for version ID
pub(crate) const VERSION_ID_HEADER: &str = "X-Version-Id";
/// The header name for client key
pub(crate) const CLIENT_KEY_HEADER: &str = "X-Client-Key";
/// The header name for client id
pub(crate) const CLIENT_ID_HEADER: &str = "X-Client-Id";
/// The header name for parent version ID
pub(crate) const PARENT_VERSION_ID_HEADER: &str = "X-Parent-Version-Id";
@@ -46,15 +46,15 @@ fn failure_to_ise(err: anyhow::Error) -> impl actix_web::ResponseError {
error::InternalError::new(err, StatusCode::INTERNAL_SERVER_ERROR)
}
/// Get the client key
fn client_key_header(req: &HttpRequest) -> Result<ClientKey> {
/// Get the client id
fn client_id_header(req: &HttpRequest) -> Result<ClientId> {
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)
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())?;
Ok(client_id)
} else {
Err(badrequest())
}