From 09790644f337dd5a3087d27658bda41da558604d Mon Sep 17 00:00:00 2001 From: Edouard Vanbelle Date: Thu, 2 Jul 2026 00:45:11 +0200 Subject: [PATCH] feat(drive): repl user_id by caller has read access repl user_id by caller has read access in readonly functions using CALLER_CAN_READ_DRIVE constant ensure webdav preview is using the permission handler --- src/application/ports/storage_ports.rs | 22 +- src/application/services/folder_service.rs | 32 +-- src/application/services/places_service.rs | 7 +- src/application/services/search_service.rs | 6 +- src/domain/repositories/folder_repository.rs | 26 ++- .../pg/file_blob_read_repository.rs | 77 +++++-- .../repositories/pg/folder_db_repository.rs | 196 ++++++++++-------- src/interfaces/api/handlers/webdav_handler.rs | 3 +- src/interfaces/nextcloud/preview_handler.rs | 28 ++- 9 files changed, 249 insertions(+), 148 deletions(-) diff --git a/src/application/ports/storage_ports.rs b/src/application/ports/storage_ports.rs index 6815985f..77347c89 100644 --- a/src/application/ports/storage_ports.rs +++ b/src/application/ports/storage_ports.rs @@ -195,7 +195,10 @@ pub trait FileReadPort: Send + Sync + 'static { /// # Arguments /// * `folder_id` - Optional folder ID to scope the search (for recursive search, pass None) /// * `criteria` - Search criteria including name_contains, file_types, date ranges, size ranges - /// * `user_id` - User ID for ownership filtering + /// * `caller_id` - Caller user id — scoped by drive-membership grants + /// (`role_grants` on `resource_type='drive'`) rather than the legacy + /// `files.user_id` column. Group memberships (direct + transitive) + /// are expanded inline via `storage.caller_group_ids($caller)`. /// /// # Returns /// A tuple of (files, total_count) where files are paginated and filtered @@ -203,36 +206,39 @@ pub trait FileReadPort: Send + Sync + 'static { &self, folder_id: Option<&str>, criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result<(Vec, usize), DomainError>; /// Search files recursively in a folder subtree using ltree. /// /// When `root_folder_id` is Some, uses ltree descendant queries to find - /// all files within the subtree rooted at that folder. When None, searches - /// all files for the user. This replaces the O(N) recursive spawn-per-folder - /// approach with O(1) SQL queries. + /// all files within the subtree rooted at that folder. When None, + /// delegates to `search_files_paginated`. + /// + /// Post-PR-B: scoped by drive-membership grants (same semantics as + /// `search_files_paginated`), not by `files.user_id`. /// /// Returns a tuple of (matching files, total count for pagination). async fn search_files_in_subtree( &self, root_folder_id: Option<&str>, criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result<(Vec, usize), DomainError> { // Default: delegate to paginated search (non-recursive fallback) - self.search_files_paginated(root_folder_id, criteria, user_id) + self.search_files_paginated(root_folder_id, criteria, caller_id) .await } /// Count files matching the search criteria (without loading them). /// /// Used for pagination metadata without fetching the actual files. + /// Same drive-membership scoping as `search_files_paginated`. async fn count_files( &self, folder_id: Option<&str>, criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result; /// Return up to `limit` files whose name contains `query` (case-insensitive). diff --git a/src/application/services/folder_service.rs b/src/application/services/folder_service.rs index b0888682..c6e484ec 100644 --- a/src/application/services/folder_service.rs +++ b/src/application/services/folder_service.rs @@ -431,23 +431,23 @@ impl FolderUseCase for FolderService { return self.list_folders_paginated(parent_id, &pagination).await; } else { let (folders, total_items) = self - .folder_storage - .list_root_folders_for_caller_paginated( - owner_id, - pagination.offset(), - pagination.limit(), - true, - ) - .await - .map_err(|e| { - DomainError::internal_error( - "FolderStorage", - format!( - "Failed to list root folders for caller '{}' with pagination: {}", - owner_id, e - ), + .folder_storage + .list_root_folders_for_caller_paginated( + owner_id, + pagination.offset(), + pagination.limit(), + true, ) - })?; + .await + .map_err(|e| { + DomainError::internal_error( + "FolderStorage", + format!( + "Failed to list root folders for caller '{}' with pagination: {}", + owner_id, e + ), + ) + })?; let total = total_items.unwrap_or(folders.len()); diff --git a/src/application/services/places_service.rs b/src/application/services/places_service.rs index 13956c23..cb538d3b 100644 --- a/src/application/services/places_service.rs +++ b/src/application/services/places_service.rs @@ -9,9 +9,10 @@ use crate::infrastructure::repositories::pg::FileBlobReadRepository; /// "Places" use case: the caller's geotagged photos aggregated into map /// clusters. /// -/// Post-§15 the surface follows the Photos scope: default personal drive -/// + drives where `policies.include_in_photo_index = true` AND caller -/// has Read. Group-membership expansion is handled inline by +/// Post-§15 the surface follows the Photos scope: drives where the +/// caller has Read AND `policies.include_in_photo_index = true` +/// (default personal drives materialise the flag at creation). +/// Group-membership expansion is handled inline by /// `storage.caller_group_ids(caller)` inside the repo's SQL, so this /// service is a thin coordinate-math wrapper — no engine dependency. pub struct PlacesService { diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 4bda37b5..92a35e05 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -328,7 +328,11 @@ impl SearchService { } }; match authz - .check(Subject::User(user_id), Permission::Read, Resource::File(file_uuid)) + .check( + Subject::User(user_id), + Permission::Read, + Resource::File(file_uuid), + ) .await { Ok(true) => verified.push(hit), diff --git a/src/domain/repositories/folder_repository.rs b/src/domain/repositories/folder_repository.rs index 22572d33..011695ab 100644 --- a/src/domain/repositories/folder_repository.rs +++ b/src/domain/repositories/folder_repository.rs @@ -179,31 +179,35 @@ pub trait FolderRepository: Send + Sync + 'static { Ok(Vec::new()) } - /// Lists all descendant folders in a subtree (ltree-based). + /// Lists all descendant folders in a subtree (ltree-based), scoped + /// to drives the caller can read. /// - /// Returns all folders whose lpath is a descendant of the given folder's - /// lpath. Used for recursive search — O(1) SQL via GiST index instead - /// of O(N) recursive traversal. + /// Returns all folders whose lpath is a descendant of the given + /// folder's lpath. Used for recursive search — O(1) SQL via GiST + /// index instead of O(N) recursive traversal. Drive-membership + /// filtering (including group cascade via `caller_group_ids`) is + /// applied inline in the SQL. /// /// The default implementation returns an empty vec (stubs / mocks). async fn list_descendant_folders( &self, folder_id: &str, name_contains: Option<&str>, - user_id: Uuid, + caller_id: Uuid, ) -> Result, DomainError> { - let _ = (folder_id, name_contains, user_id); + let _ = (folder_id, name_contains, caller_id); Ok(Vec::new()) } - /// Search folders with SQL-level filtering by name, user, and scope. + /// Search folders with SQL-level filtering by name and scope, + /// restricted to drives the caller can read. /// /// - **Non-recursive** (`recursive = false`): searches direct children of /// `parent_id` (or root folders when `None`). /// - **Recursive with `parent_id`**: delegates to `list_descendant_folders` /// (ltree GiST-indexed scan). - /// - **Recursive without `parent_id`**: searches ALL folders owned by - /// `user_id` with optional name filter in SQL. + /// - **Recursive without `parent_id`**: searches ALL folders in drives + /// the caller can read, with optional name filter in SQL. /// /// The default implementation falls back to `list_folders` + in-memory /// filter so that stubs and mocks compile without changes. @@ -211,13 +215,13 @@ pub trait FolderRepository: Send + Sync + 'static { &self, parent_id: Option<&str>, name_contains: Option<&str>, - user_id: Uuid, + caller_id: Uuid, recursive: bool, ) -> Result, DomainError> { // Recursive with folder_id → use optimised ltree scan if recursive && let Some(fid) = parent_id { return self - .list_descendant_folders(fid, name_contains, user_id) + .list_descendant_folders(fid, name_contains, caller_id) .await; } // Fallback: load + filter in memory (stubs / mocks) diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index 5003908a..b9ea54b8 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -43,6 +43,37 @@ use crate::domain::services::path_service::StoragePath; use crate::infrastructure::services::dedup_service::DedupService; use uuid::Uuid; +/// SQL `EXISTS (…)` predicate — true when the caller (bound to `$1`) has +/// any active `role_grants` on the drive owning `fi` (the aliased file +/// row). Group memberships (direct + transitive) are expanded inline via +/// `storage.caller_group_ids($1)` (recursive; see migration +/// `20260901000002_caller_group_ids_function.sql`). +/// +/// Used by every drive-scoped file search query in this repo: +/// - `search_files_paginated` +/// - `search_files_in_subtree` +/// +/// **Alias contract**: queries splicing this in MUST alias +/// `storage.files` as `fi`. `$1` is reserved for `caller_id`; other bind +/// params start at `$2`. +/// +/// This mirrors — but is intentionally not shared with — the folder +/// variant in `folder_db_repository.rs` (aliased `fo.drive_id`) and the +/// drive-listing shapes in `drive_pg_repository`/`list_media_files`. +/// When the grant model changes, update all sites in parallel. +const CALLER_CAN_READ_DRIVE: &str = "EXISTS (\ + SELECT 1 \ + FROM storage.role_grants g \ + WHERE g.resource_type = 'drive' \ + AND g.resource_id = fi.drive_id \ + AND (g.expires_at IS NULL OR g.expires_at > NOW()) \ + AND ( \ + (g.subject_type = 'user' AND g.subject_id = $1) \ + OR (g.subject_type = 'group' AND g.subject_id IN \ + (SELECT storage.caller_group_ids($1))) \ + ) \ + )"; + /// Type alias for file metadata rows from SQL queries. /// Fields: id, name, folder_id, folder_path, size, mime_type, /// created_at, updated_at, blob_hash, user_id, created_by, updated_by. @@ -200,7 +231,7 @@ impl FileBlobReadRepository { &self, ids: &[String], criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result, DomainError> { // Index hits are externally produced strings — parse defensively. let uuid_ids: Vec = ids.iter().filter_map(|id| id.parse().ok()).collect(); @@ -208,9 +239,15 @@ impl FileBlobReadRepository { return Ok(Vec::new()); } + // Post-PR-B: drive-membership scoping via + // [`CALLER_CAN_READ_DRIVE`] (bound to `$1`) replaces the legacy + // `fi.user_id = $caller` predicate. Group grants are honoured + // inline through `storage.caller_group_ids`. + // + // Bind order: $1 = caller_id, $2 = ids array, $3.. = criteria. let mut conditions: Vec = vec![ - "fi.id = ANY($1)".to_string(), - "fi.user_id = $2".to_string(), + CALLER_CAN_READ_DRIVE.to_string(), + "fi.id = ANY($2)".to_string(), "fi.is_trashed = false".to_string(), ]; let mut bind_idx = 2u32; @@ -242,8 +279,8 @@ impl FileBlobReadRepository { ); let mut query = sqlx::query_as::<_, FileRow>(&sql) - .bind(uuid_ids) - .bind(user_id); + .bind(caller_id) + .bind(uuid_ids); if let Some(folder_id) = criteria.folder_id.as_deref() { query = query.bind(folder_id); } @@ -1263,11 +1300,15 @@ impl FileReadPort for FileBlobReadRepository { /// Uses `COUNT(*) OVER()` window function to return the total matching /// count alongside the paginated rows in a **single query** — no separate /// COUNT round-trip. + /// + /// Post-PR-B: scoped by drive-membership (via [`CALLER_CAN_READ_DRIVE`]) + /// rather than the legacy `fi.user_id = $caller` predicate. Group + /// grants are honoured inline through `storage.caller_group_ids`. async fn search_files_paginated( &self, folder_id: Option<&str>, criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result<(Vec, usize), DomainError> { let offset = criteria.offset as i64; let limit = criteria.limit as i64; @@ -1285,10 +1326,10 @@ impl FileReadPort for FileBlobReadRepository { // ── Build dynamic WHERE + bind indices ─────────────────────────── let mut conditions: Vec = vec![ - "fi.user_id = $1".to_string(), + CALLER_CAN_READ_DRIVE.to_string(), "fi.is_trashed = false".to_string(), ]; - let mut bind_idx = 1u32; // $1 = user_id + let mut bind_idx = 1u32; // $1 = caller_id if folder_id.is_some() { bind_idx += 1; @@ -1341,7 +1382,7 @@ impl FileReadPort for FileBlobReadRepository { i64, ), >(&sql) - .bind(user_id); + .bind(caller_id); if let Some(fid) = folder_id { query = query.bind(fid); @@ -1386,16 +1427,20 @@ impl FileReadPort for FileBlobReadRepository { /// /// Uses `COUNT(*) OVER()` to return the total count alongside the /// paginated rows — no separate COUNT round-trip. + /// + /// Post-PR-B: scoped by drive-membership (via [`CALLER_CAN_READ_DRIVE`]) + /// rather than the legacy `fi.user_id = $caller` predicate — same + /// group-cascade semantics as `search_files_paginated`. async fn search_files_in_subtree( &self, root_folder_id: Option<&str>, criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result<(Vec, usize), DomainError> { // When no root folder specified, delegate to existing paginated search let root_id = match root_folder_id { None => { - return self.search_files_paginated(None, criteria, user_id).await; + return self.search_files_paginated(None, criteria, caller_id).await; } Some(id) => id, }; @@ -1416,10 +1461,10 @@ impl FileReadPort for FileBlobReadRepository { // ── Build dynamic WHERE clauses ── let mut conditions = Vec::new(); - let mut bind_idx = 2u32; // $1 = user_id, $2 = root_folder_id + let mut bind_idx = 2u32; // $1 = caller_id, $2 = root_folder_id conditions.push("fi.is_trashed = false".to_string()); - conditions.push("fi.user_id = $1".to_string()); + conditions.push(CALLER_CAN_READ_DRIVE.to_string()); conditions.push( "fo.lpath <@ (SELECT lpath FROM storage.folders WHERE id = $2::uuid)".to_string(), ); @@ -1472,7 +1517,7 @@ impl FileReadPort for FileBlobReadRepository { i64, ), >(&sql) - .bind(user_id) + .bind(caller_id) .bind(root_id); if let Some(name) = &criteria.name_contains @@ -1513,10 +1558,10 @@ impl FileReadPort for FileBlobReadRepository { &self, folder_id: Option<&str>, criteria: &SearchCriteriaDto, - user_id: Uuid, + caller_id: Uuid, ) -> Result { let (_, count) = self - .search_files_paginated(folder_id, criteria, user_id) + .search_files_paginated(folder_id, criteria, caller_id) .await?; Ok(count) } diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index d11d322f..5ecdd58a 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -75,6 +75,39 @@ type FolderRowOptUser = ( Option, ); +/// SQL `EXISTS (…)` predicate — true when the caller (bound to `$1`) has +/// any active `role_grants` on the drive owning `fo` (the aliased folder +/// row). Group memberships (direct + transitive) are expanded inline via +/// `storage.caller_group_ids($1)` (recursive; see migration +/// `20260901000002_caller_group_ids_function.sql`). +/// +/// Used by every drive-scoped folder query in this repo: +/// - `list_root_folders_for_caller` / `_paginated` +/// - `search_folders` (all three branches) +/// - `list_descendant_folders` +/// +/// **Alias contract**: queries splicing this in MUST alias +/// `storage.folders` as `fo`. `$1` is reserved for `caller_id`; other +/// bind params start at `$2`. +/// +/// This mirrors — but is not shared with — the drive-membership shape in +/// `drive_pg_repository::list_readable_by` (uses `JOIN` on `d.id`) and +/// the media-scoping subqueries in `file_blob_read_repository` (use +/// `IN (SELECT d.id …)` with the `include_in_photo_index` policy +/// filter). When the grant model changes, update all sites in parallel. +const CALLER_CAN_READ_DRIVE: &str = "EXISTS (\ + SELECT 1 \ + FROM storage.role_grants g \ + WHERE g.resource_type = 'drive' \ + AND g.resource_id = fo.drive_id \ + AND (g.expires_at IS NULL OR g.expires_at > NOW()) \ + AND ( \ + (g.subject_type = 'user' AND g.subject_id = $1) \ + OR (g.subject_type = 'group' AND g.subject_id IN \ + (SELECT storage.caller_group_ids($1))) \ + ) \ + )"; + /// PostgreSQL-backed folder repository. /// /// All folder metadata lives in the `storage.folders` table. The physical @@ -412,35 +445,26 @@ impl FolderRepository for FolderDbRepository { // `project_caller_role_on_file_folder_dto` and the note at the // top of `folder_repository.rs`. Frontend cross-references // `/api/drives::caller_role` via `folder.drive_id`. - let rows: Vec = sqlx::query_as( - r#" - SELECT f.id::text, f.name, f.path, f.parent_id::text, f.user_id, f.drive_id, - EXTRACT(EPOCH FROM f.created_at)::bigint, - EXTRACT(EPOCH FROM f.updated_at)::bigint, - EXTRACT(EPOCH FROM f.tree_modified_at)::bigint, - f.created_by, f.updated_by - FROM storage.folders f - WHERE f.parent_id IS NULL - AND NOT f.is_trashed - AND EXISTS ( - SELECT 1 - FROM storage.role_grants g - WHERE g.resource_type = 'drive' - AND g.resource_id = f.drive_id - AND (g.expires_at IS NULL OR g.expires_at > NOW()) - AND ( - (g.subject_type = 'user' AND g.subject_id = $1) - OR (g.subject_type = 'group' AND g.subject_id IN - (SELECT storage.caller_group_ids($1))) - ) - ) - ORDER BY f.name - "#, - ) - .bind(caller_id) - .fetch_all(self.pool()) - .await - .map_err(|e| DomainError::internal_error("FolderDb", format!("list_root_folders: {e}")))?; + let sql = format!( + "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ + fo.user_id, fo.drive_id, \ + EXTRACT(EPOCH FROM fo.created_at)::bigint, \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ + fo.created_by, fo.updated_by \ + FROM storage.folders fo \ + WHERE fo.parent_id IS NULL \ + AND NOT fo.is_trashed \ + AND {CALLER_CAN_READ_DRIVE} \ + ORDER BY fo.name" + ); + let rows: Vec = sqlx::query_as(&sql) + .bind(caller_id) + .fetch_all(self.pool()) + .await + .map_err(|e| { + DomainError::internal_error("FolderDb", format!("list_root_folders: {e}")) + })?; rows.into_iter() .map(|(id, name, path, pid, uid, did, ca, ma, tma, cb, ub)| { @@ -531,41 +555,30 @@ impl FolderRepository for FolderDbRepository { limit: usize, include_total: bool, ) -> Result<(Vec, Option), DomainError> { - let rows: Vec = sqlx::query_as( - r#" - SELECT f.id::text, f.name, f.path, f.parent_id::text, f.user_id, f.drive_id, - EXTRACT(EPOCH FROM f.created_at)::bigint, - EXTRACT(EPOCH FROM f.updated_at)::bigint, - EXTRACT(EPOCH FROM f.tree_modified_at)::bigint, - f.created_by, f.updated_by, - COUNT(*) OVER() AS total_count - FROM storage.folders f - WHERE f.parent_id IS NULL - AND NOT f.is_trashed - AND EXISTS ( - SELECT 1 - FROM storage.role_grants g - WHERE g.resource_type = 'drive' - AND g.resource_id = f.drive_id - AND (g.expires_at IS NULL OR g.expires_at > NOW()) - AND ( - (g.subject_type = 'user' AND g.subject_id = $1) - OR (g.subject_type = 'group' AND g.subject_id IN - (SELECT storage.caller_group_ids($1))) - ) - ) - ORDER BY f.name - LIMIT $2 OFFSET $3 - "#, - ) - .bind(caller_id) - .bind(limit as i64) - .bind(offset as i64) - .fetch_all(self.pool()) - .await - .map_err(|e| { - DomainError::internal_error("FolderDb", format!("list_root_folders_paginated: {e}")) - })?; + let sql = format!( + "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ + fo.user_id, fo.drive_id, \ + EXTRACT(EPOCH FROM fo.created_at)::bigint, \ + EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ + fo.created_by, fo.updated_by, \ + COUNT(*) OVER() AS total_count \ + FROM storage.folders fo \ + WHERE fo.parent_id IS NULL \ + AND NOT fo.is_trashed \ + AND {CALLER_CAN_READ_DRIVE} \ + ORDER BY fo.name \ + LIMIT $2 OFFSET $3" + ); + let rows: Vec = sqlx::query_as(&sql) + .bind(caller_id) + .bind(limit as i64) + .bind(offset as i64) + .fetch_all(self.pool()) + .await + .map_err(|e| { + DomainError::internal_error("FolderDb", format!("list_root_folders_paginated: {e}")) + })?; let total = if include_total { Some(rows.first().map_or(0, |r| r.11) as usize) @@ -1018,19 +1031,22 @@ impl FolderRepository for FolderDbRepository { /// /// - Non-recursive: `WHERE parent_id = $1 AND user_id = $2 [AND LIKE]` /// - Recursive + folder_id: delegates to `list_descendant_folders` - /// - Recursive + no folder_id: `WHERE user_id = $1 [AND LIKE]` + /// - Recursive + no folder_id: drive-scoped `EXISTS role_grants` [AND LIKE] + /// + /// Post-PR-B: filters by drive-membership grants inline (via + /// `caller_group_ids`) instead of `user_id = $caller`. #[allow(clippy::type_complexity)] async fn search_folders( &self, parent_id: Option<&str>, name_contains: Option<&str>, - user_id: Uuid, + caller_id: Uuid, recursive: bool, ) -> Result, DomainError> { // Recursive with folder scope → existing optimised ltree scan if recursive && let Some(fid) = parent_id { return self - .list_descendant_folders(fid, name_contains, user_id) + .list_descendant_folders(fid, name_contains, caller_id) .await; } @@ -1049,7 +1065,7 @@ impl FolderRepository for FolderDbRepository { }; if recursive { - // Recursive, no folder scope → ALL user folders + // Recursive, no folder scope → ALL folders in caller's readable drives let sql = format!( "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ fo.user_id, fo.drive_id, \ @@ -1058,7 +1074,7 @@ impl FolderRepository for FolderDbRepository { EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ fo.created_by, fo.updated_by \ FROM storage.folders fo \ - WHERE fo.user_id = $1 \ + WHERE {CALLER_CAN_READ_DRIVE} \ AND fo.is_trashed = false \ {name_clause} \ ORDER BY fo.name" @@ -1066,13 +1082,13 @@ impl FolderRepository for FolderDbRepository { let rows: Vec = if let Some(ref pattern) = name_pattern { sqlx::query_as(&sql) - .bind(user_id) + .bind(caller_id) .bind(pattern) .fetch_all(self.pool()) .await } else { sqlx::query_as(&sql) - .bind(user_id) + .bind(caller_id) .fetch_all(self.pool()) .await } @@ -1086,7 +1102,8 @@ impl FolderRepository for FolderDbRepository { .collect(); } - // Non-recursive: direct children of parent_id, filtered by user + // Non-recursive: direct children of parent_id, restricted to drives + // the caller can read (parent_id already establishes the subtree). let sql = if parent_id.is_some() { format!( "SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, \ @@ -1096,14 +1113,14 @@ impl FolderRepository for FolderDbRepository { EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ fo.created_by, fo.updated_by \ FROM storage.folders fo \ - WHERE fo.parent_id = $1::uuid \ - AND fo.user_id = $2 \ + WHERE fo.parent_id = $2::uuid \ + AND {CALLER_CAN_READ_DRIVE} \ AND fo.is_trashed = false \ {name_clause} \ ORDER BY fo.name" ) } else { - // Root folders: parent_id IS NULL, reindex params ($1=user_id, $2=pattern) + // Root folders: parent_id IS NULL, params ($1=caller_id, $2=pattern) let name_clause_root = match name_contains { Some(name) if name.len() >= 3 => " AND fo.name ILIKE $2", _ => "", @@ -1117,7 +1134,7 @@ impl FolderRepository for FolderDbRepository { fo.created_by, fo.updated_by \ FROM storage.folders fo \ WHERE fo.parent_id IS NULL \ - AND fo.user_id = $1 \ + AND {CALLER_CAN_READ_DRIVE} \ AND fo.is_trashed = false \ {name_clause_root} \ ORDER BY fo.name" @@ -1127,27 +1144,27 @@ impl FolderRepository for FolderDbRepository { let rows: Vec = if let Some(pid) = parent_id { if let Some(ref pattern) = name_pattern { sqlx::query_as(&sql) + .bind(caller_id) .bind(pid) - .bind(user_id) .bind(pattern) .fetch_all(self.pool()) .await } else { sqlx::query_as(&sql) + .bind(caller_id) .bind(pid) - .bind(user_id) .fetch_all(self.pool()) .await } } else if let Some(ref pattern) = name_pattern { sqlx::query_as(&sql) - .bind(user_id) + .bind(caller_id) .bind(pattern) .fetch_all(self.pool()) .await } else { sqlx::query_as(&sql) - .bind(user_id) + .bind(caller_id) .fetch_all(self.pool()) .await } @@ -1160,16 +1177,19 @@ impl FolderRepository for FolderDbRepository { .collect() } - /// Lists all descendant folders in a subtree using ltree GiST index. + /// Lists all descendant folders in a subtree using ltree GiST index, + /// scoped to drives the caller can read. /// /// Single SQL query: `fo.lpath <@ (root's lpath)` fetches the entire - /// subtree in one indexed scan. Optional name filter is pushed to SQL. + /// subtree in one indexed scan. Post-PR-B: drive-membership filter + /// inline via `caller_group_ids`, replacing the legacy + /// `fo.user_id = $caller` predicate. #[allow(clippy::type_complexity)] async fn list_descendant_folders( &self, folder_id: &str, name_contains: Option<&str>, - user_id: Uuid, + caller_id: Uuid, ) -> Result, DomainError> { let (where_extra, name_pattern) = match name_contains { Some(name) if name.len() >= 3 => { @@ -1183,10 +1203,10 @@ impl FolderRepository for FolderDbRepository { fo.user_id, fo.drive_id, \ EXTRACT(EPOCH FROM fo.created_at)::bigint, \ EXTRACT(EPOCH FROM fo.updated_at)::bigint, \ - EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ - fo.created_by, fo.updated_by \ + EXTRACT(EPOCH FROM fo.tree_modified_at)::bigint, \ + fo.created_by, fo.updated_by \ FROM storage.folders fo \ - WHERE fo.user_id = $1 \ + WHERE {CALLER_CAN_READ_DRIVE} \ AND fo.is_trashed = false \ AND fo.lpath <@ (SELECT lpath FROM storage.folders WHERE id = $2::uuid) \ AND fo.id != $2::uuid \ @@ -1196,14 +1216,14 @@ impl FolderRepository for FolderDbRepository { let rows: Vec = if let Some(ref pattern) = name_pattern { sqlx::query_as(&sql) - .bind(user_id) + .bind(caller_id) .bind(folder_id) .bind(pattern) .fetch_all(self.pool()) .await } else { sqlx::query_as(&sql) - .bind(user_id) + .bind(caller_id) .bind(folder_id) .fetch_all(self.pool()) .await diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index 8cf5de3f..01b3821b 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -22,6 +22,7 @@ use crate::application::adapters::webdav_adapter::{ }; use crate::application::dtos::file_dto::FileDto; use crate::application::dtos::folder_dto::FolderDto; +use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::file_ports::FileRetrievalUseCase; use crate::application::ports::file_ports::{FileManagementUseCase, FileUploadUseCase}; use crate::application::ports::folder_ports::FolderUseCase; @@ -29,7 +30,6 @@ 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; @@ -155,7 +155,6 @@ fn extract_user(req: &Request) -> Result { .ok_or_else(|| AppError::unauthorized("Authentication required")) } - /** * Creates and returns the WebDAV router with all required endpoints. * diff --git a/src/interfaces/nextcloud/preview_handler.rs b/src/interfaces/nextcloud/preview_handler.rs index fd75cd10..3b8ab98c 100644 --- a/src/interfaces/nextcloud/preview_handler.rs +++ b/src/interfaces/nextcloud/preview_handler.rs @@ -11,11 +11,14 @@ use axum::{ use serde::Deserialize; use std::sync::Arc; +use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::file_ports::FileRetrievalUseCase; use crate::application::ports::storage_ports::FileReadPort; use crate::application::ports::thumbnail_ports::{ThumbnailFormat, ThumbnailPort, ThumbnailSize}; use crate::common::di::AppState; +use crate::domain::services::authorization::{Permission, Resource, Subject}; use crate::interfaces::middleware::auth::AuthUser; +use uuid::Uuid; #[derive(Debug, Deserialize)] pub struct PreviewParams { @@ -89,9 +92,28 @@ pub async fn handle_preview( } }; - // Verify the authenticated user owns this file - let user_id_str = user.id.to_string(); - if file.owner_id.as_deref() != Some(user_id_str.as_str()) { + // Verify the authenticated user can Read this file. Anti-enum: any + // AuthZ denial surfaces as 404 (same shape as "unknown file" above), + // and the engine emits an `authz.denied` audit line internally. + let file_uuid = match Uuid::parse_str(&file.id) { + Ok(u) => u, + Err(_) => { + return Response::builder() + .status(StatusCode::NOT_FOUND) + .body(Body::from("File not found")) + .unwrap(); + } + }; + if state + .authorization + .require( + Subject::User(user.id), + Permission::Read, + Resource::File(file_uuid), + ) + .await + .is_err() + { return Response::builder() .status(StatusCode::NOT_FOUND) .body(Body::from("File not found"))