implement restore for spaces trashbin

changelog/unreleased/spaces-trashbin.md

@@ -0,0 +1,5 @@
+Enhancement: Add spaces aware trash-bin API
+
+Added the webdav trash-bin endpoint for spaces.
+
+https://github.com/cs3org/reva/pull/2628

internal/http/services/owncloud/ocdav/copy.go

@@ -55,7 +55,10 @@ func (s *svc) handlePathCopy(w http.ResponseWriter, r *http.Request, ns string)
 	defer span.End()
 
 	src := path.Join(ns, r.URL.Path)
-	dst, err := extractDestination(r)
+
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
@@ -274,7 +277,9 @@ func (s *svc) handleSpacesCopy(w http.ResponseWriter, r *http.Request, spaceID s
 	ctx, span := rtrace.Provider.Tracer("reva").Start(r.Context(), "spaces_copy")
 	defer span.End()
 
-	dst, err := extractDestination(r)
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return

internal/http/services/owncloud/ocdav/move.go

@@ -42,7 +42,9 @@ func (s *svc) handlePathMove(w http.ResponseWriter, r *http.Request, ns string)
 	defer span.End()
 
 	srcPath := path.Join(ns, r.URL.Path)
-	dstPath, err := extractDestination(r)
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dstPath, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return
@@ -98,7 +100,9 @@ func (s *svc) handleSpacesMove(w http.ResponseWriter, r *http.Request, srcSpaceI
 	ctx, span := rtrace.Provider.Tracer("ocdav").Start(r.Context(), "spaces_move")
 	defer span.End()
 
-	dst, err := extractDestination(r)
+	dh := r.Header.Get(net.HeaderDestination)
+	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
+	dst, err := net.ParseDestination(baseURI, dh)
 	if err != nil {
 		w.WriteHeader(http.StatusBadRequest)
 		return

internal/http/services/owncloud/ocdav/net/net.go

@@ -20,8 +20,15 @@ package net
 
 import (
 	"fmt"
+	"net/url"
 	"regexp"
 	"strings"
+
+	"github.com/pkg/errors"
+)
+
+var (
+	ErrInvalidHeaderValue = errors.New("invalid value")
 )
 
 type ctxKey int
@@ -110,7 +117,7 @@ func ParseDepth(s string) (Depth, error) {
 	case DepthInfinity.String():
 		return DepthInfinity, nil
 	default:
-		return "", fmt.Errorf("invalid depth: %s", s)
+		return "", errors.Wrapf(ErrInvalidHeaderValue, "invalid depth: %s", s)
 	}
 }
 
@@ -122,7 +129,29 @@ func ParseOverwrite(s string) (bool, error) {
 		s = "T"
 	}
 	if s != "T" && s != "F" {
-		return false, fmt.Errorf("invalid overwrite: %s", s)
+		return false, errors.Wrapf(ErrInvalidHeaderValue, "invalid overwrite: %s", s)
 	}
 	return s == "T", nil
 }
+
+// ParseDestination parses the destination header value defined in https://datatracker.ietf.org/doc/html/rfc4918#section-10.3
+// The returned path will be relative to the given baseURI.
+func ParseDestination(baseURI, s string) (string, error) {
+	if s == "" {
+		return "", errors.Wrap(ErrInvalidHeaderValue, "destination header is empty")
+	}
+	dstURL, err := url.ParseRequestURI(s)
+	if err != nil {
+		return "", errors.Wrap(ErrInvalidHeaderValue, err.Error())
+	}
+
+	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
+	// TODO make request.php optional in destination header
+	// Strip the base URI from the destination. The destination might contain redirection prefixes which need to be handled
+	urlSplit := strings.Split(dstURL.Path, baseURI)
+	if len(urlSplit) != 2 {
+		return "", errors.Wrap(ErrInvalidHeaderValue, "destination path does not contain base URI")
+	}
+
+	return urlSplit[1], nil
+}

internal/http/services/owncloud/ocdav/net/net_test.go

@@ -103,4 +103,16 @@ var _ = Describe("Net", func() {
 		Entry("F", "F", true, false),
 		Entry("f", "f", true, false),
 		Entry("invalid", "invalid", false, false))
+
+	DescribeTable("TestParseDestination",
+		func(baseURI, v string, expectSuccess bool, expectedValue string) {
+			parsed, err := net.ParseDestination(baseURI, v)
+			Expect(err == nil).To(Equal(expectSuccess))
+			Expect(parsed).To(Equal(expectedValue))
+		},
+		Entry("invalid1", "", "", false, ""),
+		Entry("invalid2", "baseURI", "", false, ""),
+		Entry("invalid3", "", "/dest/path", false, ""),
+		Entry("invalid4", "/foo", "/dest/path", false, ""),
+		Entry("valid", "/foo", "https://example.com/foo/dest/path", true, "/dest/path"))
 })

