From f5f5b1167feb01483b083aa1432f5e445f5ef78d Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Thu, 2 Jul 2026 01:06:50 +0200 Subject: [PATCH] feat(drive): remove _for_owner() and use authz remove related IDOR protection as Hurl tests are covering this surface --- src/application/ports/storage_ports.rs | 50 -- .../services/file_retrieval_service.rs | 30 +- .../services/idor_protection_test.rs | 430 ------------------ src/application/services/mod.rs | 2 - src/application/services/share_service.rs | 8 - .../services/trash_service_test.rs | 9 - src/common/stubs.rs | 4 - .../pg/file_blob_read_repository.rs | 182 -------- src/interfaces/api/handlers/folder_handler.rs | 9 +- 9 files changed, 21 insertions(+), 703 deletions(-) delete mode 100644 src/application/services/idor_protection_test.rs diff --git a/src/application/ports/storage_ports.rs b/src/application/ports/storage_ports.rs index 77347c89..5e4bf4e8 100644 --- a/src/application/ports/storage_ports.rs +++ b/src/application/ports/storage_ports.rs @@ -31,40 +31,9 @@ pub trait FileReadPort: Send + Sync + 'static { async fn get_file_or_trashed(&self, id: &str) -> Result; - /// Gets a file by its ID, scoped to a specific owner. - /// - /// Returns `NotFound` if the file does not exist **or** belongs to a - /// different user. This is the primary IDOR-safe accessor — handlers - /// serving end-user requests should always prefer this over `get_file`. - async fn get_file_for_owner(&self, id: &str, owner_id: Uuid) -> Result; - - /// Verifies that the file identified by `id` belongs to `owner_id`. - /// - /// Returns `Ok(())` on success or `NotFound` when the file does not - /// exist or belongs to another user. - async fn verify_file_owner(&self, id: &str, owner_id: Uuid) -> Result<(), DomainError> { - self.get_file_for_owner(id, owner_id).await.map(|_| ()) - } - /// Lists files in a folder. async fn list_files(&self, folder_id: Option<&str>) -> Result, DomainError>; - /// Lists files in a folder scoped to a specific owner (SQL-level). - /// - /// Default falls back to `list_files` + in-memory filter. - /// Repositories should override with a direct `AND user_id = $N` query. - async fn list_files_for_owner( - &self, - folder_id: Option<&str>, - owner_id: Uuid, - ) -> Result, DomainError> { - let all = self.list_files(folder_id).await?; - Ok(all - .into_iter() - .filter(|f| f.owner_id() == Some(owner_id)) - .collect()) - } - /// Gets content as a stream (ideal for large files). async fn get_file_stream( &self, @@ -155,25 +124,6 @@ pub trait FileReadPort: Send + Sync + 'static { Ok(all.into_iter().skip(start).take(end - start).collect()) } - /// Like [`list_files_batch`], but only returns files owned by `owner_id`. - /// - /// Used by streaming WebDAV PROPFIND to list files scoped to the - /// authenticated user, preventing cross-user data leakage. - async fn list_files_batch_for_owner( - &self, - folder_id: Option<&str>, - owner_id: Uuid, - offset: i64, - limit: i64, - ) -> Result, DomainError> { - // Default: filter in-memory (repos should override with SQL) - let all = self.list_files_batch(folder_id, offset, limit).await?; - Ok(all - .into_iter() - .filter(|f| f.owner_id() == Some(owner_id)) - .collect()) - } - /// Streams every file in the subtree rooted at `folder_id`. /// /// Uses an ltree `<@` join against `storage.folders` so the entire diff --git a/src/application/services/file_retrieval_service.rs b/src/application/services/file_retrieval_service.rs index da13caa6..522c6acc 100644 --- a/src/application/services/file_retrieval_service.rs +++ b/src/application/services/file_retrieval_service.rs @@ -346,9 +346,8 @@ impl FileRetrievalUseCase for FileRetrievalService { // 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). + // with a clear error rather than returning an empty set from a + // meaningless root-level query. if folder_id.is_none() { return Err(DomainError::validation_error("folder_id is required")); } @@ -469,20 +468,21 @@ impl FileRetrievalUseCase for FileRetrievalService { offset: i64, limit: i64, ) -> 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?; - let files = self - .file_read - .list_files_batch(folder_id, offset, limit) - .await?; - return Ok(files.into_iter().map(FileDto::from).collect()); - } - + // Post-D0: every file lives in a folder — `storage.files.folder_id` + // is NOT NULL. `folder_id = None` means the caller is asking for + // "root-level files", which by design return an empty set: the + // WebDAV synthetic root only lists drive-root folders as + // children. Skip the DB round-trip and the pre-D7 owner-fallback + // query (which used to hit `_for_owner` and would have driven + // the `files.user_id` filter this refactor is retiring). + let Some(_) = folder_id else { + return Ok(Vec::new()); + }; + self.require_target_folder_perm(folder_id, Permission::Read, owner_id) + .await?; let files = self .file_read - .list_files_batch_for_owner(folder_id, owner_id, offset, limit) + .list_files_batch(folder_id, offset, limit) .await?; Ok(files.into_iter().map(FileDto::from).collect()) } diff --git a/src/application/services/idor_protection_test.rs b/src/application/services/idor_protection_test.rs deleted file mode 100644 index 70a6909b..00000000 --- a/src/application/services/idor_protection_test.rs +++ /dev/null @@ -1,430 +0,0 @@ -//! Tests for IDOR (Insecure Direct Object Reference) protection. -//! -//! Verifies that ownership checks at the repository and service layers -//! correctly reject access when the caller is not the file owner. - -use bytes::Bytes; -use futures::Stream; -use std::collections::HashMap; -use std::path::PathBuf; -use std::pin::Pin; -use std::sync::Mutex; -use uuid::Uuid; - -use crate::application::ports::storage_ports::{FileReadPort, FileWritePort}; -use crate::common::errors::DomainError; -use crate::domain::entities::file::File; -use crate::domain::services::path_service::StoragePath; - -// ═══════════════════════════════════════════════════════════════════════════ -// Mock repositories -// ═══════════════════════════════════════════════════════════════════════════ - -/// A simple in-memory mock that maps (file_id → (File, owner_id)). -struct MockFileReadPort { - /// file_id → (File, owner_id) - files: Mutex>, -} - -impl MockFileReadPort { - fn new() -> Self { - Self { - files: Mutex::new(HashMap::new()), - } - } - - /// Insert a test file owned by `owner_id`. - fn insert(&self, id: &str, name: &str, owner_id: Uuid) { - let file = File::new( - id.to_string(), - name.to_string(), - StoragePath::from_string(&format!("/{}", name)), - 42, - "text/plain".to_string(), - None, - ) - .unwrap(); - self.files - .lock() - .unwrap() - .insert(id.to_string(), (file, owner_id)); - } -} - -impl FileReadPort for MockFileReadPort { - async fn get_file(&self, id: &str) -> Result { - let files = self.files.lock().unwrap(); - files - .get(id) - .map(|(f, _)| f.clone()) - .ok_or_else(|| DomainError::not_found("File", id.to_string())) - } - - async fn get_file_or_trashed(&self, id: &str) -> Result { - let files = self.files.lock().unwrap(); - files - .get(id) - .map(|(f, _)| f.clone()) - .ok_or_else(|| DomainError::not_found("File", id.to_string())) - } - - async fn get_file_for_owner(&self, id: &str, owner_id: Uuid) -> Result { - let files = self.files.lock().unwrap(); - match files.get(id) { - Some((file, actual_owner)) if *actual_owner == owner_id => Ok(file.clone()), - // Return NotFound regardless — do not leak existence - _ => Err(DomainError::not_found("File", id.to_string())), - } - } - - async fn list_files(&self, _folder_id: Option<&str>) -> Result, DomainError> { - Ok(Vec::new()) - } - - async fn get_file_stream( - &self, - _id: &str, - ) -> Result> + Send>, DomainError> { - unimplemented!() - } - - async fn get_file_range_stream( - &self, - _id: &str, - _start: u64, - _end: Option, - ) -> Result> + Send>, DomainError> { - unimplemented!() - } - - async fn get_file_path(&self, _id: &str) -> Result { - unimplemented!() - } - - async fn get_parent_folder_id( - &self, - _path: &str, - _drive_id: Uuid, - ) -> Result { - unimplemented!() - } - - async fn get_blob_hash(&self, _file_id: &str) -> Result { - Ok(String::new()) - } - - async fn search_files_paginated( - &self, - _folder_id: Option<&str>, - _criteria: &crate::application::dtos::search_dto::SearchCriteriaDto, - _user_id: Uuid, - ) -> Result<(Vec, usize), DomainError> { - Ok((Vec::new(), 0)) - } - - async fn count_files( - &self, - _folder_id: Option<&str>, - _criteria: &crate::application::dtos::search_dto::SearchCriteriaDto, - _user_id: Uuid, - ) -> Result { - Ok(0) - } - - async fn get_folder_id_by_path( - &self, - _folder_path: &str, - _drive_id: Uuid, - ) -> Result { - unimplemented!() - } - - async fn stream_files_in_subtree( - &self, - _folder_id: &str, - ) -> Result> + Send>>, DomainError> { - Ok(Box::pin(futures::stream::empty())) - } -} - -/// Minimal mock write port — only `move_file` and `rename_file` need real logic. -#[allow(dead_code)] -struct MockFileWritePort { - files: Mutex>, -} - -impl MockFileWritePort { - #[allow(dead_code)] - fn new() -> Self { - Self { - files: Mutex::new(HashMap::new()), - } - } - - #[allow(dead_code)] - fn insert(&self, id: &str, name: &str) { - let file = File::new( - id.to_string(), - name.to_string(), - StoragePath::from_string(&format!("/{}", name)), - 42, - "text/plain".to_string(), - None, - ) - .unwrap(); - self.files.lock().unwrap().insert(id.to_string(), file); - } -} - -impl FileWritePort for MockFileWritePort { - async fn save_file_with_blob( - &self, - _name: String, - _folder_id: Option, - _content_type: String, - _blob_hash: &str, - _size: u64, - _caller_id: Uuid, - ) -> Result { - unimplemented!() - } - - async fn move_file( - &self, - file_id: &str, - _target_folder_id: Option, - _caller_id: Uuid, - ) -> Result { - let files = self.files.lock().unwrap(); - files - .get(file_id) - .cloned() - .ok_or_else(|| DomainError::not_found("File", file_id.to_string())) - } - - async fn rename_file( - &self, - file_id: &str, - _new_name: &str, - _caller_id: Uuid, - ) -> Result { - let files = self.files.lock().unwrap(); - files - .get(file_id) - .cloned() - .ok_or_else(|| DomainError::not_found("File", file_id.to_string())) - } - - async fn delete_file(&self, _id: &str) -> Result<(), DomainError> { - Ok(()) - } - - async fn update_file_content_with_blob( - &self, - _file_id: &str, - _blob_hash: &str, - _size: u64, - _modified_at: Option, - _caller_id: Uuid, - ) -> Result<(String, i64), DomainError> { - Ok((String::new(), 0)) - } - - async fn register_file_deferred( - &self, - _name: String, - _folder_id: Option, - _content_type: String, - _size: u64, - _caller_id: Uuid, - ) -> Result<(File, PathBuf), DomainError> { - unimplemented!() - } - - async fn copy_file( - &self, - _file_id: &str, - _target_folder_id: Option, - _new_name: Option<&str>, - _caller_id: Uuid, - ) -> Result { - unimplemented!() - } - - async fn move_to_trash(&self, _file_id: &str, _caller_id: Uuid) -> Result<(), DomainError> { - Ok(()) - } - - async fn restore_from_trash( - &self, - _file_id: &str, - _original_path: &str, - _caller_id: Uuid, - ) -> Result<(), DomainError> { - Ok(()) - } - - async fn delete_file_permanently(&self, _file_id: &str) -> Result<(), DomainError> { - Ok(()) - } -} - -// ═══════════════════════════════════════════════════════════════════════════ -// Tests — FileReadPort::get_file_for_owner (Repository layer, Solution C) -// ═══════════════════════════════════════════════════════════════════════════ - -#[tokio::test] -async fn get_file_for_owner_returns_file_for_correct_owner() { - let alice_id = Uuid::new_v4(); - let repo = MockFileReadPort::new(); - repo.insert("file-1", "secret.txt", alice_id); - - let result = repo.get_file_for_owner("file-1", alice_id).await; - assert!(result.is_ok(), "owner should be able to read own file"); - assert_eq!(result.unwrap().id(), "file-1"); -} - -#[tokio::test] -async fn get_file_for_owner_rejects_wrong_owner() { - let alice_id = Uuid::new_v4(); - let bob_id = Uuid::new_v4(); - let repo = MockFileReadPort::new(); - repo.insert("file-1", "secret.txt", alice_id); - - let result = repo.get_file_for_owner("file-1", bob_id).await; - assert!(result.is_err(), "non-owner should be rejected"); - - // Must be NotFound, NOT Forbidden — avoids leaking existence - let err = result.unwrap_err(); - let msg = format!("{}", err); - assert!( - msg.contains("not found") || msg.contains("NotFound"), - "error must be NotFound, got: {}", - msg - ); -} - -#[tokio::test] -async fn get_file_for_owner_returns_not_found_for_missing_file() { - let alice_id = Uuid::new_v4(); - let repo = MockFileReadPort::new(); - - let result = repo.get_file_for_owner("nonexistent", alice_id).await; - assert!(result.is_err()); -} - -#[tokio::test] -async fn verify_file_owner_uses_default_impl() { - let alice_id = Uuid::new_v4(); - let bob_id = Uuid::new_v4(); - let repo = MockFileReadPort::new(); - repo.insert("file-1", "secret.txt", alice_id); - - // Default impl delegates to get_file_for_owner and maps to () - assert!(repo.verify_file_owner("file-1", alice_id).await.is_ok()); - assert!(repo.verify_file_owner("file-1", bob_id).await.is_err()); -} - -// ═══════════════════════════════════════════════════════════════════════════ -// Tests — FileManagementService _owned methods (Service layer, Solution B) -// ═══════════════════════════════════════════════════════════════════════════ -// -// Note: FileManagementService::with_trash takes concrete types for the write -// repository (Arc). We cannot construct real PG repos -// without a database. Instead, we test the verify_owner logic indirectly by -// testing the mock-based trait interactions at the port level, and document -// that integration tests hitting the real DB are the ultimate verification. -// -// The tests below verify the *contract*: _owned methods must call -// verify_owner before delegating, and verify_owner must fail-closed when -// no read repo is available. - -#[tokio::test] -async fn verify_file_owner_delegates_to_read_port() { - // This test verifies the FileReadPort contract that verify_file_owner - // returns Ok for the correct owner and Err for others. - let user_id = Uuid::new_v4(); - let attacker_id = Uuid::new_v4(); - let read = MockFileReadPort::new(); - read.insert("abc-123", "report.pdf", user_id); - - // Same user → Ok - let ok = read.verify_file_owner("abc-123", user_id).await; - assert!(ok.is_ok(), "correct owner should pass verify_file_owner"); - - // Different user → Err - let err = read.verify_file_owner("abc-123", attacker_id).await; - assert!(err.is_err(), "wrong owner should fail verify_file_owner"); -} - -#[tokio::test] -async fn owned_methods_require_ownership_check_first() { - // Simulate what the _owned methods do: verify_owner then delegate. - // We test with the mock read port to prove the sequence. - let owner_id = Uuid::new_v4(); - let attacker_id = Uuid::new_v4(); - let read = MockFileReadPort::new(); - read.insert("file-1", "data.csv", owner_id); - - // Step 1: verify_owner for correct owner → Ok - let step1 = read.verify_file_owner("file-1", owner_id).await; - assert!(step1.is_ok()); - - // Step 2: verify_owner for attacker → Err, so the move/rename never executes - let step2 = read.verify_file_owner("file-1", attacker_id).await; - assert!(step2.is_err()); -} - -// ═══════════════════════════════════════════════════════════════════════════ -// Tests — Trait-level _owned method stubs (StubFileManagementUseCase) -// ═══════════════════════════════════════════════════════════════════════════ - -use crate::application::ports::file_ports::FileManagementUseCase; -use crate::common::stubs::StubFileManagementUseCase; - -#[tokio::test] -async fn stub_move_file_owned_returns_ok() { - let user_id = Uuid::new_v4(); - let stub = StubFileManagementUseCase; - let result = stub - .move_file_with_perms("file-1", user_id, Some("folder-2".to_string())) - .await; - assert!(result.is_ok(), "stub should return Ok for move_file_owned"); -} - -#[tokio::test] -async fn stub_rename_file_owned_returns_ok() { - let user_id = Uuid::new_v4(); - let stub = StubFileManagementUseCase; - let result = stub - .rename_file_with_perms("file-1", user_id, "new-name.txt") - .await; - assert!( - result.is_ok(), - "stub should return Ok for rename_file_owned" - ); -} - -use crate::application::ports::file_ports::FileRetrievalUseCase; -use crate::common::stubs::StubFileRetrievalUseCase; - -#[tokio::test] -async fn stub_get_file_owned_returns_ok() { - let user_id = Uuid::new_v4(); - let stub = StubFileRetrievalUseCase; - let result = stub.get_file_with_perms("file-1", user_id).await; - assert!(result.is_ok(), "stub should return Ok for get_file_owned"); -} - -#[tokio::test] -async fn stub_get_file_optimized_owned_returns_ok() { - let user_id = Uuid::new_v4(); - let stub = StubFileRetrievalUseCase; - let result = stub - .get_file_optimized_with_perms("file-1", user_id, true, false) - .await; - assert!( - result.is_ok(), - "stub should return Ok for get_file_optimized_owned" - ); -} diff --git a/src/application/services/mod.rs b/src/application/services/mod.rs index 578e3d98..6c9539b0 100644 --- a/src/application/services/mod.rs +++ b/src/application/services/mod.rs @@ -39,8 +39,6 @@ pub mod wopi_token_service; #[cfg(test)] mod batch_operations_test; #[cfg(test)] -mod idor_protection_test; -#[cfg(test)] mod trash_service_test; // Re-exportar para facilitar acceso diff --git a/src/application/services/share_service.rs b/src/application/services/share_service.rs index 5d5aae3a..28ea3a27 100644 --- a/src/application/services/share_service.rs +++ b/src/application/services/share_service.rs @@ -928,14 +928,6 @@ mod tests { > { Ok(Box::pin(futures::stream::empty())) } - - async fn get_file_for_owner( - &self, - id: &str, - _owner_id: Uuid, - ) -> Result { - self.get_file(id).await - } } impl FolderRepository for MockFolderRepository { diff --git a/src/application/services/trash_service_test.rs b/src/application/services/trash_service_test.rs index 2adadc59..4147bf6e 100644 --- a/src/application/services/trash_service_test.rs +++ b/src/application/services/trash_service_test.rs @@ -554,15 +554,6 @@ impl FileReadPort for MockFileRepository { > { Ok(Box::pin(futures::stream::empty())) } - - async fn get_file_for_owner( - &self, - id: &str, - _owner_id: Uuid, - ) -> std::result::Result { - // In this mock, ignore ownership — trash tests don't focus on ownership - self.get_file(id).await - } } impl FileWritePort for MockFileRepository { diff --git a/src/common/stubs.rs b/src/common/stubs.rs index eed28c50..2cde43f9 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -145,10 +145,6 @@ impl FileReadPort for StubFileReadPort { ) -> Result> + Send>>, DomainError> { Ok(Box::pin(futures::stream::empty())) } - - async fn get_file_for_owner(&self, _id: &str, _owner_id: Uuid) -> Result { - Ok(File::default()) - } } // --------------------------------------------------------------------------- diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index b9ea54b8..00aaef47 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -720,54 +720,6 @@ impl FileReadPort for FileBlobReadRepository { ) } - async fn get_file_for_owner(&self, id: &str, owner_id: Uuid) -> Result { - let row = sqlx::query_as::< - _, - ( - String, // id - String, // name - Option, // folder_id - Option, // folder path - i64, // size - String, // mime_type - i64, // created_at - i64, // updated_at - String, // blob_hash - Option, // user_id (owner) - Option, // created_by (§14) - Option, // updated_by (§14) - ), - >( - r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, - fi.size, fi.mime_type, - EXTRACT(EPOCH FROM fi.created_at)::bigint, - EXTRACT(EPOCH FROM fi.updated_at)::bigint, - fi.blob_hash, - fi.user_id, - fi.created_by, fi.updated_by - FROM storage.files fi - LEFT JOIN storage.folders fo ON fo.id = fi.folder_id - WHERE fi.id = $1::uuid - AND fi.user_id = $2 - AND NOT fi.is_trashed - "#, - ) - .bind(id) - .bind(owner_id) - .fetch_optional(self.pool.as_ref()) - .await - .map_err(|e| DomainError::internal_error("FileBlobRead", format!("get_for_owner: {e}")))? - // Return NotFound (not Forbidden) to avoid leaking file existence - .ok_or_else(|| DomainError::not_found("File", id))?; - - self.hash_cache.insert(id.to_string(), row.8.clone()); - - Self::row_to_file( - row.0, row.1, row.2, row.3, row.4, row.5, row.6, row.7, row.8, row.9, row.10, row.11, - ) - } - #[allow(clippy::type_complexity)] async fn list_files(&self, folder_id: Option<&str>) -> Result, DomainError> { let rows: Vec = if let Some(fid) = folder_id { @@ -821,68 +773,6 @@ impl FileReadPort for FileBlobReadRepository { .collect() } - /// User-scoped file listing — adds `AND fi.user_id = $2` to prevent - /// cross-user data leakage in the REST API (`list_files_query`). - async fn list_files_for_owner( - &self, - folder_id: Option<&str>, - owner_id: Uuid, - ) -> Result, DomainError> { - let rows: Vec = if let Some(fid) = folder_id { - sqlx::query_as( - r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, - fi.size, fi.mime_type, - EXTRACT(EPOCH FROM fi.created_at)::bigint, - EXTRACT(EPOCH FROM fi.updated_at)::bigint, - fi.blob_hash, - fi.user_id, - fi.created_by, fi.updated_by - FROM storage.files fi - LEFT JOIN storage.folders fo ON fo.id = fi.folder_id - WHERE fi.folder_id = $1::uuid AND NOT fi.is_trashed - AND fi.user_id = $2 - ORDER BY fi.name - "#, - ) - .bind(fid) - .bind(owner_id) - .fetch_all(self.pool.as_ref()) - .await - } else { - sqlx::query_as( - r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, - fi.size, fi.mime_type, - EXTRACT(EPOCH FROM fi.created_at)::bigint, - EXTRACT(EPOCH FROM fi.updated_at)::bigint, - fi.blob_hash, - fi.user_id, - fi.created_by, fi.updated_by - FROM storage.files fi - LEFT JOIN storage.folders fo ON fo.id = fi.folder_id - WHERE fi.folder_id IS NULL AND NOT fi.is_trashed - AND fi.user_id = $1 - ORDER BY fi.name - "#, - ) - .bind(owner_id) - .fetch_all(self.pool.as_ref()) - .await - } - .map_err(|e| DomainError::internal_error("FileBlobRead", format!("list_for_owner: {e}")))?; - - rows.into_iter() - .map( - |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, cb, ub)| { - Self::row_to_file( - id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, cb, ub, - ) - }, - ) - .collect() - } - async fn get_blob_hash(&self, file_id: &str) -> Result { self.resolve_blob_hash(file_id).await } @@ -955,78 +845,6 @@ impl FileReadPort for FileBlobReadRepository { .collect() } - /// User-scoped paginated file listing — adds `AND fi.user_id = $4` to - /// prevent cross-user data leakage in WebDAV PROPFIND. - async fn list_files_batch_for_owner( - &self, - folder_id: Option<&str>, - owner_id: Uuid, - offset: i64, - limit: i64, - ) -> Result, DomainError> { - let rows: Vec = if let Some(fid) = folder_id { - sqlx::query_as( - r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, - fi.size, fi.mime_type, - EXTRACT(EPOCH FROM fi.created_at)::bigint, - EXTRACT(EPOCH FROM fi.updated_at)::bigint, - fi.blob_hash, - fi.user_id, - fi.created_by, fi.updated_by - FROM storage.files fi - LEFT JOIN storage.folders fo ON fo.id = fi.folder_id - WHERE fi.folder_id = $1::uuid AND NOT fi.is_trashed - AND fi.user_id = $4 - ORDER BY fi.name - LIMIT $2 OFFSET $3 - "#, - ) - .bind(fid) - .bind(limit) - .bind(offset) - .bind(owner_id) - .fetch_all(self.pool.as_ref()) - .await - } else { - sqlx::query_as( - r#" - SELECT fi.id::text, fi.name, fi.folder_id::text, fo.path, - fi.size, fi.mime_type, - EXTRACT(EPOCH FROM fi.created_at)::bigint, - EXTRACT(EPOCH FROM fi.updated_at)::bigint, - fi.blob_hash, - fi.user_id, - fi.created_by, fi.updated_by - FROM storage.files fi - LEFT JOIN storage.folders fo ON fo.id = fi.folder_id - WHERE fi.folder_id IS NULL AND NOT fi.is_trashed - AND fi.user_id = $3 - ORDER BY fi.name - LIMIT $1 OFFSET $2 - "#, - ) - .bind(limit) - .bind(offset) - .bind(owner_id) - .fetch_all(self.pool.as_ref()) - .await - } - .map_err(|e| { - DomainError::internal_error("FileBlobRead", format!("list_batch_for_owner: {e}")) - })?; - - rows.into_iter() - .map( - |(id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, cb, ub)| { - Self::row_to_file( - id, name, fid, fpath, size, mime, ca, ma, blob_hash, uid, cb, ub, - ) - }, - ) - .collect() - } - async fn get_file_stream( &self, id: &str, diff --git a/src/interfaces/api/handlers/folder_handler.rs b/src/interfaces/api/handlers/folder_handler.rs index 857a7d35..3a572e15 100644 --- a/src/interfaces/api/handlers/folder_handler.rs +++ b/src/interfaces/api/handlers/folder_handler.rs @@ -106,9 +106,12 @@ impl FolderHandler { Self::list_folders_scoped(service, None, &auth_user).await } - /// Internal helper: lists folders scoped to the authenticated user. - /// Uses `list_folders_for_owner` — the DB query filters by `user_id`, - /// so no data from other users ever leaves the database. + /// Internal helper: lists folders the authenticated caller can Read. + /// Post-PR-B, `list_root_folders_for_caller` scopes via + /// drive-membership grants (`role_grants` + group cascade via + /// `storage.caller_group_ids`) instead of the legacy `folders.user_id` + /// filter, so folders in shared drives the caller belongs to + /// surface here too. async fn list_folders_scoped( service: AppState, parent_id: Option<&str>,