From d192df5f48005f368d081963739d3b2aac8454ff Mon Sep 17 00:00:00 2001 From: David Christofas Date: Thu, 10 Mar 2022 16:45:25 +0100 Subject: [PATCH] implement restore for spaces trashbin --- changelog/unreleased/spaces-trashbin.md | 5 ++ internal/http/services/owncloud/ocdav/copy.go | 9 ++- internal/http/services/owncloud/ocdav/move.go | 8 ++- .../http/services/owncloud/ocdav/net/net.go | 34 +++++++++- .../services/owncloud/ocdav/net/net_test.go | 12 ++++ .../http/services/owncloud/ocdav/ocdav.go | 25 -------- .../services/owncloud/ocdav/ocdav_test.go | 62 ------------------- .../http/services/owncloud/ocdav/spaces.go | 44 +++++++++---- .../http/services/owncloud/ocdav/trashbin.go | 53 ++++------------ 9 files changed, 106 insertions(+), 146 deletions(-) create mode 100644 changelog/unreleased/spaces-trashbin.md diff --git a/changelog/unreleased/spaces-trashbin.md b/changelog/unreleased/spaces-trashbin.md new file mode 100644 index 0000000000..1518f72edc --- /dev/null +++ b/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 diff --git a/internal/http/services/owncloud/ocdav/copy.go b/internal/http/services/owncloud/ocdav/copy.go index 9363c2d3bc..51ea05b438 100644 --- a/internal/http/services/owncloud/ocdav/copy.go +++ b/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 diff --git a/internal/http/services/owncloud/ocdav/move.go b/internal/http/services/owncloud/ocdav/move.go index 7b0bb71eae..0146cd5fa8 100644 --- a/internal/http/services/owncloud/ocdav/move.go +++ b/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 diff --git a/internal/http/services/owncloud/ocdav/net/net.go b/internal/http/services/owncloud/ocdav/net/net.go index 6c4af82287..95a772a604 100644 --- a/internal/http/services/owncloud/ocdav/net/net.go +++ b/internal/http/services/owncloud/ocdav/net/net.go @@ -20,8 +20,16 @@ package net import ( "fmt" + "net/url" "regexp" "strings" + + "github.com/pkg/errors" +) + +var ( + // ErrInvalidHeaderValue defines an error which can occure when trying to parse a header value. + ErrInvalidHeaderValue = errors.New("invalid value") ) type ctxKey int @@ -110,7 +118,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 +130,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 +} diff --git a/internal/http/services/owncloud/ocdav/net/net_test.go b/internal/http/services/owncloud/ocdav/net/net_test.go index 2d289fa84a..779cf7e2f4 100644 --- a/internal/http/services/owncloud/ocdav/net/net_test.go +++ b/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")) }) diff --git a/internal/http/services/owncloud/ocdav/ocdav.go b/internal/http/services/owncloud/ocdav/ocdav.go index 1b6594990e..b5e32daf9f 100644 --- a/internal/http/services/owncloud/ocdav/ocdav.go +++ b/internal/http/services/owncloud/ocdav/ocdav.go @@ -21,7 +21,6 @@ package ocdav import ( "context" "net/http" - "net/url" "path" "strings" "time" @@ -42,13 +41,10 @@ import ( "github.com/cs3org/reva/v2/pkg/storage/favorite/registry" "github.com/cs3org/reva/v2/pkg/storage/utils/templates" "github.com/mitchellh/mapstructure" - "github.com/pkg/errors" "github.com/rs/zerolog" ) var ( - errInvalidValue = errors.New("invalid value") - nameRules = [...]nameRule{ nameNotEmpty{}, nameDoesNotContain{chars: "\f\r\n\\"}, @@ -339,24 +335,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 -} diff --git a/internal/http/services/owncloud/ocdav/ocdav_test.go b/internal/http/services/owncloud/ocdav/ocdav_test.go index d4b3f11014..13f47f4cc3 100644 --- a/internal/http/services/owncloud/ocdav/ocdav_test.go +++ b/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, diff --git a/internal/http/services/owncloud/ocdav/spaces.go b/internal/http/services/owncloud/ocdav/spaces.go index db0333e482..e803431e8d 100644 --- a/internal/http/services/owncloud/ocdav/spaces.go +++ b/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" @@ -56,23 +57,21 @@ func (h *SpacesHandler) Handler(s *svc, trashbinHandler *TrashbinHandler) http.H return } - var p string - p, r.URL.Path = router.ShiftPath(r.URL.Path) - - var spaceID string - if p == _trashbinPath { - h.handleSpacesTrashbin(w, r, s, trashbinHandler) + var segment string + segment, r.URL.Path = router.ShiftPath(r.URL.Path) + if segment == "" { + // listing is disabled, no auth will change that + w.WriteHeader(http.StatusMethodNotAllowed) return - } else { - spaceID = p } - if spaceID == "" { - // listing is disabled, no auth will change that - w.WriteHeader(http.StatusMethodNotAllowed) + if segment == _trashbinPath { + h.handleSpacesTrashbin(w, r, s, trashbinHandler) return } + spaceID := segment + switch r.Method { case MethodPropfind: p := propfind.NewHandler(config.PublicURL, func() (propfind.GatewayClient, error) { @@ -146,6 +145,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 +169,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) } } diff --git a/internal/http/services/owncloud/ocdav/trashbin.go b/internal/http/services/owncloud/ocdav/trashbin.go index 98557a48de..2aab1c7440 100644 --- a/internal/http/services/owncloud/ocdav/trashbin.go +++ b/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, }