diff --git a/src/application/ports/authorization_ports.rs b/src/application/ports/authorization_ports.rs index fb994362..1934c90a 100644 --- a/src/application/ports/authorization_ports.rs +++ b/src/application/ports/authorization_ports.rs @@ -16,6 +16,28 @@ use crate::domain::services::authorization::{ ResourceKind, Role, Subject, }; +/// Discriminates the two denial shapes surfaced by +/// [`AuthorizationEngine::require_visible`] in the `authz.denied` audit line. +/// Log-aggregation consumers key off the string form via `as_str`; keep the +/// values stable — a new denial shape means a new variant, never a renamed +/// existing one. +#[derive(Debug, Copy, Clone, PartialEq, Eq)] +pub enum AuthzDenialVisibility { + /// Caller has `Read` on the resource — 403 Forbidden. + Visible, + /// Caller has no `Read` — 404 anti-enum. + Hidden, +} + +impl AuthzDenialVisibility { + pub fn as_str(self) -> &'static str { + match self { + Self::Visible => "visible", + Self::Hidden => "hidden", + } + } +} + pub trait AuthorizationEngine: Send + Sync + 'static { /// Returns true if `subject` has `permission` on `resource`, considering /// owner short-circuit AND cascading from folder ancestors. @@ -53,9 +75,28 @@ pub trait AuthorizationEngine: Send + Sync + 'static { Ok(allowed) } - /// Convenience wrapper around `check`: returns `Ok(())` when allowed and - /// `DomainError::not_found` when denied (anti-enumeration — same error as - /// "resource doesn't exist" so attackers can't probe IDs by error shape). + /// Graduated-denial wrapper around `check`. Semantics: + /// + /// - `permission` granted → `Ok(())` + /// - `permission` denied, `Read` also denied → `DomainError::not_found` + /// (404, anti-enumeration — same shape as "doesn't exist" so a probing + /// caller can't distinguish "wrong id" from "no access") + /// - `permission` denied, `Read` granted → `DomainError::access_denied` + /// (403 — the caller can already see the resource, so hiding existence + /// leaks nothing new; a clear 403 beats a confusing 404 for UX and for + /// API-first clients like rclone) + /// + /// Special case: when `permission == Read`, the visibility gate collapses + /// onto itself — a `Read` denial IS a "hidden" outcome by definition, so + /// the method short-circuits to the strict anti-enum 404 without a second + /// DB round-trip. That's why there's only one method: strict Read-denial + /// and graduated write-denial fall out of the same signature. + /// + /// Do NOT use this in search / enumeration paths where existence itself is + /// the attack vector — those must filter at the SQL/index layer, never + /// touch this method with per-row ids. Cross-tenant probes on ids the + /// caller has no prior read handle for degrade to the 404 shape naturally + /// (Read denied → `Hidden`). async fn require( &self, subject: Subject, @@ -80,39 +121,68 @@ pub trait AuthorizationEngine: Send + Sync + 'static { permission, resource ); - Ok(()) + return Ok(()); + } + + // Visibility probe. Short-circuit: when the target permission IS + // `Read` and the check above returned false, we already know Read is + // denied — visibility is `Hidden` by definition, no second DB hop. + // Otherwise probe Read; a DB-hop failure here degrades to `Hidden` so + // the caller sees the strict anti-enum shape (safe default). + let visibility = if permission == Permission::Read { + AuthzDenialVisibility::Hidden + } else if self + .check(subject, Permission::Read, resource) + .await + .unwrap_or(false) + { + AuthzDenialVisibility::Visible } else { - let (kind, id) = match resource { - Resource::Folder(id) => ("Folder", id), - Resource::File(id) => ("File", id), - Resource::Drive(id) => ("Drive", id), - Resource::Calendar(id) => ("Calendar", id), - Resource::AddressBook(id) => ("AddressBook", id), - Resource::Playlist(id) => ("Playlist", id), - }; - // Audit-worthy: denials are the interesting signal. Routed - // through the `audit` tracing target so log aggregators can - // surface them separately from operational debug traffic. - // Span context (request_id, client_ip, user_id) is attached - // automatically by the request-scope span set in - // `interfaces/middleware/trace_span.rs`, so this log line - // doesn't need to duplicate those fields — they appear in - // the structured output of every log written inside the - // request span. - tracing::info!( - target: "audit", - event = "authz.denied", - subject_type = subject.type_str(), - subject_id = %subject.id(), - permission = permission.as_str(), - resource_type = resource.type_str(), - resource_id = %resource.id(), - "👮🏻‍♂️ perms: ⛔ Subject '{}' hasn't permission to '{}' on resource '{}'", - subject, - permission, - resource - ); - Err(DomainError::not_found(kind, id.to_string())) + AuthzDenialVisibility::Hidden + }; + + let (kind, id) = match resource { + Resource::Folder(id) => ("Folder", id), + Resource::File(id) => ("File", id), + Resource::Drive(id) => ("Drive", id), + Resource::Calendar(id) => ("Calendar", id), + Resource::AddressBook(id) => ("AddressBook", id), + Resource::Playlist(id) => ("Playlist", id), + }; + + // Audit-worthy: denials are the interesting signal. Routed through + // the `audit` tracing target so log aggregators can surface them + // separately from operational debug traffic. Span context + // (request_id, client_ip, user_id) comes from the request-scope + // span set in `interfaces/middleware/trace_span.rs`, so this line + // doesn't need to duplicate those fields. + // + // The `visibility` field discriminates the two denial shapes for + // operators grepping exists-but-denied vs fully-hidden. `visible` + // denials are the ones surfaced to the caller as 403 (and safe to + // detail in the UI); `hidden` denials are the 404 anti-enum path. + tracing::info!( + target: "audit", + event = "authz.denied", + visibility = visibility.as_str(), + subject_type = subject.type_str(), + subject_id = %subject.id(), + permission = permission.as_str(), + resource_type = resource.type_str(), + resource_id = %resource.id(), + "👮🏻‍♂️ perms: ⛔ Subject '{}' hasn't permission to '{}' on resource '{}' (visibility={})", + subject, + permission, + resource, + visibility.as_str() + ); + + match visibility { + AuthzDenialVisibility::Visible => Err(DomainError::access_denied( + kind, + format!("Missing '{}' permission on {} {}", permission, kind, id), + )), + AuthzDenialVisibility::Hidden => Err(DomainError::not_found(kind, id.to_string())), } } diff --git a/src/application/ports/inbound.rs b/src/application/ports/inbound.rs index 96d71970..456ce9e8 100644 --- a/src/application/ports/inbound.rs +++ b/src/application/ports/inbound.rs @@ -27,11 +27,15 @@ pub trait SearchUseCase: Send + Sync + 'static { ) -> Result, DomainError>; /// Returns quick suggestions for autocomplete (lightweight, fast). + /// `caller_id` scopes results to drives the caller can Read — without + /// it the endpoint leaks names + paths across every tenant on the + /// instance (AuthZ audit finding #1, 2026-07-12). async fn suggest( &self, query: &str, folder_id: Option<&str>, limit: usize, + caller_id: Uuid, ) -> Result; /// Clears the search results cache. diff --git a/src/application/ports/storage_ports.rs b/src/application/ports/storage_ports.rs index 896fc01c..9f75e790 100644 --- a/src/application/ports/storage_ports.rs +++ b/src/application/ports/storage_ports.rs @@ -205,13 +205,21 @@ pub trait FileReadPort: Send + Sync + 'static { /// Results are ordered by relevance (exact > starts-with > contains) so the /// caller can use them directly for autocomplete suggestions. /// - /// The default implementation falls back to `list_files` + in-memory filter - /// so that stubs and mocks compile without changes. + /// `caller_id` scopes results to files whose owning drive the caller can + /// Read (direct or group-mediated `role_grants`). Without it the endpoint + /// leaks names + paths across every tenant on the instance — closed as + /// AuthZ audit finding #1 (2026-07-12). + /// + /// The default implementation falls back to `list_files` + in-memory + /// filter so that stubs and mocks compile without changes. Stub-mode + /// callers already operate against a single tenant's data, so ignoring + /// `caller_id` here is safe; the PG impl enforces the real scope. async fn suggest_files_by_name( &self, folder_id: Option<&str>, query: &str, limit: usize, + _caller_id: Uuid, ) -> Result, DomainError> { let all = self.list_files(folder_id).await?; let q = query.to_lowercase(); diff --git a/src/application/services/search_service.rs b/src/application/services/search_service.rs index 9aa42050..a7dcf947 100644 --- a/src/application/services/search_service.rs +++ b/src/application/services/search_service.rs @@ -497,20 +497,28 @@ impl SearchService { /// Quick suggestions search — returns up to `limit` name suggestions /// matching the query. Pushes filtering, relevance sort and LIMIT to SQL /// so only a handful of rows cross the DB→app boundary. - pub async fn suggest( + /// + /// `caller_id` scopes the underlying repo queries to drives the caller + /// can Read. Without it (the pre-fix shape) any authenticated user — + /// including external magic-link recipients — could autocomplete both + /// names and full paths across every tenant on the instance (AuthZ + /// audit finding #1, 2026-07-12). Named `_with_perms` per the + /// AGENTS.md AuthZ convention. + pub async fn suggest_with_perms( &self, query: &str, folder_id: Option<&str>, limit: usize, + caller_id: Uuid, ) -> Result { let start = Instant::now(); // Ask SQL for at most `limit` best-matching files and folders let (files, folders) = tokio::join!( self.file_repository - .suggest_files_by_name(folder_id, query, limit), + .suggest_files_by_name(folder_id, query, limit, caller_id), self.folder_repository - .suggest_folders_by_name(folder_id, query, limit), + .suggest_folders_by_name(folder_id, query, limit, caller_id), ); let files = files?; let folders = folders?; @@ -802,14 +810,20 @@ impl SearchUseCase for SearchService { }) } - /// Returns quick suggestions for autocomplete. + /// Returns quick suggestions for autocomplete. Delegates to the + /// inherent `suggest_with_perms` — the trait method is preserved as + /// the polymorphic entry point (e.g. for `StubSearchUseCase` in + /// tests); production callers can equivalently call the inherent + /// method directly. async fn suggest( &self, query: &str, folder_id: Option<&str>, limit: usize, + caller_id: Uuid, ) -> Result { - self.suggest(query, folder_id, limit).await + self.suggest_with_perms(query, folder_id, limit, caller_id) + .await } /// Clears the search results cache. @@ -841,6 +855,7 @@ impl SearchService { _query: &str, _folder_id: Option<&str>, _limit: usize, + _caller_id: Uuid, ) -> Result { Ok(SearchSuggestionsDto { suggestions: Vec::new(), diff --git a/src/common/stubs.rs b/src/common/stubs.rs index bdd15bf5..4d72d9d4 100644 --- a/src/common/stubs.rs +++ b/src/common/stubs.rs @@ -725,6 +725,7 @@ impl SearchUseCase for StubSearchUseCase { _query: &str, _folder_id: Option<&str>, _limit: usize, + _caller_id: Uuid, ) -> Result { Ok(SearchSuggestionsDto { suggestions: Vec::new(), diff --git a/src/domain/repositories/folder_repository.rs b/src/domain/repositories/folder_repository.rs index 19cafab4..6d6d9568 100644 --- a/src/domain/repositories/folder_repository.rs +++ b/src/domain/repositories/folder_repository.rs @@ -269,13 +269,21 @@ pub trait FolderRepository: Send + Sync + 'static { /// Results are ordered by relevance (exact > starts-with > contains) for /// autocomplete suggestions. /// + /// `caller_id` scopes results to folders whose owning drive the caller + /// can Read (direct or group-mediated `role_grants`). Without it the + /// endpoint leaked names + paths across every tenant on the instance — + /// closed as AuthZ audit finding #1 (2026-07-12). + /// /// The default implementation falls back to `list_folders` + in-memory - /// filter so that stubs and mocks compile without changes. + /// filter so that stubs and mocks compile without changes. Stub-mode + /// callers already operate against a single tenant's data, so ignoring + /// `caller_id` here is safe; the PG impl enforces the real scope. async fn suggest_folders_by_name( &self, parent_id: Option<&str>, query: &str, limit: usize, + _caller_id: uuid::Uuid, ) -> Result, DomainError> { let all = self.list_folders(parent_id).await?; let q = query.to_lowercase(); diff --git a/src/infrastructure/repositories/pg/file_blob_read_repository.rs b/src/infrastructure/repositories/pg/file_blob_read_repository.rs index f70c5adb..d34b44e8 100644 --- a/src/infrastructure/repositories/pg/file_blob_read_repository.rs +++ b/src/infrastructure/repositories/pg/file_blob_read_repository.rs @@ -1413,12 +1413,20 @@ impl FileReadPort for FileBlobReadRepository { folder_id: Option<&str>, query: &str, limit: usize, + caller_id: Uuid, ) -> Result, DomainError> { + // Scope by drive membership: `CALLER_CAN_READ_DRIVE` (`$1` = + // caller_id) restricts the result set to files whose owning drive + // the caller has any active `role_grants` on — direct or via a + // transitive group cascade. Pre-fix, the query only filtered on + // `NOT is_trashed AND name ILIKE $pattern`, exposing names + paths + // across every tenant on the instance (AuthZ audit finding #1, + // 2026-07-12). let pattern = super::like_escape(query); let limit_i64 = limit as i64; let rows: Vec = if let Some(fid) = folder_id { - sqlx::query_as( + sqlx::query_as(&format!( r#" SELECT fi.id, fi.name, fi.folder_id, fo.path, fi.size, fi.mime_type, @@ -1429,7 +1437,40 @@ impl FileReadPort for FileBlobReadRepository { 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 + WHERE {CALLER_CAN_READ_DRIVE} + AND fi.folder_id = $2::uuid + AND NOT fi.is_trashed + AND fi.name ILIKE $3 + ORDER BY CASE + WHEN fi.name ILIKE $4 THEN 0 + WHEN fi.name ILIKE $4 || '%' THEN 1 + ELSE 2 + END, + fi.name + LIMIT $5 + "# + )) + .bind(caller_id) + .bind(fid) + .bind(&pattern) + .bind(query) + .bind(limit_i64) + .fetch_all(self.pool.as_ref()) + .await + } else { + sqlx::query_as(&format!( + r#" + SELECT fi.id, fi.name, fi.folder_id, 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.created_by, fi.updated_by + FROM storage.files fi + LEFT JOIN storage.folders fo ON fo.id = fi.folder_id + WHERE {CALLER_CAN_READ_DRIVE} + AND fi.folder_id IS NULL AND NOT fi.is_trashed AND fi.name ILIKE $2 ORDER BY CASE @@ -1439,38 +1480,9 @@ impl FileReadPort for FileBlobReadRepository { END, fi.name LIMIT $4 - "#, - ) - .bind(fid) - .bind(&pattern) - .bind(query) - .bind(limit_i64) - .fetch_all(self.pool.as_ref()) - .await - } else { - sqlx::query_as( - r#" - SELECT fi.id, fi.name, fi.folder_id, 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.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.name ILIKE $1 - ORDER BY CASE - WHEN fi.name ILIKE $2 THEN 0 - WHEN fi.name ILIKE $2 || '%' THEN 1 - ELSE 2 - END, - fi.name - LIMIT $3 - "#, - ) + "# + )) + .bind(caller_id) .bind(&pattern) .bind(query) .bind(limit_i64) diff --git a/src/infrastructure/repositories/pg/folder_db_repository.rs b/src/infrastructure/repositories/pg/folder_db_repository.rs index 3421b9dc..1d748fbb 100644 --- a/src/infrastructure/repositories/pg/folder_db_repository.rs +++ b/src/infrastructure/repositories/pg/folder_db_repository.rs @@ -1232,31 +1232,39 @@ impl FolderRepository for FolderDbRepository { parent_id: Option<&str>, query: &str, limit: usize, + caller_id: uuid::Uuid, ) -> Result, DomainError> { + // Same drive-scope filter as `suggest_files_by_name` — closed as + // AuthZ audit finding #1 (2026-07-12). `CALLER_CAN_READ_DRIVE` + // aliases `storage.folders` as `fo`; the pre-fix query aliased it + // as an unqualified `storage.folders`, so this rewrite adds the + // `fo` alias in every branch. let pattern = super::like_escape(query); let limit_i64 = limit as i64; let rows: Vec = if let Some(pid) = parent_id { - sqlx::query_as( + sqlx::query_as(&format!( r#" - SELECT id::text, name, path, parent_id::text, drive_id, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - EXTRACT(EPOCH FROM tree_modified_at)::bigint, - created_by, updated_by - FROM storage.folders - WHERE parent_id = $1::uuid - AND NOT is_trashed - AND name ILIKE $2 + SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, 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 {CALLER_CAN_READ_DRIVE} + AND fo.parent_id = $2::uuid + AND NOT fo.is_trashed + AND fo.name ILIKE $3 ORDER BY CASE - WHEN name ILIKE $3 THEN 0 - WHEN name ILIKE $3 || '%' THEN 1 + WHEN fo.name ILIKE $4 THEN 0 + WHEN fo.name ILIKE $4 || '%' THEN 1 ELSE 2 END, - name - LIMIT $4 - "#, - ) + fo.name + LIMIT $5 + "# + )) + .bind(caller_id) .bind(pid) .bind(&pattern) .bind(query) @@ -1264,26 +1272,28 @@ impl FolderRepository for FolderDbRepository { .fetch_all(self.pool()) .await } else { - sqlx::query_as( + sqlx::query_as(&format!( r#" - SELECT id::text, name, path, parent_id::text, drive_id, - EXTRACT(EPOCH FROM created_at)::bigint, - EXTRACT(EPOCH FROM updated_at)::bigint, - EXTRACT(EPOCH FROM tree_modified_at)::bigint, - created_by, updated_by - FROM storage.folders - WHERE parent_id IS NULL - AND NOT is_trashed - AND name ILIKE $1 + SELECT fo.id::text, fo.name, fo.path, fo.parent_id::text, 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 {CALLER_CAN_READ_DRIVE} + AND fo.parent_id IS NULL + AND NOT fo.is_trashed + AND fo.name ILIKE $2 ORDER BY CASE - WHEN name ILIKE $2 THEN 0 - WHEN name ILIKE $2 || '%' THEN 1 + WHEN fo.name ILIKE $3 THEN 0 + WHEN fo.name ILIKE $3 || '%' THEN 1 ELSE 2 END, - name - LIMIT $3 - "#, - ) + fo.name + LIMIT $4 + "# + )) + .bind(caller_id) .bind(&pattern) .bind(query) .bind(limit_i64) diff --git a/src/interfaces/api/handlers/search_handler.rs b/src/interfaces/api/handlers/search_handler.rs index 5fba8009..2893103f 100644 --- a/src/interfaces/api/handlers/search_handler.rs +++ b/src/interfaces/api/handlers/search_handler.rs @@ -140,6 +140,7 @@ impl SearchHandler { /// Autocomplete suggestions for search. pub(super) async fn suggest_files_impl( State(state): State>, + auth_user: AuthUser, Query(params): Query, ) -> impl IntoResponse { info!("API: Search suggestions for {:?}", params.query); @@ -159,7 +160,12 @@ impl SearchHandler { let limit = params.limit.unwrap_or(10).min(20); match search_service - .suggest(¶ms.query, params.folder_id.as_deref(), limit) + .suggest_with_perms( + ¶ms.query, + params.folder_id.as_deref(), + limit, + auth_user.id, + ) .await { Ok(suggestions) => { @@ -354,9 +360,10 @@ pub async fn search_files_post( )] pub async fn suggest_files( state: State>, + auth_user: AuthUser, query: Query, ) -> impl IntoResponse { - SearchHandler::suggest_files_impl(state, query).await + SearchHandler::suggest_files_impl(state, auth_user, query).await } #[utoipa::path( diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index b57c26e5..561a080c 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -2225,18 +2225,27 @@ async fn handle_delete( // optimized resolver and the read repositories disagree on path // shape for some files; see `resolve_or_legacy` docs. let _ = file_retrieval_service; // present for legacy fallback if needed elsewhere + // AuthZ audit #2 (2026-07-12): route service errors through + // `AppError::from` so authz denials from `_with_perms` surface as + // 404 (the anti-enum shape). The prior `map_err(|e| internal_error…)` + // collapsed every error — including the `NotFound` that + // `authz.require` returns on denial — into HTTP 500, giving a + // reliable "exists-but-denied" vs "missing" oracle to a probing + // caller. Also preserves `QuotaExceeded → 507`, + // `AlreadyExists → 409`, `InvalidInput → 400` shapes surfacing + // through the standard error mapping. match resolve_or_legacy(&state, &path, drive_id).await { Some(ResolvedResource::Folder(folder)) => { folder_service .delete_folder_with_perms(&folder.id, user.id) .await - .map_err(|e| AppError::internal_error(format!("Failed to delete folder: {}", e)))?; + .map_err(AppError::from)?; } Some(ResolvedResource::File(file)) => { file_management_service .delete_file_with_perms(&file.id, user.id) .await - .map_err(|e| AppError::internal_error(format!("Failed to delete file: {}", e)))?; + .map_err(AppError::from)?; } None => return Err(AppError::not_found(format!("Resource not found: {}", path))), } @@ -2385,28 +2394,23 @@ async fn handle_move( // RFC 4918 §9.9.3: when Overwrite: T, perform a DELETE on the // destination before moving. Without this the rename/move fails // on a unique-index conflict (same name in same parent). + // AuthZ audit #2 (2026-07-12): `_with_perms` returns `DomainError`; + // route through `AppError::from` so authz denials surface as 404 (the + // anti-enum shape) instead of a `map_err → internal_error` 500 that + // gives a probing caller an "exists-but-denied" oracle. Also preserves + // `QuotaExceeded → 507`, `AlreadyExists → 409`, `InvalidInput → 400`. match resolve_or_legacy(&state, &destination_path, dst_drive_id).await { Some(ResolvedResource::Folder(f)) => { folder_service .delete_folder_with_perms(&f.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to delete existing destination: {}", - e - )) - })?; + .map_err(AppError::from)?; } Some(ResolvedResource::File(f)) => { file_management_service .delete_file_with_perms(&f.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to delete existing destination: {}", - e - )) - })?; + .map_err(AppError::from)?; } None => {} } @@ -2684,28 +2688,23 @@ async fn handle_copy( // RFC 4918 §9.8.4: when Overwrite: T, the server MUST perform a // DELETE on the destination before the copy. Without this the copy // service returns a unique-index conflict (500). + // AuthZ audit #2 (2026-07-12): `_with_perms` returns `DomainError`; + // route through `AppError::from` so authz denials surface as 404 (the + // anti-enum shape) instead of a `map_err → internal_error` 500 that + // gives a probing caller an "exists-but-denied" oracle. Also preserves + // `QuotaExceeded → 507`, `AlreadyExists → 409`, `InvalidInput → 400`. match resolve_or_legacy(&state, &destination_path, dst_drive_id).await { Some(ResolvedResource::Folder(f)) => { folder_service .delete_folder_with_perms(&f.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to delete existing destination: {}", - e - )) - })?; + .map_err(AppError::from)?; } Some(ResolvedResource::File(f)) => { file_management_service .delete_file_with_perms(&f.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to delete existing destination: {}", - e - )) - })?; + .map_err(AppError::from)?; } None => {} } @@ -2759,6 +2758,12 @@ async fn handle_copy( } }; + // AuthZ audit #2 (2026-07-12): route service errors through + // `AppError::from` so authz denials from `_with_perms` surface as 404 + // (the anti-enum shape) instead of a `map_err → internal_error` 500 + // that gives a probing caller an "exists-but-denied" oracle. Also + // preserves `QuotaExceeded → 507`, `AlreadyExists → 409`, + // `InvalidInput → 400` shapes. match resolved { ResolvedResource::Folder(folder) => { let recursive = depth != "0"; @@ -2771,9 +2776,7 @@ async fn handle_copy( Some(dest_name.to_string()), ) .await - .map_err(|e| { - AppError::internal_error(format!("Failed to copy folder tree: {}", e)) - })?; + .map_err(AppError::from)?; } else { let create_dto = crate::application::dtos::folder_dto::CreateFolderDto { name: dest_name.to_string(), @@ -2782,12 +2785,7 @@ async fn handle_copy( folder_service .create_folder_with_perms(create_dto, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to create destination folder: {}", - e - )) - })?; + .map_err(AppError::from)?; } } ResolvedResource::File(file) => { @@ -2795,7 +2793,7 @@ async fn handle_copy( file_management_service .copy_file_with_perms(&file.id, user.id, target_parent_id, copy_name) .await - .map_err(|e| AppError::internal_error(format!("Failed to copy file: {}", e)))?; + .map_err(AppError::from)?; } } diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index d24eb407..a4c94061 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -932,6 +932,11 @@ async fn handle_put( // Single streaming path — handles both update and create internally, // swapping the file row onto the already-ingested blob. + // AuthZ audit #6 (2026-07-12): route `_with_perms` errors through + // `AppError::from` so authz denials surface as 404 (the anti-enum + // shape) instead of a `map_err → internal_error` 500 that gives a + // probing caller an "exists-but-denied" oracle. Also preserves + // `QuotaExceeded → 507`, `AlreadyExists → 409`, `InvalidInput → 400`. let stored = upload_service .update_file_streaming_with_perms( &internal_path, @@ -942,7 +947,7 @@ async fn handle_put( session.user.id, ) .await - .map_err(|e| AppError::internal_error(format!("Failed to store file: {}", e)))?; + .map_err(AppError::from)?; let status = if existed { StatusCode::NO_CONTENT @@ -1031,10 +1036,14 @@ async fn handle_mkcol( name: target_name.to_string(), parent_id: Some(parent_folder.id.clone()), }; + // AuthZ audit #7 (2026-07-12): route `_with_perms` errors through + // `AppError::from` so authz denials surface as 404 (the anti-enum + // shape) instead of a `map_err → internal_error` 500. Also preserves + // `AlreadyExists → 409`, `QuotaExceeded → 507`, `InvalidInput → 400`. folder_service .create_folder_with_perms(dto, user.id) .await - .map_err(|e| AppError::internal_error(format!("Failed to create folder: {}", e)))?; + .map_err(AppError::from)?; Ok(Response::builder() .status(StatusCode::CREATED) @@ -1076,20 +1085,22 @@ async fn handle_delete( Resource::Folder(folder_uuid), ) .await?; + // AuthZ audit #8 (2026-07-12): route service errors through + // `AppError::from` so authz denials surface as 404 (the + // anti-enum shape) instead of a `map_err → internal_error` + // 500 that gives a probing caller an "exists-but-denied" + // oracle. `move_to_trash` and `delete_folder_with_perms` + // both return `DomainError` and both call `authz.require`. if let Some(trash_svc) = state.trash_service.as_ref() { trash_svc .move_to_trash(&folder.id, "folder", user.id) .await - .map_err(|e| { - AppError::internal_error(format!("Failed to trash folder: {}", e)) - })?; + .map_err(AppError::from)?; } else { folder_service .delete_folder_with_perms(&folder.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!("Failed to delete folder: {}", e)) - })?; + .map_err(AppError::from)?; } } ResolvedResource::File(file) => { @@ -1103,21 +1114,18 @@ async fn handle_delete( Resource::File(file_uuid), ) .await?; + // AuthZ audit #8 (2026-07-12): same anti-enum fix as folder branch above. if let Some(trash_svc) = state.trash_service.as_ref() { trash_svc .move_to_trash(&file.id, "file", user.id) .await - .map_err(|e| { - AppError::internal_error(format!("Failed to trash file: {}", e)) - })?; + .map_err(AppError::from)?; } else { let file_mgmt = &state.applications.file_management_service; file_mgmt .delete_file_with_perms(&file.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!("Failed to delete file: {}", e)) - })?; + .map_err(AppError::from)?; } } } @@ -1195,6 +1203,12 @@ async fn handle_move( // then proceed with the move. Trashing is fine: per RFC the source // resource appears at the destination URI; what happens to the // overwritten one is up to the server. + // + // AuthZ audit #9 (2026-07-12): route the `_with_perms` delete + // errors through `AppError::from` so authz denials surface as 404 + // (anti-enum) instead of `map_err → internal_error` 500. Also + // preserves `QuotaExceeded → 507`, `AlreadyExists → 409`, + // `InvalidInput → 400`. match existing { ResolvedResource::File(existing_file) => { let file_uuid = Uuid::parse_str(&existing_file.id).map_err(|_| { @@ -1211,12 +1225,7 @@ async fn handle_move( file_mgmt .delete_and_cleanup_with_perms(&existing_file.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to overwrite destination file: {}", - e - )) - })?; + .map_err(AppError::from)?; } ResolvedResource::Folder(existing_folder) => { let folder_uuid = Uuid::parse_str(&existing_folder.id).map_err(|_| { @@ -1233,12 +1242,7 @@ async fn handle_move( folder_service .delete_folder_with_perms(&existing_folder.id, user.id) .await - .map_err(|e| { - AppError::internal_error(format!( - "Failed to overwrite destination folder: {}", - e - )) - })?; + .map_err(AppError::from)?; } } } @@ -1266,12 +1270,15 @@ async fn handle_move( None => "", }; + // AuthZ audit #9 (2026-07-12): route `_with_perms` errors + // through `AppError::from` so authz denials surface as 404 + // (anti-enum) instead of `map_err → internal_error` 500. if src_parent_sub == dest_parent_sub { // Same parent → rename. file_mgmt .rename_file_with_perms(&file.id, user.id, dest_name) .await - .map_err(|e| AppError::internal_error(format!("Rename failed: {}", e)))?; + .map_err(AppError::from)?; } else { // Different parent → move. let dest_parent = folder_service @@ -1282,14 +1289,14 @@ async fn handle_move( file_mgmt .move_file_with_perms(&file.id, user.id, Some(dest_parent.id.clone())) .await - .map_err(|e| AppError::internal_error(format!("Move failed: {}", e)))?; + .map_err(AppError::from)?; // If the filename changed too, rename after move. if file.name != dest_name { file_mgmt .rename_file_with_perms(&file.id, user.id, dest_name) .await - .map_err(|e| AppError::internal_error(format!("Rename failed: {}", e)))?; + .map_err(AppError::from)?; } } @@ -1331,6 +1338,9 @@ async fn handle_move( None => "", }; + // AuthZ audit #9 (2026-07-12): route `_with_perms` errors + // through `AppError::from` so authz denials surface as 404 + // (anti-enum) instead of `map_err → internal_error` 500. if src_parent_sub == dest_parent_sub { // Same parent → rename. use crate::application::dtos::folder_dto::RenameFolderDto; @@ -1343,7 +1353,7 @@ async fn handle_move( user.id, ) .await - .map_err(|e| AppError::internal_error(format!("Rename failed: {}", e)))?; + .map_err(AppError::from)?; } else { // Different parent → move. let dest_parent = folder_service @@ -1361,7 +1371,7 @@ async fn handle_move( user.id, ) .await - .map_err(|e| AppError::internal_error(format!("Move failed: {}", e)))?; + .map_err(AppError::from)?; // If the name changed too, rename. if folder.name != dest_name { @@ -1375,7 +1385,7 @@ async fn handle_move( user.id, ) .await - .map_err(|e| AppError::internal_error(format!("Rename failed: {}", e)))?; + .map_err(AppError::from)?; } } diff --git a/tests/api/calendar.hurl b/tests/api/calendar.hurl index d3fdd28f..e53c192e 100644 --- a/tests/api/calendar.hurl +++ b/tests/api/calendar.hurl @@ -234,13 +234,14 @@ jsonpath "$[?(@.subject.id == '{{bob_user_id}}')].resource.type" == "calendar" # ───────────────────────────────────────────────────────────── # Step 8c – Viewer Bob is denied on the unified list endpoint — -# `Share` is required, Viewer's bundle excludes it → 404 -# anti-enum shape (same treatment as any other resource type). +# `Share` is required, Viewer's bundle excludes it. Bob has Read +# on the calendar → graduated denial returns 403 (see +# [[project_authz_require_graduated_denial]]). # ───────────────────────────────────────────────────────────── GET {{base_url}}/api/grants?resource_type=calendar&resource_id={{calendar_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── diff --git a/tests/api/contacts.hurl b/tests/api/contacts.hurl index 075a8e14..9f6469b2 100644 --- a/tests/api/contacts.hurl +++ b/tests/api/contacts.hurl @@ -412,10 +412,12 @@ HTTP 200 jsonpath "$[?(@.id == '{{share_book_id}}')].is_readonly" == true -# Step 21 — Viewer bundle has no Create permission — Bob's -# contact write still 404s. Same minimal-body reasoning as -# Step 18b: keep the request valid at the wire layer so any -# rejection has to come from the AuthZ engine. +# Step 21 — Viewer bundle has no Create permission. Bob has Read +# on the address book (viewer role) so graduated denial returns +# 403, not 404 (see [[project_authz_require_graduated_denial]]). +# Same minimal-body reasoning as Step 18b: keep the request valid +# at the wire layer so any rejection has to come from the AuthZ +# engine. POST {{base_url}}/api/address-books/{{share_book_id}}/contacts Authorization: Bearer {{bob_token}} Content-Type: application/json @@ -423,7 +425,7 @@ Content-Type: application/json "full_name": "Viewer Cannot Write" } -HTTP 404 +HTTP 403 # Step 21b — Unified list-on-resource: Alice queries @@ -445,11 +447,12 @@ jsonpath "$[?(@.subject.id == '{{bob_user_id}}')].resource.type" == "address_boo # Step 21c — Viewer Bob is denied on the unified list endpoint — -# `Share` isn't in the Viewer bundle → 404 anti-enum shape. +# `Share` isn't in the Viewer bundle. Bob has Read → graduated +# denial returns 403 (see [[project_authz_require_graduated_denial]]). GET {{base_url}}/api/grants?resource_type=address_book&resource_id={{share_book_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # Step 22 — Alice revokes the grant. diff --git a/tests/api/drive_read_only.hurl b/tests/api/drive_read_only.hurl index db85331f..6f94540f 100644 --- a/tests/api/drive_read_only.hurl +++ b/tests/api/drive_read_only.hurl @@ -30,9 +30,13 @@ # 1. Baseline — drive not frozen → owner can upload / rename / # delete / trash / share (proves the fixture is writable). # 2. Admin freezes the drive via PATCH policies. -# 3. Every mutation attempt returns 404 (anti-enum): -# upload, rename, delete, trash-restore, permanent delete, -# create public link, rename the drive itself. +# 3. Every mutation attempt is refused. The engine's graduated +# denial returns 403 to the owner (who can Read their own +# drive) — anti-enum only kicks in for callers with no Read +# at all, whose 404 shape is exercised by the cross-tenant +# tests in `webdav_permissions.hurl` / `permissions.hurl`. +# Cases: upload, rename, delete, trash-restore, permanent +# delete, create public link, rename the drive itself. # 4. Read still works: GET /api/drives, GET /api/folders, # download the file, list trash. # 5. Admin unfreezes. @@ -203,9 +207,13 @@ jsonpath "$[?(@.id=='{{personal_drive_id}}')].policies.read_only" == true # ───────────────────────────────────────────────────────────── -# Step 8 — MUTATIONS BLOCKED. Upload → 404 (Create). -# Anti-enum: NotFound not 403, same shape as "no such -# folder." The engine gate emits an audit line with +# Step 8 — MUTATIONS BLOCKED. Upload → 403 (Create). +# Graduated denial: owner can Read their own frozen +# drive, so the engine returns `access_denied` → 403 +# rather than the anti-enum 404 (hiding a drive from +# its owner would be absurd). Cross-tenant callers with +# no Read on the drive still see 404 by the same code +# path. The engine gate emits an audit line with # `reason = drive_read_only` — inspectable in server # logs, not asserted here (no log-scraping harness). # ───────────────────────────────────────────────────────────── @@ -215,11 +223,11 @@ Authorization: Bearer {{owner_token}} folder_id: {{personal_root_id}} file: file,fixtures/hello.txt; text/plain -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 9 — Rename file A → 404 (Update). Endpoint is +# Step 9 — Rename file A → 403 (Update). Endpoint is # `PUT /api/files/{id}/rename` (not PATCH — the file # service exposes rename as a distinct verb, mirroring # the folder side). WebDAV MOVE would fire the same @@ -230,30 +238,30 @@ Authorization: Bearer {{owner_token}} Content-Type: application/json { "name": "renamed_during_freeze.txt" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 10 — Delete file A → 404 (Delete). +# Step 10 — Delete file A → 403 (Delete). # ───────────────────────────────────────────────────────────── DELETE {{base_url}}/api/trash/files/{{file_a_id}} Authorization: Bearer {{owner_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 11 — Restore file B from trash → 404 (Update on the +# Step 11 — Restore file B from trash → 403 (Update on the # soft-deleted row is a mutation like any other). # ───────────────────────────────────────────────────────────── POST {{base_url}}/api/trash/{{file_b_id}}/restore Authorization: Bearer {{owner_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 12 — Permanent delete of file B → 404 (Delete). +# Step 12 — Permanent delete of file B → 403 (Delete). # Note: the background retention purge SQL filter is # tested via source-review + a unit test on the # `delete_expired_bulk` query, not here — advancing @@ -265,11 +273,11 @@ HTTP 404 DELETE {{base_url}}/api/trash/{{file_b_id}} Authorization: Bearer {{owner_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 13 — Share creation → 404 (Share). Goes through +# Step 13 — Share creation → 403 (Share). Goes through # `share_service::create_shared_link` which calls # `authz.require(Share, Resource::File)` → engine gate. # ───────────────────────────────────────────────────────────── @@ -281,11 +289,11 @@ Content-Type: application/json "item_type": "file" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 14 — Grant (per-resource, not public link) → 404 (Share). +# Step 14 — Grant (per-resource, not public link) → 403 (Share). # Same engine gate — Share permission on File is # refused regardless of which endpoint asks for it. # ───────────────────────────────────────────────────────────── @@ -298,7 +306,7 @@ Content-Type: application/json "role": "viewer" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── diff --git a/tests/api/drives_membership.hurl b/tests/api/drives_membership.hurl index cc450d3a..5d454a5c 100644 --- a/tests/api/drives_membership.hurl +++ b/tests/api/drives_membership.hurl @@ -564,34 +564,35 @@ jsonpath "$[*].id" contains {{team_drive_id}} # `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. +# same `save_file_with_blob` gate. Bob has Read on the +# drive (viewer role cascades) → graduated denial returns +# 403 (see [[project_authz_require_graduated_denial]]). +# Also verify the batch / overwrite paths refuse — the +# whole chain from drive-membership to file write is +# exercised here. # ───────────────────────────────────────────────────────────── -# 22b.i — Fresh file: 404. +# 22b.i — Fresh file: 403. 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 +HTTP 403 -# 22b.ii — Overwrite attempt on the Editor-era upload: still 404. +# 22b.ii — Overwrite attempt on the Editor-era upload: still 403. # `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. +# would otherwise 409). 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 +HTTP 403 # 22b.iii — Alice's Editor-era file is untouched. @@ -698,8 +699,8 @@ jsonpath "$.role" == "viewer" # ───────────────────────────────────────────────────────────── # Step 25 — Viewer CANNOT edit drive members. -# Bob is Viewer. Every member-mutation verb → 404 -# (anti-enum: same shape as if the drive didn't exist). +# Bob is Viewer (has Read on the drive) → graduated denial +# returns 403 on every member-mutation verb. # ───────────────────────────────────────────────────────────── POST {{base_url}}/api/drives/{{team_drive_id}}/members Authorization: Bearer {{bob_token}} @@ -709,7 +710,7 @@ Content-Type: application/json "role": "editor" } -HTTP 404 +HTTP 403 PATCH {{base_url}}/api/drives/{{team_drive_id}}/members/user/{{carol_user_id}} @@ -717,13 +718,13 @@ Authorization: Bearer {{bob_token}} Content-Type: application/json { "role": "viewer" } -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/drives/{{team_drive_id}}/members/user/{{carol_user_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -739,7 +740,7 @@ Content-Type: application/json HTTP 200 -# 26a — Editor POST /api/drives/{id}/members → 404. +# 26a — Editor POST /api/drives/{id}/members → 403 (Editor has Read). POST {{base_url}}/api/drives/{{team_drive_id}}/members Authorization: Bearer {{bob_token}} Content-Type: application/json @@ -748,39 +749,39 @@ Content-Type: application/json "role": "viewer" } -HTTP 404 +HTTP 403 -# 26b — Editor PATCH a member → 404. +# 26b — Editor PATCH a member → 403. PATCH {{base_url}}/api/drives/{{team_drive_id}}/members/user/{{carol_user_id}} Authorization: Bearer {{bob_token}} Content-Type: application/json { "role": "viewer" } -HTTP 404 +HTTP 403 -# 26c — Editor DELETE a member → 404. +# 26c — Editor DELETE a member → 403. DELETE {{base_url}}/api/drives/{{team_drive_id}}/members/user/{{carol_user_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 -# 26d — Editor renames the drive (root folder) → 404. +# 26d — Editor renames the drive (root folder) → 403. # Folder rename normally requires `Permission::Update` (which # Editor has on every folder in the drive via the engine's drive # precheck). The folder service promotes the requirement to # `Permission::Manage` when the target folder has `parent_id IS # NULL` — i.e. it's a drive root — so the drive-rename surface is # Owner-only per drive.md §6, without changing the public folder -# endpoint shape. Anti-enum: refusal returns 404 (not 403). +# endpoint shape. Editor has Read → graduated denial → 403. PUT {{base_url}}/api/folders/{{team_root_folder_id}}/rename Authorization: Bearer {{bob_token}} Content-Type: application/json { "name": "team-drive-editor-renamed" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -883,7 +884,7 @@ HTTP 404 # ───────────────────────────────────────────────────────────── # Step 30 — Drive delete (D3b). -# - Non-Owner → 404 (Bob is Viewer post-Step 28). +# - Non-Owner → 403 (Bob is Viewer post-Step 28, has Read). # - Owner on non-empty drive → 409 (the editor-created-folder # from Step 27 is still live). # - Owner after the folder is trashed → 204. @@ -892,12 +893,11 @@ HTTP 404 # we exercise its 405 below. # ───────────────────────────────────────────────────────────── -# 30a — Viewer (Bob) cannot delete the drive → 404, anti-enum same as -# the member-mutation refusals. +# 30a — Viewer (Bob) cannot delete the drive → 403 (has Read). DELETE {{base_url}}/api/drives/{{team_drive_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # 30b — Owner (Alice) on a non-empty drive → 409 with the canonical diff --git a/tests/api/grants.hurl b/tests/api/grants.hurl index 75135178..d0affa9f 100644 --- a/tests/api/grants.hurl +++ b/tests/api/grants.hurl @@ -137,14 +137,15 @@ jsonpath "$.grants[0].role" == "viewer" # ───────────────────────────────────────────────────────────── -# Step 7 — Viewer cannot rename (no update grant). +# Step 7 — Viewer cannot rename (no Update grant). Dave has Read +# (viewer role) → graduated denial returns 403. # ───────────────────────────────────────────────────────────── PUT {{base_url}}/api/folders/{{shared_folder_id}}/rename Authorization: Bearer {{dave_token}} Content-Type: application/json { "name": "bob-tried-again" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -260,14 +261,15 @@ jsonpath "$[0].role" == "viewer" # ───────────────────────────────────────────────────────────── -# Step 16 — Demoted Bob can no longer rename. +# Step 16 — Demoted Bob (now Viewer) can no longer rename. Read +# is still granted → graduated denial returns 403. # ───────────────────────────────────────────────────────────── PUT {{base_url}}/api/folders/{{shared_folder_id}}/rename Authorization: Bearer {{dave_token}} Content-Type: application/json { "name": "bob-tried-after-demote" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -594,34 +596,37 @@ Authorization: Bearer {{adam_token}} HTTP 200 -# ── Mutations still denied (Viewer has no Update/Create/Delete) ─ +# ── Mutations still denied (Viewer has no Update/Create/Delete). +# Viewer has Read → graduated denial returns 403 (not 404 +# anti-enum, which is reserved for Phase 2A above where Adam +# had no Read at all). POST {{base_url}}/api/folders Authorization: Bearer {{adam_token}} Content-Type: application/json { "name": "adam-attack-2", "parent_id": "{{perm_folder_id}}" } -HTTP 404 +HTTP 403 PUT {{base_url}}/api/folders/{{perm_folder_id}}/rename Authorization: Bearer {{adam_token}} Content-Type: application/json { "name": "adam-rename-as-viewer" } -HTTP 404 +HTTP 403 PUT {{base_url}}/api/files/{{perm_file_id}}/rename Authorization: Bearer {{adam_token}} Content-Type: application/json { "name": "adam-file-rename-as-viewer" } -HTTP 404 +HTTP 403 PUT {{base_url}}/api/files/{{perm_file_id}}/thumbnail/icon Authorization: Bearer {{adam_token}} Content-Type: image/png file,fixtures/blue-image.png; -HTTP 404 +HTTP 403 POST {{base_url}}/api/files/upload Authorization: Bearer {{adam_token}} @@ -629,17 +634,17 @@ Authorization: Bearer {{adam_token}} folder_id: {{perm_folder_id}} file: file,fixtures/hello.txt; text/plain -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/files/{{perm_file_id}} Authorization: Bearer {{adam_token}} -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/folders/{{perm_folder_id}} Authorization: Bearer {{adam_token}} -HTTP 404 +HTTP 403 # ── Viewer cannot start a chunked upload (no Create grant) ── POST {{base_url}}/api/uploads @@ -653,7 +658,7 @@ Content-Type: application/json "chunk_size": 3000000 } -HTTP 404 +HTTP 403 # ════════════════════════════════════════════════════════════════════ @@ -813,16 +818,17 @@ Authorization: Bearer {{adam_token}} HTTP 204 -# ── Delete still denied (Editor excludes Delete) ──────────── +# ── Delete still denied (Editor excludes Delete). Editor has +# Read → graduated denial returns 403. DELETE {{base_url}}/api/files/{{perm_file_id}} Authorization: Bearer {{adam_token}} -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/folders/{{perm_folder_id}} Authorization: Bearer {{adam_token}} -HTTP 404 +HTTP 403 # ════════════════════════════════════════════════════════════════════ diff --git a/tests/api/grants_nested_groups.hurl b/tests/api/grants_nested_groups.hurl index a7b51a45..480edbb3 100644 --- a/tests/api/grants_nested_groups.hurl +++ b/tests/api/grants_nested_groups.hurl @@ -367,34 +367,38 @@ HTTP 200 jsonpath "$.items[?(@.resource.id=='{{perm_folder_id}}')].resource_type" == "folder" jsonpath "$.items[?(@.resource.id=='{{perm_folder_id}}')].permissions" contains "read" -# ── Mutations still denied (Viewer has no Update/Create/Delete) ─ +# ── Mutations still denied (Viewer has no Update/Create/Delete). +# Henry has Read via nested-group cascade → graduated denial +# returns 403 (see [[project_authz_require_graduated_denial]]). +# Anti-enum 404 stays reserved for the earlier phase where the +# cascade hadn't given Henry any Read at all. POST {{base_url}}/api/folders Authorization: Bearer {{henry_token}} Content-Type: application/json { "name": "henry-attack-2", "parent_id": "{{perm_folder_id}}" } -HTTP 404 +HTTP 403 PUT {{base_url}}/api/folders/{{perm_folder_id}}/rename Authorization: Bearer {{henry_token}} Content-Type: application/json { "name": "henry-rename-as-viewer" } -HTTP 404 +HTTP 403 PUT {{base_url}}/api/files/{{perm_file_id}}/rename Authorization: Bearer {{henry_token}} Content-Type: application/json { "name": "henry-file-rename-as-viewer" } -HTTP 404 +HTTP 403 PUT {{base_url}}/api/files/{{perm_file_id}}/thumbnail/icon Authorization: Bearer {{henry_token}} Content-Type: image/png file,fixtures/blue-image.png; -HTTP 404 +HTTP 403 POST {{base_url}}/api/files/upload Authorization: Bearer {{henry_token}} @@ -402,17 +406,17 @@ Authorization: Bearer {{henry_token}} folder_id: {{perm_folder_id}} file: file,fixtures/hello.txt; text/plain -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/files/{{perm_file_id}} Authorization: Bearer {{henry_token}} -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/folders/{{perm_folder_id}} Authorization: Bearer {{henry_token}} -HTTP 404 +HTTP 403 # Viewer cannot start a chunked upload (no Create grant). POST {{base_url}}/api/uploads @@ -426,7 +430,7 @@ Content-Type: application/json "chunk_size": 3000000 } -HTTP 404 +HTTP 403 # ════════════════════════════════════════════════════════════════════ @@ -520,16 +524,16 @@ HTTP 200 [Asserts] jsonpath "$[?(@.id=='{{henry_chunked_file_id}}')].name" == "henry-chunked-video.mp4" -# Editor still cannot delete. +# Editor still cannot delete. Editor bundle carries Read → 403. DELETE {{base_url}}/api/files/{{perm_file_id}} Authorization: Bearer {{henry_token}} -HTTP 404 +HTTP 403 DELETE {{base_url}}/api/folders/{{perm_folder_id}} Authorization: Bearer {{henry_token}} -HTTP 404 +HTTP 403 # ════════════════════════════════════════════════════════════════════ diff --git a/tests/api/playlists.hurl b/tests/api/playlists.hurl index fd8a8283..d8956818 100644 --- a/tests/api/playlists.hurl +++ b/tests/api/playlists.hurl @@ -204,38 +204,38 @@ jsonpath "$[*].id" contains "{{playlist_id}}" # ───────────────────────────────────────────────────────────── # Step 11 – Bob cannot rename the playlist. Viewer's bundle is -# Read-only (no Update), so `require_playlist_perm(Update)` denies -# with the 404 anti-enum shape. +# Read-only (no Update). Bob has Read → graduated denial returns +# 403 (see [[project_authz_require_graduated_denial]]). # ───────────────────────────────────────────────────────────── PUT {{base_url}}/api/playlists/{{playlist_id}} Authorization: Bearer {{bob_token}} Content-Type: application/json { "name": "hijacked" } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── # Step 12 – Bob cannot delete the playlist. Viewer's bundle -# excludes Delete → 404. +# excludes Delete → 403 (Read granted). # ───────────────────────────────────────────────────────────── DELETE {{base_url}}/api/playlists/{{playlist_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── # Step 13 – Bob cannot re-share the playlist. Viewer's bundle -# excludes Share → 404 on the legacy /share endpoint (which now -# routes through `authz.require(Share)`). +# excludes Share → 403 (Read granted). The legacy /share endpoint +# routes through `authz.require(Share)`. # ───────────────────────────────────────────────────────────── POST {{base_url}}/api/playlists/{{playlist_id}}/share Authorization: Bearer {{bob_token}} Content-Type: application/json { "user_id": "{{alice_user_id}}", "can_write": true } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -277,13 +277,14 @@ jsonpath "$[?(@.subject.id == '{{bob_user_id}}')].resource.type" == "playlist" # ───────────────────────────────────────────────────────────── # Step 14c – Bob (Viewer only) is denied on the unified list -# endpoint: `Share` is required, Viewer's bundle excludes it → -# 404 anti-enum shape. +# endpoint: `Share` is required, Viewer's bundle excludes it. +# Bob has Read → graduated denial returns 403 (see +# [[project_authz_require_graduated_denial]]). # ───────────────────────────────────────────────────────────── GET {{base_url}}/api/grants?resource_type=playlist&resource_id={{playlist_id}} Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -346,13 +347,14 @@ jsonpath "$.description" == "renamed by editor bob" # ───────────────────────────────────────────────────────────── # Step 19 – Editor still cannot Share (Share stays Owner-only). +# Bob has Read (Editor bundle) → graduated denial returns 403. # ───────────────────────────────────────────────────────────── POST {{base_url}}/api/playlists/{{playlist_id}}/share Authorization: Bearer {{bob_token}} Content-Type: application/json { "user_id": "{{alice_user_id}}", "can_write": false } -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── diff --git a/tests/api/search_basic.hurl b/tests/api/search_basic.hurl index 919ca997..02a7b92b 100644 --- a/tests/api/search_basic.hurl +++ b/tests/api/search_basic.hurl @@ -110,7 +110,7 @@ Authorization: Bearer {{admin_token}} HTTP 200 [Asserts] -jsonpath "$.files" count >= 1 +jsonpath "$.files" count >= 1 body contains "{{needle_file_id}}" @@ -124,7 +124,7 @@ Authorization: Bearer {{admin_token}} HTTP 200 [Asserts] -jsonpath "$.files" count == 0 +jsonpath "$.files" count == 0 jsonpath "$.folders" count == 0 @@ -152,6 +152,29 @@ body not contains "unique-search-needle" body not contains "{{needle_file_id}}" +# ───────────────────────────────────────────────────────────── +# 5b — REGRESSION: `/api/search/suggest` MUST also refuse to +# surface admin's file to bob. Pre-fix (AuthZ audit #1, +# 2026-07-12) the suggest endpoint had NO `AuthUser` +# extractor and its underlying `suggest_files_by_name` / +# `suggest_folders_by_name` filtered only on +# `NOT is_trashed AND name ILIKE $1` — any authenticated +# user (including externals) could autocomplete names and +# full `path` values across every tenant on the instance. +# Fix: added `caller_id` to both repo queries via the +# shared `CALLER_CAN_READ_DRIVE` predicate (`role_grants` +# + `caller_group_ids`). This assertion is the anti- +# regression pin. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/search/suggest?query=unique-search-needle +Authorization: Bearer {{bob_token}} + +HTTP 200 +[Asserts] +body not contains "unique-search-needle" +body not contains "{{needle_file_id}}" + + # ───────────────────────────────────────────────────────────── # 6 — CONTENT-search cross-drive isolation (docs/plan/drive.md §11). # The cross-user check above (step 5) verifies the NAME-search @@ -209,7 +232,7 @@ HTTP 200 # Bob has no access to admin's drive → Tantivy's Must-clause # filters every doc that doesn't carry one of Bob's drive_ids, # so the file vanishes entirely. -jsonpath "$.files" count == 0 +jsonpath "$.files" count == 0 jsonpath "$.folders" count == 0 body not contains "{{canary_file_id}}" body not contains "ContentIndexCanaryXyzzy2026Drive" @@ -221,11 +244,11 @@ body not contains "ContentIndexCanaryXyzzy2026Drive" # other field names below MUST stay absent: a future field # called `hidden_count`/`filtered`/etc. that reveals matches # Bob can't see would be the regression. -jsonpath "$.total_count" == 0 -jsonpath "$.has_more" == false +jsonpath "$.total_count" == 0 +jsonpath "$.has_more" == false jsonpath "$.hidden_count" not exists -jsonpath "$.filtered" not exists -jsonpath "$.total" not exists +jsonpath "$.filtered" not exists +jsonpath "$.total" not exists # ───────────────────────────────────────────────────────────── diff --git a/tests/api/webdav_permissions.hurl b/tests/api/webdav_permissions.hurl index 48707af3..57cf811e 100644 --- a/tests/api/webdav_permissions.hurl +++ b/tests/api/webdav_permissions.hurl @@ -6,9 +6,10 @@ # # 1. Per-role gates through the drive-scope resolver: a Viewer on a # shared drive can PROPFIND/GET but cannot MKCOL/PUT/MOVE. An -# Editor can. AuthZ denials return `NotFound` (anti-enum), so -# a probing caller can't tell a genuinely-missing folder from -# one they simply lack Create on. +# Editor can. AuthZ denials use graduated shape: a caller with +# Read on the target (Viewer here) gets 403 Forbidden — no point +# hiding existence from someone already reading it. A caller with +# no Read at all gets 404 (anti-enum), matching "no such folder". # # 2. Drive policy `forbid_cross_drive_move` gates MOVE at the # SOURCE drive (see `DrivePolicies::refuse_cross_drive_move` @@ -123,17 +124,22 @@ HTTP 207 # ───────────────────────────────────────────────────────────── # Step 6 — Bob (VIEWER) CANNOT MKCOL on the shared drive. -# `authz.require(Create, Folder)` denial returns -# `DomainError::not_found` (anti-enum), which maps to 404. +# `authz.require(Create, Folder)` denies. Bob has Read +# on the drive (viewer role) → engine's graduated denial +# returns `DomainError::access_denied` → 403 Forbidden. +# Anti-enum still holds for callers with no Read at all +# (would surface as 404); this is the "you can see it, +# but can't touch it" branch. # ───────────────────────────────────────────────────────────── MKCOL {{base_url}}/webdav/@drive/{{shared_drive_id}}/viewer-blocked-folder Authorization: Bearer {{bob_token}} -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── -# Step 7 — Bob (VIEWER) CANNOT PUT a file. +# Step 7 — Bob (VIEWER) CANNOT PUT a file. Same 403 shape +# (Bob has Read on the drive). # ───────────────────────────────────────────────────────────── PUT {{base_url}}/webdav/@drive/{{shared_drive_id}}/viewer-blocked-file.txt Authorization: Bearer {{bob_token}} @@ -142,7 +148,7 @@ Content-Type: text/plain viewer should not upload ``` -HTTP 404 +HTTP 403 # ───────────────────────────────────────────────────────────── @@ -158,13 +164,47 @@ HTTP 201 # ───────────────────────────────────────────────────────────── # Step 9 — Bob (VIEWER) CANNOT MOVE (rename) the probe folder. # MOVE requires Update on the source, which Viewer -# doesn't have. Same anti-enum 404 shape. +# doesn't have. Bob can Read the folder (viewer) → 403. # ───────────────────────────────────────────────────────────── MOVE {{base_url}}/webdav/@drive/{{shared_drive_id}}/probe-folder Authorization: Bearer {{bob_token}} Destination: {{base_url}}/webdav/@drive/{{shared_drive_id}}/probe-folder-renamed -HTTP 404 +HTTP 403 + + +# ───────────────────────────────────────────────────────────── +# Step 9b — Bob (VIEWER) CANNOT COPY the probe folder. +# COPY requires Create on the destination parent, which +# Viewer doesn't have. Bob has Read on both source and +# destination parent → 403 (graduated denial). +# +# This is the regression pin for AuthZ audit #2 +# (2026-07-12): the COPY handler used to `map_err(|e| +# AppError::internal_error(format!("Failed to copy folder +# tree: {}", e)))?` on `copy_folder_tree_with_perms`, +# collapsing the `DomainError` engine returned on denial +# into HTTP 500 — an "exists-but-denied" oracle. Fix +# routes through `AppError::from` so the same denial +# surfaces as the correct 403 / 404 per graduated-denial +# policy. +# ───────────────────────────────────────────────────────────── +COPY {{base_url}}/webdav/@drive/{{shared_drive_id}}/probe-folder +Authorization: Bearer {{bob_token}} +Destination: {{base_url}}/webdav/@drive/{{shared_drive_id}}/probe-folder-copy + +HTTP 403 + + +# ───────────────────────────────────────────────────────────── +# Step 9c — Bob (VIEWER) CANNOT DELETE the probe folder. +# DELETE requires Delete on the target, which Viewer +# doesn't have. Bob has Read → 403. +# ───────────────────────────────────────────────────────────── +DELETE {{base_url}}/webdav/@drive/{{shared_drive_id}}/probe-folder +Authorization: Bearer {{bob_token}} + +HTTP 403 # ─────────────────────────────────────────────────────────────