internal/http/services/owncloud/ocdav/ocdav.go

@@ -21,7 +21,6 @@ package ocdav
 import (
 	"context"
 	"net/http"
-	"net/url"
 	"path"
 	"strings"
 	"time"
@@ -339,24 +338,3 @@ func addAccessHeaders(w http.ResponseWriter, r *http.Request) {
 		headers.Set("Strict-Transport-Security", "max-age=63072000")
 	}
 }
-
-func extractDestination(r *http.Request) (string, error) {
-	dstHeader := r.Header.Get(net.HeaderDestination)
-	if dstHeader == "" {
-		return "", errors.Wrap(errInvalidValue, "destination header is empty")
-	}
-	dstURL, err := url.ParseRequestURI(dstHeader)
-	if err != nil {
-		return "", errors.Wrap(errInvalidValue, err.Error())
-	}
-
-	baseURI := r.Context().Value(net.CtxKeyBaseURI).(string)
-	// TODO check if path is on same storage, return 502 on problems, see https://tools.ietf.org/html/rfc4918#section-9.9.4
-	// Strip the base URI from the destination. The destination might contain redirection prefixes which need to be handled
-	urlSplit := strings.Split(dstURL.Path, baseURI)
-	if len(urlSplit) != 2 {
-		return "", errors.Wrap(errInvalidValue, "destination path does not contain base URI")
-	}
-
-	return urlSplit[1], nil
-}

internal/http/services/owncloud/ocdav/ocdav_test.go

@@ -18,14 +18,9 @@
 package ocdav
 
 import (
-	"context"
-	"errors"
-	"net/http"
-	"net/http/httptest"
 	"testing"
 
 	providerv1beta1 "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
-	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/pkg/utils/resourceid"
 )
 
@@ -38,63 +33,6 @@ func TestWrapResourceID(t *testing.T) {
 	}
 }
 
