From fe852d3b798d65e7d03b4ebcdb81aee1c4ca3567 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 19 Jun 2026 08:38:27 +0000 Subject: [PATCH] fix(dav): repair CalDAV/CardDAV client connectivity (#480) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Standard CalDAV/CardDAV clients (Thunderbird, DAVx5, Apple Calendar/Contacts) failed to connect, mounted collections read-only, or could not discover address books, even though curl worked. Three protocol-compliance gaps caused this: 1. Missing Basic-auth challenge on /caldav and /carddav. The 401 returned for these surfaces carried no `WWW-Authenticate` header (only /webdav did). Spec-compliant clients never send credentials preemptively the way `curl -u` does — they wait for the challenge — so Thunderbird never authenticated and failed with "discovery failed" / 401. Extend the challenge to all DAV surfaces via shared `is_dav_path` / `dav_basic_auth_challenge` helpers. 2. Calendars always advertised read-only. The `current-user-privilege-set` write gate compared `owner_id` against the literal string "current_user_id", which never matched a real UUID, so `` was never emitted and clients mounted every calendar read-only. Thread the caller's id through the CalDAV adapter and grant write when the caller owns the calendar. 3. CardDAV discovery was incomplete. There was no `/.well-known/carddav` route and the root PROPFIND exposed neither `current-user-principal` nor `addressbook-home-set`, so clients could not locate address books. Add the well-known redirect and root/principal discovery responses mirroring the CalDAV adapter. Adds unit tests for the auth challenge predicate, the calendar owner/non-owner privilege split, and the CardDAV root/principal discovery responses. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_016cVV9nRQjP6G6a8zbNUWMw --- src/application/adapters/caldav_adapter.rs | 39 ++- .../adapters/caldav_adapter_test.rs | 54 ++++ src/application/adapters/carddav_adapter.rs | 265 +++++++++++++++++- .../adapters/carddav_adapter_test.rs | 91 ++++++ src/interfaces/api/handlers/caldav_handler.rs | 7 + .../api/handlers/carddav_handler.rs | 84 +++++- src/interfaces/middleware/auth.rs | 109 +++++-- src/main.rs | 4 +- 8 files changed, 605 insertions(+), 48 deletions(-) diff --git a/src/application/adapters/caldav_adapter.rs b/src/application/adapters/caldav_adapter.rs index fd6cfdb2..e7e67cff 100644 --- a/src/application/adapters/caldav_adapter.rs +++ b/src/application/adapters/caldav_adapter.rs @@ -17,6 +17,19 @@ use crate::application::adapters::webdav_adapter::{ }; use crate::application::dtos::calendar_dto::{CalendarDto, CalendarEventDto}; +/// Returns whether `caller_id` owns `calendar`. +/// +/// CalDAV clients (DAVx5, Apple Calendar, Thunderbird) only mount a collection +/// read-write when its `current-user-privilege-set` advertises ``, so +/// this gate decides read-only vs read-write for the caller. `caller_id` and +/// [`CalendarDto::owner_id`] are both the user's UUID rendered via +/// `Uuid::to_string()`, so a direct comparison is exact. Calendars merely shared +/// with the caller (non-owner access) stay read-only for now — this never +/// over-grants write. +fn caller_owns_calendar(calendar: &CalendarDto, caller_id: &str) -> bool { + !caller_id.is_empty() && calendar.owner_id == caller_id +} + /// CalDAV report type #[derive(Debug, PartialEq)] pub enum CalDavReportType { @@ -205,6 +218,7 @@ impl CalDavAdapter { request: &PropFindRequest, base_href: &str, username: &str, + caller_id: &str, ) -> Result<()> { let mut xml_writer = Writer::new(writer); @@ -227,6 +241,7 @@ impl CalDavAdapter { calendar, request, &format!("{}{}/", base_href, calendar.id), + caller_id, )?; } @@ -242,6 +257,7 @@ impl CalDavAdapter { calendars: &[CalendarDto], request: &PropFindRequest, base_href: &str, + caller_id: &str, ) -> Result<()> { let mut xml_writer = Writer::new(writer); @@ -261,6 +277,7 @@ impl CalDavAdapter { calendar, request, &format!("{}{}/", base_href, calendar.id), + caller_id, )?; } @@ -557,6 +574,7 @@ impl CalDavAdapter { calendar: &CalendarDto, request: &PropFindRequest, href: &str, + caller_id: &str, ) -> Result<()> { // Start response element xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; @@ -576,7 +594,7 @@ impl CalDavAdapter { match &request.prop_find_type { PropFindType::AllProp => { // Write all standard properties for a calendar - Self::write_calendar_standard_props(xml_writer, calendar)?; + Self::write_calendar_standard_props(xml_writer, calendar, caller_id)?; } PropFindType::PropName => { // Write only property names (empty elements) @@ -584,7 +602,7 @@ impl CalDavAdapter { } PropFindType::Prop(props) => { // Write requested properties - Self::write_calendar_requested_props(xml_writer, calendar, props)?; + Self::write_calendar_requested_props(xml_writer, calendar, props, caller_id)?; } } @@ -609,6 +627,7 @@ impl CalDavAdapter { fn write_calendar_standard_props( xml_writer: &mut Writer, calendar: &CalendarDto, + caller_id: &str, ) -> Result<()> { // Common WebDAV properties @@ -676,9 +695,10 @@ impl CalDavAdapter { xml_writer.write_event(Event::Empty(BytesStart::new("D:read")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:privilege")))?; - // Only add write privilege if user owns the calendar or has write access - if calendar.owner_id == "current_user_id" { - // This should be replaced with actual user check + // Advertise write only when the caller owns the calendar. Clients + // (DAVx5, Apple Calendar, Thunderbird) mount the collection read-only + // unless this privilege is present. + if caller_owns_calendar(calendar, caller_id) { xml_writer.write_event(Event::Start(BytesStart::new("D:privilege")))?; xml_writer.write_event(Event::Empty(BytesStart::new("D:write")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:privilege")))?; @@ -735,6 +755,7 @@ impl CalDavAdapter { xml_writer: &mut Writer, calendar: &CalendarDto, props: &[QualifiedName], + caller_id: &str, ) -> Result<()> { for prop in props { match (prop.namespace.as_str(), prop.name.as_str()) { @@ -780,9 +801,8 @@ impl CalDavAdapter { xml_writer.write_event(Event::Empty(BytesStart::new("D:read")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:privilege")))?; - // Only add write privilege if user owns the calendar or has write access - if calendar.owner_id == "current_user_id" { - // This should be replaced with actual user check + // Advertise write only when the caller owns the calendar. + if caller_owns_calendar(calendar, caller_id) { xml_writer.write_event(Event::Start(BytesStart::new("D:privilege")))?; xml_writer.write_event(Event::Empty(BytesStart::new("D:write")))?; xml_writer.write_event(Event::End(BytesEnd::new("D:privilege")))?; @@ -882,6 +902,7 @@ impl CalDavAdapter { request: &PropFindRequest, base_href: &str, depth: &str, + caller_id: &str, ) -> Result<()> { let mut xml_writer = Writer::new(writer); @@ -894,7 +915,7 @@ impl CalDavAdapter { ))?; // Write the calendar collection itself - Self::write_calendar_response(&mut xml_writer, calendar, request, base_href)?; + Self::write_calendar_response(&mut xml_writer, calendar, request, base_href, caller_id)?; // If depth > 0, include event resources if depth != "0" { diff --git a/src/application/adapters/caldav_adapter_test.rs b/src/application/adapters/caldav_adapter_test.rs index 75e21e3f..5f7847c6 100644 --- a/src/application/adapters/caldav_adapter_test.rs +++ b/src/application/adapters/caldav_adapter_test.rs @@ -171,6 +171,7 @@ mod tests { &calendars, &request, "/caldav/", + "user-001", ); assert!( @@ -210,6 +211,7 @@ mod tests { &request, "/caldav/cal-001", "0", + "user-001", ); assert!( @@ -240,6 +242,7 @@ mod tests { &request, "/caldav/cal-001", "1", + "user-001", ); assert!( @@ -258,6 +261,54 @@ mod tests { ); } + #[test] + fn test_owner_gets_write_privilege_but_non_owner_is_read_only() { + // Regression for #480: the privilege gate previously compared owner_id + // against the literal "current_user_id", so was never emitted + // and every CalDAV client mounted calendars read-only. + let calendar = sample_calendar(); // owner_id = "user-001" + let request = PropFindRequest { + prop_find_type: PropFindType::AllProp, + }; + + // Owner → read + write. + let mut owner_out = Vec::new(); + CalDavAdapter::generate_calendar_collection_propfind( + &mut owner_out, + &calendar, + &[], + &request, + "/caldav/cal-001/", + "0", + "user-001", + ) + .expect("owner propfind"); + let owner_xml = String::from_utf8(owner_out).expect("utf8"); + assert!( + owner_xml.contains("D:write"), + "Owner must be granted , got: {owner_xml}" + ); + + // A different caller (e.g. a read-only share) → read only, never write. + let mut other_out = Vec::new(); + CalDavAdapter::generate_calendar_collection_propfind( + &mut other_out, + &calendar, + &[], + &request, + "/caldav/cal-001/", + "0", + "a-different-user", + ) + .expect("non-owner propfind"); + let other_xml = String::from_utf8(other_out).expect("utf8"); + assert!(other_xml.contains("D:read"), "Non-owner keeps "); + assert!( + !other_xml.contains("D:write"), + "Non-owner must NOT get , got: {other_xml}" + ); + } + // ======================== // Calendar events response tests // ======================== @@ -434,6 +485,7 @@ mod tests { &request, "/caldav/", "testuser", + "user-001", ); assert!( result.is_ok(), @@ -483,6 +535,7 @@ mod tests { &request, "/caldav/", "testuser", + "user-001", ); assert!(result.is_ok()); @@ -565,6 +618,7 @@ mod tests { &request, "/caldav/cal-001/", "0", + "user-001", ); assert!(result.is_ok(), "Failed: {:?}", result.err()); diff --git a/src/application/adapters/carddav_adapter.rs b/src/application/adapters/carddav_adapter.rs index 2d0e16f2..2f6b66e4 100644 --- a/src/application/adapters/carddav_adapter.rs +++ b/src/application/adapters/carddav_adapter.rs @@ -17,6 +17,20 @@ use crate::application::adapters::webdav_adapter::{ use crate::application::dtos::address_book_dto::AddressBookDto; use crate::application::dtos::contact_dto::ContactDto; +/// Render a requested property as a namespaced response element name, mapping +/// the known namespaces to their response prefixes (`D:` for DAV, `CR:` for +/// CardDAV). Used for the catch-all arms of the requested-property writers so +/// the prefix mapping lives in exactly one place. +fn carddav_prop_name(prop: &QualifiedName) -> String { + if prop.namespace == "urn:ietf:params:xml:ns:carddav" { + format!("CR:{}", prop.name) + } else if prop.namespace == "DAV:" { + format!("D:{}", prop.name) + } else { + prop.name.clone() + } +} + /// CardDAV report type #[derive(Debug, PartialEq)] pub enum CardDavReportType { @@ -163,6 +177,96 @@ impl CardDavAdapter { Ok(()) } + /// Generate a PROPFIND response for the CardDAV root `/carddav/`. + /// + /// Mirrors the CalDAV root: emits a discovery entry for `/carddav/` itself + /// advertising `current-user-principal` and `addressbook-home-set` (the + /// properties DAVx5 / Apple Contacts read to locate address books), then — + /// at Depth > 0 — one entry per address book. Without these discovery + /// properties clients never find the address books at all. + pub fn generate_root_propfind_response( + writer: W, + address_books: &[AddressBookDto], + request: &PropFindRequest, + base_href: &str, + username: &str, + ) -> Result<()> { + let mut xml_writer = Writer::new(writer); + + xml_writer.write_event(Event::Start( + BytesStart::new("D:multistatus").with_attributes([ + ("xmlns:D", "DAV:"), + ("xmlns:CR", "urn:ietf:params:xml:ns:carddav"), + ("xmlns:CS", "http://calendarserver.org/ns/"), + ]), + ))?; + + Self::write_carddav_root_response(&mut xml_writer, request, base_href, username)?; + + for book in address_books { + Self::write_addressbook_response( + &mut xml_writer, + book, + request, + &format!("{}{}/", base_href, book.id), + )?; + } + + xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?; + Ok(()) + } + + /// Generate a PROPFIND response for a CardDAV user principal resource at + /// `/carddav/principals/{username}/`. + /// + /// Returns `addressbook-home-set` so clients can resolve the collection + /// holding the user's address books, plus a self-referential + /// `current-user-principal`. + pub fn generate_principal_propfind_response( + writer: W, + request: &PropFindRequest, + username: &str, + ) -> Result<()> { + let mut xml_writer = Writer::new(writer); + + xml_writer.write_event(Event::Start( + BytesStart::new("D:multistatus").with_attributes([ + ("xmlns:D", "DAV:"), + ("xmlns:CR", "urn:ietf:params:xml:ns:carddav"), + ("xmlns:CS", "http://calendarserver.org/ns/"), + ]), + ))?; + + xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; + + let href = format!("/carddav/principals/{}/", username); + xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; + xml_writer.write_event(Event::Text(BytesText::new(&href)))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; + + xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; + + match &request.prop_find_type { + PropFindType::AllProp | PropFindType::PropName => { + Self::write_carddav_principal_props(&mut xml_writer, username)?; + } + PropFindType::Prop(props) => { + Self::write_carddav_principal_requested_props(&mut xml_writer, username, props)?; + } + } + + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:response")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:multistatus")))?; + + Ok(()) + } + /// Generate PROPFIND for a single address book collection + contacts pub fn generate_addressbook_collection_propfind( writer: W, @@ -384,14 +488,159 @@ impl CardDavAdapter { .write_event(Event::End(BytesEnd::new("D:current-user-privilege-set")))?; } _ => { - let prop_name = if prop.namespace == "urn:ietf:params:xml:ns:carddav" { - format!("CR:{}", prop.name) - } else if prop.namespace == "DAV:" { - format!("D:{}", prop.name) - } else { - prop.name.clone() - }; - xml_writer.write_event(Event::Empty(BytesStart::new(&prop_name)))?; + xml_writer + .write_event(Event::Empty(BytesStart::new(carddav_prop_name(prop))))?; + } + } + } + Ok(()) + } + + /// Write a populated `current-user-principal` element pointing at the user's + /// CardDAV principal. Shared by the root and principal discovery responses. + fn write_current_user_principal( + xml_writer: &mut Writer, + username: &str, + ) -> Result<()> { + xml_writer.write_event(Event::Start(BytesStart::new("D:current-user-principal")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; + xml_writer.write_event(Event::Text(BytesText::new(&format!( + "/carddav/principals/{}/", + username + ))))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:current-user-principal")))?; + Ok(()) + } + + /// Write a populated `addressbook-home-set` element pointing at the user's + /// address-book home collection. Shared by the root and principal responses. + fn write_addressbook_home_set( + xml_writer: &mut Writer, + username: &str, + ) -> Result<()> { + xml_writer.write_event(Event::Start(BytesStart::new("CR:addressbook-home-set")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; + xml_writer.write_event(Event::Text(BytesText::new(&format!( + "/carddav/{}/", + username + ))))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; + xml_writer.write_event(Event::End(BytesEnd::new("CR:addressbook-home-set")))?; + Ok(()) + } + + /// Write the root `/carddav/` discovery entry. + fn write_carddav_root_response( + xml_writer: &mut Writer, + request: &PropFindRequest, + href: &str, + username: &str, + ) -> Result<()> { + xml_writer.write_event(Event::Start(BytesStart::new("D:response")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:href")))?; + xml_writer.write_event(Event::Text(BytesText::new(href)))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:href")))?; + + xml_writer.write_event(Event::Start(BytesStart::new("D:propstat")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:prop")))?; + + match &request.prop_find_type { + PropFindType::AllProp => { + xml_writer.write_event(Event::Start(BytesStart::new("D:resourcetype")))?; + xml_writer.write_event(Event::Empty(BytesStart::new("D:collection")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:resourcetype")))?; + Self::write_current_user_principal(xml_writer, username)?; + Self::write_addressbook_home_set(xml_writer, username)?; + } + PropFindType::PropName => { + xml_writer.write_event(Event::Empty(BytesStart::new("D:resourcetype")))?; + xml_writer + .write_event(Event::Empty(BytesStart::new("D:current-user-principal")))?; + xml_writer.write_event(Event::Empty(BytesStart::new("CR:addressbook-home-set")))?; + } + PropFindType::Prop(props) => { + for prop in props { + match (prop.namespace.as_str(), prop.name.as_str()) { + ("DAV:", "resourcetype") => { + xml_writer + .write_event(Event::Start(BytesStart::new("D:resourcetype")))?; + xml_writer + .write_event(Event::Empty(BytesStart::new("D:collection")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:resourcetype")))?; + } + ("DAV:", "current-user-principal") => { + Self::write_current_user_principal(xml_writer, username)?; + } + ("urn:ietf:params:xml:ns:carddav", "addressbook-home-set") => { + Self::write_addressbook_home_set(xml_writer, username)?; + } + _ => { + xml_writer.write_event(Event::Empty(BytesStart::new( + carddav_prop_name(prop), + )))?; + } + } + } + } + } + + xml_writer.write_event(Event::End(BytesEnd::new("D:prop")))?; + xml_writer.write_event(Event::Start(BytesStart::new("D:status")))?; + xml_writer.write_event(Event::Text(BytesText::new("HTTP/1.1 200 OK")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:status")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:propstat")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:response")))?; + Ok(()) + } + + /// Write the standard properties for a CardDAV principal resource. + fn write_carddav_principal_props( + xml_writer: &mut Writer, + username: &str, + ) -> Result<()> { + xml_writer.write_event(Event::Start(BytesStart::new("D:resourcetype")))?; + xml_writer.write_event(Event::Empty(BytesStart::new("D:collection")))?; + xml_writer.write_event(Event::Empty(BytesStart::new("D:principal")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:resourcetype")))?; + + xml_writer.write_event(Event::Start(BytesStart::new("D:displayname")))?; + xml_writer.write_event(Event::Text(BytesText::new(username)))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:displayname")))?; + + Self::write_addressbook_home_set(xml_writer, username)?; + Self::write_current_user_principal(xml_writer, username)?; + Ok(()) + } + + /// Write the requested properties for a CardDAV principal resource. + fn write_carddav_principal_requested_props( + xml_writer: &mut Writer, + username: &str, + props: &[QualifiedName], + ) -> Result<()> { + for prop in props { + match (prop.namespace.as_str(), prop.name.as_str()) { + ("DAV:", "resourcetype") => { + xml_writer.write_event(Event::Start(BytesStart::new("D:resourcetype")))?; + xml_writer.write_event(Event::Empty(BytesStart::new("D:collection")))?; + xml_writer.write_event(Event::Empty(BytesStart::new("D:principal")))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:resourcetype")))?; + } + ("DAV:", "displayname") => { + xml_writer.write_event(Event::Start(BytesStart::new("D:displayname")))?; + xml_writer.write_event(Event::Text(BytesText::new(username)))?; + xml_writer.write_event(Event::End(BytesEnd::new("D:displayname")))?; + } + ("DAV:", "current-user-principal") => { + Self::write_current_user_principal(xml_writer, username)?; + } + ("urn:ietf:params:xml:ns:carddav", "addressbook-home-set") => { + Self::write_addressbook_home_set(xml_writer, username)?; + } + _ => { + xml_writer + .write_event(Event::Empty(BytesStart::new(carddav_prop_name(prop))))?; } } } diff --git a/src/application/adapters/carddav_adapter_test.rs b/src/application/adapters/carddav_adapter_test.rs index bc51e080..ac50a647 100644 --- a/src/application/adapters/carddav_adapter_test.rs +++ b/src/application/adapters/carddav_adapter_test.rs @@ -307,6 +307,97 @@ mod tests { ); } + #[test] + fn test_root_propfind_advertises_principal_and_home_set() { + // Regression for #480: without these discovery properties DAVx5 / Apple + // Contacts never locate the user's address books. + let books = vec![sample_address_book()]; + let request = PropFindRequest { + prop_find_type: PropFindType::AllProp, + }; + + let mut output = Vec::new(); + CardDavAdapter::generate_root_propfind_response( + &mut output, + &books, + &request, + "/carddav/", + "testuser", + ) + .expect("root propfind"); + + let xml = String::from_utf8(output).expect("utf8"); + assert!( + xml.contains("/carddav/principals/testuser/"), + "Root must expose current-user-principal href, got: {xml}" + ); + assert!( + xml.contains("/carddav/testuser/"), + "Root must expose addressbook-home-set href, got: {xml}" + ); + // Depth 1 also enumerates the books. + assert!(xml.contains("ab-001"), "Should list address book"); + } + + #[test] + fn test_root_propfind_prop_request_returns_populated_discovery() { + // A DAVx5-style targeted request for the two discovery properties. + let request = PropFindRequest { + prop_find_type: PropFindType::Prop(vec![ + QualifiedName { + namespace: "DAV:".to_string(), + name: "current-user-principal".to_string(), + }, + QualifiedName { + namespace: "urn:ietf:params:xml:ns:carddav".to_string(), + name: "addressbook-home-set".to_string(), + }, + ]), + }; + + let mut output = Vec::new(); + CardDavAdapter::generate_root_propfind_response( + &mut output, + &[], + &request, + "/carddav/", + "testuser", + ) + .expect("root propfind"); + + let xml = String::from_utf8(output).expect("utf8"); + assert!(xml.contains("/carddav/principals/testuser/")); + assert!(xml.contains("/carddav/testuser/")); + // Properties must be populated, not empty self-closing placeholders. + assert!(!xml.contains("")); + assert!(!xml.contains("")); + } + + #[test] + fn test_principal_propfind_returns_home_set() { + let request = PropFindRequest { + prop_find_type: PropFindType::AllProp, + }; + + let mut output = Vec::new(); + CardDavAdapter::generate_principal_propfind_response(&mut output, &request, "testuser") + .expect("principal propfind"); + + let xml = String::from_utf8(output).expect("utf8"); + assert!( + xml.contains("/carddav/principals/testuser/"), + "Principal href should be present" + ); + assert!( + xml.contains("/carddav/testuser/"), + "addressbook-home-set should be present" + ); + assert!( + xml.contains("D:principal"), + "resourcetype should include principal" + ); + } + #[test] fn test_generate_addressbook_collection_propfind_depth_0() { let addressbook = sample_address_book(); diff --git a/src/interfaces/api/handlers/caldav_handler.rs b/src/interfaces/api/handlers/caldav_handler.rs index b6d6eb88..a4f6e22b 100644 --- a/src/interfaces/api/handlers/caldav_handler.rs +++ b/src/interfaces/api/handlers/caldav_handler.rs @@ -218,6 +218,9 @@ async fn handle_propfind( .to_string(); let user = extract_user(&req)?; + // Caller UUID (string form) — gates the `` privilege on calendars + // the caller owns, so clients mount their own calendars read-write. + let caller_id = user.id.to_string(); let calendar_service = get_calendar_service(&state)?; let body_bytes = body::to_bytes(req.into_body(), MAX_CALDAV_BODY) @@ -256,6 +259,7 @@ async fn handle_propfind( &propfind_request, base_href, &user.username, + &caller_id, ) .map_err(|e| AppError::internal_error(format!("Failed to generate XML: {}", e)))?; @@ -333,6 +337,7 @@ async fn handle_propfind( &propfind_request, base_href, &depth, + &caller_id, ) .map_err(|e| AppError::internal_error(format!("Failed to generate XML: {}", e)))?; @@ -359,6 +364,7 @@ async fn handle_propfind( &calendars, &propfind_request, base_href, + &caller_id, ) .map_err(|e| AppError::internal_error(format!("Failed to generate XML: {}", e)))?; @@ -407,6 +413,7 @@ async fn handle_propfind( &propfind_request, base_href, &depth, + &caller_id, ) .map_err(|e| { AppError::internal_error(format!("Failed to generate XML: {}", e)) diff --git a/src/interfaces/api/handlers/carddav_handler.rs b/src/interfaces/api/handlers/carddav_handler.rs index 1f45b017..29a8260a 100644 --- a/src/interfaces/api/handlers/carddav_handler.rs +++ b/src/interfaces/api/handlers/carddav_handler.rs @@ -58,6 +58,24 @@ pub fn carddav_routes() -> Router> { .route("/carddav", axum::routing::any(handle_carddav_methods_root)) } +/// Creates the RFC 6764 well-known discovery route for CardDAV. +/// Public (no auth) — simply redirects to the CardDAV root so clients that +/// bootstrap from `/.well-known/carddav` can locate the service. +pub fn well_known_routes() -> Router> { + Router::new().route( + "/.well-known/carddav", + axum::routing::any(handle_well_known_carddav), + ) +} + +async fn handle_well_known_carddav() -> Response { + Response::builder() + .status(StatusCode::MOVED_PERMANENTLY) + .header(header::LOCATION, "/carddav/") + .body(Body::empty()) + .unwrap() +} + async fn handle_carddav_methods_root( axum::extract::State(state): axum::extract::State>, req: Request, @@ -226,10 +244,66 @@ async fn handle_propfind( .map_err(|e| AppError::bad_request(format!("Failed to parse PROPFIND: {}", e)))? }; + // Discovery: the true root `/carddav/` advertises current-user-principal and + // addressbook-home-set so clients (DAVx5, Apple Contacts) can locate the + // address books. Depth 0 → only the root entry; Depth 1+ → also the books. + if path.is_empty() { + let address_books = if depth == "0" { + vec![] + } else { + addressbook_service + .list_user_address_books(user.id) + .await + .map_err(|e| { + AppError::internal_error(format!("Failed to list address books: {}", e)) + })? + }; + + let mut response_body = Vec::new(); + CardDavAdapter::generate_root_propfind_response( + &mut response_body, + &address_books, + &propfind_request, + "/carddav/", + &user.username, + ) + .map_err(|e| AppError::internal_error(format!("Failed to generate XML: {}", e)))?; + + return Ok(Response::builder() + .status(StatusCode::MULTI_STATUS) + .header(header::CONTENT_TYPE, "application/xml; charset=utf-8") + .body(Body::from(response_body)) + .unwrap()); + } + + // Discovery: principal resource `/carddav/principals/{username}/` returns the + // addressbook-home-set the client should enumerate next. + if path == "principals" || path.starts_with("principals/") { + let username = path + .strip_prefix("principals/") + .map(|s| s.trim_end_matches('/')) + .filter(|s| !s.is_empty()) + .unwrap_or(&user.username); + + let mut response_body = Vec::new(); + CardDavAdapter::generate_principal_propfind_response( + &mut response_body, + &propfind_request, + username, + ) + .map_err(|e| AppError::internal_error(format!("Failed to generate XML: {}", e)))?; + + return Ok(Response::builder() + .status(StatusCode::MULTI_STATUS) + .header(header::CONTENT_TYPE, "application/xml; charset=utf-8") + .body(Body::from(response_body)) + .unwrap()); + } + let effective_path = strip_username_prefix(path); if effective_path.is_empty() { - // Root CardDAV path or user home — list user's address books + // User address-book home `/carddav/{username}/` — list the user's books. let address_books = addressbook_service .list_user_address_books(user.id) .await @@ -237,12 +311,8 @@ async fn handle_propfind( AppError::internal_error(format!("Failed to list address books: {}", e)) })?; - let base_href = if path.is_empty() { - "/carddav/".to_string() - } else { - let user_part = path.split('/').next().unwrap_or(path); - format!("/carddav/{}/", user_part) - }; + let user_part = path.split('/').next().unwrap_or(path); + let base_href = format!("/carddav/{}/", user_part); let mut response_body = Vec::new(); CardDavAdapter::generate_addressbooks_propfind_response( &mut response_body, diff --git a/src/interfaces/middleware/auth.rs b/src/interfaces/middleware/auth.rs index 34ca7c1a..547523c7 100644 --- a/src/interfaces/middleware/auth.rs +++ b/src/interfaces/middleware/auth.rs @@ -240,17 +240,12 @@ pub async fn auth_middleware( } Err(e) => { tracing::warn!("App password verification failed: {}", e); - // For WebDAV: include WWW-Authenticate so the client - // knows to re-prompt rather than silently failing. - if request.uri().path().starts_with("/webdav") { - return Ok(Response::builder() - .status(StatusCode::UNAUTHORIZED) - .header(header::WWW_AUTHENTICATE, r#"Basic realm="OxiCloud""#) - .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") - .body(axum::body::Body::from( - "Invalid username or app password", - )) - .unwrap()); + // For DAV clients: include WWW-Authenticate so the client + // re-prompts for credentials rather than failing silently. + if is_dav_path(request.uri().path()) { + return Ok(dav_basic_auth_challenge( + "Invalid username or app password", + )); } return Err(AuthError::InvalidToken( "Invalid username or app password".to_string(), @@ -312,23 +307,41 @@ pub async fn auth_middleware( return Err(AuthError::AuthServiceUnavailable); } - // For WebDAV requests with no credentials at all: return 401 with - // WWW-Authenticate so that spec-compliant clients (Nautilus, Cyberduck, - // Windows Explorer, macOS Finder) know to prompt for a username/password. - // Non-WebDAV routes return the standard AuthError which renders without - // this header — keeping browser sessions redirecting to /login as before. - if request.uri().path().starts_with("/webdav") { - return Ok(Response::builder() - .status(StatusCode::UNAUTHORIZED) - .header(header::WWW_AUTHENTICATE, r#"Basic realm="OxiCloud""#) - .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") - .body(axum::body::Body::from("Authentication required")) - .unwrap()); + // For DAV requests with no credentials at all: return 401 with + // WWW-Authenticate so that spec-compliant clients (Thunderbird, DAVx5, + // Apple Calendar/Contacts, Nautilus, Cyberduck, Windows Explorer, macOS + // Finder) know to prompt for credentials and retry. Unlike `curl -u`, these + // clients do NOT send Basic credentials preemptively — without the + // challenge they never authenticate and fail with "discovery failed" / 401. + // Non-DAV routes return the standard AuthError which renders without this + // header — keeping browser sessions redirecting to /login as before. + if is_dav_path(request.uri().path()) { + return Ok(dav_basic_auth_challenge("Authentication required")); } Err(AuthError::TokenNotProvided) } +/// DAV protocol surfaces (WebDAV, CalDAV, CardDAV) authenticate over HTTP Basic. +/// Spec-compliant clients (Thunderbird, DAVx5, Apple Calendar/Contacts, file +/// managers) only send credentials after receiving a `401` carrying a +/// `WWW-Authenticate: Basic` challenge, so these paths must emit it. Browser and +/// JSON-API routes deliberately do not, so they keep redirecting to `/login`. +fn is_dav_path(path: &str) -> bool { + path.starts_with("/webdav") || path.starts_with("/caldav") || path.starts_with("/carddav") +} + +/// Build the `401 Unauthorized` Basic-auth challenge shared by every DAV +/// surface, so clients re-prompt for credentials instead of failing silently. +fn dav_basic_auth_challenge(message: &'static str) -> Response { + Response::builder() + .status(StatusCode::UNAUTHORIZED) + .header(header::WWW_AUTHENTICATE, r#"Basic realm="OxiCloud""#) + .header(header::CONTENT_TYPE, "text/plain; charset=utf-8") + .body(axum::body::Body::from(message)) + .unwrap() +} + /// Middleware to verify that the authenticated user has an admin role. /// /// Must be applied AFTER auth_middleware, as it depends on @@ -353,3 +366,53 @@ pub async fn require_admin(request: Request, next: Next) -> Response { let error = AuthError::AccessDenied("Admin role required".to_string()); error.into_response() } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn dav_paths_receive_basic_auth_challenge() { + // Regression for #480: CalDAV/CardDAV clients (Thunderbird, DAVx5) only + // send credentials after a 401 carrying WWW-Authenticate. All three DAV + // surfaces must qualify so the challenge is emitted. + for path in [ + "/webdav/", + "/webdav/admin/file.txt", + "/caldav/", + "/caldav/admin/cal/", + "/carddav/", + "/carddav/principals/admin/", + ] { + assert!(is_dav_path(path), "{path} should be treated as a DAV path"); + } + } + + #[test] + fn non_dav_paths_do_not_receive_basic_auth_challenge() { + for path in [ + "/", + "/api/files", + "/login", + "/index.html", + "/.well-known/caldav", + ] { + assert!( + !is_dav_path(path), + "{path} must not get a Basic-auth challenge (browser/API surface)" + ); + } + } + + #[test] + fn challenge_sets_www_authenticate_header() { + let resp = dav_basic_auth_challenge("Authentication required"); + assert_eq!(resp.status(), StatusCode::UNAUTHORIZED); + assert_eq!( + resp.headers() + .get(header::WWW_AUTHENTICATE) + .and_then(|v| v.to_str().ok()), + Some(r#"Basic realm="OxiCloud""#), + ); + } +} diff --git a/src/main.rs b/src/main.rs index 3aa56196..6f462fbb 100644 --- a/src/main.rs +++ b/src/main.rs @@ -311,7 +311,9 @@ async fn main() -> Result<(), Box> { use oxicloud::interfaces::api::handlers::carddav_handler; use oxicloud::interfaces::api::handlers::webdav_handler; let caldav_router = caldav_handler::caldav_routes(); - let well_known_router = caldav_handler::well_known_routes(); + // RFC 6764 discovery for both CalDAV and CardDAV (public redirects). + let well_known_router = + caldav_handler::well_known_routes().merge(carddav_handler::well_known_routes()); let carddav_router = carddav_handler::carddav_routes(); let webdav_router = webdav_handler::webdav_routes();