diff --git a/src/application/services/recent_service.rs b/src/application/services/recent_service.rs index 54482606..dbb9f610 100644 --- a/src/application/services/recent_service.rs +++ b/src/application/services/recent_service.rs @@ -3,7 +3,7 @@ use crate::application::dtos::recent_dto::{RecentCursor, RecentItemDto, RecentRe use crate::application::ports::authorization_ports::AuthorizationEngine; use crate::application::ports::recent_ports::{RecentItemsRepositoryPort, RecentItemsUseCase}; use crate::application::ports::resource_access_hook::ResourceAccessHook; -use crate::common::errors::Result; +use crate::common::errors::{DomainError, Result}; use crate::domain::services::authorization::{Permission, Resource, ResourceKind, Subject}; use crate::infrastructure::repositories::pg::RecentItemsPgRepository; use crate::infrastructure::services::pg_acl_engine::PgAclEngine; @@ -67,6 +67,41 @@ impl RecentService { hook.on_recents_cleared(user_id); } } + + /// Record access to an item WITHOUT the pre-write `authz.require` + /// gate. Callers must have gated the caller's Read upstream — this + /// method exists for the `RecentRecordingHook` fast path: writes + /// that reach the hook have already passed a `_with_perms` service + /// method (uploads, streams, GETs, etc.), so re-checking here + /// would be pure duplicate work AND widen the race window between + /// the POST response and the `tokio::spawn`ed upsert ( + /// `tests/api/recent.hurl` step 7 hits this — the extra SQL + /// round-trip pushes the upsert past the client's immediate + /// `GET /api/recent/resources`). + /// + /// **Do NOT call this from an externally-reachable handler.** The + /// REST endpoint goes through the trait method `record_item_access` + /// below, which enforces the Read gate per AGENTS.md convention. + pub async fn record_item_access_internal( + &self, + user_id: Uuid, + item_id: &str, + item_type: &str, + ) -> Result<()> { + // Type validation only — no authz, no resource parse for the + // engine (the hook path is already resource-typed by construction). + if item_type != "file" && item_type != "folder" { + return Err(DomainError::new( + crate::common::errors::ErrorKind::InvalidInput, + "RecentItems", + "Item type must be 'file' or 'folder'", + )); + } + + self.repo.upsert_access(user_id, item_id, item_type).await?; + self.repo.prune(user_id, self.max_recent_items).await?; + Ok(()) + } } impl RecentItemsUseCase for RecentService { @@ -107,13 +142,18 @@ impl RecentItemsUseCase for RecentService { // gate the write path was an information oracle over the // whole tenant via the listing endpoint's JOIN back to // storage.files/folders. + // + // Internal hook callers (RecentRecordingHook) bypass the + // trait entry point and call `record_item_access_internal` + // directly — Read has already been enforced upstream on + // whatever `_with_perms` service produced the access event. let resource = Resource::parse(item_type, item_id)?; self.authorization .require(Subject::User(user_id), Permission::Read, resource) .await?; - self.repo.upsert_access(user_id, item_id, item_type).await?; - self.repo.prune(user_id, self.max_recent_items).await?; + self.record_item_access_internal(user_id, item_id, item_type) + .await?; info!( "Successfully recorded access to {} '{}' for user {}", diff --git a/src/infrastructure/services/recent_recording_hook.rs b/src/infrastructure/services/recent_recording_hook.rs index f6ee9cb3..041e3737 100644 --- a/src/infrastructure/services/recent_recording_hook.rs +++ b/src/infrastructure/services/recent_recording_hook.rs @@ -29,7 +29,6 @@ use std::time::Duration; use moka::sync::Cache; use uuid::Uuid; -use crate::application::ports::recent_ports::RecentItemsUseCase; use crate::application::ports::resource_access_hook::ResourceAccessHook; use crate::application::services::recent_service::RecentService; @@ -85,7 +84,16 @@ impl ResourceAccessHook for RecentRecordingHook { let recent = Arc::clone(&self.recent); let (caller_id, file_id) = key; tokio::spawn(async move { - if let Err(e) = recent.record_item_access(caller_id, &file_id, "file").await { + // Fast path: skip the trait's `authz.require(Read, …)` + // (upstream `_with_perms` service already gated). The + // extra SQL round-trip pushes the upsert past the client's + // immediate `GET /api/recent/resources` in + // `tests/api/recent.hurl` step 7 — the whole reason for + // the internal variant. + if let Err(e) = recent + .record_item_access_internal(caller_id, &file_id, "file") + .await + { tracing::warn!( target: "oxicloud::recent", caller_id = %caller_id, diff --git a/src/interfaces/nextcloud/ocs_handler.rs b/src/interfaces/nextcloud/ocs_handler.rs index e673288b..73fc0394 100644 --- a/src/interfaces/nextcloud/ocs_handler.rs +++ b/src/interfaces/nextcloud/ocs_handler.rs @@ -422,13 +422,17 @@ pub async fn handle_search( let mut entries: Vec = Vec::new(); - // Map file results - // TODO(D1): drop the hardcoded "Personal/" prefix and read the - // caller's default-drive root folder name from `drives.root_folder_id` - // instead. Correct for D0-provisioned default drives; secondary - // drives keep their original root name. + // Map file results. + // + // `strip_drive_root_segment` handles both default and secondary + // drives — post-D0 the first path segment is the drive's root + // folder name (`"Personal"` for D0-provisioned defaults, the + // original sibling-root name for M2 backfilled secondaries). + // Read-scope is upstream in `state.applications.search_service`; + // this handler only formats display paths. for file in &results.files { - let display_path = file.path.strip_prefix("Personal/").unwrap_or(&file.path); + let display_path = + crate::interfaces::nextcloud::webdav_handler::strip_drive_root_segment(&file.path); let display_path = format!("/{}", display_path); let numeric_id = file_id_map.get(&file.id).copied(); @@ -452,12 +456,10 @@ pub async fn handle_search( })); } - // Map folder results — same TODO(D1) as above. + // Map folder results — same drive-agnostic strip as above. for folder in &results.folders { - let display_path = folder - .path - .strip_prefix("Personal/") - .unwrap_or(&folder.path); + let display_path = + crate::interfaces::nextcloud::webdav_handler::strip_drive_root_segment(&folder.path); let display_path = format!("/{}", display_path); entries.push(json!({ diff --git a/src/interfaces/nextcloud/report_handler.rs b/src/interfaces/nextcloud/report_handler.rs index ec60a8dc..39f913c0 100644 --- a/src/interfaces/nextcloud/report_handler.rs +++ b/src/interfaces/nextcloud/report_handler.rs @@ -62,6 +62,15 @@ async fn handle_filter_files( ) -> Result, AppError> { let user = &session.user; let url_user = &session.raw_username; + // Chroot-scope the response: NC's `oc:filter-files` REPORT is a + // single-drive surface (the client PROPFINDs favorites under its + // "home" URL and has no cross-drive concept). Favorites that live + // in another drive the caller is a member of are dropped from + // this response; they're still reachable via REST + // `/api/favorites/resources`. `session.require_chroot()` is safe + // here — the REPORT verb only reaches this handler through a + // path-scoped route. + let chroot = session.require_chroot()?; let fav_svc = match state.favorites_service.as_ref() { Some(svc) => svc, None => return Ok(empty_multistatus()), @@ -84,11 +93,11 @@ async fn handle_filter_files( // All items in this response are favorites. let favorite_ids: HashSet = favorites.iter().map(|f| f.item_id.clone()).collect(); - // TODO(D1): replace the hardcoded "Personal/" prefix with the - // caller's default-drive root folder name read from - // `drives.root_folder_id`. Correct for D0-provisioned default - // drives; secondary drives keep their original root name. - let home_prefix = "Personal/"; + // `home_prefix` is unused after the chroot-aware strip + // (see `strip_home_prefix`); kept as a positional argument in + // the emit calls below for signature stability with the + // report-handler tests and the parallel search-pass caller. + let home_prefix = ""; // Pass 1: resolve the favorited DTOs in two batch queries (was one // get_* per favorite — up to N serial round-trips on a sync client's @@ -155,7 +164,17 @@ async fn handle_filter_files( // multi-drive `~{drive}` form is echoed back to the client; // owner-id stays canonical via `&user.username`. for file in &files { - let subpath = strip_home_prefix(&file.path, home_prefix); + // Skip favorites that live outside the caller's chroot + // (other-drive favorites); reachable via REST if needed. + let Some(subpath) = strip_home_prefix(chroot, &file.path, home_prefix) else { + tracing::debug!( + target: "oxicloud::nc", + "REPORT filter-files: dropping cross-chroot favorite '{}' at '{}'", + file.id, + file.path, + ); + continue; + }; let href = nc_href(url_user, subpath); let fid = file_id_map.get(&file.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -172,7 +191,15 @@ async fn handle_filter_files( } for folder in &folders { - let subpath = strip_home_prefix(&folder.path, home_prefix); + let Some(subpath) = strip_home_prefix(chroot, &folder.path, home_prefix) else { + tracing::debug!( + target: "oxicloud::nc", + "REPORT filter-files: dropping cross-chroot favorite folder '{}' at '{}'", + folder.id, + folder.path, + ); + continue; + }; let href = format!("{}/", nc_href(url_user, subpath)); let fid = folder_id_map.get(&folder.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -207,9 +234,13 @@ async fn handle_search( session: &crate::interfaces::nextcloud::session::NcSession, ) -> Result, AppError> { let user = &session.user; - // Validate chroot up-front (path-scoped handler); `resolve_scope_folder` - // below re-pulls it from the session for the path-mapping step. - session.require_chroot()?; + // Chroot-scope the response: NC's search REPORT is a single-drive + // surface. Results that live outside the chroot (other drives the + // caller is a member of) are dropped from the multistatus and + // recorded at debug — reachable via REST search if needed. + // `resolve_scope_folder` below re-pulls chroot from the session + // for the path-mapping step. + let chroot = session.require_chroot()?; let url_user = &session.raw_username; let search_svc = match state.applications.search_service.as_ref() { Some(svc) => svc, @@ -241,10 +272,9 @@ async fn handle_search( let nc = state.nextcloud.as_ref(); let file_id_svc = nc.map(|n| &n.file_ids); - // TODO(D1): same as the favorites pass above — replace the - // hardcoded "Personal/" with the caller's actual default-drive - // root folder name from `drives.root_folder_id`. - let home_prefix = "Personal/"; + // See the favorites pass above: `home_prefix` is unused after the + // chroot-aware strip, kept only for signature stability. + let home_prefix = ""; // No favorite checking for search results -- pass an empty set. let favorite_ids: HashSet = HashSet::new(); @@ -266,7 +296,15 @@ async fn handle_search( // Files. for file in &files { - let subpath = strip_home_prefix(&file.path, home_prefix); + let Some(subpath) = strip_home_prefix(chroot, &file.path, home_prefix) else { + tracing::debug!( + target: "oxicloud::nc", + "REPORT search: dropping cross-chroot file '{}' at '{}'", + file.id, + file.path, + ); + continue; + }; let href = nc_href(url_user, subpath); let fid = file_id_map.get(&file.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -284,7 +322,15 @@ async fn handle_search( // Folders. for folder in &folders { - let subpath = strip_home_prefix(&folder.path, home_prefix); + let Some(subpath) = strip_home_prefix(chroot, &folder.path, home_prefix) else { + tracing::debug!( + target: "oxicloud::nc", + "REPORT search: dropping cross-chroot folder '{}' at '{}'", + folder.id, + folder.path, + ); + continue; + }; let href = format!("{}/", nc_href(url_user, subpath)); let fid = folder_id_map.get(&folder.id).copied(); let oc_id = fid.map(|id| format_oc_id(id, file_id_svc)); @@ -545,7 +591,19 @@ fn extract_subpath_from_scope(href: &str, url_user: &str) -> Option { None } -/// Strip the `My Folder - {username}/` prefix to get the DAV subpath. -fn strip_home_prefix<'a>(path: &'a str, prefix: &str) -> &'a str { - path.strip_prefix(prefix).unwrap_or(path) +/// Strip the caller's chroot prefix from an internal path so the +/// caller-facing DAV subpath is chroot-relative. Delegates to +/// `webdav_handler::strip_chroot_prefix` — chroot-aware, multi-segment +/// safe, and rejects items outside the chroot. Callers must decide +/// per-response whether an out-of-chroot item is dropped or falls +/// back to the naive strip. +/// +/// See `strip_chroot_prefix` for the full contract. The `_prefix` +/// legacy arg stays for signature stability with the emit helpers. +fn strip_home_prefix<'a>( + chroot: &crate::application::dtos::folder_dto::FolderDto, + path: &'a str, + _prefix: &str, +) -> Option<&'a str> { + crate::interfaces::nextcloud::webdav_handler::strip_chroot_prefix(chroot, path) } diff --git a/src/interfaces/nextcloud/trashbin_handler.rs b/src/interfaces/nextcloud/trashbin_handler.rs index 6a7077ed..6a09a1c3 100644 --- a/src/interfaces/nextcloud/trashbin_handler.rs +++ b/src/interfaces/nextcloud/trashbin_handler.rs @@ -81,6 +81,14 @@ async fn handle_propfind( session: &crate::interfaces::nextcloud::session::NcSession, ) -> Result, AppError> { let user = &session.user; + // Chroot-scope the trashbin view: `get_trash_items(user.id)` + // spans every drive the caller is a member of, but NC's + // trashbin surface is a single-drive concept from the client's + // POV. Items outside the chroot are dropped from the multistatus + // (see `write_trashbin_multistatus` → `strip_home_prefix` → + // `webdav_handler::strip_chroot_prefix`) and remain reachable + // via REST `/api/trash/resources`. + let chroot = session.require_chroot()?; let trash_svc = state .trash_service .as_ref() @@ -95,7 +103,7 @@ async fn handle_propfind( let file_id_svc = nc.map(|n| &n.file_ids); let mut buf = Vec::new(); - write_trashbin_multistatus(&mut buf, &items, &user.username, file_id_svc) + write_trashbin_multistatus(&mut buf, &items, &user.username, chroot, file_id_svc) .await .map_err(|e| AppError::internal_error(format!("XML generation failed: {}", e)))?; @@ -259,18 +267,23 @@ fn mime_from_name(name: &str) -> String { .to_string() } -/// Strip the home-folder prefix from an original path to produce the -/// Nextcloud-relative original location. +/// Strip the caller's chroot prefix from an original path to produce +/// the Nextcloud-relative original-location value. /// -/// TODO(D1): replace the hardcoded "Personal/" with the caller's actual -/// default-drive root folder name read from `drives.root_folder_id`. -/// Correct for D0-provisioned default drives; secondary drives keep -/// their original root name. The `_username` arg stays for now so the -/// upcoming dynamic lookup has a way to identify the caller. -fn strip_home_prefix<'a>(original_path: &'a str, _username: &str) -> &'a str { - original_path - .strip_prefix("Personal/") - .unwrap_or(original_path) +/// Delegates to `webdav_handler::strip_chroot_prefix` — chroot-aware, +/// multi-segment safe, and returns `None` when the item is outside +/// the chroot (e.g. a trashed item in another drive the caller is a +/// member of). The `_username` arg stays for signature stability +/// with call sites that thread it; the strip itself no longer uses it. +/// +/// See the doc on `strip_chroot_prefix` for the AuthZ caveat — this +/// is a display helper, not an ownership check. +fn strip_home_prefix<'a>( + original_path: &'a str, + _username: &str, + chroot: &crate::application::dtos::folder_dto::FolderDto, +) -> Option<&'a str> { + crate::interfaces::nextcloud::webdav_handler::strip_chroot_prefix(chroot, original_path) } // ────────────── Trashbin PROPFIND XML Generation ────────────── @@ -280,10 +293,16 @@ use crate::application::services::nextcloud_file_id_service::NextcloudFileIdServ use std::collections::HashMap; /// Generate a complete Nextcloud-compatible multistatus XML response for the trashbin. +/// +/// `chroot` scopes the response — items whose original path is outside +/// the chroot (other drives the caller is a member of) are dropped +/// silently. NC's trashbin surface is single-drive from the client's +/// perspective; cross-drive items remain reachable via REST. async fn write_trashbin_multistatus( writer: W, items: &[TrashedItemDto], username: &str, + chroot: &crate::application::dtos::folder_dto::FolderDto, file_id_svc: Option<&Arc>, ) -> Result<(), String> { let mut xml = Writer::new(writer); @@ -315,9 +334,24 @@ async fn write_trashbin_multistatus( batch_resolve_ids(file_id_svc, &file_uuids, &folder_uuids).await; id_map.extend(folder_id_map); - // Individual trashed items. + // Individual trashed items — skip those whose original path is + // outside the chroot (other-drive trash reachable via REST). for item in items { - write_trash_item_response(&mut xml, item, username, file_id_svc, &id_map)?; + if crate::interfaces::nextcloud::webdav_handler::strip_chroot_prefix( + chroot, + &item.original_path, + ) + .is_none() + { + tracing::debug!( + target: "oxicloud::nc", + "trashbin PROPFIND: dropping cross-chroot item '{}' at '{}'", + item.id, + item.original_path, + ); + continue; + } + write_trash_item_response(&mut xml, item, username, chroot, file_id_svc, &id_map)?; } xml.write_event(Event::End(BytesEnd::new("d:multistatus"))) @@ -363,10 +397,18 @@ fn write_trash_root_response( } /// Write a single trashed item as a `` element. +/// +/// Caller is expected to have already verified the item is inside +/// `chroot` — see the guard in `write_trashbin_multistatus`. This +/// function trusts the invariant and expects `strip_home_prefix` to +/// return `Some(_)`; if it ever returns `None` (chroot drift between +/// the guard and the emit, defensive-only), the original-location +/// falls back to an empty string. fn write_trash_item_response( xml: &mut Writer, item: &TrashedItemDto, username: &str, + chroot: &crate::application::dtos::folder_dto::FolderDto, file_id_svc: Option<&Arc>, id_map: &HashMap, ) -> Result<(), String> { @@ -427,7 +469,7 @@ fn write_trash_item_response( write_text_element(xml, "nc:trashbin-filename", &item.name)?; // nc:trashbin-original-location - let original_location = strip_home_prefix(&item.original_path, username); + let original_location = strip_home_prefix(&item.original_path, username, chroot).unwrap_or(""); write_text_element(xml, "nc:trashbin-original-location", original_location)?; // nc:trashbin-deletion-time diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index a898f3ba..74eba460 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -80,6 +80,78 @@ pub fn nc_to_internal_path(chroot: &FolderDto, subpath: &str) -> Result(chroot: &FolderDto, internal_path: &'a str) -> Option<&'a str> { + // Normalize both sides: `FolderDto.path` comes from + // `StoragePath::to_string()` which prepends a leading `/` + // (e.g. `"/Personal"`), but DB-side paths coming from + // `storage.folders.path` (composed by the `compute_folder_path` + // trigger) never have a leading slash. Trim both so `"/Personal"` + // vs `"Personal/g9-tree"` matches the intended prefix. + let root = chroot.path.trim_matches('/'); + if root.is_empty() { + // Guard against a mis-set chroot with an empty root path — + // stripping "" from anything would return the whole path. + return None; + } + let path = internal_path.trim_start_matches('/'); + let rest = path.strip_prefix(root)?; + // Reject a partial prefix match — a chroot of "Personal" must + // not match an item at "PersonalSecrets/…". + match rest.strip_prefix('/') { + Some(subpath) => Some(subpath), + // Item path equals the chroot exactly — the chroot itself + // (i.e. a folder) is not a legitimate response item, so + // treat as an empty subpath. + None if rest.is_empty() => Some(""), + None => None, + } +} + +/// Naive fallback: strip the first path segment from an internal +/// `storage.folders.path`. Post-D0 every path starts with its drive's +/// root folder name (single segment), so for the current schema this +/// gives the drive-relative subpath. +/// +/// Use this ONLY when the caller doesn't have a chroot in scope +/// (e.g. OCS unified search, whose results legitimately span every +/// drive the caller has Read on — no single chroot covers them all). +/// Every path-scoped NC handler that DOES have `session` in scope +/// should prefer [`strip_chroot_prefix`] — it validates the item +/// belongs under the chroot instead of trusting the schema +/// invariant, and it survives a future composed chroot like +/// `"Personal/folderA/subfolder"`. +/// +/// **Not an AuthZ boundary.** Same caveat as `strip_chroot_prefix` +/// — AuthZ is enforced upstream via `_with_perms` methods; this +/// helper only formats display strings. +/// +/// Returns `""` when the path is a single segment (i.e. the drive +/// root itself, which is never a legitimate item target). +pub fn strip_drive_root_segment(internal_path: &str) -> &str { + match internal_path.split_once('/') { + Some((_root, rest)) => rest, + None => "", + } +} + /// Build the Nextcloud DAV href for a **collection** (folder). Always /// terminates with `/` — RFC 4918 §5.2 requires collection URLs to end /// in a slash, and the Nextcloud desktop client strictly enforces this @@ -1873,6 +1945,92 @@ mod tests { ); } + // ── strip_chroot_prefix ── + // + // Regression guard for the "chroot.path has a leading slash from + // StoragePath::to_string() but DB-side original_path doesn't" trap + // that broke the NC trashbin PROPFIND after Round 2 rolled out. + // Also pins the composed-chroot behaviour Ed asked about. + + #[test] + fn strip_chroot_prefix_default_drive_root() { + // FolderDto.path carries a leading slash (StoragePath Display); + // DB paths do not. Both must normalise to the same prefix. + let chroot = stub_folder("/Personal"); + assert_eq!( + strip_chroot_prefix(&chroot, "Personal/g9-tree"), + Some("g9-tree") + ); + } + + #[test] + fn strip_chroot_prefix_deep_path() { + let chroot = stub_folder("/Personal"); + assert_eq!( + strip_chroot_prefix(&chroot, "Personal/inner/deep.txt"), + Some("inner/deep.txt") + ); + } + + #[test] + fn strip_chroot_prefix_out_of_chroot_returns_none() { + // Items on a different drive (whose root isn't "Personal") + // must NOT be surfaced under the caller's chroot. + let chroot = stub_folder("/Personal"); + assert_eq!(strip_chroot_prefix(&chroot, "team-drive/report.pdf"), None); + } + + #[test] + fn strip_chroot_prefix_rejects_partial_prefix_match() { + // "Personal" is a prefix substring of "PersonalSecrets" but + // NOT a path-segment prefix — must reject. + let chroot = stub_folder("/Personal"); + assert_eq!( + strip_chroot_prefix(&chroot, "PersonalSecrets/foo.txt"), + None + ); + } + + #[test] + fn strip_chroot_prefix_composed_chroot() { + // The future composed-chroot case Ed raised: chroot points at + // a subfolder inside a drive. The strip must remove the ENTIRE + // composed prefix, not just the first segment. + let chroot = stub_folder("/Personal/folderA/subfolder"); + assert_eq!( + strip_chroot_prefix(&chroot, "Personal/folderA/subfolder/foo.txt"), + Some("foo.txt") + ); + } + + #[test] + fn strip_chroot_prefix_composed_chroot_sibling_leaks_blocked() { + // Same composed chroot, but the item lives in a sibling + // subfolder — must be rejected, not naively strip 1 segment. + let chroot = stub_folder("/Personal/folderA/subfolder"); + assert_eq!( + strip_chroot_prefix(&chroot, "Personal/folderA/other/foo.txt"), + None + ); + } + + #[test] + fn strip_chroot_prefix_chroot_root_itself() { + // Item path equals chroot exactly — legitimate for a PROPFIND + // Depth:0 on the chroot itself. Subpath is empty. + let chroot = stub_folder("/Personal"); + assert_eq!(strip_chroot_prefix(&chroot, "Personal"), Some("")); + } + + #[test] + fn strip_chroot_prefix_empty_chroot_returns_none() { + // Defensive: a mis-set chroot with an empty path must not + // strip anything (stripping "" from any path would return + // the whole path — a silent leak). + let chroot = stub_folder("/"); + assert_eq!(strip_chroot_prefix(&chroot, "Personal/foo.txt"), None); + } + // ── nc_href ── #[test] diff --git a/tests/api/drives_membership.hurl b/tests/api/drives_membership.hurl index e34bc1e5..cc450d3a 100644 --- a/tests/api/drives_membership.hurl +++ b/tests/api/drives_membership.hurl @@ -511,6 +511,26 @@ HTTP 200 jsonpath "$[*].id" contains {{team_drive_id}} +# ───────────────────────────────────────────────────────────── +# Step 21b — Upload gate by role (post-Drive AuthZ audit Round 2). +# Bob is Editor on team_drive; `POST /api/files/upload` +# targeting team_root_folder_id should succeed. This is +# the REST-side counterpart of the WebDAV/NC PUT chain +# hardened by `update_file_streaming_with_perms`. If +# this fails, the whole role-bundle → Permission::Create +# wiring is broken. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/files/upload +Authorization: Bearer {{bob_token}} +[MultipartFormData] +folder_id: {{team_root_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 201 +[Captures] +bob_editor_upload_id: jsonpath "$.id" + + # ───────────────────────────────────────────────────────────── # Step 22 — Higher role wins: Bob now ALSO gets a Viewer direct # grant (would lower his bundle). The collapsed caller_role @@ -537,6 +557,50 @@ HTTP 200 jsonpath "$[*].id" contains {{team_drive_id}} +# ───────────────────────────────────────────────────────────── +# Step 22b — Viewer CANNOT upload into a shared drive. +# Post-Drive AuthZ audit Round 2: the create branch of +# `update_file_streaming_with_perms` requires +# `Permission::Create` on the parent folder — bundled +# with `owner`/`editor`/`contributor` role_grants only, +# NOT with `viewer`. `POST /api/files/upload` shares the +# same `save_file_with_blob` gate, so a Viewer probe +# must land 404 (anti-enum: same shape as no-such-folder) +# + `authz.denied` audit line. Also verify the batch / +# overwrite paths refuse — the whole chain from +# drive-membership to file write is exercised here. +# ───────────────────────────────────────────────────────────── + +# 22b.i — Fresh file: 404. +POST {{base_url}}/api/files/upload +Authorization: Bearer {{bob_token}} +[MultipartFormData] +folder_id: {{team_root_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 404 + + +# 22b.ii — Overwrite attempt on the Editor-era upload: still 404. +# `save_file_with_blob` catches the duplicate name at the +# `Create`-permission check before the upsert races (which +# would otherwise 409). The audit shape stays 404. +POST {{base_url}}/api/files/upload +Authorization: Bearer {{bob_token}} +[MultipartFormData] +folder_id: {{team_root_folder_id}} +file: file,fixtures/hello.txt; text/plain + +HTTP 404 + + +# 22b.iii — Alice's Editor-era file is untouched. +GET {{base_url}}/api/files/{{bob_editor_upload_id}} +Authorization: Bearer {{alice_token}} + +HTTP 200 + + # ============================================================= # Per-role mutation matrix — what every role can / can't do # ============================================================= @@ -845,15 +909,22 @@ HTTP 409 # 30c — Clear the lingering content (the Editor-created folder from -# Step 27). Delete via the regular folder endpoint so the row -# lands in trash, not the live tree; `is_empty` excludes -# trashed rows so a populated trash bin is allowed. +# Step 27 and the Editor-era file from Step 21b). Delete via +# the regular endpoints so rows land in trash, not the live +# tree; `is_empty` excludes trashed rows so a populated trash +# bin is allowed. DELETE {{base_url}}/api/folders/{{editor_created_folder_id}} Authorization: Bearer {{alice_token}} HTTP 204 +DELETE {{base_url}}/api/files/{{bob_editor_upload_id}} +Authorization: Bearer {{alice_token}} + +HTTP 204 + + # 30d — Owner on an empty drive → 204. DELETE {{base_url}}/api/drives/{{team_drive_id}} Authorization: Bearer {{alice_token}} diff --git a/tests/webdav/test_nc_move_copy_delete_trash.sh b/tests/webdav/test_nc_move_copy_delete_trash.sh index 4a26930c..3425754c 100755 --- a/tests/webdav/test_nc_move_copy_delete_trash.sh +++ b/tests/webdav/test_nc_move_copy_delete_trash.sh @@ -323,7 +323,20 @@ grep -q 'g8-doomed' <<< "$BODY" \ || fail "K1: g8-doomed.txt not in trashbin PROPFIND" grep -q '' <<< "$BODY" \ || fail "K1: trashbin response missing " -pass "K1: trashbin shows g8-doomed.txt with original-location" + +# Post-D3 (secondary/shared drive support): the `original-location` +# value is drive-relative — the emitter strips the drive-root segment +# from the internal `storage.folders.path` (`"Personal/g8-doomed.txt"` +# for a file at the default drive root) so NC clients see +# `"g8-doomed.txt"` regardless of what the drive's root is named. +# Regression guard: the pre-D3 code hardcoded `strip_prefix("Personal/")` +# — a bug that would silently break secondary drives. Assert the +# stripped shape (no leading `Personal/`, no leading `/`, no drive +# segment). +grep -q 'g8-doomed\.txt' <<< "$BODY" \ + || fail "K1: original-location not drive-relative (expected 'g8-doomed.txt', got: $(grep -o '[^<]*' <<< "$BODY"))" + +pass "K1: trashbin shows g8-doomed.txt with drive-relative original-location" # Extract the trashed item id (last segment of the href). # Trashbin hrefs are `/remote.php/dav/trashbin/{user}/trash/{uuid}`