From 85c8c086ac8f9ff8d92e01a719d835435dcbc561 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 11 May 2026 22:26:53 +0200 Subject: [PATCH 1/6] fix(ui): fix go to parent + refactor types --- static/js/app/ui.js | 31 ++----------- static/js/features/files/contextMenus.js | 54 ++++++++++++---------- static/js/features/files/fileOperations.js | 10 ++-- 3 files changed, 39 insertions(+), 56 deletions(-) diff --git a/static/js/app/ui.js b/static/js/app/ui.js index 6ac5201d..52bf89f9 100644 --- a/static/js/app/ui.js +++ b/static/js/app/ui.js @@ -997,18 +997,8 @@ const ui = { setContextTarget(card, info); const menuId = info.type === 'folder' ? 'folder-context-menu' : 'file-context-menu'; const menu = document.getElementById(menuId); - if (contextMenus && typeof contextMenus.syncFavoriteOptionLabels === 'function') { - contextMenus.syncFavoriteOptionLabels(); - } - if (contextMenus && typeof contextMenus.syncWopiOptionVisibility === 'function') { - contextMenus.syncWopiOptionVisibility().catch(() => {}); - } - if (contextMenus && typeof contextMenus.syncAddToPlaylistOption === 'function') { - contextMenus.syncAddToPlaylistOption(); - } - if (contextMenus && typeof contextMenus.syncOpenParentFolderOption === 'function') { - contextMenus.syncOpenParentFolderOption(); - } + contextMenus.sync(); + if (menu) { menu.style.left = `${e.pageX}px`; menu.style.top = `${e.pageY}px`; @@ -1201,9 +1191,7 @@ const ui = { } // Keep context-menu label in sync if available - if (contextMenus && typeof contextMenus.syncFavoriteOptionLabels === 'function') { - contextMenus.syncFavoriteOptionLabels(); - } + contextMenus.syncFavoriteOptionLabels(); }); const shared = el.querySelector('.file-badge-shared'); @@ -1547,18 +1535,7 @@ function showContextMenuAtElement(triggerElement, menuId) { top = rect.top - 4 + window.scrollY; // flip above if no room } - if (contextMenus && typeof contextMenus.syncFavoriteOptionLabels === 'function') { - contextMenus.syncFavoriteOptionLabels(); - } - if (contextMenus && typeof contextMenus.syncWopiOptionVisibility === 'function') { - contextMenus.syncWopiOptionVisibility().catch(() => {}); - } - if (contextMenus && typeof contextMenus.syncAddToPlaylistOption === 'function') { - contextMenus.syncAddToPlaylistOption(); - } - if (contextMenus && typeof contextMenus.syncOpenParentFolderOption === 'function') { - contextMenus.syncOpenParentFolderOption(); - } + contextMenus.sync(); menu.style.left = `${left}px`; menu.style.top = `${top}px`; diff --git a/static/js/features/files/contextMenus.js b/static/js/features/files/contextMenus.js index c25145b5..0f02569f 100644 --- a/static/js/features/files/contextMenus.js +++ b/static/js/features/files/contextMenus.js @@ -70,8 +70,8 @@ const contextMenus = { const option = document.getElementById('open-parent-folder-option'); if (!option) return; const folderId = app?.contextMenuTargetFile?.folder_id; - const alreadyViewing = folderId && folderId === app?.currentPath; - option.classList.toggle('hidden', !folderId || alreadyViewing); + const isFilesSection = app.currentSection === 'files'; + option.classList.toggle('hidden', !folderId || isFilesSection); }, syncAddToPlaylistOption() { @@ -87,6 +87,12 @@ const contextMenus = { } }, + sync() { + this.syncFavoriteOptionLabels(); + this.syncWopiOptionVisibility().catch(() => {}); + this.syncAddToPlaylistOption(); + this.syncOpenParentFolderOption(); + }, /** * Assign events to menu items and dialogs */ @@ -688,8 +694,8 @@ const contextMenus = { /** * Load all folders for the move dialog (batch operations) * Uses the same navigation pattern as loadMoveDialogFolders - * @param {string} itemId - ID of the item being moved (unused, kept for compatibility) - * @param {string} mode - 'batch' for batch operations + * @param {string} _itemId - ID of the item being moved (unused, kept for compatibility) + * @param {string} _mode - 'batch' for batch operations */ async loadAllFolders(_itemId, _mode) { // For batch mode, use the same navigation as regular move dialog @@ -731,13 +737,13 @@ const contextMenus = { if (itemName) itemName.textContent = item.name; // Reset form - const pwField = document.getElementById('share-password'); - const expField = document.getElementById('share-expiration'); + const pwField = /** @type HTMLInputElement */ (document.getElementById('share-password')); + const expField = /** @type HTMLInputElement */ (document.getElementById('share-expiration')); if (pwField) pwField.value = ''; if (expField) expField.value = ''; - const permRead = document.getElementById('share-permission-read'); - const permWrite = document.getElementById('share-permission-write'); - const permReshare = document.getElementById('share-permission-reshare'); + const permRead = /** @type HTMLInputElement */ (document.getElementById('share-permission-read')); + const permWrite = /** @type HTMLInputElement */ (document.getElementById('share-permission-write')); + const permReshare = /** @type HTMLInputElement */ (document.getElementById('share-permission-reshare')); if (permRead) permRead.checked = true; if (permWrite) permWrite.checked = false; if (permReshare) permReshare.checked = false; @@ -862,11 +868,11 @@ const contextMenus = { } // Get values from form - const password = document.getElementById('share-password').value; - const expirationDate = document.getElementById('share-expiration').value; - const permissionRead = document.getElementById('share-permission-read').checked; - const permissionWrite = document.getElementById('share-permission-write').checked; - const permissionReshare = document.getElementById('share-permission-reshare').checked; + const password = /** @type HTMLInputElement */ (document.getElementById('share-password')).value; + const expirationDate = /** @type HTMLInputElement */ (document.getElementById('share-expiration')).value; + const permissionRead = /** @type HTMLInputElement */ (document.getElementById('share-permission-read')).checked; + const permissionWrite = /** @type HTMLInputElement */ (document.getElementById('share-permission-write')).checked; + const permissionReshare = /** @type HTMLInputElement */ (document.getElementById('share-permission-reshare')).checked; const item = app.shareDialogItem; const itemType = app.shareDialogItemType; @@ -905,7 +911,7 @@ const contextMenus = { const shareInfo = await response.json(); // Update UI with new share - const shareUrl = document.getElementById('generated-share-url'); + const shareUrl = /** @type HTMLInputElement */ (document.getElementById('generated-share-url')); if (shareUrl) { shareUrl.value = shareInfo.url; document.getElementById('new-share-section').classList.remove('hidden'); @@ -920,7 +926,7 @@ const contextMenus = { ui.showNotification(i18n.t('notifications.link_created'), i18n.t('notifications.share_success')); } catch (error) { console.error('Error creating shared link:', error); - ui.showNotification('Error', error.message || 'Could not create shared link'); + ui.showNotification('Error', /** @type {Error} */ (error).message || 'Could not create shared link'); } }, @@ -931,8 +937,8 @@ const contextMenus = { showEmailNotificationDialog(shareUrl) { // Update dialog content document.getElementById('notification-share-url').textContent = shareUrl; - document.getElementById('notification-email').value = ''; - document.getElementById('notification-message').value = ''; + /** @type HTMLInputElement */ (document.getElementById('notification-email')).value = ''; + /** @type HTMLInputElement */ (document.getElementById('notification-message')).value = ''; // Store the URL for later use app.notificationShareUrl = shareUrl; @@ -945,8 +951,8 @@ const contextMenus = { * Send share notification email */ sendShareNotification() { - const email = document.getElementById('notification-email').value.trim(); - const message = document.getElementById('notification-message').value.trim(); + const email = /** @type HTMLInputElement */ (document.getElementById('notification-email')).value.trim(); + const message = /** @type HTMLInputElement */ (document.getElementById('notification-message')).value.trim(); const shareUrl = app.notificationShareUrl; if (!email || !shareUrl) { @@ -1013,7 +1019,7 @@ const contextMenus = { container.innerHTML = '
'; // Reset add button state - const addBtn = document.getElementById('playlist-add-btn'); + const addBtn = /** @type {HTMLButtonElement} */ (document.getElementById('playlist-add-btn')); if (addBtn) addBtn.disabled = true; // Show dialog @@ -1057,7 +1063,7 @@ const contextMenus = { }); item.classList.add('selected'); this._selectedPlaylistId = playlist.id; - const addBtn = document.getElementById('playlist-add-btn'); + const addBtn = /** @type {HTMLButtonElement} */ (document.getElementById('playlist-add-btn')); if (addBtn) addBtn.disabled = false; }); @@ -1071,7 +1077,7 @@ const contextMenus = { if (!playlistId || files.length === 0) return; - const addBtn = document.getElementById('playlist-add-btn'); + const addBtn = /** @type {HTMLButtonElement} */ (document.getElementById('playlist-add-btn')); if (addBtn) addBtn.disabled = true; try { @@ -1101,7 +1107,7 @@ const contextMenus = { } } catch (err) { console.error('Error adding to playlist:', err); - ui.showNotification(i18n.t('music.error'), err.message || i18n.t('music.add_error')); + ui.showNotification(i18n.t('music.error'), /** @type {Error} */ (err).message || i18n.t('music.add_error')); if (addBtn) addBtn.disabled = false; } }, diff --git a/static/js/features/files/fileOperations.js b/static/js/features/files/fileOperations.js index 3afcaf69..78af5c10 100644 --- a/static/js/features/files/fileOperations.js +++ b/static/js/features/files/fileOperations.js @@ -208,7 +208,7 @@ const fileOps = { safeUpdateFile(0, 'error'); finalize({ ok: false, - errorMsg: `Client send() failed: ${e?.message || 'unknown error'}` + errorMsg: `Client send() failed: ${/** @type {Error} */ (e)?.message || 'unknown error'}` }); } }); @@ -286,7 +286,7 @@ const fileOps = { try { // Legacy progress bar (inside dropzone) — keep working for drag-drop - const progressBar = document.querySelector('.progress-fill'); + const progressBar = /** @type {HTMLDivElement} */ (document.querySelector('.progress-fill')); const uploadProgressDiv = document.querySelector('.upload-progress'); if (uploadProgressDiv) { uploadProgressDiv.classList.remove('hidden'); @@ -447,7 +447,7 @@ const fileOps = { } this._isUploading = true; - const progressBar = document.querySelector('.progress-fill'); + const progressBar = /** @type {HTMLDivElement} */ (document.querySelector('.progress-fill')); const uploadProgressDiv = document.querySelector('.upload-progress'); if (uploadProgressDiv) { uploadProgressDiv.classList.remove('hidden'); @@ -929,8 +929,8 @@ const fileOps = { /** * Copy a folder to another folder * Note: Backend folder copy is not yet implemented, this shows a notification - * @param {string} folderId - Folder ID - * @param {string} targetFolderId - Target folder ID + * @param {string} _folderId - Folder ID + * @param {string} _targetFolderId - Target folder ID * @returns {Promise} - Success status */ async copyFolder(_folderId, _targetFolderId) { From 2f49daa4eeaad127888173e68593b46b26e7d2fe Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 11 May 2026 22:58:33 +0200 Subject: [PATCH 2/6] feat(folders): implement copy_folders taking care of ownership --- src/application/ports/file_ports.rs | 10 +++ src/application/services/batch_operations.rs | 68 +++++++++++++++ .../services/file_management_service.rs | 27 ++++++ src/common/stubs.rs | 13 +++ src/interfaces/api/handlers/batch_handler.rs | 85 +++++++++++++++++++ src/interfaces/api/routes.rs | 1 + static/js/features/files/fileOperations.js | 41 ++++++--- 7 files changed, 233 insertions(+), 12 deletions(-) diff --git a/src/application/ports/file_ports.rs b/src/application/ports/file_ports.rs index 08c97189..317b84fc 100644 --- a/src/application/ports/file_ports.rs +++ b/src/application/ports/file_ports.rs @@ -334,6 +334,16 @@ pub trait FileManagementUseCase: Send + Sync + 'static { "copy_folder_tree not implemented", )) } + + /// Copies a folder tree, enforcing that `caller_id` owns both the source folder + /// and the target parent folder. + async fn copy_folder_tree_owned( + &self, + source_folder_id: &str, + caller_id: Uuid, + target_parent_id: Option, + dest_name: Option, + ) -> Result; } /// Factory for creating file use case implementations diff --git a/src/application/services/batch_operations.rs b/src/application/services/batch_operations.rs index 901abb66..1fb58a3b 100644 --- a/src/application/services/batch_operations.rs +++ b/src/application/services/batch_operations.rs @@ -12,6 +12,7 @@ use tracing::info; use crate::application::dtos::file_dto::FileDto; use crate::application::dtos::folder_dto::{FolderDto, MoveFolderDto}; use crate::application::ports::file_ports::{FileManagementUseCase, FileRetrievalUseCase}; +use crate::application::ports::storage_ports::CopyFolderTreeResult; use crate::application::ports::inbound::FolderUseCase; use crate::application::ports::trash_ports::TrashUseCase; use crate::application::services::file_management_service::FileManagementService; @@ -616,6 +617,73 @@ impl BatchOperationService { Ok(result) } + /// Copies multiple folder trees to a target parent in parallel + pub async fn copy_folders( + &self, + folder_ids: Vec, + target_folder_id: Option, + user_id: Uuid, + ) -> Result, BatchOperationError> { + info!("Starting batch copy of {} folders", folder_ids.len()); + let start_time = std::time::Instant::now(); + + let mut result = BatchResult { + successful: Vec::new(), + failed: Vec::new(), + stats: BatchStats { + total: folder_ids.len(), + ..Default::default() + }, + }; + + let target: Option> = target_folder_id.map(|s| Arc::from(s.as_str())); + + let mut operation_stream = stream::iter(folder_ids.into_iter().map(|folder_id| { + let file_management = self.file_management.clone(); + let target = target.clone(); + + async move { + let copy_result = file_management + .copy_folder_tree_owned( + &folder_id, + user_id, + target.map(|s| s.to_string()), + None, + ) + .await; + (folder_id, copy_result) + } + })) + .buffer_unordered(self.config.concurrency.max_concurrent_files); + + while let Some((folder_id, operation_result)) = operation_stream.next().await { + match operation_result { + Ok(copy_result) => { + result.successful.push(copy_result); + result.stats.successful += 1; + } + Err(e) => { + result.failed.push((folder_id, e.to_string())); + result.stats.failed += 1; + } + } + } + + result.stats.execution_time_ms = start_time.elapsed().as_millis(); + result.stats.max_concurrency = self + .config + .concurrency + .max_concurrent_files + .min(result.stats.total); + + info!( + "Batch folder copy completed: {}/{} successful in {}ms", + result.stats.successful, result.stats.total, result.stats.execution_time_ms + ); + + Ok(result) + } + /// Downloads multiple files/folders as a single ZIP archive. /// /// Writes the archive to a temporary file so RAM usage is O(buffer_size) diff --git a/src/application/services/file_management_service.rs b/src/application/services/file_management_service.rs index 1c4bf638..c5ee2ba8 100644 --- a/src/application/services/file_management_service.rs +++ b/src/application/services/file_management_service.rs @@ -326,4 +326,31 @@ impl FileManagementUseCase for FileManagementService { Ok(result) } + + async fn copy_folder_tree_owned( + &self, + source_folder_id: &str, + caller_id: Uuid, + target_parent_id: Option, + dest_name: Option, + ) -> Result { + if let Some(folder_repo) = &self.folder_repo { + let owner = folder_repo.get_folder_user_id(source_folder_id).await?; + if owner != caller_id { + return Err(DomainError::not_found( + "Folder", + "Source folder not found or access denied", + )); + } + } else { + return Err(DomainError::internal_error( + "FileManagement", + "Folder ownership verification unavailable", + )); + } + self.verify_target_folder_owner(&target_parent_id, caller_id) + .await?; + self.copy_folder_tree(source_folder_id, target_parent_id, dest_name) + .await + } } diff --git a/src/common/stubs.rs b/src/common/stubs.rs index 156563a5..01328051 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -671,6 +671,19 @@ impl FileManagementUseCase for StubFileManagementUseCase { ) -> Result { Ok(FileDto::default()) } + + async fn copy_folder_tree_owned( + &self, + _source_folder_id: &str, + _caller_id: Uuid, + _target_parent_id: Option, + _dest_name: Option, + ) -> Result { + Err(DomainError::internal_error( + "StubFileManagement", + "copy_folder_tree_owned not implemented", + )) + } } // --------------------------------------------------------------------------- diff --git a/src/interfaces/api/handlers/batch_handler.rs b/src/interfaces/api/handlers/batch_handler.rs index 0249a3e8..8b650f0e 100644 --- a/src/interfaces/api/handlers/batch_handler.rs +++ b/src/interfaces/api/handlers/batch_handler.rs @@ -9,6 +9,7 @@ use utoipa::ToSchema; use crate::application::dtos::file_dto::FileDto; use crate::application::dtos::folder_dto::FolderDto; +use crate::application::ports::storage_ports::CopyFolderTreeResult; use crate::application::services::batch_operations::{ BatchOperationService, BatchResult, BatchStats, }; @@ -851,6 +852,90 @@ pub async fn move_folders_batch( Ok((status_code, Json(response)).into_response()) } +/// DTO returned for each successfully copied folder tree +#[derive(Debug, Serialize, ToSchema)] +pub struct CopiedFolderDto { + /// UUID of the newly created root folder + pub new_root_folder_id: String, + /// Total folders created (including root) + pub folders_copied: i64, + /// Total files copied (zero-copy via dedup) + pub files_copied: i64, +} + +impl From for CopiedFolderDto { + fn from(r: CopyFolderTreeResult) -> Self { + Self { + new_root_folder_id: r.new_root_folder_id, + folders_copied: r.folders_copied, + files_copied: r.files_copied, + } + } +} + +/// Handler for copying multiple folder trees in batch +#[utoipa::path( + post, + path = "/api/batch/folders/copy", + responses( + (status = 200, description = "All folders copied"), + (status = 206, description = "Partial success"), + (status = 400, description = "Bad request"), + (status = 401, description = "Unauthorized") + ), + tag = "batch" +)] +pub async fn copy_folders_batch( + State(state): State, + auth_user: AuthUser, + Json(request): Json, +) -> ApiResult { + if request.folder_ids.is_empty() { + return Ok(( + StatusCode::BAD_REQUEST, + Json(serde_json::json!({ + "error": "No folder IDs provided" + })), + ) + .into_response()); + } + if request.folder_ids.len() > MAX_BATCH_SIZE { + return Ok(( + StatusCode::BAD_REQUEST, + Json(serde_json::json!({ + "error": format!("Batch size {} exceeds maximum of {}", request.folder_ids.len(), MAX_BATCH_SIZE) + })), + ) + .into_response()); + } + + let result = state + .batch_service + .copy_folders(request.folder_ids, request.target_folder_id, auth_user.id) + .await + .map_err(|e| { + tracing::error!("Batch copy_folders failed: {}", e); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Batch operation failed".to_string(), + ) + })?; + + let response: BatchOperationResponse = result.into(); + + let status_code = if response.stats.failed > 0 { + if response.stats.successful > 0 { + StatusCode::PARTIAL_CONTENT + } else { + StatusCode::BAD_REQUEST + } + } else { + StatusCode::OK + }; + + Ok((status_code, Json(response)).into_response()) +} + // Hander as a workarround for drag & drop (does not support POST requests) #[utoipa::path( get, diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 17de42e3..bdd28464 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -237,6 +237,7 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { .route("/folders/delete", post(batch_handler::delete_folders_batch)) .route("/folders/create", post(batch_handler::create_folders_batch)) .route("/folders/get", post(batch_handler::get_folders_batch)) + .route("/folders/copy", post(batch_handler::copy_folders_batch)) .route("/folders/move", post(batch_handler::move_folders_batch)) // Trash operations (soft delete) .route("/trash", post(batch_handler::trash_batch)) diff --git a/static/js/features/files/fileOperations.js b/static/js/features/files/fileOperations.js index 78af5c10..348936b4 100644 --- a/static/js/features/files/fileOperations.js +++ b/static/js/features/files/fileOperations.js @@ -928,23 +928,31 @@ const fileOps = { /** * Copy a folder to another folder - * Note: Backend folder copy is not yet implemented, this shows a notification - * @param {string} _folderId - Folder ID - * @param {string} _targetFolderId - Target folder ID + * @param {string} folderId - Folder ID + * @param {string} targetFolderId - Target folder ID * @returns {Promise} - Success status */ - async copyFolder(_folderId, _targetFolderId) { - // Folder copy is not yet implemented in the backend - ui.showNotification('Not implemented', 'Folder copy is not yet supported'); - return false; + async copyFolder(folderId, targetFolderId) { + const res = await fetch('/api/batch/folders/copy', { + method: 'POST', + headers: { ...getAuthHeaders(), 'Content-Type': 'application/json' }, + body: JSON.stringify({ folder_ids: [folderId], target_folder_id: targetFolderId }) + }); + return res.ok; }, + /** + * @typedef {Object} BatchCopyReturn + * @property {number} success + * @property {number} errors + */ + /** * Copy files & folders * @param {string[]} fileIds - File IDs * @param {string[]} folderIds - Folder IDs * @param {string} targetFolderId - Target folder ID - * @returns {Promise} - Success status + * @returns {Promise} - Success status */ async batchCopy(fileIds, folderIds, targetFolderId) { // FIXME ensure not moving a folder into itself @@ -964,13 +972,22 @@ const fileOps = { }); const data = await res.json(); success += data.stats?.successful || 0; - errors += data.stats?.failed || 0; + errors += data.stats?.failed || (!res.ok && !data.stats ? fileIds.length : 0); } - // Note: Folder copy is not yet implemented in batch API + // Batch copy folders if (folderIds.length > 0) { - ui.showNotification('Info', 'Folder copy is not yet supported in batch mode'); - errors += folderIds.lenngth; + const res = await fetch('/api/batch/folders/copy', { + method: 'POST', + headers: { ...getAuthHeaders(), 'Content-Type': 'application/json' }, + body: JSON.stringify({ + folder_ids: folderIds, + target_folder_id: targetFolderId + }) + }); + const data = await res.json(); + success += data.stats?.successful || 0; + errors += data.stats?.failed || (!res.ok && !data.stats ? folderIds.length : 0); } } catch (err) { console.error('Batch copy error:', err); From e9532365896139401f551494a57e0c69c5202ec9 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Tue, 12 May 2026 00:23:50 +0200 Subject: [PATCH 3/6] test(api): add functional test on copy_folder --- tests/api/batch_folder_copy.hurl | 188 +++++++++++++++++++++++++++++++ tests/api/files-folders.hurl | 2 +- tests/api/run.sh | 1 + tests/fixtures/hello.txt | 2 +- 4 files changed, 191 insertions(+), 2 deletions(-) create mode 100644 tests/api/batch_folder_copy.hurl diff --git a/tests/api/batch_folder_copy.hurl b/tests/api/batch_folder_copy.hurl new file mode 100644 index 00000000..34b5c8b1 --- /dev/null +++ b/tests/api/batch_folder_copy.hurl @@ -0,0 +1,188 @@ +# ============================================================= +# OxiCloud – Batch folder copy end-to-end scenario +# ============================================================= +# Verifies that copying a folder (containing one file) into a +# target folder produces an independent copy: the copied folder +# appears inside the target, and its child file is present. +# +# Run standalone: +# hurl --variables-file tests/api/test.env --test \ +# tests/api/batch_folder_copy.hurl +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 – Login +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ + "username": "{{username}}", + "password": "{{password}}" +} + +HTTP 200 +[Captures] +token: jsonpath "$.access_token" +[Asserts] +jsonpath "$.access_token" isString + + +# ───────────────────────────────────────────────────────────── +# Step 2 – Create source folder +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/folders +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "name": "hurl-copy-source" +} + +HTTP 201 +[Captures] +source_folder_id: jsonpath "$.id" +[Asserts] +jsonpath "$.name" == "hurl-copy-source" + + +# ───────────────────────────────────────────────────────────── +# Step 3 – Upload one file into the source folder +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{token}} +[MultipartFormData] +folder_id: {{source_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +source_file_id: jsonpath "$.id" +source_file_name: jsonpath "$.name" +[Asserts] +jsonpath "$.folder_id" == "{{source_folder_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 4 – Create target folder (copy destination) +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/folders +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "name": "hurl-copy-target" +} + +HTTP 201 +[Captures] +target_folder_id: jsonpath "$.id" +[Asserts] +jsonpath "$.name" == "hurl-copy-target" + + +# ───────────────────────────────────────────────────────────── +# Step 5 – Copy source folder into target folder +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/batch/folders/copy +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "folder_ids": ["{{source_folder_id}}"], + "target_folder_id": "{{target_folder_id}}" +} + +HTTP 200 +[Captures] +new_root_folder_id: jsonpath "$.successful[0].new_root_folder_id" +[Asserts] +jsonpath "$.stats.successful" == 1 +jsonpath "$.stats.failed" == 0 +jsonpath "$.successful[0].new_root_folder_id" isString +jsonpath "$.successful[0].folders_copied" >= 1 +jsonpath "$.successful[0].files_copied" == 1 + + +# ───────────────────────────────────────────────────────────── +# Step 6 – Verify the copied folder is inside the target folder +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/folders/{{target_folder_id}}/listing +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +jsonpath "$.folders" count == 1 +jsonpath "$.folders[0].id" == "{{new_root_folder_id}}" +jsonpath "$.folders[0].name" == "hurl-copy-source" +jsonpath "$.files" count == 0 + + +# ───────────────────────────────────────────────────────────── +# Step 7 – Verify the copied file is inside the copied folder +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/files?folder_id={{new_root_folder_id}} +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +jsonpath "$" count == 1 +jsonpath "$[0].name" == "{{source_file_name}}" +jsonpath "$[0].id" != "{{source_file_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 8 – Verify the original source folder is unchanged +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/files?folder_id={{source_folder_id}} +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +jsonpath "$" count == 1 +jsonpath "$[0].id" == "{{source_file_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 9 – Cleanup: delete the copied folder tree +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/folders/{{new_root_folder_id}} +Authorization: Bearer {{token}} + +HTTP 204 + + +# ───────────────────────────────────────────────────────────── +# Step 10 – Verify source folder and file are still intact +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/folders/{{source_folder_id}} +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +jsonpath "$.id" == "{{source_folder_id}}" +jsonpath "$.name" == "hurl-copy-source" + +GET {{base_url}}/api/files?folder_id={{source_folder_id}} +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +jsonpath "$" count == 1 +jsonpath "$[0].id" == "{{source_file_id}}" + + +# ───────────────────────────────────────────────────────────── +# Step 11 – Cleanup: delete source file and folders +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/api/files/{{source_file_id}} +Authorization: Bearer {{token}} + +HTTP 204 + +DELETE {{base_url}}/api/folders/{{source_folder_id}} +Authorization: Bearer {{token}} + +HTTP 204 + +DELETE {{base_url}}/api/folders/{{target_folder_id}} +Authorization: Bearer {{token}} + +HTTP 204 diff --git a/tests/api/files-folders.hurl b/tests/api/files-folders.hurl index ab9a4fa1..c9bab8c8 100644 --- a/tests/api/files-folders.hurl +++ b/tests/api/files-folders.hurl @@ -167,7 +167,7 @@ file_id: jsonpath "$.id" jsonpath "$.id" isString jsonpath "$.name" == "hello.txt" jsonpath "$.folder_id" == {{test2_id}} -jsonpath "$.size" == 21 +jsonpath "$.size" == 32 jsonpath "$.mime_type" == "text/plain" diff --git a/tests/api/run.sh b/tests/api/run.sh index e5485858..400d2567 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -91,6 +91,7 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/favorites.hurl" \ "$API_DIR/trash.hurl" \ "$API_DIR/recent.hurl" \ + "$API_DIR/batch_folder_copy.hurl" \ "$API_DIR/contacts.hurl" log "All tests passed." diff --git a/tests/fixtures/hello.txt b/tests/fixtures/hello.txt index 82af399e..d95f2d01 100644 --- a/tests/fixtures/hello.txt +++ b/tests/fixtures/hello.txt @@ -1 +1 @@ -hello from hurl test +Hello from OxiCloud Hurl tests. From 3a1850715894011dc78bee38b825164b0fdd5d15 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 11 May 2026 23:00:55 +0200 Subject: [PATCH 4/6] fix/security: copy_file_owned(): ensure that target is also owned by the user --- src/application/services/file_management_service.rs | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/application/services/file_management_service.rs b/src/application/services/file_management_service.rs index c5ee2ba8..eb7a3d37 100644 --- a/src/application/services/file_management_service.rs +++ b/src/application/services/file_management_service.rs @@ -186,6 +186,8 @@ impl FileManagementUseCase for FileManagementService { target_folder_id: Option, ) -> Result { self.verify_owner(file_id, caller_id).await?; + self.verify_target_folder_owner(&target_folder_id, caller_id) + .await?; self.copy_file(file_id, target_folder_id).await } From b4d4056a153a7c3702ff85d7fa74437a1e139ee8 Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Mon, 11 May 2026 23:12:30 +0200 Subject: [PATCH 5/6] fix/security: move_folder(): check that target belongs to caller --- src/application/services/folder_service.rs | 37 +++++++++------------- 1 file changed, 15 insertions(+), 22 deletions(-) diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index dd84e52f..fab1141a 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -404,12 +404,7 @@ impl FolderUseCase for FolderService { } // Verify the folder exists and belongs to the caller - let existing_folder = self.folder_storage.get_folder(id).await.map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!("Failed to get folder with ID: {} for renaming: {}", id, e), - ) - })?; + let existing_folder = self.folder_storage.get_folder(id).await?; if existing_folder.owner_id() != Some(caller_id) { tracing::warn!( @@ -444,12 +439,7 @@ impl FolderUseCase for FolderService { caller_id: Uuid, ) -> Result { // Verify the source folder exists and belongs to the caller - let source_folder = self.folder_storage.get_folder(id).await.map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!("Failed to get folder with ID: {} for moving: {}", id, e), - ) - })?; + let source_folder = self.folder_storage.get_folder(id).await?; if source_folder.owner_id() != Some(caller_id) { tracing::warn!( @@ -461,7 +451,7 @@ impl FolderUseCase for FolderService { return Err(DomainError::not_found("Folder", id)); } - // If a parent_id is specified, verify it exists + // If a parent_id is specified, verify it exists and belongs to the caller if let Some(parent_id) = &dto.parent_id { // Verify we are not trying to move the folder into itself or one of its descendants if parent_id == id { @@ -472,9 +462,17 @@ impl FolderUseCase for FolderService { )); } - // Verify the destination exists - let parent_exists = self.folder_storage.get_folder(parent_id).await.is_ok(); - if !parent_exists { + // Verify the destination exists and is owned by the caller + let parent = self.folder_storage.get_folder(parent_id).await.map_err(|_| { + DomainError::not_found("Folder", parent_id) + })?; + if parent.owner_id() != Some(caller_id) { + tracing::warn!( + "move_folder: user '{}' attempted to move into folder '{}' owned by '{:?}'", + caller_id, + parent_id, + parent.owner_id() + ); return Err(DomainError::not_found("Folder", parent_id)); } @@ -500,12 +498,7 @@ impl FolderUseCase for FolderService { /// Deletes a folder after verifying ownership. async fn delete_folder(&self, id: &str, caller_id: Uuid) -> Result<(), DomainError> { // Verify the folder exists and belongs to the caller - let folder = self.folder_storage.get_folder(id).await.map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!("Failed to get folder with ID: {} for deletion: {}", id, e), - ) - })?; + let folder = self.folder_storage.get_folder(id).await?; if folder.owner_id() != Some(caller_id) { tracing::warn!( From 1afacb65cda778d4a533727f60b1366f16fb28ed Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Tue, 12 May 2026 00:34:04 +0200 Subject: [PATCH 6/6] style(srv): correct clippy recos --- src/application/services/batch_operations.rs | 2 +- src/application/services/folder_service.rs | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/src/application/services/batch_operations.rs b/src/application/services/batch_operations.rs index 1fb58a3b..4d161272 100644 --- a/src/application/services/batch_operations.rs +++ b/src/application/services/batch_operations.rs @@ -12,8 +12,8 @@ use tracing::info; use crate::application::dtos::file_dto::FileDto; use crate::application::dtos::folder_dto::{FolderDto, MoveFolderDto}; use crate::application::ports::file_ports::{FileManagementUseCase, FileRetrievalUseCase}; -use crate::application::ports::storage_ports::CopyFolderTreeResult; use crate::application::ports::inbound::FolderUseCase; +use crate::application::ports::storage_ports::CopyFolderTreeResult; use crate::application::ports::trash_ports::TrashUseCase; use crate::application::services::file_management_service::FileManagementService; use crate::application::services::file_retrieval_service::FileRetrievalService; diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index fab1141a..c2169367 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -463,9 +463,11 @@ impl FolderUseCase for FolderService { } // Verify the destination exists and is owned by the caller - let parent = self.folder_storage.get_folder(parent_id).await.map_err(|_| { - DomainError::not_found("Folder", parent_id) - })?; + let parent = self + .folder_storage + .get_folder(parent_id) + .await + .map_err(|_| DomainError::not_found("Folder", parent_id))?; if parent.owner_id() != Some(caller_id) { tracing::warn!( "move_folder: user '{}' attempted to move into folder '{}' owned by '{:?}'",