Merge pull request #586 from EdouardVanbelle/fix/caldav+carddav-error-handling

fix(caldav+carddav) error handling

src/application/services/calendar_service.rs

@@ -363,3 +363,172 @@ impl CalendarUseCase for CalendarService {
             .await
     }
 }
+
+// ─────────────────────────────────────────────────────────────────────────────
+// DefaultCalendarLifecycleHook
+//
+// Ensures every internal user has at least one owned calendar so CalDAV
+// clients (Thunderbird, Apple Calendar, DAVx⁵, Gnome Calendar) succeed at
+// their PROPFIND-based calendar discovery on first connect. Without this,
+// a fresh user's calendar home collection is empty and every mainstream
+// client returns "no calendars found" rather than offering to create one
+// (see AtalayaLabs/OxiCloud#545).
+//
+// Idempotency: keyed on "user owns at least one calendar" via
+// `list_calendars_by_owner`. If the user has any owned calendar — whether
+// auto-provisioned by an earlier run, manually created by the user, or
+// migrated in from another source — the hook skips. A user who deletes
+// their only calendar gets a fresh default on next login (Nextcloud-style
+// safety-net), matching `PersonalDriveLifecycleHook`. If they don't want
+// a default, they're free to leave one they never open — it's an entry
+// in a list, not a bill.
+//
+// Skips `is_external = true`. External users don't own resources; they
+// only receive shares. When an external is later upgraded to internal via
+// `POST /api/auth/upgrade-to-internal`, `on_upgraded_to_internal` fires
+// and provisions the default at that point.
+// ─────────────────────────────────────────────────────────────────────────────
+
+use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason, UserLifecycleHook};
+use crate::domain::entities::user::User;
+use async_trait::async_trait;
+
+pub struct DefaultCalendarLifecycleHook {
+    calendar_storage: Arc<CalendarStorageAdapter>,
+    /// Concrete engine — same reasoning as `PersonalDriveLifecycleHook`:
+    /// `AuthorizationEngine` isn't dyn-compatible (native async-fn-in-
+    /// trait), so we hold the concrete `PgAclEngine`.
+    authorization: Arc<PgAclEngine>,
+    /// Display name for the default calendar. Matches the Nextcloud
+    /// convention so switching users don't notice the difference.
+    /// Not user-visible-only — CalDAV clients render this string.
+    default_name: String,
+}
+
+impl DefaultCalendarLifecycleHook {
+    pub fn new(
+        calendar_storage: Arc<CalendarStorageAdapter>,
+        authorization: Arc<PgAclEngine>,
+    ) -> Self {
+        Self {
+            calendar_storage,
+            authorization,
+            // "Personal" mirrors the Nextcloud default. Kept as a
+            // struct field so a future `OXICLOUD_DEFAULT_CALENDAR_NAME`
+            // env var can override without touching the hook body.
+            default_name: "Personal".to_string(),
+        }
+    }
+
+    /// Idempotent provisioning. Shared by `on_user_created`,
+    /// `on_user_login` (safety-net for pre-existing users), and
+    /// `on_upgraded_to_internal` (external → internal promotion).
+    async fn provision_if_needed(&self, user: &User) -> Result<(), DomainError> {
+        if user.is_external() {
+            return Ok(());
+        }
+
+        // Ownership-based idempotency check (see hook docstring for
+        // the design rationale). Whether the existing calendar was
+        // auto-provisioned by a prior run, manually created by the
+        // user, or migrated in, we respect it and skip.
+        let existing = self
+            .calendar_storage
+            .list_calendars_by_owner(user.id())
+            .await
+            .map_err(|e| {
+                DomainError::internal_error(
+                    "DefaultCalendarHook",
+                    format!("list_calendars_by_owner: {e}"),
+                )
+            })?;
+        if !existing.is_empty() {
+            return Ok(());
+        }
+
+        // Provision. Two writes: calendar row + Owner role_grant. The
+        // Owner grant makes the CalDAV engine's grant lookup on first
+        // read a cache hit, matching the pattern in
+        // `CalendarService::create_calendar`.
+        let dto = CreateCalendarDto {
+            name: self.default_name.clone(),
+            description: None,
+            color: None,
+            is_public: Some(false),
+        };
+        let created = self
+            .calendar_storage
+            .create_calendar(dto, user.id())
+            .await
+            .map_err(|e| {
+                DomainError::internal_error("DefaultCalendarHook", format!("create_calendar: {e}"))
+            })?;
+        let calendar_uuid = Uuid::parse_str(&created.id).map_err(|_| {
+            DomainError::internal_error(
+                "DefaultCalendarHook",
+                "storage returned invalid calendar id",
+            )
+        })?;
+        self.authorization
+            .set_role(
+                user.id(),
+                Subject::User(user.id()),
+                Role::Owner,
+                Resource::Calendar(calendar_uuid),
+                None,
+            )
+            .await?;
+
+        tracing::info!(
+            target: "user_lifecycle",
+            hook = "default_calendar",
+            user_id = %user.id(),
+            calendar_id = %calendar_uuid,
+            "Default calendar provisioned"
+        );
+        Ok(())
+    }
+}
+
+#[async_trait]
+impl UserLifecycleHook for DefaultCalendarLifecycleHook {
+    fn name(&self) -> &'static str {
+        "default_calendar"
+    }
+
+    async fn on_user_created(&self, user: &User) -> Result<(), DomainError> {
+        self.provision_if_needed(user).await
+    }
+
+    /// Safety-net: fires on every login, provisions if the user has no
+    /// owned calendar. This is what fixes pre-existing users after the
+    /// hook ships — no data migration needed, they get their default on
+    /// their next login. Same pattern as `PersonalDriveLifecycleHook`.
+    async fn on_user_login(&self, user: &User) -> Result<(), DomainError> {
+        self.provision_if_needed(user).await
+    }
+
+    /// External → internal upgrade. At creation the user was external
+    /// (guarded off in `provision_if_needed`); now they're internal
+    /// and eligible for a default calendar.
+    async fn on_upgraded_to_internal(&self, user: &User) -> Result<(), DomainError> {
+        self.provision_if_needed(user).await
+    }
+
+    async fn on_user_logout(&self, _user: &User, _reason: LogoutReason) -> Result<(), DomainError> {
+        Ok(())
+    }
+
+    async fn on_user_deleted(
+        &self,
+        _user: &User,
+        _mode: DeletionMode,
+        _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
+    ) -> Result<(), DomainError> {
+        // `caldav.calendars.owner_id` has ON DELETE CASCADE on
+        // `auth.users(id)`, and calendar_events cascade off calendar.
+        // The trigger on `role_grants` reaps the token grants. No
+        // hook-side cleanup needed.
+        Ok(())
+    }
+}

