security(nextcloud): add authz to PUT verb

This commit is contained in:
Edouard Vanbelle
2026-07-04 23:57:06 +02:00
parent b95e740b2f
commit 0342bae300
7 changed files with 107 additions and 14 deletions
+6 -1
View File
@@ -75,7 +75,12 @@ pub trait FileUploadUseCase: Send + Sync + 'static {
/// `updated_by` column reflects the principal that performed the
/// PUT — not the file's existing owner (D2 shared drives let
/// non-owners overwrite content).
async fn update_file_streaming(
/// `_with_perms` suffix (AGENTS.md AuthZ convention): the
/// implementation calls `authz.require(caller, Update, File(id))`
/// on the overwrite branch and `authz.require(caller, Create,
/// Folder|Drive(id))` on the new-file branch. Handlers just plumb
/// `caller_id` through — no protocol-layer authz.
async fn update_file_streaming_with_perms(
&self,
path: &str,
drive_id: Uuid,
@@ -42,6 +42,16 @@ pub struct FileUploadService {
/// `(file_id, blob_hash, content_type)`; the recording side needs the
/// `caller_id` the service already has in hand.
resource_access_hook: Option<Arc<dyn ResourceAccessHook>>,
/// ReBAC engine — enforces `Permission::Update` on
/// overwrite-existing and `Permission::Create` on new-file paths
/// inside `update_file_streaming_with_perms`. Optional at the
/// struct level for the minimal test constructors (`new`,
/// `new_with_read`) but the WebDAV/NC/WOPI put paths refuse
/// (fail-closed internal error) if this isn't wired. Set by
/// either `with_instant_upload` or `with_authorization` — both
/// stash the same Arc so DI callers wiring instant upload get
/// the streaming gate for free.
authorization: Option<Arc<PgAclEngine>>,
/// Dependencies of the instant-upload path
/// (`create_file_from_owned_blob_with_perms`); `None` in minimal test
/// wiring.
@@ -66,6 +76,7 @@ impl FileUploadService {
content_cache: None,
file_lifecycle_hook: None,
resource_access_hook: None,
authorization: None,
instant_upload: None,
}
}
@@ -82,18 +93,34 @@ impl FileUploadService {
content_cache: None,
file_lifecycle_hook: None,
resource_access_hook: None,
authorization: None,
instant_upload: None,
}
}
/// Wires the authorization engine used by
/// `update_file_streaming_with_perms` on the WebDAV / NC / WOPI
/// PUT path. Independent of `with_instant_upload` so callers can
/// enable the streaming gate without also opting into the
/// dedup-instant-upload check (test wiring, minimal deployments).
pub fn with_authorization(mut self, authz: Arc<PgAclEngine>) -> Self {
self.authorization = Some(authz);
self
}
/// Wires the authorization engine, dedup index and quota service that
/// power the instant-upload path.
///
/// Also stashes the `authz` handle in `self.authorization` so
/// DI callers wiring instant upload get the streaming-put gate
/// for free — a single `Arc` clone, no behavioural coupling.
pub fn with_instant_upload(
mut self,
authz: Arc<PgAclEngine>,
dedup: Arc<DedupService>,
quota: Arc<StorageUsageService>,
) -> Self {
self.authorization = Some(authz.clone());
self.instant_upload = Some(InstantUploadDeps {
authz,
dedup,
@@ -424,7 +451,16 @@ impl FileUploadUseCase for FileUploadService {
/// 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).
async fn update_file_streaming(
///
/// AuthZ (post-Drive audit Round 2 fix): overwrite path requires
/// `Update` on the target file; new-file path requires `Create`
/// on the parent folder (or on the drive when writing at drive
/// root). Fail-closed if the engine wasn't wired — this method
/// is the last line of defence between a Viewer/Commenter drive
/// member and cross-tenant PUT. See
/// `docs/plan/authz_audit/nextcloud.md` and the sibling native
/// `/webdav/*` handler.
async fn update_file_streaming_with_perms(
&self,
path: &str,
drive_id: Uuid,
@@ -433,10 +469,33 @@ impl FileUploadUseCase for FileUploadService {
modified_at: Option<i64>,
caller_id: Uuid,
) -> Result<FileDto, DomainError> {
let Some(authz) = &self.authorization else {
return Err(DomainError::internal_error(
"FileUpload",
"update_file_streaming_with_perms called without authorization engine wired",
));
};
// Try to find the existing file first
if let Some(file_read) = &self.file_read
&& let Some(file) = file_read.find_file_by_path(path, drive_id).await?
{
// Overwrite branch — caller must have `Update` on the
// target file. Denial routes through `require` → 404
// (anti-enum, matches read-side shape). Before the D7
// audit this whole branch ran unchecked; Viewer members
// of shared drives could PUT freely.
let file_uuid = Uuid::parse_str(file.id()).map_err(|_| {
DomainError::internal_error("FileUpload", "invalid file id from repository")
})?;
authz
.require(
Subject::User(caller_id),
Permission::Update,
Resource::File(file_uuid),
)
.await?;
let file_id = file.id().to_string();
let (new_hash, updated_at) = self
.file_write
@@ -505,6 +564,32 @@ impl FileUploadUseCase for FileUploadService {
None
};
// Create branch — caller must have `Create` on the parent
// scope. Two cases:
// * `parent_id.is_some()` → caller needs Create on the
// parent Folder resource.
// * `parent_id.is_none()` → the write lands at the drive
// root (either the path was single-segment, or the
// parent-folder lookup failed). We require Create on
// the Drive itself — bundled with owner/editor/contributor
// role_grants, refused for viewer/commenter.
let create_resource = match &parent_id {
Some(pid) => {
let uuid = Uuid::parse_str(pid).map_err(|_| {
DomainError::internal_error("FileUpload", "invalid parent folder id")
})?;
Resource::Folder(uuid)
}
None => Resource::Drive(drive_id),
};
authz
.require(
Subject::User(caller_id),
Permission::Create,
create_resource,
)
.await?;
let is_new_blob = blob.is_new_blob;
let created = self
.file_write
+1 -1
View File
@@ -500,7 +500,7 @@ impl FileUploadUseCase for StubFileUploadUseCase {
Ok(FileDto::default())
}
async fn update_file_streaming(
async fn update_file_streaming_with_perms(
&self,
_path: &str,
_drive_id: Uuid,
@@ -1756,7 +1756,7 @@ async fn handle_put(
// internally via its `_with_perms` shape.
let content_type = ingested.content_type.clone();
let result = file_upload_service
.update_file_streaming(
.update_file_streaming_with_perms(
&path,
drive_id,
ingested.stored(),
+1 -1
View File
@@ -264,7 +264,7 @@ async fn put_file(
.app_state
.applications
.file_upload_service
.update_file_streaming(
.update_file_streaming_with_perms(
&file.path,
drive_id,
ingested.stored(),
+11 -8
View File
@@ -367,12 +367,14 @@ async fn handle_assemble(
let chroot = session.require_chroot()?;
let drive_id = chroot.drive_id;
// TODO(D1): read the caller's default-drive root folder name from
// `drives.root_folder_id` instead of hardcoding "Personal". The
// constant is correct for every default personal drive provisioned
// by the D0 lifecycle hook, but secondary drives (M2 backfill from
// SQL-created sibling root folders) keep their original name.
let internal_path = format!("Personal/{}", dest_subpath.trim_matches('/'));
// Route through `nc_to_internal_path(chroot, …)` so the write
// lands under the caller's actual default-drive root (not the
// literal "Personal" folder). Post-D3 chroot resolution puts the
// correct FolderDto — including the drive's real root name — on
// the NcSession; secondary drives with SQL-provisioned sibling
// root names now work.
let internal_path =
crate::interfaces::nextcloud::webdav_handler::nc_to_internal_path(chroot, &dest_subpath)?;
let filename = filename_from_path(&dest_subpath).to_string();
let ingested = ingest_stream_to_cas(
@@ -393,7 +395,7 @@ async fn handle_assemble(
let etag: Option<String> = if existing.is_ok() {
let dto = upload_service
.update_file_streaming(
.update_file_streaming_with_perms(
&internal_path,
drive_id,
ingested.stored(),
@@ -412,7 +414,8 @@ async fn handle_assemble(
Some((p, n)) => (p, n),
None => ("", dest_subpath.as_str()),
};
let parent_internal = format!("Personal/{}", parent_sub.trim_matches('/'));
let parent_internal =
crate::interfaces::nextcloud::webdav_handler::nc_to_internal_path(chroot, parent_sub)?;
let parent_internal = parent_internal.trim_end_matches('/');
use crate::application::ports::folder_ports::FolderUseCase;
+1 -1
View File
@@ -871,7 +871,7 @@ async fn handle_put(
// Single streaming path — handles both update and create internally,
// swapping the file row onto the already-ingested blob.
let stored = upload_service
.update_file_streaming(
.update_file_streaming_with_perms(
&internal_path,
chroot.drive_id,
ingested.stored(),