From b9d10c7b5cc878b0b5789603e60b174fbb755b1e Mon Sep 17 00:00:00 2001 From: Dionisio Date: Sat, 7 Feb 2026 04:02:38 +0100 Subject: [PATCH] fix --- src/application/services/trash_service.rs | 37 +++++++++-- .../repositories/file_fs_repository_trash.rs | 9 +-- .../api/handlers/favorites_handler.rs | 13 ++-- src/interfaces/api/handlers/file_handler.rs | 8 ++- src/interfaces/api/handlers/folder_handler.rs | 4 +- src/interfaces/api/handlers/recent_handler.rs | 17 ++--- src/interfaces/api/routes.rs | 41 ++++++------ src/interfaces/middleware/auth.rs | 66 +++++++++++++------ 8 files changed, 128 insertions(+), 67 deletions(-) diff --git a/src/application/services/trash_service.rs b/src/application/services/trash_service.rs index eff56eec..7a424c36 100644 --- a/src/application/services/trash_service.rs +++ b/src/application/services/trash_service.rs @@ -72,13 +72,38 @@ impl TrashService { } } - /// Validates user permissions over an item + /// Validates that the given user owns the trashed item. + /// Returns an error if the item does not exist or belongs to a different user. #[instrument(skip(self))] - async fn validate_user_ownership(&self, _item_id: &str, _user_id: &str) -> Result<()> { - // Here we would implement permission validation - // For now, we simply return Ok since we don't have a complete - // implementation of user permissions - Ok(()) + async fn validate_user_ownership(&self, item_id: &str, user_id: &str) -> Result<()> { + let item_uuid = Uuid::parse_str(item_id) + .map_err(|e| DomainError::validation_error(format!("Invalid item ID: {}", e)))?; + let user_uuid = Uuid::parse_str(user_id) + .map_err(|e| DomainError::validation_error(format!("Invalid user ID: {}", e)))?; + + match self.trash_repository.get_trash_item(&item_uuid, &user_uuid).await? { + Some(item) => { + if item.user_id != user_uuid { + error!( + "User {} attempted to access trash item {} owned by {}", + user_id, item_id, item.user_id + ); + return Err(DomainError::access_denied( + "TrashItem", + "You do not have permission to access this trash item", + )); + } + Ok(()) + } + None => { + // Item not found for this user — treat as authorization error + // to avoid leaking existence information + Err(DomainError::not_found( + "TrashItem", + format!("{} (user: {})", item_id, user_id), + )) + } + } } } diff --git a/src/infrastructure/repositories/file_fs_repository_trash.rs b/src/infrastructure/repositories/file_fs_repository_trash.rs index 68cc6cd3..6eb5b903 100644 --- a/src/infrastructure/repositories/file_fs_repository_trash.rs +++ b/src/infrastructure/repositories/file_fs_repository_trash.rs @@ -26,9 +26,10 @@ impl FileFsRepository { debug!("User-specific trash directory: {}", user_trash_dir.display()); user_trash_dir } else { - // Use a default user directory if not specified - let default_dir = base_trash_dir.join("00000000-0000-0000-0000-000000000000"); - debug!("Default user trash directory: {}", default_dir.display()); + // No user ID provided - this should not happen in production + tracing::warn!("No user_id provided for trash directory, this indicates a bug"); + let default_dir = base_trash_dir.join("unknown-user"); + debug!("Fallback user trash directory: {}", default_dir.display()); default_dir } } @@ -38,7 +39,7 @@ impl FileFsRepository { debug!("Creating trash file path for file ID: {}", file_id); // Get the trash directory for the default user - let user_trash_dir = self.get_user_trash_dir(Some("00000000-0000-0000-0000-000000000000")); + let user_trash_dir = self.get_user_trash_dir(None); // Ensure the user's trash directory exists debug!("Ensuring user trash directory exists: {}", user_trash_dir.display()); diff --git a/src/interfaces/api/handlers/favorites_handler.rs b/src/interfaces/api/handlers/favorites_handler.rs index 7a2f2974..da83c30b 100644 --- a/src/interfaces/api/handlers/favorites_handler.rs +++ b/src/interfaces/api/handlers/favorites_handler.rs @@ -8,13 +8,14 @@ use axum::{ use tracing::{error, info}; use crate::application::ports::favorites_ports::FavoritesUseCase; +use crate::interfaces::middleware::auth::AuthUser; /// Handler for favorite-related API endpoints pub async fn get_favorites( State(favorites_service): State>, + auth_user: AuthUser, ) -> impl IntoResponse { - // For demo purposes, we're using a fixed user ID - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; match favorites_service.get_favorites(user_id).await { Ok(favorites) => { @@ -36,10 +37,10 @@ pub async fn get_favorites( /// Add an item to user's favorites pub async fn add_favorite( State(favorites_service): State>, + auth_user: AuthUser, Path((item_type, item_id)): Path<(String, String)>, ) -> impl IntoResponse { - // For demo purposes, we're using a fixed user ID - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; // Validate item_type if item_type != "file" && item_type != "folder" { @@ -76,10 +77,10 @@ pub async fn add_favorite( /// Remove an item from user's favorites pub async fn remove_favorite( State(favorites_service): State>, + auth_user: AuthUser, Path((item_type, item_id)): Path<(String, String)>, ) -> impl IntoResponse { - // For demo purposes, we're using a fixed user ID - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; match favorites_service.remove_from_favorites(user_id, &item_id, &item_type).await { Ok(removed) => { diff --git a/src/interfaces/api/handlers/file_handler.rs b/src/interfaces/api/handlers/file_handler.rs index 68482e52..f0ebec11 100644 --- a/src/interfaces/api/handlers/file_handler.rs +++ b/src/interfaces/api/handlers/file_handler.rs @@ -16,6 +16,7 @@ use crate::infrastructure::services::compression_service::{ CompressionService, GzipCompressionService, CompressionLevel }; use crate::common::di::AppState; +use crate::interfaces::middleware::auth::CurrentUserId; /** * Type aliases for dependency injection state. @@ -1068,6 +1069,7 @@ impl FileHandler { /// 3. Decrement dedup reference count for the content hash pub async fn delete_file( State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path, ) -> impl IntoResponse { let dedup_service = &state.core.dedup_service; @@ -1097,12 +1099,12 @@ impl FileHandler { // Debug logs to track trash components tracing::debug!("Trash service type: {}", std::any::type_name_of_val(&*trash_service)); - let default_user_id = "00000000-0000-0000-0000-000000000000".to_string(); - tracing::info!("Using default user ID: {}", default_user_id); + // User ID extracted from authenticated token via CurrentUserId + tracing::info!("Using authenticated user ID: {}", user_id); // Try to move to trash first - add more detailed logging tracing::info!("About to call trash_service.move_to_trash with id={}, type=file", id); - match trash_service.move_to_trash(&id, "file", &default_user_id).await { + match trash_service.move_to_trash(&id, "file", &user_id).await { Ok(_) => { tracing::info!("File successfully moved to trash: {}", id); diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index 324d0343..b22dc330 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -174,7 +174,7 @@ impl FolderHandler { /// Deletes a folder with trash functionality pub async fn delete_folder_with_trash( State(state): State, - _auth_user: AuthUser, + auth_user: AuthUser, Path(id): Path, ) -> impl IntoResponse { // Check if trash service is available @@ -182,7 +182,7 @@ impl FolderHandler { tracing::info!("Moving folder to trash: {}", id); // Try to move to trash first - match trash_service.move_to_trash(&id, "folder", &"00000000-0000-0000-0000-000000000000".to_string()).await { + match trash_service.move_to_trash(&id, "folder", &auth_user.id).await { Ok(_) => { tracing::info!("Folder successfully moved to trash: {}", id); return StatusCode::NO_CONTENT.into_response(); diff --git a/src/interfaces/api/handlers/recent_handler.rs b/src/interfaces/api/handlers/recent_handler.rs index 39edadf1..e6d4e097 100644 --- a/src/interfaces/api/handlers/recent_handler.rs +++ b/src/interfaces/api/handlers/recent_handler.rs @@ -9,6 +9,7 @@ use serde::Deserialize; use tracing::{error, info}; use crate::application::ports::recent_ports::RecentItemsUseCase; +use crate::interfaces::middleware::auth::AuthUser; /// Parámetros de consulta para obtener elementos recientes #[derive(Deserialize)] @@ -20,10 +21,10 @@ pub struct GetRecentParams { /// Obtener elementos recientes del usuario pub async fn get_recent_items( State(recent_service): State>, + auth_user: AuthUser, Query(params): Query, ) -> impl IntoResponse { - // Para pruebas, usando ID de usuario fijo - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; match recent_service.get_recent_items(user_id, params.limit).await { Ok(items) => { @@ -45,10 +46,10 @@ pub async fn get_recent_items( /// Registrar acceso a un elemento pub async fn record_item_access( State(recent_service): State>, + auth_user: AuthUser, Path((item_type, item_id)): Path<(String, String)>, ) -> impl IntoResponse { - // Para pruebas, usando ID de usuario fijo - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; // Validar tipo de elemento if item_type != "file" && item_type != "folder" { @@ -85,10 +86,10 @@ pub async fn record_item_access( /// Eliminar un elemento de recientes pub async fn remove_from_recent( State(recent_service): State>, + auth_user: AuthUser, Path((item_type, item_id)): Path<(String, String)>, ) -> impl IntoResponse { - // Para pruebas, usando ID de usuario fijo - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; match recent_service.remove_from_recent(user_id, &item_id, &item_type).await { Ok(removed) => { @@ -125,9 +126,9 @@ pub async fn remove_from_recent( /// Limpiar todos los elementos recientes pub async fn clear_recent_items( State(recent_service): State>, + auth_user: AuthUser, ) -> impl IntoResponse { - // Para pruebas, usando ID de usuario fijo - let user_id = "00000000-0000-0000-0000-000000000000"; + let user_id = &auth_user.id; match recent_service.clear_recent_items(user_id).await { Ok(_) => { diff --git a/src/interfaces/api/routes.rs b/src/interfaces/api/routes.rs index d9925873..90b68be3 100644 --- a/src/interfaces/api/routes.rs +++ b/src/interfaces/api/routes.rs @@ -15,6 +15,7 @@ use tower_http::{ use serde_json::json; use crate::common::config::AppConfig; use crate::common::di::AppState; +use crate::interfaces::middleware::auth::CurrentUserId; use crate::interfaces::middleware::cache::{HttpCache, start_cache_cleanup_task}; @@ -248,14 +249,14 @@ pub fn create_api_routes( let folders_ops_router = Router::new() .route("/{id}", delete(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path | async move { // Try to use trash service if available if let Some(trash_service) = &state.trash_service { tracing::info!("Moving folder to trash: {}", id); - let default_user = "default".to_string(); - match trash_service.move_to_trash(&id, "folder", &default_user).await { + match trash_service.move_to_trash(&id, "folder", &user_id).await { Ok(_) => { tracing::info!("Folder successfully moved to trash: {}", id); return StatusCode::NO_CONTENT.into_response(); @@ -360,10 +361,11 @@ pub fn create_api_routes( // Uses the correct URL pattern .route("/{id}", delete(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path | async move { tracing::info!("File delete route called explicitly for ID: {}", id); - FileHandler::delete_file(State(state), Path(id)).await + FileHandler::delete_file(State(state), CurrentUserId(user_id), Path(id)).await })) .route("/{id}/move", put(| State(state): State, @@ -518,19 +520,20 @@ pub fn create_api_routes( // Get all trash items .route("/", get(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Query(params): Query> | async move { tracing::info!("Getting trash items"); // Use a valid UUID for the default user or from query params - let default_user = params.get("userId") - .unwrap_or(&"00000000-0000-0000-0000-000000000000".to_string()) - .to_string(); + let effective_user = params.get("userId") + .cloned() + .unwrap_or(user_id); - tracing::info!("Using user ID: {}", default_user); + tracing::info!("Using user ID: {}", effective_user); // Get the trash service directly if let Some(trash_service) = &state.trash_service { // Get trash items for default user - match trash_service.get_trash_items(&default_user).await { + match trash_service.get_trash_items(&effective_user).await { Ok(items) => { tracing::info!("Found {} items in trash", items.len()); let response_data = serde_json::json!(items); @@ -554,13 +557,13 @@ pub fn create_api_routes( // Move file to trash .route("/files/{id}", delete(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path | async move { tracing::info!("Moving file to trash: {}", id); - let default_user = "00000000-0000-0000-0000-000000000000".to_string(); if let Some(trash_service) = &state.trash_service { - match trash_service.move_to_trash(&id, "file", &default_user).await { + match trash_service.move_to_trash(&id, "file", &user_id).await { Ok(_) => { tracing::info!("File moved to trash successfully"); (StatusCode::OK, Json(json!({ @@ -585,13 +588,13 @@ pub fn create_api_routes( // Move folder to trash .route("/folders/{id}", delete(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path | async move { tracing::info!("Moving folder to trash: {}", id); - let default_user = "00000000-0000-0000-0000-000000000000".to_string(); if let Some(trash_service) = &state.trash_service { - match trash_service.move_to_trash(&id, "folder", &default_user).await { + match trash_service.move_to_trash(&id, "folder", &user_id).await { Ok(_) => { tracing::info!("Folder moved to trash successfully"); (StatusCode::OK, Json(json!({ @@ -616,13 +619,13 @@ pub fn create_api_routes( // Restore item from trash .route("/{id}/restore", post(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path | async move { tracing::info!("Restoring item from trash: {}", id); - let default_user = "00000000-0000-0000-0000-000000000000".to_string(); if let Some(trash_service) = &state.trash_service { - match trash_service.restore_item(&id, &default_user).await { + match trash_service.restore_item(&id, &user_id).await { Ok(_) => { tracing::info!("Item restored from trash successfully"); (StatusCode::OK, Json(json!({ @@ -658,13 +661,13 @@ pub fn create_api_routes( // Permanently delete an item from trash .route("/{id}", delete(| State(state): State, + CurrentUserId(user_id): CurrentUserId, Path(id): Path | async move { tracing::info!("Permanently deleting item from trash: {}", id); - let default_user = "00000000-0000-0000-0000-000000000000".to_string(); if let Some(trash_service) = &state.trash_service { - match trash_service.delete_permanently(&id, &default_user).await { + match trash_service.delete_permanently(&id, &user_id).await { Ok(_) => { tracing::info!("Item permanently deleted successfully"); (StatusCode::OK, Json(json!({ @@ -699,13 +702,13 @@ pub fn create_api_routes( })) // Empty trash .route("/empty", delete(| - State(state): State + State(state): State, + CurrentUserId(user_id): CurrentUserId, | async move { tracing::info!("Emptying trash"); - let default_user = "00000000-0000-0000-0000-000000000000".to_string(); if let Some(trash_service) = &state.trash_service { - match trash_service.empty_trash(&default_user).await { + match trash_service.empty_trash(&user_id).await { Ok(_) => { tracing::info!("Trash emptied successfully"); (StatusCode::OK, Json(json!({ diff --git a/src/interfaces/middleware/auth.rs b/src/interfaces/middleware/auth.rs index 9948e543..8054f78b 100644 --- a/src/interfaces/middleware/auth.rs +++ b/src/interfaces/middleware/auth.rs @@ -1,10 +1,9 @@ use std::sync::Arc; use axum::{ - extract::{State, Request}, - http::{StatusCode, HeaderMap, header}, + extract::{State, Request, FromRequestParts}, + http::{StatusCode, HeaderMap, header, request::Parts}, middleware::Next, response::{Response, IntoResponse}, - body::Body, }; use crate::common::di::AppState; @@ -19,6 +18,51 @@ pub struct AuthUser { pub username: String, } +/// Extractor reutilizable que obtiene el user_id del usuario autenticado. +/// Se extrae automáticamente del `CurrentUser` insertado por el auth middleware. +/// +/// Uso en handlers: +/// ```rust +/// async fn my_handler(CurrentUserId(user_id): CurrentUserId) -> impl IntoResponse { ... } +/// ``` +#[derive(Clone, Debug)] +pub struct CurrentUserId(pub String); + +// Implementar FromRequestParts para AuthUser — permite usar `auth_user: AuthUser` en handlers +impl FromRequestParts for AuthUser +where + S: Send + Sync, +{ + type Rejection = AuthError; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + parts + .extensions + .get::() + .map(|cu| AuthUser { + id: cu.id.clone(), + username: cu.username.clone(), + }) + .ok_or(AuthError::UserNotFound) + } +} + +// Implementar FromRequestParts para CurrentUserId — extractor ligero solo para el user_id +impl FromRequestParts for CurrentUserId +where + S: Send + Sync, +{ + type Rejection = AuthError; + + async fn from_request_parts(parts: &mut Parts, _state: &S) -> Result { + parts + .extensions + .get::() + .map(|cu| CurrentUserId(cu.id.clone())) + .ok_or(AuthError::UserNotFound) + } +} + // Error para las operaciones de autenticación #[derive(Debug, thiserror::Error)] pub enum AuthError { @@ -56,22 +100,6 @@ impl IntoResponse for AuthError { } } -// Implementamos el extractor para AuthUser -// Use a function instead of an extractor for now -// We'll use this directly in handlers until we solve the extractor lifetime issues -pub async fn get_auth_user(req: &Request) -> Result { - // Get the current user from extensions - if let Some(current_user) = req.extensions().get::() { - return Ok(AuthUser { - id: current_user.id.clone(), - username: current_user.username.clone(), - }); - } - - // Return error if user not found - Err(AuthError::UserNotFound) -} - // Middleware de autenticación simplificado - solo valida si existe un token pub async fn auth_middleware( State(state): State>,