diff --git a/src/application/dtos/folder_dto.rs b/src/application/dtos/folder_dto.rs index cedf2289..0724069b 100644 --- a/src/application/dtos/folder_dto.rs +++ b/src/application/dtos/folder_dto.rs @@ -28,6 +28,18 @@ pub struct MoveFolderDto { pub parent_id: Option, } +/// 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 { diff --git a/src/application/ports/inbound.rs b/src/application/ports/inbound.rs index 3a7380ac..ee240a89 100644 --- a/src/application/ports/inbound.rs +++ b/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; + /// 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; + /// Gets a folder by its ID async fn get_folder(&self, id: &str) -> Result; diff --git a/src/application/services/auth_application_service.rs b/src/application/services/auth_application_service.rs index 9dace7c3..8aaf8687 100644 --- a/src/application/services/auth_application_service.rs +++ b/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::() }; diff --git a/src/application/services/batch_operations.rs b/src/application/services/batch_operations.rs index 901abb66..0d0d57c1 100644 --- a/src/application/services/batch_operations.rs +++ b/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(), ); diff --git a/src/application/services/batch_operations_test.rs b/src/application/services/batch_operations_test.rs index b0039e82..f37ca68c 100644 --- a/src/application/services/batch_operations_test.rs +++ b/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, diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index f6fbf1e9..6ba86128 100644 --- a/src/application/services/folder_service.rs +++ b/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, + path_resolver: Option>, } impl FolderService { - /// Creates a new folder service - pub fn new(folder_storage: Arc) -> 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, + path_resolver: Option>, + ) -> 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 { + Ok(EnsurePathOutcome::Created(FolderDto::empty())) + } + async fn get_folder(&self, _id: &str) -> Result { 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 { + 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 = 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` (`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); + } +} diff --git a/src/common/di.rs b/src/common/di.rs index 3334e63d..ba8e8d77 100644 --- a/src/common/di.rs +++ b/src/common/di.rs @@ -348,9 +348,12 @@ impl AppServiceFactory { repos: &RepositoryServices, trash_service: Option>, db_pool: &Arc, + path_resolver: Arc, ) -> 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 { diff --git a/src/common/stubs.rs b/src/common/stubs.rs index 156563a5..ddf8d5c7 100644 --- a/src/common/stubs.rs +++ b/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 { + Ok(EnsurePathOutcome::Created(FolderDto::default())) + } + async fn get_folder(&self, _id: &str) -> Result { Ok(FolderDto::default()) } diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 826c8664..56841137 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -999,66 +999,50 @@ async fn handle_mkcol( req: Request, path: String, ) -> Result, AppError> { + let user = extract_user(&req)?; let folder_service = &state.applications.folder_service; if path.is_empty() || path == "/" { - return Err(AppError::conflict("Root folder already exists")); + // Root collection always exists — RFC 4918 §9.3.1 → 405. + return Ok(Response::builder() + .status(StatusCode::METHOD_NOT_ALLOWED) + .body(Body::empty()) + .unwrap()); } - // Read request body - must be empty for MKCOL (RFC 4918 §9.3) + // RFC 4918 §9.3 / RFC 5689: a base-MKCOL request body is unsupported. let body_bytes = { let body = req.into_body(); body::to_bytes(body, MAX_MKCOL_BODY) .await .map_err(|e| AppError::payload_too_large(format!("MKCOL body too large: {}", e)))? }; - if !body_bytes.is_empty() { return Err(AppError::unsupported_media_type( "MKCOL request body must be empty", )); } - // Path is already translated by dispatch (e.g. "My Folder - jared/03/01"). - // Walk each segment: the first is the home folder (already exists), - // subsequent segments are created as needed with proper parent_id. - let segments: Vec<&str> = path.split('/').filter(|s| !s.is_empty()).collect(); - let mut parent_id: Option = None; - let mut accumulated_path = String::new(); - - for segment in &segments { - if !accumulated_path.is_empty() { - accumulated_path.push('/'); - } - accumulated_path.push_str(segment); - - match folder_service.get_folder_by_path(&accumulated_path).await { - Ok(existing) => { - parent_id = Some(existing.id); - } - Err(_) => { - let create_dto = crate::application::dtos::folder_dto::CreateFolderDto { - name: segment.to_string(), - parent_id: parent_id.clone(), - }; - let created = folder_service - .create_folder(create_dto) - .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to create folder '{}': {}", - accumulated_path, e - )) - })?; - parent_id = Some(created.id); - } + use crate::application::dtos::folder_dto::EnsurePathOutcome; + match folder_service.ensure_path(&path, user.id).await { + Ok(EnsurePathOutcome::Created(_)) => Ok(Response::builder() + .status(StatusCode::CREATED) + .body(Body::empty()) + .unwrap()), + Ok(EnsurePathOutcome::Existed(_)) => Ok(Response::builder() + .status(StatusCode::METHOD_NOT_ALLOWED) + .body(Body::empty()) + .unwrap()), + Err(e) if e.kind == crate::common::errors::ErrorKind::AlreadyExists => { + // A non-folder resource (e.g. a file) sits at the leaf path. + // RFC 4918 §9.3.1: MKCOL on any existing resource → 405. + Ok(Response::builder() + .status(StatusCode::METHOD_NOT_ALLOWED) + .body(Body::empty()) + .unwrap()) } + Err(e) => Err(e.into()), } - - Ok(Response::builder() - .status(StatusCode::CREATED) - .body(Body::empty()) - .unwrap()) } /** diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index c78e9b52..e035fe8d 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -617,78 +617,30 @@ async fn handle_mkcol( user: &CurrentUser, subpath: &str, ) -> Result, AppError> { - use crate::application::dtos::folder_dto::CreateFolderDto; + use crate::application::dtos::folder_dto::EnsurePathOutcome; let folder_service = &state.applications.folder_service; let internal_path = nc_to_internal_path(&user.username, subpath)?; - // If the folder already exists, return 405 per RFC 4918 §9.3.1 - if folder_service - .get_folder_by_path(&internal_path) - .await - .is_ok() - { - return Ok(Response::builder() + match folder_service.ensure_path(&internal_path, user.id).await { + Ok(EnsurePathOutcome::Created(_)) => Ok(Response::builder() + .status(StatusCode::CREATED) + .body(Body::empty()) + .unwrap()), + Ok(EnsurePathOutcome::Existed(_)) => Ok(Response::builder() .status(StatusCode::METHOD_NOT_ALLOWED) .body(Body::empty()) - .unwrap()); - } - - // Collect path segments that need to be created (walk from root to leaf) - let segments: Vec<&str> = subpath.split('/').filter(|s| !s.is_empty()).collect(); - - let user_root = nc_to_internal_path(&user.username, "")?; - let mut current_path = user_root.clone(); - let mut parent_id = folder_service - .get_folder_by_path(&user_root) - .await - .map_err(|_| AppError::not_found("User root folder not found"))? - .id - .clone(); - - for segment in &segments { - current_path = format!("{}/{}", current_path, segment); - match folder_service.get_folder_by_path(¤t_path).await { - Ok(existing) => { - parent_id = existing.id.clone(); - } - Err(_) => { - let dto = CreateFolderDto { - name: segment.to_string(), - parent_id: Some(parent_id.clone()), - }; - match folder_service.create_folder(dto).await { - Ok(created) => { - parent_id = created.id.clone(); - } - Err(e) - if e.message.contains("already exists") - || e.message.contains("Already Exists") => - { - // Race condition — folder created concurrently - let folder = folder_service - .get_folder_by_path(¤t_path) - .await - .map_err(|_| { - AppError::internal_error("Folder exists but cannot be found") - })?; - parent_id = folder.id.clone(); - } - Err(e) => { - return Err(AppError::internal_error(format!( - "Failed to create folder: {}", - e - ))); - } - } - } + .unwrap()), + Err(e) if e.kind == crate::common::errors::ErrorKind::AlreadyExists => { + // A non-folder resource (e.g. a file) sits at the leaf path. + // RFC 4918 §9.3.1: MKCOL on any existing resource → 405. + Ok(Response::builder() + .status(StatusCode::METHOD_NOT_ALLOWED) + .body(Body::empty()) + .unwrap()) } + Err(e) => Err(e.into()), } - - Ok(Response::builder() - .status(StatusCode::CREATED) - .body(Body::empty()) - .unwrap()) } // ──────────────────── DELETE ────────────────────