From f760175541d779232eaff6c3732ba57c1bb471fb Mon Sep 17 00:00:00 2001 From: nazeh Date: Thu, 1 Aug 2024 09:37:47 +0300 Subject: [PATCH] feat(pubky): add delete files by url --- pubky-homeserver/src/database/tables/blobs.rs | 17 +++-- .../src/database/tables/entries.rs | 25 +++++++ pubky-homeserver/src/routes.rs | 1 + pubky-homeserver/src/routes/public.rs | 73 +++++++++++++----- pubky/pkg/README.md | 3 + pubky/pkg/test/public.js | 33 ++++----- pubky/src/native.rs | 8 +- pubky/src/shared/public.rs | 74 ++++++++++--------- pubky/src/wasm.rs | 6 ++ 9 files changed, 160 insertions(+), 80 deletions(-) diff --git a/pubky-homeserver/src/database/tables/blobs.rs b/pubky-homeserver/src/database/tables/blobs.rs index 80c46ae..3280f47 100644 --- a/pubky-homeserver/src/database/tables/blobs.rs +++ b/pubky-homeserver/src/database/tables/blobs.rs @@ -21,20 +21,25 @@ impl DB { public_key: &PublicKey, path: &str, ) -> anyhow::Result> { - let mut rtxn = self.env.read_txn()?; + let rtxn = self.env.read_txn()?; let mut key = vec![]; key.extend_from_slice(public_key.as_bytes()); key.extend_from_slice(path.as_bytes()); - if let Some(bytes) = self.tables.entries.get(&rtxn, &key)? { + let result = if let Some(bytes) = self.tables.entries.get(&rtxn, &key)? { let entry = Entry::deserialize(bytes)?; - if let Some(blob) = self.tables.blobs.get(&rtxn, entry.content_hash())? { - return Ok(Some(bytes::Bytes::from(blob.to_vec()))); - }; + self.tables + .blobs + .get(&rtxn, entry.content_hash())? + .map(|blob| bytes::Bytes::from(blob.to_vec())) + } else { + None }; - Ok(None) + rtxn.commit(); + + Ok(result) } } diff --git a/pubky-homeserver/src/database/tables/entries.rs b/pubky-homeserver/src/database/tables/entries.rs index a0e461b..ecadf18 100644 --- a/pubky-homeserver/src/database/tables/entries.rs +++ b/pubky-homeserver/src/database/tables/entries.rs @@ -58,6 +58,31 @@ impl DB { Ok(()) } + + pub fn delete_entry(&mut self, public_key: &PublicKey, path: &str) -> anyhow::Result { + let mut wtxn = self.env.write_txn()?; + + let mut key = vec![]; + key.extend_from_slice(public_key.as_bytes()); + key.extend_from_slice(path.as_bytes()); + + let deleted = if let Some(bytes) = self.tables.entries.get(&wtxn, &key)? { + let entry = Entry::deserialize(bytes)?; + + // TODO: reference counting of blobs + let deleted_blobs = self.tables.blobs.delete(&mut wtxn, entry.content_hash())?; + + let deleted_entry = self.tables.entries.delete(&mut wtxn, &key)?; + + deleted_entry & deleted_blobs + } else { + false + }; + + wtxn.commit()?; + + Ok(deleted) + } } #[derive(Clone, Default, Serialize, Deserialize, Debug, Eq, PartialEq)] diff --git a/pubky-homeserver/src/routes.rs b/pubky-homeserver/src/routes.rs index 0d3b7e3..4d057a3 100644 --- a/pubky-homeserver/src/routes.rs +++ b/pubky-homeserver/src/routes.rs @@ -30,6 +30,7 @@ fn base(state: AppState) -> Router { .route("/:pubky/session", delete(auth::signout)) .route("/:pubky/*path", put(public::put)) .route("/:pubky/*path", get(public::get)) + .route("/:pubky/*path", delete(public::delete)) .layer(CookieManagerLayer::new()) // TODO: revisit if we enable streaming big payloads // TODO: maybe add to a separate router (drive router?). diff --git a/pubky-homeserver/src/routes/public.rs b/pubky-homeserver/src/routes/public.rs index 4a7834c..7cc48fe 100644 --- a/pubky-homeserver/src/routes/public.rs +++ b/pubky-homeserver/src/routes/public.rs @@ -7,6 +7,7 @@ use axum::{ }; use axum_extra::body::AsyncReadBody; use futures_util::stream::StreamExt; +use pkarr::PublicKey; use tower_cookies::Cookies; use tracing::debug; @@ -31,28 +32,17 @@ pub async fn put( mut body: Body, ) -> Result { let public_key = pubky.public_key().clone(); - let path = path.as_str().to_string(); + let path = path.as_str(); - // TODO: can we move this logic to the extractor or a layer - // to perform this validation? - let session = state - .db - .get_session(cookies, &public_key, &path)? - .ok_or(Error::with_status(StatusCode::UNAUTHORIZED))?; - - if !path.starts_with("pub/") { - return Err(Error::new( - StatusCode::FORBIDDEN, - "Writing to directories other than '/pub/' is forbidden".into(), - )); - } - - // TODO: should we forbid paths ending with `/`? + authorize(&mut state, cookies, &public_key, path)?; + verify(path)?; let mut stream = body.into_data_stream(); let (tx, rx) = flume::bounded::(1); + let path = path.to_string(); + // TODO: refactor Database to clean up this scope. let done = tokio::task::spawn_blocking(move || -> Result<()> { // TODO: this is a blocking operation, which is ok for small @@ -84,7 +74,7 @@ pub async fn get( pubky: Pubky, path: EntryPath, ) -> Result { - // TODO: check the path, return an error if doesn't start with `/pub/` + verify(path.as_str()); // TODO: Enable streaming @@ -96,3 +86,52 @@ pub async fn get( Ok(None) => Err(Error::with_status(StatusCode::NOT_FOUND)), } } + +pub async fn delete( + State(mut state): State, + pubky: Pubky, + path: EntryPath, + cookies: Cookies, + mut body: Body, +) -> Result { + let public_key = pubky.public_key().clone(); + let path = path.as_str(); + + authorize(&mut state, cookies, &public_key, path)?; + verify(path)?; + + state.db.delete_entry(&public_key, path)?; + + // TODO: return relevant headers, like Etag? + + Ok(()) +} + +fn authorize( + state: &mut AppState, + cookies: Cookies, + public_key: &PublicKey, + path: &str, +) -> Result<()> { + // TODO: can we move this logic to the extractor or a layer + // to perform this validation? + let session = state + .db + .get_session(cookies, public_key, path)? + .ok_or(Error::with_status(StatusCode::UNAUTHORIZED))?; + + Ok(()) +} + +fn verify(path: &str) -> Result<()> { + if !path.starts_with("pub/") { + return Err(Error::new( + StatusCode::FORBIDDEN, + "Writing to directories other than '/pub/' is forbidden".into(), + )); + } + + // TODO: should we forbid paths ending with `/`? + + Ok(()) +} diff --git a/pubky/pkg/README.md b/pubky/pkg/README.md index 3852307..9f5fcb6 100644 --- a/pubky/pkg/README.md +++ b/pubky/pkg/README.md @@ -43,6 +43,9 @@ await client.put(url, body); let response = await client.get(url); } + +// Delete public data, by authorized client +await client.delete(url); ``` ## Test and Development diff --git a/pubky/pkg/test/public.js b/pubky/pkg/test/public.js index dd116b2..9a8b5c2 100644 --- a/pubky/pkg/test/public.js +++ b/pubky/pkg/test/public.js @@ -19,28 +19,25 @@ test('public: put/get', async (t) => { // PUT public data, by authorized client await client.put(url, body); + const otherClient = PubkyClient.testnet(); // GET public data without signup or signin { - const client = PubkyClient.testnet(); - - let response = await client.get(url); + let response = await otherClient.get(url); t.ok(Buffer.from(response).equals(body)) } - // // DELETE public data, by authorized client - // await client.delete(publicKey, "/pub/example.com/arbitrary"); - // - // - // // GET public data without signup or signin - // { - // const client = new PubkyClient(); - // - // let response = await client.get(publicKey, "/pub/example.com/arbitrary"); - // - // t.notOk(response) - // } + // DELETE public data, by authorized client + await client.delete(url); + + + // GET public data without signup or signin + { + let response = await otherClient.get(url); + + t.notOk(response) + } }) test("not found", async (t) => { @@ -58,11 +55,7 @@ test("not found", async (t) => { let result = await client.get(url).catch(e => e); - t.ok(result instanceof Error); - t.is( - result.message, - `HTTP status client error (404 Not Found) for url (http://localhost:15411/${publicKey.z32()}/pub/example.com/arbitrary)` - ) + t.notOk(result); }) test("unauthorized", async (t) => { diff --git a/pubky/src/native.rs b/pubky/src/native.rs index 95f4a7d..783bce6 100644 --- a/pubky/src/native.rs +++ b/pubky/src/native.rs @@ -95,10 +95,10 @@ impl PubkyClient { self.inner_get(url).await } - // /// Delete a file at a path relative to a pubky author. - // pub async fn delete(&self, pubky: &PublicKey, path: &str) -> Result<()> { - // self.inner_delete(pubky, path).await - // } + /// Delete a file at a path relative to a pubky author. + pub async fn delete>(&self, url: T) -> Result<()> { + self.inner_delete(url).await + } } // === Internals === diff --git a/pubky/src/shared/public.rs b/pubky/src/shared/public.rs index d3a776c..e9eeb0d 100644 --- a/pubky/src/shared/public.rs +++ b/pubky/src/shared/public.rs @@ -31,12 +31,12 @@ impl PubkyClient { let response = self.request(Method::GET, url).send().await?; - response.error_for_status_ref()?; - if response.status() == StatusCode::NOT_FOUND { return Ok(None); } + response.error_for_status_ref()?; + // TODO: bail on too large files. let bytes = response.bytes().await?; @@ -125,28 +125,23 @@ mod tests { client.signup(&keypair, &server.public_key()).await.unwrap(); let url = format!("pubky://{}/pub/foo.txt", keypair.public_key()); + let url = url.as_str(); - client.put(url.as_str(), &[0, 1, 2, 3, 4]).await.unwrap(); + client.put(url, &[0, 1, 2, 3, 4]).await.unwrap(); - let response = client.get(url.as_str()).await.unwrap().unwrap(); + let response = client.get(url).await.unwrap().unwrap(); assert_eq!(response, bytes::Bytes::from(vec![0, 1, 2, 3, 4])); - // client - // .delete(&keypair.public_key(), "/pub/foo.txt") - // .await - // .unwrap(); - // - // let response = client - // .get(&keypair.public_key(), "/pub/foo.txt") - // .await - // .unwrap(); - // - // assert_eq!(response, None); + client.delete(url).await.unwrap(); + + let response = client.get(url).await.unwrap(); + + assert_eq!(response, None); } #[tokio::test] - async fn forbidden_put_delete() { + async fn unauthorized_put_delete() { let testnet = Testnet::new(10); let server = Homeserver::start_test(&testnet).await.unwrap(); @@ -159,6 +154,7 @@ mod tests { let public_key = keypair.public_key(); let url = format!("pubky://{public_key}/pub/foo.txt"); + let url = url.as_str(); let other_client = PubkyClient::test(&testnet); { @@ -170,7 +166,7 @@ mod tests { .await .unwrap(); - let response = other_client.put(url.as_str(), &[0, 1, 2, 3, 4]).await; + let response = other_client.put(url, &[0, 1, 2, 3, 4]).await; match response { Err(Error::Reqwest(error)) => { @@ -182,21 +178,33 @@ mod tests { } } - // client - // .put(&keypair.public_key(), "/pub/foo.txt", &[0, 1, 2, 3, 4]) - // .await - // .unwrap(); - // - // client - // .delete(&keypair.public_key(), "/pub/foo.txt") - // .await - // .unwrap(); - // - // let response = client - // .get(&keypair.public_key(), "/pub/foo.txt") - // .await - // .unwrap(); - // - // assert_eq!(response, None); + client.put(url, &[0, 1, 2, 3, 4]).await.unwrap(); + + { + let other = Keypair::random(); + + // TODO: remove extra client after switching to subdomains. + other_client + .signup(&other, &server.public_key()) + .await + .unwrap(); + + let response = other_client.delete(url).await; + + dbg!(&response); + + match response { + Err(Error::Reqwest(error)) => { + assert!(error.status() == Some(StatusCode::UNAUTHORIZED)) + } + error => { + panic!("expected error StatusCode::UNAUTHORIZED") + } + } + } + + let response = client.get(url).await.unwrap().unwrap(); + + assert_eq!(response, bytes::Bytes::from(vec![0, 1, 2, 3, 4])); } } diff --git a/pubky/src/wasm.rs b/pubky/src/wasm.rs index 782a5b0..43a5080 100644 --- a/pubky/src/wasm.rs +++ b/pubky/src/wasm.rs @@ -128,4 +128,10 @@ impl PubkyClient { .map(|b| b.map(|b| (&*b).into())) .map_err(|e| e.into()) } + + #[wasm_bindgen] + /// Delete a file at a path relative to a pubky author. + pub async fn delete(&self, url: &str) -> Result<(), JsValue> { + self.inner_delete(url).await.map_err(|e| e.into()) + } }