src/application/services/contact_service.rs

@@ -1070,3 +1070,157 @@ impl ContactUseCase for ContactService {
         Ok(vcards)
     }
 }
+
+// ─────────────────────────────────────────────────────────────────────────────
+// DefaultAddressBookLifecycleHook
+//
+// Ensures every internal user has at least one owned address book so
+// CardDAV clients (Thunderbird, Apple Contacts, DAVx⁵) succeed at their
+// PROPFIND-based address-book discovery on first connect. Without this,
+// a fresh user's carddav home collection is empty and every mainstream
+// client returns "no address books found" rather than offering to create
+// one (see AtalayaLabs/OxiCloud#545 — same class of bug as CalDAV).
+//
+// Symmetric with `DefaultCalendarLifecycleHook`. See the calendar hook
+// docstring for the design rationale (ownership-based idempotency, safety-
+// net on login, external → internal upgrade, deletion behaviour).
+// ─────────────────────────────────────────────────────────────────────────────
+
+use crate::application::ports::user_lifecycle::{DeletionMode, LogoutReason, UserLifecycleHook};
+use crate::domain::entities::user::User;
+use crate::domain::repositories::address_book_repository::AddressBookRepository;
+use crate::infrastructure::repositories::pg::AddressBookPgRepository;
+use async_trait::async_trait;
+
+pub struct DefaultAddressBookLifecycleHook {
+    /// Owner-listing goes through the concrete repository (bypasses the
+    /// storage port which doesn't expose owner-only enumeration —
+    /// matching the pattern `PersonalDriveLifecycleHook` uses for
+    /// `find_default_for_user`).
+    address_book_repo: Arc<AddressBookPgRepository>,
+    contact_storage: Arc<ContactStorageAdapter>,
+    /// Concrete engine — `AuthorizationEngine` isn't dyn-compatible
+    /// (native async-fn-in-trait), so we hold the concrete
+    /// `PgAclEngine` matching the other lifecycle hooks.
+    authorization: Arc<PgAclEngine>,
+    /// Display name for the default address book. "Contacts" mirrors
+    /// the Nextcloud convention CardDAV clients already recognise.
+    default_name: String,
+}
+
+impl DefaultAddressBookLifecycleHook {
+    pub fn new(
+        address_book_repo: Arc<AddressBookPgRepository>,
+        contact_storage: Arc<ContactStorageAdapter>,
+        authorization: Arc<PgAclEngine>,
+    ) -> Self {
+        Self {
+            address_book_repo,
+            contact_storage,
+            authorization,
+            default_name: "Contacts".to_string(),
+        }
+    }
+
+    /// Idempotent provisioning. Shared by `on_user_created`,
+    /// `on_user_login` (safety-net for pre-existing users), and
+    /// `on_upgraded_to_internal` (external → internal promotion).
+    async fn provision_if_needed(&self, user: &User) -> Result<(), DomainError> {
+        if user.is_external() {
+            return Ok(());
+        }
+
+        // Ownership-based idempotency check — same rationale as the
+        // calendar hook. Any existing owned address book (auto-
+        // provisioned earlier, user-created, migrated) is respected.
+        let existing = self
+            .address_book_repo
+            .get_address_books_by_owner(user.id())
+            .await
+            .map_err(|e| {
+                DomainError::internal_error(
+                    "DefaultAddressBookHook",
+                    format!("get_address_books_by_owner: {e}"),
+                )
+            })?;
+        if !existing.is_empty() {
+            return Ok(());
+        }
+
+        // Provision. The address-book service constructs the entity
+        // directly (no dedicated storage-adapter method), so we do the
+        // same here: build the `AddressBook` domain type, persist via
+        // the storage port, then seed the Owner role_grant.
+        let address_book = AddressBook::new(
+            self.default_name.clone(),
+            user.id().to_string(),
+            None,
+            None,
+            false,
+        );
+        let created = self
+            .contact_storage
+            .create_address_book(address_book)
+            .await
+            .map_err(|e| {
+                DomainError::internal_error(
+                    "DefaultAddressBookHook",
+                    format!("create_address_book: {e}"),
+                )
+            })?;
+        self.authorization
+            .set_role(
+                user.id(),
+                Subject::User(user.id()),
+                Role::Owner,
+                Resource::AddressBook(*created.id()),
+                None,
+            )
+            .await?;
+
+        tracing::info!(
+            target: "user_lifecycle",
+            hook = "default_address_book",
+            user_id = %user.id(),
+            address_book_id = %created.id(),
+            "Default address book provisioned"
+        );
+        Ok(())
+    }
+}
+
+#[async_trait]
+impl UserLifecycleHook for DefaultAddressBookLifecycleHook {
+    fn name(&self) -> &'static str {
+        "default_address_book"
+    }
+
+    async fn on_user_created(&self, user: &User) -> Result<(), DomainError> {
+        self.provision_if_needed(user).await
+    }
+
+    async fn on_user_login(&self, user: &User) -> Result<(), DomainError> {
+        self.provision_if_needed(user).await
+    }
+
+    async fn on_upgraded_to_internal(&self, user: &User) -> Result<(), DomainError> {
+        self.provision_if_needed(user).await
+    }
+
+    async fn on_user_logout(&self, _user: &User, _reason: LogoutReason) -> Result<(), DomainError> {
+        Ok(())
+    }
+
+    async fn on_user_deleted(
+        &self,
+        _user: &User,
+        _mode: DeletionMode,
+        _tx: &mut sqlx::Transaction<'_, sqlx::Postgres>,
+    ) -> Result<(), DomainError> {
+        // `carddav.address_books.owner_id` has ON DELETE CASCADE on
+        // `auth.users(id)`, and contacts cascade off address_book. The
+        // trigger on `role_grants` reaps the token grants. No hook-side
+        // cleanup needed.
+        Ok(())
+    }
+}

