diff --git a/src/application/ports/chunked_upload_ports.rs b/src/application/ports/chunked_upload_ports.rs index 3414f35d..f2b3c4f1 100644 --- a/src/application/ports/chunked_upload_ports.rs +++ b/src/application/ports/chunked_upload_ports.rs @@ -59,6 +59,7 @@ pub trait ChunkedUploadPort: Send + Sync + 'static { /// total number of chunks, and expiration timestamp. async fn create_session( &self, + user_id: &str, filename: String, folder_id: Option, content_type: String, @@ -72,13 +73,14 @@ pub trait ChunkedUploadPort: Send + Sync + 'static { async fn upload_chunk( &self, upload_id: &str, + user_id: &str, chunk_index: usize, data: Bytes, checksum: Option, ) -> Result; /// Get the current status of an upload session. - async fn get_status(&self, upload_id: &str) -> Result; + async fn get_status(&self, upload_id: &str, user_id: &str) -> Result; /// Assemble all chunks into the final file. /// @@ -88,13 +90,14 @@ pub trait ChunkedUploadPort: Send + Sync + 'static { async fn complete_upload( &self, upload_id: &str, + user_id: &str, ) -> Result<(PathBuf, String, Option, String, u64, String), DomainError>; /// Finalize upload: clean up the session and temporary files. - async fn finalize_upload(&self, upload_id: &str) -> Result<(), DomainError>; + async fn finalize_upload(&self, upload_id: &str, user_id: &str) -> Result<(), DomainError>; /// Cancel an upload and clean up all temporary data. - async fn cancel_upload(&self, upload_id: &str) -> Result<(), DomainError>; + async fn cancel_upload(&self, upload_id: &str, user_id: &str) -> Result<(), DomainError>; /// Check if a file size qualifies for chunked upload. fn should_use_chunked(&self, size: u64) -> bool; diff --git a/src/application/ports/share_ports.rs b/src/application/ports/share_ports.rs index 0b0ecca2..e8ede9aa 100644 --- a/src/application/ports/share_ports.rs +++ b/src/application/ports/share_ports.rs @@ -15,28 +15,34 @@ pub trait ShareUseCase: Send + Sync + 'static { dto: CreateShareDto, ) -> Result; - /// Get a shared link by its ID - async fn get_shared_link(&self, id: &str) -> Result; + /// Get a shared link by its ID (ownership-verified) + async fn get_shared_link( + &self, + id: &str, + requester_id: &str, + ) -> Result; /// Get a shared link by its token (for access by non-users) async fn get_shared_link_by_token(&self, token: &str) -> Result; - /// Get all shared links for a specific item + /// Get all shared links for a specific item (ownership-verified) async fn get_shared_links_for_item( &self, item_id: &str, item_type: &ShareItemType, + requester_id: &str, ) -> Result, DomainError>; - /// Update a shared link + /// Update a shared link (ownership-verified) async fn update_shared_link( &self, id: &str, + requester_id: &str, dto: UpdateShareDto, ) -> Result; - /// Delete a shared link - async fn delete_shared_link(&self, id: &str) -> Result<(), DomainError>; + /// Delete a shared link (ownership-verified) + async fn delete_shared_link(&self, id: &str, requester_id: &str) -> Result<(), DomainError>; /// Get all shared links created by a specific user async fn get_user_shared_links( @@ -63,20 +69,29 @@ pub trait ShareStoragePort: Send + Sync + 'static { share: &crate::domain::entities::share::Share, ) -> Result; - async fn find_share_by_id( - &self, - id: &str, - ) -> Result; - async fn find_share_by_token( &self, token: &str, ) -> Result; - async fn find_shares_by_item( + /// Find a share by ID only if it belongs to the given user. + /// Returns `NotFound` if the share doesn't exist OR belongs to another user + /// (prevents share-ID enumeration). + async fn find_share_by_id_for_user( + &self, + id: &str, + user_id: &str, + ) -> Result; + + /// Delete a share only if it belongs to the given user. + async fn delete_share_for_user(&self, id: &str, user_id: &str) -> Result<(), DomainError>; + + /// Find shares for a specific item that belong to the given user. + async fn find_shares_by_item_for_user( &self, item_id: &str, item_type: &ShareItemType, + user_id: &str, ) -> Result, DomainError>; async fn update_share( @@ -84,8 +99,6 @@ pub trait ShareStoragePort: Send + Sync + 'static { share: &crate::domain::entities::share::Share, ) -> Result; - async fn delete_share(&self, id: &str) -> Result<(), DomainError>; - async fn find_shares_by_user( &self, user_id: &str, diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index c7b49619..93ba79b6 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -140,6 +140,25 @@ impl ShareService { })?; self.password_hasher.hash_password(password).await } + + /// Fetch a share and verify that `requester_id` owns it. + /// + /// SECURITY: returns `NotFound` (not `Forbidden`) when the share exists + /// but belongs to a different user — this prevents share-ID enumeration + /// attacks where an attacker probes IDs and uses 403-vs-404 to learn + /// which ones are valid. + async fn fetch_owned_share( + &self, + id: &str, + requester_id: &str, + ) -> Result { + let share = self + .share_repository + .find_share_by_id_for_user(id, requester_id) + .await?; + + Ok(share) + } } impl ShareUseCase for ShareService { @@ -187,15 +206,14 @@ impl ShareUseCase for ShareService { Ok(ShareDto::from_entity(&saved_share, &self.config.base_url())) } - async fn get_shared_link(&self, id: &str) -> Result { - // Find the shared link by its ID - let share = self - .share_repository - .find_share_by_id(id) - .await - .map_err(|e| { - ShareServiceError::NotFound(format!("Share with ID {} not found: {}", id, e)) - })?; + async fn get_shared_link( + &self, + id: &str, + requester_id: &str, + ) -> Result { + // SECURITY: ownership-verified lookup — returns 404 if the share + // doesn't exist OR belongs to another user. + let share = self.fetch_owned_share(id, requester_id).await?; // Check if it has expired if share.is_expired() { @@ -229,11 +247,12 @@ impl ShareUseCase for ShareService { &self, item_id: &str, item_type: &ShareItemType, + requester_id: &str, ) -> Result, DomainError> { - // Find all shared links for the item + // SECURITY: only return shares created by the requester 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()))?; @@ -252,16 +271,11 @@ impl ShareUseCase for ShareService { async fn update_shared_link( &self, id: &str, + requester_id: &str, dto: UpdateShareDto, ) -> Result { - // Find the existing shared link - let mut share = self - .share_repository - .find_share_by_id(id) - .await - .map_err(|e| { - ShareServiceError::NotFound(format!("Share with ID {} not found: {}", id, e)) - })?; + // SECURITY: ownership-verified lookup — prevents IDOR + let mut share = self.fetch_owned_share(id, requester_id).await?; // Update permissions if provided if let Some(permissions_dto) = dto.permissions { @@ -302,12 +316,11 @@ impl ShareUseCase for ShareService { )) } - async fn delete_shared_link(&self, id: &str) -> Result<(), DomainError> { - // Delete the shared link + async fn delete_shared_link(&self, id: &str, requester_id: &str) -> Result<(), DomainError> { + // SECURITY: ownership-verified delete — only the creator can remove self.share_repository - .delete_share(id) - .await - .map_err(|e| ShareServiceError::Repository(e.to_string()))?; + .delete_share_for_user(id, requester_id) + .await?; Ok(()) } @@ -688,15 +701,6 @@ mod tests { Ok(share.clone()) } - async fn find_share_by_id(&self, id: &str) -> Result { - let shares = self.shares.lock().unwrap(); - - shares - .get(id) - .cloned() - .ok_or_else(|| DomainError::not_found("Share", id)) - } - async fn find_share_by_token(&self, token: &str) -> Result { let tokens = self.tokens.lock().unwrap(); let shares = self.shares.lock().unwrap(); @@ -711,20 +715,50 @@ mod tests { .ok_or_else(|| DomainError::not_found("Share", id.as_str())) } - async fn find_shares_by_item( + async fn find_share_by_id_for_user( + &self, + id: &str, + user_id: &str, + ) -> Result { + let shares = self.shares.lock().unwrap(); + shares + .get(id) + .filter(|s| s.created_by() == user_id) + .cloned() + .ok_or_else(|| DomainError::not_found("Share", id)) + } + + async fn delete_share_for_user(&self, id: &str, user_id: &str) -> Result<(), DomainError> { + let mut shares = self.shares.lock().unwrap(); + let mut tokens = self.tokens.lock().unwrap(); + + let share = shares + .get(id) + .filter(|s| s.created_by() == user_id) + .ok_or_else(|| DomainError::not_found("Share", id))?; + + tokens.remove(share.token()); + shares.remove(id); + Ok(()) + } + + async fn find_shares_by_item_for_user( &self, item_id: &str, item_type: &ShareItemType, + user_id: &str, ) -> Result, DomainError> { let shares = self.shares.lock().unwrap(); - let type_str = item_type.to_string(); let result: Vec = shares .values() - .filter(|s| s.item_id() == item_id && s.item_type().to_string() == type_str) + .filter(|s| { + s.item_id() == item_id + && s.item_type().to_string() == type_str + && s.created_by() == user_id + }) .cloned() .collect(); - Ok(result) } @@ -741,24 +775,6 @@ mod tests { Ok(share.clone()) } - async fn delete_share(&self, id: &str) -> Result<(), DomainError> { - let mut shares = self.shares.lock().unwrap(); - let mut tokens = self.tokens.lock().unwrap(); - - // Find the share to get the token - let share = shares - .get(id) - .ok_or_else(|| DomainError::not_found("Share", id))?; - - // Remove token mapping - tokens.remove(share.token()); - - // Remove the share - shares.remove(id); - - Ok(()) - } - async fn find_shares_by_user( &self, user_id: &str, diff --git a/src/application/services/trash_service.rs b/src/application/services/trash_service.rs index dc572a4c..9c08d320 100644 --- a/src/application/services/trash_service.rs +++ b/src/application/services/trash_service.rs @@ -144,9 +144,8 @@ impl TrashUseCase for TrashService { ); debug!("User UUID validation: {}", user_id); - // Note: We do NOT call validate_user_ownership here because the item - // is not yet in the trash. Ownership validation is only for operations - // on already-trashed items (restore, delete_permanently). + // Note: We now verify file/folder ownership BEFORE moving to trash. + // This prevents users from trashing items they do not own (IDOR). // Parse UUIDs with detailed error handling debug!("Validating item UUID: {}", item_id); @@ -183,9 +182,11 @@ impl TrashUseCase for TrashService { "file" => { info!("Processing file to move to trash: {}", item_id); - // Get the file to verify it exists and capture its data - debug!("Getting file data: {}", item_id); - let file = match self.file_read_port.get_file(item_id).await { + // Get the file — ownership-verified at SQL level. + // Returns NotFound if the file does not exist OR belongs to + // another user, preventing cross-user trash operations. + debug!("Getting file data (owner-scoped): {}", item_id); + let file = match self.file_read_port.get_file_for_owner(item_id, user_id).await { Ok(file) => { debug!("File found: {} ({})", file.name(), item_id); file @@ -254,7 +255,9 @@ impl TrashUseCase for TrashService { Ok(()) } "folder" => { - // Get the folder to verify it exists and capture its data + // Get the folder and verify ownership. + // Returns NotFound if the folder does not exist or belongs + // to another user — prevents cross-user trash operations. let folder = self .folder_storage_port .get_folder(item_id) @@ -267,6 +270,15 @@ 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) { + return Err(DomainError::not_found( + "Folder", + format!("Folder not found: {}", item_id), + )); + } + let original_path = folder.storage_path().to_string(); // Create the trash item diff --git a/src/infrastructure/repositories/pg/share_pg_repository.rs b/src/infrastructure/repositories/pg/share_pg_repository.rs index c33404e5..b55516fe 100644 --- a/src/infrastructure/repositories/pg/share_pg_repository.rs +++ b/src/infrastructure/repositories/pg/share_pg_repository.rs @@ -120,33 +120,6 @@ impl ShareStoragePort for SharePgRepository { Self::row_to_entity(&row) } - async fn find_share_by_id(&self, id: &str) -> Result { - let row = sqlx::query( - r#" - SELECT id::TEXT, item_id, item_name, item_type, token, password_hash, - expires_at, permissions_read, permissions_write, permissions_reshare, - created_at, created_by, access_count - FROM storage.shares - WHERE id = $1::UUID - "#, - ) - .bind(id) - .fetch_optional(&*self.db_pool) - .await - .map_err(|e| { - tracing::error!("Database error finding share by id: {}", e); - DomainError::internal_error("Share", format!("Failed to find share: {e}")) - })?; - - match row { - Some(r) => Self::row_to_entity(&r), - None => Err(DomainError::not_found( - "Share", - format!("Share with ID {id} not found"), - )), - } - } - async fn find_share_by_token(&self, token: &str) -> Result { let row = sqlx::query( r#" @@ -174,10 +147,70 @@ impl ShareStoragePort for SharePgRepository { } } - async fn find_shares_by_item( + async fn find_share_by_id_for_user( + &self, + id: &str, + user_id: &str, + ) -> Result { + let row = sqlx::query( + r#" + SELECT id::TEXT, item_id, item_name, item_type, token, password_hash, + expires_at, permissions_read, permissions_write, permissions_reshare, + created_at, created_by, access_count + FROM storage.shares + WHERE id = $1::UUID AND created_by = $2 + "#, + ) + .bind(id) + .bind(user_id) + .fetch_optional(&*self.db_pool) + .await + .map_err(|e| { + tracing::error!("Database error finding share by id for user: {}", e); + DomainError::internal_error("Share", format!("Failed to find share: {e}")) + })?; + + match row { + Some(r) => Self::row_to_entity(&r), + // SECURITY: return NotFound (not Forbidden) to prevent share-ID enumeration + None => Err(DomainError::not_found( + "Share", + format!("Share with ID {id} not found"), + )), + } + } + + async fn delete_share_for_user(&self, id: &str, user_id: &str) -> Result<(), DomainError> { + let result = + sqlx::query("DELETE FROM storage.shares WHERE id = $1::UUID AND created_by = $2") + .bind(id) + .bind(user_id) + .execute(&*self.db_pool) + .await + .map_err(|e| { + tracing::error!("Database error deleting share for user: {}", e); + DomainError::internal_error( + "Share", + format!("Failed to delete share: {e}"), + ) + })?; + + if result.rows_affected() == 0 { + // SECURITY: could be non-existent or owned by another user — same 404 + return Err(DomainError::not_found( + "Share", + format!("Share with ID {id} not found"), + )); + } + + Ok(()) + } + + async fn find_shares_by_item_for_user( &self, item_id: &str, item_type: &ShareItemType, + user_id: &str, ) -> Result, DomainError> { let rows = sqlx::query( r#" @@ -185,17 +218,21 @@ impl ShareStoragePort for SharePgRepository { expires_at, permissions_read, permissions_write, permissions_reshare, created_at, created_by, access_count FROM storage.shares - WHERE item_id = $1 AND item_type = $2 + WHERE item_id = $1 AND item_type = $2 AND created_by = $3 ORDER BY created_at DESC "#, ) .bind(item_id) .bind(item_type.to_string()) + .bind(user_id) .fetch_all(&*self.db_pool) .await .map_err(|e| { - tracing::error!("Database error finding shares by item: {}", e); - DomainError::internal_error("Share", format!("Failed to find shares by item: {e}")) + tracing::error!("Database error finding shares by item for user: {}", e); + DomainError::internal_error( + "Share", + format!("Failed to find shares by item: {e}"), + ) })?; rows.iter().map(Self::row_to_entity).collect() @@ -243,26 +280,6 @@ impl ShareStoragePort for SharePgRepository { } } - async fn delete_share(&self, id: &str) -> Result<(), DomainError> { - let result = sqlx::query("DELETE FROM storage.shares WHERE id = $1::UUID") - .bind(id) - .execute(&*self.db_pool) - .await - .map_err(|e| { - tracing::error!("Database error deleting share: {}", e); - DomainError::internal_error("Share", format!("Failed to delete share: {e}")) - })?; - - if result.rows_affected() == 0 { - return Err(DomainError::not_found( - "Share", - format!("Share with ID {id} not found for deletion"), - )); - } - - Ok(()) - } - async fn find_shares_by_user( &self, user_id: &str, diff --git a/src/infrastructure/services/chunked_upload_service.rs b/src/infrastructure/services/chunked_upload_service.rs index 220c7647..6e0723fb 100644 --- a/src/infrastructure/services/chunked_upload_service.rs +++ b/src/infrastructure/services/chunked_upload_service.rs @@ -73,6 +73,7 @@ pub struct ChunkInfo { #[derive(Debug, Clone, Serialize, Deserialize)] pub struct UploadSession { pub id: String, + pub user_id: String, pub filename: String, pub folder_id: Option, pub content_type: String, @@ -368,9 +369,27 @@ impl ChunkedUploadService { // ── Core operations ────────────────────────────────────────────────── + /// Verify that the given session belongs to the given user. + /// Returns 404 (not 403) to avoid revealing the existence of other users' sessions. + fn verify_session_owner( + &self, + upload_id: &str, + user_id: &str, + ) -> Result<(), String> { + let session = self + .sessions + .get(upload_id) + .ok_or_else(|| format!("Upload session not found: {}", upload_id))?; + if session.user_id != user_id { + return Err(format!("Upload session not found: {}", upload_id)); + } + Ok(()) + } + /// Create a new upload session (persists `session.json` + empty `progress.bin`) async fn create_session_inner( &self, + user_id: String, filename: String, folder_id: Option, content_type: String, @@ -412,6 +431,7 @@ impl ChunkedUploadService { let now = Utc::now(); let session = UploadSession { id: upload_id.clone(), + user_id, filename, folder_id, content_type, @@ -451,11 +471,15 @@ impl ChunkedUploadService { async fn upload_chunk_inner( &self, upload_id: &str, + user_id: &str, chunk_index: usize, data: bytes::Bytes, checksum: Option, ) -> Result { - // Validate session exists and chunk index is valid + // Verify session exists AND belongs to the requesting user + self.verify_session_owner(upload_id, user_id)?; + + // Validate chunk index is valid let (chunk_path, expected_size) = { let session = self .sessions @@ -568,7 +592,9 @@ impl ChunkedUploadService { } /// Get upload status - async fn get_status_inner(&self, upload_id: &str) -> Result { + async fn get_status_inner(&self, upload_id: &str, user_id: &str) -> Result { + self.verify_session_owner(upload_id, user_id)?; + let session = self .sessions .get(upload_id) @@ -603,7 +629,11 @@ impl ChunkedUploadService { async fn complete_upload_inner( &self, upload_id: &str, + user_id: &str, ) -> Result<(PathBuf, String, Option, String, u64, String), String> { + // Verify ownership before assembly + self.verify_session_owner(upload_id, user_id)?; + // Get session and validate completion. // Clone the session data and drop the DashMap ref immediately // so the shard is not held during the expensive assembly step. @@ -723,7 +753,9 @@ impl ChunkedUploadService { } /// Finalize upload: remove session from RAM, then clean disk OUTSIDE lock. - async fn finalize_upload_inner(&self, upload_id: &str) -> Result<(), String> { + async fn finalize_upload_inner(&self, upload_id: &str, user_id: &str) -> Result<(), String> { + self.verify_session_owner(upload_id, user_id)?; + // Remove from map (~µs) — releases shard immediately let removed = self.sessions.remove(upload_id).map(|(_, s)| s); @@ -737,7 +769,9 @@ impl ChunkedUploadService { } /// Cancel an upload and cleanup — disk I/O outside lock. - async fn cancel_upload_inner(&self, upload_id: &str) -> Result<(), String> { + async fn cancel_upload_inner(&self, upload_id: &str, user_id: &str) -> Result<(), String> { + self.verify_session_owner(upload_id, user_id)?; + // Remove from map (~µs) let removed = self.sessions.remove(upload_id).map(|(_, s)| s); @@ -767,13 +801,14 @@ impl ChunkedUploadService { impl ChunkedUploadPort for ChunkedUploadService { async fn create_session( &self, + user_id: &str, filename: String, folder_id: Option, content_type: String, total_size: u64, chunk_size: Option, ) -> Result { - self.create_session_inner(filename, folder_id, content_type, total_size, chunk_size) + self.create_session_inner(user_id.to_owned(), filename, folder_id, content_type, total_size, chunk_size) .await .map_err(|e| DomainError::new(ErrorKind::InternalError, "ChunkedUpload", e)) } @@ -781,17 +816,18 @@ impl ChunkedUploadPort for ChunkedUploadService { async fn upload_chunk( &self, upload_id: &str, + user_id: &str, chunk_index: usize, data: bytes::Bytes, checksum: Option, ) -> Result { - self.upload_chunk_inner(upload_id, chunk_index, data, checksum) + self.upload_chunk_inner(upload_id, user_id, chunk_index, data, checksum) .await .map_err(|e| DomainError::new(ErrorKind::InternalError, "ChunkedUpload", e)) } - async fn get_status(&self, upload_id: &str) -> Result { - self.get_status_inner(upload_id) + async fn get_status(&self, upload_id: &str, user_id: &str) -> Result { + self.get_status_inner(upload_id, user_id) .await .map_err(|e| DomainError::new(ErrorKind::NotFound, "ChunkedUpload", e)) } @@ -799,20 +835,21 @@ impl ChunkedUploadPort for ChunkedUploadService { async fn complete_upload( &self, upload_id: &str, + user_id: &str, ) -> Result<(PathBuf, String, Option, String, u64, String), DomainError> { - self.complete_upload_inner(upload_id) + self.complete_upload_inner(upload_id, user_id) .await .map_err(|e| DomainError::new(ErrorKind::InternalError, "ChunkedUpload", e)) } - async fn finalize_upload(&self, upload_id: &str) -> Result<(), DomainError> { - self.finalize_upload_inner(upload_id) + async fn finalize_upload(&self, upload_id: &str, user_id: &str) -> Result<(), DomainError> { + self.finalize_upload_inner(upload_id, user_id) .await .map_err(|e| DomainError::new(ErrorKind::InternalError, "ChunkedUpload", e)) } - async fn cancel_upload(&self, upload_id: &str) -> Result<(), DomainError> { - self.cancel_upload_inner(upload_id) + async fn cancel_upload(&self, upload_id: &str, user_id: &str) -> Result<(), DomainError> { + self.cancel_upload_inner(upload_id, user_id) .await .map_err(|e| DomainError::new(ErrorKind::InternalError, "ChunkedUpload", e)) } @@ -854,6 +891,7 @@ mod tests { let now = Utc::now(); let mut session = UploadSession { id: "test-id".into(), + user_id: "user-1".into(), filename: "file.bin".into(), folder_id: None, content_type: "application/octet-stream".into(), @@ -900,6 +938,7 @@ mod tests { let now = Utc::now(); let session = UploadSession { id: "abc-123".into(), + user_id: "user-1".into(), filename: "photo.jpg".into(), folder_id: Some("folder-1".into()), content_type: "image/jpeg".into(), @@ -931,6 +970,7 @@ mod tests { let restored: UploadSession = serde_json::from_slice(&json).expect("deserialise"); assert_eq!(restored.id, session.id); + assert_eq!(restored.user_id, session.user_id); assert_eq!(restored.filename, session.filename); assert_eq!(restored.folder_id, session.folder_id); assert_eq!(restored.total_size, session.total_size); @@ -944,6 +984,7 @@ mod tests { fn test_session_expiry_check() { let mut session = UploadSession { id: "exp-test".into(), + user_id: "user-1".into(), filename: "f".into(), folder_id: None, content_type: "x".into(), @@ -974,6 +1015,7 @@ mod tests { // Create a session let resp = service .create_session_inner( + "test-user".into(), "bigfile.bin".into(), Some("folder-x".into()), "application/octet-stream".into(), @@ -988,7 +1030,7 @@ mod tests { // Upload first chunk (5 MB of zeros) let chunk_data = bytes::Bytes::from(vec![0u8; 5 * 1024 * 1024]); service - .upload_chunk_inner(&upload_id, 0, chunk_data, None) + .upload_chunk_inner(&upload_id, "test-user", 0, chunk_data, None) .await .expect("upload_chunk 0"); @@ -1024,6 +1066,7 @@ mod tests { // 1. Create session (1024 bytes, 512 byte chunks → 2 chunks) let resp = service .create_session_inner( + "test-user".into(), "test.txt".into(), None, "text/plain".into(), @@ -1040,28 +1083,28 @@ mod tests { // 2. Upload chunks let chunk0 = bytes::Bytes::from(vec![b'A'; 512]); let r0 = service - .upload_chunk_inner(&id, 0, chunk0, None) + .upload_chunk_inner(&id, "test-user", 0, chunk0, None) .await .expect("chunk 0"); assert!(!r0.is_complete); let chunk1 = bytes::Bytes::from(vec![b'B'; 512]); let r1 = service - .upload_chunk_inner(&id, 1, chunk1, None) + .upload_chunk_inner(&id, "test-user", 1, chunk1, None) .await .expect("chunk 1"); assert!(r1.is_complete); assert_eq!(r1.bytes_received, 1024); // 3. Status check - let status = service.get_status_inner(&id).await.expect("status"); + let status = service.get_status_inner(&id, "test-user").await.expect("status"); assert!(status.is_complete); assert_eq!(status.completed_chunks, 2); assert!(status.pending_chunks.is_empty()); // 4. Complete (assemble) let (path, filename, _folder, _ct, size, hash) = - service.complete_upload_inner(&id).await.expect("complete"); + service.complete_upload_inner(&id, "test-user").await.expect("complete"); assert_eq!(filename, "test.txt"); assert_eq!(size, 1024); assert!(!hash.is_empty()); @@ -1073,7 +1116,7 @@ mod tests { assert_eq!(&content[512..], &[b'B'; 512]); // 6. Finalize - service.finalize_upload_inner(&id).await.expect("finalize"); + service.finalize_upload_inner(&id, "test-user").await.expect("finalize"); assert_eq!(service.active_sessions().await, 0); let _ = fs::remove_dir_all(&base).await; @@ -1086,6 +1129,7 @@ mod tests { let resp = service .create_session_inner( + "test-user".into(), "x.bin".into(), None, "application/octet-stream".into(), @@ -1099,7 +1143,7 @@ mod tests { assert!(session_dir.exists()); service - .cancel_upload_inner(&resp.upload_id) + .cancel_upload_inner(&resp.upload_id, "test-user") .await .expect("cancel"); @@ -1120,6 +1164,7 @@ mod tests { let expired_session = UploadSession { id: "expired-session".into(), + user_id: "user-1".into(), filename: "old.bin".into(), folder_id: None, content_type: "application/octet-stream".into(), @@ -1162,6 +1207,7 @@ mod tests { let session = UploadSession { id: "partial-session".into(), + user_id: "user-1".into(), filename: "file.bin".into(), folder_id: None, content_type: "application/octet-stream".into(), diff --git a/src/infrastructure/services/dedup_service.rs b/src/infrastructure/services/dedup_service.rs index aa273b7d..ba65b51d 100644 --- a/src/infrastructure/services/dedup_service.rs +++ b/src/infrastructure/services/dedup_service.rs @@ -410,6 +410,22 @@ impl DedupService { .unwrap_or(false) } + /// Returns `true` if `user_id` owns at least one (non-trashed) file that + /// references the blob identified by `hash`. + /// + /// Used by the dedup API handlers to enforce per-user access control on + /// the content-addressed blob store. + pub async fn user_owns_blob_reference(&self, hash: &str, user_id: &str) -> bool { + sqlx::query_scalar::<_, bool>( + "SELECT EXISTS(SELECT 1 FROM storage.files WHERE blob_hash = $1 AND user_id = $2 AND NOT is_trashed)", + ) + .bind(hash) + .bind(user_id) + .fetch_one(self.pool.as_ref()) + .await + .unwrap_or(false) + } + /// Get metadata for a blob from PostgreSQL. pub async fn get_blob_metadata(&self, hash: &str) -> Option { let row = sqlx::query_as::<_, (String, i64, i32, Option)>( diff --git a/src/interfaces/api/handlers/batch_handler.rs b/src/interfaces/api/handlers/batch_handler.rs index f79be065..fb384f7d 100644 --- a/src/interfaces/api/handlers/batch_handler.rs +++ b/src/interfaces/api/handlers/batch_handler.rs @@ -149,7 +149,10 @@ pub async fn move_files_batch( .batch_service .move_files(request.file_ids, request.target_folder_id, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch move_files failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Convert result to DTO let response: BatchOperationResponse = result.into(); @@ -190,7 +193,10 @@ pub async fn copy_files_batch( .batch_service .copy_files(request.file_ids, request.target_folder_id, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch copy_files failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Convert result to DTO let response: BatchOperationResponse = result.into(); @@ -231,7 +237,10 @@ pub async fn delete_files_batch( .batch_service .delete_files(request.file_ids, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch delete_files failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Create custom response for string IDs let response = BatchOperationResponse { @@ -280,7 +289,10 @@ pub async fn delete_folders_batch( .batch_service .delete_folders(request.folder_ids, request.recursive, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch delete_folders failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Create custom response for string IDs let response = BatchOperationResponse { @@ -336,7 +348,10 @@ pub async fn create_folders_batch( .batch_service .create_folders(folders, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch create_folders failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Convert result to DTO let response: BatchOperationResponse = result.into(); @@ -377,7 +392,10 @@ pub async fn get_files_batch( .batch_service .get_multiple_files(request.file_ids, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch get_files failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Convert result to DTO let response: BatchOperationResponse = result.into(); @@ -418,7 +436,10 @@ pub async fn get_folders_batch( .batch_service .get_multiple_folders(request.folder_ids, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch get_folders failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; // Convert result to DTO let response: BatchOperationResponse = result.into(); @@ -497,9 +518,10 @@ pub async fn trash_batch( ); } Err(e) => { + tracing::error!("Batch trash_files failed: {}", e); return Ok(( StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": e.to_string() })), + Json(serde_json::json!({ "error": "Batch trash operation failed" })), ) .into_response()); } @@ -523,9 +545,10 @@ pub async fn trash_batch( ); } Err(e) => { + tracing::error!("Batch trash_folders failed: {}", e); return Ok(( StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ "error": e.to_string() })), + Json(serde_json::json!({ "error": "Batch trash operation failed" })), ) .into_response()); } @@ -579,7 +602,10 @@ pub async fn move_folders_batch( .batch_service .move_folders(request.folder_ids, request.target_folder_id, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch move_folders failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch operation failed".to_string()) + })?; let response: BatchOperationResponse = result.into(); @@ -616,7 +642,10 @@ pub async fn download_batch( .batch_service .download_zip(request.file_ids, request.folder_ids, &auth_user.id) .await - .map_err(|e| (StatusCode::INTERNAL_SERVER_ERROR, e.to_string()))?; + .map_err(|e| { + tracing::error!("Batch download ZIP failed: {}", e); + (StatusCode::INTERNAL_SERVER_ERROR, "Batch download failed".to_string()) + })?; // Read file size for Content-Length before splitting ownership let file_size = temp_file @@ -624,9 +653,10 @@ pub async fn download_batch( .metadata() .map(|m| m.len()) .map_err(|e| { + tracing::error!("Failed to read temp file metadata: {}", e); ( StatusCode::INTERNAL_SERVER_ERROR, - format!("Failed to read temp file metadata: {}", e), + "Failed to prepare download".to_string(), ) })?; diff --git a/src/interfaces/api/handlers/chunked_upload_handler.rs b/src/interfaces/api/handlers/chunked_upload_handler.rs index 678c7578..2145a58b 100644 --- a/src/interfaces/api/handlers/chunked_upload_handler.rs +++ b/src/interfaces/api/handlers/chunked_upload_handler.rs @@ -22,7 +22,7 @@ use crate::application::ports::chunked_upload_ports::DEFAULT_CHUNK_SIZE; use crate::application::ports::file_ports::FileUploadUseCase; use crate::application::ports::storage_ports::StorageUsagePort; use crate::common::di::AppState; -use crate::domain::errors::ErrorKind; +use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; /// Request body for creating an upload session @@ -146,6 +146,7 @@ impl ChunkedUploadHandler { match chunked_service .create_session( + &auth_user.id, request.filename, request.folder_id, content_type, @@ -157,12 +158,7 @@ impl ChunkedUploadHandler { Ok(response) => (StatusCode::CREATED, Json(response)).into_response(), Err(e) => { tracing::error!("Failed to create upload session: {}", e); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": e.to_string() - })), - ) + AppError::internal_error(format!("Failed to create upload session: {}", e)) .into_response() } } @@ -177,6 +173,7 @@ impl ChunkedUploadHandler { /// Body: Raw bytes of the chunk pub async fn upload_chunk( State(state): State>, + auth_user: AuthUser, Path(upload_id): Path, Query(params): Query, headers: HeaderMap, @@ -193,7 +190,7 @@ impl ChunkedUploadHandler { }); match chunked_service - .upload_chunk(&upload_id, params.chunk_index, body, checksum) + .upload_chunk(&upload_id, &auth_user.id, params.chunk_index, body, checksum) .await { Ok(response) => { @@ -216,22 +213,7 @@ impl ChunkedUploadHandler { .unwrap() .into_response() } - Err(e) => { - let status = match e.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::InvalidInput => StatusCode::BAD_REQUEST, - ErrorKind::AlreadyExists => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - ( - status, - Json(serde_json::json!({ - "error": e.to_string() - })), - ) - .into_response() - } + Err(e) => AppError::from(e).into_response() } } @@ -240,11 +222,12 @@ impl ChunkedUploadHandler { /// Returns upload progress and pending chunks pub async fn get_upload_status( State(state): State>, + auth_user: AuthUser, Path(upload_id): Path, ) -> impl IntoResponse { let chunked_service = &state.core.chunked_upload_service; - match chunked_service.get_status(&upload_id).await { + match chunked_service.get_status(&upload_id, &auth_user.id).await { Ok(status) => Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, "application/json") @@ -261,13 +244,7 @@ impl ChunkedUploadHandler { )) .unwrap() .into_response(), - Err(e) => ( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ - "error": e.to_string() - })), - ) - .into_response(), + Err(e) => AppError::from(e).into_response(), } } @@ -276,6 +253,7 @@ impl ChunkedUploadHandler { /// Assembles all chunks into the final file and creates the file record pub async fn complete_upload( State(state): State>, + auth_user: AuthUser, Path(upload_id): Path, ) -> impl IntoResponse { let chunked_service = &state.core.chunked_upload_service; @@ -283,22 +261,10 @@ impl ChunkedUploadHandler { // Assemble chunks (hash-on-write: SHA-256 computed during assembly) let (assembled_path, filename, folder_id, content_type, total_size, hash) = - match chunked_service.complete_upload(&upload_id).await { + match chunked_service.complete_upload(&upload_id, &auth_user.id).await { Ok(result) => result, Err(e) => { - let status = match e.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::InvalidInput | ErrorKind::AlreadyExists => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - return ( - status, - Json(serde_json::json!({ - "error": e.to_string() - })), - ) - .into_response(); + return AppError::from(e).into_response(); } }; @@ -323,7 +289,7 @@ impl ChunkedUploadHandler { { Ok(file) => { // Cleanup session - let _ = chunked_service.finalize_upload(&upload_id).await; + let _ = chunked_service.finalize_upload(&upload_id, &auth_user.id).await; tracing::info!( "✅ CHUNKED UPLOAD COMPLETE: {} (ID: {}, {} bytes)", @@ -345,12 +311,7 @@ impl ChunkedUploadHandler { } Err(e) => { tracing::error!("Failed to create file from assembled upload: {:?}", e); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": format!("Failed to create file: {:?}", e) - })), - ) + AppError::internal_error(format!("Failed to create file: {}", e)) .into_response() } } @@ -361,18 +322,14 @@ impl ChunkedUploadHandler { /// Cancels an in-progress upload and cleans up temp files pub async fn cancel_upload( State(state): State>, + auth_user: AuthUser, Path(upload_id): Path, ) -> impl IntoResponse { let chunked_service = &state.core.chunked_upload_service; - match chunked_service.cancel_upload(&upload_id).await { + match chunked_service.cancel_upload(&upload_id, &auth_user.id).await { Ok(_) => StatusCode::NO_CONTENT.into_response(), - Err(e) => ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": e.to_string() - })), - ) + Err(e) => AppError::internal_error(format!("Failed to cancel upload: {}", e)) .into_response(), } } diff --git a/src/interfaces/api/handlers/dedup_handler.rs b/src/interfaces/api/handlers/dedup_handler.rs index 58c5fa84..2c3de0ea 100644 --- a/src/interfaces/api/handlers/dedup_handler.rs +++ b/src/interfaces/api/handlers/dedup_handler.rs @@ -9,6 +9,7 @@ use serde::Serialize; use crate::application::ports::dedup_ports::DedupResultDto; use crate::common::di::AppState; +use crate::interfaces::middleware::auth::AuthUser; use std::sync::Arc; /// Global application state for dependency injection @@ -72,14 +73,15 @@ pub struct StatsResponse { pub struct DedupHandler; impl DedupHandler { - /// Check if a blob with the given hash already exists + /// Check if the authenticated user already has a file with the given hash. /// - /// This endpoint allows clients to check if uploading a file is necessary - /// by pre-computing the hash client-side and checking against the server. + /// User-scoped: only reveals whether **this user** owns a file that + /// references the blob — never exposes global existence or ref_count. /// /// GET /api/dedup/check/{hash} pub async fn check_hash( State(state): State, + auth_user: AuthUser, Path(hash): Path, ) -> impl IntoResponse { let dedup = &state.core.dedup_service; @@ -96,35 +98,37 @@ impl DedupHandler { .into_response(); } - match dedup.get_blob_metadata(&hash).await { - Some(metadata) => { - let response = HashCheckResponse { - exists: true, - hash, - existing_size: Some(metadata.size), - ref_count: Some(metadata.ref_count), - }; - Response::builder() - .status(StatusCode::OK) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(serde_json::to_string(&response).unwrap())) - .unwrap() - .into_response() - } - None => { - let response = HashCheckResponse { - exists: false, - hash, - existing_size: None, - ref_count: None, - }; - Response::builder() - .status(StatusCode::OK) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(serde_json::to_string(&response).unwrap())) - .unwrap() - .into_response() - } + // Only reveal whether THIS user has the blob — no global oracle + let user_has_it = dedup.user_owns_blob_reference(&hash, &auth_user.id).await; + + if user_has_it { + // Fetch size from metadata (safe — user owns a reference) + let size = dedup.get_blob_metadata(&hash).await.map(|m| m.size); + let response = HashCheckResponse { + exists: true, + hash, + existing_size: size, + ref_count: None, // Never expose global ref_count + }; + Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(serde_json::to_string(&response).unwrap())) + .unwrap() + .into_response() + } else { + let response = HashCheckResponse { + exists: false, + hash, + existing_size: None, + ref_count: None, + }; + Response::builder() + .status(StatusCode::OK) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(serde_json::to_string(&response).unwrap())) + .unwrap() + .into_response() } } @@ -139,6 +143,7 @@ impl DedupHandler { /// Returns information about whether the content was new or deduplicated. pub async fn upload_with_dedup( State(state): State, + _auth_user: AuthUser, mut multipart: Multipart, ) -> impl IntoResponse { let dedup = &state.core.dedup_service; @@ -222,14 +227,13 @@ impl DedupHandler { .into_response(); } Err(e) => { - tracing::error!("❌ Dedup upload failed: {}", e); + tracing::error!("Dedup upload failed: {}", e); return Response::builder() .status(StatusCode::INTERNAL_SERVER_ERROR) .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(format!( - r#"{{"error": "Upload failed: {}"}}"#, - e - ))) + .body(Body::from( + r#"{"error": "Upload failed"}"#, + )) .unwrap() .into_response(); } @@ -256,7 +260,20 @@ impl DedupHandler { /// - Total references /// - Bytes saved /// - Deduplication ratio - pub async fn get_stats(State(state): State) -> impl IntoResponse { + pub async fn get_stats( + State(state): State, + auth_user: AuthUser, + ) -> impl IntoResponse { + // Admin-only — global dedup statistics are sensitive infrastructure data + if auth_user.role != "admin" { + return Response::builder() + .status(StatusCode::FORBIDDEN) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(r#"{"error": "Admin role required"}"#)) + .unwrap() + .into_response(); + } + let dedup = &state.core.dedup_service; let stats = dedup.get_stats().await; @@ -285,14 +302,16 @@ impl DedupHandler { .into_response() } - /// Retrieve content by hash + /// Retrieve content by hash (user-scoped). /// /// GET /api/dedup/blob/{hash} /// - /// Returns the raw content of a blob identified by its SHA-256 hash. - /// Useful for retrieving deduplicated content. + /// Returns the raw content of a blob **only if** the authenticated user + /// owns at least one file that references it. Returns 404 otherwise + /// (does not reveal whether the blob exists globally). pub async fn get_blob( State(state): State, + auth_user: AuthUser, Path(hash): Path, ) -> impl IntoResponse { let dedup = &state.core.dedup_service; @@ -307,6 +326,16 @@ impl DedupHandler { .into_response(); } + // Verify the user owns at least one file referencing this blob + if !dedup.user_owns_blob_reference(&hash, &auth_user.id).await { + return Response::builder() + .status(StatusCode::NOT_FOUND) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(r#"{"error": "Blob not found"}"#)) + .unwrap() + .into_response(); + } + // Get metadata first for content-type let metadata = dedup.get_blob_metadata(&hash).await; let content_type = metadata @@ -345,65 +374,26 @@ impl DedupHandler { } } - /// Remove a reference to a blob - /// - /// DELETE /api/dedup/blob/{hash} - /// - /// Decrements the reference count for a blob. If the reference count - /// reaches zero, the blob is deleted from storage. - pub async fn remove_reference( - State(state): State, - Path(hash): Path, - ) -> impl IntoResponse { - let dedup = &state.core.dedup_service; - - // Validate hash format - if hash.len() != 64 || !hash.chars().all(|c| c.is_ascii_hexdigit()) { - return Response::builder() - .status(StatusCode::BAD_REQUEST) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(r#"{"error": "Invalid hash format"}"#)) - .unwrap() - .into_response(); - } - - match dedup.remove_reference(&hash).await { - Ok(deleted) => { - let message = if deleted { - format!( - r#"{{"success": true, "deleted": true, "message": "Blob {} was deleted (ref_count reached 0)"}}"#, - hash - ) - } else { - format!( - r#"{{"success": true, "deleted": false, "message": "Reference removed from blob {}"}}"#, - hash - ) - }; - - Response::builder() - .status(StatusCode::OK) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(message)) - .unwrap() - .into_response() - } - Err(e) => Response::builder() - .status(StatusCode::INTERNAL_SERVER_ERROR) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(format!(r#"{{"error": "{}"}}"#, e))) - .unwrap() - .into_response(), - } - } - /// Force recalculation of statistics from disk /// /// POST /api/dedup/recalculate /// /// Verifies integrity and returns current statistics. /// Useful for health checks and auditing. - pub async fn recalculate_stats(State(state): State) -> impl IntoResponse { + pub async fn recalculate_stats( + State(state): State, + auth_user: AuthUser, + ) -> impl IntoResponse { + // Admin-only — integrity verification is a privileged operation + if auth_user.role != "admin" { + return Response::builder() + .status(StatusCode::FORBIDDEN) + .header(header::CONTENT_TYPE, "application/json") + .body(Body::from(r#"{"error": "Admin role required"}"#)) + .unwrap() + .into_response(); + } + let dedup = &state.core.dedup_service; // Verify integrity first @@ -414,13 +404,13 @@ impl DedupHandler { } } Err(e) => { + tracing::error!("Dedup integrity verification failed: {}", e); return Response::builder() .status(StatusCode::INTERNAL_SERVER_ERROR) .header(header::CONTENT_TYPE, "application/json") - .body(Body::from(format!( - r#"{{"error": "Verification failed: {}"}}"#, - e - ))) + .body(Body::from( + r#"{"error": "Verification failed"}"#, + )) .unwrap() .into_response(); } diff --git a/src/interfaces/api/handlers/favorites_handler.rs b/src/interfaces/api/handlers/favorites_handler.rs index e213f0f6..e08caea4 100644 --- a/src/interfaces/api/handlers/favorites_handler.rs +++ b/src/interfaces/api/handlers/favorites_handler.rs @@ -42,7 +42,7 @@ pub async fn get_favorites( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to retrieve favorites: {}", err) + "error": "Failed to retrieve favorites" })), ) .into_response() @@ -86,7 +86,7 @@ pub async fn add_favorite( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to add to favorites: {}", err) + "error": "Failed to add to favorites" })), ) } @@ -129,7 +129,7 @@ pub async fn remove_favorite( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to remove from favorites: {}", err) + "error": "Failed to remove from favorites" })), ) } @@ -188,7 +188,7 @@ pub async fn batch_add_favorites( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to batch add favorites: {}", err) + "error": "Failed to batch add favorites" })), ) .into_response() diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 1dc43ad9..83913852 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -17,6 +17,7 @@ use crate::application::ports::file_ports::{ use crate::application::ports::storage_ports::StorageUsagePort; use crate::application::ports::thumbnail_ports::ThumbnailPort; use crate::common::di::AppState; +use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; use std::sync::Arc; @@ -291,13 +292,7 @@ impl FileHandler { { Ok(f) => f, Err(err) => { - return ( - StatusCode::NOT_FOUND, - Json(serde_json::json!({ - "error": format!("File not found: {}", err) - })), - ) - .into_response(); + return AppError::from(err).into_response(); } }; @@ -331,13 +326,7 @@ impl FileHandler { .into_response() } Err(err) => { - tracing::error!("Thumbnail generation failed: {}", err); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": format!("Failed to generate thumbnail: {}", err) - })), - ) + AppError::internal_error(format!("Thumbnail generation failed: {}", err)) .into_response() } } @@ -366,20 +355,7 @@ impl FileHandler { let file_dto = match retrieval.get_file_owned(&id, &auth_user.id).await { Ok(f) => f, Err(err) => { - let status = if err.to_string().contains("not found") - || err.to_string().contains("NotFound") - { - StatusCode::NOT_FOUND - } else { - StatusCode::INTERNAL_SERVER_ERROR - }; - return ( - status, - Json(serde_json::json!({ - "error": err.to_string() - })), - ) - .into_response(); + return AppError::from(err).into_response(); } }; @@ -526,14 +502,7 @@ impl FileHandler { .into_response(), }, Err(err) => { - tracing::error!("Error downloading file: {}", err); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": format!("Error reading file: {}", err) - })), - ) - .into_response() + AppError::from(err).into_response() } } } @@ -547,6 +516,7 @@ impl FileHandler { /// Axum-compatible handler wrapper around [`Self::list_files`]. pub async fn list_files_query( State(state): State, + auth_user: AuthUser, headers: HeaderMap, Query(params): Query>, ) -> impl IntoResponse { @@ -554,7 +524,7 @@ impl FileHandler { tracing::info!("API: Listing files with folder_id: {:?}", folder_id); let retrieval = &state.applications.file_retrieval_service; - match retrieval.list_files(folder_id).await { + match retrieval.list_files_owned(folder_id, &auth_user.id).await { Ok(files) => { // Compute lightweight ETag from max modified_at + count let max_mod = files.iter().map(|f| f.modified_at).max().unwrap_or(0); @@ -584,14 +554,7 @@ impl FileHandler { resp } Err(err) => { - tracing::error!("Error listing files: {}", err); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": format!("Error listing files: {}", err) - })), - ) - .into_response() + AppError::from(err).into_response() } } } @@ -664,23 +627,7 @@ impl FileHandler { match result { Ok(_) => StatusCode::NO_CONTENT.into_response(), - Err(err) => { - tracing::error!("Error deleting file: {}", err); - let status = if err.to_string().contains("not found") - || err.to_string().contains("NotFound") - { - StatusCode::NOT_FOUND - } else { - StatusCode::INTERNAL_SERVER_ERROR - }; - ( - status, - Json(serde_json::json!({ - "error": format!("Error deleting file: {}", err) - })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -712,25 +659,7 @@ impl FileHandler { let mgmt = &state.applications.file_management_service; match mgmt.rename_file_owned(&id, &auth_user.id, &new_name).await { Ok(file_dto) => (StatusCode::OK, Json(file_dto)).into_response(), - Err(err) => { - tracing::error!("Error renaming file: {}", err); - let status = if err.to_string().contains("not found") - || err.to_string().contains("NotFound") - { - StatusCode::NOT_FOUND - } else if err.to_string().contains("already exists") { - StatusCode::CONFLICT - } else { - StatusCode::INTERNAL_SERVER_ERROR - }; - ( - status, - Json(serde_json::json!({ - "error": format!("Error renaming file: {}", err) - })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -750,23 +679,7 @@ impl FileHandler { .await { Ok(file) => (StatusCode::OK, Json(file)).into_response(), - Err(err) => { - tracing::error!("Error moving file: {}", err); - let status = if err.to_string().contains("not found") - || err.to_string().contains("NotFound") - { - StatusCode::NOT_FOUND - } else { - StatusCode::INTERNAL_SERVER_ERROR - }; - ( - status, - Json(serde_json::json!({ - "error": format!("Error moving file: {}", err) - })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -785,16 +698,7 @@ impl FileHandler { let mgmt = &state.applications.file_management_service; match mgmt.move_file_owned(&id, &auth_user.id, folder_id).await { Ok(file_dto) => (StatusCode::OK, Json(file_dto)).into_response(), - Err(err) => { - tracing::error!("Error moving file: {}", err); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": format!("Error moving file: {}", err) - })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -831,33 +735,12 @@ impl FileHandler { /// Build error response for DomainError. fn domain_error_response(err: crate::common::errors::DomainError) -> Response { - let status = match err.kind { - crate::common::errors::ErrorKind::NotFound => StatusCode::NOT_FOUND, - crate::common::errors::ErrorKind::QuotaExceeded => StatusCode::INSUFFICIENT_STORAGE, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - Response::builder() - .status(status) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from( - serde_json::json!({ "error": format!("Error: {}", err) }).to_string(), - )) - .unwrap() + AppError::from(err).into_response() } /// Build a quota-specific error response with 507 status and structured body. fn quota_error_response(err: crate::common::errors::DomainError) -> Response { - Response::builder() - .status(StatusCode::INSUFFICIENT_STORAGE) - .header(header::CONTENT_TYPE, "application/json") - .body(Body::from( - serde_json::json!({ - "error": err.message, - "error_type": "QuotaExceeded" - }) - .to_string(), - )) - .unwrap() + AppError::from(err).into_response() } /// Build response for cached/small files. diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index 3ca2ec70..7abed479 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -18,7 +18,7 @@ use crate::application::ports::inbound::FolderUseCase; use crate::application::ports::trash_ports::TrashUseCase; use crate::application::services::folder_service::FolderService; use crate::common::di::AppState as GlobalAppState; -use crate::common::errors::ErrorKind; +use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; type AppState = Arc; @@ -69,15 +69,7 @@ impl FolderHandler { match service.create_folder(dto).await { Ok(folder) => (StatusCode::CREATED, Json(folder)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::AlreadyExists => StatusCode::CONFLICT, - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - (status, err.to_string()).into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -100,18 +92,11 @@ impl FolderHandler { id, owner ); - return (StatusCode::NOT_FOUND, "Folder not found".to_string()).into_response(); + return AppError::not_found("Folder not found").into_response(); } (StatusCode::OK, Json(folder)).into_response() } - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - (status, err.to_string()).into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -156,17 +141,7 @@ impl FolderHandler { .await { Ok(paginated_result) => (StatusCode::OK, Json(paginated_result)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - ( - status, - Json(serde_json::json!({ "error": err.to_string() })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -183,17 +158,7 @@ impl FolderHandler { .await { Ok(folders) => (StatusCode::OK, Json(folders)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - ( - status, - Json(serde_json::json!({ "error": err.to_string() })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -233,7 +198,7 @@ impl FolderHandler { // Run both queries concurrently — no sequential wait. let (folders_result, files_result) = tokio::join!( folder_service.list_folders_for_owner(Some(&id), &auth_user.id), - file_service.list_files(Some(&id)) + file_service.list_files_owned(Some(&id), &auth_user.id) ); match (folders_result, files_result) { @@ -259,17 +224,7 @@ impl FolderHandler { .insert(header::ETAG, header::HeaderValue::from_str(&etag).unwrap()); resp } - (Err(err), _) | (_, Err(err)) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - ( - status, - Json(serde_json::json!({ "error": err.to_string() })), - ) - .into_response() - } + (Err(err), _) | (_, Err(err)) => AppError::from(err).into_response() } } @@ -282,22 +237,7 @@ impl FolderHandler { ) -> impl IntoResponse { match service.rename_folder(&id, dto, &auth_user.id).await { Ok(folder) => (StatusCode::OK, Json(folder)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::AlreadyExists => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - // Return a proper JSON error response - ( - status, - Json(serde_json::json!({ - "error": err.to_string() - })), - ) - .into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -310,15 +250,7 @@ impl FolderHandler { ) -> impl IntoResponse { match service.move_folder(&id, dto, &auth_user.id).await { Ok(folder) => (StatusCode::OK, Json(folder)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::AlreadyExists => StatusCode::CONFLICT, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - (status, err.to_string()).into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -330,14 +262,7 @@ impl FolderHandler { ) -> impl IntoResponse { match service.delete_folder(&id, &auth_user.id).await { Ok(_) => StatusCode::NO_CONTENT.into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - (status, err.to_string()).into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -376,20 +301,7 @@ impl FolderHandler { StatusCode::NO_CONTENT.into_response() } Err(err) => { - tracing::error!("Error deleting folder: {}", err); - - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - ( - status, - Json(serde_json::json!({ - "error": format!("Error deleting folder: {}", err) - })), - ) - .into_response() + AppError::from(err).into_response() } } } @@ -487,30 +399,14 @@ impl FolderHandler { } Err(err) => { tracing::error!("Error creating ZIP file: {}", err); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(serde_json::json!({ - "error": format!("Error creating ZIP file: {}", err) - })), - ) + AppError::internal_error(format!("Error creating ZIP file: {}", err)) .into_response() } } } Err(err) => { tracing::error!("Folder not found: {}", err); - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - ( - status, - Json(serde_json::json!({ - "error": format!("Error finding folder: {}", err) - })), - ) - .into_response() + AppError::from(err).into_response() } } } diff --git a/src/interfaces/api/handlers/i18n_handler.rs b/src/interfaces/api/handlers/i18n_handler.rs index 3de2a614..ea1bfdd4 100644 --- a/src/interfaces/api/handlers/i18n_handler.rs +++ b/src/interfaces/api/handlers/i18n_handler.rs @@ -56,16 +56,22 @@ impl I18nHandler { (StatusCode::OK, Json(response)).into_response() } Err(err) => { - let status = match &err { - I18nError::KeyNotFound(_) => StatusCode::NOT_FOUND, - I18nError::InvalidLocale(_) => StatusCode::BAD_REQUEST, - I18nError::LoadError(_) => StatusCode::INTERNAL_SERVER_ERROR, + let (status, error_msg) = match &err { + I18nError::KeyNotFound(_) => (StatusCode::NOT_FOUND, err.to_string()), + I18nError::InvalidLocale(_) => (StatusCode::BAD_REQUEST, err.to_string()), + I18nError::LoadError(_) => { + tracing::error!("I18n load error: {}", err); + ( + StatusCode::INTERNAL_SERVER_ERROR, + "Translation loading error".to_string(), + ) + } }; let error = TranslationErrorDto { key: query.key, locale: locale.unwrap_or(Locale::default()).as_str().to_string(), - error: err.to_string(), + error: error_msg, }; (status, Json(error)).into_response() diff --git a/src/interfaces/api/handlers/recent_handler.rs b/src/interfaces/api/handlers/recent_handler.rs index 5dee0fdf..0bfde753 100644 --- a/src/interfaces/api/handlers/recent_handler.rs +++ b/src/interfaces/api/handlers/recent_handler.rs @@ -37,7 +37,7 @@ pub async fn get_recent_items( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to retrieve recent items: {}", err) + "error": "Failed to retrieve recent items" })), ) .into_response() @@ -83,7 +83,7 @@ pub async fn record_item_access( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to record access: {}", err) + "error": "Failed to record access" })), ) .into_response() @@ -129,7 +129,7 @@ pub async fn remove_from_recent( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to remove from recents: {}", err) + "error": "Failed to remove from recents" })), ) .into_response() @@ -160,7 +160,7 @@ pub async fn clear_recent_items( ( StatusCode::INTERNAL_SERVER_ERROR, Json(serde_json::json!({ - "error": format!("Failed to clear recent items: {}", err) + "error": "Failed to clear recent items" })), ) .into_response() diff --git a/src/interfaces/api/handlers/search_handler.rs b/src/interfaces/api/handlers/search_handler.rs index 0dd02e57..32ebabdb 100644 --- a/src/interfaces/api/handlers/search_handler.rs +++ b/src/interfaces/api/handlers/search_handler.rs @@ -74,7 +74,7 @@ impl SearchHandler { error!("Search error: {}", err); ( StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": format!("Search error: {}", err) })), + Json(json!({ "error": "Search error" })), ) .into_response() } @@ -115,7 +115,7 @@ impl SearchHandler { error!("Search error: {}", err); ( StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": format!("Search error: {}", err) })), + Json(json!({ "error": "Search error" })), ) .into_response() } @@ -159,7 +159,7 @@ impl SearchHandler { error!("Suggestions error: {}", err); ( StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": format!("Suggestions error: {}", err) })), + Json(json!({ "error": "Suggestions error" })), ) .into_response() } @@ -195,7 +195,7 @@ impl SearchHandler { error!("Error clearing search cache: {}", err); ( StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": format!("Error clearing search cache: {}", err) })), + Json(json!({ "error": "Error clearing search cache" })), ) .into_response() } diff --git a/src/interfaces/api/handlers/share_handler.rs b/src/interfaces/api/handlers/share_handler.rs index 46b3d82b..5c66b600 100644 --- a/src/interfaces/api/handlers/share_handler.rs +++ b/src/interfaces/api/handlers/share_handler.rs @@ -17,7 +17,8 @@ use crate::{ }, common::errors::ErrorKind, domain::entities::share::ShareItemType, - interfaces::middleware::auth::OptionalAuthUser, + interfaces::errors::AppError, + interfaces::middleware::auth::AuthUser, }; #[derive(Debug, Deserialize)] @@ -36,40 +37,27 @@ pub struct VerifyPasswordRequest { /// Create a new shared link pub async fn create_shared_link( State(share_use_case): State>, - auth_user: OptionalAuthUser, + auth_user: AuthUser, Json(dto): Json, ) -> impl IntoResponse { - let user_id = auth_user - .0 - .map(|u| u.id) - .unwrap_or_else(|| "anonymous".to_string()); - match share_use_case.create_shared_link(&user_id, dto).await { + match share_use_case + .create_shared_link(&auth_user.id, dto) + .await + { Ok(share) => (StatusCode::CREATED, Json(share)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::InvalidInput => StatusCode::BAD_REQUEST, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - (status, Json(json!({ "error": err.to_string() }))).into_response() - } + Err(err) => AppError::from(err).into_response() } } /// Get information about a specific shared link by ID pub async fn get_shared_link( State(share_use_case): State>, + auth_user: AuthUser, Path(id): Path, ) -> impl IntoResponse { - match share_use_case.get_shared_link(&id).await { + match share_use_case.get_shared_link(&id, &auth_user.id).await { Ok(share) => (StatusCode::OK, Json(share)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - (status, Json(json!({ "error": err.to_string() }))).into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -77,13 +65,10 @@ pub async fn get_shared_link( /// Supports optional filtering by item_id + item_type query params. pub async fn get_user_shares( State(share_use_case): State>, - auth_user: OptionalAuthUser, + auth_user: AuthUser, Query(query): Query, ) -> impl IntoResponse { - let _user_id = auth_user - .0 - .map(|u| u.id) - .unwrap_or_else(|| "anonymous".to_string()); + let user_id = &auth_user.id; // If both item_id and item_type are provided, return shares for that specific item if let (Some(item_id), Some(item_type_str)) = (&query.item_id, &query.item_type) { @@ -98,15 +83,11 @@ pub async fn get_user_shares( } }; return match share_use_case - .get_shared_links_for_item(item_id, &item_type) + .get_shared_links_for_item(item_id, &item_type, user_id) .await { Ok(shares) => (StatusCode::OK, Json(shares)).into_response(), - Err(err) => ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": err.to_string() })), - ) - .into_response(), + Err(err) => AppError::from(err).into_response(), }; } @@ -115,53 +96,42 @@ pub async fn get_user_shares( let per_page = query.per_page.unwrap_or(20); match share_use_case - .get_user_shared_links(&_user_id, page, per_page) + .get_user_shared_links(user_id, page, per_page) .await { Ok(shares) => (StatusCode::OK, Json(shares)).into_response(), - Err(err) => ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ "error": err.to_string() })), - ) - .into_response(), + Err(err) => AppError::from(err).into_response(), } } /// Update a shared link's properties pub async fn update_shared_link( State(share_use_case): State>, + auth_user: AuthUser, Path(id): Path, Json(dto): Json, ) -> impl IntoResponse { - match share_use_case.update_shared_link(&id, dto).await { + match share_use_case + .update_shared_link(&id, &auth_user.id, dto) + .await + { Ok(share) => (StatusCode::OK, Json(share)).into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::AccessDenied => StatusCode::FORBIDDEN, - ErrorKind::InvalidInput => StatusCode::BAD_REQUEST, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - (status, Json(json!({ "error": err.to_string() }))).into_response() - } + Err(err) => AppError::from(err).into_response() } } /// Delete a shared link pub async fn delete_shared_link( State(share_use_case): State>, + auth_user: AuthUser, Path(id): Path, ) -> impl IntoResponse { - match share_use_case.delete_shared_link(&id).await { + match share_use_case + .delete_shared_link(&id, &auth_user.id) + .await + { Ok(_) => StatusCode::NO_CONTENT.into_response(), - Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::AccessDenied => StatusCode::FORBIDDEN, - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - (status, Json(json!({ "error": err.to_string() }))).into_response() - } + Err(err) => AppError::from(err).into_response() } } @@ -177,28 +147,24 @@ pub async fn access_shared_item( match share_use_case.get_shared_link_by_token(&token).await { Ok(item) => (StatusCode::OK, Json(item)).into_response(), Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::AccessDenied => { - if err.message.contains("expired") { - StatusCode::GONE // HTTP 410 Gone for expired links - } else if err.message.contains("password") { - return ( - StatusCode::UNAUTHORIZED, - Json(json!({ - "error": "Password required", - "requiresPassword": true - })), - ) - .into_response(); - } else { - StatusCode::FORBIDDEN - } + // Special handling for share access errors + if err.kind == ErrorKind::AccessDenied { + if err.message.contains("password") { + return ( + StatusCode::UNAUTHORIZED, + Json(json!({ + "error": "Password required", + "requiresPassword": true + })), + ) + .into_response(); } - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - - (status, Json(json!({ "error": err.to_string() }))).into_response() + if err.message.contains("expired") { + return AppError::new(StatusCode::GONE, err.message, "Expired") + .into_response(); + } + } + AppError::from(err).into_response() } } } @@ -215,20 +181,16 @@ pub async fn verify_shared_item_password( { Ok(item) => (StatusCode::OK, Json(item)).into_response(), Err(err) => { - let status = match err.kind { - ErrorKind::NotFound => StatusCode::NOT_FOUND, - ErrorKind::AccessDenied => { - if err.message.contains("expired") { - StatusCode::GONE - } else if err.message.contains("password") { - StatusCode::UNAUTHORIZED - } else { - StatusCode::FORBIDDEN - } + if err.kind == ErrorKind::AccessDenied { + if err.message.contains("expired") { + return AppError::new(StatusCode::GONE, err.message, "Expired") + .into_response(); } - _ => StatusCode::INTERNAL_SERVER_ERROR, - }; - (status, Json(json!({ "error": err.to_string() }))).into_response() + if err.message.contains("password") { + return AppError::unauthorized("Invalid password").into_response(); + } + } + AppError::from(err).into_response() } } } diff --git a/src/interfaces/api/handlers/trash_handler.rs b/src/interfaces/api/handlers/trash_handler.rs index 187b6446..1bffc6d0 100644 --- a/src/interfaces/api/handlers/trash_handler.rs +++ b/src/interfaces/api/handlers/trash_handler.rs @@ -6,7 +6,7 @@ use tracing::{debug, error, instrument, warn}; use crate::application::ports::trash_ports::TrashUseCase; use crate::common::di::AppState; -use crate::interfaces::middleware::auth::{AuthUser, OptionalAuthUser}; +use crate::interfaces::middleware::auth::AuthUser; use std::sync::Arc; /// Gets all items in the trash for the current user @@ -46,61 +46,7 @@ pub async fn get_trash_items( ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ - "error": format!("Error retrieving trash items: {}", e) - })), - ) - } - } -} - -/// Moves an item (file or folder) to the trash (generic function, not used directly in routes) -#[instrument(skip_all)] -pub async fn move_to_trash( - State(state): State>, - OptionalAuthUser(auth_user): OptionalAuthUser, - Path((item_type, item_id)): Path<(String, String)>, -) -> (StatusCode, Json) { - let user_id = auth_user - .as_ref() - .map(|u| u.id.as_str()) - .unwrap_or("anonymous"); - debug!( - "Request to move to trash: type={}, id={}, user={}", - item_type, item_id, user_id - ); - - let trash_service = match state.trash_service.as_ref() { - Some(service) => service, - None => { - return ( - StatusCode::NOT_IMPLEMENTED, - Json(json!({ - "error": "Trash feature is not enabled" - })), - ); - } - }; - let result = trash_service - .move_to_trash(&item_id, &item_type, user_id) - .await; - - match result { - Ok(_) => { - debug!("Item moved to trash successfully"); - ( - StatusCode::OK, - Json(json!({ - "success": true, - "message": "Item moved to trash successfully" - })), - ) - } - Err(e) => { - error!("Error moving item to trash: {:?}", e); - ( - StatusCode::INTERNAL_SERVER_ERROR, - Json(json!({ - "error": format!("Error moving item to trash: {}", e) + "error": "Error retrieving trash items" })), ) } @@ -111,13 +57,10 @@ pub async fn move_to_trash( #[instrument(skip_all)] pub async fn move_file_to_trash( State(state): State>, - OptionalAuthUser(auth_user): OptionalAuthUser, + auth_user: AuthUser, Path(item_id): Path, ) -> (StatusCode, Json) { - let user_id = auth_user - .as_ref() - .map(|u| u.id.as_str()) - .unwrap_or("anonymous"); + let user_id = &auth_user.id; debug!( "Request to move file to trash: id={}, user={}", item_id, user_id @@ -154,7 +97,7 @@ pub async fn move_file_to_trash( ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ - "error": format!("Error moving file to trash: {}", e) + "error": "Error moving file to trash" })), ) } @@ -165,13 +108,10 @@ pub async fn move_file_to_trash( #[instrument(skip_all)] pub async fn move_folder_to_trash( State(state): State>, - OptionalAuthUser(auth_user): OptionalAuthUser, + auth_user: AuthUser, Path(item_id): Path, ) -> (StatusCode, Json) { - let user_id = auth_user - .as_ref() - .map(|u| u.id.as_str()) - .unwrap_or("anonymous"); + let user_id = &auth_user.id; debug!( "Request to move folder to trash: id={}, user={}", item_id, user_id @@ -210,7 +150,7 @@ pub async fn move_folder_to_trash( ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ - "error": format!("Error moving folder to trash: {}", e) + "error": "Error moving folder to trash" })), ) } @@ -271,7 +211,7 @@ pub async fn restore_from_trash( ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ - "error": format!("Error restoring item from trash: {}", e) + "error": "Error restoring item from trash" })), ) } @@ -334,7 +274,7 @@ pub async fn delete_permanently( ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ - "error": format!("Error deleting item permanently: {}", e) + "error": "Error deleting item permanently" })), ) } @@ -378,7 +318,7 @@ pub async fn empty_trash( ( StatusCode::INTERNAL_SERVER_ERROR, Json(json!({ - "error": format!("Error emptying trash: {}", e) + "error": "Error emptying trash" })), ) } diff --git a/src/interfaces/api/handlers/wopi_handler.rs b/src/interfaces/api/handlers/wopi_handler.rs index d1f4ad1c..9aa545a5 100644 --- a/src/interfaces/api/handlers/wopi_handler.rs +++ b/src/interfaces/api/handlers/wopi_handler.rs @@ -394,6 +394,7 @@ pub async fn get_editor_url( AuthUser { id: user_id, username, + .. }: AuthUser, Query(params): Query, State(state): State, diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index 13e5ad45..2f9f4a32 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -288,10 +288,9 @@ pub fn create_api_routes(app_state: &Arc) -> Router> { "/blob/{hash}", get(super::handlers::dedup_handler::DedupHandler::get_blob), ) - .route( - "/blob/{hash}", - delete(super::handlers::dedup_handler::DedupHandler::remove_reference), - ) + // NOTE: remove_reference is intentionally NOT exposed as a public + // endpoint — ref_count management is an internal concern handled + // automatically when files are deleted via the file API. .route( "/recalculate", post(super::handlers::dedup_handler::DedupHandler::recalculate_stats), diff --git a/src/interfaces/errors.rs b/src/interfaces/errors.rs index 9e55ef92..e630f7b7 100644 --- a/src/interfaces/errors.rs +++ b/src/interfaces/errors.rs @@ -22,9 +22,14 @@ pub struct AppError { } /// JSON response structure for errors. +/// +/// Both `error` and `message` carry the same content for backwards compatibility: +/// - Legacy ad-hoc handlers returned `{"error": "..."}` (frontend reads `.error`) +/// - AppError returned `{"message": "..."}` (admin panel reads `.message`) #[derive(Serialize)] pub struct ErrorResponse { pub status: String, + pub error: String, pub message: String, pub error_type: String, } @@ -133,9 +138,26 @@ impl From for AppError { impl IntoResponse for AppError { fn into_response(self) -> Response { let status = self.status_code; + + // Sanitize 500 Internal Server Error to prevent information leakage. + // Log the full error server-side for debugging, return a generic + // message to the client. Other status codes (including 5xx like + // 501, 503, 507) keep their intentionally user-facing messages. + let client_message = if status == StatusCode::INTERNAL_SERVER_ERROR { + tracing::error!( + error_type = %self.error_type, + "Internal server error: {}", + self.message + ); + "An internal error occurred. Please try again later.".to_string() + } else { + self.message + }; + let error_response = ErrorResponse { status: status.to_string(), - message: self.message, + error: client_message.clone(), + message: client_message, error_type: self.error_type, }; diff --git a/src/interfaces/middleware/auth.rs b/src/interfaces/middleware/auth.rs index 1881671a..91ed111f 100644 --- a/src/interfaces/middleware/auth.rs +++ b/src/interfaces/middleware/auth.rs @@ -24,6 +24,7 @@ pub struct CookieAuthenticated; pub struct AuthUser { pub id: String, pub username: String, + pub role: String, } /// Reusable extractor that gets the user_id of the authenticated user. @@ -50,6 +51,7 @@ where .map(|cu| AuthUser { id: cu.id.clone(), username: cu.username.clone(), + role: cu.role.clone(), }) .ok_or(AuthError::UserNotFound) } @@ -108,6 +110,7 @@ where |cu| AuthUser { id: cu.id.clone(), username: cu.username.clone(), + role: cu.role.clone(), }, ))) } diff --git a/src/main.rs b/src/main.rs index f7516e26..90a39c4a 100644 --- a/src/main.rs +++ b/src/main.rs @@ -354,16 +354,14 @@ async fn main() -> Result<(), Box> { app = app .layer(SetResponseHeaderLayer::overriding( HeaderName::from_static("content-security-policy"), - // NOTE: script-src includes 'unsafe-inline' because several HTML - // pages still use inline event handlers (onclick, onsubmit) and - // + @@ -41,9 +41,9 @@
- - - + + +
@@ -79,7 +79,7 @@
- +
Public registration is disabled. Only admins can create new users.
@@ -87,7 +87,7 @@
-

User Management

+

User Management

@@ -107,8 +107,8 @@ @@ -132,7 +132,7 @@ OpenID Connect issuer URL of your identity provider
- +
@@ -146,7 +146,7 @@
-
—
+
—
Advanced Settings @@ -170,8 +170,8 @@
This will prevent ALL password-based logins!
- - + +
@@ -195,8 +195,8 @@ Set to 0 for unlimited
- - + +
@@ -235,8 +235,8 @@
- - + +
@@ -253,8 +253,8 @@
- - + +
diff --git a/static/css/base/reset.css b/static/css/base/reset.css index 56125332..227c2a59 100644 --- a/static/css/base/reset.css +++ b/static/css/base/reset.css @@ -20,3 +20,5 @@ html[dir='rtl'] .fa-sign-out-alt { -webkit-transform: rotate(180deg); transform: rotate(180deg); } +/* Utility: hide elements without inline style="" (CSP-safe) */ +.hidden { display: none; } \ No newline at end of file diff --git a/static/css/components/spinner.css b/static/css/components/spinner.css index d1c35908..5e3faf61 100644 --- a/static/css/components/spinner.css +++ b/static/css/components/spinner.css @@ -40,6 +40,11 @@ display: none; } +.dropzone-icon { + font-size: 32px; + margin-bottom: 10px; +} + .dropzone.active { border-color: #ff5e3a; background-color: rgba(255, 94, 58, 0.05); diff --git a/static/css/components/uploadDropdown.css b/static/css/components/uploadDropdown.css index ed0f89d9..72b73149 100644 --- a/static/css/components/uploadDropdown.css +++ b/static/css/components/uploadDropdown.css @@ -95,3 +95,8 @@ [data-theme="dark"] .upload-dropdown-item i { color: #94a3b8; } + +.upload-caret { + margin-left: 4px; + font-size: 12px; +} diff --git a/static/css/layout/sidebar.css b/static/css/layout/sidebar.css index b7f19d74..a6ec7dd0 100644 --- a/static/css/layout/sidebar.css +++ b/static/css/layout/sidebar.css @@ -61,6 +61,8 @@ align-items: center; border-bottom: 1px solid rgba(255,255,255,0.07); margin-bottom: 8px; + text-decoration: none; + color: inherit; } .logo { diff --git a/static/css/views/device-verify.css b/static/css/views/device-verify.css new file mode 100644 index 00000000..f7dbcc12 --- /dev/null +++ b/static/css/views/device-verify.css @@ -0,0 +1,89 @@ +/* device-verify.css — stand-alone styles for the device authorization page */ +:root { + --primary: #2563eb; + --primary-hover: #1d4ed8; + --danger: #dc2626; + --danger-hover: #b91c1c; + --success: #16a34a; + --bg: #f8fafc; + --card: #ffffff; + --text: #1e293b; + --muted: #64748b; + --border: #e2e8f0; + --radius: 12px; +} +* { box-sizing: border-box; margin: 0; padding: 0; } +body { + font-family: -apple-system, BlinkMacSystemFont, 'Segoe UI', Roboto, sans-serif; + background: var(--bg); + color: var(--text); + display: flex; + justify-content: center; + align-items: center; + min-height: 100vh; + padding: 1rem; +} +.card { + background: var(--card); + border-radius: var(--radius); + box-shadow: 0 4px 24px rgba(0,0,0,0.08); + padding: 2.5rem; + max-width: 440px; + width: 100%; +} +.logo { text-align: center; margin-bottom: 1.5rem; } +.logo h1 { font-size: 1.5rem; font-weight: 700; } +.logo span { color: var(--primary); } +h2 { font-size: 1.15rem; margin-bottom: 0.5rem; } +p.subtitle { color: var(--muted); font-size: 0.9rem; margin-bottom: 1.5rem; } +label { display: block; font-weight: 600; font-size: 0.85rem; margin-bottom: 0.4rem; } +input[type="text"] { + width: 100%; + padding: 0.75rem 1rem; + font-size: 1.4rem; + letter-spacing: 0.15em; + text-align: center; + text-transform: uppercase; + border: 2px solid var(--border); + border-radius: 8px; + outline: none; + transition: border-color 0.2s; +} +input[type="text"]:focus { border-color: var(--primary); } +.device-info { + background: #f1f5f9; + border-radius: 8px; + padding: 1rem; + margin: 1rem 0; +} +.device-info .row { display: flex; justify-content: space-between; margin-bottom: 0.3rem; } +.device-info .label { color: var(--muted); font-size: 0.85rem; } +.device-info .value { font-weight: 600; font-size: 0.85rem; } +.actions { display: flex; gap: 0.75rem; margin-top: 1.25rem; } +button { + flex: 1; + padding: 0.75rem; + border: none; + border-radius: 8px; + font-size: 0.95rem; + font-weight: 600; + cursor: pointer; + transition: background 0.2s; +} +.btn-approve { background: var(--primary); color: #fff; } +.btn-approve:hover { background: var(--primary-hover); } +.btn-deny { background: var(--danger); color: #fff; } +.btn-deny:hover { background: var(--danger-hover); } +button:disabled { opacity: 0.5; cursor: not-allowed; } +.status { + text-align: center; + padding: 1rem; + border-radius: 8px; + margin-top: 1rem; + font-weight: 600; +} +.status.success { background: #dcfce7; color: var(--success); } +.status.denied { background: #fef2f2; color: var(--danger); } +.status.error { background: #fef2f2; color: var(--danger); } +.error-text { color: var(--danger); font-size: 0.85rem; margin-top: 0.5rem; } +.hidden { display: none !important; } diff --git a/static/device-verify.html b/static/device-verify.html index ccd03345..9340979a 100644 --- a/static/device-verify.html +++ b/static/device-verify.html @@ -4,97 +4,7 @@ OxiCloud — Authorize Device - +
@@ -108,9 +18,9 @@

Enter the code displayed on your WebDAV/CalDAV client to grant access.

-
+ -
+ - -
+ -
+ -
+
- + diff --git a/static/index.html b/static/index.html index 1ddbe404..cf098ace 100644 --- a/static/index.html +++ b/static/index.html @@ -5,7 +5,7 @@ OxiCloud - + @@ -47,22 +47,14 @@ - +