feat(authz): test & cover batch cases
┌────────────────────────────────┬─────────────────────────────────┬───────────────────────┬─────────────────┬──────────────────────────────┐ │ Endpoint │ Phase 3A no-grant │ Phase 3B Viewer │ Phase 3C Editor │ Phase 3D Admin │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/files/get │ 400 (all failed) │ 200 (2 successful) │ — │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/files/move │ 400 │ 400 (no Update) │ 200 │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/files/copy │ 400 │ — │ 200 │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/files/delete │ 400 │ 400 │ 400 (no Delete) │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/folders/get │ 400 │ 200 │ — │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/folders/create │ 400 │ — │ 201 │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/folders/move │ 400 │ — │ 200 │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/folders/copy │ 400 │ — │ 200 │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/folders/delete │ 400 │ — │ 400 (no Delete) │ 200 │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/trash │ 400 │ — │ — │ 400 (owner-only, documented) │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ POST /api/batch/download │ 404 (NotFound) │ 200 + application/zip │ — │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ GET /api/batch/download?... │ 404 │ 200 + zip │ — │ — │ ├────────────────────────────────┼─────────────────────────────────┼───────────────────────┼─────────────────┼──────────────────────────────┤ │ Phase 3E lifecycle cleanup │ grants table empty after delete │ │ │ │ └────────────────────────────────┴─────────────────────────────────┴───────────────────────┴─────────────────┴──────────────────────────────┘
This commit is contained in:
@@ -715,6 +715,11 @@ impl BatchOperationService {
|
||||
let buf_writer = BufWriter::with_capacity(256 * 1024, tokio_file);
|
||||
let mut zip = ZipFileWriter::with_tokio(buf_writer);
|
||||
|
||||
// Track whether any item was authorized + added to the ZIP. If
|
||||
// none were, return NotFound — empty ZIPs are useless and mask
|
||||
// authz failures from the client.
|
||||
let mut items_added: usize = 0;
|
||||
|
||||
// ── Add individual files at the root of the ZIP ──────────────────
|
||||
for file_id in &file_ids {
|
||||
match self
|
||||
@@ -723,11 +728,14 @@ impl BatchOperationService {
|
||||
.await
|
||||
{
|
||||
Ok(file_dto) => {
|
||||
if let Err(e) = self
|
||||
match self
|
||||
.add_file_entry_streamed(&mut zip, file_id, &file_dto.name, user_id)
|
||||
.await
|
||||
{
|
||||
info!("Could not add file {} to ZIP: {}", file_dto.name, e);
|
||||
Ok(_) => items_added += 1,
|
||||
Err(e) => {
|
||||
info!("Could not add file {} to ZIP: {}", file_dto.name, e);
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -744,11 +752,14 @@ impl BatchOperationService {
|
||||
.await
|
||||
{
|
||||
Ok(root_folder) => {
|
||||
if let Err(e) = self
|
||||
match self
|
||||
.add_folder_subtree_to_zip(&mut zip, folder_id, &root_folder, user_id)
|
||||
.await
|
||||
{
|
||||
info!("Could not add folder {} to ZIP: {}", root_folder.name, e);
|
||||
Ok(_) => items_added += 1,
|
||||
Err(e) => {
|
||||
info!("Could not add folder {} to ZIP: {}", root_folder.name, e);
|
||||
}
|
||||
}
|
||||
}
|
||||
Err(e) => {
|
||||
@@ -757,6 +768,14 @@ impl BatchOperationService {
|
||||
}
|
||||
}
|
||||
|
||||
// Bail out before finalizing the ZIP if nothing was authorized.
|
||||
if items_added == 0 {
|
||||
return Err(BatchOperationError::Domain(DomainError::not_found(
|
||||
"BatchDownload",
|
||||
"No accessible files or folders in the request",
|
||||
)));
|
||||
}
|
||||
|
||||
// ── Finalize ─────────────────────────────────────────────────────
|
||||
let mut compat_writer = zip
|
||||
.close()
|
||||
|
||||
@@ -15,6 +15,7 @@ use crate::application::services::batch_operations::{
|
||||
};
|
||||
use crate::interfaces::api::deserializer;
|
||||
use crate::interfaces::api::handlers::ApiResult;
|
||||
use crate::interfaces::errors::AppError;
|
||||
use crate::interfaces::middleware::auth::AuthUser;
|
||||
|
||||
/// Maximum number of items allowed in a single batch request.
|
||||
@@ -1010,10 +1011,19 @@ async fn process_download_batch(
|
||||
.await
|
||||
.map_err(|e| {
|
||||
tracing::error!("Batch download ZIP failed: {}", e);
|
||||
(
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
"Batch download failed".to_string(),
|
||||
)
|
||||
// Surface DomainError variants (NotFound when no items were
|
||||
// authorized) with their natural HTTP status code instead of
|
||||
// collapsing everything to 500.
|
||||
match e {
|
||||
crate::application::services::batch_operations::BatchOperationError::Domain(de) => {
|
||||
let app: AppError = de.into();
|
||||
(app.status_code, app.message)
|
||||
}
|
||||
other => (
|
||||
StatusCode::INTERNAL_SERVER_ERROR,
|
||||
format!("Batch download failed: {}", other),
|
||||
),
|
||||
}
|
||||
})?;
|
||||
|
||||
// Read file size for Content-Length before splitting ownership
|
||||
|
||||
Reference in New Issue
Block a user