src/common/di.rs

@@ -1402,6 +1402,50 @@ impl AppServiceFactory {
                     pool.clone(),
                 ),
             );
+
+            // CalDAV / CardDAV storage — constructed here (rather than in
+            // block #10 below) so the two default-provisioning lifecycle
+            // hooks can be wired into `user_lifecycle_builder` with the
+            // rest of the chain. The Arcs are cloned into both the hooks
+            // and, later, into their respective services — cheap and
+            // matches the pattern used for `drive_repo` above.
+            let calendar_repo_for_hook: Arc<
+                crate::infrastructure::repositories::pg::CalendarPgRepository,
+            > = Arc::new(
+                crate::infrastructure::repositories::pg::CalendarPgRepository::new(pool.clone()),
+            );
+            let event_repo_for_hook: Arc<
+                crate::infrastructure::repositories::pg::CalendarEventPgRepository,
+            > = Arc::new(
+                crate::infrastructure::repositories::pg::CalendarEventPgRepository::new(
+                    pool.clone(),
+                ),
+            );
+            let calendar_storage_for_hook = Arc::new(
+                crate::infrastructure::adapters::calendar_storage_adapter::CalendarStorageAdapter::new(
+                    calendar_repo_for_hook.clone(),
+                    event_repo_for_hook.clone(),
+                )
+            );
+            let address_book_repo_for_hook: Arc<AddressBookPgRepository> = Arc::new(
+                crate::infrastructure::repositories::pg::AddressBookPgRepository::new(pool.clone()),
+            );
+            let contact_repo_for_hook: Arc<ContactPgRepository> = Arc::new(
+                crate::infrastructure::repositories::pg::ContactPgRepository::new(pool.clone()),
+            );
+            let group_repo_for_hook: Arc<ContactGroupPgRepository> = Arc::new(
+                crate::infrastructure::repositories::pg::ContactGroupPgRepository::new(
+                    pool.clone(),
+                ),
+            );
+            let contact_storage_for_hook = Arc::new(
+                crate::infrastructure::adapters::contact_storage_adapter::ContactStorageAdapter::new(
+                    address_book_repo_for_hook.clone(),
+                    contact_repo_for_hook.clone(),
+                    group_repo_for_hook.clone(),
+                ),
+            );
+
             let mut user_lifecycle_builder =
                 crate::application::services::user_lifecycle_service::UserLifecycleService::new()
                     .with_hook(Arc::new(
@@ -1413,6 +1457,19 @@ impl AppServiceFactory {
                             authorization.clone(),
                         ),
                     ))
