diff --git a/src/interfaces/api/handlers/webdav_handler.rs b/src/interfaces/api/handlers/webdav_handler.rs index e888b064..ff9d5718 100644 --- a/src/interfaces/api/handlers/webdav_handler.rs +++ b/src/interfaces/api/handlers/webdav_handler.rs @@ -658,6 +658,21 @@ async fn handle_proppatch( ) -> Result, AppError> { let _user = extract_user(&req)?; + // Active-lock guard (RFC 4918 §9.10.4): PROPPATCH writes properties, + // so a lock on the target must release them via `If:`. Captured + // before the body is consumed below so a rejected request doesn't + // even parse the XML. + let if_header_owned = req + .headers() + .get("If") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + if let Some(resp) = + enforce_native_lock(&state.webdav_lock_store, if_header_owned.as_deref(), &path) + { + return Ok(resp); + } + // Resolve the target resource type BEFORE consuming the body so // we can pick the correct href shape in the multi-status // response. RFC 4918 §5.2 + strict WebDAV-client parser rules @@ -1237,6 +1252,18 @@ async fn handle_delete( ) -> Result, AppError> { let user = extract_user(&req)?; + // Active-lock guard (RFC 4918 §9.10.4). + let if_header_owned = req + .headers() + .get("If") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + if let Some(resp) = + enforce_native_lock(&state.webdav_lock_store, if_header_owned.as_deref(), &path) + { + return Ok(resp); + } + // Get services from state let file_retrieval_service = &state.applications.file_retrieval_service; let file_management_service = &state.applications.file_management_service; @@ -1293,6 +1320,23 @@ async fn handle_move( let user = extract_user(&req)?; let source_path = path; + // Captured up front so a rejected MOVE doesn't run any DB work. + let if_header_owned = req + .headers() + .get("If") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + + // Active-lock guard on the SOURCE (RFC 4918 §9.10.4): the move + // removes the source resource, which counts as modifying it. + if let Some(resp) = enforce_native_lock( + &state.webdav_lock_store, + if_header_owned.as_deref(), + &source_path, + ) { + return Ok(resp); + } + // Get destination from Destination header let destination = req .headers() @@ -1321,6 +1365,17 @@ async fn handle_move( // SECURITY: reject path-traversal in destination reject_path_traversal(&destination_path)?; + // Destination lock guard: MOVE also creates/replaces a resource at + // the destination. If that path is locked, the same If: header must + // satisfy it. + if let Some(resp) = enforce_native_lock( + &state.webdav_lock_store, + if_header_owned.as_deref(), + &destination_path, + ) { + return Ok(resp); + } + // Get services from state let file_retrieval_service = &state.applications.file_retrieval_service; let file_management_service = &state.applications.file_management_service; @@ -1455,6 +1510,15 @@ async fn handle_copy( let user = extract_user(&req)?; let source_path = path; + // Captured up front (cheap; used below for the destination lock guard). + // COPY doesn't mutate the source, so no source lock check — only the + // destination needs to clear (RFC 4918 §9.10.4). + let if_header_owned = req + .headers() + .get("If") + .and_then(|v| v.to_str().ok()) + .map(|s| s.to_string()); + // Get destination from Destination header let destination = req .headers() @@ -1483,6 +1547,15 @@ async fn handle_copy( // SECURITY: reject path-traversal in destination reject_path_traversal(&destination_path)?; + // Active-lock guard on the destination (RFC 4918 §9.10.4). + if let Some(resp) = enforce_native_lock( + &state.webdav_lock_store, + if_header_owned.as_deref(), + &destination_path, + ) { + return Ok(resp); + } + // Get depth from Depth header let depth = req .headers() diff --git a/tests/webdav/test_native_webdav_lifecycle.sh b/tests/webdav/test_native_webdav_lifecycle.sh index 0951554a..48284d86 100755 --- a/tests/webdav/test_native_webdav_lifecycle.sh +++ b/tests/webdav/test_native_webdav_lifecycle.sh @@ -302,6 +302,74 @@ STATUS=$(dav_curl -o /dev/null -w "%{http_code}" -X PUT \ || fail "N2b: expected 204 No Content for PUT with correct lock token, got $STATUS" pass "N2b: PUT with matching If:() → 204" +# ───────────────────────────────────────────────────────────── +# N2c–N2f — Lock enforcement on the other mutator methods +# +# RFC 4918 §9.10.4: a lock binds every mutating method, not just +# PUT. The native handler's `enforce_native_lock` helper was +# designed to be called by handle_delete / handle_move / +# handle_copy / handle_proppatch as well — these tests prove the +# wire is in. Each case uses the n-locked.txt resource locked +# above and a `WITHOUT If:` request, expecting 423. Positive +# (with-token) coverage is implicit: the M-series above already +# exercises each method on unlocked resources and asserts the +# success codes, so a regression that hard-rejected every call +# would fail there. +# +# Order matters: each must run while the lock is still held, +# i.e. before N3 below releases it. +# ───────────────────────────────────────────────────────────── +echo " N2c: DELETE /webdav/n-locked.txt without If:() → 423" +STATUS=$(dav_curl -o /dev/null -w "%{http_code}" -X DELETE \ + "$DAV_BASE/n-locked.txt") +[[ "$STATUS" == "423" ]] \ + || fail "N2c: expected 423 Locked for DELETE on locked path without token, got $STATUS" +# The file must still be present after a rejected DELETE. +[[ "$(dav_curl -o /dev/null -w "%{http_code}" -X PROPFIND -H "Depth: 0" "$DAV_BASE/n-locked.txt")" == "207" ]] \ + || fail "N2c: file removed after rejected DELETE (423 was advisory only?)" +pass "N2c: DELETE on locked path without token → 423, resource preserved" + +echo " N2d: MOVE /webdav/n-locked.txt without If:() → 423 (source-side lock)" +STATUS=$(dav_curl -o /dev/null -w "%{http_code}" -X MOVE \ + -H "Destination: $DAV_BASE/n-locked-moved.txt" \ + "$DAV_BASE/n-locked.txt") +[[ "$STATUS" == "423" ]] \ + || fail "N2d: expected 423 Locked for MOVE on locked source without token, got $STATUS" +[[ "$(dav_curl -o /dev/null -w "%{http_code}" -X PROPFIND -H "Depth: 0" "$DAV_BASE/n-locked.txt")" == "207" ]] \ + || fail "N2d: source disappeared after rejected MOVE" +[[ "$(dav_curl -o /dev/null -w "%{http_code}" -X PROPFIND -H "Depth: 0" "$DAV_BASE/n-locked-moved.txt")" == "404" ]] \ + || fail "N2d: destination created after rejected MOVE" +pass "N2d: MOVE with locked source and no token → 423, no state mutated" + +echo " N2e: COPY into /webdav/n-locked.txt (locked destination) without If:() → 423" +# Set up a fresh unlocked source for the COPY. +dav_curl -o /dev/null -X PUT -H "Content-Type: text/plain" \ + --data-binary 'n2e copy source' \ + "$DAV_BASE/n2e-copy-src.txt" > /dev/null +STATUS=$(dav_curl -o /dev/null -w "%{http_code}" -X COPY \ + -H "Destination: $DAV_BASE/n-locked.txt" \ + "$DAV_BASE/n2e-copy-src.txt") +[[ "$STATUS" == "423" ]] \ + || fail "N2e: expected 423 Locked for COPY into locked destination without token, got $STATUS" +# The locked destination's content must not have been replaced. +BODY=$(dav_curl -s "$DAV_BASE/n-locked.txt") +[[ "$BODY" == "authorised update" ]] \ + || fail "N2e: locked destination's content was overwritten (got '$BODY')" +pass "N2e: COPY into locked destination without token → 423, target untouched" + +echo " N2f: PROPPATCH /webdav/n-locked.txt without If:() → 423" +PROPPATCH_BODY=' + + tampered +' +STATUS=$(dav_curl -o /dev/null -w "%{http_code}" -X PROPPATCH \ + -H "Content-Type: application/xml" \ + --data "$PROPPATCH_BODY" \ + "$DAV_BASE/n-locked.txt") +[[ "$STATUS" == "423" ]] \ + || fail "N2f: expected 423 Locked for PROPPATCH on locked path without token, got $STATUS" +pass "N2f: PROPPATCH on locked path without token → 423" + # ───────────────────────────────────────────────────────────── # N3 — UNLOCK with token → 204; subsequent PUT succeeds # ─────────────────────────────────────────────────────────────