From e328d33fc7d6a0f2e3d260d8bddee3ef633318eb Mon Sep 17 00:00:00 2001 From: Owen Jacobson Date: Tue, 29 Oct 2024 22:11:36 -0400 Subject: Restrict deletion to deleting your own messages. --- docs/api/channels-messages.md | 4 ++++ src/boot/routes/test.rs | 2 +- src/event/routes/test/message.rs | 6 +++--- src/message/app.rs | 14 ++++++++++++-- src/message/routes/message/mod.rs | 9 +++++++-- src/message/routes/message/test.rs | 37 ++++++++++++++++++++++++++++++++----- 6 files changed, 59 insertions(+), 13 deletions(-) diff --git a/docs/api/channels-messages.md b/docs/api/channels-messages.md index 6391b5a..d87a01c 100644 --- a/docs/api/channels-messages.md +++ b/docs/api/channels-messages.md @@ -227,3 +227,7 @@ When completed, the service will emit a [message deleted](events.md#message-dele ### Invalid message ID This endpoint will respond with a status of `404 Not Found` if the message ID is not valid. + +### Not the sender + +This endpoint will respond with a status of `403 Forbidden` if the message was sent by a different login. diff --git a/src/boot/routes/test.rs b/src/boot/routes/test.rs index 8808b70..202dcb9 100644 --- a/src/boot/routes/test.rs +++ b/src/boot/routes/test.rs @@ -85,7 +85,7 @@ async fn excludes_deleted_messages() { let deleted_message = fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await; app.messages() - .delete(&deleted_message.id, &fixtures::now()) + .delete(&sender, &deleted_message.id, &fixtures::now()) .await .expect("deleting valid message succeeds"); diff --git a/src/event/routes/test/message.rs b/src/event/routes/test/message.rs index 63a3f43..a7b25fb 100644 --- a/src/event/routes/test/message.rs +++ b/src/event/routes/test/message.rs @@ -260,7 +260,7 @@ async fn deleting() { // Delete the message app.messages() - .delete(&message.id, &fixtures::now()) + .delete(&sender, &message.id, &fixtures::now()) .await .expect("deleting a valid message succeeds"); @@ -286,7 +286,7 @@ async fn previously_deleted() { // Delete the message app.messages() - .delete(&message.id, &fixtures::now()) + .delete(&sender, &message.id, &fixtures::now()) .await .expect("deleting a valid message succeeds"); @@ -320,7 +320,7 @@ async fn previously_purged() { // Purge the message app.messages() - .delete(&message.id, &fixtures::ancient()) + .delete(&sender, &message.id, &fixtures::ancient()) .await .expect("deleting a valid message succeeds"); diff --git a/src/message/app.rs b/src/message/app.rs index eed6ba4..137a27d 100644 --- a/src/message/app.rs +++ b/src/message/app.rs @@ -45,16 +45,24 @@ impl<'a> Messages<'a> { Ok(message.as_sent()) } - pub async fn delete(&self, message: &Id, deleted_at: &DateTime) -> Result<(), DeleteError> { + pub async fn delete( + &self, + deleted_by: &Login, + message: &Id, + deleted_at: &DateTime, + ) -> Result<(), DeleteError> { let mut tx = self.db.begin().await?; let message = tx .messages() .by_id(message) .await .not_found(|| DeleteError::NotFound(message.clone()))?; - message + let snapshot = message .as_snapshot() .ok_or_else(|| DeleteError::Deleted(message.id().clone()))?; + if snapshot.sender != deleted_by.id { + return Err(DeleteError::NotSender(deleted_by.clone())); + } let deleted = tx.sequence().next(deleted_at).await?; let message = tx.messages().delete(&message, &deleted).await?; @@ -138,6 +146,8 @@ impl From for SendError { pub enum DeleteError { #[error("message {0} not found")] NotFound(Id), + #[error("login {} not the message's sender", .0.id)] + NotSender(Login), #[error("message {0} deleted")] Deleted(Id), #[error(transparent)] diff --git a/src/message/routes/message/mod.rs b/src/message/routes/message/mod.rs index 45a7e9d..e92f556 100644 --- a/src/message/routes/message/mod.rs +++ b/src/message/routes/message/mod.rs @@ -20,9 +20,11 @@ pub mod delete { State(app): State, Path(message): Path, RequestedAt(deleted_at): RequestedAt, - _: Identity, + identity: Identity, ) -> Result { - app.messages().delete(&message, &deleted_at).await?; + app.messages() + .delete(&identity.login, &message, &deleted_at) + .await?; Ok(Response { id: message }) } @@ -47,6 +49,9 @@ pub mod delete { let Self(error) = self; #[allow(clippy::match_wildcard_for_single_variants)] match error { + DeleteError::NotSender(_) => { + (StatusCode::FORBIDDEN, error.to_string()).into_response() + } DeleteError::NotFound(_) | DeleteError::Deleted(_) => { NotFound(error).into_response() } diff --git a/src/message/routes/message/test.rs b/src/message/routes/message/test.rs index ae89506..5178ab5 100644 --- a/src/message/routes/message/test.rs +++ b/src/message/routes/message/test.rs @@ -8,18 +8,17 @@ pub async fn delete_message() { // Set up the environment let app = fixtures::scratch_app().await; - let sender = fixtures::login::create(&app, &fixtures::now()).await; + let sender = fixtures::identity::create(&app, &fixtures::now()).await; let channel = fixtures::channel::create(&app, &fixtures::now()).await; - let message = fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await; + let message = fixtures::message::send(&app, &channel, &sender.login, &fixtures::now()).await; // Send the request - let deleter = fixtures::identity::create(&app, &fixtures::now()).await; let response = delete::handler( State(app.clone()), Path(message.id.clone()), fixtures::now(), - deleter, + sender, ) .await .expect("deleting a valid message succeeds"); @@ -68,7 +67,7 @@ pub async fn delete_deleted() { let message = fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await; app.messages() - .delete(&message.id, &fixtures::now()) + .delete(&sender, &message.id, &fixtures::now()) .await .expect("deleting a recently-sent message succeeds"); @@ -155,3 +154,31 @@ pub async fn delete_purged() { assert!(matches!(error, app::DeleteError::NotFound(id) if id == message.id)); } + +#[tokio::test] +pub async fn delete_not_sender() { + // Set up the environment + + let app = fixtures::scratch_app().await; + let sender = fixtures::login::create(&app, &fixtures::now()).await; + let channel = fixtures::channel::create(&app, &fixtures::now()).await; + let message = fixtures::message::send(&app, &channel, &sender, &fixtures::now()).await; + + // Send the request + + let deleter = fixtures::identity::create(&app, &fixtures::now()).await; + let delete::Error(error) = delete::handler( + State(app.clone()), + Path(message.id.clone()), + fixtures::now(), + deleter.clone(), + ) + .await + .expect_err("deleting a message someone else sent fails"); + + // Verify the response + + assert!( + matches!(error, app::DeleteError::NotSender(error_sender) if deleter.login == error_sender) + ); +} -- cgit v1.2.3