Merge pull request #572 from EdouardVanbelle/feat/nextcloud-chrooted-drive

feat/nextcloud chrooted drive

src/interfaces/nextcloud/login_v2_handler.rs

@@ -332,11 +332,30 @@ async fn complete_flow(
             base_url = %base_url,
             "Login Flow v2: flow completed successfully"
         );
-        let nc_url = format!(
-            "nc://login/server:{}&user:{}&password:{}",
-            base_url, login_name, app_password
-        );
-        axum::response::Redirect::to(&nc_url).into_response()
+        // Redirect the browser to a visible success page. NC clients
+        // that use the LFv2 poll endpoint (the standard pattern) have
+        // already received the credentials server-to-server through
+        // `login_flow.complete()` above — they don't need any browser
+        // hand-off.
+        //
+        // We deliberately do NOT redirect to `nc://login/…` here:
+        //   1. Plain browsers can't follow it → the tab looks stuck
+        //      on the picker → user clicks Continue again → second
+        //      click hits an already-consumed flow token → ends up
+        //      on `/nextcloud/error?type=session-expired`.
+        //   2. NC desktop clients that pick it up while their poll
+        //      has already succeeded try to complete the flow a
+        //      second time, which fails validation ("Impossible de
+        //      valider la requête") — the poll session is fine, the
+        //      dialog is spurious noise.
+        //
+        // If a client ever needs a frontchannel `nc://` handoff
+        // (older NC releases, mobile), reintroduce the URL as a
+        // client-side-only fragment (`#target=…`) and add a manual
+        // "Open Nextcloud" fallback on the success page. Keep the
+        // credentials out of the query string either way — the query
+        // string reaches server access logs.
+        axum::response::Redirect::to("/nextcloud/success").into_response()
     } else {
         tracing::error!(
             user = %user.username,

src/interfaces/nextcloud/trashbin_handler.rs

@@ -102,8 +102,16 @@ async fn handle_propfind(
     let nc = state.nextcloud.as_ref();
     let file_id_svc = nc.map(|n| &n.file_ids);
 
+    // Emit hrefs with `session.raw_username` (composite `admin~<uuid>` on
+    // non-home drives), NOT `user.username` (bare `admin`). The
+    // `NcSession` extractor cross-checks the URL `{user}` segment
+    // against `raw_username` and 403s on mismatch (see
+    // `session.rs::from_request_parts`). Emitting the bare form here
+    // would make every follow-up MOVE/DELETE from a non-home client
+    // 403 before the handler runs — the composite-credential Hurl
+    // regression caught this (B5 in `nc_multidrive_move_regression`).
     let mut buf = Vec::new();
-    write_trashbin_multistatus(&mut buf, &items, &user.username, chroot, file_id_svc)
+    write_trashbin_multistatus(&mut buf, &items, &session.raw_username, chroot, file_id_svc)
         .await
         .map_err(|e| AppError::internal_error(format!("XML generation failed: {}", e)))?;
 
@@ -139,8 +147,17 @@ async fn handle_restore(
     // with 412 — there is no `Overwrite: T` workflow for trash restore in
     // either Sabre/DAV or the NC desktop client (a live file being
     // silently replaced by an undeleted one would be a footgun).
+    // Use `session.raw_username` (composite `admin~<drive-uuid>` on
+    // non-home drives) to strip the destination prefix, NOT
+    // `user.username` (bare `admin`). NC clients send `Destination:
+    // /remote.php/dav/files/{raw_username}/…`; passing the bare
+    // username would leave the `~<uuid>/` marker glued to the leading
+    // subpath segment and turn the collision-check into a lookup at
+    // a fabricated path. See `uploads_handler::handle_assemble` for
+    // the same fix in the chunked-upload MOVE.
     if let Some(dest_header) = dest_header
-        && let Some(dest_subpath) = extract_nc_subpath_from_dest(&dest_header, &user.username)
+        && let Some(dest_subpath) =
+            extract_nc_subpath_from_dest(&dest_header, &session.raw_username)
     {
         let dest_internal = nc_to_internal_path(chroot, &dest_subpath)?;
         let folder_service = &state.applications.folder_service;

src/interfaces/nextcloud/uploads_handler.rs

@@ -150,7 +150,20 @@ async fn handle_propfind_session(
         .map_err(|e| AppError::internal_error(format!("Failed to list chunks: {}", e)))?
         .ok_or_else(|| AppError::not_found("Upload session not found"))?;
 
-    let session_href = format!("/remote.php/dav/uploads/{}/{}/", user.username, upload_id);
+    // Href MUST use `session.raw_username` (composite `admin~<uuid>` on
+    // non-home drives), NOT `user.username` (bare `admin`). The
+    // `NcSession` extractor cross-checks the URL `{user}` segment
+    // against `raw_username` and 403s on mismatch — a composite-cred
+    // client that PROPFINDs, then MOVEs a chunk href back to us, would
+    // otherwise 403 at the extractor before any handler runs. Same
+    // fix shape as `trashbin_handler::handle_propfind` and
+    // `handle_assemble`'s destination-URL parsing. Storage-side keying
+    // stays on `user.username` — upload sessions are per-user, not
+    // per-drive.
+    let session_href = format!(
+        "/remote.php/dav/uploads/{}/{}/",
+        session.raw_username, upload_id
+    );
     let session_last_modified =
         chrono::DateTime::<chrono::Utc>::from_timestamp(listing.session_mtime as i64, 0)
             .unwrap_or_else(chrono::Utc::now)
@@ -176,7 +189,7 @@ async fn handle_propfind_session(
     for chunk in &listing.chunks {
         let chunk_href = format!(
             "/remote.php/dav/uploads/{}/{}/{}",
-            user.username, upload_id, chunk.name
+            session.raw_username, upload_id, chunk.name
         );
         let chunk_modified = chrono::DateTime::<chrono::Utc>::from_timestamp(chunk.mtime as i64, 0)
             .unwrap_or_else(chrono::Utc::now)
@@ -342,7 +355,18 @@ async fn handle_assemble(
         .and_then(|v| v.to_str().ok())
         .and_then(|v| v.parse::<i64>().ok());
 
-    let dest_subpath = extract_files_subpath(&destination, &user.username)
+    // Strip the destination URL prefix using the SESSION's raw username
+    // (`admin~<drive-uuid>` on non-home drives), NOT `user.username`
+    // (bare `admin`). NC clients send `Destination: /remote.php/dav/files/
+    // {raw_username}/…` — the URL user-segment mirrors the credential
+    // they authenticated with. Passing bare `admin` here strips only
+    // `admin/` from a `admin~<uuid>/…` destination, leaving the tilde
+    // marker glued to the leading path segment; the write then targets
+    // `<drive-root>/~<uuid>/…` and fails with a parent-folder lookup
+    // error. Matches `webdav_handler::handle_move`'s call to
+    // `extract_nc_subpath_from_dest(&destination, url_user)` where
+    // `url_user = &session.raw_username` (webdav_handler.rs:1177).
+    let dest_subpath = extract_files_subpath(&destination, &session.raw_username)
         .ok_or_else(|| AppError::bad_request("Invalid Destination URL"))?;
 
     // Stream the chunk parts, in order, straight into the CDC chunk store —