diff --git a/src/interfaces/nextcloud/webdav_handler.rs b/src/interfaces/nextcloud/webdav_handler.rs index ca7f0ff8..b571cf6f 100644 --- a/src/interfaces/nextcloud/webdav_handler.rs +++ b/src/interfaces/nextcloud/webdav_handler.rs @@ -80,15 +80,24 @@ const HEADER_DAV: HeaderName = HeaderName::from_static("dav"); /// Replaces the pre-D0 hardcoded `"My Folder - {username}/"` prefix. pub fn nc_to_internal_path(chroot: &FolderDto, subpath: &str) -> Result { let subpath = subpath.trim_matches('/'); + // `chroot.path` comes from `Folder::path_string()` / + // `StoragePath::to_string()`, which prepends a leading `/` (e.g. + // `"/Personal"`) — trim it so the result matches the leading- + // slash-free convention `storage.folders.path` (and the plain + // WebDAV surface's `db_path`) actually use. Without this, exact- + // string comparisons against a plain-surface path (e.g. the + // in-memory WebDAV lock store's key) silently mismatch even + // though DB-backed lookups tolerate the discrepancy. + let chroot_path = chroot.path.trim_start_matches('/'); if subpath.is_empty() { - return Ok(chroot.path.clone()); + return Ok(chroot_path.to_string()); } // Reject path traversal attempts. if subpath.split('/').any(|seg| seg == ".." || seg == ".") { return Err(AppError::bad_request("Invalid path: traversal not allowed")); } - Ok(format!("{}/{}", chroot.path, subpath)) + Ok(format!("{}/{}", chroot_path, subpath)) } /// Strip the caller's chroot prefix from an internal @@ -1167,7 +1176,7 @@ async fn handle_patch( session.user.id, ) .await - .map_err(|e| AppError::internal_error(format!("Failed to store file: {}", e)))?; + .map_err(AppError::from)?; // Everything from `start` to the new EOF reflects the patch (the // untouched suffix, if any, may have shifted when the body's length @@ -2316,6 +2325,23 @@ mod tests { ); } + /// Regression: `chroot.path` as returned by `folder_service.get_folder` + /// in production carries a leading `/` (from `StoragePath::to_string()` + /// — see `Folder::path_string`), unlike this module's `stub_folder` + /// test helper which builds the path directly. A real chroot must + /// still map to the leading-slash-free convention the plain WebDAV + /// surface's `db_path` uses, or exact-string comparisons against it + /// (e.g. the WebDAV lock store's key) silently mismatch. + #[test] + fn test_strips_leading_slash_from_chroot_path() { + let home = stub_folder("/Personal"); + assert_eq!( + nc_to_internal_path(&home, "report.pdf").unwrap(), + "Personal/report.pdf" + ); + assert_eq!(nc_to_internal_path(&home, "").unwrap(), "Personal"); + } + #[test] fn test_rejects_dot_dot_traversal() { let home = stub_folder("My Folder - alice"); diff --git a/tests/api/nc_webdav_patch.hurl b/tests/api/nc_webdav_patch.hurl index 53d50b24..d916e61e 100644 --- a/tests/api/nc_webdav_patch.hurl +++ b/tests/api/nc_webdav_patch.hurl @@ -16,6 +16,16 @@ # 3. PATCH with a Content-Range header → 400 (must use X-Update-Range). # 4. PATCH without X-Update-Range → 400. # 5. PATCH on a nonexistent file → 404. +# 6. PATCH on a directory → 409 (not 404 — the NC surface previously +# had no folder-existence check and returned 404 for both a missing +# file AND an existing directory; the fix commit added an explicit +# check so the two cases are distinguishable again, matching the +# plain-surface behavior). +# +# Hurl gotcha: headers MUST come before section blocks like +# `[BasicAuth]` in a request — a header line placed after `[BasicAuth]` +# is parsed as the START OF A NEW REQUEST instead (see +# `nc_multidrive_move_regression.hurl`'s note on the same gotcha). # ============================================================= @@ -50,12 +60,10 @@ ap_id: jsonpath "$.id" # Step 1 — Seed a 10-byte probe file: "0123456789". # ───────────────────────────────────────────────────────────── PUT {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe.txt +Content-Type: text/plain [BasicAuth] {{nc_username}}: {{nc_password}} -Content-Type: text/plain -``` -0123456789 -``` +`0123456789` HTTP 201 @@ -64,13 +72,11 @@ HTTP 201 # Step 2 — PATCH bytes 3-5 ("345") with "XYZ". # ───────────────────────────────────────────────────────────── PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe.txt -[BasicAuth] -{{nc_username}}: {{nc_password}} X-Update-Range: bytes=3-5 Content-Type: text/plain -``` -XYZ -``` +[BasicAuth] +{{nc_username}}: {{nc_password}} +`XYZ` HTTP 204 [Asserts] @@ -90,13 +96,11 @@ body == "012XYZ6789" # Step 3 — PATCH append. # ───────────────────────────────────────────────────────────── PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe.txt -[BasicAuth] -{{nc_username}}: {{nc_password}} X-Update-Range: append Content-Type: text/plain -``` -END -``` +[BasicAuth] +{{nc_username}}: {{nc_password}} +`END` HTTP 204 [Asserts] @@ -116,14 +120,12 @@ body == "012XYZ6789END" # Step 4 — Content-Range header on PATCH is rejected. # ───────────────────────────────────────────────────────────── PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe.txt -[BasicAuth] -{{nc_username}}: {{nc_password}} X-Update-Range: bytes=0-2 Content-Range: bytes 0-2/13 Content-Type: text/plain -``` -abc -``` +[BasicAuth] +{{nc_username}}: {{nc_password}} +`abc` HTTP 400 @@ -132,12 +134,10 @@ HTTP 400 # Step 5 — Missing X-Update-Range header. # ───────────────────────────────────────────────────────────── PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe.txt +Content-Type: text/plain [BasicAuth] {{nc_username}}: {{nc_password}} -Content-Type: text/plain -``` -abc -``` +`abc` HTTP 400 @@ -146,17 +146,42 @@ HTTP 400 # Step 6 — PATCH on a nonexistent file → 404. # ───────────────────────────────────────────────────────────── PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-does-not-exist.txt -[BasicAuth] -{{nc_username}}: {{nc_password}} X-Update-Range: append Content-Type: text/plain -``` -abc -``` +[BasicAuth] +{{nc_username}}: {{nc_password}} +`abc` HTTP 404 +# ───────────────────────────────────────────────────────────── +# Step 7 — PATCH on a directory → 409 Conflict (not 404). +# ───────────────────────────────────────────────────────────── +MKCOL {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe-dir/ +[BasicAuth] +{{nc_username}}: {{nc_password}} + +HTTP 201 + + +PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe-dir/ +X-Update-Range: bytes=0-2 +Content-Type: text/plain +[BasicAuth] +{{nc_username}}: {{nc_password}} +`NOP` + +HTTP 409 + + +DELETE {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-patch-probe-dir/ +[BasicAuth] +{{nc_username}}: {{nc_password}} + +HTTP 204 + + # ───────────────────────────────────────────────────────────── # Cleanup # ───────────────────────────────────────────────────────────── diff --git a/tests/api/nc_webdav_patch_consistency.hurl b/tests/api/nc_webdav_patch_consistency.hurl new file mode 100644 index 00000000..c4a722a0 --- /dev/null +++ b/tests/api/nc_webdav_patch_consistency.hurl @@ -0,0 +1,541 @@ +# ============================================================= +# OxiCloud — NextCloud PATCH data-consistency + authz/lock gaps +# ============================================================= +# `nc_webdav_patch.hurl` covers the PATCH contract on the NC surface. +# This file targets the specific gaps closed by the review-fix commit +# (see nextcloud/webdav_handler.rs::handle_patch): +# +# 1. AuthZ: the NC surface previously called `get_file_by_path` +# (which performs NO authorization check) with no follow-up +# `authz.require` at all — any caller with a valid app password +# could learn a file's size/ETag via PATCH's precondition/range +# responses regardless of their actual permission on that file. +# The fix added the same `Permission::Read` check the plain +# surface already had. That Read check is only an early +# existence-proof gate, though — the actual write a few lines +# later goes through `update_file_streaming_with_perms`, which +# independently requires `Permission::Update`. So the full +# permission chain for PATCH is: EDITOR (has Update) can PATCH; +# VIEWER (Read only, no Update) gets past the early gate but is +# still denied — anti-enum 404 — at the write step; a caller +# with NO grant at all can't even establish the composite-marker +# chroot. Tested via the multi-drive composite `{user}~{folder_id}` +# credential shape (see `nc_multidrive_move_regression.hurl` for +# the mechanism). +# 2. Cross-surface lock interop: a LOCK taken via the plain +# `/webdav/` surface now also blocks PATCH via `/remote.php/dav/` +# for the same file — proves the two surfaces share one lock +# store, not two independent ones. +# 3. Quota/507 via the NC surface (previously missing entirely — +# the fix added the same per-user quota check the plain surface +# already enforced), and the failed PATCH leaves the file intact. +# +# Self-contained: provisions its own throwaway users/drive so it can +# run alongside the rest of the suite. +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Setup — Admin JWT login. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "{{username}}", "password": "{{password}}" } + +HTTP 200 +[Captures] +admin_jwt: jsonpath "$.access_token" +admin_user_id: jsonpath "$.user.id" + + +# ═════════════════════════════════════════════════════════════ +# Part A — AuthZ: Editor can PATCH; Viewer (Read only) and a +# no-grant outsider both can't +# ═════════════════════════════════════════════════════════════ + + +# ───────────────────────────────────────────────────────────── +# Step A1 — Provision `ncpatch_editor` (will get EDITOR), +# `ncpatch_viewer` (will get VIEWER), and +# `ncpatch_outsider` (gets NO grant at all). +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/admin/users +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "username": "ncpatch_editor", + "password": "NcPatchEditorPwd1!", + "email": "ncpatch_editor@example.com", + "role": "user" +} + +HTTP 201 +[Captures] +editor_user_id: jsonpath "$.id" + +POST {{base_url}}/api/admin/users +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "username": "ncpatch_viewer", + "password": "NcPatchViewerPwd1!", + "email": "ncpatch_viewer@example.com", + "role": "user" +} + +HTTP 201 +[Captures] +viewer_user_id: jsonpath "$.id" + +POST {{base_url}}/api/admin/users +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "username": "ncpatch_outsider", + "password": "NcPatchOutsiderPwd1!", + "email": "ncpatch_outsider@example.com", + "role": "user" +} + +HTTP 201 +[Captures] +outsider_user_id: jsonpath "$.id" + + +# ───────────────────────────────────────────────────────────── +# Step A2 — Log all three in, mint an NC app password for each. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "ncpatch_editor", "password": "NcPatchEditorPwd1!" } + +HTTP 200 +[Captures] +editor_jwt: jsonpath "$.access_token" + +POST {{base_url}}/api/auth/app-passwords +Authorization: Bearer {{editor_jwt}} +Content-Type: application/json +{ "label": "nc_webdav_patch_consistency (editor)" } + +HTTP 200 +[Captures] +editor_nc_username: jsonpath "$.username" +editor_nc_password: jsonpath "$.password" +editor_ap_id: jsonpath "$.id" + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "ncpatch_viewer", "password": "NcPatchViewerPwd1!" } + +HTTP 200 +[Captures] +viewer_jwt: jsonpath "$.access_token" + +POST {{base_url}}/api/auth/app-passwords +Authorization: Bearer {{viewer_jwt}} +Content-Type: application/json +{ "label": "nc_webdav_patch_consistency (viewer)" } + +HTTP 200 +[Captures] +viewer_nc_username: jsonpath "$.username" +viewer_nc_password: jsonpath "$.password" +viewer_ap_id: jsonpath "$.id" + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "ncpatch_outsider", "password": "NcPatchOutsiderPwd1!" } + +HTTP 200 +[Captures] +outsider_jwt: jsonpath "$.access_token" + +POST {{base_url}}/api/auth/app-passwords +Authorization: Bearer {{outsider_jwt}} +Content-Type: application/json +{ "label": "nc_webdav_patch_consistency (outsider)" } + +HTTP 200 +[Captures] +outsider_nc_username: jsonpath "$.username" +outsider_nc_password: jsonpath "$.password" +outsider_ap_id: jsonpath "$.id" + + +# ───────────────────────────────────────────────────────────── +# Step A3 — Admin creates a shared drive, grants `ncpatch_editor` +# EDITOR (Read + Update) and `ncpatch_viewer` VIEWER +# (Read only). `ncpatch_outsider` gets no grant at all. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/drives +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "kind": "shared", + "name": "ncpatch-shared", + "owner": { "type": "user", "id": "{{admin_user_id}}" } +} + +HTTP 201 +[Captures] +shared_drive_id: jsonpath "$.id" +shared_root_id: jsonpath "$.root_folder_id" + + +POST {{base_url}}/api/grants +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{editor_user_id}}" }, + "resource": { "type": "drive", "id": "{{shared_drive_id}}" }, + "role": "editor" +} + +HTTP 201 + +POST {{base_url}}/api/grants +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "subject": { "type": "user", "id": "{{viewer_user_id}}" }, + "resource": { "type": "drive", "id": "{{shared_drive_id}}" }, + "role": "viewer" +} + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step A4 — Admin seeds a file in the shared drive via the plain +# WebDAV surface (`@drive//` scheme). +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/webdav/@drive/{{shared_drive_id}}/ncpatch-file.txt +Authorization: Bearer {{admin_jwt}} +Content-Type: text/plain +`0123456789` + +HTTP 201 + + +# ───────────────────────────────────────────────────────────── +# Step A5 — Bootstrap the composite BasicAuth usernames (Hurl's +# [BasicAuth] parser chokes on a literal `~` split across +# two templates — alias it via [Options] variable: first, +# same workaround as nc_multidrive_move_regression.hurl). +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/ready +[Options] +variable: nc_basic_editor={{editor_nc_username}}~{{shared_root_id}} + +HTTP 200 + +GET {{base_url}}/ready +[Options] +variable: nc_basic_viewer={{viewer_nc_username}}~{{shared_root_id}} + +HTTP 200 + +GET {{base_url}}/ready +[Options] +variable: nc_basic_outsider={{outsider_nc_username}}~{{shared_root_id}} + +HTTP 200 + + +# ───────────────────────────────────────────────────────────── +# Step A6 — EDITOR (has Update via the drive grant) CAN PATCH. +# This is the positive check: the fix's authz.require(Read) +# gate plus the write step's Update requirement must not +# accidentally lock out a legitimate Update-holder. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/remote.php/dav/files/{{nc_basic_editor}}/ncpatch-file.txt +X-Update-Range: bytes=0-2 +Content-Type: text/plain +[BasicAuth] +{{nc_basic_editor}}: {{editor_nc_password}} +`XYZ` + +HTTP 204 + + +GET {{base_url}}/remote.php/dav/files/{{nc_basic_editor}}/ncpatch-file.txt +[BasicAuth] +{{nc_basic_editor}}: {{editor_nc_password}} + +HTTP 200 +[Asserts] +body == "XYZ3456789" + + +# ───────────────────────────────────────────────────────────── +# Step A7 — VIEWER (has Read via the grant, but not Update) is +# denied → 404 anti-enum. The early authz.require(Read) +# the fix added is only an existence-proof gate; the +# actual write goes through `update_file_streaming_with_perms`, +# which independently requires Update. Before fixing the +# NC surface's error-mapping bug found via this test (see +# nextcloud/webdav_handler.rs's PATCH write-step error +# mapping), this denial leaked as a raw 500 instead of the +# anti-enum 404 the plain surface already gave. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/remote.php/dav/files/{{nc_basic_viewer}}/ncpatch-file.txt +X-Update-Range: bytes=0-2 +Content-Type: text/plain +[BasicAuth] +{{nc_basic_viewer}}: {{viewer_nc_password}} +`NOP` + +HTTP 404 + + +# ───────────────────────────────────────────────────────────── +# Step A8 — OUTSIDER (no grant at all on this drive) cannot reach +# the file — denied before PATCH's own logic ever runs. +# Accept the broader 4xx-non-2xx shape here since the +# denial may surface at the app-password/session boundary +# rather than the domain authz layer. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/remote.php/dav/files/{{nc_basic_outsider}}/ncpatch-file.txt +X-Update-Range: bytes=0-2 +Content-Type: text/plain +[BasicAuth] +{{nc_basic_outsider}}: {{outsider_nc_password}} +`NOP` + +HTTP * +[Asserts] +status >= 400 +status < 500 + + +# Cleanup Part A. +DELETE {{base_url}}/webdav/@drive/{{shared_drive_id}}/ncpatch-file.txt +Authorization: Bearer {{admin_jwt}} + +HTTP 204 + +DELETE {{base_url}}/api/auth/app-passwords/{{editor_ap_id}} +Authorization: Bearer {{editor_jwt}} +HTTP 200 + +DELETE {{base_url}}/api/auth/app-passwords/{{viewer_ap_id}} +Authorization: Bearer {{viewer_jwt}} +HTTP 200 + +DELETE {{base_url}}/api/auth/app-passwords/{{outsider_ap_id}} +Authorization: Bearer {{outsider_jwt}} +HTTP 200 + + +# ═════════════════════════════════════════════════════════════ +# Part B — Cross-surface lock interop +# ═════════════════════════════════════════════════════════════ + + +# ───────────────────────────────────────────────────────────── +# Step B1 — Mint admin's own NC app password (bare-username +# surface — admin's personal drive, same file tree as +# `/webdav/`). +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/app-passwords +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ "label": "nc_webdav_patch_consistency (lock interop)" } + +HTTP 200 +[Captures] +nc_username: jsonpath "$.username" +nc_password: jsonpath "$.password" +lock_ap_id: jsonpath "$.id" + + +# ───────────────────────────────────────────────────────────── +# Step B2 — Seed the file via the plain surface, LOCK it there. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/webdav/nc-lock-interop-probe.txt +Authorization: Bearer {{admin_jwt}} +Content-Type: text/plain +`0123456789` + +HTTP 201 + + +LOCK {{base_url}}/webdav/nc-lock-interop-probe.txt +Authorization: Bearer {{admin_jwt}} +Content-Type: application/xml; charset=utf-8 +``` + + + + + nc-lock-interop-test + +``` + +HTTP 200 +[Captures] +interop_lock_token: xpath "string(//*[local-name()='locktoken']/*[local-name()='href'])" + + +# ───────────────────────────────────────────────────────────── +# Step B3 — PATCH the SAME file via the NC surface, no lock token +# → 423. Pre-fix, the NC surface didn't consult the +# plain surface's lock store at all. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/remote.php/dav/files/{{nc_username}}/nc-lock-interop-probe.txt +X-Update-Range: bytes=0-2 +Content-Type: text/plain +[BasicAuth] +{{nc_username}}: {{nc_password}} +`NOP` + +HTTP 423 + + +# Release the lock via the plain surface so cleanup below works. +UNLOCK {{base_url}}/webdav/nc-lock-interop-probe.txt +Authorization: Bearer {{admin_jwt}} +Lock-Token: <{{interop_lock_token}}> + +HTTP 204 + + +# Cleanup Part B. +DELETE {{base_url}}/webdav/nc-lock-interop-probe.txt +Authorization: Bearer {{admin_jwt}} + +HTTP 204 + + +# ═════════════════════════════════════════════════════════════ +# Part C — Quota/507 via the NC surface leaves the file untouched +# ═════════════════════════════════════════════════════════════ + + +# ───────────────────────────────────────────────────────────── +# Step C1 — Provision `ncpatch_quota_owner` with a 50-byte quota. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/admin/users +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ + "username": "ncpatch_quota_owner", + "password": "NcPatchQuotaOwnerPwd1!", + "email": "ncpatch_quota_owner@example.com", + "role": "user" +} + +HTTP 201 +[Captures] +quota_owner_id: jsonpath "$.id" + + +PUT {{base_url}}/api/admin/users/{{quota_owner_id}}/quota +Authorization: Bearer {{admin_jwt}} +Content-Type: application/json +{ "quota_bytes": 50 } + +HTTP 200 + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "ncpatch_quota_owner", "password": "NcPatchQuotaOwnerPwd1!" } + +HTTP 200 +[Captures] +quota_owner_jwt: jsonpath "$.access_token" + +POST {{base_url}}/api/auth/app-passwords +Authorization: Bearer {{quota_owner_jwt}} +Content-Type: application/json +{ "label": "nc_webdav_patch_consistency (quota)" } + +HTTP 200 +[Captures] +quota_nc_username: jsonpath "$.username" +quota_nc_password: jsonpath "$.password" +quota_ap_id: jsonpath "$.id" + + +# ───────────────────────────────────────────────────────────── +# Step C2 — Seed a 10-byte file (under quota), then append past +# it → 507. File must come back unchanged. +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/remote.php/dav/files/{{quota_nc_username}}/nc-quota-probe.txt +Content-Type: text/plain +[BasicAuth] +{{quota_nc_username}}: {{quota_nc_password}} +`0123456789` + +HTTP 201 +[Captures] +quota_probe_etag: header "ETag" + + +PATCH {{base_url}}/remote.php/dav/files/{{quota_nc_username}}/nc-quota-probe.txt +X-Update-Range: append +Content-Type: text/plain +[BasicAuth] +{{quota_nc_username}}: {{quota_nc_password}} +`this-is-a-100-byte-ish-payload-that-blows-past-the-fifty-byte-quota-set-for-this-throwaway-user-abc` + +HTTP 507 + + +GET {{base_url}}/remote.php/dav/files/{{quota_nc_username}}/nc-quota-probe.txt +[BasicAuth] +{{quota_nc_username}}: {{quota_nc_password}} + +HTTP 200 +[Asserts] +body == "0123456789" +header "ETag" contains {{quota_probe_etag}} + + +# Cleanup Part C. +DELETE {{base_url}}/remote.php/dav/files/{{quota_nc_username}}/nc-quota-probe.txt +[BasicAuth] +{{quota_nc_username}}: {{quota_nc_password}} + +HTTP 204 + +DELETE {{base_url}}/api/auth/app-passwords/{{quota_ap_id}} +Authorization: Bearer {{quota_owner_jwt}} +HTTP 200 + +DELETE {{base_url}}/api/auth/app-passwords/{{lock_ap_id}} +Authorization: Bearer {{admin_jwt}} +HTTP 200 + + +# ═════════════════════════════════════════════════════════════ +# Teardown +# ═════════════════════════════════════════════════════════════ +DELETE {{base_url}}/api/admin/users/{{editor_user_id}} +Authorization: Bearer {{admin_jwt}} + +HTTP 200 + +DELETE {{base_url}}/api/admin/users/{{viewer_user_id}} +Authorization: Bearer {{admin_jwt}} + +HTTP 200 + +DELETE {{base_url}}/api/drives/{{shared_drive_id}} +Authorization: Bearer {{admin_jwt}} + +HTTP 204 + +DELETE {{base_url}}/api/admin/users/{{outsider_user_id}} +Authorization: Bearer {{admin_jwt}} + +HTTP 200 + +DELETE {{base_url}}/api/admin/users/{{quota_owner_id}} +Authorization: Bearer {{admin_jwt}} + +HTTP 200 diff --git a/tests/api/run.sh b/tests/api/run.sh index 862758be..345db6ff 100755 --- a/tests/api/run.sh +++ b/tests/api/run.sh @@ -205,6 +205,10 @@ hurl --variables-file "$API_DIR/test.env" --file-root "$REPO_ROOT/tests" --test "$API_DIR/webdav_protected_properties.hurl" \ "$API_DIR/webdav_quota_properties.hurl" \ "$API_DIR/nc_webdav_quota_properties.hurl" \ + "$API_DIR/webdav_patch.hurl" \ + "$API_DIR/nc_webdav_patch.hurl" \ + "$API_DIR/webdav_patch_consistency.hurl" \ + "$API_DIR/nc_webdav_patch_consistency.hurl" \ "$API_DIR/webdav_drive_root.hurl" \ "$API_DIR/webdav_permissions.hurl" \ "$API_DIR/webdav_nested_move_cascade.hurl" \ diff --git a/tests/api/webdav_patch.hurl b/tests/api/webdav_patch.hurl index c61fd49d..a1c72dd2 100644 --- a/tests/api/webdav_patch.hurl +++ b/tests/api/webdav_patch.hurl @@ -17,6 +17,18 @@ # 6. PATCH on a directory → 409. # 7. PATCH on a missing resource → 404. # 8. PATCH without X-Update-Range → 400. +# 9. If-None-Match precondition failure (tag matches current ETag) → 412. +# 10. If-Match with a WEAK (`W/`) form of the current ETag → 412 (RFC 7232 +# §3.1: If-Match requires a STRONG match; a weak validator in the +# request never satisfies it, even if the underlying tag value is +# identical — see `if_match_precondition_fails`). +# +# Hurl gotcha: a triple-backtick ``` multiline body appends a trailing +# `\n` the server counts as part of Content-Length — that silently +# breaks the exact `end - start + 1` span check on a byte-range PATCH. +# Plain-text bodies below use the single-backtick ONELINE string form +# (`` `text` ``) instead, which sends exactly the bytes between the +# backticks with no injected newline. # ============================================================= @@ -38,9 +50,7 @@ token: jsonpath "$.access_token" PUT {{base_url}}/webdav/patch-probe.txt Authorization: Bearer {{token}} Content-Type: text/plain -``` -0123456789 -``` +`0123456789` HTTP 201 [Captures] @@ -55,9 +65,7 @@ PATCH {{base_url}}/webdav/patch-probe.txt Authorization: Bearer {{token}} X-Update-Range: bytes=3-5 Content-Type: text/plain -``` -XYZ -``` +`XYZ` HTTP 204 [Asserts] @@ -79,11 +87,11 @@ PATCH {{base_url}}/webdav/patch-probe.txt Authorization: Bearer {{token}} X-Update-Range: append Content-Type: text/plain -``` --APPENDED -``` +`-APPENDED` HTTP 204 +[Captures] +current_etag: header "ETag" GET {{base_url}}/webdav/patch-probe.txt @@ -103,9 +111,7 @@ PATCH {{base_url}}/webdav/patch-probe.txt Authorization: Bearer {{token}} X-Update-Range: bytes=1000-1005 Content-Type: text/plain -``` -oops -``` +`oops` HTTP 416 @@ -118,9 +124,7 @@ Authorization: Bearer {{token}} X-Update-Range: bytes=0-2 If-Match: "not-the-real-etag" Content-Type: text/plain -``` -NOP -``` +`NOP` HTTP 412 @@ -149,9 +153,7 @@ PATCH {{base_url}}/webdav/patch-probe.txt Authorization: Bearer {{token}} X-Update-Range: bytes=0-2 Content-Type: text/plain -``` -NOP -``` +`NOP` HTTP 423 @@ -177,9 +179,7 @@ PATCH {{base_url}}/webdav/patch-probe-dir/ Authorization: Bearer {{token}} X-Update-Range: bytes=0-2 Content-Type: text/plain -``` -NOP -``` +`NOP` HTTP 409 @@ -191,9 +191,7 @@ PATCH {{base_url}}/webdav/patch-probe-does-not-exist.txt Authorization: Bearer {{token}} X-Update-Range: bytes=0-2 Content-Type: text/plain -``` -NOP -``` +`NOP` HTTP 404 @@ -204,13 +202,42 @@ HTTP 404 PATCH {{base_url}}/webdav/patch-probe.txt Authorization: Bearer {{token}} Content-Type: text/plain -``` -NOP -``` +`NOP` HTTP 400 +# ───────────────────────────────────────────────────────────── +# Step 11 — If-None-Match precondition failure: the header names the +# CURRENT ETag, so the "only if it does NOT match" condition +# is violated → 412. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/patch-probe.txt +Authorization: Bearer {{token}} +X-Update-Range: bytes=0-2 +If-None-Match: {{current_etag}} +Content-Type: text/plain +`NOP` + +HTTP 412 + + +# ───────────────────────────────────────────────────────────── +# Step 12 — If-Match with a WEAK form (`W/`) of the current ETag → 412. +# RFC 7232 §3.1 requires If-Match to STRONG-match; a request +# carrying a weak validator never satisfies it even when the +# underlying tag value is identical. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/patch-probe.txt +Authorization: Bearer {{token}} +X-Update-Range: bytes=0-2 +If-Match: W/{{current_etag}} +Content-Type: text/plain +`NOP` + +HTTP 412 + + # ───────────────────────────────────────────────────────────── # Cleanup # ───────────────────────────────────────────────────────────── diff --git a/tests/api/webdav_patch_consistency.hurl b/tests/api/webdav_patch_consistency.hurl new file mode 100644 index 00000000..e3842c87 --- /dev/null +++ b/tests/api/webdav_patch_consistency.hurl @@ -0,0 +1,322 @@ +# ============================================================= +# OxiCloud — WebDAV PATCH data-consistency chain (RFC 5789) +# ============================================================= +# `webdav_patch.hurl` covers the PATCH contract itself (ranges, append, +# preconditions, locks). This file chains multiple PATCHes against the +# SAME resource and asserts the server stays consistent afterward — +# the concern behind the review-fix commit that added quota +# enforcement, an ETag re-check, and a `direct_put_max_bytes` +# prefix/suffix accounting bug (see webdav_handler.rs::handle_patch). +# +# Coverage: +# 1. Sequential overlapping-range PATCHes on one file: each step's +# GET reflects the splice, and the ETag changes every time (no +# stale-tag reuse across writes). +# 2. Cross-protocol consistency: HEAD and PROPFIND report the same +# size/ETag as the GET right after the last PATCH. +# 3. Quota rejection (507) leaves the file BYTE-FOR-BYTE unchanged — +# the ingested blob is discarded before it's ever attached +# (`upload_ingest::discard_ingested`). +# 4. `direct_put_max_bytes` bounds only the EDIT span, not the whole +# file: a small edit on a file already bigger than the cap still +# succeeds, but an edit whose OWN body exceeds the cap still 413s. +# ============================================================= + + +# ───────────────────────────────────────────────────────────── +# Step 1 — Login, capture JWT +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "{{username}}", "password": "{{password}}" } + +HTTP 200 +[Captures] +token: jsonpath "$.access_token" + + +# ═════════════════════════════════════════════════════════════ +# Part A — Sequential overlapping PATCHes + cross-protocol check +# ═════════════════════════════════════════════════════════════ + + +# ───────────────────────────────────────────────────────────── +# Step 2 — PUT a 20-byte probe: "0123456789ABCDEFGHIJ" +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} +Content-Type: text/plain +`0123456789ABCDEFGHIJ` + +HTTP 201 +[Captures] +etag0: header "ETag" + + +# ───────────────────────────────────────────────────────────── +# Step 3 — Overwrite bytes 5-9 ("56789") with "XXXXX". +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} +X-Update-Range: bytes=5-9 +Content-Type: text/plain +`XXXXX` + +HTTP 204 +[Captures] +etag1: header "ETag" +[Asserts] +header "ETag" != {{etag0}} + + +GET {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +body == "01234XXXXXABCDEFGHIJ" + + +# ───────────────────────────────────────────────────────────── +# Step 4 — Overwrite bytes 10-14 ("ABCDE") with "YYYYY". +# Overlaps neither previous edit but chains off it — +# proves each PATCH sees the result of the last one, not +# a stale copy. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} +X-Update-Range: bytes=10-14 +Content-Type: text/plain +`YYYYY` + +HTTP 204 +[Captures] +etag2: header "ETag" +[Asserts] +header "ETag" != {{etag1}} + + +GET {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +body == "01234XXXXXYYYYYFGHIJ" + + +# ───────────────────────────────────────────────────────────── +# Step 5 — HEAD reports the same size/ETag as the last GET. +# ───────────────────────────────────────────────────────────── +HEAD {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +header "Content-Length" == "20" +# GET/HEAD/PROPFIND quote the ETag (`""`) while PUT/PATCH return +# it raw/unquoted (compare webdav_handler.rs's `handle_head` vs +# `handle_patch` response builders) — `contains` tolerates that +# formatting difference instead of asserting byte-for-byte equality. +header "ETag" contains {{etag2}} + + +# ───────────────────────────────────────────────────────────── +# Step 6 — PROPFIND (named getcontentlength/getetag) agrees with +# HEAD/GET — no drift between the WebDAV property layer +# and the plain-file read path. +# ───────────────────────────────────────────────────────────── +PROPFIND {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} +Depth: 0 +Content-Type: application/xml; charset=utf-8 +``` + + + + + + + +``` + +HTTP 207 +[Asserts] +xpath "number(//*[local-name()='getcontentlength'])" == 20 +xpath "string(//*[local-name()='getetag'])" contains {{etag2}} + + +# Cleanup Part A. +DELETE {{base_url}}/webdav/patch-consist-chain.txt +Authorization: Bearer {{token}} + +HTTP 204 + + +# ═════════════════════════════════════════════════════════════ +# Part B — Quota rejection leaves the file untouched +# ═════════════════════════════════════════════════════════════ +# Dedicated low-quota user so this doesn't cap the shared admin +# account used by the rest of the suite. + + +# ───────────────────────────────────────────────────────────── +# Step 7 — Provision `patch_quota_owner` with a 50-byte quota. +# ───────────────────────────────────────────────────────────── +POST {{base_url}}/api/admin/users +Authorization: Bearer {{token}} +Content-Type: application/json +{ + "username": "patch_quota_owner", + "password": "PatchQuotaOwnerPwd1!", + "email": "patch_quota_owner@example.com", + "role": "user" +} + +HTTP 201 +[Captures] +quota_owner_id: jsonpath "$.id" + + +PUT {{base_url}}/api/admin/users/{{quota_owner_id}}/quota +Authorization: Bearer {{token}} +Content-Type: application/json +{ "quota_bytes": 50 } + +HTTP 200 + + +POST {{base_url}}/api/auth/login +Content-Type: application/json +{ "username": "patch_quota_owner", "password": "PatchQuotaOwnerPwd1!" } + +HTTP 200 +[Captures] +quota_owner_token: jsonpath "$.access_token" + + +# ───────────────────────────────────────────────────────────── +# Step 8 — Seed a 10-byte file (well under the 50-byte quota). +# ───────────────────────────────────────────────────────────── +PUT {{base_url}}/webdav/patch-quota-probe.txt +Authorization: Bearer {{quota_owner_token}} +Content-Type: text/plain +`0123456789` + +HTTP 201 +[Captures] +quota_probe_etag: header "ETag" + + +# ───────────────────────────────────────────────────────────── +# Step 9 — Append enough bytes to push the file's new total size +# (110 bytes) well past the 50-byte quota → 507. The +# ingested blob is discarded before commit — the file +# must come back completely unchanged. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/patch-quota-probe.txt +Authorization: Bearer {{quota_owner_token}} +X-Update-Range: append +Content-Type: text/plain +`this-is-a-100-byte-ish-payload-that-blows-past-the-fifty-byte-quota-set-for-this-throwaway-user-abc` + +HTTP 507 + + +GET {{base_url}}/webdav/patch-quota-probe.txt +Authorization: Bearer {{quota_owner_token}} + +HTTP 200 +[Asserts] +body == "0123456789" +header "ETag" contains {{quota_probe_etag}} + + +# Cleanup Part B. +DELETE {{base_url}}/webdav/patch-quota-probe.txt +Authorization: Bearer {{quota_owner_token}} + +HTTP 204 + +DELETE {{base_url}}/api/admin/users/{{quota_owner_id}} +Authorization: Bearer {{token}} + +HTTP 200 + + +# ═════════════════════════════════════════════════════════════ +# Part C — direct_put_max_bytes bounds the EDIT, not the whole file +# ═════════════════════════════════════════════════════════════ +# `OXICLOUD_DIRECT_PUT_MAX_BYTES` (4 MiB) can't be exceeded by a +# direct PUT, so a file bigger than the cap must be seeded through +# the chunk-agnostic multipart upload endpoint instead. Reuses the +# 5 MiB all-zero fixture `run.sh` already generates for the chunk/ +# direct-PUT cap tests. + + +# ───────────────────────────────────────────────────────────── +# Step 10 — Resolve the home folder id, seed a 5 MiB file in it. +# ───────────────────────────────────────────────────────────── +GET {{base_url}}/api/folders +Authorization: Bearer {{token}} + +HTTP 200 +[Captures] +home_folder_id: jsonpath "$[0].id" + + +POST {{base_url}}/api/files/upload +Authorization: Bearer {{token}} +[MultipartFormData] +folder_id: {{home_folder_id}} +file: file,fixtures/chunk-over-cap-5mb.bin; application/octet-stream + +HTTP 201 +[Captures] +big_file_name: jsonpath "$.name" + + +# ───────────────────────────────────────────────────────────── +# Step 11 — A SMALL mid-file edit succeeds even though the file's +# total size (5 MiB) is already over the 4 MiB cap. +# Pre-fix, the cap comparison counted prefix+suffix+edit +# against the raw cap and would have wrongly 413'd any +# edit on a file this size; post-fix only the edit span +# itself is bounded. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/{{big_file_name}} +Authorization: Bearer {{token}} +X-Update-Range: bytes=100-104 +Content-Type: application/octet-stream +`PATCH` + +HTTP 204 + + +GET {{base_url}}/webdav/{{big_file_name}} +Authorization: Bearer {{token}} + +HTTP 200 +[Asserts] +body contains "PATCH" + + +# ───────────────────────────────────────────────────────────── +# Step 12 — An edit whose OWN body meets/exceeds the cap still +# 413s — the cap still bites real over-cap edits, this +# isn't a blanket bypass. Replaces the ENTIRE file (no +# prefix/suffix at all) with a 5 MiB body. +# ───────────────────────────────────────────────────────────── +PATCH {{base_url}}/webdav/{{big_file_name}} +Authorization: Bearer {{token}} +X-Update-Range: bytes=0-5242879 +Content-Type: application/octet-stream +file,fixtures/chunk-over-cap-5mb.bin; + +HTTP 413 + + +# Cleanup Part C. +DELETE {{base_url}}/webdav/{{big_file_name}} +Authorization: Bearer {{token}} + +HTTP 204