diff --git a/src/infrastructure/services/chunked_upload_service.rs b/src/infrastructure/services/chunked_upload_service.rs index 127f6584..4f007d81 100644 --- a/src/infrastructure/services/chunked_upload_service.rs +++ b/src/infrastructure/services/chunked_upload_service.rs @@ -770,8 +770,15 @@ impl ChunkedUploadService { } /// Cancel an upload and cleanup — disk I/O outside lock. - async fn cancel_upload_inner(&self, upload_id: &str, user_id: &str) -> Result<(), String> { - self.verify_session_owner(upload_id, user_id)?; + /// + /// Returns: + /// - `DomainError::NotFound` if no session matches `upload_id` for `user_id` + /// (covers both "session missing" and "owned by someone else" — same + /// error for anti-enumeration). + /// - `DomainError::InternalError` for unexpected disk I/O failures. + async fn cancel_upload_inner(&self, upload_id: &str, user_id: &str) -> Result<(), DomainError> { + self.verify_session_owner(upload_id, user_id) + .map_err(|_| DomainError::not_found("Upload", upload_id))?; // Remove from map (~µs) let removed = self.sessions.remove(upload_id).map(|(_, s)| s); @@ -861,9 +868,11 @@ impl ChunkedUploadPort for ChunkedUploadService { } async fn cancel_upload(&self, upload_id: &str, user_id: Uuid) -> Result<(), DomainError> { + // Inner function now returns DomainError with proper variants + // (NotFound for missing/wrong-owner sessions, InternalError otherwise), + // so no mapping needed here. self.cancel_upload_inner(upload_id, &user_id.to_string()) .await - .map_err(|e| DomainError::new(ErrorKind::InternalError, "ChunkedUpload", e)) } fn should_use_chunked(&self, size: u64) -> bool { diff --git a/src/interfaces/api/handlers/chunked_upload_handler.rs b/src/interfaces/api/handlers/chunked_upload_handler.rs index 8a5b8b44..10fbe900 100644 --- a/src/interfaces/api/handlers/chunked_upload_handler.rs +++ b/src/interfaces/api/handlers/chunked_upload_handler.rs @@ -21,8 +21,10 @@ use utoipa::ToSchema; use crate::application::ports::chunked_upload_ports::ChunkedUploadPort; use crate::application::ports::chunked_upload_ports::DEFAULT_CHUNK_SIZE; use crate::application::ports::file_ports::FileUploadUseCase; +use crate::application::ports::folder_ports::FolderUseCase; use crate::application::ports::storage_ports::StorageUsagePort; use crate::common::di::AppState; +use crate::domain::services::authorization::Permission; use crate::interfaces::errors::AppError; use crate::interfaces::middleware::auth::AuthUser; @@ -90,8 +92,6 @@ impl ChunkedUploadHandler { /// "expires_at": 86400 /// } /// ``` - /// TODO: how is implemented security (owneship, permission ?) - /// current caveat: upload can start without know is path permits upload pub(super) async fn create_upload_impl( State(state): State>, auth_user: AuthUser, @@ -120,6 +120,27 @@ impl ChunkedUploadHandler { .into_response(); } + // ── Permission pre-check: caller must have Create on the target + // folder BEFORE we allocate a session and accept chunks. The + // upload service re-checks at finalize time, but failing here + // avoids wasting client+server resources on chunks that will be + // rejected. None = caller's root namespace, no check needed. + if let Some(ref fid) = request.folder_id + && let Err(err) = state + .applications + .folder_service_concrete + .has_permission(auth_user.id, Permission::Create, fid) + .await + { + tracing::warn!( + "⛔ CHUNKED UPLOAD REJECTED (no perm): user='{}' folder='{}' err='{}'", + auth_user.username, + fid, + err + ); + return AppError::from(err).into_response(); + } + // ── Quota enforcement ──────────────────────────────────── if let Some(storage_svc) = state.storage_usage_service.as_ref() && let Err(err) = storage_svc @@ -352,9 +373,7 @@ impl ChunkedUploadHandler { .await { Ok(_) => StatusCode::NO_CONTENT.into_response(), - Err(e) => { - AppError::internal_error(format!("Failed to cancel upload: {}", e)).into_response() - } + Err(e) => AppError::from(e).into_response(), } } } diff --git a/tests/api/grants.hurl b/tests/api/grants.hurl index 4bd1042c..adf09960 100644 --- a/tests/api/grants.hurl +++ b/tests/api/grants.hurl @@ -475,6 +475,21 @@ Authorization: Bearer {{adam_token}} HTTP 404 +# ── Chunked upload: cannot start session in alice's folder ── +# create_upload_impl pre-checks Permission::Create via has_permission. +POST {{base_url}}/api/uploads +Authorization: Bearer {{adam_token}} +Content-Type: application/json +{ + "filename": "adam-chunked-attack.mp4", + "folder_id": "{{perm_folder_id}}", + "content_type": "video/mp4", + "total_size": 2760653, + "chunk_size": 3000000 +} + +HTTP 404 + # ════════════════════════════════════════════════════════════════════ # Phase 2B — Alice grants adam Viewer. Read OK, mutate/delete denied. @@ -594,6 +609,20 @@ Authorization: Bearer {{adam_token}} HTTP 404 +# ── Viewer cannot start a chunked upload (no Create grant) ── +POST {{base_url}}/api/uploads +Authorization: Bearer {{adam_token}} +Content-Type: application/json +{ + "filename": "viewer-chunked-attempt.mp4", + "folder_id": "{{perm_folder_id}}", + "content_type": "video/mp4", + "total_size": 2760653, + "chunk_size": 3000000 +} + +HTTP 404 + # ════════════════════════════════════════════════════════════════════ # Phase 2C — Promote adam to Editor (read + comment + create + update). @@ -649,6 +678,80 @@ file: file,fixtures/hello.txt; text/plain HTTP 201 +# ── Chunked upload full lifecycle as Editor ───────────────── +# 1. Open session (server pre-checks Create on folder) +POST {{base_url}}/api/uploads +Authorization: Bearer {{adam_token}} +Content-Type: application/json +{ + "filename": "adam-chunked-video.mp4", + "folder_id": "{{perm_folder_id}}", + "content_type": "video/mp4", + "total_size": 2760653, + "chunk_size": 3000000 +} + +HTTP 201 +[Captures] +adam_upload_id: jsonpath "$.upload_id" + +# 2. Send the single chunk (chunk_size > total_size → 1 chunk). +PATCH {{base_url}}/api/uploads/{{adam_upload_id}}?chunk_index=0 +Authorization: Bearer {{adam_token}} +Content-Type: application/octet-stream +file,fixtures/free_video_over_1MB.mp4; + +HTTP 200 + +# 3. Status query: dave (different user) cannot peek at adam's session. +HEAD {{base_url}}/api/uploads/{{adam_upload_id}} +Authorization: Bearer {{dave_token}} + +HTTP 404 + +# 4. Cancel attempt by a different user is rejected. +DELETE {{base_url}}/api/uploads/{{adam_upload_id}} +Authorization: Bearer {{dave_token}} + +HTTP 404 + +# 5. Adam completes the upload — file is created in alice's folder. +POST {{base_url}}/api/uploads/{{adam_upload_id}}/complete +Authorization: Bearer {{adam_token}} + +HTTP 201 +[Captures] +adam_chunked_file_id: jsonpath "$.file_id" + +# 6. The new file is visible in the folder listing (caller-of-listing is alice). +GET {{base_url}}/api/files?folder_id={{perm_folder_id}} +Authorization: Bearer {{alice_token}} + +HTTP 200 +[Asserts] +jsonpath "$[?(@.id=='{{adam_chunked_file_id}}')].name" == "adam-chunked-video.mp4" + +# 7. A second session that adam cancels before completing — cleanup path. +POST {{base_url}}/api/uploads +Authorization: Bearer {{adam_token}} +Content-Type: application/json +{ + "filename": "adam-cancelled.mp4", + "folder_id": "{{perm_folder_id}}", + "content_type": "video/mp4", + "total_size": 2760653, + "chunk_size": 3000000 +} + +HTTP 201 +[Captures] +adam_cancel_id: jsonpath "$.upload_id" + +DELETE {{base_url}}/api/uploads/{{adam_cancel_id}} +Authorization: Bearer {{adam_token}} + +HTTP 204 + # ── Delete still denied (Editor excludes Delete) ──────────── DELETE {{base_url}}/api/files/{{perm_file_id}} Authorization: Bearer {{adam_token}}