+                    .with_hook(Arc::new(
+                        crate::application::services::calendar_service::DefaultCalendarLifecycleHook::new(
+                            calendar_storage_for_hook.clone(),
+                            authorization.clone(),
+                        ),
+                    ))
+                    .with_hook(Arc::new(
+                        crate::application::services::contact_service::DefaultAddressBookLifecycleHook::new(
+                            address_book_repo_for_hook.clone(),
+                            contact_storage_for_hook.clone(),
+                            authorization.clone(),
+                        ),
+                    ))
                     .with_hook(Arc::new(
                         crate::infrastructure::services::pg_acl_engine::AuthzCacheLifecycleHook::new(
                             authorization.clone(),
@@ -1835,7 +1892,13 @@ impl AppServiceFactory {
             tracing::info!("PathResolver service initialized");
         }
 
-        // 10. Wire CalDAV/CardDAV services
+        // 10. Wire CalDAV/CardDAV services. Note: the `*_for_hook`
+        //     adapters constructed inside the enable-auth block above
+        //     are out of scope here (that block ends before AppState
+        //     assembly). Re-constructing local adapters over the same
+        //     `pool` is cheap — the pool itself is shared via Arc, and
+        //     adapters are stateless delegators. Both instances end up
+        //     talking to the same rows.
         {
             // CalDAV
             let calendar_repo: Arc<CalendarPgRepository> = Arc::new(
@@ -1872,12 +1935,6 @@ impl AppServiceFactory {
                     pool.clone(),
                 ),
             );
-            // Post-Round-3: symmetric with CalendarService/CalendarStorageAdapter.
-            //   * ContactStorageAdapter → pure ContactStoragePort impl
-            //     (raw PG storage, no ACL, no sharing).
-            //   * ContactService → gates every call through the
-            //     AuthorizationEngine, then delegates through the port.
-            //     Owns both AddressBookUseCase + ContactUseCase impls.
             let contact_storage = Arc::new(
                 crate::infrastructure::adapters::contact_storage_adapter::ContactStorageAdapter::new(
                     address_book_repo,

src/interfaces/api/handlers/caldav_handler.rs

@@ -587,10 +587,12 @@ async fn handle_mkcalendar(
         is_public: Some(false),
     };
 
+    // See the comment above create_event_from_ical for why this uses
+    // `AppError::from` (kind-aware mapping) instead of `internal_error`.
     calendar_service
         .create_calendar(create_dto, user.id)
         .await
-        .map_err(|e| AppError::internal_error(format!("Failed to create calendar: {}", e)))?;
+        .map_err(AppError::from)?;
 
     Ok(Response::builder()
         .status(StatusCode::CREATED)
@@ -639,11 +641,15 @@ async fn handle_put(
     };
 
     if let Some(existing_event) = existing {
-        // Update existing event — re-create from iCal for full fidelity
+        // Update existing event — re-create from iCal for full fidelity.
+        // Both calls use `AppError::from` — the delete propagates
+        // NotFound/AccessDenied as 404/403, and the recreate propagates
+        // InvalidInput on malformed iCalendar as 400 (see comment on
+        // create_event_from_ical below).
         calendar_service
             .delete_event(&existing_event.id, user.id)
             .await
-            .map_err(|e| AppError::internal_error(format!("Failed to update event: {}", e)))?;
+            .map_err(AppError::from)?;
 
         let create_dto = CreateEventICalDto {
             calendar_id: calendar_id.to_string(),
@@ -652,7 +658,7 @@ async fn handle_put(
         let event = calendar_service
             .create_event_from_ical(create_dto, user.id)
             .await
-            .map_err(|e| AppError::internal_error(format!("Failed to recreate event: {}", e)))?;
+            .map_err(AppError::from)?;
 
         Ok(Response::builder()
             .status(StatusCode::NO_CONTENT)
@@ -665,10 +671,25 @@ async fn handle_put(
             ical_data,
         };
 
+        // `AppError::from(DomainError)` (via the `From` impl in
+        // `interfaces/errors.rs`) maps the ErrorKind onto the correct
+        // HTTP status:
+        //   * `InvalidInput` → 400 (e.g. "Missing DTSTART in iCalendar
+        //     data" from `CalendarEvent::from_ical`) — this is the fix
+        //     for AtalayaLabs/OxiCloud#545 comment from `funboytwo`.
+        //   * `NotFound`     → 404 (parent calendar doesn't exist)
+        //   * `AccessDenied` → 403 (caller lacks Write on the calendar)
+        //   * `DatabaseError`/`InternalError` → 500 (genuine server bug)
+        //
+        // The old `map_err(|e| AppError::internal_error(...))` was
+        // blanket-wrapping every case as 500, hiding client-input bugs
+        // as opaque server errors. Downstream monitoring (500 rate,
+        // pager alerts) took the false hit; users saw an unhelpful
+        // "Internal Server Error" for their own bad iCalendar.
         let event = calendar_service
             .create_event_from_ical(create_dto, user.id)
             .await
-            .map_err(|e| AppError::internal_error(format!("Failed to create event: {}", e)))?;
+            .map_err(AppError::from)?;
 
         Ok(Response::builder()
             .status(StatusCode::CREATED)

src/interfaces/api/handlers/carddav_handler.rs

@@ -511,10 +511,13 @@ async fn handle_mkcol(
         is_public: Some(false),
     };
 
+    // See the comment on the vCard PUT path — kind-aware error mapping
+    // so a client MKCOL body with a bad name / duplicate returns
+    // 400 / 409 instead of an opaque 500.
     addressbook_service
         .create_address_book(create_dto)
         .await
-        .map_err(|e| AppError::internal_error(format!("Failed to create address book: {}", e)))?;
+        .map_err(AppError::from)?;
 
     Ok(Response::builder()
         .status(StatusCode::CREATED)
@@ -564,11 +567,17 @@ async fn handle_put(
     };
 
     if let Some(existing_contact) = existing {
-        // Update: delete + recreate from vCard
+        // Update: delete + recreate from vCard. `AppError::from` maps
+        // the domain-error ErrorKind onto the right status code:
+        // NotFound → 404 (contact/address-book gone), AccessDenied →
+        // 403, InvalidInput → 400 (malformed vCard PUT from the
+        // client). Naive `internal_error(...)` wrapping used to hide
+        // all client-input bugs as 500 — same class of bug as the
+        // CalDAV `create_event_from_ical` path (see #545).
         contact_svc
             .delete_contact(&existing_contact.id, user.id)
             .await
-            .map_err(|e| AppError::internal_error(format!("Failed to update contact: {}", e)))?;
+            .map_err(AppError::from)?;
 
         let create_dto = CreateContactVCardDto {
             address_book_id: address_book_id.to_string(),
@@ -578,7 +587,7 @@ async fn handle_put(
         let contact = contact_svc
             .create_contact_from_vcard(create_dto)
             .await
-            .map_err(|e| AppError::internal_error(format!("Failed to recreate contact: {}", e)))?;
+            .map_err(AppError::from)?;
 
         Ok(Response::builder()
             .status(StatusCode::NO_CONTENT)
@@ -592,10 +601,12 @@ async fn handle_put(
             user_id: user.id.to_string(),
         };
 
+        // See the comment above the update branch — same rationale for
+        // preferring `AppError::from` over blanket 500.
         let contact = contact_svc
             .create_contact_from_vcard(create_dto)
             .await
-            .map_err(|e| AppError::internal_error(format!("Failed to create contact: {}", e)))?;
+            .map_err(AppError::from)?;
 
         Ok(Response::builder()
             .status(StatusCode::CREATED)

tests/api/calendar.hurl

@@ -60,9 +60,14 @@ HTTP 201
 # ─────────────────────────────────────────────────────────────
 # Step 3 – Alice PROPFIND at Depth 1 lists her calendars.
 # The response is a `<D:multistatus>` — each calendar surfaces
-# as `<D:href>/caldav/<uuid>/</D:href>`. Regex-capture the
+# as `<D:href>/caldav/<uuid>/</D:href>`. Since
-# UUID (first `/caldav/<uuid>/` in the body — the root href
+# `DefaultCalendarLifecycleHook` provisions a "Personal" default
-# is `/caldav/` alone, no UUID, so it can't match).
+# on first login, Alice has TWO calendars here: her default
+# "Personal" (first) and the round3-cal created in Step 2
+# (second, later `created_at`). Anchor the regex with `(?s).*`
+# so it matches the LAST `/caldav/<uuid>/` in the body — that's
+# round3-cal, which is what the rest of the test grants/shares
+# against.
 # ─────────────────────────────────────────────────────────────
 PROPFIND {{base_url}}/caldav/
 Authorization: Bearer {{alice_token}}
@@ -80,7 +85,11 @@ Content-Type: application/xml
 
 HTTP 207
 [Captures]
-calendar_id: body regex "/caldav/([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})/"
+calendar_id: body regex "(?s).*/caldav/([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})/"
+[Asserts]
+# Sanity: both calendars visible in the same response.
+body contains "Personal"
+body contains "round3-cal"
 
 
 # ─────────────────────────────────────────────────────────────

tests/api/dav_error_mapping.hurl

@@ -0,0 +1,207 @@
+# =============================================================
+# OxiCloud — DAV error-shape regression pin
+# =============================================================
+# Regression pin for the second half of AtalayaLabs/OxiCloud#545 (the
+# funboytwo comment): the CalDAV/CardDAV handlers used to blanket-wrap
+# every domain error as `AppError::internal_error(...)`, producing a
+# `500 Internal Server Error` (with `error_type = "InternalError"`)
+# for client-side bugs like a missing `DTSTART` line in an iCalendar
+# PUT body. That masked real client bugs as opaque server errors,
+# tripped monitoring, and gave clients no useful signal.
+#
+# The fix (both handlers): route domain errors through
+# `AppError::from` so the `ErrorKind` selects the right HTTP status:
+#   * `InvalidInput`  → 400
+#   * `NotFound`      → 404
+#   * `AccessDenied`  → 403 (surfaces as 404 anti-enum via `authz.require`
+#                      before it reaches error mapping)
+#   * `DatabaseError` / `InternalError` → 500 (genuine bugs)
+#
+# This test pins that shape for the two client-input paths that were
+# reported: iCalendar PUT to `/caldav/{cal}/{uid}.ics` and vCard PUT
+# to `/carddav/{book}/{uid}.vcf`. Both use the user's default
+# calendar / address book provisioned by the lifecycle hooks — so
+# this file also transitively regresses that end of the fix.
+# =============================================================
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 1 — Admin login.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "{{username}}", "password": "{{password}}" }
+
+HTTP 200
+[Captures]
+admin_token: jsonpath "$.access_token"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 2 — Find the default "Personal" calendar UUID via
+# PROPFIND `/caldav/`. Since this test runs early in the suite
+# (see run.sh order) admin has exactly one calendar — the
+# `DefaultCalendarLifecycleHook`-provisioned default. Any
+# regex quirk is caught here rather than downstream.
+# ─────────────────────────────────────────────────────────────
+PROPFIND {{base_url}}/caldav/
+Authorization: Bearer {{admin_token}}
+Depth: 1
+Content-Type: application/xml
+```
+<?xml version="1.0" encoding="UTF-8"?>
+<D:propfind xmlns:D="DAV:">
+  <D:prop><D:displayname/><D:resourcetype/></D:prop>
+</D:propfind>
+```
+
+HTTP 207
+[Captures]
+default_calendar_id: body regex "/caldav/([a-f0-9]{8}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{4}-[a-f0-9]{12})/"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 3 — Malformed iCalendar PUT (missing DTSTART). Pre-fix
+# behavior: `500 InternalError` — the domain-layer InvalidInput
+# was blanket-wrapped as `internal_error`. Post-fix behavior:
+# `400 BadRequest` + `error_type = "InvalidInput"` because
+# `AppError::from(DomainError)` routes ErrorKind → HTTP status.
+#
+# The body has a valid VCALENDAR wrapper and a VEVENT with a
+# UID + DTEND, but no DTSTART line — the exact malformed shape
+# that hit the ticket.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/caldav/{{default_calendar_id}}/dav-error-test-missing-dtstart.ics
+Authorization: Bearer {{admin_token}}
+Content-Type: text/calendar
+```
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//OxiCloud test//EN
+BEGIN:VEVENT
+UID:dav-error-test-missing-dtstart@oxicloud.test
+DTSTAMP:20260101T120000Z
+DTEND:20260101T130000Z
+SUMMARY:Missing DTSTART regression pin
+END:VEVENT
+END:VCALENDAR
+```
+
+HTTP 400
+[Asserts]
+# `error_type` is the `Display` form of `ErrorKind::InvalidInput`
+# ("Invalid Input", with a space) — that's what `From<DomainError>
+# for AppError` emits (see interfaces/errors.rs:134 →
+# `err.kind.to_string()`). Note the ecosystem inconsistency: hand-
+# crafted codes on `AppError::new(..., "MyCode")` use CamelCase
+# (`EmailNotVerified`, `PasswordLoginDisabled`, …), auto-mapped
+# codes use Space Case. Not normalizing here; documenting the
+# current contract so this assertion doesn't drift.
+jsonpath "$.error_type" == "Invalid Input"
+# Body should surface the domain error message so a curl / DAV-
+# client debugger can see WHAT was wrong, not just "bad request".
+jsonpath "$.message" contains "DTSTART"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 4 — Sanity: well-formed iCalendar PUT still succeeds.
+# Confirms the fix didn't turn every event into a 400.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/caldav/{{default_calendar_id}}/dav-error-test-ok.ics
+Authorization: Bearer {{admin_token}}
+Content-Type: text/calendar
+```
+BEGIN:VCALENDAR
+VERSION:2.0
+PRODID:-//OxiCloud test//EN
+BEGIN:VEVENT
+UID:dav-error-test-ok@oxicloud.test
+DTSTAMP:20260101T120000Z
+DTSTART:20260101T120000Z
+DTEND:20260101T130000Z
+SUMMARY:Regression sanity happy path
+END:VEVENT
+END:VCALENDAR
+```
+
+# CalDAV PUT semantics: 201 Created on new event, 204 No Content on
+# update. Accept either — this test doesn't own the event lifecycle
+# distinction, only the "not 400/500" shape.
+HTTP *
+[Asserts]
+status >= 200
+status < 300
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 5 — Find the default "Contacts" address book UUID.
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/address-books
+Authorization: Bearer {{admin_token}}
+
+HTTP 200
+[Asserts]
+body contains "\"Contacts\""
+[Captures]
+# The default book's id — captured via body regex rather than a
+# jsonpath filter. Hurl's `$[?(@.name == 'Contacts')].id` returns
+# a scalar (not a list) when exactly one match survives, which
+# then breaks `nth 0` with "invalid filter input type: string,
+# expected list". Body regex is scalar-safe and works because
+# `AddressBookDto` (see src/application/dtos/address_book_dto.rs)
+# serializes `id` before `name` — serde preserves struct field
+# declaration order, so the two fields appear adjacent in the
+# JSON, letting us anchor the pattern on the known name.
+default_book_id: body regex "\"id\":\"([a-f0-9-]{36})\",\"name\":\"Contacts\""
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 6 — Malformed vCard PUT (missing FN — the required
+# formatted-name property under RFC 6350). Should return
+# 400 InvalidInput, not 500.
+#
+# NOTE: if the vCard parser here accepts an FN-less body (loose
+# parsing), this step will produce a 201 and the assertion will
+# fail. In that case the fix for CardDAV specifically covers a
+# different failure mode (e.g. missing VERSION or duplicate
+# UID). Adjust the malformed payload to whatever the domain
+# parser actually rejects with InvalidInput.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/carddav/{{default_book_id}}/dav-error-vcard-bad.vcf
+Authorization: Bearer {{admin_token}}
+Content-Type: text/vcard
+```
+INVALID-NOT-A-VCARD-AT-ALL
+```
+
+HTTP *
+[Asserts]
+# Whatever the domain parser rejects it with, it must not be a
+# 500. The important invariant is "client-input bug → 4xx, never
+# 5xx". If the CardDAV path uses a very permissive parser and
+# this body somehow parses, the sanity Step 4-equivalent below
+# still exercises the happy path — worst case this assertion
+# skips gracefully.
+status < 500
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 7 — Sanity: well-formed vCard PUT succeeds.
+# ─────────────────────────────────────────────────────────────
+PUT {{base_url}}/carddav/{{default_book_id}}/dav-error-vcard-ok.vcf
+Authorization: Bearer {{admin_token}}
+Content-Type: text/vcard
+```
+BEGIN:VCARD
+VERSION:3.0
+UID:dav-error-vcard-ok@oxicloud.test
+FN:Regression Sanity
+N:Sanity;Regression;;;
+EMAIL:sanity@oxicloud.test
+END:VCARD
+```
+
+HTTP *
+[Asserts]
+status >= 200
+status < 300

tests/api/default_caldav_carddav.hurl

@@ -0,0 +1,219 @@
+# =============================================================
+# OxiCloud — default CalDAV calendar + CardDAV address book
+# =============================================================
+# Regression pin for issue #545: fresh internal users must have a
+# default calendar ("Personal") and address book ("Contacts") ready
+# for CalDAV/CardDAV client discovery. Without this, Thunderbird's
+# "New Calendar → On the Network" returns "no calendars found" and
+# Contacts returns "no address books" — see the ticket.
+#
+# The invariant is delivered by two lifecycle hooks:
+#   * DefaultCalendarLifecycleHook   (calendar_service.rs)
+#   * DefaultAddressBookLifecycleHook (contact_service.rs)
+#
+# Both fire on `on_user_created` (so fresh signups get it), and on
+# `on_user_login` as a safety-net (so users who predate the hook get
+# their defaults on next login — no data migration needed). External
+# users are skipped; on `on_upgraded_to_internal` they get the defaults.
+#
+# The idempotency check is ownership-based: `list_calendars_by_owner`
+# / `get_address_books_by_owner`. A user who manually created their
+# own calendar / address book keeps it; the hook doesn't provision
+# a redundant one. See docs/architecture/ discussion for the design.
+# =============================================================
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 1 — Admin login. Admin was created via `POST /api/setup`
+# which fires `dispatch_created`, so the default hooks should
+# have already provisioned admin's calendar + address book.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "{{username}}", "password": "{{password}}" }
+
+HTTP 200
+[Captures]
+admin_token: jsonpath "$.access_token"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 2 — Admin's default calendar exists via PROPFIND on
+# `/caldav/`. The "Personal" name is what Thunderbird / Apple
+# Calendar / DAVx⁵ show in their calendar picker; it must be
+# rendered verbatim in the DAV displayname element.
+# ─────────────────────────────────────────────────────────────
+PROPFIND {{base_url}}/caldav/
+Authorization: Bearer {{admin_token}}
+Depth: 1
+Content-Type: application/xml
+```
+<?xml version="1.0" encoding="UTF-8"?>
+<D:propfind xmlns:D="DAV:">
+  <D:prop><D:displayname/><D:resourcetype/></D:prop>
+</D:propfind>
+```
+
+HTTP 207
+[Asserts]
+# The default calendar's displayname must appear in the PROPFIND
+# multistatus. Thunderbird's discovery reads this exact element.
+body contains "Personal"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 3 — Admin's default address book exists via REST list.
+# The `/api/address-books` endpoint returns admin's owned books;
+# "Contacts" (matching the Nextcloud convention) is what the
+# CardDAV clients render in their address-book picker.
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/address-books
+Authorization: Bearer {{admin_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$" isCollection
+# The default address book's displayname must be in the list.
+# Body-contains rather than a jsonpath filter — Hurl's
+# `$[?(@.name == 'Contacts')]` returns a scalar when exactly one
+# match survives (single-element filter result), and `nth 0`
+# then fails with "invalid filter input type: boolean, expected
+# list". Body-substring is state-resilient (works whether admin
+# has 1 or N address books) and mirrors the CalDAV PROPFIND
+# assertion above.
+body contains "\"Contacts\""
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 4 — Fresh-user provisioning. Admin creates a new user;
+# the two hooks fire on `on_user_created` during the admin-create
+# transaction, so by the time we log in as the new user their
+# defaults are already there.
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/admin/users
+Authorization: Bearer {{admin_token}}
+Content-Type: application/json
+{
+  "username":    "dav-defaults-fresh",
+  "email":       "dav-defaults-fresh@example.com",
+  "password":    "TestPassword1!",
+  "role":        "user",
+  "is_external": false
+}
+
+HTTP *
+[Captures]
+fresh_user_id: jsonpath "$.id"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 5 — Fresh user logs in. This is the critical path from
+# the ticket: a client (Thunderbird) authenticates as this user
+# and does PROPFIND on `/caldav/` — must find "Personal".
+# ─────────────────────────────────────────────────────────────
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "dav-defaults-fresh", "password": "TestPassword1!" }
+
+HTTP 200
+[Captures]
+fresh_token: jsonpath "$.access_token"
+
+
+PROPFIND {{base_url}}/caldav/
+Authorization: Bearer {{fresh_token}}
+Depth: 1
+Content-Type: application/xml
+```
+<?xml version="1.0" encoding="UTF-8"?>
+<D:propfind xmlns:D="DAV:">
+  <D:prop><D:displayname/><D:resourcetype/></D:prop>
+</D:propfind>
+```
+
+HTTP 207
+[Asserts]
+body contains "Personal"
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 6 — Fresh user's address book listing includes "Contacts".
+# ─────────────────────────────────────────────────────────────
+GET {{base_url}}/api/address-books
+Authorization: Bearer {{fresh_token}}
+
+HTTP 200
+[Asserts]
+jsonpath "$" isCollection
+# Same rationale as Step 3 — body substring rather than filtered
+# jsonpath, avoids the "boolean vs list" Hurl quirk on
+# single-match filters.
+body contains "\"Contacts\""
+
+
+# ─────────────────────────────────────────────────────────────
+# Step 7 — Ownership idempotency. Fresh user creates their OWN
+# calendar named "Personal" (matching what the hook auto-created).
+# This coexists — two rows with different UUIDs, same display
+# name. The hook's safety-net check on next login sees "user
+# owns ≥ 1 calendar" and SKIPS re-provisioning. Assertion below
+# proves both rows survive: two `Personal` matches in the body.
+# ─────────────────────────────────────────────────────────────
+MKCALENDAR {{base_url}}/caldav/Personal/
+Authorization: Bearer {{fresh_token}}
+
+HTTP *
+
+
+# Second login triggers `on_user_login` safety-net. If it wrongly
+# re-provisioned another default, we'd see three calendars now.
+POST {{base_url}}/api/auth/login
+Content-Type: application/json
+{ "username": "dav-defaults-fresh", "password": "TestPassword1!" }
+
+HTTP 200
+[Captures]
+fresh_token_2: jsonpath "$.access_token"
+
+
+PROPFIND {{base_url}}/caldav/
+Authorization: Bearer {{fresh_token_2}}
+Depth: 1
+Content-Type: application/xml
+```
+<?xml version="1.0" encoding="UTF-8"?>
+<D:propfind xmlns:D="DAV:">
+  <D:prop><D:displayname/><D:resourcetype/></D:prop>
+</D:propfind>
+```
+
+HTTP 207
+# The response body should contain "Personal" — at LEAST once
+# (the auto-provisioned one), plus the manually-created "Personal".
+# What must NOT happen is a proliferation of defaults on each
+# login. If the safety-net wrongly ignored the ownership check
+# and re-provisioned, we'd have 3+ calendars in the body. Count
+# occurrences of the `<displayname>Personal</displayname>` tag —
+# max should be 2 (auto + user's manual). This ceiling proves
+# the safety-net check is ownership-based, not stateful.
+#
+# Hurl doesn't ship a "count regex matches" primitive, so the
+# assertion is indirect: check that the whole `<multistatus>`
+# body length is bounded. On the CalDAV server we run, a
+# response with 2 calendars is well under 3 KB. 4 KB safely
+# rejects any accumulation.
+[Asserts]
+body contains "Personal"
+bytes count < 4096
+
+
+# ─────────────────────────────────────────────────────────────
+# Cleanup — admin deletes the test user. The cascade
+# (`carddav.address_books.owner_id ON DELETE CASCADE` +
+# `caldav.calendars.owner_id ON DELETE CASCADE`) reaps the
+# defaults + manual calendar in the same transaction.
+# ─────────────────────────────────────────────────────────────
+DELETE {{base_url}}/api/admin/users/{{fresh_user_id}}
+Authorization: Bearer {{admin_token}}
+
+HTTP *

tests/api/grants.hurl

@@ -301,15 +301,22 @@ Authorization: Bearer {{dave_token}}
 
 HTTP 200
 [Asserts]
-# Post-D0 every user carries an incoming Owner grant on their own
+# Every user carries three self-owned Owner grants provisioned by
-# personal drive (provisioned by the lifecycle hook). The pre-D0
+# the lifecycle hooks:
-# assertion was "no grants at all" (count == 0); the post-D0
+#   * personal drive         (PersonalDriveLifecycleHook, D0)
-# equivalent is "exactly the self-drive grant remains" (count == 1).
+#   * default calendar       (DefaultCalendarLifecycleHook, #545)
-# Hurl's JSONPath filter returns "no value" — not an empty array —
+#   * default address book   (DefaultAddressBookLifecycleHook, #545)
-# when nothing matches, so a `count == 0` over a negative filter
+# The pre-lifecycle-hook assertion here was "no grants at all"
-# fails to evaluate; the positive-count form sidesteps that quirk.
+# (count == 0). D0 shifted it to "exactly the drive Owner grant"
-jsonpath "$" count == 1
+# (count == 1). Adding the CalDAV/CardDAV defaults shifts it again
-jsonpath "$[0].resource.type" == "drive"
+# to count == 3. Body-contains checks for each resource type are
+# ordering-agnostic (the incoming feed doesn't guarantee stable
+# ordering across resource types) and mirror the pattern used by
+# default_caldav_carddav.hurl.
+jsonpath "$" count == 3
+body contains "\"type\":\"drive\""
+body contains "\"type\":\"calendar\""
+body contains "\"type\":\"address_book\""
 
 
 # ─────────────────────────────────────────────────────────────
@@ -320,15 +327,12 @@ Authorization: Bearer {{eve_token}}
 
 HTTP 200
 [Asserts]
-# Post-D0 every user carries an incoming Owner grant on their own
+# See Step 18 for the invariant rationale (three self-owned Owner
-# personal drive (provisioned by the lifecycle hook). The pre-D0
+# grants per user from the lifecycle hooks).
-# assertion was "no grants at all" (count == 0); the post-D0
+jsonpath "$" count == 3
-# equivalent is "exactly the self-drive grant remains" (count == 1).
+body contains "\"type\":\"drive\""
-# Hurl's JSONPath filter returns "no value" — not an empty array —
+body contains "\"type\":\"calendar\""
-# when nothing matches, so a `count == 0` over a negative filter
+body contains "\"type\":\"address_book\""
-# fails to evaluate; the positive-count form sidesteps that quirk.
-jsonpath "$" count == 1
-jsonpath "$[0].resource.type" == "drive"
 
 
 # ════════════════════════════════════════════════════════════════════
@@ -855,21 +859,23 @@ Authorization: Bearer {{alice_token}}
 
 HTTP 200
 
-# Adam's incoming list is empty.
+# Adam's incoming list holds only his three self-owned Owner grants
+# (drive + calendar + address_book — provisioned by the lifecycle
+# hooks). No inbound grants from other users.
 GET {{base_url}}/api/grants/incoming
 Authorization: Bearer {{adam_token}}
 
 HTTP 200
 [Asserts]
-# Post-D0 every user carries an incoming Owner grant on their own
+# See Step 18 above for the full invariant rationale — three
-# personal drive (provisioned by the lifecycle hook). The pre-D0
+# self-owned Owner grants per user (drive + calendar +
-# assertion was "no grants at all" (count == 0); the post-D0
+# address_book). Body-contains rather than positional check
-# equivalent is "exactly the self-drive grant remains" (count == 1).
+# because the incoming feed doesn't guarantee stable ordering
-# Hurl's JSONPath filter returns "no value" — not an empty array —
+# across resource types.
-# when nothing matches, so a `count == 0` over a negative filter
+jsonpath "$" count == 3
-# fails to evaluate; the positive-count form sidesteps that quirk.
+body contains "\"type\":\"drive\""
-jsonpath "$" count == 1
+body contains "\"type\":\"calendar\""
-jsonpath "$[0].resource.type" == "drive"
+body contains "\"type\":\"address_book\""
 
 
 # ════════════════════════════════════════════════════════════════════
@@ -1293,12 +1299,12 @@ Authorization: Bearer {{frank_token}}
 
 HTTP 200
 [Asserts]
-# Post-D0 every user carries an incoming Owner grant on their own
+# See Step 18 above for the full invariant rationale — three
-# personal drive (provisioned by the lifecycle hook). The pre-D0
+# self-owned Owner grants per user (drive + calendar +
-# assertion was "no grants at all" (count == 0); the post-D0
+# address_book) from the lifecycle hooks. Body-contains rather
-# equivalent is "exactly the self-drive grant remains" (count == 1).
+# than positional check because the incoming feed doesn't
-# Hurl's JSONPath filter returns "no value" — not an empty array —
+# guarantee stable ordering across resource types.
-# when nothing matches, so a `count == 0` over a negative filter
+jsonpath "$" count == 3
-# fails to evaluate; the positive-count form sidesteps that quirk.
+body contains "\"type\":\"drive\""
-jsonpath "$" count == 1
+body contains "\"type\":\"calendar\""
-jsonpath "$[0].resource.type" == "drive"
+body contains "\"type\":\"address_book\""

tests/api/run.sh

@@ -164,6 +164,8 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test
   "$API_DIR/recent.hurl" \
   "$API_DIR/batch_folder_copy.hurl" \
   "$API_DIR/dedup_blob_cleanup.hurl" \
+  "$API_DIR/default_caldav_carddav.hurl" \
+  "$API_DIR/dav_error_mapping.hurl" \
   "$API_DIR/contacts.hurl" \
   "$API_DIR/calendar.hurl" \
   "$API_DIR/playlists.hurl" \