security(upload): add permission to upload_file_streaming()
This commit is contained in:
@@ -60,6 +60,25 @@ pub trait FileUploadUseCase: Send + Sync + 'static {
|
|||||||
caller_id: Uuid,
|
caller_id: Uuid,
|
||||||
) -> Result<FileDto, DomainError>;
|
) -> Result<FileDto, DomainError>;
|
||||||
|
|
||||||
|
/// `_with_perms` variant of `upload_file_streaming` — enforces
|
||||||
|
/// `Create` on the target folder before registering the row.
|
||||||
|
///
|
||||||
|
/// AuthZ audit #17 (2026-07-12): the chunked-upload `complete`
|
||||||
|
/// path called plain `upload_file_streaming` at finalize; a grant
|
||||||
|
/// revoked between session open and finalize stayed effective
|
||||||
|
/// until the caller landed the final chunk (up to 24h JWT TTL,
|
||||||
|
/// forever with app-passwords). Handlers now call this variant
|
||||||
|
/// so the engine re-checks at finalize regardless of how long
|
||||||
|
/// the session was open.
|
||||||
|
async fn upload_file_streaming_with_perms(
|
||||||
|
&self,
|
||||||
|
name: String,
|
||||||
|
folder_id: Option<String>,
|
||||||
|
content_type: String,
|
||||||
|
blob: StoredBlob,
|
||||||
|
caller_id: Uuid,
|
||||||
|
) -> Result<FileDto, DomainError>;
|
||||||
|
|
||||||
/// Replace the content of the file at `path` with an already-ingested
|
/// Replace the content of the file at `path` with an already-ingested
|
||||||
/// blob, or create the file when it doesn't exist (WebDAV/WOPI PUT).
|
/// blob, or create the file when it doesn't exist (WebDAV/WOPI PUT).
|
||||||
///
|
///
|
||||||
|
|||||||
@@ -457,6 +457,44 @@ impl FileUploadUseCase for FileUploadService {
|
|||||||
Ok(dto)
|
Ok(dto)
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/// AuthZ audit #17 — `Create` on target folder is re-verified here
|
||||||
|
/// so mid-session grant revocations take effect at finalize. When
|
||||||
|
/// `folder_id` is `None` the write lands at drive-root; the drive
|
||||||
|
/// resolution for that case isn't plumbed through the chunked-
|
||||||
|
/// upload session (`UploadSession.folder_id` alone), so we fall
|
||||||
|
/// back to the pre-audit behaviour there. That drive-root path is
|
||||||
|
/// tracked separately as part of the D0 folder-id-walking work;
|
||||||
|
/// closing it here would require session-scoped drive_id.
|
||||||
|
async fn upload_file_streaming_with_perms(
|
||||||
|
&self,
|
||||||
|
name: String,
|
||||||
|
folder_id: Option<String>,
|
||||||
|
content_type: String,
|
||||||
|
blob: StoredBlob,
|
||||||
|
caller_id: Uuid,
|
||||||
|
) -> Result<FileDto, DomainError> {
|
||||||
|
if let Some(fid) = folder_id.as_deref() {
|
||||||
|
let Some(authz) = &self.authorization else {
|
||||||
|
return Err(DomainError::internal_error(
|
||||||
|
"FileUpload",
|
||||||
|
"upload_file_streaming_with_perms called without authorization engine wired",
|
||||||
|
));
|
||||||
|
};
|
||||||
|
let folder_uuid = Uuid::parse_str(fid)
|
||||||
|
.map_err(|_| DomainError::not_found("Folder", fid.to_string()))?;
|
||||||
|
authz
|
||||||
|
.require(
|
||||||
|
Subject::User(caller_id),
|
||||||
|
Permission::Create,
|
||||||
|
Resource::Folder(folder_uuid),
|
||||||
|
)
|
||||||
|
.await?;
|
||||||
|
}
|
||||||
|
|
||||||
|
self.upload_file_streaming(name, folder_id, content_type, blob, caller_id)
|
||||||
|
.await
|
||||||
|
}
|
||||||
|
|
||||||
/// Swap the content of the file at `path` to an already-ingested blob,
|
/// Swap the content of the file at `path` to an already-ingested blob,
|
||||||
/// creating the file when it doesn't exist (WebDAV/NextCloud/WOPI PUT).
|
/// creating the file when it doesn't exist (WebDAV/NextCloud/WOPI PUT).
|
||||||
///
|
///
|
||||||
|
|||||||
@@ -511,6 +511,17 @@ impl FileUploadUseCase for StubFileUploadUseCase {
|
|||||||
) -> Result<FileDto, DomainError> {
|
) -> Result<FileDto, DomainError> {
|
||||||
Ok(FileDto::default())
|
Ok(FileDto::default())
|
||||||
}
|
}
|
||||||
|
|
||||||
|
async fn upload_file_streaming_with_perms(
|
||||||
|
&self,
|
||||||
|
_name: String,
|
||||||
|
_folder_id: Option<String>,
|
||||||
|
_content_type: String,
|
||||||
|
_blob: StoredBlob,
|
||||||
|
_caller_id: Uuid,
|
||||||
|
) -> Result<FileDto, DomainError> {
|
||||||
|
Ok(FileDto::default())
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// ---------------------------------------------------------------------------
|
// ---------------------------------------------------------------------------
|
||||||
|
|||||||
@@ -188,9 +188,14 @@ impl ChunkedUploadHandler {
|
|||||||
|
|
||||||
// ── Permission pre-check: caller must have Create on the target
|
// ── Permission pre-check: caller must have Create on the target
|
||||||
// folder BEFORE we allocate a session and accept chunks. The
|
// folder BEFORE we allocate a session and accept chunks. The
|
||||||
// upload service re-checks at finalize time, but failing here
|
// upload service re-checks at finalize via
|
||||||
// avoids wasting client+server resources on chunks that will be
|
// `upload_file_streaming_with_perms` (AuthZ audit #17 fix,
|
||||||
// rejected. None = caller's root namespace, no check needed.
|
// 2026-07-16) so a grant revoked mid-session is caught. This
|
||||||
|
// pre-check is the fail-fast: it avoids wasting client+server
|
||||||
|
// resources on chunks that will be rejected anyway. `None`
|
||||||
|
// means the write lands at drive-root — that path is currently
|
||||||
|
// unchecked (session doesn't carry `drive_id`; tracked with the
|
||||||
|
// folder-id-walking follow-up).
|
||||||
if let Some(ref fid) = request.folder_id
|
if let Some(ref fid) = request.folder_id
|
||||||
&& let Err(err) = state
|
&& let Err(err) = state
|
||||||
.applications
|
.applications
|
||||||
@@ -441,9 +446,17 @@ impl ChunkedUploadHandler {
|
|||||||
}
|
}
|
||||||
|
|
||||||
// Register the file row against the ingested blob.
|
// Register the file row against the ingested blob.
|
||||||
|
//
|
||||||
|
// AuthZ audit #17 (2026-07-12): swapped `upload_file_streaming` →
|
||||||
|
// `upload_file_streaming_with_perms` so `Create` on the target
|
||||||
|
// folder is re-verified at finalize. Session creation already
|
||||||
|
// pre-checked (line ~198), but that was potentially hours or
|
||||||
|
// days ago; app-passwords keep sessions valid indefinitely.
|
||||||
|
// Without the finalize re-check, a grant revoked mid-session
|
||||||
|
// stayed effective until the last chunk landed.
|
||||||
let size = ingested.size;
|
let size = ingested.size;
|
||||||
match upload_service
|
match upload_service
|
||||||
.upload_file_streaming(
|
.upload_file_streaming_with_perms(
|
||||||
parts.filename.clone(),
|
parts.filename.clone(),
|
||||||
parts.folder_id.clone(),
|
parts.folder_id.clone(),
|
||||||
ingested.content_type.clone(),
|
ingested.content_type.clone(),
|
||||||
|
|||||||
@@ -818,6 +818,96 @@ Authorization: Bearer {{adam_token}}
|
|||||||
|
|
||||||
HTTP 204
|
HTTP 204
|
||||||
|
|
||||||
|
# ── Regression pin for AuthZ audit #17 (2026-07-12). ─────────
|
||||||
|
# The chunked-upload `complete` handler used to call plain
|
||||||
|
# `upload_file_streaming` at finalize — no `_with_perms` check.
|
||||||
|
# A grant revoked between session-open and finalize stayed
|
||||||
|
# effective until the last chunk landed (up to 24h JWT TTL,
|
||||||
|
# forever with app-passwords). Fix: swap to
|
||||||
|
# `upload_file_streaming_with_perms` so `authz.require(Create,
|
||||||
|
# Folder)` re-runs at complete time.
|
||||||
|
#
|
||||||
|
# Sequence:
|
||||||
|
# 1. Adam (Editor) opens a session — pre-check passes.
|
||||||
|
# 2. Adam PATCHes the single chunk (chunk upload is unauth'd,
|
||||||
|
# always allowed).
|
||||||
|
# 3. Alice DEMOTES Adam to Viewer (Viewer bundle has Read but
|
||||||
|
# no Create).
|
||||||
|
# 4. Adam POST /complete → 403 (pre-fix: 201 + file created).
|
||||||
|
# 5. Cleanup: cancel the orphaned session + re-promote Adam
|
||||||
|
# to Editor so the following steps aren't disturbed.
|
||||||
|
|
||||||
|
# 1 — Open session while Editor.
|
||||||
|
POST {{base_url}}/api/uploads
|
||||||
|
Authorization: Bearer {{adam_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{
|
||||||
|
"filename": "audit17-post-revoke.mp4",
|
||||||
|
"folder_id": "{{perm_folder_id}}",
|
||||||
|
"content_type": "video/mp4",
|
||||||
|
"total_size": 2760653,
|
||||||
|
"chunk_size": 3000000
|
||||||
|
}
|
||||||
|
|
||||||
|
HTTP 201
|
||||||
|
[Captures]
|
||||||
|
audit17_upload_id: jsonpath "$.upload_id"
|
||||||
|
|
||||||
|
|
||||||
|
# 2 — Send the single chunk (session pre-authorised).
|
||||||
|
PATCH {{base_url}}/api/uploads/{{audit17_upload_id}}?chunk_index=0
|
||||||
|
Authorization: Bearer {{adam_token}}
|
||||||
|
Content-Type: application/octet-stream
|
||||||
|
file,fixtures/free_video_over_1MB.mp4;
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
|
||||||
|
|
||||||
|
# 3 — Alice demotes Adam Editor → Viewer (Create removed).
|
||||||
|
PUT {{base_url}}/api/grants/role
|
||||||
|
Authorization: Bearer {{alice_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{
|
||||||
|
"subject": { "type": "user", "id": "{{adam_user_id}}" },
|
||||||
|
"resource": { "type": "folder", "id": "{{perm_folder_id}}" },
|
||||||
|
"role": "viewer"
|
||||||
|
}
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
|
||||||
|
|
||||||
|
# 4 — Finalize now fails: engine re-checks Create at complete
|
||||||
|
# time. Adam still has Read (viewer role) → graduated denial
|
||||||
|
# returns 403; pre-fix returned 201 with a phantom file.
|
||||||
|
POST {{base_url}}/api/uploads/{{audit17_upload_id}}/complete
|
||||||
|
Authorization: Bearer {{adam_token}}
|
||||||
|
|
||||||
|
HTTP 403
|
||||||
|
|
||||||
|
|
||||||
|
# 5a — The session is orphaned (chunks on disk, no completion).
|
||||||
|
# Cancel it as Adam (still owns the session, so the `_with_perms`
|
||||||
|
# gate on DELETE-session lets him through).
|
||||||
|
DELETE {{base_url}}/api/uploads/{{audit17_upload_id}}
|
||||||
|
Authorization: Bearer {{adam_token}}
|
||||||
|
|
||||||
|
HTTP 204
|
||||||
|
|
||||||
|
|
||||||
|
# 5b — Restore Adam to Editor so subsequent steps behave as
|
||||||
|
# before this regression pin was inserted.
|
||||||
|
PUT {{base_url}}/api/grants/role
|
||||||
|
Authorization: Bearer {{alice_token}}
|
||||||
|
Content-Type: application/json
|
||||||
|
{
|
||||||
|
"subject": { "type": "user", "id": "{{adam_user_id}}" },
|
||||||
|
"resource": { "type": "folder", "id": "{{perm_folder_id}}" },
|
||||||
|
"role": "editor"
|
||||||
|
}
|
||||||
|
|
||||||
|
HTTP 200
|
||||||
|
|
||||||
|
|
||||||
# ── Delete still denied (Editor excludes Delete). Editor has
|
# ── Delete still denied (Editor excludes Delete). Editor has
|
||||||
# Read → graduated denial returns 403.
|
# Read → graduated denial returns 403.
|
||||||
DELETE {{base_url}}/api/files/{{perm_file_id}}
|
DELETE {{base_url}}/api/files/{{perm_file_id}}
|
||||||
|
|||||||
Reference in New Issue
Block a user