From 180295ef1ac2e2779bd3a508565a6b3bdad49072 Mon Sep 17 00:00:00 2001 From: Lennart <18233294+lennart-k@users.noreply.github.com> Date: Thu, 6 Feb 2025 15:17:49 +0100 Subject: [PATCH] Implement If-Match, If-None-Match for DELETE method --- crates/caldav/src/calendar_object/methods.rs | 3 -- crates/caldav/src/calendar_object/resource.rs | 4 ++ crates/carddav/src/address_object/methods.rs | 3 -- crates/carddav/src/address_object/resource.rs | 4 ++ crates/dav/src/resource/methods/delete.rs | 18 ++++++++- crates/dav/src/resource/mod.rs | 37 +++++++++++++++++++ 6 files changed, 61 insertions(+), 8 deletions(-) diff --git a/crates/caldav/src/calendar_object/methods.rs b/crates/caldav/src/calendar_object/methods.rs index 4037e1d..3c2d483 100644 --- a/crates/caldav/src/calendar_object/methods.rs +++ b/crates/caldav/src/calendar_object/methods.rs @@ -60,9 +60,6 @@ pub async fn put_event( return Ok(HttpResponse::Unauthorized().body("")); } - // TODO: implement If-Match - // - let overwrite = Some(&HeaderValue::from_static("*")) != req.headers().get(header::IF_NONE_MATCH); diff --git a/crates/caldav/src/calendar_object/resource.rs b/crates/caldav/src/calendar_object/resource.rs index 84f2d04..650e969 100644 --- a/crates/caldav/src/calendar_object/resource.rs +++ b/crates/caldav/src/calendar_object/resource.rs @@ -90,6 +90,10 @@ impl Resource for CalendarObjectResource { Some(&self.principal) } + fn get_etag(&self) -> Option { + Some(self.object.get_etag()) + } + fn get_user_privileges(&self, user: &User) -> Result { Ok(UserPrivilegeSet::owner_only( user.is_principal(&self.principal), diff --git a/crates/carddav/src/address_object/methods.rs b/crates/carddav/src/address_object/methods.rs index e0bdc05..c143a6d 100644 --- a/crates/carddav/src/address_object/methods.rs +++ b/crates/carddav/src/address_object/methods.rs @@ -68,9 +68,6 @@ pub async fn put_object( return Err(Error::Unauthorized); } - // TODO: implement If-Match - // - let overwrite = Some(&HeaderValue::from_static("*")) != req.headers().get(header::IF_NONE_MATCH); diff --git a/crates/carddav/src/address_object/resource.rs b/crates/carddav/src/address_object/resource.rs index b170618..c4a1f32 100644 --- a/crates/carddav/src/address_object/resource.rs +++ b/crates/carddav/src/address_object/resource.rs @@ -86,6 +86,10 @@ impl Resource for AddressObjectResource { Some(&self.principal) } + fn get_etag(&self) -> Option { + Some(self.object.get_etag()) + } + fn get_user_privileges(&self, user: &User) -> Result { Ok(UserPrivilegeSet::owner_only( user.is_principal(&self.principal), diff --git a/crates/dav/src/resource/methods/delete.rs b/crates/dav/src/resource/methods/delete.rs index d93a7fa..f63ed2e 100644 --- a/crates/dav/src/resource/methods/delete.rs +++ b/crates/dav/src/resource/methods/delete.rs @@ -2,6 +2,9 @@ use crate::privileges::UserPrivilege; use crate::resource::Resource; use crate::resource::ResourceService; use crate::Error; +use actix_web::http::header::IfMatch; +use actix_web::http::header::IfNoneMatch; +use actix_web::web; use actix_web::web::Data; use actix_web::web::Path; use actix_web::HttpRequest; @@ -18,6 +21,8 @@ pub async fn route_delete( user: User, resource_service: Data, root_span: RootSpan, + if_match: web::Header, + if_none_match: web::Header, ) -> Result { let no_trash = req .headers() @@ -26,12 +31,21 @@ pub async fn route_delete( .unwrap_or(false); let resource = resource_service.get_resource(&path).await?; + let privileges = resource.get_user_privileges(&user)?; if !privileges.has(&UserPrivilege::Write) { - // TODO: Actually the spec wants us to look whether we have unbind access in the parent - // collection return Err(Error::Unauthorized.into()); } + + if !resource.satisfies_if_match(&if_match) { + // Precondition failed + return Ok(HttpResponse::PreconditionFailed().finish()); + } + if resource.satisfies_if_none_match(&if_none_match) { + // Precondition failed + return Ok(HttpResponse::PreconditionFailed().finish()); + } + resource_service.delete_resource(&path, !no_trash).await?; Ok(HttpResponse::Ok().body("")) diff --git a/crates/dav/src/resource/mod.rs b/crates/dav/src/resource/mod.rs index a5f4bd0..ff8ee11 100644 --- a/crates/dav/src/resource/mod.rs +++ b/crates/dav/src/resource/mod.rs @@ -4,6 +4,7 @@ use crate::xml::Resourcetype; use crate::xml::{multistatus::ResponseElement, TagList}; use crate::Error; use actix_web::dev::ResourceMap; +use actix_web::http::header::{EntityTag, IfMatch, IfNoneMatch}; use actix_web::{http::StatusCode, ResponseError}; use itertools::Itertools; use quick_xml::name::Namespace; @@ -56,6 +57,42 @@ pub trait Resource: Clone + 'static { None } + fn get_etag(&self) -> Option { + None + } + + fn satisfies_if_match(&self, if_match: &IfMatch) -> bool { + match if_match { + IfMatch::Any => true, + // This is not nice but if the header doesn't exist, actix just gives us an empty + // IfMatch::Items header + IfMatch::Items(items) if items.is_empty() => true, + IfMatch::Items(items) => { + if let Some(etag) = self.get_etag() { + let etag = EntityTag::new_strong(etag.to_owned()); + return items.iter().any(|item| item.strong_eq(&etag)); + } + false + } + } + } + + fn satisfies_if_none_match(&self, if_none_match: &IfNoneMatch) -> bool { + match if_none_match { + IfNoneMatch::Any => false, + // This is not nice but if the header doesn't exist, actix just gives us an empty + // IfNoneMatch::Items header + IfNoneMatch::Items(items) if items.is_empty() => false, + IfNoneMatch::Items(items) => { + if let Some(etag) = self.get_etag() { + let etag = EntityTag::new_strong(etag.to_owned()); + return items.iter().all(|item| item.strong_ne(&etag)); + } + true + } + } + } + fn get_user_privileges(&self, user: &User) -> Result; fn propfind(