diff --git a/Cargo.toml b/Cargo.toml index 71ee207f..00e65fd4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -354,6 +354,21 @@ name = "bench_micro_allocs" path = "examples/bench_micro_allocs.rs" required-features = ["bench"] +# Round-24 battery ──────────────────────────────────────────────────────────── + +# Round-24 download_zip authz+metadata N+1 → batch, VALIDATED. The per-file +# get_file_with_perms (require + get, 2 round-trips/file) becomes one +# check_files_read_batch + one get_files_by_ids. Because the change is +# authorization-sensitive, the gate is the security property: the batch check +# must make the identical per-file inclusion decision (same set AND input order) +# as the shipped-before require loop, over owned + denied + missing ids, and +# never include a denied/missing file. Drives the real PgAclEngine. Needs the +# dev Postgres up (reads DATABASE_URL from .env). +[[example]] +name = "bench_round24_zip_authz" +path = "examples/bench_round24_zip_authz.rs" +required-features = ["bench"] + # Round-23 battery ──────────────────────────────────────────────────────────── # Round-23 CPU/alloc micro-pack (no Postgres) — deterministic alloc gates for diff --git a/benches/ROUND24.md b/benches/ROUND24.md new file mode 100644 index 00000000..39af351b --- /dev/null +++ b/benches/ROUND24.md @@ -0,0 +1,150 @@ +# Round 24 — `download_zip` per-item authz+metadata N+1 → batch (validated authorization pass) + +This is the ROUND23 "not shipped" item #2, given the dedicated validated pass it +needed. Unlike the other rounds it is **authorization-sensitive**, so the gate is +not an allocation count or a latency floor — it is the **security property +itself**: the batched authorization must make the *identical* per-file inclusion +decision as the shipped-before per-file `require` loop, and must never let a +denied or missing file into the archive. + +Reproduce (needs the dev Postgres up; reads `DATABASE_URL` from `.env`): + +``` +cargo run --release --features bench --example bench_round24_zip_authz +``` + +## The change + +`BatchOperations::download_zip` streamed a client's multi-selection into a ZIP. +For the **individually-selected files** it looped, per file: + +```rust +for file_id in &file_ids { + match self.file_retrieval.get_file_with_perms(file_id, user_id).await { // require + get = 2 round-trips + Ok(file_dto) => { self.add_file_entry_streamed(&mut zip, file_id, &file_dto.name, &file_dto.mime_type, Some(user_id)).await … } + Err(_) => { /* skip + log */ } + } +} +``` + +`get_file_with_perms` is `require_file` (a `Read` authz round-trip) **plus** +`get_file` (a metadata round-trip) — so a selection of N files is **2N serial +round-trips** before a single byte is streamed. AFTER routes the whole selection +through one new service method: + +```rust +let authorized = self.file_retrieval + .get_files_by_ids_with_perms(&file_ids, user_id).await?; // 1 batch check + 1 batch get +let by_id: HashMap = authorized.into_iter() + .filter_map(|f| Uuid::parse_str(&f.id).ok().map(|u| (u, f))).collect(); +for file_id in &file_ids { // same input order + let Some(file_dto) = Uuid::parse_str(file_id).ok().and_then(|u| by_id.get(&u)) else { + info!("Skipping file {file_id} (not accessible or missing)"); continue; + }; + self.add_file_entry_streamed(&mut zip, file_id, &file_dto.name, &file_dto.mime_type, Some(user_id)).await … +} +``` + +`FileRetrievalService::get_files_by_ids_with_perms` authorizes every id in ONE +`AuthorizationEngine::check_files_read_batch` (the `PgAclEngine` override resolves +all files' drives in a single query and reuses the per-drive role cache) and +fetches only the authorized ids in ONE `get_files_by_ids`. **2N round-trips → 2.** + +### Why this is authorization-safe (the part that made it a dedicated pass) + +Three properties had to hold, all verified against the source before touching it: + +1. **Authorization still happens before any ZIP entry is written.** + `add_file_entry_streamed` writes the entry header (the **filename**) *before* + it opens the authorized stream (`write_entry_stream` then + `get_file_stream_with_perms`). So the pre-filter is load-bearing: a denied + file must never reach `add_file_entry_streamed`, or its name would leak into + the archive (and leave a dangling entry). AFTER preserves this exactly — a + denied/missing id is absent from `by_id`, so it is `continue`-skipped and + never reaches the entry write. The authz simply moved from a per-file + `require` to one batch `check` **earlier** in the same function, not into or + after the stream. + +2. **The per-file stream-open Read check + Recents recording are unchanged.** + `add_file_entry_streamed(Some(user_id))` still calls + `get_file_stream_with_perms`, which re-checks `Read` (now a primed-cache hit — + `check_files_read_batch` seeds the resource→drive cache) and records the + access in Recents. The old loop double-notified Recents (once in + `get_file_with_perms`, once in the stream open) and the throttle coalesced it + to one entry; AFTER notifies once (the stream open) — identical net effect. + +3. **The batch authorization is identical to looping `require`.** + `check_files_read_batch` is documented and gated as "semantically identical to + looping `check`", and `require(Read)` succeeds iff `check(Read)` is true (a + denied `Read` is the 404 anti-enumeration shape). The §validation gate proves + this empirically on a mix of granted / denied / missing ids. + +The **folder** selections (`get_folder_with_perms` per root, then the already-bulk +`add_folder_subtree_to_zip`) are left as-is: root counts are small and there is no +`check_folders_read_batch` primitive to batch through — see *Not shipped*. + +## The validation + +`bench_round24_zip_authz` drives the **real `PgAclEngine`** (the `fresh_engine` +shape from `bench_favorites_authz`) against a seeded fixture designed to exercise +every inclusion outcome: + +- `owned` — N files on **drive A**, which the caller holds an `editor` grant on → **must be INCLUDED** +- `denied` — N files on **drive B**, which the caller has **no** grant on → **must be DENIED** +- `missing` — N random UUIDs that don't exist → **must be MISSING** + +interleaved `owned, denied, missing, owned, …` so the **order** test is real. The +gate asserts, and `exit(1)`s on any failure: + +- `before_included` (the per-file `require` filter, in input order) **==** + `after_included` (the batch `check_files_read_batch` filter, in input order) — + identical **set and order**; +- the included set is **exactly** the caller's `owned` files; +- **no** `denied` (other-drive) file is included — the authz-regression tripwire; +- **no** `missing` id is included; +- the batch `get_files_by_ids` of the authorized ids returns **exactly** the + `owned` files. + +Latency (cold engine, empty caches — the first-download shape), `BENCH_FILES=200` +(600-item interleaved selection, ⅓ owned / ⅓ denied / ⅓ missing): + +| arm | wall (600 items) | per file | +|---|---|---| +| per-file `require` loop | 559.47 ms | 932.45 µs | +| batch `check_files_read_batch` | 266.58 ms | 444.30 µs | + +**2.10×** — and this is the *conservative* case: with ⅓ of the ids on a drive +the caller has no role on, `check_files_read_batch` still falls back to a per-file +`check_inner` for each un-readable-drive file. The realistic "download my own N +files" selection is **all** on drives the caller has a role on, where the batch +is genuinely O(1) (one drive-resolve query + cached role checks) against the +loop's 2N round-trips — a far larger win. + +## Not shipped + +- **Folder selections** (`download_zip`'s folder loop): `get_folder_with_perms` + per selected root. Root counts are typically 1–3, and there is no + `check_folders_read_batch` batch-authz primitive (only files have one), so + batching would still loop `check` per root — no round-trip win. Left as-is. +- **Dropping the stream-open re-check**: since the batch pre-check already + authorized (and primed the cache), `add_file_entry_streamed`'s + `get_file_stream_with_perms` re-check is now redundant (a cache hit). Replacing + it with the no-perms `get_file_stream` would save the cache lookups but would + also drop the Recents recording and the second authz barrier — not worth the + behavior change; kept as belt-and-suspenders. + +## Environment / methodology + +- Real `PgAclEngine` + `FileBlobReadRepository` against a local **PostgreSQL 16** + (schema from `migrations/`). The bench seeds its own two-drive fixture + (`bench_zipauthz_*` markers) and tears it down around the run. +- Built with `RUSTFLAGS="-C target-cpu=x86-64-v3"` (this session's host + intermittently `SIGILL`ed rustc under the repo's default `-C target-cpu=native` + AVX-512 after a host migration — see benches/ROUND23.md). Local build-flag + override only; the checked-in `.cargo/config.toml` is unchanged. +- The gate is the security equivalence (set + order + denied/missing exclusion), + not a perf threshold; the latency table is supporting evidence for the + round-trip collapse. +- Verified beyond the bench: `cargo clippy --features bench --all-targets + -D warnings` clean, `cargo fmt --all --check` clean, `cargo test --lib + --features bench` = 529 passed / 0 failed. diff --git a/examples/bench_round24_zip_authz.rs b/examples/bench_round24_zip_authz.rs new file mode 100644 index 00000000..8857ce56 --- /dev/null +++ b/examples/bench_round24_zip_authz.rs @@ -0,0 +1,387 @@ +//! Round-24 — `download_zip` per-item authz+metadata N+1 → batch, VALIDATED. +//! +//! `BatchOperations::download_zip` authorized + fetched each selected file with +//! a per-file `get_file_with_perms` (= `require_file` authz + `get_file`) — 2 +//! serial round-trips per file, before any streaming. AFTER routes the whole +//! multi-select through `FileRetrievalService::get_files_by_ids_with_perms`, +//! which authorizes every id in ONE `check_files_read_batch` and fetches the +//! authorized ids in ONE `get_files_by_ids` (2 round-trips total). The +//! subsequent `add_file_entry_streamed` keeps its own per-file stream-open Read +//! check + Recents recording (now a primed-cache hit), so authorization still +//! happens BEFORE any ZIP entry is written — a denied file never leaks its name. +//! +//! Because this change is authorization-sensitive, the gate is the security +//! property itself: the batch `check_files_read_batch` must make the EXACT same +//! per-file inclusion decision as the shipped-before per-file `require` loop — +//! same **set** AND same **input order** — over a mix of +//! • files on a drive the caller is granted `editor` on (INCLUDED) +//! • files on a drive the caller has NO grant on (DENIED) +//! • ids that don't exist at all (MISSING) +//! and the batch fetch must return exactly the authorized, existing files. +//! Any divergence `std::process::exit(1)`s. +//! +//! Drives the REAL `PgAclEngine` + `FileBlobReadRepository` (the fresh_engine +//! shape from bench_favorites_authz). +//! +//! Run (needs Postgres up; reads DATABASE_URL from .env): +//! cargo run --release --features bench --example bench_round24_zip_authz +//! Tunables (env): BENCH_FILES (200), BENCH_POOL (20). + +use std::collections::HashSet; +use std::env; +use std::sync::Arc; +use std::time::{Duration, Instant}; + +use oxicloud::application::ports::authorization_ports::AuthorizationEngine; +use oxicloud::domain::services::authorization::{Permission, Resource, Subject}; +use oxicloud::infrastructure::repositories::pg::{ + FileBlobReadRepository, FolderDbRepository, SubjectGroupPgRepository, +}; +use oxicloud::infrastructure::services::dedup_service::DedupService; +use oxicloud::infrastructure::services::local_blob_backend::LocalBlobBackend; +use oxicloud::infrastructure::services::pg_acl_engine::PgAclEngine; +use sqlx::PgPool; +use sqlx::postgres::PgPoolOptions; +use uuid::Uuid; + +fn env_or(key: &str, default: T) -> T { + env::var(key) + .ok() + .and_then(|v| v.parse().ok()) + .unwrap_or(default) +} + +struct Seeded { + caller: Uuid, + other: Uuid, + drive_a: Uuid, + drive_b: Uuid, + root_a: Uuid, + root_b: Uuid, + blob_hash: String, + /// The caller's accessible files (drive A) — the expected INCLUDED set. + owned: Vec, + /// Files on drive B (no grant to caller) — expected DENIED. + denied: Vec, + /// Non-existent ids — expected MISSING. + missing: Vec, + /// The full selection, interleaved owned/denied/missing (order matters). + selection: Vec, +} + +async fn seed(pool: &PgPool, n_files: usize) -> Seeded { + let mut tx = pool.begin().await.expect("begin"); + let caller: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_zipauthz_a', 'bench_zipauthz_a@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed caller"); + let other: Uuid = sqlx::query_scalar( + "INSERT INTO auth.users (username, email, role) + VALUES ('bench_zipauthz_b', 'bench_zipauthz_b@bench.invalid', 'user') RETURNING id", + ) + .fetch_one(&mut *tx) + .await + .expect("seed other"); + + let blob_hash = "benchzipauthz00000000000000000000000000000000000000000000000b24".to_string(); + sqlx::query("INSERT INTO storage.blobs (hash, size, ref_count) VALUES ($1, 1, 1)") + .bind(&blob_hash) + .execute(&mut *tx) + .await + .expect("seed blob"); + + // Two shared drives; `caller` is granted editor on A only, `other` on B. + async fn drive_with_grant( + tx: &mut sqlx::Transaction<'_, sqlx::Postgres>, + label: &str, + grantee: Uuid, + ) -> (Uuid, Uuid) { + let drive: Uuid = + sqlx::query_scalar("INSERT INTO storage.drives (kind) VALUES ('shared') RETURNING id") + .fetch_one(&mut **tx) + .await + .expect("seed drive"); + let root: Uuid = sqlx::query_scalar( + "INSERT INTO storage.folders (name, path, lpath, drive_id) + VALUES ($1, $2, 'x', $3) RETURNING id", + ) + .bind(format!("Bench {label}")) + .bind(format!("/Bench {label}")) + .bind(drive) + .fetch_one(&mut **tx) + .await + .expect("seed folder"); + sqlx::query("UPDATE storage.drives SET root_folder_id = $1 WHERE id = $2") + .bind(root) + .bind(drive) + .execute(&mut **tx) + .await + .expect("stamp root"); + sqlx::query( + "INSERT INTO storage.role_grants + (subject_type, subject_id, resource_type, resource_id, role, granted_by) + VALUES ('user', $1, 'drive', $2, 'editor'::storage.grant_role, $1)", + ) + .bind(grantee) + .bind(drive) + .execute(&mut **tx) + .await + .expect("seed grant"); + (drive, root) + } + + let (drive_a, root_a) = drive_with_grant(&mut tx, "A", caller).await; + let (drive_b, root_b) = drive_with_grant(&mut tx, "B", other).await; + + let mut owned = Vec::with_capacity(n_files); + let mut denied = Vec::with_capacity(n_files); + for i in 0..n_files { + for (drive, root, sink) in [ + (drive_a, root_a, &mut owned), + (drive_b, root_b, &mut denied), + ] { + let id: Uuid = sqlx::query_scalar( + "INSERT INTO storage.files (name, folder_id, blob_hash, size, mime_type, drive_id) + VALUES ($1, $2, $3, 1, 'text/plain', $4) RETURNING id", + ) + .bind(format!("bench-{i:04}.txt")) + .bind(root) + .bind(&blob_hash) + .bind(drive) + .fetch_one(&mut *tx) + .await + .expect("seed file"); + sink.push(id); + } + } + tx.commit().await.expect("commit"); + + let missing: Vec = (0..n_files).map(|_| Uuid::new_v4()).collect(); + + // Interleave owned / denied / missing so the order test is meaningful. + let mut selection = Vec::with_capacity(n_files * 3); + for i in 0..n_files { + selection.push(owned[i]); + selection.push(denied[i]); + selection.push(missing[i]); + } + + Seeded { + caller, + other, + drive_a, + drive_b, + root_a, + root_b, + blob_hash, + owned, + denied, + missing, + selection, + } +} + +async fn cleanup(pool: &PgPool, s: &Seeded) { + for d in [s.drive_a, s.drive_b] { + let _ = sqlx::query("DELETE FROM storage.role_grants WHERE resource_id = $1") + .bind(d) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.files WHERE drive_id = $1") + .bind(d) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM storage.drives WHERE id = $1") + .bind(d) + .execute(pool) + .await; + } + for f in [s.root_a, s.root_b] { + let _ = sqlx::query("DELETE FROM storage.folders WHERE id = $1") + .bind(f) + .execute(pool) + .await; + } + let _ = sqlx::query("DELETE FROM storage.blobs WHERE hash = $1") + .bind(&s.blob_hash) + .execute(pool) + .await; + let _ = sqlx::query("DELETE FROM auth.users WHERE id IN ($1, $2)") + .bind(s.caller) + .bind(s.other) + .execute(pool) + .await; +} + +fn fresh_engine(pool: &Arc) -> (Arc, Arc) { + let folder_repo = Arc::new(FolderDbRepository::new(pool.clone())); + let backend = Arc::new(LocalBlobBackend::new(std::path::Path::new( + "/tmp/bench-zipauthz-blobs", + ))); + let dedup = Arc::new(DedupService::new(backend, pool.clone(), pool.clone())); + let file_repo = Arc::new(FileBlobReadRepository::new( + pool.clone(), + dedup, + folder_repo.clone(), + )); + let group_repo = Arc::new(SubjectGroupPgRepository::new(pool.clone())); + let engine = Arc::new(PgAclEngine::new( + pool.clone(), + folder_repo, + file_repo.clone(), + group_repo, + )); + (engine, file_repo) +} + +/// BEFORE, verbatim: the per-file `require` filter, preserving input order. +async fn before_included(engine: &PgAclEngine, user: Uuid, sel: &[Uuid]) -> Vec { + let mut out = Vec::new(); + for id in sel { + if engine + .require(Subject::User(user), Permission::Read, Resource::File(*id)) + .await + .is_ok() + { + out.push(*id); + } + } + out +} + +/// AFTER: one batch check, then re-associate in input order (the download_zip +/// re-association). +async fn after_included(engine: &PgAclEngine, user: Uuid, sel: &[Uuid]) -> Vec { + let allowed: HashSet = engine + .check_files_read_batch(Subject::User(user), sel) + .await + .expect("batch check"); + sel.iter() + .copied() + .filter(|id| allowed.contains(id)) + .collect() +} + +#[tokio::main(flavor = "multi_thread")] +async fn main() { + dotenvy::dotenv().ok(); + let url = env::var("DATABASE_URL") + .or_else(|_| env::var("OXICLOUD_DB_CONNECTION_STRING")) + .expect("set DATABASE_URL — the dev Postgres URL"); + let n_files: usize = env_or("BENCH_FILES", 200); + let pool_size: u32 = env_or("BENCH_POOL", 20); + + let pool = Arc::new( + PgPoolOptions::new() + .max_connections(pool_size) + .min_connections(pool_size) + .acquire_timeout(Duration::from_secs(10)) + .connect(&url) + .await + .expect("connect Postgres"), + ); + + // Clear any prior fixtures, then seed. + let _ = sqlx::query( + "DELETE FROM auth.users WHERE email IN ('bench_zipauthz_a@bench.invalid','bench_zipauthz_b@bench.invalid')", + ) + .execute(pool.as_ref()) + .await; + let seeded = seed(&pool, n_files).await; + + // ── Equivalence gate (fresh engines so neither arm rides the other's cache) ── + let (eng_before, _) = fresh_engine(&pool); + let (eng_after, file_repo) = fresh_engine(&pool); + let before = before_included(&eng_before, seeded.caller, &seeded.selection).await; + let after = after_included(&eng_after, seeded.caller, &seeded.selection).await; + + let owned_set: HashSet = seeded.owned.iter().copied().collect(); + let denied_set: HashSet = seeded.denied.iter().copied().collect(); + let missing_set: HashSet = seeded.missing.iter().copied().collect(); + + let mut fail = false; + if before != after { + eprintln!("GATE FAIL: batch inclusion set/order != per-file require loop"); + fail = true; + } + // The included set must be EXACTLY the caller's owned files, in input order. + let expected: Vec = seeded + .selection + .iter() + .copied() + .filter(|id| owned_set.contains(id)) + .collect(); + if after != expected { + eprintln!("GATE FAIL: included set is not exactly the caller's owned files (in order)"); + fail = true; + } + if after.iter().any(|id| denied_set.contains(id)) { + eprintln!("GATE FAIL: a DENIED (other-drive) file was included — authz regression!"); + fail = true; + } + if after.iter().any(|id| missing_set.contains(id)) { + eprintln!("GATE FAIL: a MISSING id was included"); + fail = true; + } + // The batch fetch of the authorized ids must return exactly those files. + let allowed_ids: Vec = after.iter().map(Uuid::to_string).collect(); + let fetched = file_repo + .get_files_by_ids(&allowed_ids) + .await + .expect("batch fetch"); + let fetched_ids: HashSet = fetched + .iter() + .filter_map(|f| Uuid::parse_str(f.id()).ok()) + .collect(); + if fetched_ids != owned_set { + eprintln!("GATE FAIL: batch fetch of authorized ids != owned files"); + fail = true; + } + if fail { + cleanup(&pool, &seeded).await; + std::process::exit(1); + } + + println!("\n#################################################################"); + println!("# download_zip authz+metadata: per-file require loop vs batch"); + println!( + "# selection = {n} owned + {n} denied + {n} missing (interleaved)", + n = n_files + ); + println!("# gate OK: identical inclusion set+order; denied+missing excluded;"); + println!( + "# batch fetch returns exactly the {} owned files.", + seeded.owned.len() + ); + println!("#################################################################\n"); + println!("| {:<26} | {:>10} | {:>12} |", "arm", "wall ms", "µs/file"); + + // Latency: cold engine each run (empty caches — the first-download shape). + let total = seeded.selection.len(); + for (label, batch) in [ + ("per-file require loop", false), + ("batch check_files_read", true), + ] { + let (engine, _) = fresh_engine(&pool); + let t = Instant::now(); + let got = if batch { + after_included(&engine, seeded.caller, &seeded.selection).await + } else { + before_included(&engine, seeded.caller, &seeded.selection).await + }; + let el = t.elapsed(); + assert_eq!(got.len(), seeded.owned.len(), "arm {label} inclusion count"); + println!( + "| {:<26} | {:>10.2} | {:>12.2} |", + label, + el.as_secs_f64() * 1e3, + el.as_secs_f64() * 1e6 / total as f64 + ); + } + + cleanup(&pool, &seeded).await; + println!("\nAll Round-24 authz-equivalence gates passed."); +} diff --git a/src/application/services/batch_operations.rs b/src/application/services/batch_operations.rs index bd75d2c5..8f249b29 100644 --- a/src/application/services/batch_operations.rs +++ b/src/application/services/batch_operations.rs @@ -726,31 +726,46 @@ impl BatchOperationService { let mut items_added: usize = 0; // ── Add individual files at the root of the ZIP ────────────────── + // Authorize + fetch metadata for the whole multi-select in 2 round-trips + // (one batch Read check + one batch get) instead of the per-file + // `get_file_with_perms` N+1 (2 round-trips/file). The batch check also + // primes the resource→drive cache, so `add_file_entry_streamed`'s + // per-file stream-open re-check lands on the cache. A denied / missing / + // unparseable id is absent from the map → skipped in the same input + // order, exactly as the old per-file loop skipped it. Authorization is + // UNCHANGED — still enforced (pre-check here + the stream open's own + // Read check + Recents recording) before any ZIP entry is written, so a + // denied file never leaks its name into the archive (benches/ROUND24.md). + let authorized = self + .file_retrieval + .get_files_by_ids_with_perms(&file_ids, user_id) + .await + .map_err(BatchOperationError::Domain)?; + let by_id: HashMap = authorized + .into_iter() + .filter_map(|f| Uuid::parse_str(&f.id).ok().map(|u| (u, f))) + .collect(); for file_id in &file_ids { + let file_dto = match Uuid::parse_str(file_id).ok().and_then(|u| by_id.get(&u)) { + Some(f) => f, + None => { + info!("Skipping file {} (not accessible or missing)", file_id); + continue; + } + }; match self - .file_retrieval - .get_file_with_perms(file_id, user_id) + .add_file_entry_streamed( + &mut zip, + file_id, + &file_dto.name, + &file_dto.mime_type, + Some(user_id), + ) .await { - Ok(file_dto) => { - match self - .add_file_entry_streamed( - &mut zip, - file_id, - &file_dto.name, - &file_dto.mime_type, - Some(user_id), - ) - .await - { - Ok(_) => items_added += 1, - Err(e) => { - info!("Could not add file {} to ZIP: {}", file_dto.name, e); - } - } - } + Ok(_) => items_added += 1, Err(e) => { - info!("Could not get file metadata {}: {}", file_id, e); + info!("Could not add file {} to ZIP: {}", file_dto.name, e); } } } diff --git a/src/application/services/file_retrieval_service.rs b/src/application/services/file_retrieval_service.rs index 882ecb46..b0262e28 100644 --- a/src/application/services/file_retrieval_service.rs +++ b/src/application/services/file_retrieval_service.rs @@ -287,6 +287,52 @@ impl FileRetrievalService { Ok(files.into_iter().map(FileDto::from).collect()) } + /// Batched, authorized multi-get for the ZIP-download multi-select — the + /// batch form of [`FileRetrievalUseCase::get_file_with_perms`] over an + /// explicit id list. + /// + /// Authorizes `Read` on every id in ONE `check_files_read_batch` + /// round-trip (which resolves all drives in a single query AND primes the + /// resource→drive cache, so the per-file re-check the subsequent stream + /// open performs becomes a cache hit), then fetches only the authorized ids + /// in ONE `get_files_by_ids` query. Replaces `download_zip`'s per-file + /// `require_file` + `get_file` loop — 2 round-trips/file → 2 total. + /// + /// Returns the authorized, existing files; a denied / missing / unparseable + /// id is simply **absent** from the result (the caller re-associates by id + /// and skips the rest, exactly as the per-file loop skipped a denied / + /// missing `get_file_with_perms`). Read-authorization is identical to the + /// per-file path (`check_files_read_batch` is documented and gated as + /// semantically identical to looping `require`). Recents recording is left + /// to the subsequent per-file stream open (`get_file_stream_with_perms`), + /// which records it (throttle-coalesced) — same net effect as the old + /// loop's `notify_file_accessed` + stream double-notify. Fail-closed if no + /// engine was injected, mirroring [`Self::require_file`]. + pub async fn get_files_by_ids_with_perms( + &self, + ids: &[String], + caller_id: Uuid, + ) -> Result, DomainError> { + let authz = self.authz.as_ref().ok_or_else(|| { + DomainError::internal_error("FileRetrieval", "Authorization engine unavailable") + })?; + // Unparseable ids can't be authorized (the per-file path 404s on them), + // so drop them here — they stay absent from the authorized set. + let uuids: Vec = ids.iter().filter_map(|s| Uuid::parse_str(s).ok()).collect(); + if uuids.is_empty() { + return Ok(Vec::new()); + } + let allowed = authz + .check_files_read_batch(Subject::User(caller_id), &uuids) + .await?; + if allowed.is_empty() { + return Ok(Vec::new()); + } + let allowed_ids: Vec = allowed.iter().map(Uuid::to_string).collect(); + let files = self.file_read.get_files_by_ids(&allowed_ids).await?; + Ok(files.into_iter().map(FileDto::from).collect()) + } + /// Range read for HTTP Range Requests, cache-aware. /// /// Media players and PDF viewers fetch these files *exclusively* through