diff --git a/src/application/ports/share_ports.rs b/src/application/ports/share_ports.rs index e8ede9aa..eb7302ba 100644 --- a/src/application/ports/share_ports.rs +++ b/src/application/ports/share_ports.rs @@ -52,12 +52,14 @@ pub trait ShareUseCase: Send + Sync + 'static { per_page: usize, ) -> Result, DomainError>; - /// Verify a password for a password-protected shared link + /// Verify a password for a password-protected shared link. + /// On success, returns the full share metadata (`ShareDto`). + /// On failure (wrong password), returns `AccessDenied`. async fn verify_shared_link_password( &self, token: &str, password: &str, - ) -> Result; + ) -> Result; /// Register an access to a shared link async fn register_shared_link_access(&self, token: &str) -> Result<(), DomainError>; diff --git a/src/application/services/admin_settings_service.rs b/src/application/services/admin_settings_service.rs index 8253930e..9604af49 100644 --- a/src/application/services/admin_settings_service.rs +++ b/src/application/services/admin_settings_service.rs @@ -378,6 +378,21 @@ impl AdminSettingsService { .await } + /// Atomically try to claim system initialization. + /// + /// Returns `Ok(true)` if this call was the one that marked the system as + /// initialized (the caller "won" the race), or `Ok(false)` if another + /// request already did it. This eliminates the race-condition window + /// between `is_system_initialized()` and `mark_system_initialized()`. + pub async fn try_claim_initialization( + &self, + admin_user_id: &str, + ) -> Result { + self.settings_repo + .try_claim_initialization(admin_user_id) + .await + } + // ======================================================================== // Registration Control // ======================================================================== diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index 93ba79b6..34ea83b5 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -20,7 +20,7 @@ use crate::{ storage_ports::FileReadPort, }, }, - common::{config::AppConfig, errors::DomainError}, + common::{config::AppConfig, errors::{DomainError, ErrorKind}}, domain::entities::share::{Share, ShareItemType, SharePermissions}, }; @@ -239,6 +239,17 @@ impl ShareUseCase for ShareService { return Err(ShareServiceError::Expired.into()); } + // SECURITY: If the share is password-protected, do NOT return + // the full metadata. Force the caller to verify the password + // first via `verify_shared_link_password`. + if share.has_password() { + return Err(DomainError::new( + ErrorKind::AccessDenied, + "Share", + "This share is password protected", + )); + } + // Convert the entity to DTO for the response Ok(ShareDto::from_entity(&share, &self.config.base_url())) } @@ -357,7 +368,7 @@ impl ShareUseCase for ShareService { &self, token: &str, password: &str, - ) -> Result { + ) -> Result { // Find the shared link by its token let share = self .share_repository @@ -374,9 +385,21 @@ impl ShareUseCase for ShareService { // Verify the password using the infrastructure port match share.password_hash() { - Some(hash) => self.password_hasher.verify_password(password, hash).await, - None => Ok(true), // No password required + Some(hash) => { + let is_valid = self.password_hasher.verify_password(password, hash).await?; + if !is_valid { + return Err(DomainError::new( + ErrorKind::AccessDenied, + "Share", + "Invalid share password", + )); + } + } + None => { /* No password required — allow access */ } } + + // Password verified (or not required) — return full share metadata + Ok(ShareDto::from_entity(&share, &self.config.base_url())) } async fn register_shared_link_access(&self, token: &str) -> Result<(), DomainError> { diff --git a/src/domain/repositories/settings_repository.rs b/src/domain/repositories/settings_repository.rs index b04e4ce2..536c9cf7 100644 --- a/src/domain/repositories/settings_repository.rs +++ b/src/domain/repositories/settings_repository.rs @@ -23,4 +23,35 @@ pub trait SettingsRepository: Send + Sync + 'static { /// Delete a setting by key async fn delete(&self, key: &str) -> Result<(), DomainError>; + + /// Atomically claim system initialization. + /// + /// Inserts `system_initialized = "true"` **only if the key does not + /// already exist**. Returns `true` when this call was the one that + /// performed the insert (i.e. the caller "won" the race), `false` if + /// the system was already initialized. + /// + /// The default implementation falls back to the non-atomic + /// get-then-set pattern for repositories that don't support a native + /// atomic upsert. + async fn try_claim_initialization( + &self, + admin_user_id: &str, + ) -> Result { + // Default: non-atomic fallback (overridden by PG implementation) + match self.get("system_initialized").await? { + Some(v) if v == "true" => Ok(false), + _ => { + self.set( + "system_initialized", + "true", + "system", + false, + Some(admin_user_id), + ) + .await?; + Ok(true) + } + } + } } diff --git a/src/infrastructure/repositories/pg/settings_pg_repository.rs b/src/infrastructure/repositories/pg/settings_pg_repository.rs index 98a1d241..30499af8 100644 --- a/src/infrastructure/repositories/pg/settings_pg_repository.rs +++ b/src/infrastructure/repositories/pg/settings_pg_repository.rs @@ -97,4 +97,28 @@ impl SettingsRepository for SettingsPgRepository { Ok(()) } + + /// Atomically claim system initialization using INSERT … ON CONFLICT DO NOTHING. + /// + /// Only the first caller that inserts the row gets `rows_affected == 1`; + /// concurrent callers see 0 rows affected and receive `false`. + async fn try_claim_initialization( + &self, + admin_user_id: &str, + ) -> Result { + let result = sqlx::query( + "INSERT INTO auth.admin_settings (key, value, category, is_secret, updated_by, updated_at) + VALUES ('system_initialized', 'true', 'system', false, $1, NOW()) + ON CONFLICT (key) DO NOTHING" + ) + .bind(admin_user_id) + .execute(self.pool.as_ref()) + .await + .map_err(|e| DomainError::new( + ErrorKind::InternalError, "Settings", format!("DB error: {}", e), + ))?; + + // rows_affected == 1 means we inserted; 0 means another caller already did + Ok(result.rows_affected() == 1) + } } diff --git a/src/interfaces/api/handlers/auth_handler.rs b/src/interfaces/api/handlers/auth_handler.rs index d44bbb6a..b6ace6a7 100644 --- a/src/interfaces/api/handlers/auth_handler.rs +++ b/src/interfaces/api/handlers/auth_handler.rs @@ -354,6 +354,10 @@ async fn logout( /// Requires the setup token that was printed to the server log on first boot. /// Once the admin is created, the system is marked as initialized and this /// endpoint returns 403 for all subsequent requests. +/// +/// Uses an atomic "claim" operation to prevent race conditions: even if two +/// requests arrive simultaneously with the correct token, only one will +/// succeed in marking the system as initialized and creating the admin. async fn setup_admin( State(state): State>, Json(dto): Json, @@ -366,12 +370,14 @@ async fn setup_admin( .as_ref() .ok_or_else(|| AppError::internal_error("Authentication service not configured"))?; - // 2. Check if system is already initialized (fail-closed: DB error → deny) + // 2. Verify admin settings service exists let admin_svc = state .admin_settings_service .as_ref() .ok_or_else(|| AppError::internal_error("Admin settings service not configured"))?; + // 3. Quick pre-check: if the system is already initialized, reject early + // (avoids token validation and Argon2 work on obviously-late requests) if admin_svc.is_system_initialized().await { tracing::warn!( "Setup admin rejected: system already initialized (user: {})", @@ -384,7 +390,7 @@ async fn setup_admin( )); } - // 3. Verify the one-time setup token + // 4. Verify the one-time setup token let expected_token = state.setup_token.as_deref().ok_or_else(|| { AppError::new( StatusCode::FORBIDDEN, @@ -405,7 +411,30 @@ async fn setup_admin( )); } - // 4. Create the first admin user + // 5. ATOMIC: claim initialization — only one concurrent request can win. + // We use a placeholder user_id ("pending") because the admin user + // doesn't exist yet. It will be updated to the real id below. + let claimed = admin_svc + .try_claim_initialization("pending") + .await + .map_err(|e| { + tracing::error!("Failed to claim system initialization: {}", e); + AppError::internal_error("Failed to claim system initialization") + })?; + + if !claimed { + tracing::warn!( + "Setup admin rejected: another request already claimed initialization (user: {})", + dto.username + ); + return Err(AppError::new( + StatusCode::FORBIDDEN, + "System is already initialized. Use the admin panel to manage users.", + "SystemAlreadyInitialized", + )); + } + + // 6. Create the first admin user (we hold the exclusive claim) let user = auth_service .auth_application_service .setup_create_admin(dto.username.clone(), dto.email, dto.password) @@ -415,13 +444,12 @@ async fn setup_admin( AppError::from(e) })?; - // 5. Mark system as initialized + // 7. Update the initialization record with the real admin user_id if let Err(e) = admin_svc.mark_system_initialized(&user.id).await { - // Admin was created but we couldn't mark as initialized. - // This is not fatal — the setup token check prevents re-use, and - // on next restart the system will detect the admin in DB. + // Not fatal — the claim already prevents concurrent re-initialization, + // and the "pending" marker is still "true" so the system stays locked. tracing::error!( - "Created admin but failed to mark system as initialized: {}", + "Created admin but failed to update initialized_by with real user id: {}", e ); } diff --git a/src/interfaces/api/handlers/caldav_handler.rs b/src/interfaces/api/handlers/caldav_handler.rs index 6cd09c9e..172abbc1 100644 --- a/src/interfaces/api/handlers/caldav_handler.rs +++ b/src/interfaces/api/handlers/caldav_handler.rs @@ -80,6 +80,7 @@ async fn handle_caldav_methods( ) -> Result, AppError> { let uri = req.uri().clone(); let path = extract_caldav_path(uri.path()); + reject_path_traversal(&path)?; handle_caldav_methods_inner(state, req, path).await } @@ -119,6 +120,18 @@ fn extract_caldav_path(uri_path: &str) -> String { percent_decode_str(encoded).decode_utf8_lossy().into_owned() } +/// Reject paths that contain path-traversal segments (`.` or `..`). +fn reject_path_traversal(path: &str) -> Result<(), AppError> { + for segment in path.split('/') { + if segment == ".." || segment == "." { + return Err(AppError::bad_request( + "Path must not contain '.' or '..' segments", + )); + } + } + Ok(()) +} + // ─── Helper: extract user from request ─────────────────────────────── fn extract_user(req: &Request) -> Result { diff --git a/src/interfaces/api/handlers/carddav_handler.rs b/src/interfaces/api/handlers/carddav_handler.rs index c1c3b6a0..8298a19e 100644 --- a/src/interfaces/api/handlers/carddav_handler.rs +++ b/src/interfaces/api/handlers/carddav_handler.rs @@ -66,6 +66,7 @@ async fn handle_carddav_methods( ) -> Result, AppError> { let uri = req.uri().clone(); let path = extract_carddav_path(uri.path()); + reject_path_traversal(&path)?; handle_carddav_methods_inner(state, req, path).await } @@ -107,6 +108,18 @@ fn extract_carddav_path(uri_path: &str) -> String { .into_owned() } +/// Reject paths that contain path-traversal segments (`.` or `..`). +fn reject_path_traversal(path: &str) -> Result<(), AppError> { + for segment in path.split('/') { + if segment == ".." || segment == "." { + return Err(AppError::bad_request( + "Path must not contain '.' or '..' segments", + )); + } + } + Ok(()) +} + // ─── Helper: extract user from request ─────────────────────────────── fn extract_user(req: &Request) -> Result { diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 871333d8..9f5855e6 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -130,6 +130,22 @@ pub fn webdav_routes() -> Router> { .route("/webdav", axum::routing::any(handle_webdav_methods_root)) } +/// Reject paths that contain path-traversal segments (`.` or `..`). +/// +/// Although deeper layers (PathResolver, StoragePath) also filter these out, +/// blocking them at the HTTP boundary provides defense-in-depth and ensures +/// no handler ever receives a traversal attempt. +fn reject_path_traversal(path: &str) -> Result<(), AppError> { + for segment in path.split('/') { + if segment == ".." || segment == "." { + return Err(AppError::bad_request( + "Path must not contain '.' or '..' segments", + )); + } + } + Ok(()) +} + /// Extract the resource path from the request URI, stripping the `/webdav/` prefix /// and percent-decoding the result so that folder/file names with spaces and /// special characters match the values stored in the database. @@ -160,6 +176,7 @@ async fn handle_webdav_methods( req: Request, ) -> Result, AppError> { let path = extract_webdav_path(req.uri()); + reject_path_traversal(&path)?; handle_webdav_dispatch(state, req, path).await } @@ -1109,6 +1126,9 @@ async fn handle_move( return Err(AppError::bad_request("Invalid destination URL")); }; + // SECURITY: reject path-traversal in destination + reject_path_traversal(&destination_path)?; + // Get services from state let file_retrieval_service = &state.applications.file_retrieval_service; let file_management_service = &state.applications.file_management_service; @@ -1206,6 +1226,12 @@ async fn handle_move( }; if source_parent_path != dest_parent_path { + // SECURITY: verify destination parent belongs to caller (V-08) + if !dest_parent_path.is_empty() { + if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { + assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?; + } + } file_management_service .move_file(&file.id, Some(dest_parent_path.to_string())) .await @@ -1301,6 +1327,12 @@ async fn handle_move( }; if source_parent_path != dest_parent_path { + // SECURITY: verify destination parent belongs to caller (V-08) + if !dest_parent_path.is_empty() { + if let Ok(parent) = folder_service.get_folder_by_path(dest_parent_path).await { + assert_owner(parent.owner_id.as_deref(), &user.id, dest_parent_path)?; + } + } file_management_service .move_file(&file.id, Some(dest_parent_path.to_string())) .await @@ -1367,6 +1399,9 @@ async fn handle_copy( return Err(AppError::bad_request("Invalid destination URL")); }; + // SECURITY: reject path-traversal in destination + reject_path_traversal(&destination_path)?; + // Get depth from Depth header let depth = req .headers()