feat(drive): remove _for_owner() and use authz
remove related IDOR protection as Hurl tests are covering this surface
This commit is contained in:
@@ -31,40 +31,9 @@ pub trait FileReadPort: Send + Sync + 'static {
|
|||||||
|
|
||||||
async fn get_file_or_trashed(&self, id: &str) -> Result<File, DomainError>;
|
async fn get_file_or_trashed(&self, id: &str) -> Result<File, DomainError>;
|
||||||
|
|
||||||
/// 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<File, DomainError>;
|
|
||||||
|
|
||||||
/// 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.
|
/// Lists files in a folder.
|
||||||
async fn list_files(&self, folder_id: Option<&str>) -> Result<Vec<File>, DomainError>;
|
async fn list_files(&self, folder_id: Option<&str>) -> Result<Vec<File>, 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<Vec<File>, 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).
|
/// Gets content as a stream (ideal for large files).
|
||||||
async fn get_file_stream(
|
async fn get_file_stream(
|
||||||
&self,
|
&self,
|
||||||
@@ -155,25 +124,6 @@ pub trait FileReadPort: Send + Sync + 'static {
|
|||||||
Ok(all.into_iter().skip(start).take(end - start).collect())
|
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<Vec<File>, 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`.
|
/// Streams every file in the subtree rooted at `folder_id`.
|
||||||
///
|
///
|
||||||
/// Uses an ltree `<@` join against `storage.folders` so the entire
|
/// Uses an ltree `<@` join against `storage.folders` so the entire
|
||||||
|
|||||||
@@ -346,9 +346,8 @@ impl FileRetrievalUseCase for FileRetrievalService {
|
|||||||
// Files always have a `folder_id` in the D0+ model — there is no
|
// Files always have a `folder_id` in the D0+ model — there is no
|
||||||
// longer any concept of "root-level files". A `None` from the
|
// longer any concept of "root-level files". A `None` from the
|
||||||
// caller means the query string was missing `folder_id`; reject
|
// caller means the query string was missing `folder_id`; reject
|
||||||
// with a clear error instead of routing through the legacy
|
// with a clear error rather than returning an empty set from a
|
||||||
// `list_files_for_owner` fallback (which used the doomed
|
// meaningless root-level query.
|
||||||
// `user_id` column and returned empty in practice anyway).
|
|
||||||
if folder_id.is_none() {
|
if folder_id.is_none() {
|
||||||
return Err(DomainError::validation_error("folder_id is required"));
|
return Err(DomainError::validation_error("folder_id is required"));
|
||||||
}
|
}
|
||||||
@@ -469,20 +468,21 @@ impl FileRetrievalUseCase for FileRetrievalService {
|
|||||||
offset: i64,
|
offset: i64,
|
||||||
limit: i64,
|
limit: i64,
|
||||||
) -> Result<Vec<FileDto>, DomainError> {
|
) -> Result<Vec<FileDto>, DomainError> {
|
||||||
if folder_id.is_some() {
|
// Post-D0: every file lives in a folder — `storage.files.folder_id`
|
||||||
// folder id is defined, check permissions
|
// is NOT NULL. `folder_id = None` means the caller is asking for
|
||||||
self.require_target_folder_perm(folder_id, Permission::Read, owner_id)
|
// "root-level files", which by design return an empty set: the
|
||||||
.await?;
|
// WebDAV synthetic root only lists drive-root folders as
|
||||||
let files = self
|
// children. Skip the DB round-trip and the pre-D7 owner-fallback
|
||||||
.file_read
|
// query (which used to hit `_for_owner` and would have driven
|
||||||
.list_files_batch(folder_id, offset, limit)
|
// the `files.user_id` filter this refactor is retiring).
|
||||||
.await?;
|
let Some(_) = folder_id else {
|
||||||
return Ok(files.into_iter().map(FileDto::from).collect());
|
return Ok(Vec::new());
|
||||||
}
|
};
|
||||||
|
self.require_target_folder_perm(folder_id, Permission::Read, owner_id)
|
||||||
|
.await?;
|
||||||
let files = self
|
let files = self
|
||||||
.file_read
|
.file_read
|
||||||
.list_files_batch_for_owner(folder_id, owner_id, offset, limit)
|
.list_files_batch(folder_id, offset, limit)
|
||||||
.await?;
|
.await?;
|
||||||
Ok(files.into_iter().map(FileDto::from).collect())
|
Ok(files.into_iter().map(FileDto::from).collect())
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -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<HashMap<String, (File, Uuid)>>,
|
|
||||||
}
|
|
||||||
|
|
||||||
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<File, DomainError> {
|
|
||||||
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<File, DomainError> {
|
|
||||||
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<File, DomainError> {
|
|
||||||
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<Vec<File>, DomainError> {
|
|
||||||
Ok(Vec::new())
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn get_file_stream(
|
|
||||||
&self,
|
|
||||||
_id: &str,
|
|
||||||
) -> Result<Box<dyn Stream<Item = Result<Bytes, std::io::Error>> + Send>, DomainError> {
|
|
||||||
unimplemented!()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn get_file_range_stream(
|
|
||||||
&self,
|
|
||||||
_id: &str,
|
|
||||||
_start: u64,
|
|
||||||
_end: Option<u64>,
|
|
||||||
) -> Result<Box<dyn Stream<Item = Result<Bytes, std::io::Error>> + Send>, DomainError> {
|
|
||||||
unimplemented!()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn get_file_path(&self, _id: &str) -> Result<StoragePath, DomainError> {
|
|
||||||
unimplemented!()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn get_parent_folder_id(
|
|
||||||
&self,
|
|
||||||
_path: &str,
|
|
||||||
_drive_id: Uuid,
|
|
||||||
) -> Result<String, DomainError> {
|
|
||||||
unimplemented!()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn get_blob_hash(&self, _file_id: &str) -> Result<String, DomainError> {
|
|
||||||
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<File>, 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<usize, DomainError> {
|
|
||||||
Ok(0)
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn get_folder_id_by_path(
|
|
||||||
&self,
|
|
||||||
_folder_path: &str,
|
|
||||||
_drive_id: Uuid,
|
|
||||||
) -> Result<String, DomainError> {
|
|
||||||
unimplemented!()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn stream_files_in_subtree(
|
|
||||||
&self,
|
|
||||||
_folder_id: &str,
|
|
||||||
) -> Result<Pin<Box<dyn Stream<Item = Result<File, DomainError>> + 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<HashMap<String, File>>,
|
|
||||||
}
|
|
||||||
|
|
||||||
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<String>,
|
|
||||||
_content_type: String,
|
|
||||||
_blob_hash: &str,
|
|
||||||
_size: u64,
|
|
||||||
_caller_id: Uuid,
|
|
||||||
) -> Result<File, DomainError> {
|
|
||||||
unimplemented!()
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn move_file(
|
|
||||||
&self,
|
|
||||||
file_id: &str,
|
|
||||||
_target_folder_id: Option<String>,
|
|
||||||
_caller_id: Uuid,
|
|
||||||
) -> Result<File, DomainError> {
|
|
||||||
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<File, DomainError> {
|
|
||||||
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<i64>,
|
|
||||||
_caller_id: Uuid,
|
|
||||||
) -> Result<(String, i64), DomainError> {
|
|
||||||
Ok((String::new(), 0))
|
|
||||||
}
|
|
||||||
|
|
||||||
async fn register_file_deferred(
|
|
||||||
&self,
|
|
||||||
_name: String,
|
|
||||||
_folder_id: Option<String>,
|
|
||||||
_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<String>,
|
|
||||||
_new_name: Option<&str>,
|
|
||||||
_caller_id: Uuid,
|
|
||||||
) -> Result<File, DomainError> {
|
|
||||||
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<FileBlobWriteRepository>). 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"
|
|
||||||
);
|
|
||||||
}
|
|
||||||
@@ -39,8 +39,6 @@ pub mod wopi_token_service;
|
|||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod batch_operations_test;
|
mod batch_operations_test;
|
||||||
#[cfg(test)]
|
#[cfg(test)]
|
||||||
mod idor_protection_test;
|
|
||||||
#[cfg(test)]
|
|
||||||
mod trash_service_test;
|
mod trash_service_test;
|
||||||
|
|
||||||
// Re-exportar para facilitar acceso
|
// Re-exportar para facilitar acceso
|
||||||
|
|||||||
@@ -928,14 +928,6 @@ mod tests {
|
|||||||
> {
|
> {
|
||||||
Ok(Box::pin(futures::stream::empty()))
|
Ok(Box::pin(futures::stream::empty()))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn get_file_for_owner(
|
|
||||||
&self,
|
|
||||||
id: &str,
|
|
||||||
_owner_id: Uuid,
|
|
||||||
) -> Result<crate::domain::entities::file::File, DomainError> {
|
|
||||||
self.get_file(id).await
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl FolderRepository for MockFolderRepository {
|
impl FolderRepository for MockFolderRepository {
|
||||||
|
|||||||
@@ -554,15 +554,6 @@ impl FileReadPort for MockFileRepository {
|
|||||||
> {
|
> {
|
||||||
Ok(Box::pin(futures::stream::empty()))
|
Ok(Box::pin(futures::stream::empty()))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn get_file_for_owner(
|
|
||||||
&self,
|
|
||||||
id: &str,
|
|
||||||
_owner_id: Uuid,
|
|
||||||
) -> std::result::Result<File, DomainError> {
|
|
||||||
// In this mock, ignore ownership — trash tests don't focus on ownership
|
|
||||||
self.get_file(id).await
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
impl FileWritePort for MockFileRepository {
|
impl FileWritePort for MockFileRepository {
|
||||||
|
|||||||
@@ -145,10 +145,6 @@ impl FileReadPort for StubFileReadPort {
|
|||||||
) -> Result<Pin<Box<dyn Stream<Item = Result<File, DomainError>> + Send>>, DomainError> {
|
) -> Result<Pin<Box<dyn Stream<Item = Result<File, DomainError>> + Send>>, DomainError> {
|
||||||
Ok(Box::pin(futures::stream::empty()))
|
Ok(Box::pin(futures::stream::empty()))
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn get_file_for_owner(&self, _id: &str, _owner_id: Uuid) -> Result<File, DomainError> {
|
|
||||||
Ok(File::default())
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -720,54 +720,6 @@ impl FileReadPort for FileBlobReadRepository {
|
|||||||
)
|
)
|
||||||
}
|
}
|
||||||
|
|
||||||
async fn get_file_for_owner(&self, id: &str, owner_id: Uuid) -> Result<File, DomainError> {
|
|
||||||
let row = sqlx::query_as::<
|
|
||||||
_,
|
|
||||||
(
|
|
||||||
String, // id
|
|
||||||
String, // name
|
|
||||||
Option<String>, // folder_id
|
|
||||||
Option<String>, // folder path
|
|
||||||
i64, // size
|
|
||||||
String, // mime_type
|
|
||||||
i64, // created_at
|
|
||||||
i64, // updated_at
|
|
||||||
String, // blob_hash
|
|
||||||
Option<Uuid>, // user_id (owner)
|
|
||||||
Option<Uuid>, // created_by (§14)
|
|
||||||
Option<Uuid>, // 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)]
|
#[allow(clippy::type_complexity)]
|
||||||
async fn list_files(&self, folder_id: Option<&str>) -> Result<Vec<File>, DomainError> {
|
async fn list_files(&self, folder_id: Option<&str>) -> Result<Vec<File>, DomainError> {
|
||||||
let rows: Vec<FileRow> = if let Some(fid) = folder_id {
|
let rows: Vec<FileRow> = if let Some(fid) = folder_id {
|
||||||
@@ -821,68 +773,6 @@ impl FileReadPort for FileBlobReadRepository {
|
|||||||
.collect()
|
.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<Vec<File>, DomainError> {
|
|
||||||
let rows: Vec<FileRow> = 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<String, DomainError> {
|
async fn get_blob_hash(&self, file_id: &str) -> Result<String, DomainError> {
|
||||||
self.resolve_blob_hash(file_id).await
|
self.resolve_blob_hash(file_id).await
|
||||||
}
|
}
|
||||||
@@ -955,78 +845,6 @@ impl FileReadPort for FileBlobReadRepository {
|
|||||||
.collect()
|
.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<Vec<File>, DomainError> {
|
|
||||||
let rows: Vec<FileRow> = 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(
|
async fn get_file_stream(
|
||||||
&self,
|
&self,
|
||||||
id: &str,
|
id: &str,
|
||||||
|
|||||||
@@ -106,9 +106,12 @@ impl FolderHandler {
|
|||||||
Self::list_folders_scoped(service, None, &auth_user).await
|
Self::list_folders_scoped(service, None, &auth_user).await
|
||||||
}
|
}
|
||||||
|
|
||||||
/// Internal helper: lists folders scoped to the authenticated user.
|
/// Internal helper: lists folders the authenticated caller can Read.
|
||||||
/// Uses `list_folders_for_owner` — the DB query filters by `user_id`,
|
/// Post-PR-B, `list_root_folders_for_caller` scopes via
|
||||||
/// so no data from other users ever leaves the database.
|
/// 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(
|
async fn list_folders_scoped(
|
||||||
service: AppState,
|
service: AppState,
|
||||||
parent_id: Option<&str>,
|
parent_id: Option<&str>,
|
||||||
|
|||||||
Reference in New Issue
Block a user