diff --git a/src/application/ports/file_ports.rs b/src/application/ports/file_ports.rs index d55ef437..43228a5d 100644 --- a/src/application/ports/file_ports.rs +++ b/src/application/ports/file_ports.rs @@ -245,7 +245,7 @@ pub trait FileRetrievalUseCase: Send + Sync + 'static { let all = self.list_files_batch(folder_id, offset, limit).await?; Ok(all .into_iter() - .filter(|f| f.owner_id.as_deref().map_or(false, |o| o == owner_id)) + .filter(|f| f.owner_id.as_deref().is_some_and(|o| o == owner_id)) .collect()) } } diff --git a/src/application/ports/storage_ports.rs b/src/application/ports/storage_ports.rs index c375ef0d..2b6a0332 100644 --- a/src/application/ports/storage_ports.rs +++ b/src/application/ports/storage_ports.rs @@ -58,7 +58,7 @@ pub trait FileReadPort: Send + Sync + 'static { let all = self.list_files(folder_id).await?; Ok(all .into_iter() - .filter(|f| f.owner_id().map_or(false, |o| o == owner_id)) + .filter(|f| f.owner_id().is_some_and(|o| o == owner_id)) .collect()) } @@ -142,7 +142,7 @@ pub trait FileReadPort: Send + Sync + 'static { let all = self.list_files_batch(folder_id, offset, limit).await?; Ok(all .into_iter() - .filter(|f| f.owner_id().map_or(false, |o| o == owner_id)) + .filter(|f| f.owner_id().is_some_and(|o| o == owner_id)) .collect()) } diff --git a/src/application/services/batch_operations.rs b/src/application/services/batch_operations.rs index 9c1a5430..ea8efdcf 100644 --- a/src/application/services/batch_operations.rs +++ b/src/application/services/batch_operations.rs @@ -928,11 +928,9 @@ impl BatchOperationService { async move { // If a parent is specified, verify the caller owns it - if let Some(ref pid) = parent_id { - if let Err(e) = folder_service.get_folder_owned(pid, &caller).await { - let id = format!("{}:{}", name, pid); - return (id, Err(e.into())); - } + if let Some(ref pid) = parent_id && let Err(e) = folder_service.get_folder_owned(pid, &caller).await { + let id = format!("{}:{}", name, pid); + return (id, Err(e)); } let dto = crate::application::dtos::folder_dto::CreateFolderDto { name: name.clone(), diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index fd39555b..e26d5ffb 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -553,10 +553,10 @@ mod tests { Ok(ShareDto::from_entity(&saved_share, &self.config.base_url())) } - async fn get_shared_link(&self, id: &str) -> Result { + async fn get_shared_link(&self, id: &str, requester_id: &str) -> Result { let share = self .share_repository - .find_share_by_id(id) + .find_share_by_id_for_user(id, requester_id) .await .map_err(|e| { ShareServiceError::NotFound(format!("Share {} not found: {}", id, e)) @@ -585,10 +585,11 @@ mod tests { &self, item_id: &str, item_type: &ShareItemType, + requester_id: &str, ) -> Result, DomainError> { let shares = self .share_repository - .find_shares_by_item(item_id, item_type) + .find_shares_by_item_for_user(item_id, item_type, requester_id) .await .map_err(|e| ShareServiceError::Repository(e.to_string()))?; Ok(shares @@ -601,11 +602,12 @@ mod tests { async fn update_shared_link( &self, id: &str, + requester_id: &str, dto: UpdateShareDto, ) -> Result { let mut share = self .share_repository - .find_share_by_id(id) + .find_share_by_id_for_user(id, requester_id) .await .map_err(|e| { ShareServiceError::NotFound(format!("Share {} not found: {}", id, e)) @@ -632,9 +634,9 @@ mod tests { Ok(ShareDto::from_entity(&updated, &self.config.base_url())) } - async fn delete_shared_link(&self, id: &str) -> Result<(), DomainError> { + async fn delete_shared_link(&self, id: &str, requester_id: &str) -> Result<(), DomainError> { self.share_repository - .delete_share(id) + .delete_share_for_user(id, requester_id) .await .map_err(|e| ShareServiceError::Repository(e.to_string()))?; Ok(()) @@ -663,7 +665,7 @@ mod tests { &self, token: &str, password: &str, - ) -> Result { + ) -> Result { let share = self .share_repository .find_share_by_token(token) @@ -675,8 +677,18 @@ mod tests { return Err(ShareServiceError::Expired.into()); } match share.password_hash() { - Some(hash) => self.password_hasher.verify_password(password, hash).await, - None => Ok(true), + Some(hash) => { + let valid = self.password_hasher.verify_password(password, hash).await?; + if !valid { + return Err(DomainError::new( + crate::common::errors::ErrorKind::AccessDenied, + "Share", + "Invalid share password", + )); + } + Ok(ShareDto::from_entity(&share, &self.config.base_url())) + } + None => Ok(ShareDto::from_entity(&share, &self.config.base_url())), } } diff --git a/src/application/services/trash_service.rs b/src/application/services/trash_service.rs index 9c08d320..90699e09 100644 --- a/src/application/services/trash_service.rs +++ b/src/application/services/trash_service.rs @@ -272,7 +272,7 @@ impl TrashUseCase for TrashService { // Ownership check — return NotFound (not Forbidden) to // prevent leaking whether the folder exists. - if folder.owner_id().map_or(true, |o| o != user_id) { + if folder.owner_id().is_none_or(|o| o != user_id) { return Err(DomainError::not_found( "Folder", format!("Folder not found: {}", item_id), diff --git a/src/application/services/trash_service_test.rs b/src/application/services/trash_service_test.rs index 0470e688..cd20fded 100644 --- a/src/application/services/trash_service_test.rs +++ b/src/application/services/trash_service_test.rs @@ -20,6 +20,7 @@ use crate::domain::services::path_service::StoragePath; /// Test-only service that mirrors `TrashService` logic but accepts generic repos, /// allowing mock repositories to be injected in unit tests. +#[allow(dead_code)] struct TrashServiceForTest { trash_repository: Arc, file_read_port: Arc, @@ -35,6 +36,7 @@ where FW: FileWritePort, FoR: FolderRepository, { + #[allow(dead_code)] fn new( trash_repository: Arc, file_read_port: Arc, @@ -308,6 +310,7 @@ where } // Mock repositories for testing +#[allow(dead_code)] struct MockTrashRepository { trash_items: Mutex>, /// Shared refs to the file/folder trashed maps so `clear_trash` can @@ -317,6 +320,7 @@ struct MockTrashRepository { } impl MockTrashRepository { + #[allow(dead_code)] fn new( trashed_files: Arc>>, trashed_folders: Arc>>, @@ -394,12 +398,14 @@ impl TrashRepository for MockTrashRepository { } } +#[allow(dead_code)] struct MockFileRepository { files: Mutex>, trashed_files: Arc>>, } impl MockFileRepository { + #[allow(dead_code)] fn new(trashed_files: Arc>>) -> Self { Self { files: Mutex::new(HashMap::new()), @@ -407,6 +413,7 @@ impl MockFileRepository { } } + #[allow(dead_code)] fn add_test_file(&self, id: &str, name: &str, path: &str) { let file = File::new( id.to_string(), @@ -625,12 +632,14 @@ impl FileWritePort for MockFileRepository { } } +#[allow(dead_code)] struct MockFolderRepository { folders: Mutex>, trashed_folders: Arc>>, } impl MockFolderRepository { + #[allow(dead_code)] fn new(trashed_folders: Arc>>) -> Self { Self { folders: Mutex::new(HashMap::new()), @@ -638,6 +647,7 @@ impl MockFolderRepository { } } + #[allow(dead_code)] fn add_test_folder(&self, id: &str, name: &str, path: &str) { let folder = Folder::new( id.to_string(), diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 9f5855e6..2cf0d0eb 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -434,6 +434,7 @@ async fn handle_propfind( /// (sub-folders and files) are fetched in batches of `PROPFIND_BATCH_SIZE`. /// Each batch is serialised to XML and sent as a chunk, so memory stays /// constant at O(batch_size) regardless of the total number of children. +#[allow(clippy::too_many_arguments)] async fn build_streaming_propfind_response( folder: FolderDto, folder_id: Option, @@ -1227,10 +1228,8 @@ async fn handle_move( if source_parent_path != dest_parent_path { // SECURITY: verify destination parent belongs to caller (V-08) - if !dest_parent_path.is_empty() { - if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { - assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?; - } + if !dest_parent_path.is_empty() && let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { + assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?; } file_management_service .move_file(&file.id, Some(dest_parent_path.to_string())) @@ -1328,10 +1327,8 @@ async fn handle_move( if source_parent_path != dest_parent_path { // SECURITY: verify destination parent belongs to caller (V-08) - if !dest_parent_path.is_empty() { - if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { - assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?; - } + if !dest_parent_path.is_empty() && let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { + assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?; } file_management_service .move_file(&file.id, Some(dest_parent_path.to_string()))