diff --git a/src/application/services/file_retrieval_service.rs b/src/application/services/file_retrieval_service.rs index ec416eb5..da13caa6 100644 --- a/src/application/services/file_retrieval_service.rs +++ b/src/application/services/file_retrieval_service.rs @@ -343,19 +343,18 @@ impl FileRetrievalUseCase for FileRetrievalService { folder_id: Option<&str>, owner_id: Uuid, ) -> Result, DomainError> { - if folder_id.is_some() { - // folder id is defined, check permissions - self.require_target_folder_perm(folder_id, Permission::Read, owner_id) - .await?; - self.list_files(folder_id).await - } else { - // no folder id, get owners's files' root - let files = self - .file_read - .list_files_for_owner(folder_id, owner_id) - .await?; - Ok(files.into_iter().map(FileDto::from).collect()) + // Files always have a `folder_id` in the D0+ model — there is no + // longer any concept of "root-level files". A `None` from the + // caller means the query string was missing `folder_id`; reject + // with a clear error instead of routing through the legacy + // `list_files_for_owner` fallback (which used the doomed + // `user_id` column and returned empty in practice anyway). + if folder_id.is_none() { + return Err(DomainError::validation_error("folder_id is required")); } + self.require_target_folder_perm(folder_id, Permission::Read, owner_id) + .await?; + self.list_files(folder_id).await } async fn get_file_stream( diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index 263bd7e5..5003908a 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -316,20 +316,6 @@ impl FileBlobReadRepository { }) } - /// Returns the user_id (owner) for a given file ID. - /// Mirrors `FolderDbRepository::get_folder_user_id`. - /// Used by the AuthorizationEngine for owner short-circuit. - pub async fn get_file_user_id(&self, file_id: &str) -> Result { - sqlx::query_scalar::<_, uuid::Uuid>("SELECT user_id FROM storage.files WHERE id = $1::uuid") - .bind(file_id) - .fetch_optional(self.pool.as_ref()) - .await - .map_err(|e| { - DomainError::internal_error("FileBlobRead", format!("user_id lookup: {e}")) - })? - .ok_or_else(|| DomainError::not_found("File", file_id)) - } - /// Returns `drive_id` for a given file. Drives the permission-floor /// short-circuit in `PgAclEngine::check_inner` — drive membership is /// the baseline floor per `drive.md §5`. diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index 685a104f..d11d322f 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -1359,16 +1359,6 @@ impl FolderRepository for FolderDbRepository { // ── Extra helpers for blob-storage bootstrap ── impl FolderDbRepository { - /// Returns user_id for a given folder. Used by file repositories. - pub async fn get_folder_user_id(&self, folder_id: &str) -> Result { - sqlx::query_scalar::<_, Uuid>("SELECT user_id FROM storage.folders WHERE id = $1::uuid") - .bind(folder_id) - .fetch_optional(self.pool()) - .await - .map_err(|e| DomainError::internal_error("FolderDb", format!("user_id lookup: {e}")))? - .ok_or_else(|| DomainError::not_found("Folder", folder_id)) - } - /// Returns `drive_id` for a given folder. Drives the new permission-floor /// short-circuit in `PgAclEngine::check_inner` (a caller with any role /// on the folder's drive automatically passes the check — drive @@ -1382,22 +1372,6 @@ impl FolderDbRepository { .ok_or_else(|| DomainError::not_found("Folder", folder_id)) } - /// Verifies that `folder_id` is owned by `owner_id`. - /// - /// Returns `DomainError::not_found(...)` for both "folder missing" and - /// "folder owned by someone else" — same error to avoid leaking the - /// existence of resources belonging to other users. - pub async fn verify_owner(&self, folder_id: &str, owner_id: Uuid) -> Result<(), DomainError> { - let actual = self.get_folder_user_id(folder_id).await?; - if actual != owner_id { - return Err(DomainError::not_found( - "Folder", - "Target folder not found or access denied", - )); - } - Ok(()) - } - /// Cursor-paginated combined listing of sub-folders and files inside /// `parent_id`, sorted by `order_by`. /// diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index b612ffe0..8cf5de3f 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -29,7 +29,9 @@ use crate::application::ports::storage_ports::StorageUsagePort; use crate::application::services::file_retrieval_service::FileRetrievalService; use crate::application::services::folder_service::FolderService; use crate::common::di::AppState; +use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::domain::repositories::drive_repository::DriveRepository; +use crate::domain::services::authorization::{Permission, Resource, Subject}; use crate::infrastructure::services::path_resolver_service::ResolvedResource; use crate::infrastructure::services::webdav_dead_property_store::{DeadPropertyStore, ResourceRef}; use crate::interfaces::errors::AppError; @@ -153,18 +155,6 @@ fn extract_user(req: &Request) -> Result { .ok_or_else(|| AppError::unauthorized("Authentication required")) } -/// Assert that a resolved resource belongs to `user_id`. -/// -/// Used in the legacy (no-PathResolver) fallback paths where -/// `get_folder_by_path` / `get_file_by_path` are not user-scoped. -/// Returns `AppError::not_found` on mismatch so we don't leak the -/// existence of another user's resource. -fn assert_owner(owner_id: Option<&str>, user_id: &str, path: &str) -> Result<(), AppError> { - match owner_id { - Some(oid) if oid == user_id => Ok(()), - _ => Err(AppError::not_found(format!("Resource not found: {}", path))), - } -} /** * Creates and returns the WebDAV router with all required endpoints. @@ -518,7 +508,16 @@ async fn handle_propfind( // the caller's default drive once and reuse it for both probes. let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; if let Ok(folder) = folder_service.get_folder_by_path(&path, drive_id).await { - assert_owner(folder.owner_id.as_deref(), &user.id.to_string(), &path)?; + let folder_uuid = Uuid::parse_str(&folder.id) + .map_err(|_| AppError::not_found(format!("Resource not found: {}", path)))?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Read, + Resource::Folder(folder_uuid), + ) + .await?; let folder_id = folder.id.clone(); return build_streaming_propfind_response( folder, @@ -537,7 +536,16 @@ async fn handle_propfind( .get_file_by_path(&path, drive_id) .await { - assert_owner(file.owner_id.as_deref(), &user.id.to_string(), &path)?; + let file_uuid = Uuid::parse_str(&file.id) + .map_err(|_| AppError::not_found(format!("Resource not found: {}", path)))?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Read, + Resource::File(file_uuid), + ) + .await?; let dead_props = file_dead_props(&state, &file).await; let file_href = webdav_href(&client_path); let mut buf = Vec::with_capacity(1024); @@ -880,15 +888,25 @@ async fn handle_get( } } } else { - // Legacy fallback — fetch + ownership check. `drive_id` is the + // Legacy fallback — fetch + AuthZ check. `drive_id` is the // path-lookup scope post-D0 (`storage.files.path` repeats across - // drives), derived once from the caller's default drive. + // drives), derived once from the caller's default drive. PUT + // overwrite = Update on the existing file. let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; let f = file_retrieval_service .get_file_by_path(&path, drive_id) .await .map_err(|_e| AppError::not_found(format!("File not found: {}", path)))?; - assert_owner(f.owner_id.as_deref(), &user.id.to_string(), &path)?; + let file_uuid = Uuid::parse_str(&f.id) + .map_err(|_| AppError::not_found(format!("File not found: {}", path)))?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Update, + Resource::File(file_uuid), + ) + .await?; f }; @@ -996,7 +1014,16 @@ async fn handle_head( // reuse for both the folder and file probes. let drive_id = resolve_drive_id_for_native_webdav(&state, user.id).await?; if let Ok(folder) = folder_service.get_folder_by_path(&path, drive_id).await { - assert_owner(folder.owner_id.as_deref(), &user.id.to_string(), &path)?; + let folder_uuid = Uuid::parse_str(&folder.id) + .map_err(|_| AppError::not_found(format!("Resource not found: {}", path)))?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Read, + Resource::Folder(folder_uuid), + ) + .await?; return Ok(Response::builder() .status(StatusCode::OK) .header(header::CONTENT_TYPE, "httpd/unix-directory") @@ -1011,7 +1038,16 @@ async fn handle_head( .get_file_by_path(&path, drive_id) .await .map_err(|_e| AppError::not_found(format!("Resource not found: {}", path)))?; - assert_owner(file.owner_id.as_deref(), &user.id.to_string(), &path)?; + let file_uuid = Uuid::parse_str(&file.id) + .map_err(|_| AppError::not_found(format!("Resource not found: {}", path)))?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Read, + Resource::File(file_uuid), + ) + .await?; Ok(Response::builder() .status(StatusCode::OK) @@ -1043,9 +1079,12 @@ async fn handle_head( /// path-match). MOVE / DELETE / COPY previously 404'd on every /// root-level file because they only used the optimized resolver. /// -/// Ownership is enforced in both branches: the optimized resolver -/// includes `user_id = $4` in its SQL; the fallback runs `assert_owner` -/// explicitly so a foreign-owned hit can't leak through. +/// Cross-drive isolation is enforced in both branches: the optimized +/// resolver scopes by `drive_id`; the fallback derives the caller's +/// default `drive_id` and `get_folder_by_path` / `get_file_by_path` +/// scope by that. Callsites in the fallback additionally run +/// `authz.require(Permission::…, Resource::…)` per operation, matching +/// the modern AuthZ path. async fn resolve_or_legacy( state: &Arc, path: &str, @@ -1848,11 +1887,20 @@ async fn handle_move( .await { Ok(parent) => { - assert_owner( - parent.owner_id.as_deref(), - &user.id.to_string(), - dest_parent_path, - )?; + let parent_uuid = Uuid::parse_str(&parent.id).map_err(|_| { + AppError::conflict(format!( + "Destination parent not found: {}", + dest_parent_path + )) + })?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Create, + Resource::Folder(parent_uuid), + ) + .await?; Some(parent.id) } Err(_) => { @@ -1898,11 +1946,20 @@ async fn handle_move( dest_parent_path )) })?; - assert_owner( - parent.owner_id.as_deref(), - &user.id.to_string(), - dest_parent_path, - )?; + let parent_uuid = Uuid::parse_str(&parent.id).map_err(|_| { + AppError::conflict(format!( + "Destination parent not found: {}", + dest_parent_path + )) + })?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Create, + Resource::Folder(parent_uuid), + ) + .await?; Some(parent.id) }; file_management_service @@ -2106,11 +2163,20 @@ async fn handle_copy( .await { Ok(parent) => { - assert_owner( - parent.owner_id.as_deref(), - &user.id.to_string(), - dest_parent_path, - )?; + let parent_uuid = Uuid::parse_str(&parent.id).map_err(|_| { + AppError::conflict(format!( + "Destination parent not found: {}", + dest_parent_path + )) + })?; + state + .authorization + .require( + Subject::User(user.id), + Permission::Create, + Resource::Folder(parent_uuid), + ) + .await?; Some(parent.id) } Err(_) => {