feat(server): admin folder move + recursive delete (file API)
Directory MOVE and DELETE were hard-rejected with 409 "not supported" for everyone, so a folder could never be renamed, relocated, or removed — even in admin mode. The browse menu offered Rename/Delete on folder rows, but they failed at the server. This is exactly the restructuring admin mode exists for (e.g. doing a layout migration by hand instead of a script). serveFileMove: a directory source is now allowed when the principal is an active admin (zddc.IsSubtreeAdmin) over BOTH the source subtree and the destination's parent — a root admin covers all; a subtree admin within scope. os.Rename relocates the whole subtree (bypassing the per-file WORM/ACL gates on its contents, which is the point), and a move into the directory's own descendant is refused (409). File moves are unchanged. serveFileDelete: a directory target is now allowed for an active admin over that subtree and removes it recursively (os.RemoveAll). Non-admins get 403. Both relax the trailing-slash guard (the browse client sends folder ops with a trailing slash) and decide file-vs-directory by stat. Directory ops skip the If-Match precondition (a directory carries no ETag). Recursive deletes are audited with a "(recursive)" marker. Non-admin directory ops now return 403 rather than the old blanket 409. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
c7ab633653
commit
894610d59e
2 changed files with 143 additions and 21 deletions
|
|
@ -497,10 +497,8 @@ func serveFileDelete(cfg config.Config, w http.ResponseWriter, r *http.Request)
|
|||
http.Error(w, msg, status)
|
||||
return
|
||||
}
|
||||
if strings.HasSuffix(cleanURL, "/") {
|
||||
http.Error(w, "DELETE must target a file, not a directory", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// (Directory vs file is decided by stat below; the client sends a folder
|
||||
// DELETE with a trailing slash. A directory delete is admin-gated.)
|
||||
|
||||
// Register rows are real files — a DELETE targets them directly with
|
||||
// the normal ACL gate. (Deleting an ssr/<party>.yaml de-registers the
|
||||
|
|
@ -517,7 +515,32 @@ func serveFileDelete(cfg config.Config, w http.ResponseWriter, r *http.Request)
|
|||
return
|
||||
}
|
||||
if info.IsDir() {
|
||||
http.Error(w, "Conflict — DELETE of directories is not supported", http.StatusConflict)
|
||||
// Directory delete is recursive (os.RemoveAll), which bypasses the
|
||||
// per-file WORM/delete gates protecting the contents — so it's
|
||||
// admin-only: an active admin over this subtree (a root admin, or a
|
||||
// subtree admin within scope). This is the "admin mode exists for
|
||||
// restructuring" capability.
|
||||
p := PrincipalFromContext(r)
|
||||
if !zddc.IsSubtreeAdmin(cfg.Root, abs, p) {
|
||||
http.Error(w, "Forbidden — deleting a directory requires admin authority over it", http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
if err := os.RemoveAll(abs); err != nil {
|
||||
auditFile(r, "delete", cleanURL+" (recursive)", http.StatusInternalServerError, 0, err)
|
||||
http.Error(w, "Internal Server Error", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
etagCacheM.Delete(abs)
|
||||
purgeConverted(abs)
|
||||
w.Header().Set("X-ZDDC-Source", "fileapi:delete-dir")
|
||||
w.WriteHeader(http.StatusNoContent)
|
||||
auditFile(r, "delete", cleanURL+" (recursive)", http.StatusNoContent, 0, nil)
|
||||
return
|
||||
}
|
||||
|
||||
// File delete: a trailing slash is a directory URL — reject the mismatch.
|
||||
if strings.HasSuffix(cleanURL, "/") {
|
||||
http.Error(w, "DELETE must target a file, not a directory", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -571,10 +594,9 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) {
|
|||
http.Error(w, msg, status)
|
||||
return
|
||||
}
|
||||
if strings.HasSuffix(srcURL, "/") {
|
||||
http.Error(w, "MOVE source must be a file path", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// (A trailing slash on src/dst signals a directory target; we no longer
|
||||
// reject it here — file-vs-directory is decided by stat below, and a
|
||||
// directory move is admin-gated.)
|
||||
|
||||
dstHeader := r.Header.Get(headerDestination)
|
||||
if dstHeader == "" {
|
||||
|
|
@ -590,10 +612,6 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) {
|
|||
http.Error(w, "destination: "+msg, status)
|
||||
return
|
||||
}
|
||||
if strings.HasSuffix(dstURL, "/") {
|
||||
http.Error(w, "MOVE destination must be a file path", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
// A move whose destination introduces a new party folder under a
|
||||
// party_source peer requires the party to be registered.
|
||||
if rejected, why, _ := partySourceGate(cfg.Root, dstAbs); rejected {
|
||||
|
|
@ -619,8 +637,28 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) {
|
|||
}
|
||||
return
|
||||
}
|
||||
if srcInfo.IsDir() {
|
||||
http.Error(w, "Conflict — MOVE of directories is not supported", http.StatusConflict)
|
||||
isDir := srcInfo.IsDir()
|
||||
if isDir {
|
||||
// Directory moves relocate the whole subtree with one os.Rename,
|
||||
// which sidesteps the per-file WORM/ACL gates protecting the
|
||||
// descendants — so they're admin-only: an active admin over BOTH the
|
||||
// source subtree and the destination's parent (a root admin covers
|
||||
// all; a subtree admin within their own scope). This is the "admin
|
||||
// mode exists for restructuring" capability.
|
||||
p := PrincipalFromContext(r)
|
||||
if !zddc.IsSubtreeAdmin(cfg.Root, srcAbs, p) ||
|
||||
!zddc.IsSubtreeAdmin(cfg.Root, filepath.Dir(dstAbs), p) {
|
||||
http.Error(w, "Forbidden — moving a directory requires admin authority over the source and destination", http.StatusForbidden)
|
||||
return
|
||||
}
|
||||
// Refuse moving a directory into itself or one of its descendants.
|
||||
if dstAbs == srcAbs || strings.HasPrefix(dstAbs, srcAbs+string(filepath.Separator)) {
|
||||
http.Error(w, "Conflict — cannot move a directory into itself", http.StatusConflict)
|
||||
return
|
||||
}
|
||||
} else if strings.HasSuffix(dstURL, "/") {
|
||||
// A file move must target a file path, not a directory URL.
|
||||
http.Error(w, "destination: MOVE of a file must target a file path", http.StatusBadRequest)
|
||||
return
|
||||
}
|
||||
|
||||
|
|
@ -643,8 +681,12 @@ func serveFileMove(cfg config.Config, w http.ResponseWriter, r *http.Request) {
|
|||
if !authorizeAction(cfg, w, r, dstAbs, dstURL, policy.ActionCreate) {
|
||||
return
|
||||
}
|
||||
if !checkIfMatch(w, r, srcAbs) {
|
||||
return
|
||||
// If-Match concurrency applies to the source bytes — only meaningful for
|
||||
// a file. A directory carries no ETag, so skip the precondition.
|
||||
if !isDir {
|
||||
if !checkIfMatch(w, r, srcAbs) {
|
||||
return
|
||||
}
|
||||
}
|
||||
|
||||
// Ensure destination's canonical ancestors are created (with auto-own
|
||||
|
|
|
|||
|
|
@ -284,11 +284,34 @@ func TestFileAPI_DeleteMissing404(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
func TestFileAPI_DeleteDirectoryConflict(t *testing.T) {
|
||||
// Directory delete is admin-only and recursive. A non-admin (elevated but not
|
||||
// named in admins:) is forbidden; an admin recursively removes the subtree.
|
||||
func TestFileAPI_DeleteDirectoryNonAdminForbidden(t *testing.T) {
|
||||
_, do, _ := fileAPITestSetup(t, []string{"Incoming/sub"}, nil)
|
||||
rec := do(http.MethodDelete, "/Incoming/sub", "alice@example.com", nil, nil)
|
||||
if rec.Code != http.StatusConflict {
|
||||
t.Fatalf("want 409, got %d: %s", rec.Code, rec.Body.String())
|
||||
rec := do(http.MethodDelete, "/Incoming/sub/", "alice@example.com", nil, nil)
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("want 403, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileAPI_DeleteDirectoryAdminRecursive(t *testing.T) {
|
||||
_, do, root := fileAPITestSetup(t, []string{"Incoming/sub"}, map[string]string{
|
||||
"Incoming/sub/a.txt": "one",
|
||||
"Incoming/sub/deep/b.txt": "two",
|
||||
})
|
||||
// Promote alice to root admin.
|
||||
if err := os.WriteFile(filepath.Join(root, ".zddc"),
|
||||
[]byte("acl:\n permissions:\n \"*@example.com\": rwcd\nadmins:\n - alice@example.com\n"), 0o644); err != nil {
|
||||
t.Fatalf("rewrite root .zddc: %v", err)
|
||||
}
|
||||
zddc.InvalidateCache(root)
|
||||
|
||||
rec := do(http.MethodDelete, "/Incoming/sub/", "alice@example.com", nil, nil)
|
||||
if rec.Code != http.StatusNoContent {
|
||||
t.Fatalf("want 204, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(root, "Incoming/sub")); !os.IsNotExist(err) {
|
||||
t.Fatalf("dir should be gone recursively, err=%v", err)
|
||||
}
|
||||
}
|
||||
|
||||
|
|
@ -319,6 +342,63 @@ func TestFileAPI_MoveRenames(t *testing.T) {
|
|||
}
|
||||
}
|
||||
|
||||
// Directory move is admin-only and relocates the whole subtree. A non-admin
|
||||
// (elevated but not in admins:) is forbidden; an admin renames/relocates it.
|
||||
func TestFileAPI_MoveDirectoryNonAdminForbidden(t *testing.T) {
|
||||
_, do, _ := fileAPITestSetup(t, []string{"Docs/sub"}, nil)
|
||||
rec := do(http.MethodPost, "/Docs/sub/", "alice@example.com", nil, map[string]string{
|
||||
"X-ZDDC-Op": "move",
|
||||
"X-ZDDC-Destination": "/Docs/renamed/",
|
||||
})
|
||||
if rec.Code != http.StatusForbidden {
|
||||
t.Fatalf("want 403, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileAPI_MoveDirectoryAdmin(t *testing.T) {
|
||||
_, do, root := fileAPITestSetup(t, []string{"Docs/sub"}, map[string]string{
|
||||
"Docs/sub/a.txt": "x",
|
||||
"Docs/sub/deep/b.txt": "y",
|
||||
})
|
||||
if err := os.WriteFile(filepath.Join(root, ".zddc"),
|
||||
[]byte("acl:\n permissions:\n \"*@example.com\": rwcd\nadmins:\n - alice@example.com\n"), 0o644); err != nil {
|
||||
t.Fatalf("rewrite root .zddc: %v", err)
|
||||
}
|
||||
zddc.InvalidateCache(root)
|
||||
|
||||
rec := do(http.MethodPost, "/Docs/sub/", "alice@example.com", nil, map[string]string{
|
||||
"X-ZDDC-Op": "move",
|
||||
"X-ZDDC-Destination": "/Docs/renamed/",
|
||||
})
|
||||
if rec.Code != http.StatusOK {
|
||||
t.Fatalf("want 200, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
if _, err := os.Stat(filepath.Join(root, "Docs/sub")); !os.IsNotExist(err) {
|
||||
t.Fatalf("source dir should be gone, err=%v", err)
|
||||
}
|
||||
if b, err := os.ReadFile(filepath.Join(root, "Docs/renamed/deep/b.txt")); err != nil || string(b) != "y" {
|
||||
t.Fatalf("moved subtree content missing: b=%q err=%v", b, err)
|
||||
}
|
||||
}
|
||||
|
||||
// Refuse moving a directory into itself or a descendant (would orphan the tree).
|
||||
func TestFileAPI_MoveDirectoryIntoItself(t *testing.T) {
|
||||
_, do, root := fileAPITestSetup(t, []string{"Docs/sub"}, nil)
|
||||
if err := os.WriteFile(filepath.Join(root, ".zddc"),
|
||||
[]byte("acl:\n permissions:\n \"*@example.com\": rwcd\nadmins:\n - alice@example.com\n"), 0o644); err != nil {
|
||||
t.Fatalf("rewrite root .zddc: %v", err)
|
||||
}
|
||||
zddc.InvalidateCache(root)
|
||||
|
||||
rec := do(http.MethodPost, "/Docs/sub/", "alice@example.com", nil, map[string]string{
|
||||
"X-ZDDC-Op": "move",
|
||||
"X-ZDDC-Destination": "/Docs/sub/inner/",
|
||||
})
|
||||
if rec.Code != http.StatusConflict {
|
||||
t.Fatalf("want 409, got %d: %s", rec.Code, rec.Body.String())
|
||||
}
|
||||
}
|
||||
|
||||
func TestFileAPI_MoveDestinationExistsConflict(t *testing.T) {
|
||||
_, do, _ := fileAPITestSetup(t, nil, map[string]string{
|
||||
"Incoming/a.txt": "a",
|
||||
|
|
|
|||
Loading…
Reference in a new issue