-func TestExtractDestination(t *testing.T) {
-	expected := "/dst"
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-	request.Header.Set(net.HeaderDestination, "https://example.org/remote.php/dav/dst")
-
-	ctx := context.WithValue(context.Background(), net.CtxKeyBaseURI, "remote.php/dav")
-	destination, err := extractDestination(request.WithContext(ctx))
-	if err != nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if destination != expected {
-		t.Errorf("Extracted destination is not expected, got %s want %s", destination, expected)
-	}
-}
-
-func TestExtractDestinationWithoutHeader(t *testing.T) {
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-
-	_, err := extractDestination(request)
-	if err == nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if !errors.Is(err, errInvalidValue) {
-		t.Errorf("Expected error invalid value, got %s", err)
-	}
-}
-
-func TestExtractDestinationWithInvalidDestination(t *testing.T) {
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-	request.Header.Set(net.HeaderDestination, "://example.org/remote.php/dav/dst")
-	_, err := extractDestination(request)
-	if err == nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if !errors.Is(err, errInvalidValue) {
-		t.Errorf("Expected error invalid value, got %s", err)
-	}
-}
-
-func TestExtractDestinationWithDestinationWrongBaseURI(t *testing.T) {
-	request := httptest.NewRequest(http.MethodGet, "https://example.org/remote.php/dav/src", nil)
-	request.Header.Set(net.HeaderDestination, "https://example.org/remote.php/dav/dst")
-
-	ctx := context.WithValue(context.Background(), net.CtxKeyBaseURI, "remote.php/webdav")
-	_, err := extractDestination(request.WithContext(ctx))
-	if err == nil {
-		t.Errorf("Expected err to be nil got %s", err)
-	}
-
-	if !errors.Is(err, errInvalidValue) {
-		t.Errorf("Expected error invalid value, got %s", err)
-	}
-}
-
 func TestNameNotEmptyRule(t *testing.T) {
 	tests := map[string]bool{
 		"":      false,

internal/http/services/owncloud/ocdav/spaces.go

@@ -24,6 +24,7 @@ import (
 
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/errors"
+	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/net"
 	"github.com/cs3org/reva/v2/internal/http/services/owncloud/ocdav/propfind"
 	"github.com/cs3org/reva/v2/pkg/appctx"
 	"github.com/cs3org/reva/v2/pkg/rgrpc/todo/pool"
@@ -146,6 +147,9 @@ func (h *SpacesHandler) Handler(s *svc, trashbinHandler *TrashbinHandler) http.H
 }
 
 func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Request, s *svc, trashbinHandler *TrashbinHandler) {
+	ctx := r.Context()
+	log := appctx.GetLogger(ctx)
+
 	var spaceID string
 	spaceID, r.URL.Path = router.ShiftPath(r.URL.Path)
 	if spaceID == "" {
@@ -167,7 +171,27 @@ func (h *SpacesHandler) handleSpacesTrashbin(w http.ResponseWriter, r *http.Requ
 	switch r.Method {
 	case MethodPropfind:
 		trashbinHandler.listTrashbin(w, r, s, ref, path.Join(_trashbinPath, spaceID), key, r.URL.Path)
+	case MethodMove:
+		if key == "" {
+			http.Error(w, "501 Not implemented", http.StatusNotImplemented)
+			break
+		}
+		// find path in url relative to trash base
+		baseURI := ctx.Value(net.CtxKeyBaseURI).(string)
+		baseURI = path.Join(baseURI, spaceID)
+
+		dh := r.Header.Get(net.HeaderDestination)
+		dst, err := net.ParseDestination(baseURI, dh)
+		if err != nil {
+			w.WriteHeader(http.StatusBadRequest)
+			return
+		}
+
+		log.Debug().Str("key", key).Str("path", r.URL.Path).Str("dst", dst).Msg("spaces restore")
+		trashbinHandler.restore(w, r, s, ref, dst, key, r.URL.Path)
 	case http.MethodDelete:
 		trashbinHandler.delete(w, r, s, ref, key, r.URL.Path)
+	default:
+		http.Error(w, "501 Not implemented", http.StatusNotImplemented)
 	}
 }

internal/http/services/owncloud/ocdav/trashbin.go

@@ -25,7 +25,6 @@ import (
 	"fmt"
 	"net/http"
 	"path"
-	"path/filepath"
 	"strconv"
 	"strings"
 	"time"
@@ -38,7 +37,6 @@ import (
 	rtrace "github.com/cs3org/reva/v2/pkg/trace"
 	"github.com/cs3org/reva/v2/pkg/utils/resourceid"
 
-	userpb "github.com/cs3org/go-cs3apis/cs3/identity/user/v1beta1"
 	rpc "github.com/cs3org/go-cs3apis/cs3/rpc/v1beta1"
 	provider "github.com/cs3org/go-cs3apis/cs3/storage/provider/v1beta1"
 	"github.com/cs3org/reva/v2/pkg/appctx"
@@ -146,20 +144,16 @@ func (h *TrashbinHandler) Handler(s *svc) http.Handler {
 			// find path in url relative to trash base
 			trashBase := ctx.Value(net.CtxKeyBaseURI).(string)
 			baseURI := path.Join(path.Dir(trashBase), "files", username)
-			ctx = context.WithValue(ctx, net.CtxKeyBaseURI, baseURI)
-			r = r.WithContext(ctx)
 
-			// TODO make request.php optional in destination header
-			dst, err := extractDestination(r)
+			dh := r.Header.Get(net.HeaderDestination)
+			dst, err := net.ParseDestination(baseURI, dh)
 			if err != nil {
 				w.WriteHeader(http.StatusBadRequest)
 				return
 			}
-			dst = path.Join(basePath, dst)
 
 			log.Debug().Str("key", key).Str("dst", dst).Msg("restore")
-
-			h.restore(w, r, s, user, basePath, dst, key, r.URL.Path)
+			h.restore(w, r, s, ref, dst, key, r.URL.Path)
 		case http.MethodDelete:
 			h.delete(w, r, s, ref, key, r.URL.Path)
 		default:
@@ -444,7 +438,7 @@ func (h *TrashbinHandler) itemToPropResponse(ctx context.Context, s *svc, refBas
 	return &response, nil
 }
 
-func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, u *userpb.User, basePath, dst, key, itemPath string) {
+func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc, ref *provider.Reference, dst, key, itemPath string) {
 	ctx, span := rtrace.Provider.Tracer("trash-bin").Start(r.Context(), "restore")
 	defer span.End()
 
@@ -464,22 +458,11 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 		w.WriteHeader(http.StatusInternalServerError)
 		return
 	}
-	space, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, dst)
-	if err != nil {
-		sublog.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-	if rpcstatus.Code != rpc.Code_CODE_OK {
-		errors.HandleErrorStatus(&sublog, w, rpcstatus)
-		return
-	}
-	dstRef := spacelookup.MakeRelativeReference(space, dst, false)
 
-	dstStatReq := &provider.StatRequest{
-		Ref: dstRef,
-	}
+	dstRef := ref
+	dstRef.Path = utils.MakeRelativePath(dst)
 
+	dstStatReq := &provider.StatRequest{Ref: dstRef}
 	dstStatRes, err := client.Stat(ctx, dstStatReq)
 	if err != nil {
 		sublog.Error().Err(err).Msg("error sending grpc stat request")
@@ -495,9 +478,9 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 	// Restoring to a non-existent location is not supported by the WebDAV spec. The following block ensures the target
 	// restore location exists, and if it doesn't returns a conflict error code.
 	if dstStatRes.Status.Code == rpc.Code_CODE_NOT_FOUND && isNested(dst) {
-		parentStatReq := &provider.StatRequest{
-			Ref: spacelookup.MakeRelativeReference(space, filepath.Dir(dst), false),
-		}
+		parentRef := ref
+		parentRef.Path = utils.MakeRelativePath(path.Dir(dst))
+		parentStatReq := &provider.StatRequest{Ref: parentRef}
 
 		parentStatResponse, err := client.Stat(ctx, parentStatReq)
 		if err != nil {
@@ -543,22 +526,8 @@ func (h *TrashbinHandler) restore(w http.ResponseWriter, r *http.Request, s *svc
 		}
 	}
 
-	sourceSpace, rpcstatus, err := spacelookup.LookUpStorageSpaceForPath(ctx, client, basePath)
-	if err != nil {
-		sublog.Error().Err(err).Str("path", basePath).Msg("failed to look up storage space")
-		w.WriteHeader(http.StatusInternalServerError)
-		return
-	}
-	if rpcstatus.Code != rpc.Code_CODE_OK {
-		errors.HandleErrorStatus(&sublog, w, rpcstatus)
-		return
-	}
 	req := &provider.RestoreRecycleItemRequest{
-		// use the target path to find the storage provider
-		// this means we can only undelete on the same storage, not to a different folder
-		// use the key which is prefixed with the StoragePath to lookup the correct storage ...
-		// TODO currently limited to the home storage
-		Ref:        spacelookup.MakeRelativeReference(sourceSpace, basePath, false),
+		Ref:        ref,
 		Key:        path.Join(key, itemPath),
 		RestoreRef: dstRef,
 	}