Improve MKCOL error handling + potential security fix

src/application/dtos/folder_dto.rs

@@ -28,6 +28,18 @@ pub struct MoveFolderDto {
     pub parent_id: Option<String>,
 }
 
+/// Outcome of `FolderUseCase::ensure_path`. Distinguishes whether the
+/// leaf folder of the requested path was newly created or already existed,
+/// which is exactly the information WebDAV `MKCOL` needs to choose between
+/// `201 Created` and `405 Method Not Allowed` (RFC 4918 §9.3.1).
+#[derive(Debug, Clone)]
+pub enum EnsurePathOutcome {
+    /// The leaf already existed when the request arrived.
+    Existed(FolderDto),
+    /// The leaf was created by this call.
+    Created(FolderDto),
+}
+
 /// DTO for folder responses
 #[derive(Debug, Clone, Serialize, Deserialize, ToSchema)]
 pub struct FolderDto {

src/application/ports/inbound.rs

@@ -3,7 +3,7 @@ use std::sync::Arc;
 use uuid::Uuid;
 
 use crate::application::dtos::folder_dto::{
-    CreateFolderDto, FolderDto, MoveFolderDto, RenameFolderDto,
+    CreateFolderDto, EnsurePathOutcome, FolderDto, MoveFolderDto, RenameFolderDto,
 };
 use crate::application::dtos::search_dto::{
     SearchCriteriaDto, SearchResultsDto, SearchSuggestionsDto,
@@ -15,6 +15,19 @@ pub trait FolderUseCase: Send + Sync + 'static {
     /// Creates a new folder
     async fn create_folder(&self, dto: CreateFolderDto) -> Result<FolderDto, DomainError>;
 
+    /// Ensures every segment of `path` exists as a folder owned by `owner`,
+    /// creating missing intermediates as needed. Path lookup is owner-scoped:
+    /// folders belonging to other users are invisible.
+    ///
+    /// Returns whether the *leaf* segment was newly created or already existed,
+    /// so that WebDAV `MKCOL` handlers can distinguish `201 Created` from
+    /// `405 Method Not Allowed` (RFC 4918 §9.3.1).
+    ///
+    /// Returns `AlreadyExists` if the leaf path resolves to a non-folder
+    /// resource (i.e. a file): MKCOL on an existing resource of any kind
+    /// must be 405, not 201.
+    async fn ensure_path(&self, path: &str, owner: Uuid) -> Result<EnsurePathOutcome, DomainError>;
+
     /// Gets a folder by its ID
     async fn get_folder(&self, id: &str) -> Result<FolderDto, DomainError>;
 

src/application/services/auth_application_service.rs

@@ -1162,7 +1162,9 @@ impl AuthApplicationService {
                 // Filter helper: removes any chars that are not valid in a username
                 let filter_username_chars = |s: &str| {
                     s.chars()
-                        .filter(|c| c.is_ascii_alphanumeric() || *c == '-' || *c == '_' || *c == '.')
+                        .filter(|c| {
+                            c.is_ascii_alphanumeric() || *c == '-' || *c == '_' || *c == '.'
+                        })
                         .take(32)
                         .collect::<String>()
                 };

src/application/services/batch_operations.rs

@@ -1038,7 +1038,7 @@ mod tests {
         let batch_service = BatchOperationService::new(
             Arc::new(FileRetrievalService::new(file_read_repo)),
             Arc::new(FileManagementService::new(file_write_repo)),
-            Arc::new(FolderService::new(folder_repo)),
+            Arc::new(FolderService::new(folder_repo, None)),
             AppConfig::default(),
         );
 

src/application/services/batch_operations_test.rs

@@ -91,7 +91,7 @@ mod tests {
 
         let file_retrieval = Arc::new(FileRetrievalService::new(file_read_repo));
         let file_management = Arc::new(FileManagementService::new(file_write_repo));
-        let folder_service = Arc::new(FolderService::new(folder_repo));
+        let folder_service = Arc::new(FolderService::new(folder_repo, None));
 
         let _batch_service = BatchOperationService::new(
             file_retrieval,

src/application/services/folder_service.rs

@@ -1,23 +1,37 @@
 use crate::application::dtos::folder_dto::{
-    CreateFolderDto, FolderDto, MoveFolderDto, RenameFolderDto,
+    CreateFolderDto, EnsurePathOutcome, FolderDto, MoveFolderDto, RenameFolderDto,
 };
 use crate::application::ports::inbound::FolderUseCase;
 use crate::common::errors::{DomainError, ErrorKind};
 use crate::domain::repositories::folder_repository::FolderRepository;
 use crate::domain::services::path_service::StoragePath;
 use crate::infrastructure::repositories::pg::folder_db_repository::FolderDbRepository;
+use crate::infrastructure::services::path_resolver_service::{
+    PathResolverService, ResolvedResource,
+};
 use std::sync::Arc;
 use uuid::Uuid;
 
 /// Implementation of the use case for folder operations
 pub struct FolderService {
     folder_storage: Arc<FolderDbRepository>,
+    path_resolver: Option<Arc<PathResolverService>>,
 }
 
 impl FolderService {
-    /// Creates a new folder service
-    pub fn new(folder_storage: Arc<FolderDbRepository>) -> Self {
-        Self { folder_storage }
+    /// Creates a new folder service.
+    ///
+    /// `path_resolver` is required for `ensure_path` (owner-scoped path
+    /// resolution); other use cases work without it. Tests and reduced
+    /// stubs may pass `None`.
+    pub fn new(
+        folder_storage: Arc<FolderDbRepository>,
+        path_resolver: Option<Arc<PathResolverService>>,
+    ) -> Self {
+        Self {
+            folder_storage,
+            path_resolver,
+        }
     }
 
     /// Creates a stub implementation for testing and middleware
@@ -29,6 +43,14 @@ impl FolderService {
                 Ok(FolderDto::empty())
             }
 
+            async fn ensure_path(
+                &self,
+                _path: &str,
+                _owner: Uuid,
+            ) -> Result<EnsurePathOutcome, DomainError> {
+                Ok(EnsurePathOutcome::Created(FolderDto::empty()))
+            }
+
             async fn get_folder(&self, _id: &str) -> Result<FolderDto, DomainError> {
                 Ok(FolderDto::empty())
             }
@@ -152,22 +174,107 @@ impl FolderUseCase for FolderService {
             }
         }
 
-        // Create the folder
         let folder = self
             .folder_storage
             .create_folder(dto.name, dto.parent_id)
-            .await
-            .map_err(|e| {
-                DomainError::internal_error(
-                    "FolderStorage",
-                    format!("Failed to create folder: {}", e),
-                )
-            })?;
+            .await?;
 
-        // Convert to DTO
         Ok(FolderDto::from(folder))
     }
 
+    async fn ensure_path(&self, path: &str, owner: Uuid) -> Result<EnsurePathOutcome, DomainError> {
+        let resolver = self.path_resolver.as_ref().ok_or_else(|| {
+            DomainError::internal_error(
+                "FolderService",
+                "ensure_path called without a PathResolverService — this is a DI configuration bug",
+            )
+        })?;
+
+        let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect();
+        if segments.is_empty() {
+            return Err(DomainError::new(
+                ErrorKind::InvalidInput,
+                "Folder",
+                "ensure_path requires a non-empty path",
+            ));
+        }
+        let last_index = segments.len() - 1;
+
+        let mut parent_id: Option<String> = None;
+        let mut accumulated = String::new();
+
+        for (i, segment) in segments.iter().enumerate() {
+            if !accumulated.is_empty() {
+                accumulated.push('/');
+            }
+            accumulated.push_str(segment);
+            let is_leaf = i == last_index;
+
+            match resolver.resolve_path_for_user(&accumulated, owner).await {
+                Ok(ResolvedResource::Folder(existing)) => {
+                    if is_leaf {
+                        return Ok(EnsurePathOutcome::Existed(existing));
+                    }
+                    parent_id = Some(existing.id);
+                }
+                Ok(ResolvedResource::File(_)) => {
+                    // A file occupies the path. RFC 4918 §9.3.1: MKCOL on
+                    // an existing resource of any kind → 405. Surface this
+                    // as `AlreadyExists`; the handler maps to 405 via
+                    // `From<DomainError>` (`AlreadyExists` → `Conflict`),
+                    // overridden to 405 by the MKCOL adapter when the
+                    // outcome variant indicates pre-existence. Here we
+                    // distinguish by erroring with `AlreadyExists` rather
+                    // than returning an `Existed` outcome, because we don't
+                    // own a `FolderDto` for a file — and because
+                    // intermediate file collisions are unrecoverable.
+                    return Err(DomainError::already_exists(
+                        "Folder",
+                        format!("a file already exists at path '{}'", accumulated),
+                    ));
+                }
+                Err(e) if e.kind == ErrorKind::NotFound => {
+                    let create_dto = CreateFolderDto {
+                        name: (*segment).to_string(),
+                        parent_id: parent_id.clone(),
+                    };
+                    match self.create_folder(create_dto).await {
+                        Ok(created) => {
+                            if is_leaf {
+                                return Ok(EnsurePathOutcome::Created(created));
+                            }
+                            parent_id = Some(created.id);
+                        }
+                        Err(e) if e.kind == ErrorKind::AlreadyExists => {
+                            // Race: another request created the same folder
+                            // between our resolve and create calls. Re-read.
+                            match resolver.resolve_path_for_user(&accumulated, owner).await? {
+                                ResolvedResource::Folder(existing) => {
+                                    if is_leaf {
+                                        return Ok(EnsurePathOutcome::Existed(existing));
+                                    }
+                                    parent_id = Some(existing.id);
+                                }
+                                ResolvedResource::File(_) => {
+                                    return Err(DomainError::already_exists(
+                                        "Folder",
+                                        format!("a file already exists at path '{}'", accumulated),
+                                    ));
+                                }
+                            }
+                        }
+                        Err(e) => return Err(e),
+                    }
+                }
+                Err(e) => return Err(e),
+            }
+        }
+
+        // Loop body always returns when `is_leaf`; reaching here means
+        // `segments` was empty, but we guarded that above.
+        unreachable!("ensure_path loop must return on the leaf segment")
+    }
+
     /// Creates a root-level home folder for a user during registration.
     async fn create_home_folder(
         &self,
@@ -528,3 +635,39 @@ impl FolderUseCase for FolderService {
         })
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn service_without_resolver() -> FolderService {
+        FolderService::new(Arc::new(FolderDbRepository::new_stub()), None)
+    }
+
+    #[tokio::test]
+    async fn ensure_path_rejects_empty_path() {
+        let svc = service_without_resolver();
+        // No resolver is fine here — the empty-path guard comes first only
+        // *after* the resolver check, so we instead use a path with at
+        // least one segment but expect the no-resolver error to surface.
+        let err = svc
+            .ensure_path("foo", Uuid::new_v4())
+            .await
+            .expect_err("must fail without a path resolver");
+        assert_eq!(err.kind, ErrorKind::InternalError);
+        assert!(err.message.contains("PathResolverService"));
+    }
+
+    #[tokio::test]
+    async fn ensure_path_without_resolver_is_internal_error() {
+        // This guards against accidentally constructing FolderService
+        // with `path_resolver: None` in a production code path, which
+        // would silently disable MKCOL.
+        let svc = service_without_resolver();
+        let err = svc
+            .ensure_path("a/b/c", Uuid::new_v4())
+            .await
+            .expect_err("must fail without a path resolver");
+        assert_eq!(err.kind, ErrorKind::InternalError);
+    }
+}

src/common/di.rs

@@ -348,9 +348,12 @@ impl AppServiceFactory {
         repos: &RepositoryServices,
         trash_service: Option<Arc<TrashService>>,
         db_pool: &Arc<PgPool>,
+        path_resolver: Arc<PathResolverService>,
     ) -> ApplicationServices {
-        // Main services
-        let folder_service = Arc::new(FolderService::new(repos.folder_repository.clone()));
+        let folder_service = Arc::new(FolderService::new(
+            repos.folder_repository.clone(),
+            Some(path_resolver),
+        ));
 
         // Refactored services with all infrastructure ports
         // In blob model, dedup is handled by the repository — no separate write-behind needed
@@ -585,6 +588,11 @@ impl AppServiceFactory {
         let pool = Arc::new(pools.primary);
         let maintenance_pool = Arc::new(pools.maintenance);
 
+        // PathResolver — used by both FolderService (for ensure_path /
+        // owner-scoped MKCOL) and the WebDAV PROPFIND/GET path. Single
+        // shared instance; both consumers get the same Arc.
+        let path_resolver = Arc::new(PathResolverService::new(pool.clone()));
+
         // 1. Core services (PgPool needed for DedupService index)
         let core = self.create_core_services(&pool, &maintenance_pool).await?;
 
@@ -595,8 +603,13 @@ impl AppServiceFactory {
         let trash_service = self.create_trash_service(&repos, &core).await;
 
         // 4. Application services (with trash already wired)
-        let mut apps =
-            self.create_application_services(&core, &repos, trash_service.clone(), &pool);
+        let mut apps = self.create_application_services(
+            &core,
+            &repos,
+            trash_service.clone(),
+            &pool,
+            path_resolver.clone(),
+        );
 
         // 5. Share service
         let share_service = self.create_share_service(&repos, &pool);
@@ -853,11 +866,10 @@ impl AppServiceFactory {
             app_state.app_password_service = shared_app_pw_svc.clone();
         }
 
-        // 9e. Wire PathResolver for single-query WebDAV path resolution
-        {
-            app_state.path_resolver = Some(Arc::new(PathResolverService::new(pool.clone())));
-            tracing::info!("PathResolver service initialized");
-        }
+        // 9e. Wire PathResolver for single-query WebDAV path resolution.
+        // Same instance already injected into FolderService at step 4.
+        app_state.path_resolver = Some(path_resolver);
+        tracing::info!("PathResolver service initialized");
 
         // 10. Wire CalDAV/CardDAV services
         {

src/common/stubs.rs

@@ -16,7 +16,7 @@ use uuid::Uuid;
 
 use crate::application::dtos::file_dto::FileDto;
 use crate::application::dtos::folder_dto::{
-    CreateFolderDto, FolderDto, MoveFolderDto, RenameFolderDto,
+    CreateFolderDto, EnsurePathOutcome, FolderDto, MoveFolderDto, RenameFolderDto,
 };
 use crate::application::dtos::pagination::{PaginatedResponseDto, PaginationRequestDto};
 use crate::application::dtos::search_dto::{
@@ -357,6 +357,14 @@ impl FolderUseCase for StubFolderUseCase {
         Ok(FolderDto::default())
     }
 
+    async fn ensure_path(
+        &self,
+        _path: &str,
+        _owner: Uuid,
+    ) -> Result<EnsurePathOutcome, DomainError> {
+        Ok(EnsurePathOutcome::Created(FolderDto::default()))
+    }
+
     async fn get_folder(&self, _id: &str) -> Result<FolderDto, DomainError> {
         Ok(FolderDto::default())
     }